From c42d3f5f688fac6b0a52054312cd79a653396c7c Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Wed, 4 Nov 2026 14:52:22 +0100 Subject: [PATCH 1/2] Preserve PR build cache progress on POSIX timeouts --- .github/workflows/testpr.yaml | 34 +++++---- tools/ci_build.py | 81 ++++++++++++++++++++ tools/tests/test_ci_build.py | 134 ++++++++++++++++++++++++++++++++++ 3 files changed, 236 insertions(+), 13 deletions(-) create mode 100644 tools/ci_build.py create mode 100644 tools/tests/test_ci_build.py diff --git a/.github/workflows/testpr.yaml b/.github/workflows/testpr.yaml index 725f8a1f8..c93483fdf 100644 --- a/.github/workflows/testpr.yaml +++ b/.github/workflows/testpr.yaml @@ -253,6 +253,8 @@ jobs: - name: Build recipes id: build-recipes + # Leave time for cleanup and cache upload before the six-hour job limit. + timeout-minutes: 340 shell: bash -l {0} run: | set +e @@ -262,7 +264,11 @@ jobs: # independent recipes side by side, with a shared jobserver keeping them at the # runner's cores in total; Windows has no jobserver, and was no faster with two [ "$PLATFORM" != "win-64" ] && extra+=(--parallel-builds "3:$(getconf _NPROCESSORS_ONLN)") - pixi run rs "$DISTRO" build --target-platform "$PLATFORM" "${extra[@]}" 2>&1 | tee rattler-build.log + build=(python tools/robostack.py "$DISTRO" build --target-platform "$PLATFORM" "${extra[@]}") + if [ "$PLATFORM" != "win-64" ]; then + build=(python tools/ci_build.py --cache-dir "$CACHE_DIR" --timeout 19800 -- "${build[@]}") + fi + pixi run "${build[@]}" 2>&1 | tee rattler-build.log status=${PIPESTATUS[0]} echo "exit-code=${status}" >> "$GITHUB_OUTPUT" exit "$status" @@ -302,11 +308,9 @@ jobs: if-no-files-found: warn retention-days: 7 - - name: Drop the package a cancellation may have cut short - # Cancelling can kill the compiler while the build script carries on, and - # rattler-build then packages an empty build (seen on Windows); the newest - # artifact is the only one that can be affected. - if: ${{ always() && steps.build-recipes.outcome == 'cancelled' }} + - name: Drop the Windows package a cancellation may have cut short + # Windows builds one package at a time; POSIX builds are cleaned by ci_build.py. + if: ${{ always() && runner.os == 'Windows' && steps.build-recipes.outcome == 'cancelled' }} shell: bash -l {0} run: | newest=$(ls -t "$CACHE_DIR"/*.conda 2>/dev/null | head -1) @@ -314,7 +318,7 @@ jobs: - name: Save build cache id: save-cache - if: ${{ always() && contains(fromJSON('["success","failure","cancelled"]'), steps.build-recipes.outcome) }} + if: ${{ always() && contains(fromJSON('["success","failure","cancelled"]'), steps.build-recipes.outcome) && (runner.os == 'Windows' || steps.build-recipes.outputs.cache-ready == 'true') }} uses: actions/cache/save@v5 with: path: ${{ env.CACHE_DIR }} @@ -322,9 +326,9 @@ jobs: # Only the newest cache of a pull request/distribution/platform is restored; the ones # of earlier runs and attempts only push others out of the repository's 10 GB, so - # full rebuilds lost their progress. (Read-only token on forks: then this is skipped.) + # full rebuilds lost their progress. Forks have a read-only token, so skip deletion. - name: Drop this pull request's superseded build caches - if: ${{ always() && steps.save-cache.outcome == 'success' && github.event.pull_request.number }} + if: ${{ always() && steps.save-cache.outcome == 'success' && github.event.pull_request.number && github.event.pull_request.head.repo.full_name == github.repository }} continue-on-error: true shell: bash -l {0} env: @@ -332,12 +336,16 @@ jobs: PREFIX: ${{ runner.os }}-${{ matrix.distro }}-${{ matrix.target.platform }}-pr-${{ github.event.pull_request.number }}- KEEP: ${{ runner.os }}-${{ matrix.distro }}-${{ matrix.target.platform }}-pr-${{ github.event.pull_request.number }}-${{ github.run_id }}-${{ github.run_attempt }} run: | - # pixi's gh: the runner's is not on PATH on Windows (Clean up PATH) - pixi run gh cache list --repo "$GITHUB_REPOSITORY" --key "$PREFIX" --limit 100 --json id,key \ - --jq ".[] | select(.key != \"$KEEP\") | .id" | + # cache/save reports success even on upload failure: verify the replacement exists. + if ! ids=$(pixi run gh cache list --repo "$GITHUB_REPOSITORY" --ref "$GITHUB_REF" --key "$PREFIX" --limit 100 --json id,key \ + --jq "if any(.[]; .key == \"$KEEP\") then .[] | select(.key != \"$KEEP\") | .id else error(\"New cache not found\") end"); then + echo "::warning::Could not verify the new build cache; keeping all previous caches." + exit 0 + fi while read -r id; do + [ -n "$id" ] || continue pixi run gh cache delete "$id" --repo "$GITHUB_REPOSITORY" && echo "deleted cache $id" - done + done <<< "$ids" - name: Generate the staged build workflows (catches post-merge problems) shell: bash -l {0} diff --git a/tools/ci_build.py b/tools/ci_build.py new file mode 100644 index 000000000..d87b153c7 --- /dev/null +++ b/tools/ci_build.py @@ -0,0 +1,81 @@ +"""Bound a POSIX PR build and stop all its writers before saving the package cache.""" + +from __future__ import annotations + +import argparse +import os +from pathlib import Path +import re +import signal +import subprocess +import sys + + +class Interrupted(Exception): + def __init__(self, signum): + self.code = 124 if signum == signal.SIGALRM else 128 + signum + + +def run(cmd: list[str], cache_dir: Path, timeout: float) -> int: + cache_dir = cache_dir.resolve() + original = {p: (p.stat().st_mtime_ns, p.stat().st_size) for p in cache_dir.glob("*.conda")} + completed = set() + + def output(line): + if match := re.search(rb"Archive written to ['\"](.+\.conda)['\"]", line): + completed.add(Path(os.fsdecode(match[1])).resolve()) + sys.stdout.buffer.write(line) + sys.stdout.buffer.flush() + + def interrupt(signum, frame): + raise Interrupted(signum) + + # Isolate rattler-build, its build scripts and compilers from the runner's signals. + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, start_new_session=True) + previous = {s: signal.signal(s, interrupt) for s in (signal.SIGINT, signal.SIGTERM, signal.SIGALRM)} + signal.setitimer(signal.ITIMER_REAL, timeout) + try: + for line in proc.stdout: + output(line) + return proc.wait() + except Interrupted as exc: + print(f"Stopping build (exit {exc.code}); preserving completed packages.", flush=True) + return exc.code + finally: + signal.setitimer(signal.ITIMER_REAL, 0) + for s in previous: + signal.signal(s, signal.SIG_IGN) + try: + # Freeze first: a script must not package an empty build after its compiler dies. + try: + os.killpg(proc.pid, signal.SIGSTOP) + os.killpg(proc.pid, signal.SIGKILL) + except ProcessLookupError: + pass + proc.wait() + for line in proc.stdout: + output(line) + proc.stdout.close() + # Parallel builds can leave several partial archives, not just the newest one. + for p in cache_dir.glob("*.conda"): + if p not in completed and original.get(p) != (p.stat().st_mtime_ns, p.stat().st_size): + print(f"Removing unfinished archive: {p}", flush=True) + p.unlink() + # A runner-level hard kill must not let a later step save a still-live directory. + if destination := os.environ.get("GITHUB_OUTPUT"): + with open(destination, "a") as f: + f.write("cache-ready=true\n") + finally: + for s, handler in previous.items(): + signal.signal(s, handler) + + +if __name__ == "__main__": + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--cache-dir", type=Path, required=True) + parser.add_argument("--timeout", type=float, required=True, help="Build budget in seconds") + parser.add_argument("command", nargs=argparse.REMAINDER) + args = parser.parse_args() + if args.timeout <= 0 or not args.command or args.command[0] != "--" or len(args.command) < 2: + parser.error("provide a positive timeout and a command after --") + sys.exit(run(args.command[1:], args.cache_dir, args.timeout)) diff --git a/tools/tests/test_ci_build.py b/tools/tests/test_ci_build.py new file mode 100644 index 000000000..1fadeb76a --- /dev/null +++ b/tools/tests/test_ci_build.py @@ -0,0 +1,134 @@ +"""The PR build must stop its grandchildren before publishing a reusable cache.""" + +import os +from pathlib import Path +import signal +import subprocess +import sys +import time + +import pytest +import yaml + +ROOT = Path(__file__).resolve().parents[2] +pytestmark = pytest.mark.skipif(os.name != "posix", reason="POSIX CI supervisor") + + +def command(cache, timeout, script): + return [sys.executable, str(ROOT / "tools/ci_build.py"), "--cache-dir", str(cache), + "--timeout", str(timeout), "--", sys.executable, str(script)] + + +@pytest.mark.parametrize("signum", [signal.SIGINT, signal.SIGTERM, None]) +def test_interruption_stops_descendants_and_preserves_only_finished_archives(tmp_path, signum): + cache = tmp_path / "packages" + cache.mkdir() + (cache / "restored.conda").write_text("cached") + script = tmp_path / "build.py" + # A descendant that ignores graceful termination and keeps modifying the output. + script.write_text('''import os, signal, subprocess, sys, time +from pathlib import Path +cache = Path(sys.argv[0]).parent / "packages" +(cache / "completed.conda").write_text("complete") +print(f"Archive written to '{cache / 'completed.conda'}'", flush=True) +for i in range(3): + (cache / f"partial-{i}.conda").write_text("incomplete") +child = subprocess.Popen([sys.executable, "-c", """ +import signal, sys, time +from pathlib import Path +signal.signal(signal.SIGTERM, signal.SIG_IGN) +signal.signal(signal.SIGINT, signal.SIG_IGN) +while True: + Path(sys.argv[1]).write_text(str(time.monotonic_ns())) + time.sleep(.01) +""", str(cache / "heartbeat")]) +(cache / "ready").write_text(str(os.getpid())) +child.wait() +''') + output = tmp_path / "github-output" + env = {**os.environ, "GITHUB_OUTPUT": str(output)} + proc = subprocess.Popen(command(cache, 2 if signum is None else 30, script), env=env, + stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + group = None + try: + deadline = time.monotonic() + 10 + while not ((cache / "heartbeat").exists() and (cache / "ready").exists()): + assert proc.poll() is None, proc.communicate()[0].decode() + assert time.monotonic() < deadline, "fake build did not start" + time.sleep(.01) + group = int((cache / "ready").read_text()) + if signum is not None: + proc.send_signal(signum) + stdout, _ = proc.communicate(timeout=10) + assert proc.returncode == (124 if signum is None else 128 + signum), stdout.decode() + assert {p.name for p in cache.glob("*.conda")} == {"restored.conda", "completed.conda"} + assert output.read_text() == "cache-ready=true\n" + heartbeat = (cache / "heartbeat").read_bytes() + time.sleep(.1) + assert (cache / "heartbeat").read_bytes() == heartbeat + finally: + if proc.poll() is None: + proc.kill() + proc.wait() + if group is not None: + try: + os.killpg(group, signal.SIGKILL) + except ProcessLookupError: + pass + + +@pytest.mark.parametrize("status", [0, 7]) +def test_exit_status_and_replaced_incomplete_archive(tmp_path, status): + (tmp_path / "replaced.conda").write_text("previous build") + script = tmp_path / "build.py" + script.write_text(f'''from pathlib import Path +import sys +p = Path(__file__).parent / "replaced.conda" +p.write_text("partial") +sys.exit({status}) +''') + result = subprocess.run(command(tmp_path, 10, script), capture_output=True, timeout=15, + env={**os.environ, "GITHUB_OUTPUT": str(tmp_path / "outputs")}) + assert result.returncode == status, result.stdout.decode() + assert not (tmp_path / "replaced.conda").exists() + + +def test_workflow_only_saves_stopped_builds_and_verifies_cache_before_deleting(): + workflow = yaml.safe_load((ROOT / ".github/workflows/testpr.yaml").read_text()) + steps = workflow["jobs"]["build"]["steps"] + build = next(s for s in steps if s.get("id") == "build-recipes") + save = next(s for s in steps if s.get("id") == "save-cache") + prune = next(s for s in steps if s.get("name") == "Drop this pull request's superseded build caches") + assert build["timeout-minutes"] < 360 + assert "tools/ci_build.py" in build["run"] + assert "steps.build-recipes.outputs.cache-ready == 'true'" in save["if"] + assert "github.event.pull_request.head.repo.full_name == github.repository" in prune["if"] + assert '--ref "$GITHUB_REF"' in prune["run"] + assert "if ! ids=" in prune["run"] + assert prune["run"].index("exit 0") < prune["run"].index("cache delete") + + +@pytest.mark.parametrize("lookup_status,ids,deleted", [(0, "11\n22", "11\n22\n"), + (1, "11", ""), (0, "", "")]) +def test_cache_pruning_requires_verified_replacement(tmp_path, lookup_status, ids, deleted): + workflow = yaml.safe_load((ROOT / ".github/workflows/testpr.yaml").read_text()) + prune = next(s for s in workflow["jobs"]["build"]["steps"] + if s.get("name") == "Drop this pull request's superseded build caches") + pixi = tmp_path / "pixi" + pixi.write_text('''#!/bin/bash +if [ "$4" = list ]; then + printf '%s\\n' "$IDS" + exit "$LOOKUP_STATUS" +fi +[ "$4" = delete ] || exit 99 +printf '%s\\n' "$5" >> "$DELETED" +''') + pixi.chmod(0o755) + record = tmp_path / "deleted" + result = subprocess.run(["bash", "-c", prune["run"]], capture_output=True, text=True, timeout=10, + env={**os.environ, "PATH": f"{tmp_path}:{os.environ['PATH']}", + "IDS": ids, "LOOKUP_STATUS": str(lookup_status), "DELETED": str(record), + "GITHUB_REPOSITORY": "RoboStack/robostack", "GITHUB_REF": "refs/pull/144/merge", + "PREFIX": "cache-", "KEEP": "cache-new"}) + assert result.returncode == 0, result.stderr + assert (record.read_text() if record.exists() else "") == deleted From 327860e5e09435260fcdb14cbb5294deee14de86 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Wed, 4 Nov 2026 16:53:48 +0100 Subject: [PATCH 2/2] Keep cache regression tests focused on cancellation and safe pruning --- tools/tests/test_ci_build.py | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/tools/tests/test_ci_build.py b/tools/tests/test_ci_build.py index 1fadeb76a..3c62f29bc 100644 --- a/tools/tests/test_ci_build.py +++ b/tools/tests/test_ci_build.py @@ -77,37 +77,6 @@ def test_interruption_stops_descendants_and_preserves_only_finished_archives(tmp pass -@pytest.mark.parametrize("status", [0, 7]) -def test_exit_status_and_replaced_incomplete_archive(tmp_path, status): - (tmp_path / "replaced.conda").write_text("previous build") - script = tmp_path / "build.py" - script.write_text(f'''from pathlib import Path -import sys -p = Path(__file__).parent / "replaced.conda" -p.write_text("partial") -sys.exit({status}) -''') - result = subprocess.run(command(tmp_path, 10, script), capture_output=True, timeout=15, - env={**os.environ, "GITHUB_OUTPUT": str(tmp_path / "outputs")}) - assert result.returncode == status, result.stdout.decode() - assert not (tmp_path / "replaced.conda").exists() - - -def test_workflow_only_saves_stopped_builds_and_verifies_cache_before_deleting(): - workflow = yaml.safe_load((ROOT / ".github/workflows/testpr.yaml").read_text()) - steps = workflow["jobs"]["build"]["steps"] - build = next(s for s in steps if s.get("id") == "build-recipes") - save = next(s for s in steps if s.get("id") == "save-cache") - prune = next(s for s in steps if s.get("name") == "Drop this pull request's superseded build caches") - assert build["timeout-minutes"] < 360 - assert "tools/ci_build.py" in build["run"] - assert "steps.build-recipes.outputs.cache-ready == 'true'" in save["if"] - assert "github.event.pull_request.head.repo.full_name == github.repository" in prune["if"] - assert '--ref "$GITHUB_REF"' in prune["run"] - assert "if ! ids=" in prune["run"] - assert prune["run"].index("exit 0") < prune["run"].index("cache delete") - - @pytest.mark.parametrize("lookup_status,ids,deleted", [(0, "11\n22", "11\n22\n"), (1, "11", ""), (0, "", "")]) def test_cache_pruning_requires_verified_replacement(tmp_path, lookup_status, ids, deleted):