From 68d9fb0eeb2d423cdf9254cec4919e7e84627ca7 Mon Sep 17 00:00:00 2001 From: clover caruso Date: Sat, 10 Oct 2026 19:29:56 -0700 Subject: [PATCH] fix: invalidate local packages before shared Cargo cache gates Derive local path packages, including excluded vendor patches, from Cargo metadata. Clean their artifacts before concurrent CI lanes and desktop, web, relay and iOS build epochs while retaining registry and Git dependencies. Scope cleanup to each target and release profile so other architectures remain available until packaging. Reject empty local-package metadata and stop before builds or packaging if cleanup fails. Reproduce stale tests with frozen source mtimes in two disposable checkouts; verify changed Rust, build-script data and vendor code, plus dependency and other-target artifact retention. Use the frozen CI Linux wrapper too. Fixes #112 Assisted-by: gpt-6.1-sol --- apps/ios/build-rust.sh | 5 +- crates/snowbound/linux/package.sh | 1 + tools/TESTING.md | 13 +++- tools/canvas/build_macos.py | 5 ++ tools/cargo_fresh.py | 26 +++++++ tools/check_public.py | 1 + tools/ci.py | 22 +++++- tools/release.py | 4 ++ tools/release_relay.py | 3 + tools/release_web.py | 6 +- tools/test_cargo_fresh.py | 110 ++++++++++++++++++++++++++++++ tools/test_release.py | 2 +- 12 files changed, 192 insertions(+), 6 deletions(-) create mode 100644 tools/cargo_fresh.py create mode 100644 tools/test_cargo_fresh.py diff --git a/apps/ios/build-rust.sh b/apps/ios/build-rust.sh index 2737aad4120df5b480bb01f45bcfdf581510b140..1135cc8e524153f5cac29e60545866846f138740 100755 --- a/apps/ios/build-rust.sh +++ b/apps/ios/build-rust.sh @@ -11,4 +11,7 @@ root="$SRCROOT/../.." # Xcode's SDK variables would point host build scripts at the iOS SDK. exec env -i HOME="$HOME" PATH="$HOME/.cargo/bin:/etc/profiles/per-user/$USER/bin:/run/current-system/sw/bin:/usr/bin:/bin:/usr/sbin:/sbin" USER="$USER" \ CARGO_TARGET_DIR="$root/target/ios" IPHONEOS_DEPLOYMENT_TARGET="$IPHONEOS_DEPLOYMENT_TARGET" \ - cargo build --manifest-path "$root/Cargo.toml" -p mobile --target "$target" $release + sh -ec ' + python3 "$1/tools/cargo_fresh.py" --target "$2" $3 + exec cargo build --manifest-path "$1/Cargo.toml" -p mobile --target "$2" $3 + ' sh "$root" "$target" "$release" diff --git a/crates/snowbound/linux/package.sh b/crates/snowbound/linux/package.sh index bef78477ae8b222b1f31aa24fc72bbd6d86376ff..8e827d78287cd7a6272ef824c87f779d17e736c2 100755 --- a/crates/snowbound/linux/package.sh +++ b/crates/snowbound/linux/package.sh @@ -10,6 +10,7 @@ mkdir -p "$target/dist" [ $# -gt 0 ] || set -- x86_64 aarch64 for arch do triple=$arch-unknown-linux-gnu + python3 "$root/tools/cargo_fresh.py" --target "$triple" --release sh "$root/platform/linux/cargo.sh" "$arch" build \ --manifest-path "$root/Cargo.toml" --release -p snowbound # PT_INTERP becomes PT_NULL, so the kernel starts the executable at `loader`'s entry, which diff --git a/tools/TESTING.md b/tools/TESTING.md index cb38492137dbdaa1f63e8ff94af7d0c8fbadbe44..cc82bb4ce5b3de129294d034a147849aa009a3b0 100644 --- a/tools/TESTING.md +++ b/tools/TESTING.md @@ -7,9 +7,20 @@ checkout's `@`) in the jj workspace `workspaces/ci`: it points that workspace's own commit, a child of `main`, at the revision's files, so other checkouts' edits in progress never reach the result, and a working copy is frozen as it was when the run started. Its build cache is -`workspaces/ci/target`, apart from every agent's `target/`. Runs wait for one +`workspaces/ci/target`, which can link to a shared `target/`. Runs wait for one another, and the exit status is the result. +Cargo normally checks source mtimes, so a shared cache can reuse another +checkout's artifacts when the current sources are older. CI and release/build +entry points clean local packages from `cargo metadata` before each build epoch, +scoped to its target and release profile where applicable. This removes only +workspace and other path package artifacts, including vendored patches; +registry and Git dependencies stay cached. +All CI cleanup precedes concurrent lanes. Direct Cargo runs after switching +checkouts need the same cleanup; `python3 tools/cargo_fresh.py` accepts Cargo's +`--target`, `--target-dir`, and `--release` options. Shared-cache builds and +cleanup must run serially across checkouts. + | Lane | Runs | | --- | --- | | `fmt` | `cargo fmt --all --check` | diff --git a/tools/canvas/build_macos.py b/tools/canvas/build_macos.py index f47e05205fdd899dfd22e51d42f2cff0beaa8e63..3f2fdc481409a6ac4008f6adac84a520fad85637 100644 --- a/tools/canvas/build_macos.py +++ b/tools/canvas/build_macos.py @@ -10,6 +10,7 @@ import plistlib import re import shutil import subprocess +import sys import tempfile parser = argparse.ArgumentParser(description=__doc__) @@ -38,6 +39,8 @@ minimum = '10.6' if args.snow_leopard else {'x86_64': '10.13', 'aarch64': '11.0' if args.output and (args.output.suffix != '.app' or args.output.exists()): parser.error('Choose a new output path ending in .app.') root = Path(__file__).resolve().parents[2] +sys.path.insert(0, str(root / 'tools')) +from cargo_fresh import clean_local # The Apple Developer team and what is registered under it. With the team's Developer ID # Application identity and a Developer ID profile for BUNDLE_ID naming CONTAINER, the app is # signed with hardened runtime and the iCloud container (iCloud notebooks, NSUbiquitousContainers); @@ -88,6 +91,7 @@ def developer_id_profile(): return None profile = ['--release'] if args.release else [] if args.snow_leopard: + clean_local(root, *profile, command=[root / 'platform/snow-leopard/cargo.sh']) subprocess.run([root / 'platform/snow-leopard/cargo.sh', 'build', '-p', 'snowbound', '--no-default-features'] + profile, cwd=root, check=True) target = root / 'target/snow-leopard' @@ -98,6 +102,7 @@ else: cross = [] if args.arch == host else ['--target', f'{args.arch}-apple-darwin'] if cross: built = target / cross[1] + clean_local(root, *profile, *cross) subprocess.run(['cargo', 'build', '-p', 'snowbound'] + profile + cross, cwd=root, check=True, env={**os.environ, 'MACOSX_DEPLOYMENT_TARGET': minimum} if args.arch == 'x86_64' else None) bundle = args.output.resolve() if args.output else target / 'Snowbound.app' diff --git a/tools/cargo_fresh.py b/tools/cargo_fresh.py new file mode 100644 index 0000000000000000000000000000000000000000..8fe30d11266c46f1b2dccd371e55172a1528cd5d --- /dev/null +++ b/tools/cargo_fresh.py @@ -0,0 +1,26 @@ +"""Discard local package artifacts before building against a shared Cargo cache.""" +import json +from pathlib import Path +import subprocess +import sys + + +def local_packages(root, environment=None): + metadata = json.loads(subprocess.check_output( + ['cargo', 'metadata', '--locked', '--all-features', '--format-version=1', + '--manifest-path', str(root / 'Cargo.toml')], cwd=root, env=environment)) + packages = sorted({package['name'] for package in metadata['packages'] if package['source'] is None}) + if not packages: + raise ValueError('Cargo metadata contains no local packages') + return packages + + +def clean_local(root, *args, environment=None, command=('cargo',)): + packages = local_packages(root, environment) + subprocess.run([*command, 'clean', '--locked', '--manifest-path', str(root / 'Cargo.toml'), + *map(str, args), *(arg for package in packages for arg in ('-p', package))], + cwd=root, env=environment, check=True) + + +if __name__ == '__main__': + clean_local(Path(__file__).resolve().parents[1], *sys.argv[1:]) diff --git a/tools/check_public.py b/tools/check_public.py index f4915d2e0a3402f70f993ff9d707db5d0d24f3db..581c0e27ecb1d1bd09af9bf7f17b4ff226e6f512 100644 --- a/tools/check_public.py +++ b/tools/check_public.py @@ -24,6 +24,7 @@ def main(): commands = [ ('python-dependencies', [sys.executable, '-c', 'import PIL, pdfplumber']), ('format', ['cargo', 'fmt', '--all', '--check']), + ('clean', [sys.executable, str(root / 'tools/cargo_fresh.py')]), ('rust', ['cargo', 'test', '--locked', '--workspace', '--all-features', '--all-targets']), ('doctests', ['cargo', 'test', '--locked', '--workspace', '--all-features', '--doc']), ('clippy', ['cargo', 'clippy', '--locked', '--workspace', '--all-features', '--all-targets', '--', '-D', 'warnings']), diff --git a/tools/ci.py b/tools/ci.py index c644657bb1c9610a1bf555e27e5c5d8f88482eba..23da9832023b6b05fb20ae2f14a69247b7838365 100644 --- a/tools/ci.py +++ b/tools/ci.py @@ -17,6 +17,8 @@ import sys import threading import time +from cargo_fresh import local_packages + ROOT = Path(__file__).resolve().parents[1] # The checkout holding the jj repository, which a nested workspace's `.jj/repo` file names. STORE = ROOT / '.jj/repo' @@ -30,11 +32,11 @@ JSON = '--message-format=json' def cargo(*args): - return ['cargo', args[0], '--locked', JSON, *args[1:]] + return ['cargo', args[0], '--locked', *([] if args[0] == 'clean' else [JSON]), *args[1:]] def cross(script, arch, *args): - return ['sh', script, arch, args[0], '--locked', JSON, *args[1:]] + return ['sh', script, arch, *cargo(*args)[1:]] def lanes(): @@ -67,6 +69,7 @@ def lanes(): for arch in ('x86_64', 'aarch64'): result.append(dict( name=f'windows-{arch}', minutes=45, packages=['snowbound'], paths=('platform/windows/',), + clean=cross(windows, arch, 'clean'), # x86_64's nightly Clippy would lint std, which it builds, and nightly's own new lints. commands=[*([] if arch == 'x86_64' else [cross(windows, arch, *cross_clippy)]), cross(windows, arch, 'build', '-p', 'snowbound')], @@ -75,6 +78,7 @@ def lanes(): for arch in ('x86_64', 'aarch64'): result.append(dict( name=f'linux-{arch}', minutes=30, packages=['snowbound'], paths=('platform/linux/',), + clean=cross(linux, arch, 'clean'), commands=[cross(linux, arch, *cross_clippy), cross(linux, arch, 'build', '-p', 'snowbound')], environment={'CARGO_TARGET_DIR': str(TARGET / 'linux')}, missing=None if shutil.which('zig') else 'needs zig')) @@ -86,6 +90,7 @@ def lanes(): wasm = web_environment() result.append(dict( name='web', minutes=20, packages=['snowbound'], paths=('crates/snowbound/web/', 'tools/release_web.py', 'tools/web/'), + clean=cargo('clean', '--target', 'wasm32-unknown-unknown'), # Clippy builds it; the module itself is linked by release_web.py. commands=[cargo('clippy', '-p', 'snowbound', '--target', 'wasm32-unknown-unknown', '--no-default-features', '--features', 'wgpu,live', '--', '-D', 'warnings'), ['python3', 'tools/web/receipt_probe.py']], @@ -101,6 +106,7 @@ def lanes(): missing=None if shutil.which('xcodebuild') else 'needs Xcode')) result.append(dict( name='macos-10.6', minutes=45, packages=['snowbound'], paths=('platform/snow-leopard/',), + clean=['sh', CI / 'platform/snow-leopard/cargo.sh', 'clean', '--locked'], commands=[['sh', CI / 'platform/snow-leopard/cargo.sh', 'build', '--locked', JSON, '-p', 'snowbound', '--no-default-features']], environment={'SNOW_LEOPARD_SDK': str(sdk)}, @@ -429,6 +435,18 @@ def main(): if not key.startswith(('ONESTORE_', 'SNOWBOUND_', 'CARGO_TARGET_DIR'))} run = Run(folder, environment) + ready = [lane for lane in chosen if not lane.get('missing')] + cleaning = ([(cargo('clean'), {})] + if any(lane['name'] in ('test', 'clippy', 'python') for lane in ready) else []) + cleaning += [(lane['clean'], lane.get('environment', {})) for lane in ready if 'clean' in lane] + packages = local_packages(CI, environment) if cleaning else [] + for command, extra in cleaning: + command += [arg for package in packages for arg in ('-p', package)] + code = run.execute(command, folder / 'clean.log', time.monotonic() + 300, extra) + if code != 0: + lock.close() + sys.exit(f'Workspace cleanup {status(code)}: {folder / "clean.log"}') + def gate(lane): result = {'name': lane['name']} if lane.get('missing'): diff --git a/tools/release.py b/tools/release.py index 3e38a2731d0823cd289255206678337c6de62ff3..13c3dcb34155bb12411603a652327fd3e1dc480a 100755 --- a/tools/release.py +++ b/tools/release.py @@ -16,6 +16,8 @@ import time import zipfile from zoneinfo import ZoneInfo +from cargo_fresh import clean_local + ROOT = Path(__file__).resolve().parents[1] PUBLISHED = Path('/Volumes/clover/Documents/Public/Snowbound') URL = 'https://file.paperclover.net/shr/snowbound/' @@ -231,6 +233,7 @@ def build_windows(architectures): Windows it supports.""" built = {} for arch in architectures: + clean_local(ROOT, '--release', command=['sh', ROOT / 'platform/windows/cargo.sh', arch]) run(['sh', ROOT / 'platform/windows/cargo.sh', arch, 'build', '--release', '-p', 'snowbound']) built[f'windows-{arch}'] = ROOT / f'target/windows/{WINDOWS[arch]}/release/snowbound.exe' return built @@ -314,6 +317,7 @@ def main(): symbols.append(debug.with_name(f'{debug.name}.zip')) with zipfile.ZipFile(symbols[-1], 'w', zipfile.ZIP_DEFLATED) as archive: archive.write(debug, debug.name) + clean_local(ROOT, '--release') signatures = sign(files.values()) # A dry run's changes too start after the newest build the share holds. before = [each for each in builds(PUBLISHED) if each < version] if PUBLISHED.is_dir() else [] diff --git a/tools/release_relay.py b/tools/release_relay.py index e734e11f8c4ec5c64ddb520576ca08673a6c990b..bfce39bd99d6a8fde4ef794bbc99fc9a6b07b64b 100755 --- a/tools/release_relay.py +++ b/tools/release_relay.py @@ -10,6 +10,8 @@ from pathlib import Path import shutil import subprocess +from cargo_fresh import clean_local + ROOT = Path(__file__).resolve().parents[1] ARCHITECTURES = ['x86_64', 'aarch64'] @@ -21,6 +23,7 @@ def build(arch, output): subprocess.run(['rustup', 'target', 'add', triple], check=True, capture_output=True) environment = {**os.environ, f'CARGO_TARGET_{triple.upper().replace("-", "_")}_LINKER': 'rust-lld', 'CARGO_PROFILE_RELEASE_STRIP': 'symbols'} + clean_local(ROOT, '--release', '--target', triple, environment=environment) subprocess.run(['cargo', 'build', '--locked', '--release', '-p', 'relay', '--target', triple], cwd=ROOT, env=environment, check=True) target = Path(os.environ.get('CARGO_TARGET_DIR') or ROOT / 'target') diff --git a/tools/release_web.py b/tools/release_web.py index dd36dd7385b3dee3e0b08e1cbaa0a9b6b837cc0e..8463a614bc09384dc0841da32518e03e9119b432 100644 --- a/tools/release_web.py +++ b/tools/release_web.py @@ -27,6 +27,8 @@ import tempfile import time import urllib.request +from cargo_fresh import clean_local + ROOT = Path(__file__).resolve().parents[1] # The one target folder wasm builds share, apart from the native builds in target/. TARGET = ROOT / 'target/wasm' @@ -100,8 +102,10 @@ def build(out): """Writes index.html, the module, its JavaScript and the fonts to `out`, the module knowing when it was built.""" built = str(int(time.time())) + build_environment = environment() + clean_local(ROOT, '--release', '--target', 'wasm32-unknown-unknown', environment=build_environment) run(['cargo', 'build', '--locked', '-p', 'snowbound', '--release', '--target', 'wasm32-unknown-unknown', '--no-default-features', '--features', 'wgpu,live', - *PROFILE], env={**environment(), 'SNOWBOUND_WEB_BUILD': built}) + *PROFILE], env={**build_environment, 'SNOWBOUND_WEB_BUILD': built}) if out.exists(): shutil.rmtree(out) out.mkdir(parents=True) diff --git a/tools/test_cargo_fresh.py b/tools/test_cargo_fresh.py new file mode 100644 index 0000000000000000000000000000000000000000..a56c1513be45b61c8f61ebc0e4c50021b58c27ae --- /dev/null +++ b/tools/test_cargo_fresh.py @@ -0,0 +1,110 @@ +import os +from pathlib import Path +import shutil +import subprocess +import tempfile +import unittest +from unittest.mock import patch + +import cargo_fresh +import ci +import release_web + + +@unittest.skipUnless(shutil.which('cargo'), 'needs Cargo') +class CargoFreshTest(unittest.TestCase): + def test_old_checkout_inputs_are_rebuilt_without_cleaning_dependencies(self): + with tempfile.TemporaryDirectory(prefix='snowbound-cargo-fresh-') as folder: + folder = Path(folder) + for name, value in [('a', 1), ('b', 2)]: + root = folder / name + (root / 'app/src').mkdir(parents=True) + (root / 'owned/src').mkdir(parents=True) + (root / 'vendor/src').mkdir(parents=True) + (root / 'vendor/Cargo.toml').write_text( + '[package]\nname="shared_dependency"\nversion="0.1.0"\nedition="2024"\n') + (root / 'vendor/src/lib.rs').write_text(f'pub fn value() -> u8 {{ {value * 10} }}\n') + (root / 'Cargo.toml').write_text( + '[workspace]\nmembers=["app", "owned"]\nexclude=["vendor"]\nresolver="2"\n') + (root / 'app/Cargo.toml').write_text( + '[package]\nname="freshness_fixture"\nversion="0.1.0"\nedition="2024"\n' + '[dependencies]\nowned={path="../owned"}\nshared_dependency={path="../vendor"}\nitoa="=1.0.18"\n') + (root / 'owned/Cargo.toml').write_text( + '[package]\nname="owned"\nversion="0.1.0"\nedition="2024"\n') + (root / 'owned/src/lib.rs').write_text('include!(concat!(env!("OUT_DIR"), "/value.rs"));\n') + (root / 'owned/value.txt').write_text(str(value)) + (root / 'owned/build.rs').write_text( + 'fn main() {\n' + ' println!("cargo::rerun-if-changed=value.txt");\n' + ' let value = std::fs::read_to_string("value.txt").unwrap();\n' + ' let out = std::path::PathBuf::from(std::env::var_os("OUT_DIR").unwrap());\n' + ' std::fs::write(out.join("value.rs"), format!("pub fn value() -> u8 {{ {value} }}")).unwrap();\n' + '}\n') + (root / 'app/src/lib.rs').write_text( + f'#[test] fn checkout_{name}() {{ ' + f'assert_eq!(itoa::Buffer::new().format(owned::value() + shared_dependency::value()), "{value * 11}"); }}\n') + for path in root.rglob('*'): + if path.is_file(): + os.utime(path, (1, 1)) + environment = {key: value for key, value in os.environ.items() + if not key.startswith(('CARGO_', 'RUST', 'SNOWBOUND_', 'ONESTORE_'))} + environment.update(CARGO_TARGET_DIR=str(folder / 'cache'), CARGO_BUILD_JOBS='2', CARGO_NET_OFFLINE='true') + + def cargo(root, *args): + return subprocess.run(['cargo', *args], cwd=root, env=environment, check=True, + capture_output=True, text=True).stdout + + self.assertIn('test checkout_a ... ok', cargo(folder / 'a', 'test', '--offline')) + artifacts = {path: (path.stat().st_mtime_ns, path.read_bytes()) + for path in (folder / 'cache/debug/deps').glob('*itoa*')} + self.assertTrue(artifacts) + cargo(folder / 'b', 'generate-lockfile', '--offline') + host = subprocess.check_output(['rustc', '-vV'], env=environment, text=True) + host = next(line.removeprefix('host: ') for line in host.splitlines() if line.startswith('host: ')) + cargo(folder / 'a', 'build', '--offline', '--release') + cargo(folder / 'a', 'build', '--offline', '--release', '--target', host) + previous = {path: (path.stat().st_mtime_ns, path.read_bytes()) + for path in (folder / 'cache/release/deps').glob('*.rlib')} + self.assertTrue(previous) + cargo_fresh.clean_local(folder / 'b', '--release', '--target', host, environment=environment) + cargo(folder / 'b', 'build', '--offline', '--release', '--target', host) + for path, expected in previous.items(): + self.assertEqual((path.stat().st_mtime_ns, path.read_bytes()), expected) + cargo_fresh.clean_local(folder / 'b', environment=environment) + self.assertIn('test checkout_b ... ok', + cargo(folder / 'b', 'test', '--offline', '--', '--exact', 'checkout_b')) + for path, expected in artifacts.items(): + self.assertEqual((path.stat().st_mtime_ns, path.read_bytes()), expected) + + def test_clean_failure_stops_web_build_before_packaging(self): + with patch.object(release_web, 'environment', return_value={}), \ + patch.object(release_web, 'clean_local', side_effect=subprocess.CalledProcessError(1, 'cargo')), \ + patch.object(release_web, 'run') as run: + with self.assertRaises(subprocess.CalledProcessError): + release_web.build(Path('unused-output')) + run.assert_not_called() + + def test_empty_metadata_never_cleans_the_entire_cache(self): + with patch.object(cargo_fresh.subprocess, 'check_output', return_value='{"packages": []}'), \ + patch.object(cargo_fresh.subprocess, 'run') as run: + with self.assertRaisesRegex(ValueError, 'no local packages'): + cargo_fresh.clean_local(Path('unused-checkout')) + run.assert_not_called() + + def test_clean_failure_stops_ci_before_concurrent_lanes(self): + with tempfile.TemporaryDirectory(prefix='snowbound-ci-clean-') as folder, \ + patch.object(ci, 'RUNS', Path(folder)), \ + patch.object(ci, 'lanes', return_value=[{'name': 'test'}]), \ + patch.object(ci, 'checkout', return_value='commit'), \ + patch.object(ci, 'jj', return_value='fixture'), \ + patch.object(ci, 'local_packages', return_value=['fixture']), \ + patch.object(ci.sys, 'argv', ['ci.py']), \ + patch.object(ci.Run, 'execute', return_value=101), \ + patch.object(ci, 'ThreadPoolExecutor') as pool: + with self.assertRaisesRegex(SystemExit, 'Workspace cleanup failed'): + ci.main() + pool.assert_not_called() + + +if __name__ == '__main__': + unittest.main() diff --git a/tools/test_release.py b/tools/test_release.py index 248a615e7f0f9801bb649980e22afb6bcf53c808..bdf26fd375e3acd5b2e00eb85fab594699698d93 100644 --- a/tools/test_release.py +++ b/tools/test_release.py @@ -85,7 +85,7 @@ class ReleaseTest(unittest.TestCase): overrides = { 'ROOT': root, 'PUBLISHED': published, 'FIRST': 'a', - 'jj': Mock(return_value='a'), 'clean': Mock(), 'run': Mock(), + 'jj': Mock(return_value='a'), 'clean': Mock(), 'clean_local': Mock(), 'run': Mock(), 'history': lambda: {'a': ([], utc('2026-10-05T12:00:00'), 'fix: publish')}, 'build_linux': lambda architectures: {'linux-x86_64': source}, 'split_debug': split_debug, 'sign': lambda files: ['00' for _ in files], -- 2.54.0