Merge bitcoin/bitcoin#33732: ci: Call docker exec from Python script to fix word splitting

fa336053aada79d13cd771ce025857256814465e Move ci_exec to the Python script (MarcoFalke)
fa83555d163ff7fdcdaaa0e34bfa3eaa41fa6dfc ci: Require rsync to pass (MarcoFalke)
eeee02ea53dd1a3fb2eb62acd68fbd797d9b9ba8 ci: Untangle CI_EXEC bash function (MarcoFalke)
fa21fd1dc2e5649f8c4e7c04d28312beb51761fb ci: Move macos snippet under DANGER_RUN_CI_ON_HOST (MarcoFalke)
fa37559ac5b7bf83eefa30e7770ccae9fd19556b ci: Document the retry script in PATH (MarcoFalke)
666675e95fe823b7809f64508aea5b57b1867c19 ci: Move folder creation and docker kill to Python script (MarcoFalke)

Pull request description:

  The remaining `ci/test/02_run_container.sh` is fine, but has a bunch of shellcheck SC2086 word splitting violations.

  This is fine currently, because the only place that needed them had additional escaping, and all other commands happened to split fine on spaces.

  However, this may change in the future. So fix it now, by rewriting it in Python, which is recommended in the dev notes.

ACKs for top commit:
  frankomosh:
    Code Review ACK [fa33605](fa336053aa)
  m3dwards:
    ACK fa336053aada79d13cd771ce025857256814465e

Tree-SHA512: 472decb13edca75566dffe49b9b3f554ab977fa60ec7902d5a060fe53381aee8606a10ff0c990a62ee2454dc6d9430cc064f58320b9043070b7bf08845413bf4

ci/test/02_run_container.py

@@ -47,7 +47,26 @@ 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)
+
+        # 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"
 
         # Use buildx unconditionally
@@ -142,18 +161,31 @@ def main():
             stdout=subprocess.PIPE,
             text=True,
         ).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"
+    def ci_exec(cmd_inner, **kwargs):
+        if os.getenv("DANGER_RUN_CI_ON_HOST"):
+            prefix = []
+        else:
+            prefix = ["docker", "exec", container_id]
 
-    run(["./ci/test/02_run_container.sh"])  # run the remainder
+        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])
 
 
 if __name__ == "__main__":

ci/test/02_run_container.sh

@@ -1,33 +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}"
-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 () {
-  $CI_EXEC_CMD_PREFIX bash -c "export PATH=\"/path_with space:${BASE_ROOT_DIR}/ci/retry:\$PATH\" && cd \"${BASE_ROOT_DIR}\" && $*"
-}
-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 "${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

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"

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/