Migrate from make to cmake and add GitHub Actions CI workflow#123
Migrate from make to cmake and add GitHub Actions CI workflow#123slanzi00 wants to merge 19 commits into
Conversation
Replace Makefile and src/Makefile with a modern CMake build system (≥3.20) that produces the same artifacts: libND_CAFMaker.so, makeCAF, and optionally testHDF.
- Version derived from git tags instead of hardcoded - ROOT uses imported targets (ROOT::*) instead of root-config flags - find_ups_package() calls expanded to multi-line keyword style - Add .cmake-format.yaml with find_ups_package() signature
- Extract find_ups_package() helper into cmake/FindUPSPackage.cmake - Drop the RelWithDebInfo CMAKE_BUILD_TYPE default - Replace -Wall with a full set of warning flags (-Wextra, -Wpedantic, -Wconversion, -Wsign-conversion, -Wshadow, etc.) - Add ASan + UBSan for Debug builds, with _GLIBCXX_SANITIZE_STD_ALLOCATOR on GCC - Add "Build environment" section documenting the required Singularity image (fnal-wn-sl7:latest) and ndcaf_setup.sh step before CMake
Integrate CMake build system from branch 11-build-is-very-fragile. Resolves conflict by accepting deletion of src/Makefile (superseded by CMake). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
chenel
left a comment
chenel
left a comment
There was a problem hiding this comment.
This is brilliant, thanks so much for implementing it. Bravo! 👏🏻
This will be a large-scale breaking change for a number of workflows, so we'll need to make sure we shop it around before merging it. Stakeholders I can think of:
- 2x2 production
- ND Reco/Sim production
- Expert ND_CAFMaker users from ND-LAr/2x2, TMS, SAND
We should start by discussing this at a ND CAFMaker scrum on a Wednesday and discuss the plan there. So I won't approve it yet, not because I think there's anything wrong with it necessarily, but just to make sure nobody accidentally clicks "merge" before we are ready.
Once the CI workflow part of this PR works, I imagine making it a requirement that CI pass before a PR can be merged, which would be a dream come true. (Even more than that, I imagine adding a testing CI workflow to ensure that future changes don't break the output except when we intend to. Etc. But obviously that's not for this PR.)
slanzi00
changed the title
From the ND Prod side, I've made a few changes to the run and install scripts in this (currently draft) PR. |
|
I just merged the latest TMS changes with this branch and have no issues building and running with CMake. Lovely work! |
slanzi00
force-pushed
the
47-continuous-integration
branch
from
9f9eb1d to
1fd2ab9
Compare
Modern UPS products that ship CMakeConfig files (ROOT, EDepSim, fhiclcpp, duneanaobj, Boost, TBB) are now resolved via find_package(... CONFIG), with transitive deps coming along for free. Legacy products without CMake support keep going through the find_ups_package helper, and GENIE retains its raw-flag integration via genie-config — see the inline comment in CMakeLists.txt for why.
4 participants
Resolves #11 and #123
#11 — CMake migration: Replace the fragile GNU Make build with a proper CMake
setup. Non-CMake-aware UPS packages are wrapped as
INTERFACE IMPORTEDtargets(
deps::hdf5,deps::genie, etc.); CMake-aware packages use the standardfind_package(...). Build artifacts are isolated underbuild/,the project version is pinned to 5.1.0, optional features are exposed as CMake
options (
ENABLE_TMS,ENABLE_TESTEXE), SANDReco is auto-detected from environmentvariables, and the compiler warning set has been expanded.
ndcaf_setup.shis split into two scripts:ndcaf_setup_deps.sh, sourced beforecompilation to set up all required UPS dependencies, and
ndcaf_setup.sh, whichinternally sources the former and is meant to be sourced after installation to
configure the runtime environment.
#123 — CI & fixes: Integrate the CMake build from
11-build-is-very-fragile,add
.github/workflows/ci.ymlto run on every PR using Apptainer + CVMFS insidethe
fnal-wn-sl7SL7 container, and fix a compilation error inSANDRecoBranchFiller.cxxwhere the no-SAND stub forGetTriggerswas missingthe
bool beamOnlyparameter, causing a signature mismatch with the header.