[CMake] Don't use find_package(ROOT) in tutorials/ subdirectory
#20996
Conversation
As Sergey said in the comment in `tutorials/CMakeLists.txt`, this tutorials project stopped working standalone. But, it still uses `find_package(ROOT)` to find ROOT, even though it's not installed yet as we are building ROOT itself at the same time. This should better not be done. This commit suggest to remove it, and also replaces the variables defined by `find_package(ROOT)` with the built time variables. This follows up on 9d2dc6e c21bc3f, and 458803e, which did the same for the `test/` and `roottest/` subdirectories.
Test Results 22 files 22 suites 3d 11h 12m 43s ⏱️ Results for commit 0eb4990. |
hageboeck
left a comment
hageboeck
left a comment
There was a problem hiding this comment.
LGTM, but maybe make the path to root.exe explicit.
| set(ROOT_root_CMD root.exe) | ||
| endif() | ||
| enable_testing() | ||
| set(ROOT_root_CMD root.exe) |
There was a problem hiding this comment.
This will call /usr/bin/root.exe or similar if installed.
| set(ROOT_root_CMD root.exe) | |
| set(ROOT_root_CMD $<TARGET_FILE:root>) |
| enable_testing() | ||
| set(ROOT_root_CMD root.exe) | ||
| if(NOT MSVC) # Ignore environment on Windows | ||
| set(ROOT_environ PATH=${CMAKE_BINARY_DIR}/bin:$ENV{PATH} |
There was a problem hiding this comment.
This PATH will theoretically fix up the root.exe path issue above, but I would still invoke it with the full path. It makes things clearer when you debug.
If you think whether the PATH can be removed now, I don't think so, because there might be some things like hist2workspace that might need to be invoked during the tutorials. 🙂
There was a problem hiding this comment.
Good point! In the end, I want to make everything independent of environment variables, for better portability. If there is a tutorial that starts root binaries, they should prepend gROOT->GetBinaryDir(). Would you agree?
2 participants
As Sergey said in the comment in
tutorials/CMakeLists.txt, this tutorials project stopped working standalone. But, it still usesfind_package(ROOT)to find ROOT, even though it's not installed yet as we are building ROOT itself at the same time.This should better not be done. This commit suggest to remove it, and also replaces the variables defined by
find_package(ROOT)with the built time variables.This follows up on 9d2dc6e c21bc3f, and 458803e, which did the same for the
test/androottest/subdirectories.