search

mpusz / mp-units

The quantities and units library for C++
https://mpusz.github.io/mp-units/
MIT License
1.01k stars 80 forks source link

Inconsistent cmake_minimum_required #592

Closed psalvaggio closed 1 week ago

psalvaggio commented 2 weeks ago

In the top-level CMakeLists.txt, the version required is 3.23. In test/runtime/CMakeLists.txt, it is 3.5. This results in CMP0074 not being set in the test directory. As a result, specifying dependency locations via _ROOT cache variables doesn't work for Catch2 and wg21_linear_algebra, while it does work for the other dependencies. Was there a reason there is a different version set in the test file?

mpusz commented 2 weeks ago

I typically set the minimum required for a specific script file so it might be easier ported between various project. However, in this case it seems that the script could benefit from the higher version of CMake.

psalvaggio commented 2 weeks ago

I see. Yeah, so on the documentation page: https://cmake.org/cmake/help/latest/command/cmake_minimum_required.html It says that cmake_minimum_required should be called at the beginning of the top-level CMakeLists.txt so that policy settings are established first.

Due to the side effect here: https://cmake.org/cmake/help/latest/command/cmake_minimum_required.html#policy-settings, calling it multiple times can lead to changing of the policy settings throughout your project, as we are seeing here. find_package has a couple important ones that have changed in the 3.x series surrounding these _ROOT variables for explicitly specifying where your dependencies live.