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

ryanofsky commented at 9:42 AM on September 17, 2025: collaborator

Add CI job testing latest released version of cmake, and latest unreleased version of cap'n proto to uncover any problems there may be with new releases. Use git version of cap'n proto because we've submitted patches to that project to fix issues in the past, so it would be good to know about any new issues as soon as possible.

DrahtBot commented at 9:42 AM on September 17, 2025: none

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

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	maflcko, hebasto

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#353 (ci: add cmake --parallel build option by ryanofsky)
#352 (ci: add cmake debug output by ryanofsky)
#333 (nix: Make clang tooling and IWYU find the right standard library headers by ryanofsky)
#231 (Add windows support by ryanofsky)
#209 (cmake: Increase cmake policy version by ryanofsky)
#175 (Set cmake_minimum_required(VERSION 3.22) by maflcko)

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 commented at 9:43 AM on September 17, 2025: collaborator

These changes were originally part of #209 but moved them to a separate PR since #209 might be blocked or require more time to discuss

in ci/scripts/ci.sh:48 in 6ba1050bef outdated

  44 | +git --no-pager log -1 || true
  45 | +cmake_args=("${CMAKE_ARGS[@]+"${CMAKE_ARGS[@]}"}")
  46 | +if ! cmake "$src_dir" "${cmake_args[@]}"; then
  47 | +  # If cmake failed, try it again with debug options.
  48 | +  # Could add --trace / --trace-expand here too but they are very verbose.
  49 | +  cmake_args+=(--debug-find --debug-output --debug-trycompile --log-level=DEBUG)

hebasto commented at 10:55 AM on September 18, 2025:

While this project maintains support for CMake versions as old as 3.12, I believe this support should also extend consistently to the CI infrastructure, particularly given the "olddeps" job, which runs CMake 3.12.4. Therefore, features incompatible with CMake 3.12, such as --debug-find and --log-level, should be avoided. For example, the "olddeps" CI job may result in:

+ cmake /home/runner/work/libmultiprocess/libmultiprocess --debug-find --debug-output --debug-trycompile --log-level=DEBUG
CMake Error: The source directory "/home/runner/work/libmultiprocess/libmultiprocess/build-olddeps/--log-level=DEBUG" does not exist.

Alternatively, the minimum supported CMake version could be bumped.

ryanofsky commented at 3:46 PM on September 18, 2025:

re: #212 (review)

For example, the "olddeps" CI job may result in

If the problem you would like me to solve is that extra output potentially appearing in the olddeps job, it is trivial to skip the debug step there with:

-if ! cmake "$src_dir" "${cmake_args[@]}"; then
+if ! cmake "$src_dir" "${cmake_args[@]}" && ver_ge "$cmake_ver" "3.17"; then

But I don't see a problem with this extra output and I don't know if this the issue you actually care about. If you are arguing that systems using cmake 3.16 should not be supported because it does not support the --debug-find option, than that sounds like something to mention in #175 more than here.

in ci/scripts/ci.sh:50 in 6ba1050bef outdated

  46 | +if ! cmake "$src_dir" "${cmake_args[@]}"; then
  47 | +  # If cmake failed, try it again with debug options.
  48 | +  # Could add --trace / --trace-expand here too but they are very verbose.
  49 | +  cmake_args+=(--debug-find --debug-output --debug-trycompile --log-level=DEBUG)
  50 | +  cmake "$src_dir" "${cmake_args[@]}" || : "cmake exited with $?"
  51 | +  cat CMakeFiles/CMakeConfigureLog.yaml || true

hebasto commented at 11:01 AM on September 18, 2025:

Moving discussion from #209 (review).

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.

One may argue: why introduce complexity for a feature that doesn’t work with the minimum supported CMake version in the first place?

Would happily accept a PR to patch to improve this though.

This would make sense if it were the only compatibility issue in this PR.

ryanofsky commented at 3:35 PM on September 18, 2025:

re: #212 (review)

One may argue: why introduce complexity for a feature that doesn’t work with the minimum supported CMake version in the first place?

It would be helpful to know what complexity you are referring to specifically here. The find_packge(Threads REQUIRED) issue would have been impossible to debug without the line:

cat CMakeFiles/CMakeConfigureLog.yaml || true

and I do not feel like it adds much complexity. This CI script in general seems short and simple to me. I don't know what problem you have with the other debug prints either. In general, I feel like it would be a lot easier to respond to your comments if they could make it clear:

What observable problem you think this change causes, or could potentially cause.
What change you would suggest to fix the problem.

ryanofsky commented at 3:55 PM on September 18, 2025: collaborator

Appreciate your review, even though I am having trouble understanding what your practical concerns are and what specific changes I could make that would address them.

Hopefully we both accept the premise that it is good to have a new CI job testing newer versions of cmake and capnproto, and it is good to show more debug output when cmake configuration fails.

in ci/scripts/ci.sh:43 in 0ffdb140bf outdated

  38 | +fi
  39 | +
  40 |  src_dir=$PWD
  41 |  mkdir -p "$CI_DIR"
  42 |  cd "$CI_DIR"
  43 | +git --no-pager log -1 || true

maflcko commented at 9:03 AM on September 22, 2025:

is there a ci config that does not have git? I can't see one and it seems easy enough to just require it, so that logs always have the commit embedded?

ryanofsky commented at 9:17 PM on August 25, 2026:

re: #212 (review)

is there a ci config that does not have git? I can't see one and it seems easy enough to just require it, so that logs always have the commit embedded?

Sounds like the request here is to remove || true?

I'd slightly prefer to keep it just because these CI scripts are useful for testing locally even outside context of a real CI system, and there are reasons it can be useful to copy files around and not have valid git checkouts during testing. But it is true that a drawback of || true is that it could allow someone to add a CI job that does not have print the commit hash here. So I'd be ok with a change dropping || true in my planned followup, just let me know.

fdtwd8vv45-sketch approved

DrahtBot added the label Needs rebase on Aug 11, 2026

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

ryanofsky force-pushed on Aug 14, 2026

ryanofsky commented at 5:10 PM on August 14, 2026: collaborator

Rebased 6ba1050bef0b5ea5df35d3623c9e101cceb82677 -> 03b147e5cbb6b8b4d362f57304a29a14031002e5 (pr/newdeps.1 -> pr/newdeps.2, compare) due to conflict with #334

Updated 03b147e5cbb6b8b4d362f57304a29a14031002e5 -> b3fc922ee4a663811e0048556b788b4ca473d781 (pr/newdeps.2 -> pr/newdeps.3, compare) dropping debug commit 03b147e5cbb6b8b4d362f57304a29a14031002e5

DrahtBot removed the label Needs rebase on Aug 14, 2026

ryanofsky force-pushed on Aug 20, 2026

in ci/configs/newdeps.bash:4 in b3fc922ee4

   0 | @@ -0,0 +1,6 @@
   1 | +CI_DESC="CI job using newest Cap'n Proto and cmake versions"
   2 | +CI_DIR=build-newdeps
   3 | +export CXXFLAGS="-Werror -Wall -Wextra -Wpedantic -Wno-unused-parameter -Wno-error=array-bounds"
   4 | +CAPNP_CHECKOUT=master

maflcko commented at 5:06 AM on August 21, 2026:

CAPNP_CHECKOUT=master  # This is the v1.x release branch

The default branch is v2, maybe clarify what master means?

ryanofsky commented at 9:18 PM on August 25, 2026:

re: #212 (review)

The default branch is v2, maybe clarify what master means?

Good suggestion, have added in followup

maflcko commented at 5:06 AM on August 21, 2026: contributor

lgtm ACK b3fc922ee4a663811e0048556b788b4ca473d781

hebasto approved

hebasto commented at 1:46 PM on August 23, 2026: member

ACK b3fc922ee4a663811e0048556b788b4ca473d781.

Add CI job testing latest released version of cmake...

Could be updated as follows:

--- a/ci/configs/newdeps.bash
+++ b/ci/configs/newdeps.bash
@@ -2,5 +2,5 @@ CI_DESC="CI job using newest Cap'n Proto and cmake versions"
 CI_DIR=build-newdeps
 export CXXFLAGS="-Werror -Wall -Wextra -Wpedantic -Wno-unused-parameter -Wno-error=array-bounds"
 CAPNP_CHECKOUT=master
-NIX_ARGS=(--argstr capnprotoVersion "none" --argstr cmakeVersion "4.1.1")
+NIX_ARGS=(--argstr capnprotoVersion "none" --argstr cmakeVersion "4.3.4")
 BUILD_ARGS=(-k)
--- a/shell.nix
+++ b/shell.nix
@@ -55,7 +55,7 @@ let
   clang-tools = llvm.clang-tools.override { inherit enableLibcxx; };
   cmakeHashes = {
     "3.12.4" = "sha256-UlVYS/0EPrcXViz/iULUcvHA5GecSUHYS6raqbKOMZQ=";
-    "4.1.1" = "sha256-sp9vGXM6oiS3djUHoQikJ+1Ixojh+vIrKcROHDBUkoI=";
+    "4.3.4" = "sha256-/e/4l7nrSddkU58rHtxut+FEDfMlZ4qXwZeEmekxrdo=";
   };
   gcc = if gccVersion == null then null else builtins.getAttr ("gcc" + gccVersion) pkgs;
   cmakeBuild = if cmakeVersion == null then pkgs.cmake else (pkgs.cmake.overrideAttrs (old: {

ryanofsky commented at 9:20 PM on August 25, 2026: collaborator

Thanks for the reviews! Am planning to merge as-is since this is reviewed and passes ci, but will make a followup bumping the cmake version and implementing other suggestions