Include core modules in CMake config

[olantwin]

While packaging cetmodules for conda and aliBuild/bits for SHiP, I noticed that not all modules/tools are available included in the CMake config.

The cetmodulesConfig.cmake generated by cet_cmake_config() should auto-include the core modules that cetmodules' own CMakeLists.txt loads (all of which use include_guard(), so double-inclusion is safe). This would make find_package(cetmodules) a drop-in replacement for the FetchContent path, which is important for package managers (conda, spack, etc.) that provide pre-built cetmodules, requiring (trivial) patches.

I can prepare a small PR to address this, the patch we apply during the build currently is:

# Patch config so find_package(cetmodules) includes modules loaded via FetchContent
cat >> "${PREFIX}/share/cetmodules/cmake/cetmodulesConfig.cmake" <<'EOF'
include(CetProvideDependency)
include(CetCMakeEnv)
include(CetCMakeUtils)
include(CetCMakeConfig)
include(CetMake)
EOF

[greenc-FNAL]

@olantwin Many thanks for taking the time to create this issue! Prior to version 4, there were many inclusions triggered by cet_cmake_env() for user convenience, and a conscious effort was made to reduce the bloat when UPS features were removed for v4 (UPS is Fermilab’s home-grown package-management system). It's possible we went too far in the other direction.

I am certainly concerned that Cetmodules work correctly for external users regardless of whether it is invoked via find_package() or FetchContent, but the overall aim is for users to include the parts they need only when they need them. For instance, of the list you provide I would expect find_package(CetModules) to include CetCMakeEnv.cmake, CetProvideDependency.cmake, and CetCMakeConfig.cmake for functions whose invocation is a practical certainty in the user's top level CMakeLists.txt, but I'm surprised you needed to bring in CetCMakeUtils.cmake and CetMake.cmake, unless it was just for consistency with FetchContent. In that case, I think I'd rather rearrange the Cetmodules CMakeLists.txt to take them out of the top level directory scope, all other things being equal.

If you're able to share, I'd appreciate knowing how you arrived at the list in your patch above, and why each include() was necessary.

Thanks again for your report.

[olantwin]

@greenc-FNAL Thanks a lot for your reply!

Probably we don't need all, I just included all for consistency with FetchContent to get Phlex to build. I'll have a look what the minimal set is ASAP (probably tomorrow, as it's late in Europe).

Do all projects using cetmodules use it via FetchContent? Within our stack I'd prefer all build inputs be known at build time for reproducibility and not mix different package management systems, so I'd prefer find_package.

For reference the conda-forge staged recipe and the original recipe I wrote for aliBuild.

[olantwin]

A first look shows that CetCMakeUtils is not needed, but CetMake is. CetTest is only needed when running phlex tests. All the other inclusions seem necessary.

[greenc-FNAL]

I'd call that more of a problem with Phlex than with Cetmodules, but I'll definitely take a careful look.

To answer your question, FetchContent is a recent thing, but at least in Phlex the FetchContent is configured to use find_package() first: if a Cetmodules satisfying the version requirements is findable, the FetchContent will be avoided. So one thing this has shown me that there is a gap in our CI testing of Phlex.

[olantwin]

It seems like in the spack repo no CetModules package is defined nor defined as a dependency, so that FetchContent is used.

I did however find the art repo's cetmodules package, I'll read it tomorrow and see whether I can align the conda-forge package to it more.

[greenc-FNAL]

I'm sorry, my brain is remembering things very slowly and unreliably at the moment:

1. https://github.com/FNALssi/fnal_art is currently the official home of the Cetmodules Spack recipe.
2. Just a short while ago, I discovered that a PR I put in several months ago to update the Cetmodules recipe to include version 4.02.02 was approved but never merged: that has now been rectified.
3. I am currently attempting to test out a build of Phlex with a recipe that requires Cetmodules 4.02.02 available at build time: this should preempt the FetchContent operation and allow me to ensure that Phlex builds against an external Cetmodules except for certain rare circumstances while developing. Once this is successful, I will update the CI container to ensure that CI builds occur against a pinned version of Cetmodules as installed by Spack rather than FetchContent.

I apologize it took me this long to get my head around things. By the time we have Phlex 0.3 released you should be able to remove your patch.

[greenc-FNAL]

@olantwin One more thing: please note that building Phlex with a CMake preset such as default is quite important, and would probably solve the missing CetProvideDependency.cmake issue.

[olantwin]

I'm sorry, my brain is remembering things very slowly and unreliably at the moment:

1. https://github.com/FNALssi/fnal_art is currently the official home of the Cetmodules Spack recipe.

2. Just a short while ago, I discovered that a [PR I put in several months ago to update the Cetmodules recipe to include version 4.02.02](https://github.com/FNALssi/fnal_art/pull/138) was approved but never merged: that has now been rectified.

3. I am currently attempting to test out a build of Phlex with a recipe that requires Cetmodules 4.02.02 available at build time: this should preempt the `FetchContent` operation and allow me to ensure that Phlex builds against an external Cetmodules except for certain rare circumstances while developing. Once this is successful, I will update the CI container to ensure that CI builds occur against a pinned version of Cetmodules as installed by Spack rather than `FetchContent`.

I apologize it took me this long to get my head around things. By the time we have Phlex 0.3 released you should be able to remove your patch.

Thanks a lot! I'll keep an eye on the spack recipes and am looking forward to 0.3.

I have also updated out phlex recipe to use a CMake preset, I must confess that I hadn't noticed the preset, I'm used to packaging a lot of older CMake projects that don't have them.