cmake: Bugfix and other improvements after bumping CMake up to 3.13

hebasto commented at 8:14 pm on March 12, 2023: member

This PR:

resolves two items from #1235, including a bugfix with package version compatibility
includes other improvements which have become available for CMake 3.13+.

To test the GENERATOR_IS_MULTI_CONFIG property on Linux, one can use the “Ninja Multi-Config” generator:

0cmake -S . -B build -G "Ninja Multi-Config"

hebasto cross-referenced this on Mar 12, 2023 from issue build: Meta-issue for follow-ups to initial CMake merge (#1113) by hebasto

hebasto renamed this:
~~cmake: Fix bug and other improvements after bumping CMake up to 3.13~~
cmake: Bugfix and other improvements after bumping CMake up to 3.13
on Mar 12, 2023

theuni commented at 6:01 pm on March 17, 2023: contributor

Concept ACK and quick review ACK, assuming #1238 goes in.

hebasto force-pushed on Mar 21, 2023

hebasto marked this as ready for review on Mar 21, 2023

hebasto commented at 3:39 pm on March 21, 2023: member

… assuming #1238 goes in.

Rebased on top of the merged #1238 and un-drafted.

in CMakeLists.txt:185 in 1b0ead1789 outdated

181@@ -177,7 +182,7 @@ if(cached_cmake_build_type)
182 endif()
183 
184 set(default_build_type "RelWithDebInfo")
185-if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
186+if(NOT CMAKE_BUILD_TYPE AND NOT GENERATOR_IS_MULTI_CONFIG)

real-or-random commented at 8:52 am on March 26, 2023:

0if(NOT GENERATOR_IS_MULTI_CONFIG)

https://cmake.org/cmake/help/latest/prop_gbl/GENERATOR_IS_MULTI_CONFIG.html says “Multi-config generators (…) ignore CMAKE_BUILD_TYPE.”, so I guess we should do the same?

hebasto commented at 10:09 am on March 26, 2023:

The NOT GENERATOR_IS_MULTI_CONFIG condition ensures that we use a single-config generator.

The NOT CMAKE_BUILD_TYPE condition checks whether we need to set the default build type. This is actually is the case when the user does not provide -DCMAKE_BUILD_TYPE=... on the command line.

hebasto commented at 10:20 am on March 26, 2023:

Going to update this branch shortly with a more readable implementation.

hebasto commented at 11:44 am on March 26, 2023:

Updated.

hebasto force-pushed on Mar 26, 2023

hebasto commented at 11:44 am on March 26, 2023: member

The commit “cmake: Use dedicated GENERATOR_IS_MULTI_CONFIG property” has been reworked entirely. Now the GENERATOR_IS_MULTI_CONFIG is treated correctly as a property, not as CMake variable.

hebasto force-pushed on Mar 26, 2023

hebasto commented at 7:09 pm on March 26, 2023: member

Added commit “cmake: Use if(... IN_LIST ...) command”.

in src/CMakeLists.txt:144 in e1f27737d2 outdated

140@@ -141,7 +141,7 @@ configure_package_config_file(
141   NO_SET_AND_CHECK_MACRO
142 )
143 write_basic_package_version_file(${PROJECT_NAME}-config-version.cmake
144-  COMPATIBILITY SameMajorVersion
145+  COMPATIBILITY SameMinorVersion

theuni commented at 9:12 pm on March 28, 2023:

Hmm, is this true? 0.4 will be incompatible?

real-or-random commented at 10:23 am on April 27, 2023:

see #1239 (review)

hebasto force-pushed on Apr 13, 2023

hebasto commented at 12:18 pm on April 13, 2023: member

Rebased 1341b4ba8c00ef7a4d182c0c5fa94d5e693f02e8 -> bf5a373e473d03821effc02c338cb0a425989074 (pr1239.07 -> pr1239.08) due to the conflict with #1268.

cmake: Use `SameMinorVersion` compatibility mode

Available in CMake 3.11+.

8a8b6536ef

cmake: Add `DESCRIPTION` and `HOMEPAGE_URL` options to `project` command

`DESCRIPTION` is available in CMake 3.9+.
`HOMEPAGE_URL` is available in CMake 3.12+.

04d4cc071a

cmake: Use recommended `add_compile_definitions` command

Available in CMake 3.12+.

8c2017035a

cmake: Use dedicated `CMAKE_HOST_APPLE` variable 9f8703ef17

cmake: Use dedicated `GENERATOR_IS_MULTI_CONFIG` property

Available in CMake 3.9+.

2445808c02

cmake: Use `if(... IN_LIST ...)` command

Available in CMake 3.3+.

6a58b483ef

cmake: Improve version comparison a273d74b2e

hebasto force-pushed on Apr 20, 2023

hebasto commented at 4:06 pm on April 20, 2023: member

Updated bf5a373e473d03821effc02c338cb0a425989074 -> a273d74b2ea1ef115a7e40fe89a64a6c744018c6 (pr1239.08 -> pr1239.09):

rebased due to the conflict with #1263
added a new “cmake: Improve version comparison” commit suggested by @real-or-random on IRC

in src/CMakeLists.txt:117 in a273d74b2e

113@@ -114,7 +114,7 @@ if(SECP256K1_INSTALL)
114     NO_SET_AND_CHECK_MACRO
115   )
116   write_basic_package_version_file(${PROJECT_NAME}-config-version.cmake
117-    COMPATIBILITY SameMajorVersion
118+    COMPATIBILITY SameMinorVersion

real-or-random commented at 11:07 am on April 21, 2023:

note: If we’d follow semver strictly, then that’s not true for versions pre 1.0.0.

But I think it’s good to add “SameMinorVersion” here. It seems that we have agreed on bumping minor for incompatible API changes even pre 1.0.0. (We bumped 0.2.0 to 0.3.0 for example because of this.)

(The rationale in semver is that you should release 1.0.0 as soon as people rely on the API. We don’t do that either, so I think what we do makes sense in the end, even though we deviate from semver.)

real-or-random approved

real-or-random commented at 11:20 am on April 21, 2023: contributor

ACK a273d74b2ea1ef115a7e40fe89a64a6c744018c6

real-or-random commented at 11:21 am on April 21, 2023: contributor

(Not this PR:) This is not rejected: cmake -B build -DCMAKE_BUILD_TYPE=yolo. Maybe it should be rejected.

theuni approved

theuni commented at 10:22 am on April 27, 2023: contributor

ACK a273d74b2ea1ef115a7e40fe89a64a6c744018c6

real-or-random merged this on Apr 27, 2023

real-or-random closed this on Apr 27, 2023

hebasto deleted the branch on Apr 27, 2023

sipa referenced this in commit b4eb644b6c on May 12, 2023

hebasto referenced this in commit 49c52ea2b1 on May 13, 2023

vmta referenced this in commit e1120c94a1 on Jun 4, 2023

sipa referenced this in commit 901336eee7 on Jun 21, 2023

vmta referenced this in commit 8f03457eed on Jul 1, 2023

jonasnick cross-referenced this on Jul 24, 2023 from issue Upstream PRs 1268, 1276, 1267, 1265, 1230, 1279, 1273, 1263, 1231, 1285, 1283, 1205, 1286, 1275, 1234, 1239, 1240, 1284, 1277, 1289, 1270, 1296, 1301, 1299, 1066, 1300, 1292, 1305, 1303, 1133, 1306, 1207, 1304, 1307, 1311, 1309, 1312 by jonasnick

cmake: Bugfix and other improvements after bumping CMake up to 3.13 #1239