[roottest] Re-introduce LD_LIBRARY_PATH to macro tests. #20833
Conversation
cmake/modules/RootMacros.cmake
Outdated
| # the library, which will itself load all its dependencies (e.g. libCore, libHist, etc). However, | ||
| # since the macro library doesn't contain RUNPATHs, this might load /usr/lib64/libCore.so if ROOT is installed, | ||
| # or load another ROOT from LD_LIBRARY_PATH. This leads to two libCore being loaded, messing up all TClassTables, | ||
| # and possibly leading to ABI breakages. |
There was a problem hiding this comment.
Can we also write in the comment that a possible alternative would be to embed the RUNPATHS in the macro libraries compiled with ACLiC, maybe even as a TODO? I think that would be the more sustainable solution once effort for that is available. I can already see the problems ahead with people doing pip install root on lxplus, where there is also ROOT installed on the system (that's also a big reason why I try to get rid of relying on environment variables where we can).
There was a problem hiding this comment.
OK, maybe I found a way to have a working RUNPATH in all ACLiC .so. The tests are running locally ...
guitargeek
left a comment
guitargeek
left a comment
There was a problem hiding this comment.
LGTM! It would just be nice to add some comment avoid the a possible more future-proof solution as a TODO, so we maybe attract effort to implement RUNPATH embedding in ACLiC.
This addresses a problem caused by 3161321, where LD_LIBRARY_PATH was removed from roottest. It seems to be needed for libraries resulting from compiling a macro. When ROOT is installed in the system, or when an LD_LIBRARY_PATH is set such that a different ROOT installation is visible, compiled macros can fail. The sequence of events is as follows: - roottest compiles a macro into an .so - In a subsequent test, that .so is loaded. - The dynamic loader will try to load the dependencies of that .so, e.g. libROOTVecOps.so, libCore.so, ... Given that the library being opened (the .so resulting from the macro) doesn't have any RUNPATHs set, the ROOT libraries it depends on are searched in LD_LIBRARY_PATH or in the system. - If a library of another ROOT is found, the whole chain of library loads starts with the library of the other ROOT until a second libCore.so is loaded. This will provoke a lot of warnings, because TClassTable is already populated, and potentially crashes the program. To avoid the above problem, this adds the ROOT library directory of the ROOT that compiles the macro as RUNPATH. This can be overridden by the user by either removing the rpath variable from TSystem::GetMakeSharedLib() or by setting an LD_LIBRARY_PATH.
hageboeck
force-pushed
the
roottest-LD_LIBRARY_PATH
branch
from
25229a7 to
6e2dcc0
Compare
| cmd.ReplaceAll("\"$BuildDir","$BuildDir"); | ||
| cmd.ReplaceAll("$BuildDir","\"$BuildDir\""); | ||
| cmd.ReplaceAll("$BuildDir",build_loc); | ||
| cmd.ReplaceAll("$RPath", "-Wl,-rpath=" + gROOT->GetSharedLibDir()); |
There was a problem hiding this comment.
cmd.ReplaceAll("$RPath", "-Wl,-rpath=" + gROOT->GetSharedLibDir());
This syntax is likely to be platform dependent (eg. different on Windows) and thus the -Wl,-rpath= may belong to the compiledata.sh.
There was a problem hiding this comment.
humm .. on the other hand for the Windows case, just not having $PATH on the produced line would have the same effect ... still is -Wl,-rpath= for all other platforms?
Test Results 22 files 22 suites 3d 22h 46m 22s ⏱️ For more details on these failures, see this check. Results for commit 6e2dcc0. |
3 participants
This reverts a part of 3161321, where LD_LIBRARY_PATH was removed from roottest. It seems to be needed at least for macro tests, though:
When ROOT is installed in the system, or when an LD_LIBRARY_PATH is set such that a different ROOT installation is visible, compiled macros can fail. The sequence of events is as follows:
This can be avoided by prepending the ROOT installation that is running the macro tests to LD_LIBRARY_PATH or to the Mac equivalent.