Conversation
Install and export the geometry-central target as a CMake package so downstream projects can use find_package(GeometryCentral CONFIG) without a custom FindGeometryCentral.cmake module. Also split the build and install interfaces so the exported target does not depend on this project's local helper targets, while installed consumers still inherit Eigen transitively.
Install the vendored headers referenced by public geometry-central headers so the install tree is self-contained. This removes the need for downstream post-install copy steps for happly.h and nanort/nanort.h.
|
|
||
| include(CMakeFindDependencyMacro) | ||
|
|
||
| find_dependency(Eigen3 3.3 REQUIRED NO_MODULE) |
There was a problem hiding this comment.
How does this interact with the system for resolving Eigen from multiple possible sources here https://github.com/nmwsharp/geometry-central/blob/master/deps/CMakeLists.txt#L30-L95 ?
Does this mean we'd immediately error-out if called by a user who lacks Eigen, rather than falling back on downloading it?
There was a problem hiding this comment.
Evidently the answer to my question is "no", because the CI tests download Eigen in this way and it still worked.
I guess I need to understand when this line gets executed
There was a problem hiding this comment.
This only runs in the installed-package path, after geometry-central has already been built/installed and a downstream project later does find_package(GeometryCentral REQUIRED CONFIG).
The existing Eigen resolution logic in deps/CMakeLists.txt is still the path used when building geometry-central itself from source, so the fallback/download behavior there is unchanged. That’s why CI could still download Eigen and succeed. It was exercising the source-build path, not the installed GeometryCentralConfig.cmake path.
So the split is:
- source build of geometry-central:
deps/CMakeLists.txtresolves Eigen and can still auto-download it - downstream consumption of an installed geometry-central package:
GeometryCentralConfig.cmakeruns, and at that point Eigen needs to be discoverable via CMake
I intentionally kept the installed package on the normal find_dependency() path rather than trying to re-run the repo-local download logic from an installed package.
Hopefully this helps!
Summary
This PR makes the installed
geometry-centralpackage self-contained for downstream CMake and Nix consumers.It removes the need for:
FindGeometryCentral.cmake/Findgeometry-central.cmakefilespostInstallcopy steps for vendored headersThe changes are intentionally small and split into two logical commits:
What Changed
1. Export an installable CMake package
This PR teaches
geometry-centralto install a standard CMake config package under:lib/cmake/GeometryCentral/GeometryCentralConfig.cmakelib/cmake/GeometryCentral/GeometryCentralTargets.cmakeThat allows downstream projects to use:
without providing a custom
FindGeometryCentral.cmake.To make the export work cleanly, the target interface was adjusted so the installed package does not depend on this repository's local helper targets from
deps/, while installed consumers still inheritEigen3::Eigentransitively.2. Install bundled public dependency headers
Some installed public headers currently reference vendored headers that were not being installed.
This PR installs the missing public dependency headers:
happly.hnanort/nanort.hso the install tree is usable without downstream
postInstallcopy steps.Why
Before this PR, downstream packaging needed extra repository-specific fixes.
In Nix, a working derivation required:
FindGeometryCentral.cmake/Findgeometry-central.cmakefiles$out/includeinpostInstallAfter this PR, those workarounds are no longer needed.
Verification
The changes were verified with:
A real
nix buildof the package using:-DGC_ALWAYS_DOWNLOAD_EIGEN=OFF-DGC_EIGEN_LOCATION=.../include/eigen3-DSUITESPARSE=OFFInspection of the install tree to confirm it now contains:
lib/libgeometry-central.*lib/cmake/GeometryCentral/GeometryCentralConfig.cmakelib/cmake/GeometryCentral/GeometryCentralTargets.cmakeinclude/happly.hinclude/nanort/nanort.hA downstream smoke test that successfully configured and built with:
find_package(GeometryCentral REQUIRED CONFIG)target_link_libraries(... geometry-central::geometry-central)Notes
pkg-configfiles.geometry-central.