compile-time-regular-expressions icon indicating copy to clipboard operation
compile-time-regular-expressions copied to clipboard

Improve the quality of the CMake build scripts

Open friendlyanon opened this issue 4 years ago • 5 comments
trafficstars

This commit does a couple changes:

  • Actually adheres to the declared CMake 3.8 requirement
    • HOMEPAGE_URL in project() was added in 3.12
    • install(TARGETS) requires the DESTINATION argument
    • Tests' CML uses CONFIGURE_DEPENDS from CMake 3.12
  • Removes unnecessary things
    • Looking for RPM and DEB packagers is pointless
    • Misc arguments to CMake commands
  • Improves correctness
    • Includes CPack and CTest modules only if top level
    • Assigns install rules to a project specific component to not clobber the global, Unspecified one
    • Adds a warning guard for projects that vendor ctre
    • CMAKE_INSTALL_PREFIX should be set at configure time if the person building wants .pc files, so attempt to detect that error
    • Enables the install rules to be opt-out using the CMake built-in variable
    • Emulates ARCH_INDEPENDENT for the version config
    • Installs headers to their own directory (fixes #207)
    • Use CTest for testing
  • Small optimization
    • LANGUAGES can be set to NONE, because this is a header only library

friendlyanon avatar Nov 08 '21 02:11 friendlyanon

Hmm, I thought about the pkgconfig code and it's actually not quite right as it is now either.

The problem with generating a pkgconfig file via CMake is that it does not fit the model of CMake. The prefix variable has to be absolute in the .pc file, however CMake deals with paths relative to the prefix. The final prefix can change at all times, via CMAKE_INSTALL_PREFIX, --prefix parameter during install time, at CPack time and the resulting deb/rpm files can be extracted anywhere as well. If one needs a pkgconfig file for packaging purposes, both deb and rpm CPack generators allow the user to add post-install scripts that can generate it if necessary.

As a clarification: the case where the user is installing with cmake --install or the install target both work fine, but packaging is where things break due to the prefix variable needing to be absolute in the .pc file.

friendlyanon avatar Nov 11 '21 14:11 friendlyanon

It seems that when you run

cmake .
make test

Screenshot from 2021-11-14 18-18-00

alexios-angel avatar Nov 14 '21 23:11 alexios-angel

You are correct. The project doesn't use the normal test mechanisms for whatever reason.

https://github.com/hanickadot/compile-time-regular-expressions/blob/b51d3290865615487cbef75c7c9dba36b9e70c6a/tests/CMakeLists.txt#L1-L10

Note the lack of add_test calls. I think that could be added to this PR as well, since that fits the topic.
I will rebase the first commit and add the appropriate changes there.

friendlyanon avatar Nov 14 '21 23:11 friendlyanon

I cleaned up things around the CPack config and pkgconfig.

The CPack config with the project specific defaults is configured to the CPack(Source)Config-ctre.cmake file in the root binary directory. These files include() the CMake generated ones and overwrite some values with set() calls. Using these files can be done with cpack --config ....

The .pc file now uses the configure time CMAKE_INSTALL_PREFIX variable to get its prefix value. This makes it possible to anticipate ahead of time where the generated package files will be installed to. There is a warning emitted during the very first configuration if the pkgconfig option is enabled without CMAKE_INSTALL_PREFIX also being set. If the project is installed with cmake --install ... --prefix ... however, then the .pc file will contain the wrong prefix. This sould probably be documented in https://github.com/hanickadot/compile-time-regular-expressions/pull/231 cc @angeletakis

friendlyanon avatar Nov 24 '21 21:11 friendlyanon

@friendlyanon Will do 💯

alexios-angel avatar Nov 24 '21 21:11 alexios-angel