Skip to content

Conversation

@hollowsunhc
Copy link

@hollowsunhc hollowsunhc commented Mar 19, 2024

Closes #175 (comment)

Depends on #174

@hollowsunhc
Copy link
Author

The only thing I don't like about this MR is getting the version based on the tag. In this case, the extracted version would be 0.1.0, which is a relatively old version. It may not appeal to you

@hollowsunhc
Copy link
Author

@nmwsharp Answering to your comment at #206 (comment)

  • It's up to you on how you want to manage versions. Personally, I like to be able to use git tags for releases and identifying reliable commits. Having it set as a single source of truth makes it easier to maintain consistency. The code is heavily inspired from VTK.
  • include(GNUInstallDirs): Whenever you run CMake: Install, the locations of the install files will follow the GNU coding standards as described by the doc. GNUInstallDirs itself is the standard, cross-platform way to get install dir vars (CMAKE_INSTALL_BINDIR, …LIBDIR, …INCLUDEDIR, …DATADIR), which are used later in CMakeLists.txt.
  • Next, downstream users will be able to link to geometrycentral::geometry-central, linking to the whole codebase. Currently, it offers no level of granularity but at least makes it easier to link. Later on, if you plan to split the code into modules, you'll be able to create targets like geometrycentral::intrinsic or geometrycentral::signed-heat-method
  • Multiple CMake files are generated by the install:
    • geometrycentral-config-install.cmake.in: This file tells CMake how to find your library and its dependencies when someone runs find_package(geometrycentral CONFIG) in their own project. It sets up things like dependent packages (Eigen, SuiteSparse), options, and the exported targets.
    • geometrycentral-config-version.cmake: When users request a specific version of your library (e.g., find_package(geometrycentral 0.5.0 CONFIG)), this file helps CMake decide if the installed version is suitable.
    • geometrycentral.cmake: Downstream projects can link directly to your targets (geometrycentral::geometry-central), with all dependencies set up automatically. For example, if one uses a suitesparse-dependent GC, that file will trigger a find(SuiteSparse CONFIG) because it is required.
  • nanort, nanoflann and happly are now exposed in the install, making it easier for downstream users to target those.
  • Next, the logic for linking GC to SuiteSparse has been updated for the most recent versions of SuiteSparse because they now use targets too. (Perhaps, they have kept some form of reverse compatibility). Currently, all the code links to SuiteSparse::cholmod, SuiteSparse::spqr and SuiteSparse::umfpack. I recommend splitting into modules for a more targeted approach.

Also tagging @aschier

@aschier
Copy link

aschier commented Sep 15, 2025

What I usually include is:

  • the GNU directories for sorting files by type (and also matching a global installation in Unix-Systems) instead of putting everying into one folder,
  • set_property(GLOBAL PROPERTY USE_FOLDERS ON) can be useful to have targets sorted into folders in IDEs like MSVC. Works best with also adding headers to the build targets.
  • Exported Targets: After find_package One just has to set PROJECT_DIR so everything can be used like own targets. By setting CMAKE_PREFIX_PATH in the environment the directories are auto-detected when you have for example a libs folder somewhere with the project installed to libs/geometry-central.
  • Version file: allows to specifiy valid version in find_package. To be really useful one needs a versioning that has some kind of guarantee on the API, though.
  • INSTALL_INTERFACE / BUILD_INTERFACE makes sure you use the recent headers in development and the installed version when using other programs. It also allows to optionally allow use the library in a subdir instead of installing it.
  • Suffixes for installing debug and release at the same time: CMAKE_DEBUG_POSTFIX, CMAKE_RELEASE_POSTFIX, possibly also for relwithdebinfo and minsizerel, even though I think relwithdebinfo can often just replace the release build.
  • A doxygen target is nice to have

More Information can be found in the modern cmake guide, e.g. on exported targets

@hollowsunhc I wonder what get_project_version_from_git does when it is no git checkout? Can it have a fallback? One can also use something like currentversion-dev or nextversion-dev for git checkouts without clear version. But I must say that I'm not good in versioning research code ... semver is hard to do with such moving targets.

It is also useful to deactivate optional features automatically on a missing dependency and output a warning for more important features. I'd have to look what's optional here, but I think some libraries like SuiteSparse can sometimes be hard to setup (many numeric libraries are annoying to use with Windows) and if one needs a feature that does not depend on it, it is nice to be able to skip it.

@hollowsunhc
Copy link
Author

hollowsunhc commented Sep 15, 2025

@hollowsunhc I wonder what get_project_version_from_git does when it is no git checkout? Can it have a fallback? One can also use something like currentversion-dev or nextversion-dev for git checkouts without clear version. But I must say that I'm not good in versioning research code ... semver is hard to do with such moving targets.

Good point! If you try to compile a source .zip, you will probably get a status message and all the variables will be undefined. In that case, it is probably better to hard code the version in the sources.

My favorite way of versioning is through a regex that also captures the commit. That way, a dev build will have a version looking like: 0.1.0-27hsda13. It's really helpful whenever you want to share specific builds on custom branches for testing purposes. I'd have to know more about geometrycentral's requirements.

It is also useful to deactivate optional features automatically on a missing dependency and output a warning for more important features. I'd have to look what's optional here, but I think some libraries like SuiteSparse can sometimes be hard to setup (many numeric libraries are annoying to use with Windows) and if one needs a feature that does not depend on it, it is nice to be able to skip it.

The SuiteSparse I'm compiling on Windows is the following: https://github.com/DrTimothyAldenDavis/SuiteSparse but I agree that you may want to have it turned off for licensing issues or if in a pre-optimization phase. In that case, if you compile with suitesparse turned off, find_package(geometrycentral CONFIG) disappears inside geometrycentral-config-install.cmake

@aschier
Copy link

aschier commented Sep 15, 2025

I currently can't test Windows builds, but optional libraries being autodetected is good on the nice-to-have list even when no priority. I think the exported targets are the most important point here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Naming Convention for Installer CMake Config Files

2 participants