ci: Enable more shellcheck #32970

maflcko commented at 12:44 PM on July 14, 2025: member

shellcheck is often the main "reviewer" of CI code written in Bash, so it seems odd to disable it by putting commands into bash -c "cmd...".

Fix that by removing bash -c, where it isn't intended and where the removal is easily possible.

DrahtBot added the label Tests on Jul 14, 2025

DrahtBot commented at 12:44 PM on July 14, 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/32970.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

maflcko force-pushed on Jul 14, 2025

DrahtBot added the label CI failed on Jul 14, 2025

DrahtBot commented at 2:25 PM on July 14, 2025: contributor

🚧 At least one of the CI tasks failed. <sub>Task lint: https://github.com/bitcoin/bitcoin/runs/45926063054</sub> <sub>LLM reason (✨ experimental): The CI failure is caused by errors in shell script linting, specifically from shellcheck warnings that caused the lint check to fail.</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

ci: Enable more shellcheck fa1fd07468

maflcko force-pushed on Jul 14, 2025

DrahtBot removed the label CI failed on Jul 14, 2025

in ci/test/03_test_script.sh:125 in fa1fd07468

 121 | @@ -122,7 +122,11 @@ if [[ "${RUN_TIDY}" == "true" ]]; then
 122 |    BITCOIN_CONFIG_ALL="$BITCOIN_CONFIG_ALL -DCMAKE_EXPORT_COMPILE_COMMANDS=ON"
 123 |  fi
 124 |  
 125 | -bash -c "cmake -S $BASE_ROOT_DIR -B ${BASE_BUILD_DIR} $BITCOIN_CONFIG_ALL $BITCOIN_CONFIG || ( (cat $(cmake -P "${BASE_ROOT_DIR}/ci/test/GetCMakeLogFiles.cmake")) && false)"
 126 | +bash -c "cmake -S $BASE_ROOT_DIR -B ${BASE_BUILD_DIR} $BITCOIN_CONFIG_ALL $BITCOIN_CONFIG" || (

hebasto commented at 3:55 PM on July 17, 2025:

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

maflcko commented at 4:01 PM on July 17, 2025:

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.

hebasto commented at 4:08 PM on July 17, 2025:

Maybe

# shellcheck disable=SC2086
cmake -S "$BASE_ROOT_DIR" -B "$BASE_BUILD_DIR" $BITCOIN_CONFIG_ALL $BITCOIN_CONFIG || (

?

maflcko commented at 4:18 PM on July 17, 2025:

No this won't work, because some elements contain spaces. SC2086 is about this, to warn about word splitting.

For example. DCMAKE_CXX_FLAGS will be split:

[13:10:44.210] + cmake -S /ci_container_base -B /ci_container_base/ci/scratch/build-arm-linux-gnueabihf -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DCMAKE_TOOLCHAIN_FILE=/ci_container_base/depends/arm-linux-gnueabihf/toolchain.cmake -DWERROR=ON -DCMAKE_INSTALL_PREFIX=/ci_container_base/ci/scratch/out -Werror=dev -DREDUCE_EXPORTS=ON '-DCMAKE_CXX_FLAGS='\''-Wno-psabi' '-Wno-error=maybe-uninitialized'\'''
[13:10:44.374] -- The CXX compiler identification is GNU 13.3.0
[13:10:44.411] -- Detecting CXX compiler ABI info
[13:10:44.484] -- Detecting CXX compiler ABI info - failed
[13:10:44.484] -- Check for working CXX compiler: /bin/arm-linux-gnueabihf-g++
[13:10:44.560] -- Check for working CXX compiler: /bin/arm-linux-gnueabihf-g++ - broken
[13:10:44.560] CMake Error at /usr/share/cmake-3.28/Modules/CMakeTestCXXCompiler.cmake:60 (message):
[13:10:44.560]   The C++ compiler
[13:10:44.560] 
[13:10:44.560]     "/bin/arm-linux-gnueabihf-g++"
[13:10:44.560] 
[13:10:44.560]   is not able to compile a simple test program.
[13:10:44.560] 
[13:10:44.560]   It fails with the following output:
[13:10:44.560] 
[13:10:44.560]     Change Dir: '/ci_container_base/ci/scratch/build-arm-linux-gnueabihf/CMakeFiles/CMakeScratch/TryCompile-Vg0n9k'
[13:10:44.560]     
[13:10:44.560]     Run Build Command(s): /usr/bin/cmake -E env VERBOSE=1 /bin/gmake -f Makefile cmTC_7493d/fast
[13:10:44.560]     /bin/gmake  -f CMakeFiles/cmTC_7493d.dir/build.make CMakeFiles/cmTC_7493d.dir/build
[13:10:44.560]     gmake[1]: Entering directory '/ci_container_base/ci/scratch/build-arm-linux-gnueabihf/CMakeFiles/CMakeScratch/TryCompile-Vg0n9k'
[13:10:44.560]     Building CXX object CMakeFiles/cmTC_7493d.dir/testCXXCompiler.cxx.o
[13:10:44.560]     /bin/arm-linux-gnueabihf-g++   '-Wno-psabi  -o CMakeFiles/cmTC_7493d.dir/testCXXCompiler.cxx.o -c /ci_container_base/ci/scratch/build-arm-linux-gnueabihf/CMakeFiles/CMakeScratch/TryCompile-Vg0n9k/testCXXCompiler.cxx
[13:10:44.560]     /bin/sh: 1: Syntax error: Unterminated quoted string
[13:10:44.560]     gmake[1]: *** [CMakeFiles/cmTC_7493d.dir/build.make:78: CMakeFiles/cmTC_7493d.dir/testCXXCompiler.cxx.o] Error 2
[13:10:44.560]     gmake[1]: Leaving directory '/ci_container_base/ci/scratch/build-arm-linux-gnueabihf/CMakeFiles/CMakeScratch/TryCompile-Vg0n9k'
[13:10:44.560]     gmake: *** [Makefile:127: cmTC_7493d/fast] Error 2
[13:10:44.560]     
[13:10:44.560]     
[13:10:44.560] 
[13:10:44.560]   
[13:10:44.560] 
[13:10:44.560]   CMake will not be able to correctly generate this project.
[13:10:44.560] Call Stack (most recent call first):
[13:10:44.560]   CMakeLists.txt:77 (enable_language)
[13:10:44.560] 
[13:10:44.560] 
[13:10:44.561] -- Configuring incomplete, errors occurred!
[13:10:44.568] ++ cmake -P /ci_container_base/ci/test/GetCMakeLogFiles.cmake
[13:10:44.599] + cat CMakeFiles/CMakeConfigureLog.yaml
[13:10:44.602] cat: can't open 'CMakeFiles/CMakeConfigureLog.yaml': No such file or directory

maflcko commented at 4:29 PM on July 17, 2025:

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)

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

(can be done in a follow-up)

See: #33401.

hebasto approved

hebasto commented at 4:20 PM on July 17, 2025: member

ACK fa1fd074685ca96b9bd3855e9e6fe730a4f6462c.

fanquake merged this on Jul 25, 2025

fanquake closed this on Jul 25, 2025

maflcko deleted the branch on Jul 25, 2025

TheCharlatan referenced this in commit 02ded863ba on Jul 28, 2025

TheCharlatan referenced this in commit 52d7f32bd6 on Jul 28, 2025

alexanderwiederin referenced this in commit 28fe919bf7 on Aug 6, 2025

TheCharlatan referenced this in commit b98d982d79 on Aug 7, 2025

alexanderwiederin referenced this in commit 4152176d02 on Aug 8, 2025

alexanderwiederin referenced this in commit 9ef94c31db on Aug 8, 2025

stringintech referenced this in commit 71275a1b5e on Aug 17, 2025

yuvicc referenced this in commit 22f55cf11d on Aug 26, 2025