ci: add cmake debug output #352

ryanofsky commented at 8:34 PM on August 20, 2026: collaborator

Show detailed debug information if cmake fails in CI

DrahtBot commented at 8:34 PM on August 20, 2026: 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	hebasto
Stale ACK	ViniciusCestarii, maflcko

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

Conflicts

No conflicts as of last run.

in ci/scripts/ci.sh:33 in 6d719cf7cc outdated

  29 | +cmake_args=("${CMAKE_ARGS[@]+"${CMAKE_ARGS[@]}"}")
  30 | +if ! cmake "$src_dir" "${cmake_args[@]}"; then
  31 | +  # If cmake failed, try it again with debug options.
  32 | +  # Could add --trace / --trace-expand here too but they are very verbose.
  33 | +  cmake_args+=(--debug-find --debug-output --debug-trycompile --log-level=DEBUG)
  34 | +  cmake "$src_dir" "${cmake_args[@]}" || : "cmake exited with $?"

maflcko commented at 6:35 AM on August 21, 2026:

  cmake "$src_dir" "${cmake_args[@]}" || echo "cmake exited with $?"

nit: I know this is duplicate, but this way non-Bash people won't have to confirm this works:

# set -o xtrace
false || : "command exited with $?"
set +o xtrace
+ false
+ : 'command exited with 1'
+ set +o xtrace

hebasto commented at 12:12 PM on August 23, 2026:

The second cmake invocation is reusing CMakeCache.txt, skipping the checks whose results are cached. Is this the desired behaviour?

ryanofsky commented at 2:54 AM on August 26, 2026:

re: #352 (review)

From my perspective, the suggested change makes output more verbose, and I don't know what problem it would be solving. I'd definitely change this if it looked misleading, but it seems pretty obvious this is trying to show an error message. I also don't know why someone who didn't know bash would worry about this. It should take only few seconds to look up how this works if anyone is worried.

ryanofsky commented at 3:02 AM on August 26, 2026:

re: #352 (review)

The second cmake invocation is reusing CMakeCache.txt, skipping the checks whose results are cached. Is this the desired behaviour?

I think there are tradeoffs. I'd presume more likely than not if if results were cached they were probably successful results, so the current change lets the script be faster and simpler and show strictly more information than it did previously. It could be a good idea to delete the cache or build directory though, and I'd happy review if someone wanted to implement this followup.

ryanofsky commented at 2:10 PM on August 26, 2026:

re: #352 (review)

The second cmake invocation is reusing CMakeCache.txt, skipping the checks whose results are cached.

Decided not to implement a change here because erasing CMakeCache.txt could make failures harder to debug locally and would not be respecting the CI_CLEAN option. I think it could be reasonable to erase the cache (or entire build directory) after a failure when CI_CLEAN is true and to set CI_CLEAN in CI jobs, but erasing things by default when these scripts are run locally seems unsafe and inconvenient, and value of extra debug output that would be provided seems low.

maflcko commented at 6:35 AM on August 21, 2026: contributor

lgtm ACK 6d719cf7ccb1cd6d958898cc11db528a334b9d39

in ci/scripts/ci.sh:32 in 6d719cf7cc

  24 | @@ -25,7 +25,16 @@ src_dir=$PWD
  25 |  mkdir -p "$CI_DIR"
  26 |  cd "$CI_DIR"
  27 |  export CMAKE_BUILD_PARALLEL_LEVEL="$(nproc)"
  28 | -cmake "$src_dir" "${CMAKE_ARGS[@]+"${CMAKE_ARGS[@]}"}"
  29 | +cmake_args=("${CMAKE_ARGS[@]+"${CMAKE_ARGS[@]}"}")
  30 | +if ! cmake "$src_dir" "${cmake_args[@]}"; then
  31 | +  # If cmake failed, try it again with debug options.
  32 | +  # Could add --trace / --trace-expand here too but they are very verbose.
  33 | +  cmake_args+=(--debug-find --debug-output --debug-trycompile --log-level=DEBUG)

ViniciusCestarii commented at 1:21 PM on August 21, 2026:

Since this project supports cmake 3.12+ it would be good to guard these flags against a minimum version otherwise the command will just fail for these lower versions.

if ver_ge "$cmake_ver" "3.16"; then cmake_args+=(--log-level=DEBUG); fi if ver_ge "$cmake_ver" "3.17"; then cmake_args+=(--debug-find); fi

ryanofsky commented at 2:58 AM on August 26, 2026:

re: https://github.com/bitcoin-core/libmultiprocess/pull/352/changes#r3830510005

Since this project supports cmake 3.12+ it would be good to guard these flags against a minimum version otherwise the command will just fail for these lower versions.

Agree these would be nice changes and I'd be happy to review a followup adding them.

ryanofsky commented at 2:03 PM on August 26, 2026:

re: #352 (review)

Since this project supports cmake 3.12+ it would be good to guard these flags against a minimum version otherwise the command will just fail for these lower versions.

Thanks! Added this change in latest rebase

ViniciusCestarii commented at 1:26 PM on August 21, 2026: contributor

ACK 6d719cf7ccb1cd6d958898cc11db528a334b9d39

Looks good, just added a comment about cmake support for the flags used.

in ci/scripts/ci.sh:34 in 6d719cf7cc

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

hebasto commented at 12:04 PM on August 23, 2026:

The CMakeFiles/CMakeConfigureLog.yaml is available only in CMake >=3.26.

ryanofsky commented at 2:57 AM on August 26, 2026:

re: #352 (review)

The CMakeFiles/CMakeConfigureLog.yaml is available only in CMake >=3.26.

Thanks, this good to know. The || true should let the script continue if the file isn't created for any other reason as well. But it could be a good idea to add a ver_ge check like the ones ViniciusCestarii suggested above

ryanofsky commented at 2:03 PM on August 26, 2026:

re: #352 (review)

The CMakeFiles/CMakeConfigureLog.yaml is available only in CMake >=3.26.

Thanks! Added guard in latest rebase

ci: add cmake debug output

Co-Authored-By: ViniciusCestarii <124843824+ViniciusCestarii@users.noreply.github.com>
Co-Authored-By: hebasto <32963518+hebasto@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

a7ff9d5da2

ryanofsky commented at 3:42 AM on August 26, 2026: collaborator

Thanks for the reviews! Looks like this conflicted with #212 so I'll make all the suggested changes above in the rebase.

DrahtBot added the label Needs rebase on Aug 26, 2026

ryanofsky force-pushed on Aug 26, 2026

maflcko commented at 2:09 PM on August 26, 2026: contributor

Seeing Claude being added as co-author here in the force-push, makes me a bit flabbergasted, but I guess this is the thing now. lgtm ACK fbd50d2f5dd2ed5311ba457ea9de57322da9b0b2

$ git range-diff upstream/master 6d719cf7ccb1cd6d958898cc11db528a334b9d39 fbd50d2f5dd2ed5311ba457ea9de57322da9b0b2  -U0
1:  6d719cf ! 1:  fbd50d2 ci: add cmake debug output
    @@ Commit message
    +    Co-Authored-By: ViniciusCestarii <124843824+ViniciusCestarii@users.noreply.github.com>
    +    Co-Authored-By: hebasto <32963518+hebasto@users.noreply.github.com>
    +    Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
    +
    @@ ci/scripts/ci.sh
    -@@ ci/scripts/ci.sh: src_dir=$PWD
    - mkdir -p "$CI_DIR"
    +@@ ci/scripts/ci.sh: mkdir -p "$CI_DIR"
    @@ ci/scripts/ci.sh: src_dir=$PWD
    + git --no-pager log -1 || true
    @@ ci/scripts/ci.sh: src_dir=$PWD
    -+  cmake_args+=(--debug-find --debug-output --debug-trycompile --log-level=DEBUG)
    ++  cmake_args+=(--debug-output --debug-trycompile)
    ++  if ver_ge "$cmake_ver" "3.16"; then cmake_args+=(--log-level=DEBUG); fi
    ++  if ver_ge "$cmake_ver" "3.17"; then cmake_args+=(--debug-find); fi
    @@ ci/scripts/ci.sh: src_dir=$PWD
    -+  cat CMakeFiles/CMakeConfigureLog.yaml || true
    ++  if ver_ge "$cmake_ver" "3.26"; then cat CMakeFiles/CMakeConfigureLog.yaml || true; fi

DrahtBot requested review from ViniciusCestarii on Aug 26, 2026

ryanofsky commented at 2:20 PM on August 26, 2026: collaborator

Rebased 6d719cf7ccb1cd6d958898cc11db528a334b9d39 -> fbd50d2f5dd2ed5311ba457ea9de57322da9b0b2 (pr/cmake-debug.1 -> pr/cmake-debug.2, compare) due to conflict with #212, also implementing review suggestions.

re: #352 (comment)

Seeing Claude being added as co-author here in the force-push, makes me a bit flabbergasted, but I guess this is the thing now.

Sorry about that and I sympathize but I instantly muted 35794 as soon as I saw the title because I thought it would be too distracting for me, so I am not caught up right now. I do plan to read the discussion at some point and would like to talk about it in person. Appreciate you reviewing and approving despite this.

DrahtBot removed the label Needs rebase on Aug 26, 2026

in ci/scripts/ci.sh:53 in fbd50d2f5d

  49 | +  # Could add --trace / --trace-expand here too but they are very verbose.
  50 | +  cmake_args+=(--debug-output --debug-trycompile)
  51 | +  if ver_ge "$cmake_ver" "3.16"; then cmake_args+=(--log-level=DEBUG); fi
  52 | +  if ver_ge "$cmake_ver" "3.17"; then cmake_args+=(--debug-find); fi
  53 | +  cmake "$src_dir" "${cmake_args[@]}" || : "cmake exited with $?"
  54 | +  if ver_ge "$cmake_ver" "3.26"; then cat CMakeFiles/CMakeConfigureLog.yaml || true; fi

hebasto commented at 2:02 PM on August 27, 2026:

  if ver_ge "$cmake_ver" "3.26"; then
    cat CMakeFiles/CMakeConfigureLog.yaml || true
  else
    cat CMakeFiles/CMakeError.log CMakeFiles/CMakeOutput.log || true
  fi

hebasto approved

hebasto commented at 2:02 PM on August 27, 2026: member

ACK fbd50d2f5dd2ed5311ba457ea9de57322da9b0b2.

ryanofsky force-pushed on Sep 8, 2026

ryanofsky commented at 7:22 PM on September 8, 2026: collaborator

Updated fbd50d2f5dd2ed5311ba457ea9de57322da9b0b2 -> a7ff9d5da2290c1edc08460262c909f31cff938a (pr/cmake-debug.2 -> pr/cmake-debug.3, compare) applying CMakeError.log / CMakeOutput.log suggestion

hebasto approved

hebasto commented at 8:16 AM on September 9, 2026: member

re-ACK a7ff9d5da2290c1edc08460262c909f31cff938a.

DrahtBot requested review from maflcko on Sep 9, 2026

ryanofsky merged this on Sep 10, 2026

ryanofsky closed this on Sep 10, 2026