Modernize CMake package config and install layout#204
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #204 +/- ##
=======================================
Coverage 47.86% 47.86%
=======================================
Files 37 37
Lines 4745 4745
Branches 857 857
=======================================
Hits 2271 2271
Misses 2469 2469
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- No nested project() call - use BUILD_INTERFACE and INSTALL_INTERFACE in target_include_directories call - Improve/simplify phold's cmake as well as drop BPGM since BG/Q doesn't exist anymore :)
The previous hand-written ROSSConfig.cmake had no version file, no
find_dependency(MPI), no namespaced target, and lived at <prefix>/lib/
where consumers needed a -DROSS_DIR hint to discover it. Out-of-tree
projects had to re-run find_package(MPI) themselves and link the bare
ROSS target.
After this change consumers can write:
find_package(ROSS REQUIRED)
target_link_libraries(myapp PRIVATE ROSS::ROSS)
and get include dirs, the MPI dependency, and link libs propagated
transparently. The config installs at the canonical
<prefix>/lib/cmake/ROSS/ which CMake discovers from CMAKE_PREFIX_PATH
without any explicit hint, and a generated ROSSConfigVersion.cmake
makes version-constrained discovery (find_package(ROSS 8.0)) work.
A back-compat ALIAS keeps legacy target_link_libraries(... ROSS) call
sites working without modification, and a tiny shim at the old
<prefix>/lib/ROSSConfig.cmake location preserves -DROSS_DIR=<prefix>/lib
callers, emitting a deprecation warning and delegating to the
canonical config, while also setting the ROSS_INCLUDE_DIRS variable
master used to export.
Top-level is the place for project-level build infrastructure, not among core C files.
Previously every *.h was installed flat into <prefix>/include/, polluting the consumer's include namespace with common names (config.h, buddy.h, lz4.h, io.h). Moving them under <prefix>/include/ross/ contains the namespace; the exported target's INSTALL_INTERFACE still resolves #include <ross.h> transparently for consumers.
Installed headers were moved under <prefix>/include/ross/, but
ross.pc.in still pointed Cflags at the flat <prefix>/include/ so any
pkg-config consumer (CODES today) would emit the wrong -I and miss
the headers. Rewriting includedir to point at the new location keeps
the pkg-config flow transparent for downstream consumers; no source
changes needed on their side.
Install destination uses ${CMAKE_INSTALL_LIBDIR}/pkgconfig so
multilib (lib64) and Spack-style overrides work.
Hoist include(GNUInstallDirs) to the top-level CMakeLists.txt so the
install-RPATH stanza can resolve ${CMAKE_INSTALL_LIBDIR} instead of
forcing <prefix>/lib. The phold install rule likewise switches from
literal "bin" to ${CMAKE_INSTALL_BINDIR}, completing the sweep that
earlier commits had started in core/.
On macOS and standard Linux layouts the resulting paths are unchanged;
the difference shows up on Debian multiarch (lib/x86_64-linux-gnu) and
RHEL/Fedora multilib (lib64), where installed binaries now find
libROSS through the correct directory.
8f6f9bb to
961dd65
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes
find_package(ROSS)a self-contained modern CMake package. Out-of-tree consumers can write:and get include dirs, the MPI dependency (re-resolved on the consumer's machine, not baked in), and link libraries propagated transparently. Master's hand-written 3-line
ROSSConfig.cmakehad no version file, nofind_dependency(MPI), no namespaced target, and lived at a non-canonical location requiring-DROSS_DIRhints to find.What changed:
<prefix>/lib/cmake/ROSS/with proper version file, MPI dependency re-resolution, andROSS::ROSSnamespaced target. CMake discovers it fromCMAKE_PREFIX_PATHautomatically — no hints needed.<prefix>/include/to<prefix>/include/ross/to stop polluting the consumer include namespace with common names (config.h, buddy.h, lz4.h, io.h).PROJECT(ROSS C)incore/, attached public include dirs viatarget_include_directories(... BUILD_INTERFACE / INSTALL_INTERFACE), moved CMake helper modules to top-level cmake/.config files, pkg-config file, headers, phold binaries, and the
install-RPATH all derive from
${CMAKE_INSTALL_LIBDIR}/${CMAKE_INSTALL_INCLUDEDIR}/${CMAKE_INSTALL_BINDIR}. No effect onmacOS or standard Linux layouts, but installs cleanly on Debian
multiarch (lib/x86_64-linux-gnu) and RHEL/Fedora multilib (lib64).
models/pholdupdated to install all 6 phold variants (master only installedphold) and dropped a dead BGPM branch.Back-compat:
ross.pckeeps working for pkg-config consumers. CODES rebuilds cleanly against the new install with no source changes .libROSS.{a,so}unchanged, cachedpkgcfg_lib_ROSS_ROSSentries in consumer build dirs stay valid.find_package(ROSS)+-DROSS_DIR=<prefix>/libcallers keep working via a tiny shim at the old location; emits a deprecation warning and delegates to the canonical config (preserves theROSS_INCLUDE_DIRSvariable master set).target_link_libraries(... ROSS)callers keep working via a back-compat ALIAS in the newROSSConfig.cmake.Checklist
-Walland-WextraDocumentation/dev/, unless the change is invisible to anyone outside the PR (test refactors, internal renames, comment-only tweaks)