Fix FindROOT.cmake in case thisroot.sh wasn't sourced

[Vindaar]

Related to #133.

While helping @jovoy, we stumbled over the FindROOT.cmake script which currently:

• neither checks whether ROOTSYS is defined (i.e. whether thisroot.* was sourced)
• nor just uses the root-config to get all information it needs.

So this PR:

• throws a fatal error on Windows in case ROOTSYS is undefined
• uses the result of root-config --bindir on unix, as that is all we need to determine the path to rootcint.

I'm neither a cmake expert, nor sure whether it's wise in the first place to continue if ROOTSYS is not defined. I can't foresee if later not having sourced the file leads to even harder to debug issues. Alternatively, we could also just check for the existence of the variable on unix and fatal error in the same way.

If FindROOT.cmake is buried soon, just make sure the behavior of the alternative is sane & checks for this as well.

@rest-for-physics/core_dev

[lobis]

Thanks for this.

It is my goal to remove the FindROOT.cmake macro soon (in #111).

Now the way to find root is to call the standard cmake macro: find_package(ROOT REQUIRED) which will throw a standard cmake exception when ROOT is not found, so it is not necessary to check for ROOTSYS since it will always be defined if the find_package macro doesn't throw.

Regarding rootcint in the mentioend PR it is defined as set(ROOTCINT_EXECUTABLE $ENV{ROOTSYS}/bin/rootcint) which should always work if run after find_package. I have now read https://root.cern/manual/integrate_root_into_my_cmake_project/ more carefully and have defined in a better way in 891522c. This should work in all platforms but I cannot test it in Windows or macOS.

[jgalan]

Pipeline is green so for me is good to merge and solve installation problems.

[lobis]

FindROOT.cmake macro will be deprecated soon.

[jgalan]

I guess it is fine to merge now. @nkx is using Windows, but I believe he will not get in troubles with this patch?

[Vindaar]

Thanks for this.

It is my goal to remove the FindROOT.cmake macro soon (in #111).

Now the way to find root is to call the standard cmake macro: find_package(ROOT REQUIRED) which will throw a standard cmake exception when ROOT is not found, so it is not necessary to check for ROOTSYS since it will always be defined if the find_package macro doesn't throw.

I don't quite understand how that will work. If I understand the cmake docs correctly:

https://cmake.org/cmake/help/latest/command/find_package.html

it says that find_package(foo, ...) simply dispatches to a FindFoo.cmake file. So if one removes the FindROOT.cmake usage of find_package breaks, no?

I guess it is fine to merge now. @nkx is using Windows, but I believe he will not get in troubles with this patch?

Unless I made some mistake, nothing should change in case the ROOT installation is set up correctly. If it wasn't there probably would have been other errors for @nkx111.

[lobis]

it says that find_package(foo, ...) simply dispatches to a FindFoo.cmake file. So if one removes the FindROOT.cmake usage of find_package breaks, no?

@Vindaar
No, find_package will try to find this package looking first into standard places. If root is sourced correctly it will find ROOTConfig.cmake (or something similar) probably in $ROOTSYS/cmake and load it. I removed the FindROOT.cmake because it is shadowing this functionality, we're using some frozen version of ROOTConfig.cmake (named FindROOT.cmake) instead of the current version packaged in the installation directory.

[Vindaar]

it says that find_package(foo, ...) simply dispatches to a FindFoo.cmake file. So if one removes the FindROOT.cmake usage of find_package breaks, no?

@Vindaar No, find_package will try to find this package looking first into standard places. If root is sourced correctly it will find ROOTConfig.cmake (or something similar) probably in $ROOTSYS/cmake and load it. I removed the FindROOT.cmake because it is shadowing this functionality, we're using some frozen version of ROOTConfig.cmake (named FindROOT.cmake) instead of the current version packaged in the installation directory.

Ah, I had no idea cmake had such a feature. So that means also all binary installations of root include that cmake file?

[lobis]

Ah, I had no idea cmake had such a feature. So that means also all binary installations of root include that cmake file?

Yes, atleast the most recent ones. This also happens for Garfield, Geant4 and most modern c++ libraries/packages.

[nkx111]

Unless I made some mistake, nothing should change in case the ROOT installation is set up correctly. If it wasn't there probably would have been other errors for @nkx111.

No errors for me. Actually on Windows I only do the coding. When building REST I always switch to linux.