Skip to content

Conversation

guitargeek
Copy link
Contributor

@guitargeek guitargeek commented Aug 1, 2025

When using the rpath=ON option, the global CMake variable CMAKE_INSTALL_RPATH is mutated, which is problematic in several corner cases.

Furthermore, the path is set to match the install tree, resulting in nonsensical relative rpath for the build tree if the directory structure is different (e.g. because of gnuinstall=ON).

This commit suggests to append the rpaths at the target property level, and also set the rpaths for the build tree separately so they make sense.

New ROOT macros are introduced for this rpath setting, which are also used for the PyROOT libraries, which also need custom rpaths.

Closes #19327 and #19134.

Here are some rpaths for relevant libraries and executables with gnuinstall=ON and rpath=ON, before and after this PR:

root_install/bin/root
 - before: $ORIGIN:$ORIGIN/../lib/root
 - after : $ORIGIN/../lib/root

root_build/bin/root
 - before: :::::::::::::::::::::::::::
 - after : :::::::::::::::::::

root_install/lib/root/libThread.so
 - before: $ORIGIN:$ORIGIN/../lib/root
 - after : $ORIGIN/

root_build/lib/libThread.so
 - before: /home/rembserj/code/root/root_build/lib:
 - after : /home/rembserj/code/root/root_build/lib:

root_install/site-packages/libcppyy.so
 - before: $ORIGIN/../lib/root
 - after : $ORIGIN/../lib/root

root_build/lib/libcppyy.so
 - before: $ORIGIN/:/home/rembserj/code/root/root_build/lib:
 - after : /home/rembserj/code/root/root_build/lib:

root_install/site-packages/ROOT/libROOTPythonizations.so
 - before: $ORIGIN/../../lib/root
 - after : $ORIGIN/../../lib/root

root_build/lib/ROOT/libROOTPythonizations.so
 - before: $ORIGIN/../:/home/rembserj/code/root/root_build/lib:
 - after : /home/rembserj/code/root/root_build/lib:

root_build/math/mathcore/test/libTrackMathCoreUnitDict.so
 - before: /home/rembserj/code/root/root_build/lib
 - after : /home/rembserj/code/root/root_build/lib

Copy link

github-actions bot commented Aug 1, 2025

Test Results

    19 files      19 suites   3d 4h 23m 44s ⏱️
 3 220 tests  3 220 ✅ 0 💤 0 ❌
59 579 runs  59 579 ✅ 0 💤 0 ❌

Results for commit 7af2b1d.

♻️ This comment has been updated with latest results.

Copy link
Member

@hageboeck hageboeck left a comment

Choose a reason for hiding this comment

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

LGTM, but I added a few more suggestions.

When using the `rpath=ON` option, the global CMake variable
`CMAKE_INSTALL_RPATH` is mutated, which is problematic in several corner
cases.

This commit suggests to append the rpaths at the target property level,
and also set the rpaths for the build tree separately so they make
sense.

A new ROOT macro is introduced for this rpath setting, which are also
used for the PyROOT libraries, which also need custom rpaths.

Closes root-project#19327 and root-project#19134.
Since ROOT 6.36, the relative `RPATH` to the main ROOT libraries is only
appended to the existing path, and not replacing the path that is e.g.
passed by the user via `CMAKE_INSTALL_RPATH`. Therefore, there is no
problematic side effect of `rpath=ON` anymore. We can drop this option
and always append to the runpath.

This makes the ROOT configuration less confusing, also because from the
option name "rpath", it is not clear what it did.
@guitargeek guitargeek merged commit 26d24de into root-project:master Aug 4, 2025
46 of 48 checks passed
@guitargeek guitargeek deleted the rpath_issues branch August 4, 2025 12:18
@hahnjo
Copy link
Member

hahnjo commented Aug 5, 2025

I think this PR results in a breaking change that root-config --libs no longer adds -Wl,-rpath, which was quite convenient for building quick test programs...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug clean build Ask CI to do non-incremental build on PR in:Build System
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build Fails on MacOS (ARM, 15)
3 participants