fix(build): include GNUInstallDirs before add_subdirectory(subsystems)

[olantwin]

Summary

subsystems/DecayVolume/CMakeLists.txt and subsystems/Calorimeter/CMakeLists.txt install sbt.toml / calo.toml via

install(FILES ... DESTINATION ${CMAKE_INSTALL_DATADIR}/SHiPGeometry)

and bake CMAKE_INSTALL_PREFIX/CMAKE_INSTALL_DATADIR into the compiled *_TOML_INSTALL_PATH macros.

But the top-level CMakeLists.txt only does include(GNUInstallDirs) inside the bottom "Installation" block — i.e. after add_subdirectory(subsystems). So when subsystems are configured, CMAKE_INSTALL_DATADIR is undefined → empty, and the resulting cmake_install.cmake writes:

file(INSTALL DESTINATION "/SHiPGeometry" TYPE FILE FILES ".../sbt.toml")

A non-root install then fails:

CMake Error at build/subsystems/DecayVolume/cmake_install.cmake:54 (file):
  file cannot create directory: /SHiPGeometry.  Maybe need administrative privileges.

(Permissive environments silently drop the toml file from the install tree instead, which is arguably worse — surfaced this while bumping the conda recipe for v0.2.0.)

Fix

• Move include(GNUInstallDirs) above add_subdirectory(src) / subsystems / apps in the top-level CMakeLists.txt.
• Drop the now-redundant include(GNUInstallDirs) in apps/CMakeLists.txt.

Verification

Locally:

pixi run configure
grep DESTINATION build/subsystems/DecayVolume/cmake_install.cmake
# → file(INSTALL DESTINATION "${CMAKE_INSTALL_PREFIX}/share/SHiPGeometry" ...)

Downstream (SHiP conda recipe) without -DCMAKE_INSTALL_DATADIR=share workaround now lands sbt.toml and calo.toml under $PREFIX/share/SHiPGeometry/.

Summary by CodeRabbit

• Chores
  • Optimized build system configuration for improved installation directory handling across nested subsystems.

[coderabbitai]

📝 Walkthrough

Walkthrough

include(GNUInstallDirs) is moved earlier in the top-level CMakeLists.txt, before any add_subdirectory() calls, and the later duplicate inclusion is removed. A redundant include(GNUInstallDirs) in apps/CMakeLists.txt is deleted since the variable is now inherited from the parent scope.

Changes

GNUInstallDirs inclusion reorder

Layer / File(s)	Summary
Centralize GNUInstallDirs before subdirectories CMakeLists.txt, apps/CMakeLists.txt	include(GNUInstallDirs) is repositioned before add_subdirectory() calls in the top-level file with explanatory comments, its later duplicate occurrence is removed, and apps/CMakeLists.txt drops its own now-redundant inclusion while keeping the install(TARGETS ...) rule unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

• ShipSoft/Geometry#17: Adds install(TARGETS build_geometry validate_geometry ...) to ${CMAKE_INSTALL_BINDIR} in apps/CMakeLists.txt, which depends on CMAKE_INSTALL_BINDIR being defined — directly addressed by this PR's reordering.
• ShipSoft/Geometry#21: Also adjusts the top-level CMakeLists.txt to include GNUInstallDirs earlier and removes a duplicate inclusion, mirroring the exact same structural change made in this PR.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(build): include GNUInstallDirs before add_subdirectory(subsystems)' directly summarizes the main change—moving GNUInstallDirs inclusion earlier in CMakeLists.txt to resolve the build configuration issue where nested subsystems couldn't access CMAKE_INSTALL_*DIR variables.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/gnuinstalldirs-before-subsystems

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[olantwin]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CMakeLists.txt`:
- Around line 46-49: The include(GNUInstallDirs) statement needs to be
repositioned in CMakeLists.txt because it currently appears after the first
add_subdirectory() call, but it must come before all add_subdirectory() calls to
make CMAKE_INSTALL_DATADIR, CMAKE_INSTALL_BINDIR, CMAKE_INSTALL_LIBDIR, and
CMAKE_INSTALL_INCLUDEDIR available to subsystem CMakeLists files. Move the
include(GNUInstallDirs) line earlier in the file so it executes before any
add_subdirectory() calls begin.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: df9bd688-8d00-442e-b4af-d181d10aefc3

📥 Commits

Reviewing files that changed from the base of the PR and between 6a1455d and 406b3c5.

📒 Files selected for processing (2)

• CMakeLists.txt
• apps/CMakeLists.txt

💤 Files with no reviewable changes (1)

• apps/CMakeLists.txt