Skip to content

Conversation

guitargeek
Copy link
Contributor

This follows up on 26d24de, where it was made sure that ROOT executables always set the required RPATHs to the ROOT libraries, unless the user disables it with CMAKE_SKIP_INSTALL_RPATH at build time.

Therefore, the code path to set the LD_LIBRARY_PATH in the root executable was still kept for the case where CMAKE_SKIP_INSTALL_RPATH was enabled.

However, when thinking about it further, this is not required. The reason to set CMAKE_SKIP_INSTALL_RPATH is usually because ROOT is intended to be installed into directories that are already in the paths, like in the MacPorts configuration. So if the user doesn't need rpaths, they also don't need ROOT to figure out its library paths.

Therefore, this commit suggests so simply never set the library path in the root executable.

Closes https://its.cern.ch/jira/browse/ROOT-9758

This follows up on 26d24de, where it was made sure that ROOT
executables always set the required RPATHs to the ROOT libraries, unless
the user disables it with `CMAKE_SKIP_INSTALL_RPATH` at build time.

Therefore, the code path to set the `LD_LIBRARY_PATH` in the `root`
executable was still kept for the case where `CMAKE_SKIP_INSTALL_RPATH`
was enabled.

However, when thinking about it further, this is not required. The
reason to set `CMAKE_SKIP_INSTALL_RPATH` is usually because ROOT is
intended to be installed into directories that are already in the
paths, like in the MacPorts configuration. So if the user doesn't need
rpaths, they also don't need ROOT to figure out its library paths.

Therefore, this commit suggests so simply never set the library path in
the `root` executable.

Closes https://its.cern.ch/jira/browse/ROOT-9758
@ferdymercury
Copy link
Collaborator

Thanks! Is the Dlopen trick in TUnixSystem still needed?

#if defined(R__WINGCC) || defined(R__MACOSX)
if (!dynpath.EndsWith(":")) dynpath += ":";
dynpath += "/usr/local/lib:/usr/X11R6/lib:/usr/lib:/lib:";
dynpath += "/lib/x86_64-linux-gnu:/usr/local/lib64:/usr/lib64:/lib64:";
#else
// trick to get the system search path
std::string cmd("LD_DEBUG=libs LD_PRELOAD=DOESNOTEXIST ls 2>&1");
FILE *pf = popen(cmd.c_str (), "r");
std::string result = "";
char buffer[128];
while (!feof(pf)) {
if (fgets(buffer, 128, pf) != NULL)
result += buffer;
}
pclose(pf);
std::size_t from = result.find("search path=", result.find("(LD_LIBRARY_PATH)"));
std::size_t to = result.find("(system search path)");
if (from != std::string::npos && to != std::string::npos) {
from += 12;
std::string sys_path = result.substr(from, to-from);
sys_path.erase(std::remove_if(sys_path.begin(), sys_path.end(), isspace), sys_path.end());
if (!dynpath.EndsWith(":")) dynpath += ":";
dynpath += sys_path.c_str();
}
dynpath.ReplaceAll("::", ":");
#endif

@guitargeek
Copy link
Contributor Author

I think that's unrelated and maybe still needed to fix https://its.cern.ch/jira/browse/ROOT-5858. See also the commit that reproduces it:

Why you're asking? Is there another GitHub issue or Jira ticket related to it?

Copy link

Test Results

    21 files      21 suites   3d 22h 20m 55s ⏱️
 3 667 tests  3 662 ✅ 0 💤 5 ❌
75 189 runs  75 175 ✅ 5 💤 9 ❌

For more details on these failures, see this check.

Results for commit d345257.

@ferdymercury
Copy link
Collaborator

Why you're asking?

Just because it was mentioned in the description of https://its.cern.ch/jira/browse/ROOT-9758

@pcanal
Copy link
Member

pcanal commented Sep 22, 2025

The reason to set CMAKE_SKIP_INSTALL_RPATH is usually because ROOT is intended to be installed into directories that are already in the paths,

Can you produce a relocatable build without CMAKE_SKIP_INSTALL_RPATH ?

because ROOT is intended to be installed into directories that are already in the paths

Yes maybe ... but one of the 'point' of the root executable is to be (as opposed to root.exe) self sufficient (i.e. no need to set LD_LIBRARY_PATH nor ROOTSYS) even if the build was relocated.

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