From 666675e95fe823b7809f64508aea5b57b1867c19 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 29 Oct 2025 11:46:09 +0100 Subject: [PATCH 1/6] ci: Move folder creation and docker kill to Python script The container_id is already known in the Python script, as well as the folders to create, so just do it there. --- ci/test/02_run_container.py | 13 ++++++++++++- ci/test/02_run_container.sh | 10 ---------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/ci/test/02_run_container.py b/ci/test/02_run_container.py index 4d0bed2ad41..79d9581be6f 100755 --- a/ci/test/02_run_container.py +++ b/ci/test/02_run_container.py @@ -48,7 +48,15 @@ def main(): file.write(f"{k}={v}\n") run(["cat", env_file]) - if not os.getenv("DANGER_RUN_CI_ON_HOST"): + if os.getenv("DANGER_RUN_CI_ON_HOST"): + print("Running on host system without docker wrapper") + print("Create missing folders") + for create_dir in [ + os.environ["CCACHE_DIR"], + os.environ["PREVIOUS_RELEASES_DIR"], + ]: + Path(create_dir).mkdir(parents=True, exist_ok=True) + else: CI_IMAGE_LABEL = "bitcoin-ci-test" # Use buildx unconditionally @@ -155,6 +163,9 @@ def main(): os.environ["IN_GETOPT_BIN"] = f"{prefix}/bin/getopt" run(["./ci/test/02_run_container.sh"]) # run the remainder + if not os.getenv("DANGER_RUN_CI_ON_HOST"): + print("Stop and remove CI container by ID") + run(["docker", "container", "kill", container_id]) if __name__ == "__main__": diff --git a/ci/test/02_run_container.sh b/ci/test/02_run_container.sh index 41128b5c023..f462db25f9c 100755 --- a/ci/test/02_run_container.sh +++ b/ci/test/02_run_container.sh @@ -10,11 +10,6 @@ set -o errexit -o pipefail -o xtrace if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then export CI_EXEC_CMD_PREFIX="docker exec ${CI_CONTAINER_ID}" -else - echo "Running on host system without docker wrapper" - echo "Create missing folders" - mkdir -p "${CCACHE_DIR}" - mkdir -p "${PREVIOUS_RELEASES_DIR}" fi CI_EXEC () { @@ -26,8 +21,3 @@ export -f CI_EXEC CI_EXEC rsync --recursive --perms --stats --human-readable "${BASE_READ_ONLY_DIR}/" "${BASE_ROOT_DIR}" || echo "Nothing to copy from ${BASE_READ_ONLY_DIR}/" CI_EXEC "${BASE_ROOT_DIR}/ci/test/01_base_install.sh" CI_EXEC "${BASE_ROOT_DIR}/ci/test/03_test_script.sh" - -if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then - echo "Stop and remove CI container by ID" - docker container kill "${CI_CONTAINER_ID}" -fi From fa37559ac5b7bf83eefa30e7770ccae9fd19556b Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 29 Oct 2025 15:24:13 +0100 Subject: [PATCH 2/6] ci: Document the retry script in PATH The `retry` script is required for CI_RETRY_EXE and there are two ways to put it into PATH: * When running in a container engine, by copying it into /usr/bin * When running without a container engine, by prepending its location to PATH --- ci/test/02_run_container.py | 3 +++ ci/test/02_run_container.sh | 2 +- ci/test_imagefile | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ci/test/02_run_container.py b/ci/test/02_run_container.py index 79d9581be6f..0865350d092 100755 --- a/ci/test/02_run_container.py +++ b/ci/test/02_run_container.py @@ -56,6 +56,9 @@ def main(): os.environ["PREVIOUS_RELEASES_DIR"], ]: Path(create_dir).mkdir(parents=True, exist_ok=True) + + # Modify PATH to prepend the retry script, needed for CI_RETRY_EXE + os.environ["PATH"] = f"{os.environ['BASE_ROOT_DIR']}/ci/retry:{os.environ['PATH']}" else: CI_IMAGE_LABEL = "bitcoin-ci-test" diff --git a/ci/test/02_run_container.sh b/ci/test/02_run_container.sh index f462db25f9c..77c70f82bfa 100755 --- a/ci/test/02_run_container.sh +++ b/ci/test/02_run_container.sh @@ -13,7 +13,7 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then fi CI_EXEC () { - $CI_EXEC_CMD_PREFIX bash -c "export PATH=\"/path_with space:${BASE_ROOT_DIR}/ci/retry:\$PATH\" && cd \"${BASE_ROOT_DIR}\" && $*" + $CI_EXEC_CMD_PREFIX bash -c "export PATH=\"/path_with space:\$PATH\" && cd \"${BASE_ROOT_DIR}\" && $*" } export -f CI_EXEC diff --git a/ci/test_imagefile b/ci/test_imagefile index a0e1714ed5f..5c34d8efae0 100644 --- a/ci/test_imagefile +++ b/ci/test_imagefile @@ -14,6 +14,7 @@ ENV FILE_ENV=${FILE_ENV} ARG BASE_ROOT_DIR ENV BASE_ROOT_DIR=${BASE_ROOT_DIR} +# Make retry available in PATH, needed for CI_RETRY_EXE COPY ./ci/retry/retry /usr/bin/retry COPY ./ci/test/00_setup_env.sh ./${FILE_ENV} ./ci/test/01_base_install.sh /ci_container_base/ci/test/ From fa21fd1dc2e5649f8c4e7c04d28312beb51761fb Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 29 Oct 2025 15:40:59 +0100 Subject: [PATCH 3/6] ci: Move macos snippet under DANGER_RUN_CI_ON_HOST This move-only refactor clarifies that macos assumes and requires DANGER_RUN_CI_ON_HOST. So move the snippet under the condition for self-documenting code. Can be reviewed with the git options: --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space --- ci/test/02_run_container.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/ci/test/02_run_container.py b/ci/test/02_run_container.py index 0865350d092..377ccfa9cfd 100755 --- a/ci/test/02_run_container.py +++ b/ci/test/02_run_container.py @@ -59,6 +59,14 @@ def main(): # Modify PATH to prepend the retry script, needed for CI_RETRY_EXE os.environ["PATH"] = f"{os.environ['BASE_ROOT_DIR']}/ci/retry:{os.environ['PATH']}" + # GNU getopt is required for the CI_RETRY_EXE script + if os.getenv("CI_OS_NAME") == "macos": + prefix = run( + ["brew", "--prefix", "gnu-getopt"], + stdout=subprocess.PIPE, + text=True, + ).stdout.strip() + os.environ["IN_GETOPT_BIN"] = f"{prefix}/bin/getopt" else: CI_IMAGE_LABEL = "bitcoin-ci-test" @@ -156,15 +164,6 @@ def main(): ).stdout.strip() os.environ["CI_CONTAINER_ID"] = container_id - # GNU getopt is required for the CI_RETRY_EXE script - if os.getenv("CI_OS_NAME") == "macos": - prefix = run( - ["brew", "--prefix", "gnu-getopt"], - stdout=subprocess.PIPE, - text=True, - ).stdout.strip() - os.environ["IN_GETOPT_BIN"] = f"{prefix}/bin/getopt" - run(["./ci/test/02_run_container.sh"]) # run the remainder if not os.getenv("DANGER_RUN_CI_ON_HOST"): print("Stop and remove CI container by ID") From eeee02ea53dd1a3fb2eb62acd68fbd797d9b9ba8 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 29 Oct 2025 12:31:34 +0100 Subject: [PATCH 4/6] ci: Untangle CI_EXEC bash function It contains a large `bash -c` string, which is hard to parse. So pull out components: * CI_EXEC is only called with absolute folders as args, so the `cd` is not needed in CI_EXEC. It is only needed to specify the working dir of running the tests in 03_test_script.sh, so move it there. * The PATH modification is only needed after commit 4756114e505cff8848fb6344ef9a48d8822066c1 to check that depends does work properly, even when the PATH contains a space. * This allows to also drop the `bash -c` and use the proper and safer "$@" to forward args without the risk of word splitting. --- ci/test/02_run_container.sh | 2 +- ci/test/03_test_script.sh | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/ci/test/02_run_container.sh b/ci/test/02_run_container.sh index 77c70f82bfa..eac8a0e589c 100755 --- a/ci/test/02_run_container.sh +++ b/ci/test/02_run_container.sh @@ -13,7 +13,7 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then fi CI_EXEC () { - $CI_EXEC_CMD_PREFIX bash -c "export PATH=\"/path_with space:\$PATH\" && cd \"${BASE_ROOT_DIR}\" && $*" + $CI_EXEC_CMD_PREFIX "$@" } export -f CI_EXEC diff --git a/ci/test/03_test_script.sh b/ci/test/03_test_script.sh index e61dc33edc9..39e13945e25 100755 --- a/ci/test/03_test_script.sh +++ b/ci/test/03_test_script.sh @@ -8,6 +8,9 @@ export LC_ALL=C.UTF-8 set -ex +cd "${BASE_ROOT_DIR}" + +export PATH="/path_with space:${PATH}" export ASAN_OPTIONS="detect_leaks=1:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1" 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:second_deadlock_stack=1" From fa83555d163ff7fdcdaaa0e34bfa3eaa41fa6dfc Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 3 Nov 2025 11:47:27 +0100 Subject: [PATCH 5/6] ci: Require rsync to pass In theory one could run the CI without the rsync package installed, and with DANGER_RUN_CI_ON_HOST=1. However, this seems to be an edge case. Simply requiring rsync to be installed is less code and avoids brittle edge cases around rsync failures. --- ci/test/02_run_container.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/test/02_run_container.sh b/ci/test/02_run_container.sh index eac8a0e589c..9771065e0ee 100755 --- a/ci/test/02_run_container.sh +++ b/ci/test/02_run_container.sh @@ -18,6 +18,6 @@ CI_EXEC () { export -f CI_EXEC # Normalize all folders to BASE_ROOT_DIR -CI_EXEC rsync --recursive --perms --stats --human-readable "${BASE_READ_ONLY_DIR}/" "${BASE_ROOT_DIR}" || echo "Nothing to copy from ${BASE_READ_ONLY_DIR}/" +CI_EXEC rsync --recursive --perms --stats --human-readable "${BASE_READ_ONLY_DIR}/" "${BASE_ROOT_DIR}" CI_EXEC "${BASE_ROOT_DIR}/ci/test/01_base_install.sh" CI_EXEC "${BASE_ROOT_DIR}/ci/test/03_test_script.sh" From fa336053aada79d13cd771ce025857256814465e Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 29 Oct 2025 13:11:04 +0100 Subject: [PATCH 6/6] Move ci_exec to the Python script The Bash script was acceptable, but CI_EXEC_CMD_PREFIX was a single string, relying on brittle word splitting that the shellcheck SC2086 would warn about. So just fix that by moving everything to the Python script and deleting the Bash script. This also removes the need to export the CI_CONTAINER_ID env var. --- ci/test/02_run_container.py | 23 +++++++++++++++++++++-- ci/test/02_run_container.sh | 23 ----------------------- 2 files changed, 21 insertions(+), 25 deletions(-) delete mode 100755 ci/test/02_run_container.sh diff --git a/ci/test/02_run_container.py b/ci/test/02_run_container.py index 377ccfa9cfd..d9ef27b490a 100755 --- a/ci/test/02_run_container.py +++ b/ci/test/02_run_container.py @@ -162,9 +162,28 @@ def main(): stdout=subprocess.PIPE, text=True, ).stdout.strip() - os.environ["CI_CONTAINER_ID"] = container_id - run(["./ci/test/02_run_container.sh"]) # run the remainder + def ci_exec(cmd_inner, **kwargs): + if os.getenv("DANGER_RUN_CI_ON_HOST"): + prefix = [] + else: + prefix = ["docker", "exec", container_id] + + return run([*prefix, *cmd_inner], **kwargs) + + # Normalize all folders to BASE_ROOT_DIR + ci_exec([ + "rsync", + "--recursive", + "--perms", + "--stats", + "--human-readable", + f"{os.environ['BASE_READ_ONLY_DIR']}/", + f"{os.environ['BASE_ROOT_DIR']}", + ]) + ci_exec([f"{os.environ['BASE_ROOT_DIR']}/ci/test/01_base_install.sh"]) + ci_exec([f"{os.environ['BASE_ROOT_DIR']}/ci/test/03_test_script.sh"]) + if not os.getenv("DANGER_RUN_CI_ON_HOST"): print("Stop and remove CI container by ID") run(["docker", "container", "kill", container_id]) diff --git a/ci/test/02_run_container.sh b/ci/test/02_run_container.sh deleted file mode 100755 index 9771065e0ee..00000000000 --- a/ci/test/02_run_container.sh +++ /dev/null @@ -1,23 +0,0 @@ -#!/usr/bin/env bash -# -# Copyright (c) 2018-present The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. - -export LC_ALL=C.UTF-8 - -set -o errexit -o pipefail -o xtrace - -if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then - export CI_EXEC_CMD_PREFIX="docker exec ${CI_CONTAINER_ID}" -fi - -CI_EXEC () { - $CI_EXEC_CMD_PREFIX "$@" -} -export -f CI_EXEC - -# Normalize all folders to BASE_ROOT_DIR -CI_EXEC rsync --recursive --perms --stats --human-readable "${BASE_READ_ONLY_DIR}/" "${BASE_ROOT_DIR}" -CI_EXEC "${BASE_ROOT_DIR}/ci/test/01_base_install.sh" -CI_EXEC "${BASE_ROOT_DIR}/ci/test/03_test_script.sh"