cmake: Increase cmake policy version #209

ryanofsky · 2025-09-09T18:40:19Z

Increase cmake policy version from 3.12 to 4.1 to stop using old and deprecated CMake policies in standalone builds.

Also stop overriding policy version if a cmake parent project has already set one, so parent projects are able to control which policies are enabled by default based on their own needs. Specifically, in the Bitcoin Core subtree, this change causes the libmultiprocess cmake policy version to increase from 3.12 to 3.22, which is the version Bitcoin Core uses.

This PR also adds a new newdeps CI job to test CMake 4.1 and the master branch of Cap'n Proto. This PR doesn't change the minimum version of cmake required to build the project, which is still 3.12.

This PR is an alternative to #163 which increases the policy version to 3.31 but doesn't include fixes for CI jobs, and doesn't allow the bitcoin core build to choose a lower policy version. This PR is also an alternative to #175 which sets the policy version to 3.22 like the bitcoin build, but also causes builds with earlier versions of cmake to fail.

DrahtBot · 2025-09-09T18:40:32Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#175 (Set cmake_minimum_required(VERSION 3.22) by maflcko)
#163 (build: set cmake policy version to 3.31 by purpleKarrot)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

ryanofsky · 2025-09-09T19:23:06Z

Updated 6e29c02 -> 233b394 (pr/policy.1 -> pr/policy.2, compare) with a few fixes for CI errors: openbsd and freebsd git log errors and newdeps git clone TLS certificate error

Updated 233b394 -> 31206fc (pr/policy.2 -> pr/policy.3, compare) with more CI fixes: openbsd and freebsd FindThreads errors and newdeps capnproto build openssl include error

Updated 31206fc -> e6fd980 (pr/policy.3 -> pr/policy.4, compare) to fix more CI errors: openbsd and freebsd FindThreads errors and new llvm build FindThreads error.

Updated e6fd980 -> c33b812 (pr/policy.4 -> pr/policy.5, compare) to fix CI errors: openbsd and freebsd "/bin/sh: CMAKE_CXX_COMPILER_CLANG_SCAN_DEPS-NOTFOUND: not found" error, and default and llvm IWYU errors

Updated c33b812 -> 1c2958b (pr/policy.5 -> pr/policy.6, compare) with better debug output and better commit message after figuring out root cause of the find_package threads failure

CMakeLists.txt

With latest cmake policies, find_package(Threads REQUIRED) fails in freebsd and openbsd builds. The specific failures in these jobs happens due to the CMP0155 policy turning CMAKE_CXX_SCAN_FOR_MODULES on, which is turned off in the next commit. But there are other things that could cause the threads package not to be found, and there are platforms where it may not be required, so it is better to make it an optional instead of required dependency. Technically this change is not needed to make all CI jobs pass as long as the next commit is present. But since this find_package error obscures other errors that would be clearer, and since there are other conditions that could trigger it, it is worth fixing generally. For example, this same failure also seems to happen in the llvm job as well when CMP0137 is disabled or CMAKE_TRY_COMPILE_NO_PLATFORM_VARIABLES is set to true. Error looks like: + cmake /home/runner/work/libmultiprocess/libmultiprocess -G Ninja -- The CXX compiler identification is Clang 16.0.6 -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Check for working CXX compiler: /usr/bin/c++ - skipped -- Detecting CXX compile features -- Detecting CXX compile features - done -- Performing Test CMAKE_HAVE_LIBC_PTHREAD -- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Failed -- Looking for pthread_create in pthreads -- Looking for pthread_create in pthreads - not found -- Looking for pthread_create in pthread -- Looking for pthread_create in pthread - not found -- Check if compiler accepts -pthread -- Check if compiler accepts -pthread - no CMake Error at /usr/local/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:233 (message): Could NOT find Threads (missing: Threads_FOUND) Call Stack (most recent call first): /usr/local/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:603 (_FPHSA_FAILURE_MESSAGE) /usr/local/share/cmake/Modules/FindThreads.cmake:226 (FIND_PACKAGE_HANDLE_STANDARD_ARGS) CMakeLists.txt:41 (find_package) and inside the CMakeConfigureLog.yaml file there are "/bin/sh: CMAKE_CXX_COMPILER_CLANG_SCAN_DEPS-NOTFOUND: not found" errors.

… cmake policies With latest cmake policies, openbsd and freebsd CI jobs fail due to a lack of a clang-scan-deps tool. The tool could potentially be installed on these platforms but it is unclear how to do that and the project isn't using modules anyway, so just disable them here. Errors look like: + cmake --build . --parallel -t all tests mpexamples -- -k 0 [1/114] Scanning /home/runner/work/libmultiprocess/libmultiprocess/src/mp/util.cpp for CXX dependencies FAILED: CMakeFiles/mputil.dir/src/mp/util.cpp.o.ddi "CMAKE_CXX_COMPILER_CLANG_SCAN_DEPS-NOTFOUND" -format=p1689 -- /usr/bin/c++ -I/home/runner/work/libmultiprocess/libmultiprocess/include -I/home/runner/work/libmultiprocess/libmultiprocess/build-openbsd/include -isystem /usr/local/include -Werror -Wall -Wextra -Wpedantic -Wno-unused-parameter -std=gnu++20 -x c++ /home/runner/work/libmultiprocess/libmultiprocess/src/mp/util.cpp -c -o CMakeFiles/mputil.dir/src/mp/util.cpp.o -resource-dir "/usr/lib/clang/16" -MT CMakeFiles/mputil.dir/src/mp/util.cpp.o.ddi -MD -MF CMakeFiles/mputil.dir/src/mp/util.cpp.o.ddi.d > CMakeFiles/mputil.dir/src/mp/util.cpp.o.ddi.tmp && mv CMakeFiles/mputil.dir/src/mp/util.cpp.o.ddi.tmp CMakeFiles/mputil.dir/src/mp/util.cpp.o.ddi /bin/sh: CMAKE_CXX_COMPILER_CLANG_SCAN_DEPS-NOTFOUND: not found

Increase cmake policy version from 3.12 to 4.1 in standalone builds to stop using very old and deprecated CMake policies in standalone builds. Also stop overriding policy version if a parent project has already set one, so parent projects are able to control which policies are used by default based on their own practices and needs. In the Bitcoin Core subtree, this change causes the libmultiprocess policy version to increase from 3.12 to 3.22, which is the version Bitcoin Core sets. This commit only changes the policy version, it does not change the specified minimum version of cmake required to build the project, which is 3.12.

hebasto · 2025-09-15T12:34:12Z

ci/scripts/ci.sh

+  cmake --build . --parallel -t "${BUILD_TARGETS[@]}" -- "${BUILD_ARGS[@]+"${BUILD_ARGS[@]}"}"
 else
  # Older versions of cmake can only build one target at a time with --target,
  # and do not support -t shortcut
  for t in "${BUILD_TARGETS[@]}"; do
-    cmake --build . --target "$t" -- "${BUILD_ARGS[@]+"${BUILD_ARGS[@]}"}"
+    cmake --build . --parallel --target "$t" -- "${BUILD_ARGS[@]+"${BUILD_ARGS[@]}"}"


6ba1050

If the --parallel option is used without an argument:

the native build tool's default number is used.

This means that the build will be efficiently parallelized with the "Ninja" generator, but not with the "Unix Makefiles" generator. However, not all scripts in ci/configs/ pass -G Ninja.

re: #209 (comment)

This means that the build will be efficiently parallelized with the "Ninja" generator, but not with the "Unix Makefiles" generator. However, not all scripts in ci/configs/ pass -G Ninja.

On my system I see it passing -j to make which seems to work well in practice because this is a small build. Would happily accept a PR or patch to do something different, but I think this change is an improvement.

hebasto · 2025-09-15T12:36:24Z

ci/scripts/ci.sh

+  # Could add --trace / --trace-expand here too but they are very verbose.
+  cmake_args+=(--debug-find --debug-output --debug-trycompile --log-level=DEBUG)
+  cmake "$src_dir" "${cmake_args[@]}" || : "cmake exited with $?"
+  cat CMakeFiles/CMakeConfigureLog.yaml || true


6ba1050

This log file name has been used only since CMake 3.26.

re: #209 (comment)

6ba1050

This log file name has been used only since CMake 3.26.

Yes that's part of the reason for adding || true here. I think this code could potentially print log files used by older versions of cmake too, and originally this change tried that, but since all CI jobs except one are using new enough versions of cmake it didn't seem worth complexity. Would happily accept a PR to patch to improve this though.

purpleKarrot · 2025-09-15T13:28:32Z

I am not happy with this. The title of the topic says "Increase cmake policy version", which I would expect to be a one line change. Instead, it has +95/-17 changes. The changes themselves are questionable:

the length of the bash buildscript is doubled. Why isn't this written in a portable cmake script? Why is CMake preferred over handwritten Makefiles when the actual CI logic is written in bash?
The ``cmake_minimum_required()` call has a 25 line comment to explain a rationale. If you have to explain what you are doing, you are doing it wrong.
Putting Threads::Threads behind a non-namespaced abstraction mpdeps makes the build less robust.

Please don't proceed in this direction. Make sure the CMakeLists.txt files are declarative and follow best practices instead of inventing new approaches and deviations.

hebasto · 2025-09-15T13:30:19Z

I'm struggling to create a MRE for the issue with the FindThreads module on FreeBSD. The following works just fine:

cmake_minimum_required(VERSION 3.12...4.1)
project(freebsd-threads CXX)
set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED YES)
find_package(Threads REQUIRED)

What am I missing?

ryanofsky · 2025-09-15T14:51:12Z

re: #209 (comment)

I am not happy with this. The title of the topic says "Increase cmake policy version", which I would expect to be a one line change. Instead, it has +95/-17 changes.

Happy to change the title if you have a suggestion. This PR is only increasing the policy version and improving the CI, and I did not feel the CI changes were were mentioning since they are supporting changes, not the goal of the PR.

* the  length of the bash buildscript is doubled. Why isn't this written in a portable cmake script? Why is CMake preferred over handwritten Makefiles when the actual CI logic is written in bash?

I think the language question should be raised new issue if you want to discuss it. (But just to provide a little context here if it can avoid an argument about the language of a ~70 line script, I just personally find bash a lot nicer and more useful than cmake and am much familiar with it. More generally, bash is commonly used in CI, there many more developers who know bash than cmake.)

The script does increase from 37 to 67 lines here to do two things:

To support cloning and building unreleased versions of cap'n proto, so we can fix or report compatibility issues upstream cap'n proto earlier, and not be blindsided by new changes.
To provide better output when find_package fails, because by default the information cmake provides is not useful for debugging.

IMO both changes are worth making and do not have any real downsides.

If you have to explain what you are doing, you are doing it wrong.

Interesting point. It might be helpful if you could explain practical downsides of the change. Unless you also believe that if you have to explain why a change is wrong, then it must be right?

Putting Threads::Threads behind a non-namespaced abstraction mpdeps makes the build less robust.

Can you explain this a little more as well? Is there a practical downside here or a different approach you would recommend?

ryanofsky · 2025-09-15T15:11:26Z

re: #209 (comment)

I'm struggling to create a MRE for the issue with the FindThreads module on FreeBSD. The following works just fine:

The error happens when you don't have clang-scan-deps installed on the build machine, as seems to be the case currently with our freebsd and openbs vms. This issue is not specifically related to the find_package(Threads REQUIRED) call but the find_package(Threads REQUIRED) call makes it harder to understand what is wrong because it obscures the real error. Making the threads package optional produces a clearer error later, and I think it's just more correct anyway since modern platforms don't use separate threading libraries.

If it helps there is a failing CI run with more complete output:

https://github.com/ryanofsky/libmultiprocess/actions/runs/17622518177/job/50071319558

And the relevant cmake policy is https://cmake.org/cmake/help/latest/policy/CMP0155.html

ryanofsky · 2025-09-15T15:28:39Z

Make sure the CMakeLists.txt files are declarative and follow best practices instead of inventing new approaches and deviations.

To be sure, I agree with you it would be better if Bitcoin Core followed cmake's recommended best practices and used new policies instead of setting an really old policy version.

But bitcoin core isn't doing that, and other bitcoin core developers have given reasons why it shouldn't do that, and while I don't agree with these reasons, I think it is a reasonable judgement call, and they are just choosing a different tradeoffs than I would. This PR makes libmultiprocess compatible with either approach for choosing a policy version and stops using really old policies (older than the ones bitcoin core chooses) in any case.

hebasto · 2025-09-15T15:37:15Z

The error happens when you don't have clang-scan-deps installed on the build machine, as seems to be the case currently with our freebsd and openbs vms.

Here is a build log from GHA using FreeBSD:

-- The CXX compiler identification is Clang 19.1.7
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Failed
-- Looking for pthread_create in pthreads
-- Looking for pthread_create in pthreads - not found
-- Looking for pthread_create in pthread
-- Looking for pthread_create in pthread - found
-- Found Threads: TRUE
-- Configuring done (1.2s)
-- Generating done (0.0s)
-- Build files have been written to: /home/runner/work/github-actions/github-actions/build

ryanofsky · 2025-09-15T16:04:37Z

Here is a build log from GHA using FreeBSD:

Is that link right? It seems to be a 404 for me. I also don't think I understand what it indicates if find_package(Threads REQUIRED) works in another build. We already know it works in many builds. You can see some builds where it is failing
in #209 (comment) and https://github.com/ryanofsky/libmultiprocess/actions/runs/17622518177/job/50071319558. Making the threads library optional improves error output, works well and doesn't have other downsides, and makes sense conceptually on modern platforms that do not use separate libraries for threading.

hebasto · 2025-09-15T16:13:24Z

Here is a build log from GHA using FreeBSD:

Is that link right? It seems to be a 404 for me.

Sorry. I've made that repo public. The link should work now.

I also don't think I understand what it indicates if find_package(Threads REQUIRED) works in another build. We already know it works in many builds. You can see some builds where it is failing in #209 (comment) and https://github.com/ryanofsky/libmultiprocess/actions/runs/17622518177/job/50071319558. Making the threads library optional improves error output, works well and doesn't have other downsides, and makes sense conceptually on modern platforms that do not use separate libraries for threading.

I want to report the issue upstream and I need an MRE for it.

ryanofsky · 2025-09-15T16:41:47Z

I want to report the issue upstream and I need an MRE for it.

Oh, thanks! It seems like your branch is https://github.com/hebasto/github-actions/compare/250915-freebsd-cmake-threads and I agree I don't see an obvious reason why that CI succeeds while my CI runs such as https://github.com/ryanofsky/libmultiprocess/actions/runs/17622518177/job/50071319558 with branch https://github.com/ryanofsky/libmultiprocess/commits/citest/policy at 3b8a0e6 failed. Some things I might suggest:

Starting from my failing commit and deleting unnecessary steps to find a minimal failing case
Printing yaml build log unconditionally instead of only on failure in your current branch
Callling clang-scan-deps --version or seeing if it is available

ryanofsky · 2025-09-15T16:44:11Z

I want to report the issue upstream and I need an MRE for it.

Also, I don't think there is necessarily there is necessarily an upstream bug here, since we are asking cmake to look for a package and it fails and reports the failure. It just doesn't seem to do a good job reporting the cause of the failure.

hebasto · 2025-09-15T22:05:57Z

@ryanofsky

Some things I might suggest:

Starting from my failing commit and deleting unnecessary steps to find a minimal failing case

Thank you for the suggestion.

The culprit turned out to be the "Ninja" generator, which was quite surprising to me.

ryanofsky · 2025-09-15T23:08:32Z

The culprit turned out to be the "Ninja" generator, which was quite surprising to me.

Nice! And that is really surprising. I would expect the generator not to matter much during configuration.

ryanofsky

Thanks for the review! Note: first two commits are now part of #212

ryanofsky · 2025-09-17T09:44:15Z

ci/scripts/ci.sh

+  # Could add --trace / --trace-expand here too but they are very verbose.
+  cmake_args+=(--debug-find --debug-output --debug-trycompile --log-level=DEBUG)
+  cmake "$src_dir" "${cmake_args[@]}" || : "cmake exited with $?"
+  cat CMakeFiles/CMakeConfigureLog.yaml || true


re: #209 (comment)

6ba1050

This log file name has been used only since CMake 3.26.

Yes that's part of the reason for adding || true here. I think this code could potentially print log files used by older versions of cmake too, and originally this change tried that, but since all CI jobs except one are using new enough versions of cmake it didn't seem worth complexity. Would happily accept a PR to patch to improve this though.

ryanofsky · 2025-09-17T09:44:27Z

ci/scripts/ci.sh

+  cmake --build . --parallel -t "${BUILD_TARGETS[@]}" -- "${BUILD_ARGS[@]+"${BUILD_ARGS[@]}"}"
 else
  # Older versions of cmake can only build one target at a time with --target,
  # and do not support -t shortcut
  for t in "${BUILD_TARGETS[@]}"; do
-    cmake --build . --target "$t" -- "${BUILD_ARGS[@]+"${BUILD_ARGS[@]}"}"
+    cmake --build . --parallel --target "$t" -- "${BUILD_ARGS[@]+"${BUILD_ARGS[@]}"}"


re: #209 (comment)

This means that the build will be efficiently parallelized with the "Ninja" generator, but not with the "Unix Makefiles" generator. However, not all scripts in ci/configs/ pass -G Ninja.

On my system I see it passing -j to make which seems to work well in practice because this is a small build. Would happily accept a PR or patch to do something different, but I think this change is an improvement.

ci: add newdeps job testing newest versions of cmake and capnproto

0ffdb14

ryanofsky mentioned this pull request Sep 9, 2025

build: set cmake policy version to 3.31 #163

Open

ryanofsky force-pushed the pr/policy branch from 6e29c02 to 233b394 Compare September 9, 2025 19:22

ryanofsky force-pushed the pr/policy branch from 233b394 to 31206fc Compare September 9, 2025 20:19

hebasto reviewed Sep 9, 2025

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

ryanofsky added 2 commits September 10, 2025 07:06

ci: add cmake debug output and --parallel build option

6ba1050

ryanofsky force-pushed the pr/policy branch from 31206fc to e6fd980 Compare September 10, 2025 11:08

ryanofsky added 2 commits September 10, 2025 10:50

ryanofsky force-pushed the pr/policy branch from e6fd980 to c33b812 Compare September 10, 2025 15:07

DrahtBot mentioned this pull request Sep 10, 2025

Set cmake_minimum_required(VERSION 3.22) #175

Open

ryanofsky force-pushed the pr/policy branch from c33b812 to 1c2958b Compare September 10, 2025 18:25

hebasto reviewed Sep 15, 2025

View reviewed changes

ryanofsky mentioned this pull request Sep 17, 2025

ci: add newdeps job testing newer versions of cmake and capnproto #212

Open

ryanofsky commented Sep 17, 2025

View reviewed changes

cmake: Increase cmake policy version #209

Are you sure you want to change the base?

cmake: Increase cmake policy version #209

Uh oh!

Conversation

ryanofsky commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

ryanofsky commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

hebasto Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

ryanofsky Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

hebasto Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

ryanofsky Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

purpleKarrot commented Sep 15, 2025

Uh oh!

hebasto commented Sep 15, 2025

Uh oh!

ryanofsky commented Sep 15, 2025

Uh oh!

ryanofsky commented Sep 15, 2025

Uh oh!

ryanofsky commented Sep 15, 2025

Uh oh!

hebasto commented Sep 15, 2025

Uh oh!

ryanofsky commented Sep 15, 2025

Uh oh!

hebasto commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanofsky commented Sep 15, 2025

Uh oh!

ryanofsky commented Sep 15, 2025

Uh oh!

hebasto commented Sep 15, 2025

Uh oh!

ryanofsky commented Sep 15, 2025

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

ryanofsky Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

ryanofsky Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ryanofsky commented Sep 9, 2025 •

edited

Loading

DrahtBot commented Sep 9, 2025 •

edited

Loading

ryanofsky commented Sep 9, 2025 •

edited

Loading

hebasto commented Sep 15, 2025 •

edited

Loading