Merge bitcoin/bitcoin#34548: ci: Add and use ci-windows-cross.py helper

fa13b13239 ci: [refactor] Use pathlib over os.path (MarcoFalke)
fa2719ab1b ci: [refactor] Move run_unit_tests to ci-windows-cross.py (MarcoFalke)
fa99ba5f14 ci: Set PREVIOUS_RELEASES_DIR env var in ci-windows-cross.py (MarcoFalke)
fa4a1cab6c ci: Move run_functional_tests into ci-windows-cross.py (MarcoFalke)
1111108685 ci: [refactor] Move pyzmq install and get_previous_releases into ci-windows-cross.py (MarcoFalke)
fac9c7bd66 ci: [refactor] Move config.ini rewrite to ci-windows-cross.py (MarcoFalke)
faf7389466 ci: Move check_manifests step to ci-windows-cross.py (MarcoFalke)
fa674d55df ci: [refactor] Move print_version step into ci-windows-cross.py helper (MarcoFalke)

Pull request description:

  Currently the ci yaml has a mix of Bash and Pwsh snippets, which is problematic:

  * The `shellcheck` tool does not review the Bash
  * The ci yaml is not merged with master on re-runs, but the code is, leading to possibly confusing CI errors on re-runs
  * The Pwsh isn't reviewed at all by any tool
  * It is tedious to run the CI commands locally on Windows

  Fix all issues by extracting them into a step-based Python script.

ACKs for top commit:
  janb84:
    re ACK fa13b13239
  hebasto:
    ACK fa13b13239, I have reviewed the code and it looks OK.

Tree-SHA512: 23d21d3bfb07e102fe1cc15ba5749d553d9766ae6c4a7648bd77df0705469bd138c76a9a2fdeb4d91d3f889a425b7caf25878ecb2e68b604faf9665f8df4eb6d

.github/ci-windows-cross.py

@@ -0,0 +1,165 @@
+#!/usr/bin/env python3
+# Copyright (c) The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or https://opensource.org/license/mit/.
+
+import argparse
+import os
+import shlex
+import subprocess
+import sys
+from pathlib import Path
+
+
+def run(cmd, **kwargs):
+    print("+ " + shlex.join(cmd), flush=True)
+    kwargs.setdefault("check", True)
+    try:
+        return subprocess.run(cmd, **kwargs)
+    except Exception as e:
+        sys.exit(str(e))
+
+
+def print_version():
+    bitcoind = Path.cwd() / "bin" / "bitcoind.exe"
+    run([str(bitcoind), "-version"])
+
+
+def check_manifests():
+    release_dir = Path.cwd() / "bin"
+    manifest_path = release_dir / "bitcoind.manifest"
+
+    cmd_bitcoind_manifest = [
+        "mt.exe",
+        "-nologo",
+        f"-inputresource:{release_dir / 'bitcoind.exe'}",
+        f"-out:{manifest_path}",
+    ]
+    run(cmd_bitcoind_manifest)
+    print(manifest_path.read_text())
+
+    skipped = {  # Skip as they currently do not have manifests
+        "fuzz.exe",
+        "bench_bitcoin.exe",
+        "test_kernel.exe",
+    }
+    for entry in release_dir.iterdir():
+        if entry.suffix.lower() != ".exe":
+            continue
+        if entry.name in skipped:
+            print(f"Skipping {entry.name} (no manifest present)")
+            continue
+        print(f"Checking {entry.name}")
+        run(["mt.exe", "-nologo", f"-inputresource:{entry}", "-validate_manifest"])
+
+
+def prepare_tests():
+    workspace = Path.cwd()
+    config_path = workspace / "test" / "config.ini"
+    rpcauth_path = workspace / "share" / "rpcauth" / "rpcauth.py"
+    replacements = {
+        "SRCDIR=": f"SRCDIR={workspace}",
+        "BUILDDIR=": f"BUILDDIR={workspace}",
+        "RPCAUTH=": f"RPCAUTH={rpcauth_path}",
+    }
+    lines = config_path.read_text().splitlines()
+    for index, line in enumerate(lines):
+        for prefix, new_value in replacements.items():
+            if line.startswith(prefix):
+                lines[index] = new_value
+                break
+    content = "\n".join(lines) + "\n"
+    config_path.write_text(content)
+    print(content)
+    previous_releases_dir = Path(os.environ["PREVIOUS_RELEASES_DIR"])
+    cmd_download_prev_rel = [
+        sys.executable,
+        str(workspace / "test" / "get_previous_releases.py"),
+        "--target-dir",
+        str(previous_releases_dir),
+    ]
+    run(cmd_download_prev_rel)
+    run([sys.executable, "-m", "pip", "install", "pyzmq"])
+
+
+def run_functional_tests():
+    workspace = Path.cwd()
+    num_procs = str(os.process_cpu_count())
+    test_runner_cmd = [
+        sys.executable,
+        str(workspace / "test" / "functional" / "test_runner.py"),
+        "--jobs",
+        num_procs,
+        "--quiet",
+        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.
+        "--exclude",
+        "feature_unsupported_utxo_db.py",
+        # See https://github.com/bitcoin/bitcoin/issues/31409.
+        "--exclude",
+        "wallet_multiwallet.py",
+    ]
+    run(test_runner_cmd)
+
+    # Run feature_unsupported_utxo_db sequentially in ASCII-only tmp dir,
+    # because it is excluded above due to lack of UTF-8 support in the
+    # ancient release.
+    cmd_feature_unsupported_db = [
+        sys.executable,
+        str(workspace / "test" / "functional" / "feature_unsupported_utxo_db.py"),
+        "--previous-releases",
+        "--tmpdir",
+        str(Path(workspace) / "test_feature_unsupported_utxo_db"),
+    ]
+    run(cmd_feature_unsupported_db)
+
+
+def run_unit_tests():
+    # Can't use ctest here like other jobs as we don't have a CMake build tree.
+    commands = [
+        ["./bin/test_bitcoin-qt.exe"],
+        # Intentionally run sequentially here, to catch test case failures caused by dirty global state from prior test cases:
+        ["./bin/test_bitcoin.exe", "-l", "test_suite"],
+        ["./src/secp256k1/bin/exhaustive_tests.exe"],
+        ["./src/secp256k1/bin/noverify_tests.exe"],
+        ["./src/secp256k1/bin/tests.exe"],
+        ["./src/univalue/object.exe"],
+        ["./src/univalue/unitester.exe"],
+    ]
+    for cmd in commands:
+        run(cmd)
+
+
+def main():
+    parser = argparse.ArgumentParser(description="Utility to run Windows CI steps.")
+    steps = [
+        "print_version",
+        "check_manifests",
+        "prepare_tests",
+        "run_unit_tests",
+        "run_functional_tests",
+    ]
+    parser.add_argument("step", choices=steps, help="CI step to perform.")
+    args = parser.parse_args()
+
+    os.environ.setdefault(
+        "PREVIOUS_RELEASES_DIR",
+        str(Path.cwd() / "previous_releases"),
+    )
+
+    if args.step == "print_version":
+        print_version()
+    elif args.step == "check_manifests":
+        check_manifests()
+    elif args.step == "prepare_tests":
+        prepare_tests()
+    elif args.step == "run_unit_tests":
+        run_unit_tests()
+    elif args.step == "run_functional_tests":
+        run_functional_tests()
+
+
+if __name__ == "__main__":
+    main()

.github/ci-windows.py

@@ -106,7 +106,7 @@ def prepare_tests(ci_type):
     if ci_type == "standard":
         run([sys.executable, "-m", "pip", "install", "pyzmq"])
     elif ci_type == "fuzz":
-        repo_dir = os.path.join(os.getcwd(), "qa-assets")
+        repo_dir = str(Path.cwd() / "qa-assets")
         clone_cmd = [
             "git",
             "clone",
@@ -120,9 +120,9 @@ def prepare_tests(ci_type):
 
 
 def run_tests(ci_type):
-    build_dir = "build"
+    build_dir = Path.cwd() / "build"
     num_procs = str(os.process_cpu_count())
-    release_bin = os.path.join(os.getcwd(), build_dir, "bin", "Release")
+    release_bin = build_dir / "bin" / "Release"
 
     if ci_type == "standard":
         test_envs = {
@@ -136,12 +136,12 @@ def run_tests(ci_type):
             "BITCOINCHAINSTATE": "bitcoin-chainstate.exe",
         }
         for var, exe in test_envs.items():
-            os.environ[var] = os.path.join(release_bin, exe)
+            os.environ[var] = str(release_bin / exe)
 
         ctest_cmd = [
             "ctest",
             "--test-dir",
-            build_dir,
+            str(build_dir),
             "--output-on-failure",
             "--stop-on-failure",
             "-j",
@@ -153,26 +153,26 @@ def run_tests(ci_type):
 
         test_cmd = [
             sys.executable,
-            os.path.join(build_dir, "test", "functional", "test_runner.py"),
+            str(build_dir / "test" / "functional" / "test_runner.py"),
             "--jobs",
             num_procs,
             "--quiet",
-            f"--tmpdirprefix={os.getcwd()}",
+            f"--tmpdirprefix={Path.cwd()}",
             "--combinedlogslen=99999999",
             *shlex.split(os.environ.get("TEST_RUNNER_EXTRA", "").strip()),
         ]
         run(test_cmd)
 
     elif ci_type == "fuzz":
-        os.environ["BITCOINFUZZ"] = os.path.join(release_bin, "fuzz.exe")
+        os.environ["BITCOINFUZZ"] = str(release_bin / "fuzz.exe")
         fuzz_cmd = [
             sys.executable,
-            os.path.join(build_dir, "test", "fuzz", "test_runner.py"),
+            str(build_dir / "test" / "fuzz" / "test_runner.py"),
             "--par",
             num_procs,
             "--loglevel",
             "DEBUG",
-            os.path.join(os.getcwd(), "qa-assets", "fuzz_corpora"),
+            str(Path.cwd() / "qa-assets" / "fuzz_corpora"),
         ]
         run(fuzz_cmd)
 

.github/workflows/ci.yml

@@ -416,67 +416,25 @@ jobs:
           name: ${{ matrix.artifact-name }}-${{ github.run_id }}
 
       - name: Run bitcoind.exe
-        run: ./bin/bitcoind.exe -version
+        run: py -3 .github/ci-windows-cross.py print_version
 
       - *SET_UP_VS
 
       - name: Check executable manifests
-        shell: pwsh -Command "$PSVersionTable; $PSNativeCommandUseErrorActionPreference = $true; $ErrorActionPreference = 'Stop'; & '{0}'"
+        run: py -3 .github/ci-windows-cross.py check_manifests
-        run: |
-          mt.exe -nologo -inputresource:bin\bitcoind.exe -out:bitcoind.manifest
-          Get-Content bitcoind.manifest
-
-          Get-ChildItem -Filter "bin\*.exe" | ForEach-Object {
-            $exeName = $_.Name
-
-            # Skip as they currently do not have manifests
-            if ($exeName -eq "fuzz.exe" -or $exeName -eq "bench_bitcoin.exe" -or $exeName -eq "test_kernel.exe") {
-              Write-Host "Skipping $exeName (no manifest present)"
-              return
-            }
-
-            Write-Host "Checking $exeName"
-            & mt.exe -nologo -inputresource:$_.FullName -validate_manifest
-          }
 
       - name: Run unit tests
-        # Can't use ctest here like other jobs as we don't have a CMake build tree.
+        run: py -3 .github/ci-windows-cross.py run_unit_tests
-        run: |
-          ./bin/test_bitcoin-qt.exe
-          ./bin/test_bitcoin.exe -l test_suite  # Intentionally run sequentially here, to catch test case failures caused by dirty global state from prior test cases.
-          ./src/secp256k1/bin/exhaustive_tests.exe
-          ./src/secp256k1/bin/noverify_tests.exe
-          ./src/secp256k1/bin/tests.exe
-          ./src/univalue/object.exe
-          ./src/univalue/unitester.exe
 
-      - name: Adjust paths in test/config.ini
+      - name: Prepare Windows test environment
-        shell: pwsh
         run: |
-          (Get-Content "test/config.ini") -replace '(?<=^SRCDIR=).*', '${{ github.workspace }}' -replace '(?<=^BUILDDIR=).*', '${{ github.workspace }}' -replace '(?<=^RPCAUTH=).*', '${{ github.workspace }}/share/rpcauth/rpcauth.py' | Set-Content "test/config.ini"
+          py -3 .github/ci-windows-cross.py prepare_tests
-          Get-Content "test/config.ini"
-
-      - name: Set previous release directory
-        run: |
-          echo "PREVIOUS_RELEASES_DIR=${{ runner.temp }}/previous_releases" >> "$GITHUB_ENV"
-
-      - name: Get previous releases
-        run: ./test/get_previous_releases.py --target-dir $PREVIOUS_RELEASES_DIR
 
       - name: Run functional tests
         env:
-          TEST_RUNNER_EXTRA: ${{ github.event_name != 'pull_request' && '--extended' || '' }}
+          TEST_RUNNER_EXTRA: "--timeout-factor=${{ env.TEST_RUNNER_TIMEOUT_FACTOR }} ${{ case(github.event_name == 'pull_request', '', '--extended') }}"
         run: |
-          py -3 -m pip install pyzmq
+          py -3 .github/ci-windows-cross.py run_functional_tests
-          py -3 test/functional/test_runner.py --jobs $NUMBER_OF_PROCESSORS --quiet --tmpdirprefix="$RUNNER_TEMP" --combinedlogslen=99999999 --timeout-factor=$TEST_RUNNER_TIMEOUT_FACTOR $TEST_RUNNER_EXTRA \
-            `# feature_unsupported_utxo_db.py fails on Windows because of emojis in the test data directory.` \
-            --exclude feature_unsupported_utxo_db.py \
-            `# See https://github.com/bitcoin/bitcoin/issues/31409.` \
-            --exclude wallet_multiwallet.py
-          # Run feature_unsupported_utxo_db sequentially in ASCII-only tmp dir,
-          # because it is excluded above due to lack of UTF-8 support in the
-          # ancient release.
-          py -3 test/functional/feature_unsupported_utxo_db.py --previous-releases --tmpdir="${RUNNER_TEMP}/test_feature_unsupported_utxo_db"
 
   ci-matrix:
     name: ${{ matrix.name }}