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..7c625b2f94a8 100644 --- a/.github/workflows/test-e2e.yml +++ b/.github/workflows/test-e2e.yml @@ -382,6 +382,29 @@ 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 + timeout-minutes: 10 + env: + GH_TOKEN: ${{ github.token }} + WARM_KEY: ${{ steps.compilation-cache-key.outputs.fingerprint }} + 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 id: compile if: ${{ steps.reuse.outputs.hit != 'true' }} @@ -433,6 +456,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 +471,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 +492,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.record-inputs.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..fcc237d820df --- /dev/null +++ b/scripts/ci/e2e_warm_derived_data.py @@ -0,0 +1,189 @@ +#!/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 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 +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"}) +# Beyond this a download loses to the compile it replaces. +MAX_ARTIFACT_BYTES = 12 * 1024**3 + + +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): + os.utime(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(): + # 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) + 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}") + 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]: + repository = os.environ["GITHUB_REPOSITORY"] + 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: + 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 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) + 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/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-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..884ba3ceabda 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,10 @@ 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", "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", ""), } 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..509d22e381d8 --- /dev/null +++ b/tests/test_e2e_warm_derived_data.py @@ -0,0 +1,114 @@ +#!/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_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"]) + + +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_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) + self.assertTrue((destination / "Build/Intermediates.noindex/a.o").is_file()) + + +if __name__ == "__main__": + unittest.main()