Merge bitcoin/bitcoin#27573: ci: Remove CI_EXEC bloat in test/06_script_b.sh

fa1dbd04ca ci: Remove CI_EXEC bloat in test/06_script_b.sh (MarcoFalke) fae8de926a ci: Move CI container kill out of 06_script_b.sh (MarcoFalke) fa7d75540e ci: Pass full env to CI pod to avoid missing a var (MarcoFalke) Pull request description: `CI_EXEC` has many issues: * It is roughly equivalent to `bash -c "$*"`, meaning that the full command will be treated as a single string, ignoring tokens. * It must be put in front of (almost) every command, making it easy to forget, hard to debug the resulting failure, and the code verbose. Fix all issues in one script by removing it. ACKs for top commit: fanquake: ACK fa1dbd04ca - this conflicts with #27125, but that is going to be rebased soon, and this could be merged in the interim. cc TheCharlatan TheCharlatan: ACK fa1dbd04ca Tree-SHA512: e5ab5503a05a787f2bc6ca25e71ad3dc166aade57e25d9677e72b1ca4e5fb6045c058dfd55f47ac93f710538e62d57c12cd7eb9d1260c6f55f3c8091908dc70d
2024-11-20 02:25:40 +01:00 · 2023-05-10 11:40:03 +01:00 · 2023-05-10 11:40:03 +01:00 · 883766fa45
commit 883766fa45
parent 3777c75d14 fa1dbd04ca
4 changed files with 78 additions and 73 deletions
--- a/ci/README.md
+++ b/ci/README.md
@ -14,10 +14,10 @@ testing compared to other parts of the codebase. If you want to keep the work tr
 system in a virtual machine with a Linux operating system of your choice.

 To allow for a wide range of tested environments, but also ensure reproducibility to some extent, the test stage
-requires `docker` to be installed. To install all requirements on Ubuntu, run
+requires `bash`, `docker`, and `python3` to be installed. To install all requirements on Ubuntu, run

 ```
-sudo apt install docker.io bash
+sudo apt install bash docker.io python3
 ```

 To run the default test stage,
--- a/ci/test/04_install.sh
+++ b/ci/test/04_install.sh
@ -18,7 +18,6 @@ export ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order=
 export LSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/lsan"
 export TSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/tsan:halt_on_error=1:log_path=${BASE_SCRATCH_DIR}/sanitizer-output/tsan"
 export UBSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1"
-env | grep -E '^(BITCOIN_CONFIG|BASE_|QEMU_|CCACHE_|LC_ALL|BOOST_TEST_RANDOM|DEBIAN_FRONTEND|CONFIG_SHELL|(ASAN|LSAN|TSAN|UBSAN)_OPTIONS|PREVIOUS_RELEASES_DIR)' | tee /tmp/env
 if [[ $BITCOIN_CONFIG = *--with-sanitizers=*address* ]]; then # If ran with (ASan + LSan), Docker needs access to ptrace (https://github.com/google/sanitizers/issues/764)
  CI_CONTAINER_CAP="--cap-add SYS_PTRACE"
 fi
@ -27,6 +26,9 @@ export P_CI_DIR="$PWD"
 export BINS_SCRATCH_DIR="${BASE_SCRATCH_DIR}/bins/"

 if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then
+  # Export all env vars to avoid missing some.
+  # Though, exclude those with newlines to avoid parsing problems.
+  python3 -c 'import os; [print(f"{key}={value}") for key, value in os.environ.items() if "\n" not in value]' | tee /tmp/env
  echo "Creating $CI_IMAGE_NAME_TAG container to run in"
  DOCKER_BUILDKIT=1 ${CI_RETRY_EXE} docker build \
      --file "${BASE_ROOT_DIR}/ci/test_imagefile" \
--- a/ci/test/06_script_b.sh
+++ b/ci/test/06_script_b.sh
@ -6,101 +6,98 @@

 export LC_ALL=C.UTF-8

+set -ex
+
 if [[ $HOST = *-mingw32 ]]; then
  # Generate all binaries, so that they can be wrapped
-  CI_EXEC make "$MAKEJOBS" -C src/secp256k1 VERBOSE=1
-  CI_EXEC make "$MAKEJOBS" -C src minisketch/test.exe VERBOSE=1
-  CI_EXEC "${BASE_ROOT_DIR}/ci/test/wrap-wine.sh"
+  make "$MAKEJOBS" -C src/secp256k1 VERBOSE=1
+  make "$MAKEJOBS" -C src minisketch/test.exe VERBOSE=1
+  "${BASE_ROOT_DIR}/ci/test/wrap-wine.sh"
 fi

 if [ -n "$QEMU_USER_CMD" ]; then
  # Generate all binaries, so that they can be wrapped
-  CI_EXEC make "$MAKEJOBS" -C src/secp256k1 VERBOSE=1
-  CI_EXEC make "$MAKEJOBS" -C src minisketch/test VERBOSE=1
-  CI_EXEC "${BASE_ROOT_DIR}/ci/test/wrap-qemu.sh"
+  make "$MAKEJOBS" -C src/secp256k1 VERBOSE=1
+  make "$MAKEJOBS" -C src minisketch/test VERBOSE=1
+  "${BASE_ROOT_DIR}/ci/test/wrap-qemu.sh"
 fi

 if [ -n "$USE_VALGRIND" ]; then
-  CI_EXEC "${BASE_ROOT_DIR}/ci/test/wrap-valgrind.sh"
+  "${BASE_ROOT_DIR}/ci/test/wrap-valgrind.sh"
 fi

 if [ "$RUN_UNIT_TESTS" = "true" ]; then
-  CI_EXEC "${TEST_RUNNER_ENV}" DIR_UNIT_TEST_DATA="${DIR_UNIT_TEST_DATA}" LD_LIBRARY_PATH="${DEPENDS_DIR}/${HOST}/lib" make "$MAKEJOBS" check VERBOSE=1
+  bash -c "${TEST_RUNNER_ENV} DIR_UNIT_TEST_DATA=${DIR_UNIT_TEST_DATA} LD_LIBRARY_PATH=${DEPENDS_DIR}/${HOST}/lib make $MAKEJOBS check VERBOSE=1"
 fi

 if [ "$RUN_UNIT_TESTS_SEQUENTIAL" = "true" ]; then
-  CI_EXEC "${TEST_RUNNER_ENV}" DIR_UNIT_TEST_DATA="${DIR_UNIT_TEST_DATA}" LD_LIBRARY_PATH="${DEPENDS_DIR}/${HOST}/lib" "${BASE_OUTDIR}/bin/test_bitcoin" --catch_system_errors=no -l test_suite
+  bash -c "${TEST_RUNNER_ENV} DIR_UNIT_TEST_DATA=${DIR_UNIT_TEST_DATA} LD_LIBRARY_PATH=${DEPENDS_DIR}/${HOST}/lib ${BASE_OUTDIR}/bin/test_bitcoin --catch_system_errors=no -l test_suite"
 fi

 if [ "$RUN_FUNCTIONAL_TESTS" = "true" ]; then
-  CI_EXEC LD_LIBRARY_PATH="${DEPENDS_DIR}/${HOST}/lib" "${TEST_RUNNER_ENV}" test/functional/test_runner.py --ci "$MAKEJOBS" --tmpdirprefix "${BASE_SCRATCH_DIR}/test_runner/" --ansi --combinedlogslen=99999999 --timeout-factor="${TEST_RUNNER_TIMEOUT_FACTOR}" "${TEST_RUNNER_EXTRA}" --quiet --failfast
+  bash -c "LD_LIBRARY_PATH=${DEPENDS_DIR}/${HOST}/lib ${TEST_RUNNER_ENV} test/functional/test_runner.py --ci $MAKEJOBS --tmpdirprefix ${BASE_SCRATCH_DIR}/test_runner/ --ansi --combinedlogslen=99999999 --timeout-factor=${TEST_RUNNER_TIMEOUT_FACTOR} ${TEST_RUNNER_EXTRA} --quiet --failfast"
 fi

 if [ "${RUN_TIDY}" = "true" ]; then
  set -eo pipefail
-  export P_CI_DIR="${BASE_BUILD_DIR}/bitcoin-$HOST/src/"
-  ( CI_EXEC run-clang-tidy-16 -quiet "${MAKEJOBS}" ) | grep -C5 "error"
-  export P_CI_DIR="${BASE_BUILD_DIR}/bitcoin-$HOST/"
-  CI_EXEC "python3 ${DIR_IWYU}/include-what-you-use/iwyu_tool.py"\
-          " src/common/args.cpp"\
-          " src/common/config.cpp"\
-          " src/common/init.cpp"\
-          " src/common/url.cpp"\
-          " src/compat"\
-          " src/dbwrapper.cpp"\
-          " src/init"\
-          " src/kernel"\
-          " src/node/chainstate.cpp"\
-          " src/node/chainstatemanager_args.cpp"\
-          " src/node/mempool_args.cpp"\
-          " src/node/minisketchwrapper.cpp"\
-          " src/node/utxo_snapshot.cpp"\
-          " src/node/validation_cache_args.cpp"\
-          " src/policy/feerate.cpp"\
-          " src/policy/packages.cpp"\
-          " src/policy/settings.cpp"\
-          " src/primitives/transaction.cpp"\
-          " src/random.cpp"\
-          " src/rpc/fees.cpp"\
-          " src/rpc/signmessage.cpp"\
-          " src/test/fuzz/string.cpp"\
-          " src/test/fuzz/txorphan.cpp"\
-          " src/test/fuzz/util/"\
-          " src/test/util/coins.cpp"\
-          " src/uint256.cpp"\
-          " src/util/bip32.cpp"\
-          " src/util/bytevectorhash.cpp"\
-          " src/util/check.cpp"\
-          " src/util/error.cpp"\
-          " src/util/exception.cpp"\
-          " src/util/getuniquepath.cpp"\
-          " src/util/hasher.cpp"\
-          " src/util/message.cpp"\
-          " src/util/moneystr.cpp"\
-          " src/util/serfloat.cpp"\
-          " src/util/spanparsing.cpp"\
-          " src/util/strencodings.cpp"\
-          " src/util/string.cpp"\
-          " src/util/syserror.cpp"\
-          " src/util/threadinterrupt.cpp"\
-          " src/zmq"\
-          " -p . ${MAKEJOBS}"\
-          " -- -Xiwyu --cxx17ns -Xiwyu --mapping_file=${BASE_BUILD_DIR}/bitcoin-$HOST/contrib/devtools/iwyu/bitcoin.core.imp"\
-          " |& tee /tmp/iwyu_ci.out"
-  export P_CI_DIR="${BASE_ROOT_DIR}/src"
-  CI_EXEC "python3 ${DIR_IWYU}/include-what-you-use/fix_includes.py --nosafe_headers < /tmp/iwyu_ci.out"
-  CI_EXEC "git --no-pager diff"
+  cd "${BASE_BUILD_DIR}/bitcoin-$HOST/src/"
+  ( run-clang-tidy-16 -quiet "${MAKEJOBS}" ) | grep -C5 "error"
+  cd "${BASE_BUILD_DIR}/bitcoin-$HOST/"
+  python3 "${DIR_IWYU}/include-what-you-use/iwyu_tool.py" \
+           src/common/args.cpp \
+           src/common/config.cpp \
+           src/common/init.cpp \
+           src/common/url.cpp \
+           src/compat \
+           src/dbwrapper.cpp \
+           src/init \
+           src/kernel \
+           src/node/chainstate.cpp \
+           src/node/chainstatemanager_args.cpp \
+           src/node/mempool_args.cpp \
+           src/node/minisketchwrapper.cpp \
+           src/node/utxo_snapshot.cpp \
+           src/node/validation_cache_args.cpp \
+           src/policy/feerate.cpp \
+           src/policy/packages.cpp \
+           src/policy/settings.cpp \
+           src/primitives/transaction.cpp \
+           src/random.cpp \
+           src/rpc/fees.cpp \
+           src/rpc/signmessage.cpp \
+           src/test/fuzz/string.cpp \
+           src/test/fuzz/txorphan.cpp \
+           src/test/fuzz/util \
+           src/test/util/coins.cpp \
+           src/uint256.cpp \
+           src/util/bip32.cpp \
+           src/util/bytevectorhash.cpp \
+           src/util/check.cpp \
+           src/util/error.cpp \
+           src/util/exception.cpp \
+           src/util/getuniquepath.cpp \
+           src/util/hasher.cpp \
+           src/util/message.cpp \
+           src/util/moneystr.cpp \
+           src/util/serfloat.cpp \
+           src/util/spanparsing.cpp \
+           src/util/strencodings.cpp \
+           src/util/string.cpp \
+           src/util/syserror.cpp \
+           src/util/threadinterrupt.cpp \
+           src/zmq \
+           -p . "${MAKEJOBS}" \
+           -- -Xiwyu --cxx17ns -Xiwyu --mapping_file="${BASE_BUILD_DIR}/bitcoin-$HOST/contrib/devtools/iwyu/bitcoin.core.imp" \
+           2>&1 | tee /tmp/iwyu_ci.out
+  cd "${BASE_ROOT_DIR}/src"
+  python3 "${DIR_IWYU}/include-what-you-use/fix_includes.py" --nosafe_headers < /tmp/iwyu_ci.out
+  git --no-pager diff
 fi

 if [ "$RUN_SECURITY_TESTS" = "true" ]; then
-  CI_EXEC make test-security-check
+  make test-security-check
 fi

 if [ "$RUN_FUZZ_TESTS" = "true" ]; then
-  CI_EXEC LD_LIBRARY_PATH="${DEPENDS_DIR}/${HOST}/lib" test/fuzz/test_runner.py "${FUZZ_TESTS_CONFIG}" "$MAKEJOBS" -l DEBUG "${DIR_FUZZ_IN}"
-fi
-
-if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then
-  echo "Stop and remove CI container by ID"
-  docker container kill "${CI_CONTAINER_ID}"
+  bash -c "LD_LIBRARY_PATH=${DEPENDS_DIR}/${HOST}/lib test/fuzz/test_runner.py ${FUZZ_TESTS_CONFIG} $MAKEJOBS -l DEBUG ${DIR_FUZZ_IN}"
 fi
--- a/ci/test_run_all.sh
+++ b/ci/test_run_all.sh
@ -10,4 +10,10 @@ set -o errexit; source ./ci/test/00_setup_env.sh
 set -o errexit; source ./ci/test/04_install.sh
 set -o errexit; source ./ci/test/05_before_script.sh
 set -o errexit; source ./ci/test/06_script_a.sh
-set -o errexit; source ./ci/test/06_script_b.sh
+set -o errexit
+CI_EXEC "${BASE_ROOT_DIR}/ci/test/06_script_b.sh"
+
+if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then
+  echo "Stop and remove CI container by ID"
+  docker container kill "${CI_CONTAINER_ID}"
+fi