-
Notifications
You must be signed in to change notification settings - Fork 44
Set Demo and Utility RPATHs to match system defaults
#291
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Marius Behrens <marius.behrens@synopsys.com>
Signed-off-by: Marius Behrens <marius.behrens@synopsys.com>
Signed-off-by: Marius Behrens <marius.behrens@synopsys.com>
Signed-off-by: Marius Behrens <marius.behrens@synopsys.com>
a67cf8a to
2fe1585
Compare
RPATHs to match sytem defaultsRPATHs to match system defaults
VJanKraemer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me and congratulations on your first PR 🥳
Please squash the commits before merging and I think the PR title would also be a good, descriptive commit msg header. Any additional info should be in the msg body.
| FOLDER "Utilities" | ||
| RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/$<CONFIG>" | ||
| INSTALL_RPATH "$ORIGIN/../lib" | ||
| INSTALL_RPATH "$ORIGIN/../${CMAKE_INSTALL_LIBDIR}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question that occurred to me looking at this again: Is there a reason the sil-kit-monitor does not have the trailing $ORIGIN that the other executables have? Or maybe the other way around; what is the reason that all the other executables are installed with a trailing $ORIGIN in the RPATH?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was before my time, so I don't know the exact reason, but the whole RPATH shenanigans are happening so we/users can take the SIL Kit Zip distributions and just copy them wherever on the target system and as long as the folder hierarchy of the zip is kept intact, everything works. The trailing $ORIGIN might be a remnant from earlier distribution models, but afaik it is not needed in the current distribution model. Can't be sure though to change that without breaking some users workflow.
To be frank I'd rather get rid of the whole rpath setting by default via CMakeLists.txt and put it specifically into the CMake cmdline/scripts that build the ZIP distributions. But that is a bigger discussion to be had, so for the moment I am just glad someone fixed it for non Debian derived Linux distributions :)
Description
When a SIL Kit installation is built, the system default is used to name the lib-directory in the install hierarchy. On certain Linux distributions (Alma Linux, for instance) this default lib-directory is
lib64instead oflib. This results in the SIL Kit Demos and Utilities not being able to findlibSilKit.sobecause their RPATH is hard-coded to uselib.This PR proposes setting these
RPATHs to match the naming scheme used when creating the SIL Kit installation (as defined by theGNUInstallDirsCMake module).Developer checklist (address before review)