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

pull hebasto wants to merge 7 commits into bitcoin-core:master from hebasto:230312-modernize changing 4 files +44 −38
  1. 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:

    cmake -S . -B build -G "Ninja Multi-Config"
  2. 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
  3. theuni commented at 6:01 PM on March 17, 2023: contributor

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

  4. hebasto force-pushed on Mar 21, 2023
  5. hebasto marked this as ready for review on Mar 21, 2023
  6. 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.

  7. 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: 
    if(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.

  8. hebasto force-pushed on Mar 26, 2023
  9. 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.

  10. hebasto force-pushed on Mar 26, 2023
  11. hebasto force-pushed on Mar 26, 2023
  12. hebasto force-pushed on Mar 26, 2023
  13. hebasto commented at 7:09 PM on March 26, 2023: member

    Added commit "cmake: Use if(... IN_LIST ...) command".

  14. 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)

  15. hebasto force-pushed on Apr 13, 2023
  16. hebasto commented at 12:18 PM on April 13, 2023: member

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

  17. cmake: Use `SameMinorVersion` compatibility mode
    Available in CMake 3.11+.
    8a8b6536ef
  18. 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
  19. cmake: Use recommended `add_compile_definitions` command
    Available in CMake 3.12+.
    8c2017035a
  20. cmake: Use dedicated `CMAKE_HOST_APPLE` variable 9f8703ef17
  21. cmake: Use dedicated `GENERATOR_IS_MULTI_CONFIG` property
    Available in CMake 3.9+.
    2445808c02
  22. cmake: Use `if(... IN_LIST ...)` command
    Available in CMake 3.3+.
    6a58b483ef
  23. cmake: Improve version comparison a273d74b2e
  24. hebasto force-pushed on Apr 20, 2023
  25. 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
  26. 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.)

  27. real-or-random approved
  28. real-or-random commented at 11:20 AM on April 21, 2023: contributor

    ACK a273d74b2ea1ef115a7e40fe89a64a6c744018c6

  29. 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.

  30. theuni approved
  31. theuni commented at 10:22 AM on April 27, 2023: contributor

    ACK a273d74b2ea1ef115a7e40fe89a64a6c744018c6

  32. real-or-random merged this on Apr 27, 2023
  33. real-or-random closed this on Apr 27, 2023
  34. hebasto deleted the branch on Apr 27, 2023
  35. sipa referenced this in commit b4eb644b6c on May 12, 2023
  36. hebasto referenced this in commit 49c52ea2b1 on May 13, 2023
  37. vmta referenced this in commit e1120c94a1 on Jun 4, 2023
  38. sipa referenced this in commit 901336eee7 on Jun 21, 2023
  39. vmta referenced this in commit 8f03457eed on Jul 1, 2023
  40. delta1 referenced this in commit 3f32c20932 on Aug 8, 2023
  41. delta1 referenced this in commit 31ac0c1081 on Aug 31, 2023
  42. janus referenced this in commit c4348d88db on Sep 11, 2023
  43. div72 referenced this in commit af627d47c3 on Apr 12, 2025
  44. str4d referenced this in commit 5a6bf5f178 on Jun 4, 2025
Contributors
hebastotheunireal-or-random
Linked (view graph)
#1235 build: Meta-issue for follow-ups to initial CMake merge (#1113)
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:52 UTC