Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set RPATH for the loader #550

Merged
merged 3 commits into from
Sep 10, 2024
Merged

Set RPATH for the loader #550

merged 3 commits into from
Sep 10, 2024

Conversation

al42and
Copy link
Contributor

@al42and al42and commented Aug 7, 2024

Description

Having the user manage LD_LIBRARY_PATH is sub-optimal. It is mentioned three times in the docs, which suggest it is a common issue. By setting the RUNPATH on the loader library we can make dlopen automagically find the right libraries.

As a downside, we won't be able to point the loader to another oneMKL installation. I'd consider it more of an upside since it prevents accidentally mixing up two versions, but perhaps someone was relying on it?

Only tested on Linux, but official oneAPI plugins only support Linux, so 🤷

Checklist

All Submissions

  • Do all unit tests pass locally? Attach a log.
  • Have you formatted the code using clang-format?
    • Not touching the code

New interfaces

  • Have you provided motivation for adding a new feature as part of RFC and
    it was accepted? # (RFC)
  • What version of oneAPI spec the interface is targeted?
  • Complete New features checklist

New features

  • Have you provided motivation for adding a new feature?
  • Have you added relevant tests?

Bug fixes

  • Have you added relevant regression tests?
  • Have you included information on how to reproduce the issue (either in a
    GitHub issue or in this PR)?

Copy link
Contributor

@Rbiessy Rbiessy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, I think this is much better!

@Rbiessy Rbiessy requested a review from a team August 19, 2024 14:33
Copy link
Contributor

@mkrainiuk mkrainiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for fixing this problem!

src/CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@Rbiessy Rbiessy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Rbiessy Rbiessy merged commit 4a9961d into uxlfoundation:develop Sep 10, 2024
8 checks passed
@al42and al42and deleted the aa-rpath branch September 10, 2024 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants