-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Open
Description
I am sorry if this issue is a bit big, but there are a lot of inter-connected components that I want to address. Primarily this comes from some discussions with Laurent on Fedora Bugzilla and doing a quick look at the project structure. So a few points that I want to raise, primarily about the CMake project structure.
- CMake: Clear distinction between various sections. This is the structure I have converged to currently
- CMake: Avoid bundling
Find<Package>.cmake. If there is a need for compatibility layer, e.g. for fallback topkg-config, try a structure like this. I have a project that I am developing, and the project is still flexible right now to accommodate for more needs. - CMake: Use
FetchContentparticularly in combination withFIND_PACKAGE_ARGSas much as possible - CMake: Make the project available for
FetchContent. This means namespacing options, aliasing targets and exporting equivalent variables - CMake: Do not use hard-coded paths, especially OS-dependent formats
- CMake: Add a cmake native
ctesttestsuite. A simple format is to usectest --build-and-testto call ctest recursively. This also makes it possible to symlink examples to test directly. - CMake: Add labeled ctest for simple unit-test, short-run, long-run, etc. so that tests are run efficiently, including in downstream packaging such as Fedora
- CMake: Simplify
CGalConfig.cmake. Rely on the built-in support for targets to pull in appropriate dependencies. - CMake: Avoid using bare
include_directoriesetc. Use target scoped ones instead - CMake: Minimize the use of custom functions and macros, document them and mark appropriate compatibility modules in a separate
compatfolder, with specificversionguarding and with plans for removal and deprecation following OS lts releases - CMake: Do not add global
include, etc. in theCGalConfig.cmake. This is propagated to all downstream users of CGal, e.g.FindEigen3.cmakeoverrides the use system installedEigen3Config.cmake. - CMake: Individual sub-projects should be added as cmake subprojects. To handle the dependencies, make use of
FetchContentlogic settingFETCHCONTENT_SOURCE_DIR_<uppercaseName>to the relative path from the top-levelPROJECT_SOURCE_DIR - (optional) CMake: Remove cpack/cmake_uninstall targets. These days the package manager does a better job of keeping track of these. Caveat being that I am not a Windows user, and I don't know if there is a need there
- Project: Switch to simple
src/docs/test/example(maybeinclude) folder structure. The internal folder structure can be enforced via the CI - Project: Provide non-header-only support. CMake and FetchContent have come a long way, and the need for header-only projects is unjustified these days. Other projects are moving away from header-only, e.g. Catch2, primarily to improve compilation time and executable sizes.
- Project: Add CODEOWNERS to keep track of who should be notified of any changes and reviewing based on changes to file paths
- Project: Package python tools in a
pyproject.tomlfor visibility and ease of use, even if these are meant for development and CI. Use appropriateentry_pointsinstead of shebangs. - CI tools: Add common development tools
-
pre-commit -
readthedocs -
codecov -
clang-tidy - other static code analyzers (e.g.
qodana), even for development scripts
-
- CI: Simplify the workflows, use as much external actions as possible, distinguish between main workflows and components. Dynamic and selective workflows (depending on changed files) is possible (I did one in Improve CI workflows swig/swig#2647)
- CI: Add Fedora CI through
packit - Packaging: Do not package bundled modules/files without marking them as such
I have done quite a bit of CMake implementation and modernization across multiple projects so I can come up with a PR addressing all of these issues over a weekend, if the team is onboard with such refactoring.