ci: Enable more shellcheck #32970

pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2507-ci-shellcheck changing 1 files +5 −1
  1. 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.

  2. DrahtBot added the label Tests on Jul 14, 2025
  3. 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.

  4. maflcko force-pushed on Jul 14, 2025
  5. DrahtBot added the label CI failed on Jul 14, 2025
  6. DrahtBot commented at 2:25 pm on July 14, 2025: contributor

    🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/45926063054 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.

    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.

  7. ci: Enable more shellcheck fa1fd07468
  8. maflcko force-pushed on Jul 14, 2025
  9. DrahtBot removed the label CI failed on Jul 14, 2025
  10. 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:

    0  # parses TEST_RUNNER_EXTRA as an array which allows for multiple arguments such as TEST_RUNNER_EXTRA='--exclude "rpc_bind.py --ipv6"'
1
2  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

    0# shellcheck disable=SC2086
1cmake -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:

     0[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'\'''
 1[13:10:44.374] -- The CXX compiler identification is GNU 13.3.0
 2[13:10:44.411] -- Detecting CXX compiler ABI info
 3[13:10:44.484] -- Detecting CXX compiler ABI info - failed
 4[13:10:44.484] -- Check for working CXX compiler: /bin/arm-linux-gnueabihf-g++
 5[13:10:44.560] -- Check for working CXX compiler: /bin/arm-linux-gnueabihf-g++ - broken
 6[13:10:44.560] CMake Error at /usr/share/cmake-3.28/Modules/CMakeTestCXXCompiler.cmake:60 (message):
 7[13:10:44.560]   The C++ compiler
 8[13:10:44.560] 
 9[13:10:44.560]     "/bin/arm-linux-gnueabihf-g++"
10[13:10:44.560] 
11[13:10:44.560]   is not able to compile a simple test program.
12[13:10:44.560] 
13[13:10:44.560]   It fails with the following output:
14[13:10:44.560] 
15[13:10:44.560]     Change Dir: '/ci_container_base/ci/scratch/build-arm-linux-gnueabihf/CMakeFiles/CMakeScratch/TryCompile-Vg0n9k'
16[13:10:44.560]     
17[13:10:44.560]     Run Build Command(s): /usr/bin/cmake -E env VERBOSE=1 /bin/gmake -f Makefile cmTC_7493d/fast
18[13:10:44.560]     /bin/gmake  -f CMakeFiles/cmTC_7493d.dir/build.make CMakeFiles/cmTC_7493d.dir/build
19[13:10:44.560]     gmake[1]: Entering directory '/ci_container_base/ci/scratch/build-arm-linux-gnueabihf/CMakeFiles/CMakeScratch/TryCompile-Vg0n9k'
20[13:10:44.560]     Building CXX object CMakeFiles/cmTC_7493d.dir/testCXXCompiler.cxx.o
21[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
22[13:10:44.560]     /bin/sh: 1: Syntax error: Unterminated quoted string
23[13:10:44.560]     gmake[1]: *** [CMakeFiles/cmTC_7493d.dir/build.make:78: CMakeFiles/cmTC_7493d.dir/testCXXCompiler.cxx.o] Error 2
24[13:10:44.560]     gmake[1]: Leaving directory '/ci_container_base/ci/scratch/build-arm-linux-gnueabihf/CMakeFiles/CMakeScratch/TryCompile-Vg0n9k'
25[13:10:44.560]     gmake: *** [Makefile:127: cmTC_7493d/fast] Error 2
26[13:10:44.560]     
27[13:10:44.560]     
28[13:10:44.560] 
29[13:10:44.560]   
30[13:10:44.560] 
31[13:10:44.560]   CMake will not be able to correctly generate this project.
32[13:10:44.560] Call Stack (most recent call first):
33[13:10:44.560]   CMakeLists.txt:77 (enable_language)
34[13:10:44.560] 
35[13:10:44.560] 
36[13:10:44.561] -- Configuring incomplete, errors occurred!
37[13:10:44.568] ++ cmake -P /ci_container_base/ci/test/GetCMakeLogFiles.cmake
38[13:10:44.599] + cat CMakeFiles/CMakeConfigureLog.yaml
39[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:

    0$ 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 
1_-DREDUCE_EXPORTS=ON_
2_-DCMAKE_CXX_FLAGS=-Wno-psabi     -Wno-error=maybe-uninitialized_

    (can be done in a follow-up)

  11. hebasto approved
  12. hebasto commented at 4:20 pm on July 17, 2025: member
    ACK fa1fd074685ca96b9bd3855e9e6fe730a4f6462c.


maflcko DrahtBot hebasto

Labels
Tests

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-07-23 00:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me