From a6c46c35dbb5448544c850e71a1f7625abc34e3e Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 23 Sep 2026 09:10:41 -0700 Subject: [PATCH 1/2] ci(e2e): start builds from main's DerivedData so test-only changes skip the app compile The E2E build adopts the newest DerivedData a main run of test-e2e.yml published, then restores the recorded modification time onto every input whose content is unchanged. Changed inputs keep their checkout time, so Xcode rebuilds only what differs. Main runs of contained revisions publish their DerivedData as a 3-day artifact. Every step is optional: any miss or failure is today's cold build. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci-guards.yml | 1 + .github/workflows/test-e2e.yml | 54 +++++++ scripts/ci/e2e_warm_derived_data.py | 173 ++++++++++++++++++++++ tests/test-execution.toml | 4 + tests/test_ci_guard_workflow_structure.py | 1 + tests/test_ci_self_hosted_guard.sh | 5 +- tests/test_e2e_warm_derived_data.py | 96 ++++++++++++ 7 files changed, 333 insertions(+), 1 deletion(-) create mode 100755 scripts/ci/e2e_warm_derived_data.py create mode 100644 tests/test_e2e_warm_derived_data.py diff --git a/.github/workflows/ci-guards.yml b/.github/workflows/ci-guards.yml index cf5b218dbd0f..8bad61c6c842 100644 --- a/.github/workflows/ci-guards.yml +++ b/.github/workflows/ci-guards.yml @@ -725,6 +725,7 @@ jobs: python3 tests/test_ci_guard_workflow_structure.py python3 tests/test_app_host_test_products.py python3 tests/test_reuse_app_host_products.py + python3 tests/test_e2e_warm_derived_data.py python3 tests/test_ci_product_publication.py - name: Validate Python R2 appcast upload guard diff --git a/.github/workflows/test-e2e.yml b/.github/workflows/test-e2e.yml index 62e3d85431e3..fa8c52bac006 100644 --- a/.github/workflows/test-e2e.yml +++ b/.github/workflows/test-e2e.yml @@ -382,6 +382,21 @@ jobs: sleep $((attempt * 5)) done + # A fresh checkout makes every file newer than any earlier build, so + # without this even a one-line test change recompiles the app host. + - name: Adopt main's DerivedData + id: warm + if: ${{ steps.reuse.outputs.hit != 'true' }} + continue-on-error: true + env: + GH_TOKEN: ${{ github.token }} + WARM_KEY: ${{ steps.compilation-cache-key.outputs.fingerprint }} + run: python3 scripts/ci/e2e_warm_derived_data.py restore "$GITHUB_WORKSPACE" "$CMUX_DERIVED_DATA_PATH" "$WARM_KEY" + + - name: Record build input times + if: ${{ steps.reuse.outputs.hit != 'true' }} + run: python3 scripts/ci/e2e_warm_derived_data.py record "$GITHUB_WORKSPACE" "$CMUX_DERIVED_DATA_PATH/cmux-e2e-input-mtimes.json" + - name: Build the app-host and UI test product id: compile if: ${{ steps.reuse.outputs.hit != 'true' }} @@ -433,6 +448,10 @@ jobs: REUSE_PRODUCER: ${{ steps.reuse.outputs.producer_run_id }} REUSE_SAVED: ${{ steps.reuse.outputs.macos_runner_minutes_saved }} REUSE_TRANSFER: ${{ steps.reuse.outputs.transfer_seconds }} + WARM_HIT: ${{ steps.warm.outputs.hit }} + WARM_PRODUCER: ${{ steps.warm.outputs.producer_run_id }} + WARM_CHANGED: ${{ steps.warm.outputs.changed_inputs }} + WARM_REASON: ${{ steps.warm.outputs.reason }} run: | set -euo pipefail { @@ -444,6 +463,11 @@ jobs: else echo "Compiled from source in ${COMPILE_SECONDS:-unknown} s." echo "No product to adopt (${REUSE_MISSES:-none})." + if [ "$WARM_HIT" = "true" ]; then + echo "Started from the DerivedData run $WARM_PRODUCER built; ${WARM_CHANGED:-unknown} inputs differed." + else + echo "Started from empty DerivedData (${WARM_REASON:-not attempted})." + fi fi echo "Archive: ${ARCHIVE_BYTES:-unknown} bytes." } >> "$GITHUB_STEP_SUMMARY" @@ -460,6 +484,36 @@ jobs: echo "contained=$contained" >> "$GITHUB_OUTPUT" echo "Selected revision contained in main: $contained" + # Same trust rule as the compilation cache: only code main already + # contains may seed builds of other revisions. + - name: Package DerivedData for later builds + id: warm-package + continue-on-error: true + if: ${{ !cancelled() && github.ref == 'refs/heads/main' && steps.compile.outcome == 'success' && steps.revision-on-main.outputs.contained == 'true' }} + run: | + set -euo pipefail + archive="$RUNNER_TEMP/derived-data.tar.gz" + tar -cf - -C "$CMUX_DERIVED_DATA_PATH" --exclude ./Logs --exclude ./Index.noindex . | gzip -1 > "$archive" + bytes="$(wc -c < "$archive" | tr -d ' ')" + echo "DerivedData archive: $bytes bytes" + if [ "$bytes" -gt $((12 * 1024 * 1024 * 1024)) ]; then + echo "::warning::DerivedData archive exceeds 12 GiB; not publishing" + exit 0 + fi + echo "bytes=$bytes" >> "$GITHUB_OUTPUT" + echo "publish=true" >> "$GITHUB_OUTPUT" + + - name: Publish DerivedData for later builds + continue-on-error: true + if: ${{ !cancelled() && steps.warm-package.outputs.publish == 'true' }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: e2e-derived-data-v1-${{ steps.compilation-cache-key.outputs.fingerprint }} + path: ${{ runner.temp }}/derived-data.tar.gz + if-no-files-found: error + retention-days: 3 + compression-level: 0 + - name: Bound E2E compilation cache id: compilation-cache-bound continue-on-error: true diff --git a/scripts/ci/e2e_warm_derived_data.py b/scripts/ci/e2e_warm_derived_data.py new file mode 100755 index 000000000000..02163e1c4236 --- /dev/null +++ b/scripts/ci/e2e_warm_derived_data.py @@ -0,0 +1,173 @@ +#!/usr/bin/env python3 +"""Let an E2E build start from the last DerivedData main compiled. + + e2e_warm_derived_data.py record WORKSPACE MANIFEST + e2e_warm_derived_data.py replay WORKSPACE MANIFEST + e2e_warm_derived_data.py restore WORKSPACE DERIVED_DATA KEY + +The compiled product archive carries Build/Products only. Without the build +database and intermediates next to it, xcodebuild cannot tell what is already +built, so a revision that changes one test file recompiles the whole app host: +691 of the 735 seconds `build-for-testing` spends is the app scheme. + +Xcode decides what to rebuild from modification times, and a fresh checkout +stamps every file with the checkout time. `record` writes the content digest +and modification time of every build input before a compile. `replay` restores +the recorded time only onto files whose content is byte-identical, so an +unchanged file looks as old as the build that consumed it and a changed file +keeps its checkout time and is rebuilt. Correctness never depends on how close +the adopted DerivedData is to this revision; distance only costs compile time. + +`restore` adopts the newest DerivedData archive for KEY that a `main` run of +this workflow published. Any miss, expiry or transfer failure is a cold build. +""" +from __future__ import annotations + +import hashlib +import json +import os +from pathlib import Path +import shutil +import subprocess +import sys +import tarfile +import tempfile +import zipfile + +WORKFLOW_PATH = ".github/workflows/test-e2e.yml" +ARCHIVE = "derived-data.tar.gz" +MANIFEST = "cmux-e2e-input-mtimes.json" +PREFIX = "e2e-derived-data-v1-" +# Never walk into build outputs or git metadata: they are not inputs, and +# DerivedData lives inside the workspace on every runner pool. +SKIPPED_DIRECTORIES = frozenset({".git", "DerivedData"}) + + +def digest(path: Path) -> str: + checksum = hashlib.sha256() + with path.open("rb") as stream: + for block in iter(lambda: stream.read(1024 * 1024), b""): + checksum.update(block) + return checksum.hexdigest() + + +def inputs(workspace: Path): + for root, directories, files in os.walk(workspace): + directories[:] = sorted(d for d in directories if d not in SKIPPED_DIRECTORIES) + for name in sorted(files): + path = Path(root, name) + if path.is_symlink() or not path.is_file(): + continue + yield path.relative_to(workspace).as_posix(), path + + +def record(workspace: Path) -> dict[str, list]: + return { + relative: [digest(path), path.stat().st_mtime_ns] + for relative, path in inputs(workspace) + } + + +def replay(workspace: Path, recorded: dict[str, list]) -> tuple[int, int]: + restored = changed = 0 + for relative, path in inputs(workspace): + entry = recorded.get(relative) + if entry is None or entry[0] != digest(path): + changed += 1 + continue + os.utime(path, ns=(entry[1], entry[1])) + restored += 1 + return restored, changed + + +def api(path: str) -> dict: + output = subprocess.run(["gh", "api", path], check=True, capture_output=True, text=True).stdout + return json.loads(output) + + +def trusted(artifact: dict, repository: str) -> bool: + """Only main's own runs of this workflow may seed a build of another ref.""" + run = artifact.get("workflow_run") or {} + if artifact.get("expired") or run.get("head_branch") != "main": + return False + if run.get("head_repository_id") not in (None, run.get("repository_id")): + return False + details = api(f"repos/{repository}/actions/runs/{run['id']}") + return details.get("path") == WORKFLOW_PATH and details.get("event") == "workflow_dispatch" + + +def newest(repository: str, key: str) -> dict | None: + listing = api(f"repos/{repository}/actions/artifacts?name={PREFIX}{key}&per_page=20") + candidates = sorted(listing.get("artifacts", []), key=lambda a: a.get("created_at", ""), reverse=True) + return next((a for a in candidates if trusted(a, repository)), None) + + +def extract(archive: Path, destination: Path) -> None: + with tarfile.open(archive) as bundle: + for member in bundle.getmembers(): + target = (destination / member.name).resolve() + if destination.resolve() not in target.parents and target != destination.resolve(): + raise ValueError(f"archive member escapes DerivedData: {member.name}") + if member.issym() or member.islnk(): + link = Path(member.linkname) + if link.is_absolute() or ".." in link.parts: + raise ValueError(f"archive link escapes DerivedData: {member.name}") + bundle.extractall(destination) + + +def restore(workspace: Path, derived: Path, key: str) -> dict[str, object]: + repository = os.environ["GITHUB_REPOSITORY"] + artifact = newest(repository, key) + if artifact is None: + return {"hit": "false", "reason": "no-main-derived-data"} + with tempfile.TemporaryDirectory() as staging: + bundle = Path(staging, "artifact.zip") + with bundle.open("wb") as stream: + subprocess.run( + ["gh", "api", f"repos/{repository}/actions/artifacts/{artifact['id']}/zip"], + check=True, stdout=stream, + ) + with zipfile.ZipFile(bundle) as archive: + archive.extractall(staging) + extract(Path(staging, ARCHIVE), derived) + recorded = json.loads((derived / MANIFEST).read_text()) + restored, changed = replay(workspace, recorded) + return { + "hit": "true", + "producer_run_id": str(artifact["workflow_run"]["id"]), + "unchanged_inputs": str(restored), + "changed_inputs": str(changed), + } + + +def main(argv: list[str]) -> int: + if len(argv) == 4 and argv[1] in {"record", "replay"}: + workspace, manifest = Path(argv[2]).resolve(), Path(argv[3]) + if argv[1] == "record": + manifest.write_text(json.dumps(record(workspace), sort_keys=True)) + print(f"Recorded {len(json.loads(manifest.read_text()))} build inputs") + else: + restored, changed = replay(workspace, json.loads(manifest.read_text())) + print(f"Replayed {restored} unchanged inputs; {changed} changed or new") + return 0 + if len(argv) == 5 and argv[1] == "restore": + derived = Path(argv[3]) + try: + result = restore(Path(argv[2]).resolve(), derived, argv[4]) + except (OSError, ValueError, KeyError, subprocess.CalledProcessError, zipfile.BadZipFile, tarfile.TarError) as error: + # A half-extracted DerivedData is worse than none: start cold. + shutil.rmtree(derived, ignore_errors=True) + derived.mkdir(parents=True, exist_ok=True) + result = {"hit": "false", "reason": f"{type(error).__name__}: {error}"[:200]} + print(json.dumps(result, sort_keys=True)) + if "GITHUB_OUTPUT" in os.environ: + with open(os.environ["GITHUB_OUTPUT"], "a") as handle: + for name, value in result.items(): + handle.write(f"{name}={value}\n") + return 0 + print(__doc__, file=sys.stderr) + return 2 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/tests/test-execution.toml b/tests/test-execution.toml index ea1f9366c5c1..0dc6c42fb059 100644 --- a/tests/test-execution.toml +++ b/tests/test-execution.toml @@ -575,6 +575,10 @@ lane = "linux-guard" path = "tests/test_preflight_trust.py" lane = "linux-guard" +[[test]] +path = "tests/test_e2e_warm_derived_data.py" +lane = "linux-guard" + [[test]] path = "tests/test_reuse_app_host_products.py" lane = "linux-guard" diff --git a/tests/test_ci_guard_workflow_structure.py b/tests/test_ci_guard_workflow_structure.py index ce5829d96384..db441c14f89d 100644 --- a/tests/test_ci_guard_workflow_structure.py +++ b/tests/test_ci_guard_workflow_structure.py @@ -13,6 +13,7 @@ "python3 tests/test_ci_guard_workflow_structure.py", "python3 tests/test_app_host_test_products.py", "python3 tests/test_reuse_app_host_products.py", + "python3 tests/test_e2e_warm_derived_data.py", "python3 tests/test_ci_product_publication.py", ] diff --git a/tests/test_ci_self_hosted_guard.sh b/tests/test_ci_self_hosted_guard.sh index f141dfae8d92..513b70a7b9af 100755 --- a/tests/test_ci_self_hosted_guard.sh +++ b/tests/test_ci_self_hosted_guard.sh @@ -185,7 +185,7 @@ import sys import yaml document = yaml.safe_load(open(sys.argv[1])) -# Compilation caching and the fast artifact transport are optimizations with +# Compilation caching, adopted DerivedData and the fast artifact transport are optimizations with # canonical fallbacks. Everything else must fail the job it runs in. allowed = { ("build", "compilation-cache-restore", "Restore E2E compilation cache", "actions/cache/restore"), @@ -193,6 +193,9 @@ allowed = { ("build", "compilation-cache-bound", "Bound E2E compilation cache", ""), ("build", "revision-on-main", "Check the selected revision against main", ""), ("build", "reuse", "Reuse a compiled product instead of building one", ""), + ("build", "warm", "Adopt main's DerivedData", ""), + ("build", "warm-package", "Package DerivedData for later builds", ""), + ("build", None, "Publish DerivedData for later builds", "actions/upload-artifact"), ("test", "parallel-product", "Read the compiled test product over parallel range requests", ""), } for job_id, job in document["jobs"].items(): diff --git a/tests/test_e2e_warm_derived_data.py b/tests/test_e2e_warm_derived_data.py new file mode 100644 index 000000000000..98967a9b56df --- /dev/null +++ b/tests/test_e2e_warm_derived_data.py @@ -0,0 +1,96 @@ +#!/usr/bin/env python3 +"""Adopting main's DerivedData must rebuild exactly the inputs that changed.""" +import io +import os +from pathlib import Path +import sys +import tarfile +import tempfile +import unittest +from unittest import mock + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "scripts/ci")) +import e2e_warm_derived_data as warm + +BUILD_TIME_NS = 1_700_000_000_000_000_000 + + +class ReplayTimes(unittest.TestCase): + def setUp(self): + self.root = Path(tempfile.mkdtemp()) + self.producer = self.root / "producer" + self.consumer = self.root / "consumer" + for workspace in (self.producer, self.consumer): + (workspace / "Sources").mkdir(parents=True) + (workspace / "cmuxTests").mkdir() + (workspace / "Sources/App.swift").write_text("let app = 1\n") + (workspace / "cmuxTests/AppTests.swift").write_text("let test = 1\n") + for path in self.producer.rglob("*.swift"): + os.utime(path, ns=(BUILD_TIME_NS, BUILD_TIME_NS)) + (self.producer / "DerivedData").mkdir() + (self.producer / "DerivedData/output.o").write_text("object") + (self.producer / ".git").mkdir() + (self.producer / ".git/index").write_text("git") + + def mtime(self, relative): + return (self.consumer / relative).stat().st_mtime_ns + + def test_unchanged_inputs_take_the_producer_time_and_changed_inputs_do_not(self): + recorded = warm.record(self.producer) + (self.consumer / "cmuxTests/AppTests.swift").write_text("let test = 2\n") + (self.consumer / "cmuxTests/NewTests.swift").write_text("let added = 1\n") + + restored, changed = warm.replay(self.consumer, recorded) + + self.assertEqual((restored, changed), (1, 2)) + self.assertEqual(self.mtime("Sources/App.swift"), BUILD_TIME_NS) + self.assertGreater(self.mtime("cmuxTests/AppTests.swift"), BUILD_TIME_NS) + self.assertGreater(self.mtime("cmuxTests/NewTests.swift"), BUILD_TIME_NS) + + def test_build_outputs_and_git_metadata_are_not_inputs(self): + recorded = warm.record(self.producer) + self.assertEqual(sorted(recorded), ["Sources/App.swift", "cmuxTests/AppTests.swift"]) + + +class TrustedProducers(unittest.TestCase): + def artifact(self, branch="main", expired=False): + return {"expired": expired, "workflow_run": {"id": 7, "head_branch": branch, "repository_id": 1, "head_repository_id": 1}} + + def test_only_main_dispatches_of_this_workflow_are_adopted(self): + run = {"path": warm.WORKFLOW_PATH, "event": "workflow_dispatch"} + with mock.patch.object(warm, "api", return_value=run): + self.assertTrue(warm.trusted(self.artifact(), "o/r")) + self.assertFalse(warm.trusted(self.artifact(branch="feature"), "o/r")) + self.assertFalse(warm.trusted(self.artifact(expired=True), "o/r")) + with mock.patch.object(warm, "api", return_value={**run, "path": ".github/workflows/other.yml"}): + self.assertFalse(warm.trusted(self.artifact(), "o/r")) + + +class ArchiveBounds(unittest.TestCase): + def archive(self, name, link=None): + path = Path(tempfile.mkdtemp(), "derived-data.tar.gz") + with tarfile.open(path, "w:gz") as bundle: + member = tarfile.TarInfo(name) + if link is not None: + member.type, member.linkname = tarfile.SYMTYPE, link + bundle.addfile(member) + else: + member.size = 1 + bundle.addfile(member, io.BytesIO(b"x")) + return path + + def test_members_outside_derived_data_are_rejected(self): + destination = Path(tempfile.mkdtemp()) + for archive in (self.archive("../escape"), self.archive("link", link="/etc/passwd")): + with self.assertRaises(ValueError): + warm.extract(archive, destination) + self.assertEqual(list(destination.iterdir()), []) + + def test_a_contained_archive_extracts(self): + destination = Path(tempfile.mkdtemp()) + warm.extract(self.archive("Build/Intermediates.noindex/a.o"), destination) + self.assertTrue((destination / "Build/Intermediates.noindex/a.o").is_file()) + + +if __name__ == "__main__": + unittest.main() From eadec1a7ec9040bf80c959ece397602b24dc28d7 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 23 Sep 2026 09:18:28 -0700 Subject: [PATCH 2/2] ci(e2e): restamp changed inputs, bound the adopted archive, keep in-tree links Review of the warm DerivedData adoption found a changed input could keep an archive's older mtime (GhosttyKit, SwiftPM binaries) and escape a rebuild. Replay now stamps every changed or new input with the current time. Adoption also skips archives over 12 GiB, times out after 10 minutes, treats any error as a cold build, accepts absolute links that stay inside DerivedData, and ignores fresh-checkout inode changes. The helper joins the product identity inputs, and publishing requires a recorded input manifest. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/test-e2e.yml | 12 ++++++++++-- scripts/ci/e2e_warm_derived_data.py | 26 +++++++++++++++++++++----- scripts/ci/product_input_identity.py | 1 + tests/test_ci_self_hosted_guard.sh | 1 + tests/test_e2e_warm_derived_data.py | 18 ++++++++++++++++++ 5 files changed, 51 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test-e2e.yml b/.github/workflows/test-e2e.yml index fa8c52bac006..7c625b2f94a8 100644 --- a/.github/workflows/test-e2e.yml +++ b/.github/workflows/test-e2e.yml @@ -388,13 +388,21 @@ jobs: id: warm if: ${{ steps.reuse.outputs.hit != 'true' }} continue-on-error: true + timeout-minutes: 10 env: GH_TOKEN: ${{ github.token }} WARM_KEY: ${{ steps.compilation-cache-key.outputs.fingerprint }} - run: python3 scripts/ci/e2e_warm_derived_data.py restore "$GITHUB_WORKSPACE" "$CMUX_DERIVED_DATA_PATH" "$WARM_KEY" + run: | + set -euo pipefail + # A fresh checkout gives every file a new inode; without this the + # build system reruns every task whose inputs merely moved. + defaults write com.apple.dt.XCBuild IgnoreFileSystemDeviceInodeChanges -bool YES + python3 scripts/ci/e2e_warm_derived_data.py restore "$GITHUB_WORKSPACE" "$CMUX_DERIVED_DATA_PATH" "$WARM_KEY" - name: Record build input times + id: record-inputs if: ${{ steps.reuse.outputs.hit != 'true' }} + continue-on-error: true run: python3 scripts/ci/e2e_warm_derived_data.py record "$GITHUB_WORKSPACE" "$CMUX_DERIVED_DATA_PATH/cmux-e2e-input-mtimes.json" - name: Build the app-host and UI test product @@ -489,7 +497,7 @@ jobs: - name: Package DerivedData for later builds id: warm-package continue-on-error: true - if: ${{ !cancelled() && github.ref == 'refs/heads/main' && steps.compile.outcome == 'success' && steps.revision-on-main.outputs.contained == 'true' }} + if: ${{ !cancelled() && github.ref == 'refs/heads/main' && steps.compile.outcome == 'success' && steps.record-inputs.outcome == 'success' && steps.revision-on-main.outputs.contained == 'true' }} run: | set -euo pipefail archive="$RUNNER_TEMP/derived-data.tar.gz" diff --git a/scripts/ci/e2e_warm_derived_data.py b/scripts/ci/e2e_warm_derived_data.py index 02163e1c4236..fcc237d820df 100755 --- a/scripts/ci/e2e_warm_derived_data.py +++ b/scripts/ci/e2e_warm_derived_data.py @@ -14,8 +14,10 @@ stamps every file with the checkout time. `record` writes the content digest and modification time of every build input before a compile. `replay` restores the recorded time only onto files whose content is byte-identical, so an -unchanged file looks as old as the build that consumed it and a changed file -keeps its checkout time and is rebuilt. Correctness never depends on how close +unchanged file looks as old as the build that consumed it, and stamps every +other file with the current time. A changed file cannot keep an old time: files +unpacked from an archive (GhosttyKit, SwiftPM binary artifacts) carry the +archive's times, which may predate the producer's build. Correctness never depends on how close the adopted DerivedData is to this revision; distance only costs compile time. `restore` adopts the newest DerivedData archive for KEY that a `main` run of @@ -41,6 +43,8 @@ # Never walk into build outputs or git metadata: they are not inputs, and # DerivedData lives inside the workspace on every runner pool. SKIPPED_DIRECTORIES = frozenset({".git", "DerivedData"}) +# Beyond this a download loses to the compile it replaces. +MAX_ARTIFACT_BYTES = 12 * 1024**3 def digest(path: Path) -> str: @@ -73,6 +77,7 @@ def replay(workspace: Path, recorded: dict[str, list]) -> tuple[int, int]: for relative, path in inputs(workspace): entry = recorded.get(relative) if entry is None or entry[0] != digest(path): + os.utime(path) changed += 1 continue os.utime(path, ns=(entry[1], entry[1])) @@ -109,10 +114,19 @@ def extract(archive: Path, destination: Path) -> None: if destination.resolve() not in target.parents and target != destination.resolve(): raise ValueError(f"archive member escapes DerivedData: {member.name}") if member.issym() or member.islnk(): + # Xcode links within DerivedData, sometimes by absolute path; + # the key pins that path, so it is the same on both sides. link = Path(member.linkname) - if link.is_absolute() or ".." in link.parts: + base = destination if member.islnk() or link.is_absolute() else target.parent + resolved = (base / link).resolve() + if destination.resolve() not in resolved.parents and resolved != destination.resolve(): raise ValueError(f"archive link escapes DerivedData: {member.name}") - bundle.extractall(destination) + if hasattr(tarfile, "tar_filter"): + # Every member and link target is bounded above. The default + # `data` filter would also refuse Xcode's absolute in-tree links. + bundle.extractall(destination, filter="tar") + else: + bundle.extractall(destination) def restore(workspace: Path, derived: Path, key: str) -> dict[str, object]: @@ -120,6 +134,8 @@ def restore(workspace: Path, derived: Path, key: str) -> dict[str, object]: artifact = newest(repository, key) if artifact is None: return {"hit": "false", "reason": "no-main-derived-data"} + if int(artifact.get("size_in_bytes") or 0) > MAX_ARTIFACT_BYTES: + return {"hit": "false", "reason": "derived-data-too-large"} with tempfile.TemporaryDirectory() as staging: bundle = Path(staging, "artifact.zip") with bundle.open("wb") as stream: @@ -154,7 +170,7 @@ def main(argv: list[str]) -> int: derived = Path(argv[3]) try: result = restore(Path(argv[2]).resolve(), derived, argv[4]) - except (OSError, ValueError, KeyError, subprocess.CalledProcessError, zipfile.BadZipFile, tarfile.TarError) as error: + except Exception as error: # noqa: BLE001 - every failure means a cold build # A half-extracted DerivedData is worse than none: start cold. shutil.rmtree(derived, ignore_errors=True) derived.mkdir(parents=True, exist_ok=True) diff --git a/scripts/ci/product_input_identity.py b/scripts/ci/product_input_identity.py index 231269c68c8d..6cd8c5c70f36 100644 --- a/scripts/ci/product_input_identity.py +++ b/scripts/ci/product_input_identity.py @@ -24,6 +24,7 @@ PRODUCT_CI_INPUTS = frozenset({ "scripts/ci/app_host_test_products.py", "scripts/ci/compile-app-host-test-product.sh", + "scripts/ci/e2e_warm_derived_data.py", "scripts/ci/canonical-build-root.sh", "scripts/ci/sanitize-xcode-source-packages-cache.py", }) diff --git a/tests/test_ci_self_hosted_guard.sh b/tests/test_ci_self_hosted_guard.sh index 513b70a7b9af..884ba3ceabda 100755 --- a/tests/test_ci_self_hosted_guard.sh +++ b/tests/test_ci_self_hosted_guard.sh @@ -194,6 +194,7 @@ allowed = { ("build", "revision-on-main", "Check the selected revision against main", ""), ("build", "reuse", "Reuse a compiled product instead of building one", ""), ("build", "warm", "Adopt main's DerivedData", ""), + ("build", "record-inputs", "Record build input times", ""), ("build", "warm-package", "Package DerivedData for later builds", ""), ("build", None, "Publish DerivedData for later builds", "actions/upload-artifact"), ("test", "parallel-product", "Read the compiled test product over parallel range requests", ""), diff --git a/tests/test_e2e_warm_derived_data.py b/tests/test_e2e_warm_derived_data.py index 98967a9b56df..509d22e381d8 100644 --- a/tests/test_e2e_warm_derived_data.py +++ b/tests/test_e2e_warm_derived_data.py @@ -47,6 +47,18 @@ def test_unchanged_inputs_take_the_producer_time_and_changed_inputs_do_not(self) self.assertGreater(self.mtime("cmuxTests/AppTests.swift"), BUILD_TIME_NS) self.assertGreater(self.mtime("cmuxTests/NewTests.swift"), BUILD_TIME_NS) + def test_a_changed_input_unpacked_with_an_old_time_is_still_rebuilt(self): + recorded = warm.record(self.producer) + # An archive-extracted file (GhosttyKit, SwiftPM binaries) keeps the + # archive's time, which can predate the producer's build. + header = self.consumer / "Sources/App.swift" + header.write_text("let app = 2\n") + os.utime(header, ns=(BUILD_TIME_NS - 10**12, BUILD_TIME_NS - 10**12)) + + warm.replay(self.consumer, recorded) + + self.assertGreater(self.mtime("Sources/App.swift"), BUILD_TIME_NS) + def test_build_outputs_and_git_metadata_are_not_inputs(self): recorded = warm.record(self.producer) self.assertEqual(sorted(recorded), ["Sources/App.swift", "cmuxTests/AppTests.swift"]) @@ -86,6 +98,12 @@ def test_members_outside_derived_data_are_rejected(self): warm.extract(archive, destination) self.assertEqual(list(destination.iterdir()), []) + def test_an_absolute_link_inside_derived_data_is_accepted(self): + destination = Path(tempfile.mkdtemp()) + target = str(destination / "Build/Products/Debug/PackageFrameworks") + warm.extract(self.archive("Build/Products/link", link=target), destination) + self.assertTrue((destination / "Build/Products/link").is_symlink()) + def test_a_contained_archive_extracts(self): destination = Path(tempfile.mkdtemp()) warm.extract(self.archive("Build/Intermediates.noindex/a.o"), destination)