ci: Remove bash -c from cmake invocation using eval #33401

pull BrandonOdiwuor wants to merge 1 commits into bitcoin:master from BrandonOdiwuor:ci-cmake-eval changing 1 files +2 −1

BrandonOdiwuor commented at 10:09 AM on September 16, 2025: contributor
Follow up to #32970

#32970#r2213730157

Does cmake -S ... still need to be wrapped in bash -c "..."?

#32970#r2213741192
It is not trivial to replace. Maybe the eval hack from below can be used:

# parses TEST_RUNNER_EXTRA as an array which allows for multiple arguments such as TEST_RUNNER_EXTRA='--exclude "rpc_bind.py --ipv6"' eval "TEST_RUNNER_EXTRA=($TEST_RUNNER_EXTRA)"

however, I haven't tried this yet.
#32970#r2213801696
Yeah, the eval hack should work:

$ export T="-DREDUCE_EXPORTS=ON -DCMAKE_CXX_FLAGS='-Wno-psabi -Wno-error=maybe-uninitialized'"; eval "T=($T)"; for i in "${T[@]}"; do echo "_${i}_" ; done _-DREDUCE_EXPORTS=ON_ _-DCMAKE_CXX_FLAGS=-Wno-psabi -Wno-error=maybe-uninitialized_

(can be done in a follow-up)
This replaces the bash -c wrapper with an eval-based array parsing to preserve spaces in flag values (e.g., in CMAKE_CXX_FLAGS), allowing ShellCheck to lint the cmake command
DrahtBot added the label Tests on Sep 16, 2025
DrahtBot commented at 10:09 AM on September 16, 2025: contributor



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



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33401.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK maflcko, hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
hebasto commented at 10:19 AM on September 16, 2025: member

Concept ACK on expanding ShellCheck lint coverage.
maflcko commented at 6:10 PM on September 25, 2025: member

nice, lgtm ACK 8406c1e6321fbc3ce739aa7ccb18c67fb706a4b9
DrahtBot requested review from hebasto on Sep 25, 2025

Type	Reviewers
ACK	maflcko, hebasto

in ci/test/03_test_script.sh:134 in 8406c1e632

 129 | @@ -130,7 +130,9 @@ if [[ "${RUN_TIDY}" == "true" ]]; then
 130 |    BITCOIN_CONFIG_ALL="$BITCOIN_CONFIG_ALL -DCMAKE_EXPORT_COMPILE_COMMANDS=ON"
 131 |  fi
 132 |  
 133 | -bash -c "cmake -S $BASE_ROOT_DIR -B ${BASE_BUILD_DIR} $BITCOIN_CONFIG_ALL $BITCOIN_CONFIG" || (
 134 | +CMAKE_ARGS="$BITCOIN_CONFIG_ALL $BITCOIN_CONFIG"
 135 | +eval "CMAKE_ARGS=($CMAKE_ARGS)"

hebasto commented at 9:55 AM on September 26, 2025:

eval "CMAKE_ARGS=($BITCOIN_CONFIG_ALL $BITCOIN_CONFIG)"

BrandonOdiwuor commented at 6:31 AM on October 1, 2025:

fixed

hebasto approved
hebasto commented at 9:56 AM on September 26, 2025: member

ACK 8406c1e6321fbc3ce739aa7ccb18c67fb706a4b9.
ci: Remove bash -c from cmake invocation using eval 50194029e7
BrandonOdiwuor force-pushed on Oct 1, 2025
maflcko commented at 6:02 AM on October 1, 2025: member

lgtm ACK 50194029e7c2581b751931080f5999785a39929f
DrahtBot requested review from hebasto on Oct 1, 2025
hebasto approved
hebasto commented at 10:35 AM on October 1, 2025: member

ACK 50194029e7c2581b751931080f5999785a39929f.
hebasto merged this on Oct 1, 2025
hebasto closed this on Oct 1, 2025

Contributors

Labels

Linked (view graph)

#32970 ci: Enable more shellcheck

ci: Remove bash -c from cmake invocation using eval #33401

Code Coverage & Benchmarks

Reviews