Merge bitcoin/bitcoin#34614: ci: Put space and non-ASCII char in scratch dir

7777a92a92 ci: Use path with spaces on windows as well (MarcoFalke)
fac6c4270d ci: Put space and non-ASCII char in scratch dir (MarcoFalke)
fa38759823 ci: Require $FILE_ENV (MarcoFalke)

Pull request description:

  It seems unlikely that many users have a space in their paths, but it seems a use-case worth enough to be tested by CI, so that it does not have to be done manually. Ref https://github.com/bitcoin/bitcoin/pull/33929#discussion_r2590523065 / https://mirror.b10c.me/bitcoin-bitcoin/33929/#discussion_r2590523065

  So do that here, and also add a non-ASCII char while touching.

  Also, fix all tests that are broken and assume no space exists in paths.

ACKs for top commit:
  hebasto:
    ACK 7777a92a92.
  sedited:
    ACK 7777a92a92

Tree-SHA512: eceb1f6c932c6966cdca8ca8df750081ec5134db5e5f558f7d955716409117bec7c8585d75865e2c98bc1ae7394f3ce64dff87bcebe1e68591afaeef1831d6dd

.github/ci-windows-cross.py

@@ -96,7 +96,7 @@ def run_functional_tests():
         "--jobs",
         num_procs,
         "--quiet",
-        f"--tmpdirprefix={workspace}",
+        f"--tmpdirprefix={workspace / '_ _'}",
         "--combinedlogslen=99999999",
         *shlex.split(os.environ.get("TEST_RUNNER_EXTRA", "").strip()),
         # feature_unsupported_utxo_db.py fails on Windows because of emojis in the test data directory.

.github/ci-windows.py

@@ -198,7 +198,7 @@ def run_tests(ci_type):
             "--jobs",
             num_procs,
             "--quiet",
-            f"--tmpdirprefix={workspace}",
+            f"--tmpdirprefix={workspace / '_ _'}",
             "--combinedlogslen=99999999",
             *shlex.split(os.environ.get("TEST_RUNNER_EXTRA", "").strip()),
         ]

ci/test/00_setup_env.sh

@@ -6,7 +6,7 @@
 
 export LC_ALL=C.UTF-8
 
-set -o errexit -o pipefail -o xtrace
+set -o errexit -o nounset -o pipefail -o xtrace
 
 # The source root dir, usually from git, usually read-only.
 # The ci system copies this folder.
@@ -20,15 +20,14 @@ export BASE_ROOT_DIR="${BASE_ROOT_DIR:-/ci_container_base}"
 # This folder exists only on the ci guest, and on the ci host as a volume.
 export DEPENDS_DIR=${DEPENDS_DIR:-$BASE_ROOT_DIR/depends}
 # A folder for the ci system to put temporary files (build result, datadirs for tests, ...)
+# The name contains a space and a non-ASCII symbol to confirm the build and
+# tests handle word-splitting and UTF8 correctly.
 # This folder only exists on the ci guest.
-export BASE_SCRATCH_DIR=${BASE_SCRATCH_DIR:-$BASE_ROOT_DIR/ci/scratch}
+export BASE_SCRATCH_DIR=${BASE_SCRATCH_DIR:-$BASE_ROOT_DIR/ci/scratch_ ₿🧪_}
 
 echo "Setting specific values in env"
-if [ -n "${FILE_ENV}" ]; then
-  set -o errexit;
-  # shellcheck disable=SC1090
-  source "${FILE_ENV}"
-fi
+# shellcheck disable=SC1090
+source "${FILE_ENV}"
 
 echo "Fallback to default values in env (if not yet set)"
 # The number of parallel jobs to pass down to make and test_runner.py

ci/test/03_test_script.sh

@@ -109,7 +109,7 @@ ccache --zero-stats
 # Folder where the build is done.
 BASE_BUILD_DIR=${BASE_BUILD_DIR:-$BASE_SCRATCH_DIR/build-$HOST}
 
-BITCOIN_CONFIG_ALL="$BITCOIN_CONFIG_ALL -DCMAKE_INSTALL_PREFIX=$BASE_OUTDIR -Werror=dev"
+BITCOIN_CONFIG_ALL="$BITCOIN_CONFIG_ALL '-DCMAKE_INSTALL_PREFIX=$BASE_OUTDIR' -Werror=dev"
 
 if [[ "${RUN_IWYU}" == true || "${RUN_TIDY}" == true ]]; then
   BITCOIN_CONFIG_ALL="$BITCOIN_CONFIG_ALL -DCMAKE_EXPORT_COMPILE_COMMANDS=ON"

test/functional/feature_logging.py

@@ -37,8 +37,8 @@ class LoggingTest(BitcoinTestFramework):
         invdir = self.relative_log_path("foo")
         invalidname = os.path.join("foo", "foo.log")
         self.stop_node(0)
-        exp_stderr = r"Error: Could not open debug log file \S+$"
-        self.nodes[0].assert_start_raises_init_error([f"-debuglogfile={invalidname}"], exp_stderr, match=ErrorMatch.FULL_REGEX)
+        exp_stderr = "Error: Could not open debug log file "
+        self.nodes[0].assert_start_raises_init_error([f"-debuglogfile={invalidname}"], exp_stderr, match=ErrorMatch.PARTIAL_REGEX)
         assert not os.path.isfile(os.path.join(invdir, "foo.log"))
 
         # check that invalid log (relative) works after path exists
@@ -51,7 +51,7 @@ class LoggingTest(BitcoinTestFramework):
         self.stop_node(0)
         invdir = os.path.join(self.options.tmpdir, "foo")
         invalidname = os.path.join(invdir, "foo.log")
-        self.nodes[0].assert_start_raises_init_error([f"-debuglogfile={invalidname}"], exp_stderr, match=ErrorMatch.FULL_REGEX)
+        self.nodes[0].assert_start_raises_init_error([f"-debuglogfile={invalidname}"], exp_stderr, match=ErrorMatch.PARTIAL_REGEX)
         assert not os.path.isfile(os.path.join(invdir, "foo.log"))
 
         # check that invalid log (absolute) works after path exists

test/functional/feature_notifications.py

@@ -31,6 +31,10 @@ LARGE_WORK_INVALID_CHAIN_WARNING = (
 def notify_outputname(walletname, txid):
     return txid if platform.system() == 'Windows' else f'{walletname}_{txid}'
 
+def shell_escape_posix(arg):
+    # Identical to ShellEscape() in the C++ code
+    return "'" + arg.replace("'", "'\"'\"'") + "'"
+
 
 class NotificationsTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -51,13 +55,18 @@ class NotificationsTest(BitcoinTestFramework):
         os.mkdir(self.walletnotify_dir)
         os.mkdir(self.shutdownnotify_dir)
 
+        if platform.system() == 'Windows':
+            walletnotify_path = f"\"{os.path.join(self.walletnotify_dir, notify_outputname('%w', '%s'))}\""
+        else:
+            walletnotify_path = f"{shell_escape_posix(os.path.join(self.walletnotify_dir, ''))}{notify_outputname('%w', '%s')}"
+
         # -alertnotify and -blocknotify on node0, walletnotify on node1
         self.extra_args = [[
-            f"-alertnotify=echo %s >> {self.alertnotify_file}",
-            f"-blocknotify=echo > {os.path.join(self.blocknotify_dir, '%s')}",
-            f"-shutdownnotify=echo > {self.shutdownnotify_file}",
+            f"-alertnotify=echo %s >> \"{self.alertnotify_file}\"",
+            f"-blocknotify=echo > \"{os.path.join(self.blocknotify_dir, '%s')}\"",
+            f"-shutdownnotify=echo > \"{self.shutdownnotify_file}\"",
         ], [
-            f"-walletnotify=echo %h_%b > {os.path.join(self.walletnotify_dir, notify_outputname('%w', '%s'))}",
+            f"-walletnotify=echo %h_%b > {walletnotify_path}",
         ]]
         self.wallet_names = [self.default_wallet_name, self.wallet]
         super().setup_network()