[rootx] Turn root executable into symbolic link to root.exe
#20006
Conversation
Test Results 22 files 22 suites 3d 20h 34m 5s ⏱️ For more details on these failures, see this check. Results for commit 3a8ac12. ♻️ This comment has been updated with latest results. |
ferdymercury
left a comment
ferdymercury
left a comment
There was a problem hiding this comment.
Thanks a lot!
Does this close https://its.cern.ch/jira/browse/ROOT-10697 ?
Please also fix these messages:
core/base/src/TApplication.cxx: fprintf(stderr, "ROOT splash screen is not visible with root.exe, use root instead.\n");
roofit/histfactory/config/prepareHistFactory:# we will switch here to use root.exe as it is done for the whole ROOT.
And the lost usage-help part.
Potentially add this change to the release-notes 6.38
| if (std::getenv("ROOTIGNOREPREFIX")) { | ||
| #endif | ||
| // Try to set ROOTSYS depending on pathname of the executable | ||
| SetRootSys(); |
There was a problem hiding this comment.
There are still a few places in the code that looks up ROOTSYS (and thus could potentially lead to a difference in behavior between root and root.exe). Some (eg the one in TUnixSystem.cxx) have additional search to find the 'right' value when not set) but (seemingly) not all.
As part of this PR, we need to review those later ones and either remove if the result is not used or annotated them as needed.
There was a problem hiding this comment.
Absolutely. I should mark this PR as draft to make clear that more work is needed.
Rather than this technical choice, we may want to just copy/duplicate. We just replaced the symbolic link genreflex/rootcling by a copy. |
With debug symbols, they are < 100k, so this would be a viable choice. |
|
About how Like this, our tests have a higher change of spotting problems if setting |
guitargeek
force-pushed
the
rootx
branch
2 times, most recently
from
2ac3069 to
84dccba
Compare
This comment was marked as outdated.
This comment was marked as outdated.
There is no need to have different executables anymore that start each
other anymore, but for backwards compatibility we keep `root.exe` also
for macOS and Linux. Before, `root` was launching `root.exe` after doing
the following things:
1. In the past, show the splash screen
(already removed in 6.36 with commit 4042121)
2. In the past, set `(DY)LD_LIBRARY_PATH`
(already removed in 6.38 with commit daf50ee)
3. Set `ROOTSYS`
The value of `ROOTSYS` can be inferred from the opened shared libraries
(a mechanism introduced in commit ????), which can always correctly be
opened because the `RPATH` is now always set (as of commit 26d24de).
The `root.exe` was launched with the full path. That's also not needed,
because `root` and `root.exe` were in the same directory anyway. So if
`root` was in the path already, `root.exe` will be too.
In conclusion, there is no need anymore for a separate "wrapper"
executable and this commit suggests to make the former `root` wrapper
simply a copy of `root.exe`.
4 participants
There is no need to have different executables anymore that start each
other anymore, but for backwards compatibility we keep
root.exealsofor macOS and Linux. Before,
rootwas launchingroot.exeafter doingthe following things:
(already removed in 6.36 with commit guitargeek@4042121)
(DY)LD_LIBRARY_PATH(already removed in 6.38 with commit guitargeek@daf50ee)
ROOTSYSThe value of
ROOTSYScan be inferred from the opened shared libraries(a mechanism introduced in commit ????), which can always correctly be
opened because the
RPATHis now always set (as of commit 26d24de).The
root.exewas launched with the full path. That's also not needed,because
rootandroot.exewere in the same directory anyway. So ifrootwas in the path already,root.exewill be too.In conclusion, there is no need anymore for a separate "wrapper"
executable and this commit suggests to make the former
rootwrappersimply a copy of
root.exe.This is similar to what was done for rootcling. The actual executable is small anyway, being linked against the ROOT shared libraries.