Migrate from make to cmake and add GitHub Actions CI workflow#123
Migrate from make to cmake and add GitHub Actions CI workflow#123
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! |
4 participants
This merge resolves #11 and #123.
#11 — CMake migration: The fragile GNU Make build has been replaced with a proper CMake setup. UPS packages are now wrapped in
INTERFACE IMPORTEDtargets (deps::hdf5,deps::genie, etc.), build artifacts are isolated underbuild/for out-of-source builds, and the version is derived automatically fromgit describe --tags. Optional features are exposed as CMake options (ENABLE_TMS,ENABLE_TESTEXE), SANDReco auto-detection is expressed as a properif(DEFINED ENV{...})block, and the warning set has been significantly expanded beyond the previous-Wallonly.#123 — CI & fixes: Integrated the CMake build system from branch
11-build-is-very-fragile, added.github/workflows/ci.ymlto build on every push and PR using Apptainer + CVMFS inside thefnal-wn-sl7SL7 container, and fixed a compilation error inSANDRecoBranchFiller.cxxwhere the no-SAND stub forGetTriggerswas missing thebool beamOnlyparameter, causing a signature mismatch with the header.