FairRootGroup / FairRoot

C++ simulation, reconstruction and analysis framework for particle physics experiments
http://fairroot.gsi.de
Other
57 stars 96 forks source link

FairRootConfig.cmake should find relevant dependencies #1545

Closed YanzhaoW closed 2 months ago

YanzhaoW commented 3 months ago

Is your feature request related to a problem? Please describe.

Importing FairRoot with find_package reads the cmake file FairRootConfig.cmake. Currently, it only contains the targets and some extra variables. The dependent project still needs to import all the necessary packages like ROOT, Geant4, etc. It would be much better if this can be automatically done in the config file.

Describe the solution you'd like

In the file FairRootConfig.cmake.in, ROOT, Geant4, etc should be added there:

include(CMakeFindDependencyMacro)

find_dependency(ROOT
  version 6.18.00
  COMPONENTS ${required_root_components})

Same goes for other packages.

Additional context Some other configuration variables, such as CMAKE_PREFIX_PATH should also be added here to make sure packages can be found and correctly imported.

dennisklein commented 3 months ago

Thank you for this issue. We intentionally do not use the find_dependency pattern, but - as you can see - we export all the dependencies in the CMake package as variables which can then be used in combination with the FairFindPackage2.cmake module.

Basically, what you do in the consuming project, is:

find_package(FairCMakeModules 1.0 QUIET REQUIRED)
include(FairFindPackage2)
find_package2(PUBLIC FairRoot VERSION 19 REQUIRED)

# put your own direct dependencies here via `find_package2()`

find_package2_implicit_dependencies()

# optional, from `FairSummary` module
fair_summary_package_dependencies()

For detailed docs see:

In cases, where your project shares a direct dependency with FairRoot, find_package2 will automatically merge both sets of constraints meaningfully - instead of calling find_package multiple times for the same dependency with different sets of constraints. Also, we had cases where find modules were not behaving idempotently when calling them multiple times.

Note beside: When choosing to use find_package2, you can also benefit from fair_summary_package_dependencies() which prints a nice dependency list (yes, there is also an upstream CMake module for this, but the output is not as nice as ours ;) )


That said, as most of the CMake community tends to go the find_dependency route, I may be convinced to add this as well (by opt-in via a variable or component constraint) in a future release. E.g.

find_package(FairRoot 20 COMPONENTS WITH_FIND_DEPENDENCY)

or similar.

YanzhaoW commented 3 months ago

Thanks for your comment. Does this work only for the latest version (jan24) of FairSoft?

I'm trying with nov22p1. It seems find_package2_implicit_dependencies doesn't work.

dennisklein commented 3 months ago

nov22p1 should be fine. How does it fail?

YanzhaoW commented 3 months ago

@dennisklein

It does nothing. No output and no dependency packages installed. I also checked the FairCMakeModules installed in the system. It also has no fair_summary_package_dependencies.

I will try to use the latest version of FairSoft. Is it possible to include

find_dependecy(FairCMakeModules 1.0)
include(FairFindPackage2)

into FairRootConfig.cmake? Then user can just deal with one dependency instead of two:

 # probably find_package2 should also be called in the config file
find_package(PUBLIC FairRoot VERSION 19 REQUIRED) 
find_package2_implicit_dependencies()
fair_summary_package_dependencies()

If this is not possible, it shows find_dependency does have certain advantages:

  1. user only needs to import one package.
  2. dependencies are guaranteed to be installed (even if users forget to call find_package2_implicit_dependencies()).

It's much easier to break the build system if we need to deal with two imported packages and both of them must work correctly. I suspect it's the cause in my case of nov22p1 as the FairCMakeModules could be not probably imported.

That being said, find_dependency does have a caveat as is mentioned in Craig Scott CMake book (section 27.8.1. Config Files For CMake Projects):

Be aware that prior to CMake 3.15, find_dependency() contained an optimization that bypassed the call if it detected that the requested package had previously been found. This optimization worked fine unless later calls needed to request a different set of package components. The first time find_dependency() succeeds, the pre-3.15 behavior effectively locks in the set of components found. If later calls to find_dependency() pass a different set of components, they would be ignored. Because find_dependency() calls are typically made within files that are installed for other projects to use, it usually cannot be guaranteed what CMake version will ultimately be used. Therefore, if the dependency supports package components, projects should avoid find_dependency() and call find_package() directly, handling the QUIET and REQUIRED options themselves.

dennisklein commented 3 months ago

I will try to use the latest version of FairSoft. Is it possible to include

find_dependecy(FairCMakeModules 1.0)
include(FairFindPackage2)

into FairRootConfig.cmake? Then user can just deal with one dependency instead of two:

 # probably find_package2 should also be called in the config file
find_package(PUBLIC FairRoot VERSION 19 REQUIRED) 
find_package2_implicit_dependencies()
fair_summary_package_dependencies()

No, the FairFindPackage2 module is just opt-in. There are no plans to force a FairRoot user to use it. As I said earlier, I may add a mode with find_dependency being called in the package file.

It does nothing. No output and no dependency packages installed.

Not sure I understand, no output is expected. find_package2_implicit_dependencies() will not install anything. It will loop over dependencies declared by packages discovered via find_package2() and call find_package2 on each if the dependency has not yet been found.

I also checked the FairCMakeModules installed in the system. It also has no fair_summary_package_dependencies.

Which system?

Note: fair_summary_package_dependencies() is part of the FairSummary module. If you want to use any function/macro from it, you need to include(FairSummary).

dennisklein commented 3 months ago

I suspect it's the cause in my case of nov22p1 as the FairCMakeModules could be not probably imported.

FairCMakeModules v1.0 is included in nov22p1 - see https://github.com/FairRootGroup/FairSoft/blob/nov22p1/legacy/README.md#included-packages

dennisklein commented 3 months ago

Please also compare your CMakeLists.txt with our project template: https://github.com/FairRootGroup/FairRoot/blob/master/templates/project_stl_containers/CMakeLists.txt

These you can leave out, you should just need to find_package2 your own direct dependencies: https://github.com/FairRootGroup/FairRoot/blob/99be5f48d4bfed7ee7179524c07bc49a6b37a5c5/templates/project_stl_containers/CMakeLists.txt#L81-L123

dennisklein commented 3 months ago

Here is a minimum CMakeLists.txt:

cmake_minimum_required(VERSION 3.18 FATAL_ERROR)

project(MinimumExample LANGUAGES CXX)

find_package(FairCMakeModules 0.2 REQUIRED)
include(FairFindPackage2)
find_package2(PRIVATE FairRoot VERSION 19 REQUIRED)
list(PREPEND CMAKE_MODULE_PATH ${FairRoot_PREFIX}/share/fairbase/cmake/modules)
find_package2_implicit_dependencies()

include(FairSummary)
fair_summary_package_dependencies()
❯ cmake -S. -Bbuild -DCMAKE_PREFIX_PATH=/home/dklein/fairsoft/jan24 -DFairRoot_ROOT=/home/dklein/projects/FairRoot3/install
-- The CXX compiler identification is GNU 14.1.1
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/lib64/ccache/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
--
--   DEPENDENCY FOUND     VERSION                   PREFIX
--   FairCMakeModules     1.0.0 (>= 0.2)            /home/dklein/fairsoft/jan24
--   FairRoot             19.0.0 (>= 19)            /home/dklein/projects/FairRoot3/install
--   ROOT                 6.30.04 (>= 6.18.00)      /home/dklein/fairsoft/jan24
--   VMC                  2.0.0                     /home/dklein/fairsoft/jan24
--   Pythia6              unknown                   /home/dklein/fairsoft/jan24
--   Pythia8              8.310                     /home/dklein/fairsoft/jan24
--   Protobuf             3.19.6                    /usr
--   Flatbuffers          23.5.26                   /home/dklein/fairsoft/jan24
--   Geant3               4.2.0                     /home/dklein/fairsoft/jan24
--   Geant4               11.2.0                    /home/dklein/fairsoft/jan24
--   VGM                  5.2.0                     /home/dklein/fairsoft/jan24
--   Geant4VMC            6.5.0                     /home/dklein/fairsoft/jan24
--   GSL                  unknown                   /usr
--   FairMQ               1.8.4 (>= 1.4.0)          /home/dklein/fairsoft/jan24
--   DDS                  3.8                       /home/dklein/fairsoft/jan24
--   FairLogger           1.11.1 (>= 1.6.0)         /home/dklein/fairsoft/jan24
--   Boost                1.83.0 (>= 1.67)          /home/dklein/fairsoft/jan24
--   yaml-cpp             0.7.0                     /usr
--   Threads              unknown
--   fmt                  10.1.1 (>= 5.3.0)         /home/dklein/fairsoft/jan24
-- Configuring done (1.8s)
-- Generating done (0.0s)
-- Build files have been written to: /home/dklein/projects/FairRootExamples/build

I have not tried, but I expect this to work with FairSoft nov22p1 as well.


It is not perfect yet, and it will not be for a while. FairRoot is still a mess ;)

YanzhaoW commented 3 months ago

Hi, @dennisklein Thanks for your detailed explanation.

Not sure I understand, no output is expected. find_package2_implicit_dependencies() will not install anything. It will loop over dependencies declared by packages discovered via find_package2() and call find_package2 on each if the dependency has not yet been found.

Sorry, I meant "imported" instead of "installed". But later the issue was to be caused by not specifying FairRoot to be 19 and FairRoot 18 was actually used.

I tried your minimum example in a debian 10 server and it still didn't work out.

Here is my cmake:

cmake_minimum_required(VERSION 3.18 FATAL_ERROR)

project(MinimumExample LANGUAGES CXX)

list(APPEND CMAKE_PREFIX_PATH "/u/yanwang/software/FairRoot")
list(APPEND CMAKE_PREFIX_PATH "/cvmfs/fairsoft.gsi.de/debian10/fairsoft/nov22p1")

find_package(FairCMakeModules 0.2 REQUIRED)
include(FairFindPackage2)
find_package2(PRIVATE FairRoot VERSION 19 REQUIRED)
list(PREPEND CMAKE_MODULE_PATH ${FairRoot_PREFIX}/share/fairbase/cmake/modules)
find_package2_implicit_dependencies()

include(FairSummary)
fair_summary_package_dependencies()

add_executable(main main.cpp)
target_link_libraries(main PRIVATE ROOT::RIO)

I didn't define any env variables except those for the compiler and cmake.

Here is the output:

-- The CXX compiler identification is GNU 8.3.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Looking for Root... - Not found
CMake Error at /u/yanwang/software/FairRoot/share/fairbase/cmake/modules/FindROOT.cmake:111 (Message):
  ROOT not installed in the searchpath and ROOTSYS is not set.  Please set
  ROOTSYS or add the path to your ROOT installation in the Macro
  FindROOT.cmake in the subdirectory cmake/modules.
Call Stack (most recent call first):
  /cvmfs/fairsoft.gsi.de/debian10/fairsoft/nov22p1/share/faircmakemodules/modules/FairFindPackage2.cmake:212 (find_package)
  /cvmfs/fairsoft.gsi.de/debian10/fairsoft/nov22p1/share/faircmakemodules/modules/FairFindPackage2.cmake:392 (find_package2)
  CMakeLists.txt:12 (find_package2_implicit_dependencies)

-- Configuring incomplete, errors occurred!

Error shows that CMake is trying to import ROOT using MODULE mode instead of the suggested CONFIG mode.

Here are some version info:

dennisklein commented 2 months ago
  1. Something seems wrong with your FairRoot v19 installation. The file /u/yanwang/software/FairRoot/share/fairbase/cmake/modules/FindROOT.cmake is no longer part of FairRoot v19. Try deleting /u/yanwang/software/FairRoot and re-installing pls.

  2. add_executable(main main.cpp)
    target_link_libraries(main PRIVATE ROOT::RIO)

    If you link against ROOT::RIO like this, the project has a direct dependency to ROOT. You should not rely on find_package2_implicit_dependencies() to find ROOT for you in this case, but add your own explicit find_package2(PRIVATE ROOT REQUIRED) (before the find_package2_implicit_dependencies() statement) or similar.

dennisklein commented 2 months ago
  1. -- The CXX compiler identification is GNU 8.3.0

    Further down you write gcc: 13 - so it seems CMake is not honoring your custom compiler location somehow as well, no? You can find more info here: https://cmake.org/cmake/help/latest/manual/cmake-toolchains.7.html. For non-system compilers, I recommend creating a CMake toolchain file, which is a nice way to keep your toolchain related settings separately from the project CMakeLists.txt files. See also https://cmake.org/cmake/help/latest/manual/cmake.1.html#cmdoption-cmake-toolchain

YanzhaoW commented 2 months ago

@dennisklein Very nice! It works now.

Something seems wrong with your FairRoot v19 installation. The file /u/yanwang/software/FairRoot/share/fairbase/cmake/modules/FindROOT.cmake is no longer part of FairRoot v19. Try deleting /u/yanwang/software/FairRoot and re-installing pls.

That's absolutely right. But unfortunately, most of time I need to install FairRoot v18 and v19 together such that I could test whether our software could work with both versions. For now, it seems impossible by just changing the version in find_package:

find_package2(PRIVATE FairRoot VERSION 18 REQUIRED) # change to 19 if needed.

I wonder whether it's a better idea to put fairbase installation position inside /lib/cmake/FairRoot.{VERSION}. This could prevent sourcing the cmake scripts in a wrong version. And maybe the following line should also be defined in FairRootConfig.cmake?

list(PREPEND CMAKE_MODULE_PATH ${FairRoot_PREFIX}/share/fairbase/cmake/modules)

If in the next time we change the path /share/fairbase/cmake/modules to something better, that would be a breaking changing from the user side.

Another thing I would like to ask is whether it's possible to import ROOT using CONFIG mode explicitly. But that would change the implementation of find_package2_implicit_dependencies.

Further down you write gcc: 13 - so it seems CMake is not honoring your custom compiler location somehow as well, no? You can find more info here: https://cmake.org/cmake/help/latest/manual/cmake-toolchains.7.html. For non-system compilers, I recommend creating a CMake toolchain file, which is a nice way to keep your toolchain related settings separately from the project CMakeLists.txt files.

Yes, you have to define proper env variables before launch the cmake. I normally do this with my own function:

function use_gcc(){
    export CC=gcc
    export CXX=g++
    [ -z "$ld_library_path" ] && typeset -gT LD_LIBRARY_PATH ld_library_path

    path=('/u/land/software/gcc/latest/bin' $path)
    export LD_LIBRARY_PATH="/u/land/software/gcc/latest/lib64${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}"
    export LIBRARY_PATH="/u/land/software/gcc/latest/lib64${LIBRARY_PATH:+:$LIBRARY_PATH}"
    typeset -gU path ld_library_path
}

However, I don't think it's a good idea to hard-code compiler info (or generator type) into project cmake configuration. Maybe next time I want to use gcc 14, I have to unnecessarily change the project.

dennisklein commented 2 months ago

But unfortunately, most of time I need to install FairRoot v18 and v19 together such that I could test whether our software could work with both versions.

This is a well understood use case! Actually, I just proposed a week ago in our internal group meeting that we should have a new repository that contains both our examples and the project templates exactly to ensure better this common requirement. This new repo would then be part of our CI in such a way, that it has to pass working with the 2+ latest FairRoot releases and also functions as an example repo on how we envision the user CMakeLists.txt to look like.

Note beside: My idea is also to move FairRoot more and more towards a standard CMake library from the point of view a user's (experiment's) project. However, this is an invasive change to the current model which ties the experiment's project very tightly in non-standard ways (for historic reasons - mainly CMake v2 did not have standards for many aspects). So, it will likely take a few releases to accomplish that - and we also need to keep the steps small enough to not alienate anyone too much.

For now, it seems impossible by just changing the version in find_package:

find_package2(PRIVATE FairRoot VERSION 18 REQUIRED) # change to 19 if needed.

The idea is that you always require the minimum version your project supports. Then you list the FairRoot install prefix - you want to be found - in your CMAKE_PREFIX_PATH (either only the single one or if you list both, the order matters - it is a standard search path, all directories will be considered in order).

Note: It is not supported to install multiple versions of FairRoot into the same install prefix!

And maybe the following line should also be defined in FairRootConfig.cmake?

list(PREPEND CMAKE_MODULE_PATH ${FairRoot_PREFIX}/share/fairbase/cmake/modules)

Yes, there will be more FairRoot_* style variables exported in the near future, one of them FairRoot_CMAKEMODULEDIR or similar. Prepending it to the CMAKE_MODULE_PATH shall remain the opt-in decision of the consuming project however.

Also, find modules are currently shipped as part of the FairRoot install tree, but we want to move them to the FairCMakeModules project instead as we re-use those find modules in multiple places and it they should not be tied to a FairRoot release in any case.

Another thing I would like to ask is whether it's possible to import ROOT using CONFIG mode explicitly. But that would change the implementation of find_package2_implicit_dependencies.

There is a different standard mechanism in CMake to control this: -DCMAKE_FIND_PACKAGE_PREFER_CONFIG=ON

However, I don't think it's a good idea to hard-code compiler info (or generator type) into project cmake configuration. Maybe next time I want to use gcc 14, I have to unnecessarily change the project.

I agree, that is what I wrote, no? CMake toolchain files are basically an alternative to your shell function.

dennisklein commented 2 months ago

One more thing:

As long as your experiment project supports FairRoot pre ~v19~ edit: v18.8, you need to have a copy of FindFairRoot.cmake around (or some variant of it, but in your repo of course). Note, that we updated the logic to do a CONFIG mode search first: https://github.com/FairRootGroup/FairRoot/blob/99be5f48d4bfed7ee7179524c07bc49a6b37a5c5/cmake/modules/FindFairRoot.cmake#L25 before falling back to the classic module logic. That should allow for the transition until you bump the minimum required FairRoot to v19+.

YanzhaoW commented 2 months ago

Thanks again. That's pretty clear.

I agree, that is what I wrote, no? CMake toolchain files are basically an alternative to your shell function.

Sorry for the misunderstanding. I haven't used cmake toolchain except the one from conan package manager. But I will try it out.

Thus, this issue can be closed.

dennisklein commented 2 months ago

Thank you for your feedback and questions. I added a few separate issues for some of the topics/issues we discussed, see #1547, #1548, #1549 and #1550. Hope, I did not forget anything, if so, let me know.