From 965d7d3748976bff66eb1807e975296936ae14a5 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 11:17:43 +0200 Subject: [PATCH 01/20] ci: the host cache is read by content, sealed only by a cold run, and pruned to one entry; the licence step fetches only the fork's library/ Freshness. Cargo calls a path crate fresh when no source is newer than the build that read it, and actions/checkout dates every source at the checkout, so every path crate recompiled in every host run (152 compile events, 472 s median of a PR run's 817 s). `-Z checksum-freshness` would fix it inside cargo, and is refused here: it is still unstable (nightly-2026-09-21 lists it under -Z, stable 1.98.1 rejects -Z), and the only ways to it are a nightly toolchain or RUSTC_BOOTSTRAP=1 on stable. Either one moves the gate's verdict off what a user's stable compiles: RUSTC_BOOTSTRAP lifts the feature gate for every crate and flips the nightly probes in dependencies' build scripts, and cargo passes -Zchecksum-hash-algorithm to rustc, so rustc needs it too. The fork builds no cargo of its own (toolchain.rs lends the machine's). An mtime made up from history (a commit's time) would call a changed file fresh whenever its commit predates the entry's build. Instead the entry carries the git blob id of every source its build read (target/ci-sources), and the writer dates every file under every target 2001-09-09. A reader dates each source whose blob matches the same, and every other one now. Cargo's comparison is `source <= reference` is fresh, so a match is fresh and a change or an addition is newer than everything in the entry, whatever any runner's clock says. A package that lost a file keeps its Cargo.toml dated now, because cargo's package fingerprint (a build script that names no input) is the newest of the remaining files. Only a cold run seals: a warm run's targets hold units its steps never rebuilt, compiled from sources no manifest it could write describes. A reader deletes the manifest it read, so a warm tree is never saved, and targets restored without a manifest are refused. The driver builds in target/ci-driver: cargo compiles it before any of this runs, so its path crates recompile every time, and in the steps' target that rebuild would make every dependent stale. Cache discipline. nightly.yml's host is the writer on main and restores nothing, so the entry is one cold build's tree and never an accumulation (4.25 GB from 2.65 GB in one write). It then deletes every other host entry, and reds if its own is not on main. A pull request that found no entry seals and saves its own, which only its later runs read. The key carries the runner's OS. Licence step. The fork's std names toyos and toyos-abi by path from rust/, so the library stays in rust/; a runner fetches the pinned commit's library/ alone (blobless, sparse) instead of the whole tree, and a developer's uninitialised rust/ is refused rather than left sparse for a later build. --- .github/workflows/ci.yml | 22 +- .github/workflows/nightly.yml | 30 +- src/ci.rs | 58 +++- src/cicache.rs | 540 ++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + src/licence.rs | 34 ++- 6 files changed, 647 insertions(+), 38 deletions(-) create mode 100644 src/cicache.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4f348c1b995..96ef0005e85 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,11 +24,13 @@ jobs: steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - # nightly.yml's `host` is the one writer; the paths are the cache's - # version, so they are the writer's list. + # An entry is read by content (src/cicache.rs): main's, which + # nightly.yml's `host` writes, or one this pull request saved when it + # found none. The paths are the cache's version, so they are the + # writer's list. - uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 with: - path: | + path: &host-paths | ~/.cargo/registry/index ~/.cargo/registry/cache ~/.cargo/git/db @@ -37,7 +39,15 @@ jobs: toyos/target kernel/target bootloader/target - key: host-${{ github.run_id }} - restore-keys: host- + key: host-sealed-${{ runner.os }}-${{ github.run_id }} + restore-keys: host-sealed-${{ runner.os }}- - - run: cargo run -- --ci host + - run: cargo run --target-dir target/ci-driver -- --ci host + + # Only a run that restored nothing leaves a manifest, and what a pull + # request saves only its own later runs read. + - if: github.event_name == 'pull_request' && hashFiles('target/ci-sources') != '' + uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + with: + path: *host-paths + key: host-sealed-${{ runner.os }}-${{ github.run_id }} diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 36d2aa93359..15af206a8c4 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -16,17 +16,28 @@ concurrency: cancel-in-progress: false jobs: + # The host cache's writer: only main's entries are readable from every + # branch, and it restores nothing, so its tree is one cold build's and + # src/cicache.rs seals it. On a branch it would only repeat that branch's + # pull request. host: + if: github.ref == 'refs/heads/main' runs-on: macos-latest timeout-minutes: 90 + permissions: + actions: write + contents: read steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 with: fetch-depth: 0 - - uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + - run: cargo run --target-dir target/ci-driver -- --ci host + + - if: hashFiles('target/ci-sources') != '' + uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 with: - path: &host-paths | + path: | ~/.cargo/registry/index ~/.cargo/registry/cache ~/.cargo/git/db @@ -35,18 +46,11 @@ jobs: toyos/target kernel/target bootloader/target - key: host-${{ github.run_id }} - restore-keys: host- + key: host-sealed-${{ runner.os }}-${{ github.run_id }} - - run: cargo run -- --ci host - - # The host cache's one writer. Only main's entries are readable from - # every branch. - - if: github.ref == 'refs/heads/main' - uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 - with: - path: *host-paths - key: host-${{ github.run_id }} + - env: + GH_TOKEN: ${{ github.token }} + run: cargo run --target-dir target/ci-driver -- --ci prune # Publishes this tree's toolchain if nobody has, and on main moves the SDK # alias onto it. Bare `ubuntu-24.04`, not a container: its glibc is the diff --git a/src/ci.rs b/src/ci.rs index 5323f09f130..bae90d82943 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -29,6 +29,7 @@ use std::path::{Path, PathBuf}; use std::process::Command; use crate::arch::Arch; +use crate::cicache::{self, Start}; use crate::{flags, release, sdkversion, sync}; pub const GUEST_ARCH: Arch = Arch::X86_64; @@ -37,6 +38,8 @@ const USAGE: &str = "cargo run -- --ci , where is one of: host every host test: the build system, the harness's own checks, the host workspace, the licences of what ships, clippy, the model controls, userland and the SDK (ci.yml, nightly) + prune delete every host cache entry but the one this run saved + on main (nightly) toolchain publish this tree's toolchain if nobody has (nightly) guest / one shard of the guest suite (nightly) tcg one test on an emulated CPU (nightly) @@ -45,6 +48,7 @@ const USAGE: &str = "cargo run -- --ci , where is one of: #[derive(Debug, PartialEq, Eq)] enum Job { Host, + Prune, Toolchain, Guest(String), Tcg, @@ -59,6 +63,7 @@ fn parse(words: &[String]) -> Result { }; let job = match words.first().map(String::as_str) { Some("host") => Job::Host, + Some("prune") => Job::Prune, Some("toolchain") => Job::Toolchain, Some("guest") => Job::Guest(shard(words.get(1))?), Some("tcg") => Job::Tcg, @@ -80,6 +85,7 @@ pub fn dispatch(root: &Path, args: &[String]) { }); let steps = match &job { Job::Host => host(root), + Job::Prune => vec![step("the host cache's other entries", || cicache::prune(root))], Job::Toolchain => vec![step("the toolchain release", || release::ensure_published(root))], Job::Guest(shard) => guest(root, &suite_args(&["--shard", shard, "--jobs", "1"])), Job::Tcg => guest(root, &suite_args(&["--jobs", "1", "empty_dir_stat"])), @@ -123,7 +129,7 @@ fn step(label: &str, f: impl FnOnce() -> Result) -> Step { Step { label: label.to_string(), verdict } } -fn on_runner() -> bool { +pub(crate) fn on_runner() -> bool { std::env::var("GITHUB_ACTIONS").is_ok_and(|v| v == "true") } @@ -421,6 +427,10 @@ fn run_control(root: &Path, control: &Control) -> Result { /// ([`crate::clippy::BARE_TARGETS`]), which any rustup installs, and userland carries no /// clippy shape (`src/clippy.rs`). Userland and the SDK are tested against the /// host triple for the same reason. +/// +/// On a runner the restored cache entry is read before any step and a run that +/// restored none seals its tree after the last ([`cicache`]); a developer's +/// tree keeps the dates its edits gave it. fn host(root: &Path) -> Vec { let tmp = toyos_tmpdir::TempDir::new("ci-host"); let short = Path::new(toyos_tmpdir::SHORT_BASE); @@ -429,14 +439,30 @@ fn host(root: &Path) -> Vec { // concurrently with the write, and every child inherits it. std::env::set_var("TMPDIR", tmp.path()); let host_triple = crate::toolchain::host_triple(); - let mut steps = vec![ + let mut steps = Vec::new(); + let mut cold = None; + if on_runner() { + let mut start = None; + steps.push(step("the cache entry, read by content", || { + let (found, said) = cicache::read(root)?; + start = Some(found); + Ok(said) + })); + match start { + // Every step after an unreadable entry would be judged against it. + None => return steps, + Some(Start::Cold(stamped)) => cold = Some(stamped), + Some(Start::Warm) => {} + } + } + steps.extend([ step("the build system", || cargo(root, &["test", "--lib"])), step("the harness's own checks", || cargo(root, &["test", "--test", "toyos-checks"])), step("the host workspace", || { cargo(root, &["test", "--workspace", "--exclude", "toyos-build"]) }), step("the licences of what ships", || crate::licence::judge(root)), - ]; + ]); steps.push(step("clippy and the bare targets", || { for args in [ vec!["component", "add", "clippy"], @@ -491,6 +517,9 @@ fn host(root: &Path) -> Vec { cargo(root, &["test", "--manifest-path", "toyos/Cargo.toml", "--target", &host_triple]) })); steps.push(step("nothing left in $TMPDIR or /tmp", || left_behind(&tmp, short, &before))); + if let Some(stamped) = cold { + steps.push(step("the tree, sealed as a cache entry", || cicache::seal(root, &stamped))); + } steps } @@ -807,6 +836,7 @@ mod tests { #[test] fn a_job_is_named_and_a_shard_is_a_shard() { assert_eq!(parse(&words("host")), Ok(Job::Host)); + assert_eq!(parse(&words("prune")), Ok(Job::Prune)); assert_eq!(parse(&words("guest 3/12")), Ok(Job::Guest("3/12".into()))); assert!(parse(&words("guest")).is_err()); assert!(parse(&words("guest 13/12")).is_err()); @@ -889,12 +919,15 @@ mod tests { assert_eq!(seen, 3, "ci.yml, nightly.yml and publish.yml"); } - /// Exactly one job writes each cache, on the nightly, so what a pull request - /// restores is one run's tree and never a race between two writers. + /// A host entry is saved only beside its manifest, which only a sealed tree + /// holds ([`cicache`]); every other cache has exactly one writer, on the + /// nightly, so what a run restores is one run's tree and never a race + /// between two writers. #[test] - fn each_cache_has_one_writer() { + fn a_cache_is_saved_only_where_a_reader_can_trust_it() { let dir = repo_root().join(".github/workflows"); let mut writers = Vec::new(); + let mut host = 0; for entry in std::fs::read_dir(&dir).expect(".github/workflows is readable").flatten() { let text = std::fs::read_to_string(entry.path()).expect("a readable workflow"); let name = entry.file_name().to_string_lossy().into_owned(); @@ -906,11 +939,20 @@ mod tests { .iter() .find_map(|l| l.trim_start().strip_prefix("key: ")) .expect("a save names its key"); - writers.push((name.clone(), key.split('$').next().unwrap_or("").to_string())); + let prefix = key.split('$').next().unwrap_or("").to_string(); + if !prefix.starts_with("host-") { + writers.push((name.clone(), prefix)); + continue; + } + host += 1; + assert_eq!(prefix, cicache::SEALED, "{name}"); + let manifest = format!("hashFiles('{}') != ''", cicache::MANIFEST); + assert!(lines[at - 1].contains(&manifest), "{name}: a host save without `{manifest}`"); } } } - assert!(!writers.is_empty(), "no job writes a cache, so every restore is cold"); + assert!(host > 0, "no job writes the host cache, so every host run is cold"); + assert!(!writers.is_empty(), "no job writes the guest cache, so every guest run is cold"); writers.sort(); let mut prefixes: Vec<&String> = writers.iter().map(|(_, p)| p).collect(); prefixes.dedup(); diff --git a/src/cicache.rs b/src/cicache.rs new file mode 100644 index 00000000000..369c73bf17a --- /dev/null +++ b/src/cicache.rs @@ -0,0 +1,540 @@ +//! The host job's cache entry is trusted by content, never by mtime. +//! +//! Cargo calls a path crate fresh when none of its sources is newer than the +//! build that read it, and a checkout dates every source at the checkout: a +//! restored target is stale in every path crate, and a date made up for a +//! source from anything but its bytes could as well call a changed one fresh. +//! An entry instead carries [`MANIFEST`], the git blob id of every source its +//! build read, and every file under its targets is dated [`built`], before any +//! real time. [`read`] dates each source whose blob matches the same, and every +//! other one now: cargo's comparison is strict, so a matching source is no +//! newer than the build, and a changed or new one is newer than everything in +//! the entry, whatever any runner's clock says. +//! +//! **Only a run that restored nothing seals an entry** ([`Start::Cold`], +//! [`seal`]): a warm run's targets hold units none of its steps rebuilt, +//! compiled from sources no manifest it could write describes. [`read`] deletes +//! the manifest it reads, so a warm tree carries none and is never saved, and +//! targets restored without one are refused. +//! +//! **The driver is built in [`DRIVER`]**: cargo builds it before this runs, so +//! its path crates are compiled again every time, and in the steps' target that +//! would make every crate depending on them stale. +//! +//! A package that lost a file keeps its `Cargo.toml` dated now: cargo's package +//! fingerprint, which decides a build script that names no input, is the +//! newest of the files that remain. + +use std::collections::{BTreeMap, BTreeSet}; +use std::fs; +use std::io::{ErrorKind, Write}; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; + +pub const MANIFEST: &str = "target/ci-sources"; +pub const DRIVER: &str = "target/ci-driver"; +/// Every host entry's key starts with this; [`prune`] lists by it. +const HOST: &str = "host-"; +/// A sealed entry's key is this, `${{ runner.os }}-${{ github.run_id }}`. +pub const SEALED: &str = "host-sealed-"; + +/// 2001-09-09T01:46:40Z: older than any build, so a file dated so is never +/// newer than one. +fn built() -> SystemTime { + UNIX_EPOCH + Duration::from_secs(1_000_000_000) +} + +/// Every source as git tracks it, with the blob id of its bytes on disk. +type Sources = BTreeMap; + +/// What a job found before its first step. +pub enum Start { + /// An entry, read and its manifest consumed. + Warm, + /// No entry: every source is dated [`built`], and these are what [`seal`] + /// holds the tree to at the end. + Cold(Sources), +} + +/// Before the first step: the entry restored here, read by content. +pub fn read(root: &Path) -> Result<(Start, String), String> { + let exe = std::env::current_exe() + .and_then(fs::canonicalize) + .map_err(|e| format!("the driver's own path: {e}"))?; + let driver = fs::canonicalize(root.join(DRIVER)).map_err(|e| format!("{DRIVER}: {e}"))?; + if !exe.starts_with(&driver) { + return Err(format!( + "the driver runs from {}: a workflow builds it with `cargo run --target-dir {DRIVER} \ + -- --ci `", + exe.display() + )); + } + open(root) +} + +fn open(root: &Path) -> Result<(Start, String), String> { + let current = sources(root)?; + let manifest = root.join(MANIFEST); + let text = match fs::read_to_string(&manifest) { + Ok(text) => text, + Err(e) if e.kind() == ErrorKind::NotFound => { + let restored = restored(root)?; + if !restored.is_empty() { + return Err(format!( + "{} restored without {MANIFEST}, so nothing says what they were built from", + restored.iter().map(|p| p.display().to_string()).collect::>().join(", ") + )); + } + for path in current.keys() { + date(&root.join(path), built())?; + } + let said = format!( + "none restored; {} sources dated as built, and the tree is sealed last", + current.len() + ); + return Ok((Start::Cold(current), said)); + } + Err(e) => return Err(format!("read {MANIFEST}: {e}")), + }; + fs::remove_file(&manifest).map_err(|e| format!("remove {MANIFEST}: {e}"))?; + let (commit, entry) = parse(&text)?; + let lost = lost(&entry, ¤t); + let now = SystemTime::now(); + let mut same = 0; + for (path, blob) in ¤t { + let fresh = entry.get(path) == Some(blob) && !lost.contains(path); + same += usize::from(fresh); + date(&root.join(path), if fresh { built() } else { now })?; + } + let changed = current.iter().filter(|(p, b)| entry.get(*p).is_some_and(|e| e != *b)).count(); + let added = current.keys().filter(|p| !entry.contains_key(*p)).count(); + let removed = entry.keys().filter(|p| !current.contains_key(*p)).count(); + Ok(( + Start::Warm, + format!( + "built from {commit}: {same} of {} sources unchanged, {changed} changed, {added} added, \ + {removed} removed", + current.len() + ), + )) +} + +/// After the last step of a run that started [`Start::Cold`]: every file under +/// every target dated [`built`], then the manifest. +pub fn seal(root: &Path, stamped: &Sources) -> Result { + let now = sources(root)?; + let moved: Vec<&String> = stamped + .iter() + .filter(|(path, blob)| { + now.get(*path) != Some(*blob) || modified(&root.join(path)).ok() != Some(built()) + }) + .map(|(path, _)| path) + .chain(now.keys().filter(|path| !stamped.contains_key(*path))) + .collect(); + if !moved.is_empty() { + return Err(format!("a step wrote tracked sources, which no entry can describe: {moved:?}")); + } + let (mut files, mut bytes) = (0u64, 0u64); + for target in targets(root)? { + age(&target, &mut files, &mut bytes)?; + } + let head = crate::sync::git(root, &["rev-parse", "HEAD"])?; + let mut text = format!("{head}\n"); + for (path, blob) in stamped { + text.push_str(&format!("{blob} {path}\n")); + } + fs::write(root.join(MANIFEST), text).map_err(|e| format!("write {MANIFEST}: {e}"))?; + Ok(format!("{} sources; {files} files, {} MiB, dated as built", stamped.len(), bytes >> 20)) +} + +/// Every host entry but the one this run saved, which must be on `main`: +/// pull requests read that one, and every other only fills the repository's +/// cache budget. +pub fn prune(root: &Path) -> Result { + let var = + |name: &str| std::env::var(name).map_err(|_| format!("{name} is unset: a runner prunes")); + let (repo, os, run) = (var("GITHUB_REPOSITORY")?, var("RUNNER_OS")?, var("GITHUB_RUN_ID")?); + let ours = format!("{SEALED}{os}-{run}"); + let caches = format!("repos/{repo}/actions/caches"); + let listing = + gh(root, &["api", "-X", "GET", &caches, "-f", &format!("key={HOST}"), "-F", "per_page=100"])?; + let doc: serde_json::Value = + serde_json::from_str(&listing).map_err(|e| format!("the cache listing is not JSON: {e}"))?; + let doomed = doomed(&doc, &ours)?; + for (id, _) in &doomed { + gh(root, &["api", "-X", "DELETE", &format!("{caches}/{id}")])?; + } + let keys: Vec<&str> = doomed.iter().map(|(_, key)| key.as_str()).collect(); + Ok(format!("kept {ours}; deleted {}: {}", keys.len(), keys.join(", "))) +} + +/// Every entry in the listing but `ours`, which must be in it on `main`. +fn doomed(doc: &serde_json::Value, ours: &str) -> Result, String> { + let entries = doc["actions_caches"].as_array().ok_or("the listing holds no actions_caches")?; + let total = doc["total_count"].as_u64().ok_or("the listing holds no total_count")?; + if total != entries.len() as u64 { + return Err(format!( + "{total} host entries and a page of {}: pruning a page is no bound", + entries.len() + )); + } + let mut kept = false; + let mut doomed = Vec::new(); + for entry in entries { + let (Some(id), Some(key), Some(scope)) = + (entry["id"].as_u64(), entry["key"].as_str(), entry["ref"].as_str()) + else { + return Err(format!("an entry without an id, key or ref: {entry}")); + }; + if key == ours && scope == "refs/heads/main" { + kept = true; + } else { + doomed.push((id, key.to_string())); + } + } + if !kept { + return Err(format!( + "{ours} is not among main's entries: the save before this wrote none, and pruning would \ + leave pull requests nothing to read" + )); + } + Ok(doomed) +} + +fn gh(root: &Path, args: &[&str]) -> Result { + let out = Command::new("gh") + .args(args) + .current_dir(root) + .output() + .map_err(|e| format!("gh: {e}"))?; + if !out.status.success() { + let said = String::from_utf8_lossy(&out.stderr); + return Err(format!("gh {} exited {}: {}", args.join(" "), out.status, said.trim())); + } + Ok(String::from_utf8_lossy(&out.stdout).into_owned()) +} + +fn parse(text: &str) -> Result<(String, Sources), String> { + let mut lines = text.lines(); + let commit = lines.next().ok_or_else(|| format!("{MANIFEST} is empty"))?.to_string(); + let entry = lines + .map(|line| { + line.split_once(' ') + .filter(|(blob, _)| !blob.is_empty() && blob.bytes().all(|b| b.is_ascii_hexdigit())) + .map(|(blob, path)| (path.to_string(), blob.to_string())) + .ok_or_else(|| format!("{MANIFEST} holds {line:?}")) + }) + .collect::>()?; + Ok((commit, entry)) +} + +/// The `Cargo.toml` of every package that lost a source since `entry`. +fn lost(entry: &Sources, current: &Sources) -> BTreeSet { + let mut lost = BTreeSet::new(); + for gone in entry.keys().filter(|p| !current.contains_key(*p)) { + let manifest = Path::new(gone) + .ancestors() + .skip(1) + .map(|dir| dir.join("Cargo.toml").to_string_lossy().into_owned()) + .find(|manifest| current.contains_key(manifest)); + lost.extend(manifest); + } + lost +} + +fn sources(root: &Path) -> Result { + let out = Command::new("git") + .args(["ls-files", "-s", "-z"]) + .current_dir(root) + .output() + .map_err(|e| format!("git ls-files: {e}"))?; + if !out.status.success() { + return Err(format!("git ls-files: {}", String::from_utf8_lossy(&out.stderr).trim())); + } + let mut paths = Vec::new(); + for entry in out.stdout.split(|b| *b == 0).filter(|e| !e.is_empty()) { + let entry = std::str::from_utf8(entry).map_err(|_| "git tracks a name that is not UTF-8")?; + let (meta, path) = + entry.split_once('\t').ok_or_else(|| format!("git ls-files -s printed {entry:?}"))?; + // A gitlink names a commit, not bytes: what is under it keeps its + // checkout's date, which no entry is newer than. + if meta.starts_with("160000 ") { + continue; + } + if path.contains('\n') { + return Err(format!("git tracks {path:?}, which `--stdin-paths` cannot be given")); + } + paths.push(path.to_string()); + } + let mut child = Command::new("git") + .args(["hash-object", "--no-filters", "--stdin-paths"]) + .current_dir(root) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .map_err(|e| format!("git hash-object: {e}"))?; + let mut stdin = child.stdin.take().expect("piped"); + let input: String = paths.iter().map(|p| format!("{p}\n")).collect(); + // Written beside the read: the ids fill the pipe before the paths are in. + let writer = std::thread::spawn(move || stdin.write_all(input.as_bytes())); + let out = child.wait_with_output().map_err(|e| format!("git hash-object: {e}"))?; + writer.join().expect("the writer").map_err(|e| format!("git hash-object's input: {e}"))?; + if !out.status.success() { + return Err(format!("git hash-object: {}", String::from_utf8_lossy(&out.stderr).trim())); + } + let blobs: Vec = String::from_utf8_lossy(&out.stdout).lines().map(String::from).collect(); + if blobs.len() != paths.len() { + return Err(format!("git hash-object gave {} ids for {} paths", blobs.len(), paths.len())); + } + Ok(paths.into_iter().zip(blobs).collect()) +} + +/// Every `target` directory under `root`, none inside another. +fn targets(root: &Path) -> Result, String> { + fn walk(dir: &Path, found: &mut Vec) -> Result<(), String> { + for entry in fs::read_dir(dir).map_err(|e| format!("read {}: {e}", dir.display()))? { + let entry = entry.map_err(|e| format!("read {}: {e}", dir.display()))?; + let kind = entry.file_type().map_err(|e| format!("{}: {e}", entry.path().display()))?; + if !kind.is_dir() || entry.file_name() == ".git" { + continue; + } + if entry.file_name() == "target" { + found.push(entry.path()); + } else { + walk(&entry.path(), found)?; + } + } + Ok(()) + } + let mut found = Vec::new(); + walk(root, &mut found)?; + Ok(found) +} + +/// What a restore put here: every target but the root's, and in the root's +/// everything but [`DRIVER`], which this job's own `cargo run` made. +fn restored(root: &Path) -> Result, String> { + let ours = root.join("target"); + let mut found = Vec::new(); + for target in targets(root)? { + if target != ours { + found.push(target); + continue; + } + for entry in fs::read_dir(&target).map_err(|e| format!("read {}: {e}", target.display()))? { + let path = entry.map_err(|e| format!("read {}: {e}", target.display()))?.path(); + if path != root.join(DRIVER) { + found.push(path); + } + } + } + Ok(found) +} + +/// Date everything under `dir`, and `dir`, as [`built`]. A symbolic link is +/// left alone: dating one dates whatever it names. +fn age(dir: &Path, files: &mut u64, bytes: &mut u64) -> Result<(), String> { + for entry in fs::read_dir(dir).map_err(|e| format!("read {}: {e}", dir.display()))? { + let entry = entry.map_err(|e| format!("read {}: {e}", dir.display()))?; + let meta = entry.metadata().map_err(|e| format!("{}: {e}", entry.path().display()))?; + if meta.is_dir() { + age(&entry.path(), files, bytes)?; + } else if meta.is_file() { + date(&entry.path(), built())?; + *files += 1; + *bytes += meta.len(); + } + } + date(dir, built()) +} + +fn date(path: &Path, when: SystemTime) -> Result<(), String> { + fs::File::open(path) + .and_then(|f| f.set_modified(when)) + .map_err(|e| format!("date {}: {e}", path.display())) +} + +fn modified(path: &Path) -> std::io::Result { + fs::metadata(path)?.modified() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::gitfixture::{configure, sh}; + use toyos_tmpdir::TempDir; + + fn write(root: &Path, path: &str, text: &str) { + let path = root.join(path); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, text).unwrap(); + } + + /// `cargo build` in `root`, and each workspace crate with whether cargo + /// called it fresh. + fn build(root: &Path) -> BTreeMap { + let out = Command::new("cargo") + .args(["build", "--offline", "--message-format=json"]) + .current_dir(root) + .env_remove("CARGO_TARGET_DIR") + .output() + .expect("run cargo"); + assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr)); + let mut fresh = BTreeMap::new(); + for line in String::from_utf8_lossy(&out.stdout).lines() { + let doc: serde_json::Value = serde_json::from_str(line).unwrap(); + if doc["reason"] == "compiler-artifact" && doc["target"]["kind"][0] != "custom-build" { + let name = doc["target"]["name"].as_str().unwrap().to_string(); + fresh.insert(name, doc["fresh"].as_bool().unwrap()); + } + } + fresh + } + + fn run(root: &Path) -> String { + let out = Command::new(root.join("target/debug/app")).output().unwrap(); + String::from_utf8(out.stdout).unwrap() + } + + /// A checkout of `origin` at its head, as a runner makes one. + fn checkout(origin: &Path, dir: &Path) { + let (from, to) = (origin.to_str().unwrap(), dir.to_str().unwrap()); + sh(origin.parent().unwrap(), &["clone", "-q", from, to]); + configure(dir); + } + + /// The oracle is cargo and the program it builds: an entry serves what + /// a cold build of the reader's tree would, recompiling exactly what + /// changed and what depends on it — a changed source the checkout dated + /// older than the entry's build included, and a build script whose package + /// lost a file it never named. + #[test] + fn an_entry_serves_exactly_the_sources_it_was_built_from() { + let tmp = TempDir::new("cicache-entry"); + let origin = tmp.join("origin"); + fs::create_dir(&origin).unwrap(); + sh(&origin, &["init", "-q", "-b", "main"]); + configure(&origin); + write(&origin, ".gitignore", "target/\n"); + let members = "[workspace]\nmembers = [\"app\", \"leaf\", \"count\", \"idle\"]\nresolver = \"2\"\n"; + write(&origin, "Cargo.toml", members); + let package = |name: &str, deps: &str| { + let head = format!("[package]\nname = \"{name}\"\nversion = \"0.1.0\"\nedition = \"2021\"\n"); + format!("{head}\n[dependencies]\n{deps}") + }; + write(&origin, "leaf/Cargo.toml", &package("leaf", "")); + write(&origin, "leaf/src/lib.rs", "pub fn word() -> &'static str { \"one\" }\n"); + write(&origin, "count/Cargo.toml", &package("count", "")); + let script = r#"fn main() { + let files = std::fs::read_dir("src").unwrap().count(); + println!("cargo:rustc-env=FILES={files}"); +} +"#; + write(&origin, "count/build.rs", script); + write(&origin, "count/src/lib.rs", "pub const FILES: &str = env!(\"FILES\");\n"); + write(&origin, "count/src/spare.txt", "read by nothing but the count\n"); + write(&origin, "idle/Cargo.toml", &package("idle", "")); + write(&origin, "idle/src/lib.rs", "pub fn idle() {}\n"); + let deps = "leaf = { path = \"../leaf\" }\ncount = { path = \"../count\" }\n"; + write(&origin, "app/Cargo.toml", &package("app", deps)); + let main = "fn main() { print!(\"{} {}\", leaf::word(), count::FILES); }\n"; + write(&origin, "app/src/main.rs", main); + sh(&origin, &["add", "-A"]); + sh(&origin, &["commit", "-qm", "built"]); + + let writer = tmp.join("writer"); + checkout(&origin, &writer); + let Start::Cold(stamped) = open(&writer).unwrap().0 else { + panic!("a tree with no target is cold") + }; + build(&writer); + assert_eq!(run(&writer), "one 2"); + seal(&writer, &stamped).unwrap(); + + write(&origin, "leaf/src/lib.rs", "pub fn word() -> &'static str { \"two\" }\n"); + sh(&origin, &["rm", "-q", "count/src/spare.txt"]); + sh(&origin, &["commit", "-qam", "read"]); + let reader = tmp.join("reader"); + checkout(&origin, &reader); + fs::rename(writer.join("target"), reader.join("target")).unwrap(); + // What an mtime made up from history would say of a file committed + // before the entry was built. + date(&reader.join("leaf/src/lib.rs"), UNIX_EPOCH + Duration::from_secs(1)).unwrap(); + + let said = open(&reader).unwrap(); + assert!(matches!(said.0, Start::Warm), "{}", said.1); + assert!(!reader.join(MANIFEST).exists(), "a warm tree keeps no manifest"); + let fresh = build(&reader); + assert_eq!(run(&reader), "two 1"); + let expected = [("app", false), ("count", false), ("idle", true), ("leaf", false)]; + assert_eq!(fresh, expected.into_iter().map(|(k, v)| (k.to_string(), v)).collect()); + } + + /// Targets with no manifest beside them were built from nothing anyone can + /// name; the driver's own is this job's. + #[test] + fn targets_restored_without_a_manifest_are_refused() { + let tmp = TempDir::new("cicache-foreign"); + sh(&tmp, &["init", "-q"]); + configure(&tmp); + write(&tmp, "kernel/src/lib.rs", ""); + sh(&tmp, &["add", "-A"]); + fs::create_dir_all(tmp.join(DRIVER)).unwrap(); + assert!(matches!(open(&tmp).unwrap().0, Start::Cold(_))); + fs::create_dir_all(tmp.join("kernel/target/debug")).unwrap(); + let refusal = open(&tmp).err().expect("a restored target with no manifest"); + assert!(refusal.contains("kernel/target"), "{refusal}"); + } + + /// A sealed tree is dated as built throughout, and a step that wrote a + /// tracked source is refused: the manifest would name bytes no build read. + #[test] + fn a_seal_dates_every_target_and_refuses_a_written_source() { + let tmp = TempDir::new("cicache-seal"); + sh(&tmp, &["init", "-q"]); + configure(&tmp); + write(&tmp, "a.rs", "a\n"); + sh(&tmp, &["add", "-A"]); + sh(&tmp, &["commit", "-qm", "a"]); + let Start::Cold(stamped) = open(&tmp).unwrap().0 else { panic!("cold") }; + write(&tmp, "target/debug/deps/x", "x"); + write(&tmp, "userland/target/y", "y"); + seal(&tmp, &stamped).unwrap(); + for file in ["target/debug/deps/x", "target/debug", "userland/target/y"] { + assert_eq!(modified(&tmp.join(file)).unwrap(), built(), "{file}"); + } + fs::remove_file(tmp.join(MANIFEST)).unwrap(); + write(&tmp, "a.rs", "b\n"); + let refusal = seal(&tmp, &stamped).unwrap_err(); + assert!(refusal.contains("a.rs"), "{refusal}"); + } + + #[test] + fn a_prune_keeps_this_runs_entry_on_main_and_nothing_else() { + let entry = + |id: u64, key: &str, scope: &str| serde_json::json!({"id": id, "key": key, "ref": scope}); + let ours = "host-sealed-Linux-7"; + let listing = |entries: Vec| { + serde_json::json!({"total_count": entries.len(), "actions_caches": entries}) + }; + let doc = listing(vec![ + entry(1, ours, "refs/heads/main"), + entry(2, "host-sealed-Linux-6", "refs/heads/main"), + entry(3, "host-sealed-Linux-5", "refs/pull/9/merge"), + entry(4, "host-36696295750", "refs/heads/main"), + ]); + let ids: Vec = doomed(&doc, ours).unwrap().into_iter().map(|(id, _)| id).collect(); + assert_eq!(ids, [2, 3, 4]); + + let unsaved = listing(vec![ + entry(1, ours, "refs/pull/9/merge"), + entry(2, "host-sealed-Linux-6", "refs/heads/main"), + ]); + assert!(doomed(&unsaved, ours).unwrap_err().contains("not among main's")); + let page = [entry(1, ours, "refs/heads/main")]; + let paged = serde_json::json!({"total_count": 101, "actions_caches": page}); + assert!(doomed(&paged, ours).is_err()); + } +} diff --git a/src/lib.rs b/src/lib.rs index 1bb0c9c3579..b19a286b3ef 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,6 +4,7 @@ pub mod bootlog; pub mod build; pub mod buildlock; pub mod ci; +pub mod cicache; pub mod clang; pub mod clippy; pub mod compiler; diff --git a/src/licence.rs b/src/licence.rs index 437c9ac1924..c20b6e0db15 100644 --- a/src/licence.rs +++ b/src/licence.rs @@ -1121,19 +1121,31 @@ fn metadata( serde_json::from_slice(&out).map_err(|e| format!("cargo metadata printed no JSON: {e}")) } -/// The fork's `library/`, checked out at the commit this tree pins. A checkout -/// whose `rust/` was never initialised — a CI runner's — fetches that commit -/// alone. +/// The fork's `library/` at the commit this tree pins, in `rust/`, where its +/// manifests' paths to `toyos` and `toyos-abi` land in this tree. A runner's +/// `rust/` was never initialised, so it fetches that commit's `library/` alone, +/// with no other blob of the fork: a checkout that sparse would be all a +/// developer's later build found. fn std_library(root: &Path) -> Result { let fork = crate::sysroot::fork_checkout(root); - if !fork.join("library/Cargo.toml").exists() { - run( - Command::new("git") - .args(["submodule", "update", "--init", "--depth", "1", "rust"]) - .current_dir(root), - "git submodule update --init --depth 1 rust", - )?; - } + if fork.join("library/Cargo.toml").exists() { + return Ok(fork.join("library")); + } + if !crate::ci::on_runner() { + return Err(format!("{} holds no fork checkout; any `cargo run` makes one", fork.display())); + } + let pin = crate::sync::git(root, &["rev-parse", ":rust"])?; + let url = crate::sync::git(root, &["config", "-f", ".gitmodules", "submodule.rust.url"])?; + for args in [ + &["init", "-q"][..], + &["remote", "add", "origin", &url], + &["fetch", "-q", "--depth", "1", "--filter=blob:none", "origin", &pin], + &["sparse-checkout", "set", "library"], + &["checkout", "-q", "--detach", &pin], + ] { + crate::sync::git(&fork, args)?; + } + println!("the fork's library/ at {pin}, fetched"); Ok(fork.join("library")) } From 1f86e1806f5fdb38d01905638210b75f28cf0625 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 11:29:42 +0200 Subject: [PATCH 02/20] clippy: toyos-abi's own lints check it in a target of their own Cargo keeps one check of a unit, and the toyos-abi shape lints the unit the workspace shape also lints, under other lints: each re-checks it every run, and the workspace shape then re-checks every crate depending on it. Measured on this host: after the shared-target toyos-abi shape the workspace shape checked 13 crates again; after one in target/clippy-abi, none. A cache read by content otherwise serves every one of those checks. The guest cache's discipline is filed: its writer restores before it saves, and its readers judge by mtime. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- ...and-its-writer-restores-before-it-saves.md | 21 +++++++++++++++++++ src/clippy.rs | 5 ++++- 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md diff --git a/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md b/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md new file mode 100644 index 00000000000..0749a86c411 --- /dev/null +++ b/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md @@ -0,0 +1,21 @@ +--- +status: open +kind: tooling +opened: 2026-10-01 +--- + +# The guest cache is read by mtime, and its writer restores before it saves + +`nightly.yml`'s `tcg` restores the newest `guest-` entry, builds on it and saves +the result: nothing prunes what no step rebuilt, so every write keeps the last +one's artifacts and adds its own (3,281,375,938 B on 2026-10-01, beside the +host entry in the repository's 10 GB). And every guest job restores its targets +under a checkout that dated every source at the checkout, so cargo calls every +path crate in them stale. + +The host cache stopped both (`src/cicache.rs`): an entry is sealed only by a +run that restored nothing, carries the blob of every source its build read, +and its reader dates sources by content. The guest lanes are being reshaped by +the guest suite's cut, so the same discipline waits for their shape to settle. + +Done when the guest entry is written cold, read by content, and bounded. diff --git a/src/clippy.rs b/src/clippy.rs index 29a8f34f5b5..0dd24d0a8bd 100644 --- a/src/clippy.rs +++ b/src/clippy.rs @@ -153,9 +153,12 @@ const SHAPES: &[Shape] = &[ ], after: &["$ADOPTED", "-D", "warnings"], }, + // Its own target directory: cargo keeps one check of a unit, so a unit + // linted under two sets of lints is checked again by each, every run, and + // so is every crate depending on it. Shape { dir: "", - before: &["-p", "toyos-abi", "--all-targets", "--keep-going"], + before: &["-p", "toyos-abi", "--all-targets", "--keep-going", "--target-dir", "target/clippy-abi"], after: &["-W", "clippy::undocumented_unsafe_blocks", "-D", "warnings"], }, ]; From 455780ef61a2668f272728dd098c3686982b52d3 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 12:00:37 +0200 Subject: [PATCH 03/20] toyos-dhcp: a trailing blank line, so this pull request's next run has one leaf crate to recompile A measurement, reverted by the next commit: the run before this one sealed its tree into this pull request's cache scope, and this run reads it. Nothing depends on toyos-dhcp, so it is the only crate whose bytes differ. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- toyos-dhcp/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/toyos-dhcp/src/lib.rs b/toyos-dhcp/src/lib.rs index e9734d6aa75..430edee33be 100644 --- a/toyos-dhcp/src/lib.rs +++ b/toyos-dhcp/src/lib.rs @@ -871,3 +871,4 @@ struct Offered { t1: Option, t2: Option, } + From e57e0bed5ec72d5fc7b4b5d17a284a9853b02039 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 12:10:59 +0200 Subject: [PATCH 04/20] Revert "toyos-dhcp: a trailing blank line, so this pull request's next run has one leaf crate to recompile" This reverts commit 455780ef61a2668f272728dd098c3686982b52d3: its run measured what it was for. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- toyos-dhcp/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/toyos-dhcp/src/lib.rs b/toyos-dhcp/src/lib.rs index 430edee33be..e9734d6aa75 100644 --- a/toyos-dhcp/src/lib.rs +++ b/toyos-dhcp/src/lib.rs @@ -871,4 +871,3 @@ struct Offered { t1: Option, t2: Option, } - From 94050127506aeb5a7a6f10ebb922a56df2ad15a6 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 12:11:25 +0200 Subject: [PATCH 05/20] ci: a runner's steps build with no incremental state An entry is read by content, so a step recompiles only a crate whose bytes changed, and incremental state is most of an entry's bytes: on this host one host run's root target held 4.36 of 6.66 GB under incremental/, and the workspace's test build took 2.83 GB with it and 1.01 GB without. This pull request's first warm run restored its 3,250,736,567 B entry in 109 s, more than every compile in it together (23.0 s). The driver keeps it: its path crates recompile every run, and incremental state is what makes that 13.2 s on a runner. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- src/ci.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ci.rs b/src/ci.rs index 13b5a542eac..6843344f780 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -429,6 +429,10 @@ fn host(root: &Path) -> Vec { let mut steps = Vec::new(); let mut cold = None; if on_runner() { + // No incremental state in an entry: it is most of an entry's bytes, + // and after a read by content it helps only a crate whose bytes + // changed. + std::env::set_var("CARGO_INCREMENTAL", "0"); let mut start = None; steps.push(step("the cache entry, read by content", || { let (found, said) = cicache::read(root)?; From 47a0d9217697c8b53340f8c58acbf6255edcb13d Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 12:32:15 +0200 Subject: [PATCH 06/20] cicache: the module doc says which way cargo's comparison is strict Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- src/cicache.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cicache.rs b/src/cicache.rs index 369c73bf17a..c46e1041e8e 100644 --- a/src/cicache.rs +++ b/src/cicache.rs @@ -7,9 +7,9 @@ //! An entry instead carries [`MANIFEST`], the git blob id of every source its //! build read, and every file under its targets is dated [`built`], before any //! real time. [`read`] dates each source whose blob matches the same, and every -//! other one now: cargo's comparison is strict, so a matching source is no -//! newer than the build, and a changed or new one is newer than everything in -//! the entry, whatever any runner's clock says. +//! other one now: cargo calls a source stale only when it is strictly newer +//! than the build, so a match is fresh, and a changed or new source is newer +//! than everything in the entry, whatever any runner's clock says. //! //! **Only a run that restored nothing seals an entry** ([`Start::Cold`], //! [`seal`]): a warm run's targets hold units none of its steps rebuilt, From 59cc178b674580b67edd3dd59a80b86945dc9e13 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 13:19:20 +0200 Subject: [PATCH 07/20] Review round 1: one writer, no gh, sha2 in-process, packages dated whole, the image in the manifest Answers the review at #669 (issuecomment-5930046178). - The pull request's own cache save goes, with the comments that described it and the loosened writer test: nightly.yml's `host` is the host cache's one writer again, and `each_cache_has_one_writer` is main's test plus the assertion that a host save is guarded by the manifest. - `--ci prune`, `doomed`, `gh` and nightly's `actions: write` go. Eviction is by last access, and with one writer the newest host and guest entries are the most recently read. - A tracked file's identity is the SHA-256 of its bytes, hashed in-process over `sysroot::tracked_files`; the `git hash-object` child, its writer thread and its count check go. The gitlink is skipped as a directory. - The licence step's sparse fetch (git init, remote add, an HTTPS `fetch --filter=blob:none`, sparse-checkout, checkout --detach) goes back to main's `git submodule update --init --depth 1 rust`, the use the admitted git row already declares. Measuring gitoxide's partial-clone and sparse-checkout support for a verdict was not worth the licence step's 15-20 s. - A package with any tracked file changed, added or removed has every file dated now. A file rustc probed for and did not find (a new `src/x/mod.rs` beside `src/x.rs`, E0761 cold) is in no dep-info, so dating the new file alone left the crate fresh; dating its package's files rebuilds it. This subsumes the lost-file rule. What a build reads outside its own package undeclared stays trusted, as cargo's own incremental build trusts it, and is filed with its exit. - The runner image goes in the manifest, not the key: no workflow expression sees `ImageOS` or `ImageVersion` (the `env` context holds only what a workflow sets; rclone's and apache/httpd's workflows say so at their sites). The key carries `runner.os` and `runner.arch`; a reader whose `RUNNER_OS RUNNER_ARCH ImageOS ImageVersion` differs from the entry's deletes the restored targets and runs cold. - A warm reader whose clock does not read after 2001-09-09 is refused. - New tests: a written-back source is refused by the seal; a restored `target/debug` without a manifest is refused; a driver built elsewhere is refused; another image's entry is deleted; a clock at the entry's date is refused; E0761 rebuilds warm as it fails cold. - REMOVEs: ci.yml's "runs the same on a dev host", identity.rs's "one definition" sentence, and cicache's "whatever any runner's clock says". Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- .github/workflows/ci.yml | 23 +- .github/workflows/nightly.yml | 9 +- ...-what-a-build-reads-outside-its-package.md | 23 + ...and-its-writer-restores-before-it-saves.md | 4 +- src/ci.rs | 44 +- src/cicache.rs | 509 +++++++++--------- src/identity.rs | 7 +- src/licence.rs | 34 +- 8 files changed, 317 insertions(+), 336 deletions(-) create mode 100644 issues/build/a-warm-host-run-trusts-cargo-for-what-a-build-reads-outside-its-package.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 96ef0005e85..654c25ffe6e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,7 +1,6 @@ name: ci -# A pull request and the merge queue: the host tests, and no guest. Each step -# is `cargo run -- --ci ` (src/ci.rs), which runs the same on a dev host; +# A pull request and the merge queue: the host tests, and no guest. # nightly.yml boots the guests. on: @@ -24,13 +23,11 @@ jobs: steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - # An entry is read by content (src/cicache.rs): main's, which - # nightly.yml's `host` writes, or one this pull request saved when it - # found none. The paths are the cache's version, so they are the - # writer's list. + # nightly.yml's `host` is the one writer; the paths are the cache's + # version, so they are the writer's list. - uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 with: - path: &host-paths | + path: | ~/.cargo/registry/index ~/.cargo/registry/cache ~/.cargo/git/db @@ -39,15 +36,7 @@ jobs: toyos/target kernel/target bootloader/target - key: host-sealed-${{ runner.os }}-${{ github.run_id }} - restore-keys: host-sealed-${{ runner.os }}- + key: host-sealed-${{ runner.os }}-${{ runner.arch }}-${{ github.run_id }} + restore-keys: host-sealed-${{ runner.os }}-${{ runner.arch }}- - run: cargo run --target-dir target/ci-driver -- --ci host - - # Only a run that restored nothing leaves a manifest, and what a pull - # request saves only its own later runs read. - - if: github.event_name == 'pull_request' && hashFiles('target/ci-sources') != '' - uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 - with: - path: *host-paths - key: host-sealed-${{ runner.os }}-${{ github.run_id }} diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 98496b6a20e..0b8d571f2da 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -24,9 +24,6 @@ jobs: if: github.ref == 'refs/heads/main' runs-on: macos-latest timeout-minutes: 90 - permissions: - actions: write - contents: read steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 with: @@ -46,11 +43,7 @@ jobs: toyos/target kernel/target bootloader/target - key: host-sealed-${{ runner.os }}-${{ github.run_id }} - - - env: - GH_TOKEN: ${{ github.token }} - run: cargo run --target-dir target/ci-driver -- --ci prune + key: host-sealed-${{ runner.os }}-${{ runner.arch }}-${{ github.run_id }} # Publishes this tree's toolchain if nobody has, and on main moves the SDK # alias onto it. Bare `ubuntu-24.04`, not a container: its glibc is the diff --git a/issues/build/a-warm-host-run-trusts-cargo-for-what-a-build-reads-outside-its-package.md b/issues/build/a-warm-host-run-trusts-cargo-for-what-a-build-reads-outside-its-package.md new file mode 100644 index 00000000000..ed51e76440d --- /dev/null +++ b/issues/build/a-warm-host-run-trusts-cargo-for-what-a-build-reads-outside-its-package.md @@ -0,0 +1,23 @@ +--- +status: open +kind: tooling +opened: 2026-10-01 +--- + +# A warm host run trusts cargo for what a build reads outside its package + +The merge queue's `host` run reads main's cache entry by content +(`src/cicache.rs`): every file of a package that changed is dated now, so +whatever a build reads inside its own package rebuilds it. An input outside +that package reaches a warm run only through what cargo is told, a build +script's `rerun-if-*` lines and rustc's dep-info. A path build script or proc +macro that reads another package's file, an environment variable or a tool +without declaring it passes stale there, where before every path crate +recompiled; only the cold nightly finds it, after main has moved. + +None does in the host job today: its one path build script, `userland/calc`'s, +declares the font it reads from `assets/`, and no path crate is a proc macro. + +Done when a test in `src/cicache.rs` whose path build script reads another +package's file without declaring it, read warm after a change to that file, +sees the build script run again. diff --git a/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md b/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md index 9e76ab038cd..9ff10d4df3e 100644 --- a/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md +++ b/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md @@ -14,7 +14,7 @@ under a checkout that dated every source at the checkout, so cargo calls every path crate in them stale. The host cache stopped both (`src/cicache.rs`): an entry is sealed only by a -run that restored nothing, carries the blob of every source its build read, -and its reader dates sources by content. +run that restored nothing, carries the hash of every tracked file, and its +reader dates sources by content. Done when the guest entry is written cold, read by content, and bounded. diff --git a/src/ci.rs b/src/ci.rs index df4b056d437..851c0477235 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -35,8 +35,6 @@ const USAGE: &str = "cargo run -- --ci , where is one of: host every host test: the build system, the harness's own checks, the host workspace, the licences of what ships, clippy, the model controls, userland and the SDK (ci.yml, nightly) - prune delete every host cache entry but the one this run saved - on main (nightly) toolchain publish this tree's toolchain if nobody has (nightly) guest the guest suite (nightly) publish put main's SDK crates on crates.io (publish.yml)"; @@ -44,7 +42,6 @@ const USAGE: &str = "cargo run -- --ci , where is one of: #[derive(Debug, PartialEq, Eq)] enum Job { Host, - Prune, Toolchain, Guest, Publish, @@ -53,7 +50,6 @@ enum Job { fn parse(words: &[String]) -> Result { let job = match words.first().map(String::as_str) { Some("host") => Job::Host, - Some("prune") => Job::Prune, Some("toolchain") => Job::Toolchain, Some("guest") => Job::Guest, Some("publish") => Job::Publish, @@ -73,7 +69,6 @@ pub fn dispatch(root: &Path, args: &[String]) { }); let steps = match &job { Job::Host => host(root), - Job::Prune => vec![step("the host cache's other entries", || cicache::prune(root))], Job::Toolchain => vec![step("the toolchain release", || release::ensure_published(root))], Job::Guest => guest(root, &suite_args(&["--jobs", "1"])), Job::Publish => vec![step("the SDK crates on crates.io", || publish(root))], @@ -116,7 +111,7 @@ fn step(label: &str, f: impl FnOnce() -> Result) -> Step { Step { label: label.to_string(), verdict } } -pub(crate) fn on_runner() -> bool { +fn on_runner() -> bool { std::env::var("GITHUB_ACTIONS").is_ok_and(|v| v == "true") } @@ -472,8 +467,8 @@ fn run_control(root: &Path, control: &Control) -> Result { /// host triple for the same reason. /// /// On a runner the restored cache entry is read before any step and a run that -/// restored none seals its tree after the last ([`cicache`]); a developer's -/// tree keeps the dates its edits gave it. +/// starts cold seals its tree after the last ([`cicache`]); a developer's tree +/// keeps the dates its edits gave it. fn host(root: &Path) -> Vec { let tmp = toyos_tmpdir::TempDir::new("ci-host"); let short = Path::new(toyos_tmpdir::SHORT_BASE); @@ -498,7 +493,7 @@ fn host(root: &Path) -> Vec { match start { // Every step after an unreadable entry would be judged against it. None => return steps, - Some(Start::Cold(stamped)) => cold = Some(stamped), + Some(Start::Cold(found)) => cold = Some(found), Some(Start::Warm) => {} } } @@ -564,8 +559,8 @@ fn host(root: &Path) -> Vec { cargo(root, &["test", "--manifest-path", "toyos/Cargo.toml", "--target", &host_triple]) })); steps.push(step("nothing left in $TMPDIR or /tmp", || left_behind(&tmp, short, &before))); - if let Some(stamped) = cold { - steps.push(step("the tree, sealed as a cache entry", || cicache::seal(root, &stamped))); + if let Some(cold) = cold { + steps.push(step("the tree, sealed as a cache entry", || cicache::seal(root, &cold))); } steps } @@ -888,7 +883,6 @@ mod tests { #[test] fn a_job_is_named_and_takes_nothing_after_it() { assert_eq!(parse(&words("host")), Ok(Job::Host)); - assert_eq!(parse(&words("prune")), Ok(Job::Prune)); assert_eq!(parse(&words("guest")), Ok(Job::Guest)); assert!(parse(&words("guest 3/12")).is_err()); assert!(parse(&words("tcg")).is_err()); @@ -999,15 +993,14 @@ mod tests { assert_eq!(seen, 3, "ci.yml, nightly.yml and publish.yml"); } - /// A host entry is saved only beside its manifest, which only a sealed tree - /// holds ([`cicache`]); every other cache has exactly one writer, on the - /// nightly, so what a run restores is one run's tree and never a race - /// between two writers. + /// Exactly one job writes each cache, on the nightly, so what a pull request + /// restores is one run's tree and never a race between two writers. The + /// host cache's saves only a sealed tree, the one that holds a manifest + /// ([`cicache`]). #[test] - fn a_cache_is_saved_only_where_a_reader_can_trust_it() { + fn each_cache_has_one_writer() { let dir = repo_root().join(".github/workflows"); let mut writers = Vec::new(); - let mut host = 0; for entry in std::fs::read_dir(&dir).expect(".github/workflows is readable").flatten() { let text = std::fs::read_to_string(entry.path()).expect("a readable workflow"); let name = entry.file_name().to_string_lossy().into_owned(); @@ -1020,19 +1013,16 @@ mod tests { .find_map(|l| l.trim_start().strip_prefix("key: ")) .expect("a save names its key"); let prefix = key.split('$').next().unwrap_or("").to_string(); - if !prefix.starts_with("host-") { - writers.push((name.clone(), prefix)); - continue; + if prefix.starts_with("host-") { + assert_eq!(prefix, cicache::SEALED, "{name}"); + let manifest = format!("hashFiles('{}') != ''", cicache::MANIFEST); + assert!(lines[at - 1].contains(&manifest), "{name}: a host save without `{manifest}`"); } - host += 1; - assert_eq!(prefix, cicache::SEALED, "{name}"); - let manifest = format!("hashFiles('{}') != ''", cicache::MANIFEST); - assert!(lines[at - 1].contains(&manifest), "{name}: a host save without `{manifest}`"); + writers.push((name.clone(), prefix)); } } } - assert!(host > 0, "no job writes the host cache, so every host run is cold"); - assert!(!writers.is_empty(), "no job writes the guest cache, so every guest run is cold"); + assert!(writers.iter().any(|(_, p)| p == cicache::SEALED), "no job writes the host cache: {writers:?}"); writers.sort(); let mut prefixes: Vec<&String> = writers.iter().map(|(_, p)| p).collect(); prefixes.dedup(); diff --git a/src/cicache.rs b/src/cicache.rs index c46e1041e8e..b3ced404b2f 100644 --- a/src/cicache.rs +++ b/src/cicache.rs @@ -4,12 +4,25 @@ //! build that read it, and a checkout dates every source at the checkout: a //! restored target is stale in every path crate, and a date made up for a //! source from anything but its bytes could as well call a changed one fresh. -//! An entry instead carries [`MANIFEST`], the git blob id of every source its -//! build read, and every file under its targets is dated [`built`], before any -//! real time. [`read`] dates each source whose blob matches the same, and every -//! other one now: cargo calls a source stale only when it is strictly newer -//! than the build, so a match is fresh, and a changed or new source is newer -//! than everything in the entry, whatever any runner's clock says. +//! An entry instead carries [`MANIFEST`], the runner it was built on and the +//! SHA-256 of every tracked file, and every file under its targets is dated +//! [`built`], before any real time. [`read`] dates each tracked file whose hash +//! matches the same, and every other one now: cargo calls a source stale only +//! when it is strictly newer than the build, so a match is fresh, and a changed +//! or new source is newer than everything in the entry. +//! +//! **A package with a file changed, added or removed has every file dated +//! now.** Cargo is told only what a build read; a file rustc probed for and did +//! not find (`src/x/mod.rs` beside `src/x.rs`), or one a build script read +//! without naming it, is still its package's. What a build reads outside its +//! own package without telling cargo, a warm run trusts as cargo's own +//! incremental build does: +//! `issues/build/a-warm-host-run-trusts-cargo-for-what-a-build-reads-outside-its-package.md`. +//! +//! **An entry built on another runner image is deleted, and the run is cold**: +//! the image's linker and C compiler made its units, and cargo's fingerprint +//! names neither. The cache key cannot carry the image, because no workflow +//! expression sees `ImageOS` or `ImageVersion`. //! //! **Only a run that restored nothing seals an entry** ([`Start::Cold`], //! [`seal`]): a warm run's targets hold units none of its steps rebuilt, @@ -20,23 +33,18 @@ //! **The driver is built in [`DRIVER`]**: cargo builds it before this runs, so //! its path crates are compiled again every time, and in the steps' target that //! would make every crate depending on them stale. -//! -//! A package that lost a file keeps its `Cargo.toml` dated now: cargo's package -//! fingerprint, which decides a build script that names no input, is the -//! newest of the files that remain. use std::collections::{BTreeMap, BTreeSet}; use std::fs; -use std::io::{ErrorKind, Write}; +use std::io::ErrorKind; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; +use sha2::{Digest, Sha256}; + pub const MANIFEST: &str = "target/ci-sources"; pub const DRIVER: &str = "target/ci-driver"; -/// Every host entry's key starts with this; [`prune`] lists by it. -const HOST: &str = "host-"; -/// A sealed entry's key is this, `${{ runner.os }}-${{ github.run_id }}`. +/// A sealed entry's key is this, `${{ runner.os }}-${{ runner.arch }}-${{ github.run_id }}`. pub const SEALED: &str = "host-sealed-"; /// 2001-09-09T01:46:40Z: older than any build, so a file dated so is never @@ -45,16 +53,21 @@ fn built() -> SystemTime { UNIX_EPOCH + Duration::from_secs(1_000_000_000) } -/// Every source as git tracks it, with the blob id of its bytes on disk. +/// Every file git tracks, with the SHA-256 of its bytes on disk. type Sources = BTreeMap; /// What a job found before its first step. pub enum Start { /// An entry, read and its manifest consumed. Warm, - /// No entry: every source is dated [`built`], and these are what [`seal`] - /// holds the tree to at the end. - Cold(Sources), + /// No entry this runner can use: every source is dated [`built`]. + Cold(Cold), +} + +/// What [`seal`] holds a cold run's tree to at the end. +pub struct Cold { + runner: String, + sources: Sources, } /// Before the first step: the entry restored here, read by content. @@ -70,10 +83,15 @@ pub fn read(root: &Path) -> Result<(Start, String), String> { exe.display() )); } - open(root) + let runner = ["RUNNER_OS", "RUNNER_ARCH", "ImageOS", "ImageVersion"] + .iter() + .map(|name| std::env::var(name).map_err(|_| format!("{name} is unset: a hosted runner sets it"))) + .collect::, _>>()? + .join(" "); + open(root, &runner, SystemTime::now()) } -fn open(root: &Path) -> Result<(Start, String), String> { +fn open(root: &Path, runner: &str, now: SystemTime) -> Result<(Start, String), String> { let current = sources(root)?; let manifest = root.join(MANIFEST); let text = match fs::read_to_string(&manifest) { @@ -86,51 +104,69 @@ fn open(root: &Path) -> Result<(Start, String), String> { restored.iter().map(|p| p.display().to_string()).collect::>().join(", ") )); } - for path in current.keys() { - date(&root.join(path), built())?; - } - let said = format!( - "none restored; {} sources dated as built, and the tree is sealed last", - current.len() - ); - return Ok((Start::Cold(current), said)); + return cold(root, runner, current, "none restored".into()); } Err(e) => return Err(format!("read {MANIFEST}: {e}")), }; fs::remove_file(&manifest).map_err(|e| format!("remove {MANIFEST}: {e}"))?; - let (commit, entry) = parse(&text)?; - let lost = lost(&entry, ¤t); - let now = SystemTime::now(); + let (commit, built_on, entry) = parse(&text)?; + if built_on != runner { + for path in restored(root)? { + let gone = if path.is_dir() { fs::remove_dir_all(&path) } else { fs::remove_file(&path) }; + gone.map_err(|e| format!("remove {}: {e}", path.display()))?; + } + return cold(root, runner, current, format!("{commit}'s entry, built on {built_on}, deleted")); + } + if now <= built() { + return Err(format!("this runner's clock reads {now:?}, which no change dated now is newer than")); + } + let dirty: BTreeSet> = current + .iter() + .filter(|(path, hash)| entry.get(*path) != Some(*hash)) + .map(|(path, _)| path) + .chain(entry.keys().filter(|path| !current.contains_key(*path))) + .map(|path| package(path, ¤t)) + .collect(); let mut same = 0; - for (path, blob) in ¤t { - let fresh = entry.get(path) == Some(blob) && !lost.contains(path); + for (path, hash) in ¤t { + let fresh = entry.get(path) == Some(hash) && !dirty.contains(&package(path, ¤t)); same += usize::from(fresh); date(&root.join(path), if fresh { built() } else { now })?; } - let changed = current.iter().filter(|(p, b)| entry.get(*p).is_some_and(|e| e != *b)).count(); + let changed = current.iter().filter(|(p, h)| entry.get(*p).is_some_and(|e| e != *h)).count(); let added = current.keys().filter(|p| !entry.contains_key(*p)).count(); let removed = entry.keys().filter(|p| !current.contains_key(*p)).count(); Ok(( Start::Warm, format!( - "built from {commit}: {same} of {} sources unchanged, {changed} changed, {added} added, \ - {removed} removed", - current.len() + "built from {commit}: {same} of {} sources dated as built; {changed} changed, {added} \ + added, {removed} removed, in {} packages", + current.len(), + dirty.len() ), )) } +fn cold(root: &Path, runner: &str, sources: Sources, why: String) -> Result<(Start, String), String> { + for path in sources.keys() { + date(&root.join(path), built())?; + } + let said = format!("{why}; {} sources dated as built, and the tree is sealed last", sources.len()); + Ok((Start::Cold(Cold { runner: runner.to_string(), sources }), said)) +} + /// After the last step of a run that started [`Start::Cold`]: every file under /// every target dated [`built`], then the manifest. -pub fn seal(root: &Path, stamped: &Sources) -> Result { +pub fn seal(root: &Path, cold: &Cold) -> Result { let now = sources(root)?; - let moved: Vec<&String> = stamped + let moved: Vec<&String> = cold + .sources .iter() - .filter(|(path, blob)| { - now.get(*path) != Some(*blob) || modified(&root.join(path)).ok() != Some(built()) + .filter(|(path, hash)| { + now.get(*path) != Some(*hash) || modified(&root.join(path)).ok() != Some(built()) }) .map(|(path, _)| path) - .chain(now.keys().filter(|path| !stamped.contains_key(*path))) + .chain(now.keys().filter(|path| !cold.sources.contains_key(*path))) .collect(); if !moved.is_empty() { return Err(format!("a step wrote tracked sources, which no entry can describe: {moved:?}")); @@ -140,155 +176,53 @@ pub fn seal(root: &Path, stamped: &Sources) -> Result { age(&target, &mut files, &mut bytes)?; } let head = crate::sync::git(root, &["rev-parse", "HEAD"])?; - let mut text = format!("{head}\n"); - for (path, blob) in stamped { - text.push_str(&format!("{blob} {path}\n")); + let mut text = format!("{head}\n{}\n", cold.runner); + for (path, hash) in &cold.sources { + text.push_str(&format!("{hash} {path}\n")); } fs::write(root.join(MANIFEST), text).map_err(|e| format!("write {MANIFEST}: {e}"))?; - Ok(format!("{} sources; {files} files, {} MiB, dated as built", stamped.len(), bytes >> 20)) -} - -/// Every host entry but the one this run saved, which must be on `main`: -/// pull requests read that one, and every other only fills the repository's -/// cache budget. -pub fn prune(root: &Path) -> Result { - let var = - |name: &str| std::env::var(name).map_err(|_| format!("{name} is unset: a runner prunes")); - let (repo, os, run) = (var("GITHUB_REPOSITORY")?, var("RUNNER_OS")?, var("GITHUB_RUN_ID")?); - let ours = format!("{SEALED}{os}-{run}"); - let caches = format!("repos/{repo}/actions/caches"); - let listing = - gh(root, &["api", "-X", "GET", &caches, "-f", &format!("key={HOST}"), "-F", "per_page=100"])?; - let doc: serde_json::Value = - serde_json::from_str(&listing).map_err(|e| format!("the cache listing is not JSON: {e}"))?; - let doomed = doomed(&doc, &ours)?; - for (id, _) in &doomed { - gh(root, &["api", "-X", "DELETE", &format!("{caches}/{id}")])?; - } - let keys: Vec<&str> = doomed.iter().map(|(_, key)| key.as_str()).collect(); - Ok(format!("kept {ours}; deleted {}: {}", keys.len(), keys.join(", "))) -} - -/// Every entry in the listing but `ours`, which must be in it on `main`. -fn doomed(doc: &serde_json::Value, ours: &str) -> Result, String> { - let entries = doc["actions_caches"].as_array().ok_or("the listing holds no actions_caches")?; - let total = doc["total_count"].as_u64().ok_or("the listing holds no total_count")?; - if total != entries.len() as u64 { - return Err(format!( - "{total} host entries and a page of {}: pruning a page is no bound", - entries.len() - )); - } - let mut kept = false; - let mut doomed = Vec::new(); - for entry in entries { - let (Some(id), Some(key), Some(scope)) = - (entry["id"].as_u64(), entry["key"].as_str(), entry["ref"].as_str()) - else { - return Err(format!("an entry without an id, key or ref: {entry}")); - }; - if key == ours && scope == "refs/heads/main" { - kept = true; - } else { - doomed.push((id, key.to_string())); - } - } - if !kept { - return Err(format!( - "{ours} is not among main's entries: the save before this wrote none, and pruning would \ - leave pull requests nothing to read" - )); - } - Ok(doomed) -} - -fn gh(root: &Path, args: &[&str]) -> Result { - let out = Command::new("gh") - .args(args) - .current_dir(root) - .output() - .map_err(|e| format!("gh: {e}"))?; - if !out.status.success() { - let said = String::from_utf8_lossy(&out.stderr); - return Err(format!("gh {} exited {}: {}", args.join(" "), out.status, said.trim())); - } - Ok(String::from_utf8_lossy(&out.stdout).into_owned()) + Ok(format!("{} sources; {files} files, {} MiB, dated as built", cold.sources.len(), bytes >> 20)) } -fn parse(text: &str) -> Result<(String, Sources), String> { +/// The commit an entry was built from, the runner, and its sources. +fn parse(text: &str) -> Result<(String, String, Sources), String> { let mut lines = text.lines(); - let commit = lines.next().ok_or_else(|| format!("{MANIFEST} is empty"))?.to_string(); + let (Some(commit), Some(runner)) = (lines.next(), lines.next()) else { + return Err(format!("{MANIFEST} ends before its commit and runner")); + }; let entry = lines .map(|line| { line.split_once(' ') - .filter(|(blob, _)| !blob.is_empty() && blob.bytes().all(|b| b.is_ascii_hexdigit())) - .map(|(blob, path)| (path.to_string(), blob.to_string())) + .filter(|(hash, _)| !hash.is_empty() && hash.bytes().all(|b| b.is_ascii_hexdigit())) + .map(|(hash, path)| (path.to_string(), hash.to_string())) .ok_or_else(|| format!("{MANIFEST} holds {line:?}")) }) .collect::>()?; - Ok((commit, entry)) + Ok((commit.to_string(), runner.to_string(), entry)) } -/// The `Cargo.toml` of every package that lost a source since `entry`. -fn lost(entry: &Sources, current: &Sources) -> BTreeSet { - let mut lost = BTreeSet::new(); - for gone in entry.keys().filter(|p| !current.contains_key(*p)) { - let manifest = Path::new(gone) - .ancestors() - .skip(1) - .map(|dir| dir.join("Cargo.toml").to_string_lossy().into_owned()) - .find(|manifest| current.contains_key(manifest)); - lost.extend(manifest); - } - lost +/// The `Cargo.toml` of the package `path` is in: the nearest one above it. +fn package(path: &str, current: &Sources) -> Option { + Path::new(path) + .ancestors() + .skip(1) + .map(|dir| dir.join("Cargo.toml").to_string_lossy().into_owned()) + .find(|manifest| current.contains_key(manifest)) } fn sources(root: &Path) -> Result { - let out = Command::new("git") - .args(["ls-files", "-s", "-z"]) - .current_dir(root) - .output() - .map_err(|e| format!("git ls-files: {e}"))?; - if !out.status.success() { - return Err(format!("git ls-files: {}", String::from_utf8_lossy(&out.stderr).trim())); - } - let mut paths = Vec::new(); - for entry in out.stdout.split(|b| *b == 0).filter(|e| !e.is_empty()) { - let entry = std::str::from_utf8(entry).map_err(|_| "git tracks a name that is not UTF-8")?; - let (meta, path) = - entry.split_once('\t').ok_or_else(|| format!("git ls-files -s printed {entry:?}"))?; + let mut sources = Sources::new(); + for path in crate::sysroot::tracked_files(root, &[])? { + let file = root.join(&path); // A gitlink names a commit, not bytes: what is under it keeps its // checkout's date, which no entry is newer than. - if meta.starts_with("160000 ") { + if file.is_dir() { continue; } - if path.contains('\n') { - return Err(format!("git tracks {path:?}, which `--stdin-paths` cannot be given")); - } - paths.push(path.to_string()); - } - let mut child = Command::new("git") - .args(["hash-object", "--no-filters", "--stdin-paths"]) - .current_dir(root) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .map_err(|e| format!("git hash-object: {e}"))?; - let mut stdin = child.stdin.take().expect("piped"); - let input: String = paths.iter().map(|p| format!("{p}\n")).collect(); - // Written beside the read: the ids fill the pipe before the paths are in. - let writer = std::thread::spawn(move || stdin.write_all(input.as_bytes())); - let out = child.wait_with_output().map_err(|e| format!("git hash-object: {e}"))?; - writer.join().expect("the writer").map_err(|e| format!("git hash-object's input: {e}"))?; - if !out.status.success() { - return Err(format!("git hash-object: {}", String::from_utf8_lossy(&out.stderr).trim())); + let bytes = fs::read(&file).map_err(|e| format!("read {path}: {e}"))?; + sources.insert(path, format!("{:x}", Sha256::digest(bytes))); } - let blobs: Vec = String::from_utf8_lossy(&out.stdout).lines().map(String::from).collect(); - if blobs.len() != paths.len() { - return Err(format!("git hash-object gave {} ids for {} paths", blobs.len(), paths.len())); - } - Ok(paths.into_iter().zip(blobs).collect()) + Ok(sources) } /// Every `target` directory under `root`, none inside another. @@ -364,23 +298,46 @@ fn modified(path: &Path) -> std::io::Result { mod tests { use super::*; use crate::gitfixture::{configure, sh}; + use std::process::{Command, Output}; use toyos_tmpdir::TempDir; + const RUNNER: &str = "macOS ARM64 macos15 20260928.1"; + fn write(root: &Path, path: &str, text: &str) { let path = root.join(path); fs::create_dir_all(path.parent().unwrap()).unwrap(); fs::write(path, text).unwrap(); } - /// `cargo build` in `root`, and each workspace crate with whether cargo - /// called it fresh. - fn build(root: &Path) -> BTreeMap { - let out = Command::new("cargo") + /// `files` committed to a new repository at `dir`. + fn origin(dir: &Path, files: &[(&str, &str)]) { + fs::create_dir_all(dir).unwrap(); + sh(dir, &["init", "-q", "-b", "main"]); + configure(dir); + for (path, text) in files { + write(dir, path, text); + } + sh(dir, &["add", "-A"]); + sh(dir, &["commit", "-qm", "files"]); + } + + fn crate_toml(name: &str, deps: &str) -> String { + format!("[package]\nname = \"{name}\"\nversion = \"0.1.0\"\nedition = \"2021\"\n\n[dependencies]\n{deps}") + } + + fn cargo_build(root: &Path) -> Output { + Command::new("cargo") .args(["build", "--offline", "--message-format=json"]) .current_dir(root) .env_remove("CARGO_TARGET_DIR") .output() - .expect("run cargo"); + .expect("run cargo") + } + + /// `cargo build` in `root`, and each workspace crate with whether cargo + /// called it fresh. + fn build(root: &Path) -> BTreeMap { + let out = cargo_build(root); assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr)); let mut fresh = BTreeMap::new(); for line in String::from_utf8_lossy(&out.stdout).lines() { @@ -405,6 +362,22 @@ mod tests { configure(dir); } + /// An entry at `writer`: a checkout of `origin` built cold and sealed. + fn entry(origin: &Path, writer: &Path) { + checkout(origin, writer); + let Start::Cold(cold) = open(writer, RUNNER, SystemTime::now()).unwrap().0 else { + panic!("a tree with no target is cold") + }; + build(writer); + seal(writer, &cold).unwrap(); + } + + /// The entry at `writer`, restored under a fresh checkout of `origin`. + fn restore(origin: &Path, writer: &Path, reader: &Path) { + checkout(origin, reader); + fs::rename(writer.join("target"), reader.join("target")).unwrap(); + } + /// The oracle is cargo and the program it builds: an entry serves what /// a cold build of the reader's tree would, recompiling exactly what /// changed and what depends on it — a changed source the checkout dated @@ -413,57 +386,41 @@ mod tests { #[test] fn an_entry_serves_exactly_the_sources_it_was_built_from() { let tmp = TempDir::new("cicache-entry"); - let origin = tmp.join("origin"); - fs::create_dir(&origin).unwrap(); - sh(&origin, &["init", "-q", "-b", "main"]); - configure(&origin); - write(&origin, ".gitignore", "target/\n"); - let members = "[workspace]\nmembers = [\"app\", \"leaf\", \"count\", \"idle\"]\nresolver = \"2\"\n"; - write(&origin, "Cargo.toml", members); - let package = |name: &str, deps: &str| { - let head = format!("[package]\nname = \"{name}\"\nversion = \"0.1.0\"\nedition = \"2021\"\n"); - format!("{head}\n[dependencies]\n{deps}") - }; - write(&origin, "leaf/Cargo.toml", &package("leaf", "")); - write(&origin, "leaf/src/lib.rs", "pub fn word() -> &'static str { \"one\" }\n"); - write(&origin, "count/Cargo.toml", &package("count", "")); let script = r#"fn main() { let files = std::fs::read_dir("src").unwrap().count(); println!("cargo:rustc-env=FILES={files}"); } "#; - write(&origin, "count/build.rs", script); - write(&origin, "count/src/lib.rs", "pub const FILES: &str = env!(\"FILES\");\n"); - write(&origin, "count/src/spare.txt", "read by nothing but the count\n"); - write(&origin, "idle/Cargo.toml", &package("idle", "")); - write(&origin, "idle/src/lib.rs", "pub fn idle() {}\n"); let deps = "leaf = { path = \"../leaf\" }\ncount = { path = \"../count\" }\n"; - write(&origin, "app/Cargo.toml", &package("app", deps)); - let main = "fn main() { print!(\"{} {}\", leaf::word(), count::FILES); }\n"; - write(&origin, "app/src/main.rs", main); - sh(&origin, &["add", "-A"]); - sh(&origin, &["commit", "-qm", "built"]); - + let source = tmp.join("origin"); + origin(&source, &[ + (".gitignore", "target/\n"), + ("Cargo.toml", "[workspace]\nmembers = [\"app\", \"leaf\", \"count\", \"idle\"]\nresolver = \"2\"\n"), + ("leaf/Cargo.toml", &crate_toml("leaf", "")), + ("leaf/src/lib.rs", "pub fn word() -> &'static str { \"one\" }\n"), + ("count/Cargo.toml", &crate_toml("count", "")), + ("count/build.rs", script), + ("count/src/lib.rs", "pub const FILES: &str = env!(\"FILES\");\n"), + ("count/src/spare.txt", "read by nothing but the count\n"), + ("idle/Cargo.toml", &crate_toml("idle", "")), + ("idle/src/lib.rs", "pub fn idle() {}\n"), + ("app/Cargo.toml", &crate_toml("app", deps)), + ("app/src/main.rs", "fn main() { print!(\"{} {}\", leaf::word(), count::FILES); }\n"), + ]); let writer = tmp.join("writer"); - checkout(&origin, &writer); - let Start::Cold(stamped) = open(&writer).unwrap().0 else { - panic!("a tree with no target is cold") - }; - build(&writer); + entry(&source, &writer); assert_eq!(run(&writer), "one 2"); - seal(&writer, &stamped).unwrap(); - write(&origin, "leaf/src/lib.rs", "pub fn word() -> &'static str { \"two\" }\n"); - sh(&origin, &["rm", "-q", "count/src/spare.txt"]); - sh(&origin, &["commit", "-qam", "read"]); + write(&source, "leaf/src/lib.rs", "pub fn word() -> &'static str { \"two\" }\n"); + sh(&source, &["rm", "-q", "count/src/spare.txt"]); + sh(&source, &["commit", "-qam", "read"]); let reader = tmp.join("reader"); - checkout(&origin, &reader); - fs::rename(writer.join("target"), reader.join("target")).unwrap(); + restore(&source, &writer, &reader); // What an mtime made up from history would say of a file committed // before the entry was built. date(&reader.join("leaf/src/lib.rs"), UNIX_EPOCH + Duration::from_secs(1)).unwrap(); - let said = open(&reader).unwrap(); + let said = open(&reader, RUNNER, SystemTime::now()).unwrap(); assert!(matches!(said.0, Start::Warm), "{}", said.1); assert!(!reader.join(MANIFEST).exists(), "a warm tree keeps no manifest"); let fresh = build(&reader); @@ -472,69 +429,111 @@ mod tests { assert_eq!(fresh, expected.into_iter().map(|(k, v)| (k.to_string(), v)).collect()); } + /// A file no dep-info names still decides a build: beside `src/x.rs`, a + /// new `src/x/mod.rs` is E0761 to a cold build, and so to a warm one. + #[test] + fn a_file_cargo_was_never_told_about_rebuilds_its_package() { + let tmp = TempDir::new("cicache-probe"); + let source = tmp.join("origin"); + origin(&source, &[ + (".gitignore", "target/\n"), + ("Cargo.toml", "[workspace]\nmembers = [\"probed\"]\nresolver = \"2\"\n"), + ("probed/Cargo.toml", &crate_toml("probed", "")), + ("probed/src/lib.rs", "mod x;\npub fn f() -> u8 { x::X }\n"), + ("probed/src/x.rs", "pub const X: u8 = 1;\n"), + ]); + let writer = tmp.join("writer"); + entry(&source, &writer); + + write(&source, "probed/src/x/mod.rs", "pub const X: u8 = 2;\n"); + sh(&source, &["add", "-A"]); + sh(&source, &["commit", "-qm", "probed"]); + let reader = tmp.join("reader"); + restore(&source, &writer, &reader); + assert!(matches!(open(&reader, RUNNER, SystemTime::now()).unwrap().0, Start::Warm)); + let out = cargo_build(&reader); + let said = String::from_utf8_lossy(&out.stdout); + assert!(!out.status.success() && said.contains("E0761"), "{said}"); + } + + /// One commit of `a.rs`, read cold. + fn cold_repo(tmp: &Path) -> Cold { + sh(tmp, &["init", "-q"]); + configure(tmp); + write(tmp, "a.rs", "a\n"); + sh(tmp, &["add", "-A"]); + sh(tmp, &["commit", "-qm", "a"]); + let Start::Cold(cold) = open(tmp, RUNNER, SystemTime::now()).unwrap().0 else { panic!("cold") }; + cold + } + /// Targets with no manifest beside them were built from nothing anyone can /// name; the driver's own is this job's. #[test] fn targets_restored_without_a_manifest_are_refused() { let tmp = TempDir::new("cicache-foreign"); - sh(&tmp, &["init", "-q"]); - configure(&tmp); - write(&tmp, "kernel/src/lib.rs", ""); - sh(&tmp, &["add", "-A"]); + cold_repo(&tmp); fs::create_dir_all(tmp.join(DRIVER)).unwrap(); - assert!(matches!(open(&tmp).unwrap().0, Start::Cold(_))); - fs::create_dir_all(tmp.join("kernel/target/debug")).unwrap(); - let refusal = open(&tmp).err().expect("a restored target with no manifest"); - assert!(refusal.contains("kernel/target"), "{refusal}"); + assert!(matches!(open(&tmp, RUNNER, SystemTime::now()).unwrap().0, Start::Cold(_))); + for target in ["kernel/target", "target/debug"] { + fs::create_dir_all(tmp.join(target)).unwrap(); + let refusal = open(&tmp, RUNNER, SystemTime::now()).err().expect("a target with no manifest"); + assert!(refusal.contains(target), "{refusal}"); + fs::remove_dir(tmp.join(target)).unwrap(); + } } /// A sealed tree is dated as built throughout, and a step that wrote a - /// tracked source is refused: the manifest would name bytes no build read. + /// tracked source is refused, even one that wrote its bytes back: the + /// manifest would name bytes no build read. #[test] fn a_seal_dates_every_target_and_refuses_a_written_source() { let tmp = TempDir::new("cicache-seal"); - sh(&tmp, &["init", "-q"]); - configure(&tmp); - write(&tmp, "a.rs", "a\n"); - sh(&tmp, &["add", "-A"]); - sh(&tmp, &["commit", "-qm", "a"]); - let Start::Cold(stamped) = open(&tmp).unwrap().0 else { panic!("cold") }; + let cold = cold_repo(&tmp); write(&tmp, "target/debug/deps/x", "x"); write(&tmp, "userland/target/y", "y"); - seal(&tmp, &stamped).unwrap(); + seal(&tmp, &cold).unwrap(); for file in ["target/debug/deps/x", "target/debug", "userland/target/y"] { assert_eq!(modified(&tmp.join(file)).unwrap(), built(), "{file}"); } fs::remove_file(tmp.join(MANIFEST)).unwrap(); - write(&tmp, "a.rs", "b\n"); - let refusal = seal(&tmp, &stamped).unwrap_err(); - assert!(refusal.contains("a.rs"), "{refusal}"); + for bytes in ["b\n", "a\n"] { + write(&tmp, "a.rs", bytes); + let refusal = seal(&tmp, &cold).unwrap_err(); + assert!(refusal.contains("a.rs"), "{bytes:?}: {refusal}"); + } } + /// Another image's entry is no entry: its targets go and the run is cold. #[test] - fn a_prune_keeps_this_runs_entry_on_main_and_nothing_else() { - let entry = - |id: u64, key: &str, scope: &str| serde_json::json!({"id": id, "key": key, "ref": scope}); - let ours = "host-sealed-Linux-7"; - let listing = |entries: Vec| { - serde_json::json!({"total_count": entries.len(), "actions_caches": entries}) - }; - let doc = listing(vec![ - entry(1, ours, "refs/heads/main"), - entry(2, "host-sealed-Linux-6", "refs/heads/main"), - entry(3, "host-sealed-Linux-5", "refs/pull/9/merge"), - entry(4, "host-36696295750", "refs/heads/main"), - ]); - let ids: Vec = doomed(&doc, ours).unwrap().into_iter().map(|(id, _)| id).collect(); - assert_eq!(ids, [2, 3, 4]); + fn an_entry_built_on_another_runner_is_deleted() { + let tmp = TempDir::new("cicache-image"); + let cold = cold_repo(&tmp); + write(&tmp, "target/debug/x", "x"); + write(&tmp, "kernel/target/y", "y"); + seal(&tmp, &cold).unwrap(); + let (start, said) = open(&tmp, "macOS ARM64 macos15 20261005.1", SystemTime::now()).unwrap(); + assert!(matches!(start, Start::Cold(_)), "{said}"); + assert!(!tmp.join("target/debug").exists() && !tmp.join("kernel/target").exists(), "{said}"); + } - let unsaved = listing(vec![ - entry(1, ours, "refs/pull/9/merge"), - entry(2, "host-sealed-Linux-6", "refs/heads/main"), - ]); - assert!(doomed(&unsaved, ours).unwrap_err().contains("not among main's")); - let page = [entry(1, ours, "refs/heads/main")]; - let paged = serde_json::json!({"total_count": 101, "actions_caches": page}); - assert!(doomed(&paged, ours).is_err()); + /// A source dated now is newer than the entry only on a clock that reads + /// after [`built`]. + #[test] + fn a_reader_whose_clock_is_not_after_the_entry_is_refused() { + let tmp = TempDir::new("cicache-clock"); + let cold = cold_repo(&tmp); + write(&tmp, "target/debug/x", "x"); + seal(&tmp, &cold).unwrap(); + let refusal = open(&tmp, RUNNER, built()).err().expect("a clock at the entry's date"); + assert!(refusal.contains("clock"), "{refusal}"); + } + + #[test] + fn a_driver_built_in_any_other_target_is_refused() { + let tmp = TempDir::new("cicache-driver"); + fs::create_dir_all(tmp.join(DRIVER)).unwrap(); + let refusal = read(&tmp).err().expect("a test binary is no driver"); + assert!(refusal.contains("the driver runs from"), "{refusal}"); } } diff --git a/src/identity.rs b/src/identity.rs index c366ad718eb..574c17b5ce4 100644 --- a/src/identity.rs +++ b/src/identity.rs @@ -1,9 +1,8 @@ //! What a source file is to a build: its token stream, not its text. //! -//! **One definition, read by every question of the form "did this source -//! change what gets built"**: the key a sysroot is filed under. A comment -//! — a doc comment included — and the whitespace around tokens change no item, -//! no layout and no code, so a change made only of them is no new sysroot. +//! A comment — a doc comment included — and the whitespace around tokens change +//! no item, no layout and no code, so a change made only of them is no new +//! sysroot. //! //! A `.rs` file is lexed just far enough to find its comments: every string, //! raw string and character literal is kept byte for byte, every comment and diff --git a/src/licence.rs b/src/licence.rs index 7418b70e7a6..c309857aa25 100644 --- a/src/licence.rs +++ b/src/licence.rs @@ -1115,31 +1115,19 @@ fn metadata( serde_json::from_slice(&out).map_err(|e| format!("cargo metadata printed no JSON: {e}")) } -/// The fork's `library/` at the commit this tree pins, in `rust/`, where its -/// manifests' paths to `toyos` and `toyos-abi` land in this tree. A runner's -/// `rust/` was never initialised, so it fetches that commit's `library/` alone, -/// with no other blob of the fork: a checkout that sparse would be all a -/// developer's later build found. +/// The fork's `library/`, checked out at the commit this tree pins. A checkout +/// whose `rust/` was never initialised — a CI runner's — fetches that commit +/// alone. fn std_library(root: &Path) -> Result { let fork = crate::sysroot::fork_checkout(root); - if fork.join("library/Cargo.toml").exists() { - return Ok(fork.join("library")); - } - if !crate::ci::on_runner() { - return Err(format!("{} holds no fork checkout; any `cargo run` makes one", fork.display())); - } - let pin = crate::sync::git(root, &["rev-parse", ":rust"])?; - let url = crate::sync::git(root, &["config", "-f", ".gitmodules", "submodule.rust.url"])?; - for args in [ - &["init", "-q"][..], - &["remote", "add", "origin", &url], - &["fetch", "-q", "--depth", "1", "--filter=blob:none", "origin", &pin], - &["sparse-checkout", "set", "library"], - &["checkout", "-q", "--detach", &pin], - ] { - crate::sync::git(&fork, args)?; - } - println!("the fork's library/ at {pin}, fetched"); + if !fork.join("library/Cargo.toml").exists() { + run( + Command::new("git") + .args(["submodule", "update", "--init", "--depth", "1", "rust"]) + .current_dir(root), + "git submodule update --init --depth 1 rust", + )?; + } Ok(fork.join("library")) } From 02fa5c511859658b05457cd58042474524ee5a84 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 14:27:49 +0200 Subject: [PATCH 08/20] Review round 2: the runner is read by one function a test reaches, and code run at build time runs again on every read The runner-variable list in `read` was reached by no test: dropping `ImageVersion` from it stayed green. `runner` now builds the runner from a lookup, `read` hands it the environment, and `an_entry_built_on_another_runner_is_deleted` builds both runners with it, from environments that differ in one variable, for each of the four. It also refuses an environment missing one, so an unset variable is never defaulted. The warm read's trust in cargo for what a build reads outside its package is removed rather than recorded. Every package with a build script or a proc macro has every file dated now on every warm read, so cargo reruns the script and rebuilds the macro and what expands it, whatever either reads. A package is one when its manifest has `build.rs` beside it and no `build = false`, a `build` key naming a script, `proc-macro` or `proc_macro`, or a `proc-macro` crate type. In the host job that is userland/calc alone, whose font already sits in the root package that nearly every landing changes. - `code_run_at_build_time_runs_again_on_every_read` is the deleted issue's exit: a build script and a proc macro read another package's file and never say so, and a warm read after a change to that file prints what a cold build prints. - `every_spelling_of_code_run_at_build_time_is_found` holds each spelling. - The oracle test gains `gone`, a package that lost a file nothing read. `count`'s build script now reruns on every read, which would otherwise hide a removed file's package dating from every test. What a warm run still trusts cargo for is a file only a flag names, a linker script in another package: filed as issues/build/a-warm-host-run-never-relinks-for-a-file-only-a-flag-names.md. Removed: SEALED's doc line, which restated the workflows' key, and the guest cache issue's restatement of the host cache's design. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- ...er-relinks-for-a-file-only-a-flag-names.md | 19 ++ ...-what-a-build-reads-outside-its-package.md | 23 --- ...and-its-writer-restores-before-it-saves.md | 4 - src/cicache.rs | 186 ++++++++++++++---- 4 files changed, 171 insertions(+), 61 deletions(-) create mode 100644 issues/build/a-warm-host-run-never-relinks-for-a-file-only-a-flag-names.md delete mode 100644 issues/build/a-warm-host-run-trusts-cargo-for-what-a-build-reads-outside-its-package.md diff --git a/issues/build/a-warm-host-run-never-relinks-for-a-file-only-a-flag-names.md b/issues/build/a-warm-host-run-never-relinks-for-a-file-only-a-flag-names.md new file mode 100644 index 00000000000..1abca38f0bb --- /dev/null +++ b/issues/build/a-warm-host-run-never-relinks-for-a-file-only-a-flag-names.md @@ -0,0 +1,19 @@ +--- +status: open +kind: tooling +opened: 2026-10-01 +--- + +# A warm host run never relinks for a file only a flag names + +A file that reaches a build only through a flag, a linker script named by +`-Clink-arg=-T…` in a `.cargo/config.toml` or in `RUSTFLAGS`, is read by the +linker, and cargo compares the flag, never the file. When that file sits +outside the package that links with it and changes alone, a warm `host` run +(`src/cicache.rs`) keeps the old link where a cold run makes a new one. + +No flag the host job passes names a file: the tracked `.cargo/config.toml` +files pass none, and its steps set no `RUSTFLAGS`. + +Done when a warm read refuses a flag that names a tracked file, or dates whole +the package that links with it, with a test. diff --git a/issues/build/a-warm-host-run-trusts-cargo-for-what-a-build-reads-outside-its-package.md b/issues/build/a-warm-host-run-trusts-cargo-for-what-a-build-reads-outside-its-package.md deleted file mode 100644 index ed51e76440d..00000000000 --- a/issues/build/a-warm-host-run-trusts-cargo-for-what-a-build-reads-outside-its-package.md +++ /dev/null @@ -1,23 +0,0 @@ ---- -status: open -kind: tooling -opened: 2026-10-01 ---- - -# A warm host run trusts cargo for what a build reads outside its package - -The merge queue's `host` run reads main's cache entry by content -(`src/cicache.rs`): every file of a package that changed is dated now, so -whatever a build reads inside its own package rebuilds it. An input outside -that package reaches a warm run only through what cargo is told, a build -script's `rerun-if-*` lines and rustc's dep-info. A path build script or proc -macro that reads another package's file, an environment variable or a tool -without declaring it passes stale there, where before every path crate -recompiled; only the cold nightly finds it, after main has moved. - -None does in the host job today: its one path build script, `userland/calc`'s, -declares the font it reads from `assets/`, and no path crate is a proc macro. - -Done when a test in `src/cicache.rs` whose path build script reads another -package's file without declaring it, read warm after a change to that file, -sees the build script run again. diff --git a/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md b/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md index 9ff10d4df3e..fce47f8f41d 100644 --- a/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md +++ b/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md @@ -13,8 +13,4 @@ host entry in the repository's 10 GB). And every guest job restores its targets under a checkout that dated every source at the checkout, so cargo calls every path crate in them stale. -The host cache stopped both (`src/cicache.rs`): an entry is sealed only by a -run that restored nothing, carries the hash of every tracked file, and its -reader dates sources by content. - Done when the guest entry is written cold, read by content, and bounded. diff --git a/src/cicache.rs b/src/cicache.rs index b3ced404b2f..049c2b9549c 100644 --- a/src/cicache.rs +++ b/src/cicache.rs @@ -13,11 +13,9 @@ //! //! **A package with a file changed, added or removed has every file dated //! now.** Cargo is told only what a build read; a file rustc probed for and did -//! not find (`src/x/mod.rs` beside `src/x.rs`), or one a build script read -//! without naming it, is still its package's. What a build reads outside its -//! own package without telling cargo, a warm run trusts as cargo's own -//! incremental build does: -//! `issues/build/a-warm-host-run-trusts-cargo-for-what-a-build-reads-outside-its-package.md`. +//! not find (`src/x/mod.rs` beside `src/x.rs`) is still its package's. **So +//! has every package with a build script or a proc macro, on every read**: +//! what code run at build time reads, cargo knows only if that code says so. //! //! **An entry built on another runner image is deleted, and the run is cold**: //! the image's linker and C compiler made its units, and cargo's fingerprint @@ -44,7 +42,6 @@ use sha2::{Digest, Sha256}; pub const MANIFEST: &str = "target/ci-sources"; pub const DRIVER: &str = "target/ci-driver"; -/// A sealed entry's key is this, `${{ runner.os }}-${{ runner.arch }}-${{ github.run_id }}`. pub const SEALED: &str = "host-sealed-"; /// 2001-09-09T01:46:40Z: older than any build, so a file dated so is never @@ -83,12 +80,16 @@ pub fn read(root: &Path) -> Result<(Start, String), String> { exe.display() )); } - let runner = ["RUNNER_OS", "RUNNER_ARCH", "ImageOS", "ImageVersion"] + open(root, &runner(|name| std::env::var(name).ok())?, SystemTime::now()) +} + +/// The runner, as the variables a hosted runner sets name it: its OS, its +/// architecture and its image. +fn runner(var: impl Fn(&str) -> Option) -> Result { + let values = ["RUNNER_OS", "RUNNER_ARCH", "ImageOS", "ImageVersion"] .iter() - .map(|name| std::env::var(name).map_err(|_| format!("{name} is unset: a hosted runner sets it"))) - .collect::, _>>()? - .join(" "); - open(root, &runner, SystemTime::now()) + .map(|name| var(name).ok_or_else(|| format!("{name} is unset: a hosted runner sets it"))); + Ok(values.collect::, _>>()?.join(" ")) } fn open(root: &Path, runner: &str, now: SystemTime) -> Result<(Start, String), String> { @@ -120,13 +121,21 @@ fn open(root: &Path, runner: &str, now: SystemTime) -> Result<(Start, String), S if now <= built() { return Err(format!("this runner's clock reads {now:?}, which no change dated now is newer than")); } - let dirty: BTreeSet> = current + let mut dirty: BTreeSet> = current .iter() .filter(|(path, hash)| entry.get(*path) != Some(*hash)) .map(|(path, _)| path) .chain(entry.keys().filter(|path| !current.contains_key(*path))) .map(|path| package(path, ¤t)) .collect(); + let packages = dirty.len(); + let mut build_time = 0; + for manifest in current.keys().filter(|path| Path::new(path).ends_with("Cargo.toml")) { + if runs_at_build(root, manifest, ¤t)? { + build_time += 1; + dirty.insert(Some(manifest.clone())); + } + } let mut same = 0; for (path, hash) in ¤t { let fresh = entry.get(path) == Some(hash) && !dirty.contains(&package(path, ¤t)); @@ -140,9 +149,9 @@ fn open(root: &Path, runner: &str, now: SystemTime) -> Result<(Start, String), S Start::Warm, format!( "built from {commit}: {same} of {} sources dated as built; {changed} changed, {added} \ - added, {removed} removed, in {} packages", + added, {removed} removed, in {packages} packages; {build_time} packages run code at \ + build time", current.len(), - dirty.len() ), )) } @@ -201,6 +210,23 @@ fn parse(text: &str) -> Result<(String, String, Sources), String> { Ok((commit.to_string(), runner.to_string(), entry)) } +/// Whether the package of `manifest` has a build script or is a proc macro, in +/// every spelling cargo accepts. +fn runs_at_build(root: &Path, manifest: &str, current: &Sources) -> Result { + let text = fs::read_to_string(root.join(manifest)).map_err(|e| format!("read {manifest}: {e}"))?; + let doc: toml::Value = text.parse().map_err(|e| format!("{manifest}: {e}"))?; + let lib = |key: &str, alias: &str| doc.get("lib").and_then(|lib| lib.get(key).or_else(|| lib.get(alias))); + let script = match doc.get("package").and_then(|package| package.get("build")) { + None => current.contains_key(&*Path::new(manifest).with_file_name("build.rs").to_string_lossy()), + Some(build) => build.as_bool() != Some(false), + }; + let proc_macro = lib("proc-macro", "proc_macro").and_then(toml::Value::as_bool) == Some(true) + || lib("crate-type", "crate_type") + .and_then(toml::Value::as_array) + .is_some_and(|types| types.iter().any(|t| t.as_str() == Some("proc-macro"))); + Ok(script || proc_macro) +} + /// The `Cargo.toml` of the package `path` is in: the nearest one above it. fn package(path: &str, current: &Sources) -> Option { Path::new(path) @@ -381,8 +407,8 @@ mod tests { /// The oracle is cargo and the program it builds: an entry serves what /// a cold build of the reader's tree would, recompiling exactly what /// changed and what depends on it — a changed source the checkout dated - /// older than the entry's build included, and a build script whose package - /// lost a file it never named. + /// older than the entry's build included, a build script whose package + /// lost a file it never named, and a package that lost a file nothing read. #[test] fn an_entry_serves_exactly_the_sources_it_was_built_from() { let tmp = TempDir::new("cicache-entry"); @@ -395,7 +421,7 @@ mod tests { let source = tmp.join("origin"); origin(&source, &[ (".gitignore", "target/\n"), - ("Cargo.toml", "[workspace]\nmembers = [\"app\", \"leaf\", \"count\", \"idle\"]\nresolver = \"2\"\n"), + ("Cargo.toml", "[workspace]\nmembers = [\"app\", \"leaf\", \"count\", \"idle\", \"gone\"]\nresolver = \"2\"\n"), ("leaf/Cargo.toml", &crate_toml("leaf", "")), ("leaf/src/lib.rs", "pub fn word() -> &'static str { \"one\" }\n"), ("count/Cargo.toml", &crate_toml("count", "")), @@ -404,6 +430,9 @@ mod tests { ("count/src/spare.txt", "read by nothing but the count\n"), ("idle/Cargo.toml", &crate_toml("idle", "")), ("idle/src/lib.rs", "pub fn idle() {}\n"), + ("gone/Cargo.toml", &crate_toml("gone", "")), + ("gone/src/lib.rs", "pub fn gone() {}\n"), + ("gone/notes.txt", "read by nothing\n"), ("app/Cargo.toml", &crate_toml("app", deps)), ("app/src/main.rs", "fn main() { print!(\"{} {}\", leaf::word(), count::FILES); }\n"), ]); @@ -412,7 +441,7 @@ mod tests { assert_eq!(run(&writer), "one 2"); write(&source, "leaf/src/lib.rs", "pub fn word() -> &'static str { \"two\" }\n"); - sh(&source, &["rm", "-q", "count/src/spare.txt"]); + sh(&source, &["rm", "-q", "count/src/spare.txt", "gone/notes.txt"]); sh(&source, &["commit", "-qam", "read"]); let reader = tmp.join("reader"); restore(&source, &writer, &reader); @@ -425,7 +454,7 @@ mod tests { assert!(!reader.join(MANIFEST).exists(), "a warm tree keeps no manifest"); let fresh = build(&reader); assert_eq!(run(&reader), "two 1"); - let expected = [("app", false), ("count", false), ("idle", true), ("leaf", false)]; + let expected = [("app", false), ("count", false), ("gone", false), ("idle", true), ("leaf", false)]; assert_eq!(fresh, expected.into_iter().map(|(k, v)| (k.to_string(), v)).collect()); } @@ -456,14 +485,83 @@ mod tests { assert!(!out.status.success() && said.contains("E0761"), "{said}"); } - /// One commit of `a.rs`, read cold. - fn cold_repo(tmp: &Path) -> Cold { + /// A build script and a proc macro that read another package's file and + /// never say so are run again on a read, as a cold build runs them: `app`'s + /// script reads `word.txt`, and so does `mac`, expanded in `said`. + #[test] + fn code_run_at_build_time_runs_again_on_every_read() { + let tmp = TempDir::new("cicache-buildtime"); + let script = "fn main() { + let word = std::fs::read_to_string(\"../word.txt\").unwrap(); + println!(\"cargo:rustc-env=WORD={}\", word.trim()); +} +"; + let mac = r#"#[proc_macro] +pub fn word(_: proc_macro::TokenStream) -> proc_macro::TokenStream { + let dir = std::env::var("CARGO_MANIFEST_DIR").unwrap(); + let word = std::fs::read_to_string(format!("{dir}/../word.txt")).unwrap(); + format!("{:?}", word.trim()).parse().unwrap() +} +"#; + let source = tmp.join("origin"); + origin(&source, &[ + (".gitignore", "target/\n"), + ("Cargo.toml", "[workspace]\nmembers = [\"app\", \"mac\", \"said\"]\nresolver = \"2\"\n"), + ("word.txt", "one\n"), + ("mac/Cargo.toml", &format!("{}\n[lib]\nproc-macro = true\n", crate_toml("mac", ""))), + ("mac/src/lib.rs", mac), + ("said/Cargo.toml", &crate_toml("said", "mac = { path = \"../mac\" }\n")), + ("said/src/lib.rs", "pub const WORD: &str = mac::word!();\n"), + ("app/Cargo.toml", &crate_toml("app", "said = { path = \"../said\" }\n")), + ("app/build.rs", script), + ("app/src/main.rs", "fn main() { print!(\"{} {}\", env!(\"WORD\"), said::WORD); }\n"), + ]); + let writer = tmp.join("writer"); + entry(&source, &writer); + assert_eq!(run(&writer), "one one"); + + write(&source, "word.txt", "two\n"); + sh(&source, &["commit", "-qam", "word"]); + let reader = tmp.join("reader"); + restore(&source, &writer, &reader); + assert!(matches!(open(&reader, RUNNER, SystemTime::now()).unwrap().0, Start::Warm)); + build(&reader); + assert_eq!(run(&reader), "two two"); + } + + /// Each way a manifest can declare a build script or a proc macro. + #[test] + fn every_spelling_of_code_run_at_build_time_is_found() { + let tmp = TempDir::new("cicache-spellings"); + let cases = [ + ("", false, false), + ("", true, true), + ("build = false\n", true, false), + ("build = \"gen.rs\"\n", false, true), + ("[lib]\nproc-macro = true\n", false, true), + ("[lib]\nproc_macro = true\n", false, true), + ("[lib]\ncrate-type = [\"proc-macro\"]\n", false, true), + ("[lib]\ncrate_type = [\"proc-macro\"]\n", false, true), + ("[lib]\ncrate-type = [\"rlib\"]\n", false, false), + ]; + for (keys, script, expected) in cases { + write(&tmp, "Cargo.toml", &format!("[package]\nname = \"p\"\n{keys}")); + let mut current = Sources::from([("Cargo.toml".to_string(), String::new())]); + if script { + current.insert("build.rs".into(), String::new()); + } + assert_eq!(runs_at_build(&tmp, "Cargo.toml", ¤t), Ok(expected), "{keys:?}, build.rs: {script}"); + } + } + + /// One commit of `a.rs`, read cold on `runner`. + fn cold_repo(tmp: &Path, runner: &str) -> Cold { sh(tmp, &["init", "-q"]); configure(tmp); write(tmp, "a.rs", "a\n"); sh(tmp, &["add", "-A"]); sh(tmp, &["commit", "-qm", "a"]); - let Start::Cold(cold) = open(tmp, RUNNER, SystemTime::now()).unwrap().0 else { panic!("cold") }; + let Start::Cold(cold) = open(tmp, runner, SystemTime::now()).unwrap().0 else { panic!("cold") }; cold } @@ -472,7 +570,7 @@ mod tests { #[test] fn targets_restored_without_a_manifest_are_refused() { let tmp = TempDir::new("cicache-foreign"); - cold_repo(&tmp); + cold_repo(&tmp, RUNNER); fs::create_dir_all(tmp.join(DRIVER)).unwrap(); assert!(matches!(open(&tmp, RUNNER, SystemTime::now()).unwrap().0, Start::Cold(_))); for target in ["kernel/target", "target/debug"] { @@ -489,7 +587,7 @@ mod tests { #[test] fn a_seal_dates_every_target_and_refuses_a_written_source() { let tmp = TempDir::new("cicache-seal"); - let cold = cold_repo(&tmp); + let cold = cold_repo(&tmp, RUNNER); write(&tmp, "target/debug/deps/x", "x"); write(&tmp, "userland/target/y", "y"); seal(&tmp, &cold).unwrap(); @@ -504,17 +602,37 @@ mod tests { } } - /// Another image's entry is no entry: its targets go and the run is cold. + /// The runner [`read`] names in an environment holding `vars`. + fn runner_in(vars: &BTreeMap<&str, &str>) -> Result { + runner(|name| vars.get(name).map(|value| value.to_string())) + } + + /// Another runner's entry is no entry: its targets go and the run is cold, + /// whichever variable that names a runner differs, and a runner without + /// one of them names none. #[test] fn an_entry_built_on_another_runner_is_deleted() { - let tmp = TempDir::new("cicache-image"); - let cold = cold_repo(&tmp); - write(&tmp, "target/debug/x", "x"); - write(&tmp, "kernel/target/y", "y"); - seal(&tmp, &cold).unwrap(); - let (start, said) = open(&tmp, "macOS ARM64 macos15 20261005.1", SystemTime::now()).unwrap(); - assert!(matches!(start, Start::Cold(_)), "{said}"); - assert!(!tmp.join("target/debug").exists() && !tmp.join("kernel/target").exists(), "{said}"); + let image = BTreeMap::from([ + ("RUNNER_OS", "macOS"), + ("RUNNER_ARCH", "ARM64"), + ("ImageOS", "macos15"), + ("ImageVersion", "20260928.1"), + ]); + for name in image.keys() { + let tmp = TempDir::new("cicache-image"); + let cold = cold_repo(&tmp, &runner_in(&image).unwrap()); + write(&tmp, "target/debug/x", "x"); + write(&tmp, "kernel/target/y", "y"); + seal(&tmp, &cold).unwrap(); + let mut other = image.clone(); + other.insert(name, "another"); + let (start, said) = open(&tmp, &runner_in(&other).unwrap(), SystemTime::now()).unwrap(); + assert!(matches!(start, Start::Cold(_)), "{name}: {said}"); + assert!(!tmp.join("target/debug").exists() && !tmp.join("kernel/target").exists(), "{name}: {said}"); + other.remove(name); + let refusal = runner_in(&other).expect_err("a runner without a variable"); + assert!(refusal.contains(name), "{refusal}"); + } } /// A source dated now is newer than the entry only on a clock that reads @@ -522,7 +640,7 @@ mod tests { #[test] fn a_reader_whose_clock_is_not_after_the_entry_is_refused() { let tmp = TempDir::new("cicache-clock"); - let cold = cold_repo(&tmp); + let cold = cold_repo(&tmp, RUNNER); write(&tmp, "target/debug/x", "x"); seal(&tmp, &cold).unwrap(); let refusal = open(&tmp, RUNNER, built()).err().expect("a clock at the entry's date"); From db4654fb62e0454eec72600d16af4107952c251d Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 15:11:04 +0200 Subject: [PATCH 09/20] Each host step is `cargo run -- --ci host`, and the driver says which job carries the cache `src/CLAUDE.md` says a workflow step runs `cargo run -- --ci ` and nothing else. Both host steps passed `--target-dir target/ci-driver`, and nightly's save was guarded by `hashFiles('target/ci-sources')`. - The driver's own target is now the `CARGO_TARGET_DIR` of ci.yml's and nightly.yml's `host` jobs, so each step is the bare command. Cargo hands that variable to the program it runs, so `host` takes it out of its environment before any step and no step builds in the driver's target. - `cicache::carried` asks where the driver runs from: a job that builds it in `target/ci-driver` carries the cache, and any other runs the host suite as a developer's tree does. That reconciles #667's `cargo run -- --ci host` in nightly's `portability-macos`, which follows `--build-only`: its driver is in `target/debug`, and `read` would have refused the targets `--build-only` left, which no manifest describes. - No runtime refusal of a driver built elsewhere is left. Instead `each_cache_has_one_writer` holds every job that restores or saves the host cache to that `CARGO_TARGET_DIR` at job level, so a workflow that drops it reds in the pull request that drops it. - Nightly's save has no guard. Its job restores nothing, so its run is cold, and a cold run's last step is the seal: the job is green only with a sealed tree, and a save follows only a green job. `each_cache_has_one_writer` reds on a host writer that restores anything, in place of the guard it checked. - The seal names the runner it records, so a cold run's log shows what a warm reader will compare against. - `MANIFEST` is private: the guard was its one reader outside the module. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- .github/workflows/ci.yml | 6 +++- .github/workflows/nightly.yml | 15 ++++---- src/ci.rs | 64 ++++++++++++++++++++++++----------- src/cicache.rs | 53 +++++++++++++++++------------ 4 files changed, 91 insertions(+), 47 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 802cb7a4d17..6d45466c6a9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,10 @@ jobs: if: github.event_name == 'merge_group' || github.event.pull_request.draft == false runs-on: ubuntu-24.04 timeout-minutes: 45 + # The driver's own target, and no step's: it says this job carries the + # host cache (src/cicache.rs). + env: + CARGO_TARGET_DIR: target/ci-driver steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 @@ -37,4 +41,4 @@ jobs: key: host-sealed-${{ runner.os }}-${{ runner.arch }}-${{ github.run_id }} restore-keys: host-sealed-${{ runner.os }}-${{ runner.arch }}- - - run: cargo run --target-dir target/ci-driver -- --ci host + - run: cargo run -- --ci host diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 7bcc55fa0af..1c93b0fabb1 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -17,22 +17,25 @@ concurrency: jobs: # The host cache's writer: only main's entries are readable from every - # branch, and it restores nothing, so its tree is one cold build's and - # src/cicache.rs seals it. On a branch it would only repeat that branch's - # pull request. + # branch. It restores nothing, so its run is cold, and a cold run is green + # only once src/cicache.rs has sealed its tree. On a branch it would only + # repeat that branch's pull request. host: if: github.ref == 'refs/heads/main' runs-on: ubuntu-24.04 timeout-minutes: 90 + # The driver's own target, and no step's: it says this job carries the + # host cache (src/cicache.rs). + env: + CARGO_TARGET_DIR: target/ci-driver steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 with: fetch-depth: 0 - - run: cargo run --target-dir target/ci-driver -- --ci host + - run: cargo run -- --ci host - - if: hashFiles('target/ci-sources') != '' - uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + - uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 with: path: | ~/.cargo/registry/index diff --git a/src/ci.rs b/src/ci.rs index 851c0477235..240fb05cac9 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -466,9 +466,9 @@ fn run_control(root: &Path, control: &Control) -> Result { /// clippy shape (`src/clippy.rs`). Userland and the SDK are tested against the /// host triple for the same reason. /// -/// On a runner the restored cache entry is read before any step and a run that -/// starts cold seals its tree after the last ([`cicache`]); a developer's tree -/// keeps the dates its edits gave it. +/// In a job that carries the cache ([`cicache::carried`]) the restored entry is +/// read before any step and a run that starts cold seals its tree after the +/// last; a developer's tree keeps the dates its edits gave it. fn host(root: &Path) -> Vec { let tmp = toyos_tmpdir::TempDir::new("ci-host"); let short = Path::new(toyos_tmpdir::SHORT_BASE); @@ -479,7 +479,10 @@ fn host(root: &Path) -> Vec { let host_triple = crate::toolchain::host_triple(); let mut steps = Vec::new(); let mut cold = None; - if on_runner() { + if cicache::carried(root, &std::env::current_exe().expect("the driver's own path")) { + // The job's `CARGO_TARGET_DIR` names the driver's target, and no step + // builds there. + std::env::remove_var("CARGO_TARGET_DIR"); // No incremental state in an entry: it is most of an entry's bytes, // and after a read by content it helps only a crate whose bytes // changed. @@ -993,32 +996,55 @@ mod tests { assert_eq!(seen, 3, "ci.yml, nightly.yml and publish.yml"); } + /// Each job of a workflow: its name and its lines. + fn jobs(text: &str) -> Vec<(&str, Vec<&str>)> { + let mut jobs: Vec<(&str, Vec<&str>)> = Vec::new(); + for line in text.split_once("\njobs:\n").map_or("", |(_, jobs)| jobs).lines() { + match line.strip_prefix(" ").and_then(|l| l.strip_suffix(':')) { + Some(name) if !name.starts_with([' ', '#']) => jobs.push((name, Vec::new())), + _ => jobs.last_mut().into_iter().for_each(|(_, lines)| lines.push(line)), + } + } + jobs + } + /// Exactly one job writes each cache, on the nightly, so what a pull request - /// restores is one run's tree and never a race between two writers. The - /// host cache's saves only a sealed tree, the one that holds a manifest - /// ([`cicache`]). + /// restores is one run's tree and never a race between two writers. A job + /// that restores or saves the host cache builds the driver in + /// [`cicache::DRIVER`], so it carries the cache, and the one that saves it + /// restores nothing: its run is cold, and a cold run is green only once its + /// tree is sealed ([`cicache`]). #[test] fn each_cache_has_one_writer() { let dir = repo_root().join(".github/workflows"); + let carries = format!(" CARGO_TARGET_DIR: {}", cicache::DRIVER); let mut writers = Vec::new(); for entry in std::fs::read_dir(&dir).expect(".github/workflows is readable").flatten() { let text = std::fs::read_to_string(entry.path()).expect("a readable workflow"); let name = entry.file_name().to_string_lossy().into_owned(); assert!(!text.contains("actions/cache@"), "{name}: the combined action saves too"); - let lines: Vec<&str> = text.lines().collect(); - for (at, line) in lines.iter().enumerate() { - if line.contains("actions/cache/save@") { - let key = lines[at..] - .iter() - .find_map(|l| l.trim_start().strip_prefix("key: ")) - .expect("a save names its key"); - let prefix = key.split('$').next().unwrap_or("").to_string(); + for (job, lines) in jobs(&text) { + let caches: Vec<(bool, String)> = lines + .iter() + .enumerate() + .filter(|(_, l)| l.contains("actions/cache/restore@") || l.contains("actions/cache/save@")) + .map(|(at, l)| { + let key = lines[at..] + .iter() + .find_map(|l| l.trim_start().strip_prefix("key: ")) + .expect("a cache step names its key"); + (l.contains("actions/cache/save@"), key.split('$').next().unwrap_or("").to_string()) + }) + .collect(); + for (save, prefix) in &caches { if prefix.starts_with("host-") { - assert_eq!(prefix, cicache::SEALED, "{name}"); - let manifest = format!("hashFiles('{}') != ''", cicache::MANIFEST); - assert!(lines[at - 1].contains(&manifest), "{name}: a host save without `{manifest}`"); + assert_eq!(prefix, cicache::SEALED, "{name} {job}"); + assert!(lines.contains(&carries.as_str()), "{name} {job}: the host cache without `{}`", carries.trim()); + assert!(!save || caches.iter().all(|(s, _)| *s), "{name} {job}: the host cache's writer restores"); + } + if *save { + writers.push((name.clone(), prefix.clone())); } - writers.push((name.clone(), prefix)); } } } diff --git a/src/cicache.rs b/src/cicache.rs index 049c2b9549c..ad4d5afd836 100644 --- a/src/cicache.rs +++ b/src/cicache.rs @@ -28,9 +28,11 @@ //! the manifest it reads, so a warm tree carries none and is never saved, and //! targets restored without one are refused. //! -//! **The driver is built in [`DRIVER`]**: cargo builds it before this runs, so -//! its path crates are compiled again every time, and in the steps' target that -//! would make every crate depending on them stale. +//! **A job carries the cache when its workflow builds the driver in +//! [`DRIVER`]** ([`carried`]): cargo builds the driver before this runs, so its +//! path crates are compiled again every time, and in the steps' target that +//! would make every crate depending on them stale. A job that builds it +//! anywhere else runs as a developer's tree does. use std::collections::{BTreeMap, BTreeSet}; use std::fs; @@ -40,7 +42,7 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; use sha2::{Digest, Sha256}; -pub const MANIFEST: &str = "target/ci-sources"; +const MANIFEST: &str = "target/ci-sources"; pub const DRIVER: &str = "target/ci-driver"; pub const SEALED: &str = "host-sealed-"; @@ -67,19 +69,20 @@ pub struct Cold { sources: Sources, } -/// Before the first step: the entry restored here, read by content. -pub fn read(root: &Path) -> Result<(Start, String), String> { - let exe = std::env::current_exe() - .and_then(fs::canonicalize) - .map_err(|e| format!("the driver's own path: {e}"))?; - let driver = fs::canonicalize(root.join(DRIVER)).map_err(|e| format!("{DRIVER}: {e}"))?; - if !exe.starts_with(&driver) { - return Err(format!( - "the driver runs from {}: a workflow builds it with `cargo run --target-dir {DRIVER} \ - -- --ci `", - exe.display() - )); +/// Whether the driver at `exe` was built in [`DRIVER`], which is how a +/// workflow says its job carries the cache. +pub fn carried(root: &Path, exe: &Path) -> bool { + let exe = fs::canonicalize(exe).unwrap_or_else(|e| panic!("{}: {e}", exe.display())); + match fs::canonicalize(root.join(DRIVER)) { + Ok(driver) => exe.starts_with(driver), + Err(e) if e.kind() == ErrorKind::NotFound => false, + Err(e) => panic!("{DRIVER}: {e}"), } +} + +/// Before the first step of a job that carries the cache: the entry restored +/// here, read by content. +pub fn read(root: &Path) -> Result<(Start, String), String> { open(root, &runner(|name| std::env::var(name).ok())?, SystemTime::now()) } @@ -190,7 +193,12 @@ pub fn seal(root: &Path, cold: &Cold) -> Result { text.push_str(&format!("{hash} {path}\n")); } fs::write(root.join(MANIFEST), text).map_err(|e| format!("write {MANIFEST}: {e}"))?; - Ok(format!("{} sources; {files} files, {} MiB, dated as built", cold.sources.len(), bytes >> 20)) + Ok(format!( + "{} sources, built on {}; {files} files, {} MiB, dated as built", + cold.sources.len(), + cold.runner, + bytes >> 20 + )) } /// The commit an entry was built from, the runner, and its sources. @@ -648,10 +656,13 @@ pub fn word(_: proc_macro::TokenStream) -> proc_macro::TokenStream { } #[test] - fn a_driver_built_in_any_other_target_is_refused() { + fn only_a_driver_built_in_its_own_target_carries_the_cache() { let tmp = TempDir::new("cicache-driver"); - fs::create_dir_all(tmp.join(DRIVER)).unwrap(); - let refusal = read(&tmp).err().expect("a test binary is no driver"); - assert!(refusal.contains("the driver runs from"), "{refusal}"); + let (ours, other) = (format!("{DRIVER}/debug/toyos-build"), "target/debug/toyos-build"); + write(&tmp, other, ""); + assert!(!carried(&tmp, &tmp.join(other)), "a tree with no {DRIVER}"); + write(&tmp, &ours, ""); + assert!(carried(&tmp, &tmp.join(&ours)), "{ours}"); + assert!(!carried(&tmp, &tmp.join(other)), "{other}"); } } From e0ced587c535f44a99146471fa6013a56789ed5e Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 16:30:07 +0200 Subject: [PATCH 10/20] Review round 3: no step inherits the driver's target, incremental state or full debuginfo, and the writer bounds what actions/cache stores - `ci::carry` is the one function `host()` calls for a job that carries the cache. `a_step_of_a_job_that_carries_the_cache_builds_in_its_own_target` runs it in a driver of its own, spawned with the job's `CARGO_TARGET_DIR` and without `CARGO_INCREMENTAL` or `CARGO_PROFILE_DEV_DEBUG`, and requires `cargo metadata`'s `target_directory` to be its fixture's `target`, and `cargo build -v`'s rustc line to carry no `-C incremental` and `-C debuginfo=line-tables-only`. - The steps build with line tables alone (`CARGO_PROFILE_DEV_DEBUG=line-tables-only`). A backtrace in a step's log reads them: a panic while panicking prints one, as the `doorbell-kick-relaxed` control does in run 36867673999. Nothing reads the rest: no step sets `RUST_BACKTRACE`, and no host test reads a binary's DWARF. A Linux link copies the DWARF of every object it takes into the binary, which macOS leaves in the objects: the Linux run sealed 7684 MiB where macOS sealed 3575 MiB. `toyos-build`'s lib test, cross-linked on the dev host for x86_64-unknown-linux-gnu by rust-lld with no C runtime, so that what it keeps is almost all DWARF, is 124,452,112 B with full debuginfo, 35,983,096 B with line tables alone, and 848,744 B with none. - `cicache::bound` runs after the seal. It makes the archive actions/cache v4.3.0 makes of `cicache::PATHS` (its log on a runner: `gtar --posix -cf cache.tzst --exclude cache.tzst -P -C --files-from manifest.txt --use-compress-program zstdmt`) with the runner's own `tar` and `zstdmt`, counts it as it streams, and refuses it above 2,000,000,000 B, so the save, which follows only a green run, never runs. - The limit: eviction is by last access past the repository's 10 GB, and on a night whose guest jobs restore after the host entry is saved the repository holds two host entries beside a guest one, then one beside two guest ones. With the last guest entry, G = 3,281,375,938 B, and H at the limit, 2H + G = 7,281,375,938 B and H + 2G = 8,562,751,876 B. G may grow to 4,000,000,000 B before H + 2G reaches 10 GB. - `each_cache_has_one_writer` holds every host-cache step's `path:` list to `cicache::PATHS`, which the bound measures, and refuses a step taken by an alias, or anchored for one, in a job that names the host cache: its reader sees neither. - `runs_at_build` reads `[project]` as cargo reads `[package]`. - `issues/build/a-warm-host-run-keeps-what-a-registry-proc-macro-expanded-from-a-file-it-never-named.md` files the residual a warm read leaves for a registry proc macro. - The host-tools row for `tar` and `zstd` names `src/cicache.rs` beside `src/release.rs`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- ...cro-expanded-from-a-file-it-never-named.md | 23 ++++ ...d-runs-host-tools-outside-rust-and-qemu.md | 2 +- src/ci.rs | 99 ++++++++++++++--- src/cicache.rs | 102 +++++++++++++++--- 4 files changed, 196 insertions(+), 30 deletions(-) create mode 100644 issues/build/a-warm-host-run-keeps-what-a-registry-proc-macro-expanded-from-a-file-it-never-named.md diff --git a/issues/build/a-warm-host-run-keeps-what-a-registry-proc-macro-expanded-from-a-file-it-never-named.md b/issues/build/a-warm-host-run-keeps-what-a-registry-proc-macro-expanded-from-a-file-it-never-named.md new file mode 100644 index 00000000000..206bfc2c6af --- /dev/null +++ b/issues/build/a-warm-host-run-keeps-what-a-registry-proc-macro-expanded-from-a-file-it-never-named.md @@ -0,0 +1,23 @@ +--- +status: open +kind: tooling +opened: 2026-10-01 +--- + +# A warm host run keeps what a registry proc macro expanded from a file it never named + +A registry proc macro is compiled once and runs inside every compile of the +crate that expands it. One that reads a file without telling rustc, the way +`wayland-scanner`'s `generate_client_code!` opens its XML, reads it again only +when cargo recompiles the expanding crate. A warm `host` run +(`src/cicache.rs`) recompiles a path crate only when its own package changed, +so when that file sits outside the expanding crate's package and changes +alone, the warm run keeps the old expansion where a cold run makes a new one. +Cargo never dates a registry source, so nothing dates the macro's package. + +Of the proc macros in the lockfiles of the five workspaces the host job builds, +`wayland-scanner` alone reads a file it does not name, and no tracked source +names it. + +Done when a warm read serves what a cold build serves for a path crate that +expands a registry proc macro reading another package's file, with a test. diff --git a/issues/build/the-build-runs-host-tools-outside-rust-and-qemu.md b/issues/build/the-build-runs-host-tools-outside-rust-and-qemu.md index b0239d20584..8e7d1270094 100644 --- a/issues/build/the-build-runs-host-tools-outside-rust-and-qemu.md +++ b/issues/build/the-build-runs-host-tools-outside-rust-and-qemu.md @@ -28,7 +28,7 @@ arrives and is not one. M4 and M5 are stages of `issues/build/toyos-builds-itsel | `sh` running `rust/x`, and Python running `x.py` and `bootstrap.py` | every toolchain build (`src/toolchain.rs`) | refused: a Rust tool does it, upstream's bootstrap binary, which builds with stable cargo, fetches its own stage0 (`rust/src/bootstrap/src/core/download.rs`) and needs no Python | `src/toolchain.rs` runs the bootstrap binary | | `curl` in rustc's bootstrap | fetches the stage0 `rust/src/stage0` pins, for a compiler or LLVM build whose build directory lacks it, whichever bootstrap runs | refused: a Rust tool does it, rustup installs the dated beta the pin names, and bootstrap takes a stage0 through `build.rustc` and `build.cargo`, as `src/sysroot.rs` hands it one | no toolchain build fetches with `curl` | | `curl` in `src/release.rs` and `src/sdkversion.rs` | the toolchain release's lookup and download and the crates.io index, on CI runners only | refused: a Rust tool does it, `ureq`, which `userland/doom/build.rs` already fetches with | those fetches are Rust's | -| `tar` and `zstd` | `src/release.rs` packs and unpacks the toolchain release, on CI runners only | refused: a Rust tool does it, the `tar` crate `userland/doom/build.rs` already unpacks with, and a zstd crate | both are done in Rust, in-process | +| `tar` and `zstd` | `src/release.rs` packs and unpacks the toolchain release; `src/cicache.rs` makes the host cache's entry as actions/cache will, `tar --posix` through `zstdmt`, to bound what it stores; on CI runners only | refused: a Rust tool does it, the `tar` crate `userland/doom/build.rs` already unpacks with, and a zstd crate | both are done in Rust, in-process | | `gh` | `src/release.rs` asks whether a toolchain release exists, creates it and moves the `sdk-` alias, on the nightly's `build` runner | refused: not C or C++ source, it is Go | `src/release.rs` speaks GitHub's REST API itself | | `ssh` | `src/metal.rs` reaches the T14's Ubuntu with it, only in the metal loop | refused: a Rust tool does it, the repository's own russh client `crate::build::ssh_client_host`, which `src/metaltalk.rs` already drives | `src/metal.rs` drives that client, or Ubuntu leaves the loop | | `cc`, `c++`, `ar` and `xcrun` on a macOS host, Apple's Command Line Tools | what the Linux row's tools do, and rustc asks `xcrun` for the SDK on every host link that names no `SDKROOT` (`rust/compiler/rustc_codegen_ssa/src/back/apple.rs`), as `src/llvm.rs` does for the LLVM's key | refused: one host OS alone | M5: no host in the loop | diff --git a/src/ci.rs b/src/ci.rs index 240fb05cac9..bd890d8c6e5 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -467,8 +467,9 @@ fn run_control(root: &Path, control: &Control) -> Result { /// host triple for the same reason. /// /// In a job that carries the cache ([`cicache::carried`]) the restored entry is -/// read before any step and a run that starts cold seals its tree after the -/// last; a developer's tree keeps the dates its edits gave it. +/// read before any step, and a run that starts cold seals its tree after the +/// last and refuses an entry above its bound; a developer's tree keeps the +/// dates its edits gave it. fn host(root: &Path) -> Vec { let tmp = toyos_tmpdir::TempDir::new("ci-host"); let short = Path::new(toyos_tmpdir::SHORT_BASE); @@ -480,13 +481,7 @@ fn host(root: &Path) -> Vec { let mut steps = Vec::new(); let mut cold = None; if cicache::carried(root, &std::env::current_exe().expect("the driver's own path")) { - // The job's `CARGO_TARGET_DIR` names the driver's target, and no step - // builds there. - std::env::remove_var("CARGO_TARGET_DIR"); - // No incremental state in an entry: it is most of an entry's bytes, - // and after a read by content it helps only a crate whose bytes - // changed. - std::env::set_var("CARGO_INCREMENTAL", "0"); + carry(); let mut start = None; steps.push(step("the cache entry, read by content", || { let (found, said) = cicache::read(root)?; @@ -564,10 +559,26 @@ fn host(root: &Path) -> Vec { steps.push(step("nothing left in $TMPDIR or /tmp", || left_behind(&tmp, short, &before))); if let Some(cold) = cold { steps.push(step("the tree, sealed as a cache entry", || cicache::seal(root, &cold))); + steps.push(step("the entry, as actions/cache will store it", || cicache::bound(root))); } steps } +/// What every step of a job that carries the cache inherits, set before any +/// thread as `host`'s `TMPDIR` is. +fn carry() { + // The job's `CARGO_TARGET_DIR` names the driver's target, and no step + // builds there. + std::env::remove_var("CARGO_TARGET_DIR"); + // No incremental state in an entry: it is most of an entry's bytes, and + // after a read by content it helps only a crate whose bytes changed. + std::env::set_var("CARGO_INCREMENTAL", "0"); + // Line tables alone: a backtrace in a step's log reads them, and nothing + // reads the rest of the debuginfo, of which a Linux link copies every + // dependency's into each test binary. + std::env::set_var("CARGO_PROFILE_DEV_DEBUG", "line-tables-only"); +} + /// What `tmp` holds but the lock `toyos_tmpdir` keeps in it, and every root /// under `short` whose process is gone that `before` does not name. /// @@ -875,6 +886,50 @@ mod tests { assert!(refusal.contains(&died) && !refusal.contains(&earlier), "{refusal}"); } + /// The crate [`a_carried_jobs_step`] builds; unset, it is not a test. + const FIXTURE: &str = "TOYOS_CI_TEST_FIXTURE"; + + /// What a job that carries the cache hands its driver reaches no step: each + /// builds in its own workspace's target, with no incremental state and line + /// tables alone. The driver is a process of its own, because `carry` + /// writes the environment, which no other thread may read meanwhile. + #[test] + fn a_step_of_a_job_that_carries_the_cache_builds_in_its_own_target() { + let fixture = toyos_tmpdir::TempDir::new("ci-carried"); + std::fs::create_dir(fixture.join("src")).unwrap(); + let manifest = "[package]\nname = \"one\"\nversion = \"0.1.0\"\nedition = \"2021\"\n\n[workspace]\n"; + std::fs::write(fixture.join("Cargo.toml"), manifest).unwrap(); + std::fs::write(fixture.join("src/lib.rs"), "pub fn one() -> u8 {\n 1\n}\n").unwrap(); + let out = crate::buildlock::tests::rerun("ci::tests::a_carried_jobs_step") + .env(FIXTURE, fixture.path()) + .env("CARGO_TARGET_DIR", cicache::DRIVER) + .env_remove("CARGO_INCREMENTAL") + .env_remove("CARGO_PROFILE_DEV_DEBUG") + .output() + .expect("run the driver"); + let said = String::from_utf8_lossy(&out.stdout).into_owned() + &String::from_utf8_lossy(&out.stderr); + assert!(out.status.success() && said.contains("test result: ok. 1 passed"), "{said}"); + } + + #[test] + #[ignore = "the driver of the test above; never runs on its own"] + fn a_carried_jobs_step() { + let fixture = PathBuf::from(std::env::var_os(FIXTURE).unwrap_or_else(|| panic!("run without {FIXTURE}"))); + carry(); + let cargo = |args: &[&str]| { + let out = Command::new("cargo").args(args).current_dir(&fixture).output().expect("run cargo"); + assert!(out.status.success(), "cargo {args:?}: {}", String::from_utf8_lossy(&out.stderr)); + out + }; + let metadata = cargo(&["metadata", "--format-version", "1", "--no-deps", "--offline"]); + let metadata: serde_json::Value = serde_json::from_slice(&metadata.stdout).unwrap(); + let ours = std::fs::canonicalize(&fixture).unwrap().join("target"); + assert_eq!(metadata["target_directory"].as_str().map(Path::new), Some(ours.as_path())); + let build = String::from_utf8(cargo(&["build", "-v", "--offline"]).stderr).unwrap(); + let rustc = build.lines().find(|l| l.contains("--crate-name one")).unwrap_or_else(|| panic!("{build}")); + assert!(!rustc.contains("-C incremental") && rustc.contains("-C debuginfo=line-tables-only"), "{rustc}"); + } + fn repo_root() -> PathBuf { PathBuf::from(env!("CARGO_MANIFEST_DIR")) } @@ -1011,9 +1066,11 @@ mod tests { /// Exactly one job writes each cache, on the nightly, so what a pull request /// restores is one run's tree and never a race between two writers. A job /// that restores or saves the host cache builds the driver in - /// [`cicache::DRIVER`], so it carries the cache, and the one that saves it - /// restores nothing: its run is cold, and a cold run is green only once its - /// tree is sealed ([`cicache`]). + /// [`cicache::DRIVER`], so it carries the cache, names + /// [`cicache::PATHS`], which its bound measures, and takes no step by an + /// alias or lends one, which this reader cannot follow; and the one that + /// saves it restores nothing: its run is cold, and a cold run is green only + /// once its tree is sealed ([`cicache`]). #[test] fn each_cache_has_one_writer() { let dir = repo_root().join(".github/workflows"); @@ -1024,7 +1081,7 @@ mod tests { let name = entry.file_name().to_string_lossy().into_owned(); assert!(!text.contains("actions/cache@"), "{name}: the combined action saves too"); for (job, lines) in jobs(&text) { - let caches: Vec<(bool, String)> = lines + let caches: Vec<(bool, String, Vec<&str>)> = lines .iter() .enumerate() .filter(|(_, l)| l.contains("actions/cache/restore@") || l.contains("actions/cache/save@")) @@ -1033,14 +1090,24 @@ mod tests { .iter() .find_map(|l| l.trim_start().strip_prefix("key: ")) .expect("a cache step names its key"); - (l.contains("actions/cache/save@"), key.split('$').next().unwrap_or("").to_string()) + let paths = lines[at..] + .iter() + .skip_while(|l| l.trim() != "path: |") + .skip(1) + .take_while(|l| l.starts_with(" ")) + .map(|l| l.trim()) + .collect(); + (l.contains("actions/cache/save@"), key.split('$').next().unwrap_or("").to_string(), paths) }) .collect(); - for (save, prefix) in &caches { + for (save, prefix, paths) in &caches { if prefix.starts_with("host-") { assert_eq!(prefix, cicache::SEALED, "{name} {job}"); + assert_eq!(paths, &cicache::PATHS, "{name} {job}: the host cache's paths"); assert!(lines.contains(&carries.as_str()), "{name} {job}: the host cache without `{}`", carries.trim()); - assert!(!save || caches.iter().all(|(s, _)| *s), "{name} {job}: the host cache's writer restores"); + assert!(!save || caches.iter().all(|(s, ..)| *s), "{name} {job}: the host cache's writer restores"); + let named = |l: &&str| ["- *", "- &"].iter().any(|by| l.trim_start().starts_with(by)); + assert!(!lines.iter().any(named), "{name} {job}: a step by an alias, or lent to one"); } if *save { writers.push((name.clone(), prefix.clone())); diff --git a/src/cicache.rs b/src/cicache.rs index ad4d5afd836..a3fff208ef1 100644 --- a/src/cicache.rs +++ b/src/cicache.rs @@ -33,11 +33,15 @@ //! path crates are compiled again every time, and in the steps' target that //! would make every crate depending on them stale. A job that builds it //! anywhere else runs as a developer's tree does. +//! +//! **An entry that would store more than [`LIMIT`] is refused before the save** +//! ([`bound`]). use std::collections::{BTreeMap, BTreeSet}; use std::fs; use std::io::ErrorKind; use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use sha2::{Digest, Sha256}; @@ -46,6 +50,25 @@ const MANIFEST: &str = "target/ci-sources"; pub const DRIVER: &str = "target/ci-driver"; pub const SEALED: &str = "host-sealed-"; +/// What every step that names the host cache archives: the cache's version is +/// computed from the list, so a restore whose list differs finds nothing. +pub const PATHS: [&str; 8] = [ + "~/.cargo/registry/index", + "~/.cargo/registry/cache", + "~/.cargo/git/db", + "target", + "userland/target", + "toyos/target", + "kernel/target", + "bootloader/target", +]; + +/// The most an entry may store. The repository's caches are evicted by last +/// access past 10 GB, and a night whose guest jobs restore after this entry is +/// saved holds two host entries beside a guest one, then one beside two guest +/// ones. +const LIMIT: u64 = 2_000_000_000; + /// 2001-09-09T01:46:40Z: older than any build, so a file dated so is never /// newer than one. fn built() -> SystemTime { @@ -201,6 +224,47 @@ pub fn seal(root: &Path, cold: &Cold) -> Result { )) } +/// After [`seal`]: what actions/cache will store, refused above [`LIMIT`], so +/// the save that follows a green run never runs. +pub fn bound(root: &Path) -> Result { + within(stored(root)?) +} + +fn within(stored: u64) -> Result { + if stored > LIMIT { + return Err(format!( + "{stored} B, above the {LIMIT} B an entry may store: saved, it could evict the guest \ + entry or the next host one" + )); + } + Ok(format!("{stored} B, within the {LIMIT} B an entry may store")) +} + +/// The archive actions/cache makes of [`PATHS`], made as it makes it, with the +/// runner's own `tar` and `zstdmt`, and counted as it streams. +fn stored(root: &Path) -> Result { + let home = std::env::var_os("HOME").ok_or("HOME is unset, and actions/cache reads `~` from it")?; + let paths: Vec = PATHS + .iter() + .map(|path| path.strip_prefix("~/").map_or_else(|| PathBuf::from(path), |rest| Path::new(&home).join(rest))) + .filter(|path| root.join(path).exists()) + .collect(); + let mut tar = Command::new("tar") + .args(["--posix", "-cf", "-", "-P", "--use-compress-program", "zstdmt", "-C"]) + .arg(root) + .args(&paths) + .stdout(Stdio::piped()) + .spawn() + .map_err(|e| format!("tar: {e}"))?; + let stored = std::io::copy(&mut tar.stdout.take().expect("piped"), &mut std::io::sink()) + .map_err(|e| format!("reading tar: {e}"))?; + let status = tar.wait().map_err(|e| format!("tar: {e}"))?; + if !status.success() { + return Err(format!("tar exited {status}")); + } + Ok(stored) +} + /// The commit an entry was built from, the runner, and its sources. fn parse(text: &str) -> Result<(String, String, Sources), String> { let mut lines = text.lines(); @@ -224,7 +288,8 @@ fn runs_at_build(root: &Path, manifest: &str, current: &Sources) -> Result current.contains_key(&*Path::new(manifest).with_file_name("build.rs").to_string_lossy()), Some(build) => build.as_bool() != Some(false), }; @@ -542,26 +607,37 @@ pub fn word(_: proc_macro::TokenStream) -> proc_macro::TokenStream { fn every_spelling_of_code_run_at_build_time_is_found() { let tmp = TempDir::new("cicache-spellings"); let cases = [ - ("", false, false), - ("", true, true), - ("build = false\n", true, false), - ("build = \"gen.rs\"\n", false, true), - ("[lib]\nproc-macro = true\n", false, true), - ("[lib]\nproc_macro = true\n", false, true), - ("[lib]\ncrate-type = [\"proc-macro\"]\n", false, true), - ("[lib]\ncrate_type = [\"proc-macro\"]\n", false, true), - ("[lib]\ncrate-type = [\"rlib\"]\n", false, false), + ("[package]", "", false, false), + ("[package]", "", true, true), + ("[package]", "build = false\n", true, false), + ("[package]", "build = \"gen.rs\"\n", false, true), + ("[project]", "build = \"gen.rs\"\n", false, true), + ("[project]", "build = false\n", true, false), + ("[package]", "[lib]\nproc-macro = true\n", false, true), + ("[package]", "[lib]\nproc_macro = true\n", false, true), + ("[package]", "[lib]\ncrate-type = [\"proc-macro\"]\n", false, true), + ("[package]", "[lib]\ncrate_type = [\"proc-macro\"]\n", false, true), + ("[package]", "[lib]\ncrate-type = [\"rlib\"]\n", false, false), ]; - for (keys, script, expected) in cases { - write(&tmp, "Cargo.toml", &format!("[package]\nname = \"p\"\n{keys}")); + for (table, keys, script, expected) in cases { + write(&tmp, "Cargo.toml", &format!("{table}\nname = \"p\"\n{keys}")); let mut current = Sources::from([("Cargo.toml".to_string(), String::new())]); if script { current.insert("build.rs".into(), String::new()); } - assert_eq!(runs_at_build(&tmp, "Cargo.toml", ¤t), Ok(expected), "{keys:?}, build.rs: {script}"); + assert_eq!(runs_at_build(&tmp, "Cargo.toml", ¤t), Ok(expected), "{table} {keys:?}, build.rs: {script}"); } } + /// An entry above the bound is refused, so the save after it never runs; + /// one at the bound is kept. + #[test] + fn an_entry_above_its_bound_is_refused() { + assert!(within(LIMIT).is_ok()); + let refusal = within(LIMIT + 1).expect_err("an entry above the bound"); + assert!(refusal.starts_with(&format!("{} B", LIMIT + 1)), "{refusal}"); + } + /// One commit of `a.rs`, read cold on `runner`. fn cold_repo(tmp: &Path, runner: &str) -> Cold { sh(tmp, &["init", "-q"]); From b4dfda5508115c076df653cb9a8ab4d9f514aed0 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 17:03:54 +0200 Subject: [PATCH 11/20] Round 6: the seal bounds the bytes it dates, and the runner's tar and zstdmt go The bound ran the runner's `tar` and `zstdmt` to count what actions/cache would store. The host-tools declaration refuses both, and the Rust tool its row names is a zstd crate, a new dependency only to measure. The seal already summed the lengths of the files it dates under every target, for its log line; it now refuses a tree above `LIMIT` with that sum, before it writes the manifest. The step is red, so the save, which runs only after a green step, stores nothing, and the tree carries no manifest a reader would take. The seal's line prints the sum in bytes beside the limit, where it printed MiB. The limit is 8,000,000,000 B of targets, uncompressed. Run 36878222090 sealed 5369 MiB of targets, at least 5,629,804,544 B, and the archive the bound made of the cache's paths, as actions/cache makes one, was 1,403,326,566 B: a ratio of 4.01. At that ratio the limit stores at most 1,994,138,951 B. With the last guest entry's 3,281,375,938 B, a night then holds 2H + G = 7,269,653,840 B and H + 2G = 8,556,890,827 B, both under the 10 GB eviction limit, and the ratio may fall to 2.38 before 2H + G reaches it. The 7,684 MiB tree is db4654fb6's, sealed with full debuginfo in run 36867673999, which ran no bound. `an_entry_above_its_bound_is_refused` seals a fixture whose targets hold the limit plus one byte, the limit in a sparse file under `target/` and the byte under `kernel/target/`, and requires the refusal and no manifest. It then seals the same tree at the limit exactly. The host-tools row is main's again: no ToyOS code runs `tar` or `zstd` for the cache. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- ...d-runs-host-tools-outside-rust-and-qemu.md | 2 +- src/ci.rs | 10 +-- src/cicache.rs | 86 +++++++------------ 3 files changed, 35 insertions(+), 63 deletions(-) diff --git a/issues/build/the-build-runs-host-tools-outside-rust-and-qemu.md b/issues/build/the-build-runs-host-tools-outside-rust-and-qemu.md index 8e7d1270094..b0239d20584 100644 --- a/issues/build/the-build-runs-host-tools-outside-rust-and-qemu.md +++ b/issues/build/the-build-runs-host-tools-outside-rust-and-qemu.md @@ -28,7 +28,7 @@ arrives and is not one. M4 and M5 are stages of `issues/build/toyos-builds-itsel | `sh` running `rust/x`, and Python running `x.py` and `bootstrap.py` | every toolchain build (`src/toolchain.rs`) | refused: a Rust tool does it, upstream's bootstrap binary, which builds with stable cargo, fetches its own stage0 (`rust/src/bootstrap/src/core/download.rs`) and needs no Python | `src/toolchain.rs` runs the bootstrap binary | | `curl` in rustc's bootstrap | fetches the stage0 `rust/src/stage0` pins, for a compiler or LLVM build whose build directory lacks it, whichever bootstrap runs | refused: a Rust tool does it, rustup installs the dated beta the pin names, and bootstrap takes a stage0 through `build.rustc` and `build.cargo`, as `src/sysroot.rs` hands it one | no toolchain build fetches with `curl` | | `curl` in `src/release.rs` and `src/sdkversion.rs` | the toolchain release's lookup and download and the crates.io index, on CI runners only | refused: a Rust tool does it, `ureq`, which `userland/doom/build.rs` already fetches with | those fetches are Rust's | -| `tar` and `zstd` | `src/release.rs` packs and unpacks the toolchain release; `src/cicache.rs` makes the host cache's entry as actions/cache will, `tar --posix` through `zstdmt`, to bound what it stores; on CI runners only | refused: a Rust tool does it, the `tar` crate `userland/doom/build.rs` already unpacks with, and a zstd crate | both are done in Rust, in-process | +| `tar` and `zstd` | `src/release.rs` packs and unpacks the toolchain release, on CI runners only | refused: a Rust tool does it, the `tar` crate `userland/doom/build.rs` already unpacks with, and a zstd crate | both are done in Rust, in-process | | `gh` | `src/release.rs` asks whether a toolchain release exists, creates it and moves the `sdk-` alias, on the nightly's `build` runner | refused: not C or C++ source, it is Go | `src/release.rs` speaks GitHub's REST API itself | | `ssh` | `src/metal.rs` reaches the T14's Ubuntu with it, only in the metal loop | refused: a Rust tool does it, the repository's own russh client `crate::build::ssh_client_host`, which `src/metaltalk.rs` already drives | `src/metal.rs` drives that client, or Ubuntu leaves the loop | | `cc`, `c++`, `ar` and `xcrun` on a macOS host, Apple's Command Line Tools | what the Linux row's tools do, and rustc asks `xcrun` for the SDK on every host link that names no `SDKROOT` (`rust/compiler/rustc_codegen_ssa/src/back/apple.rs`), as `src/llvm.rs` does for the LLVM's key | refused: one host OS alone | M5: no host in the loop | diff --git a/src/ci.rs b/src/ci.rs index bd890d8c6e5..fb2bcdc1784 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -559,7 +559,6 @@ fn host(root: &Path) -> Vec { steps.push(step("nothing left in $TMPDIR or /tmp", || left_behind(&tmp, short, &before))); if let Some(cold) = cold { steps.push(step("the tree, sealed as a cache entry", || cicache::seal(root, &cold))); - steps.push(step("the entry, as actions/cache will store it", || cicache::bound(root))); } steps } @@ -1066,11 +1065,10 @@ mod tests { /// Exactly one job writes each cache, on the nightly, so what a pull request /// restores is one run's tree and never a race between two writers. A job /// that restores or saves the host cache builds the driver in - /// [`cicache::DRIVER`], so it carries the cache, names - /// [`cicache::PATHS`], which its bound measures, and takes no step by an - /// alias or lends one, which this reader cannot follow; and the one that - /// saves it restores nothing: its run is cold, and a cold run is green only - /// once its tree is sealed ([`cicache`]). + /// [`cicache::DRIVER`], so it carries the cache, names [`cicache::PATHS`], + /// and takes no step by an alias or lends one, which this reader cannot + /// follow; and the one that saves it restores nothing: its run is cold, and + /// a cold run is green only once its tree is sealed ([`cicache`]). #[test] fn each_cache_has_one_writer() { let dir = repo_root().join(".github/workflows"); diff --git a/src/cicache.rs b/src/cicache.rs index a3fff208ef1..ff1e33ee7d2 100644 --- a/src/cicache.rs +++ b/src/cicache.rs @@ -34,14 +34,13 @@ //! would make every crate depending on them stale. A job that builds it //! anywhere else runs as a developer's tree does. //! -//! **An entry that would store more than [`LIMIT`] is refused before the save** -//! ([`bound`]). +//! **A tree whose targets hold more than [`LIMIT`] is refused at the seal**, so +//! the save, which follows only a green run, never stores it. use std::collections::{BTreeMap, BTreeSet}; use std::fs; use std::io::ErrorKind; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use sha2::{Digest, Sha256}; @@ -63,11 +62,11 @@ pub const PATHS: [&str; 8] = [ "bootloader/target", ]; -/// The most an entry may store. The repository's caches are evicted by last -/// access past 10 GB, and a night whose guest jobs restore after this entry is -/// saved holds two host entries beside a guest one, then one beside two guest -/// ones. -const LIMIT: u64 = 2_000_000_000; +/// The most an entry's targets may hold, uncompressed. The repository's caches +/// are evicted by last access past 10 GB, and a night whose guest jobs restore +/// after this entry is saved holds two host entries beside a guest one, then +/// one beside two guest ones. +const LIMIT: u64 = 8_000_000_000; /// 2001-09-09T01:46:40Z: older than any build, so a file dated so is never /// newer than one. @@ -191,7 +190,8 @@ fn cold(root: &Path, runner: &str, sources: Sources, why: String) -> Result<(Sta } /// After the last step of a run that started [`Start::Cold`]: every file under -/// every target dated [`built`], then the manifest. +/// every target dated [`built`], then the manifest, which a tree holding more +/// than [`LIMIT`] never gets. pub fn seal(root: &Path, cold: &Cold) -> Result { let now = sources(root)?; let moved: Vec<&String> = cold @@ -210,6 +210,12 @@ pub fn seal(root: &Path, cold: &Cold) -> Result { for target in targets(root)? { age(&target, &mut files, &mut bytes)?; } + if bytes > LIMIT { + return Err(format!( + "{bytes} B in {files} files under the targets, above the {LIMIT} B an entry may hold: \ + saved, it could evict the guest entry or the next host one" + )); + } let head = crate::sync::git(root, &["rev-parse", "HEAD"])?; let mut text = format!("{head}\n{}\n", cold.runner); for (path, hash) in &cold.sources { @@ -217,54 +223,13 @@ pub fn seal(root: &Path, cold: &Cold) -> Result { } fs::write(root.join(MANIFEST), text).map_err(|e| format!("write {MANIFEST}: {e}"))?; Ok(format!( - "{} sources, built on {}; {files} files, {} MiB, dated as built", + "{} sources, built on {}; {files} files, {bytes} B of the {LIMIT} B an entry may hold, \ + dated as built", cold.sources.len(), cold.runner, - bytes >> 20 )) } -/// After [`seal`]: what actions/cache will store, refused above [`LIMIT`], so -/// the save that follows a green run never runs. -pub fn bound(root: &Path) -> Result { - within(stored(root)?) -} - -fn within(stored: u64) -> Result { - if stored > LIMIT { - return Err(format!( - "{stored} B, above the {LIMIT} B an entry may store: saved, it could evict the guest \ - entry or the next host one" - )); - } - Ok(format!("{stored} B, within the {LIMIT} B an entry may store")) -} - -/// The archive actions/cache makes of [`PATHS`], made as it makes it, with the -/// runner's own `tar` and `zstdmt`, and counted as it streams. -fn stored(root: &Path) -> Result { - let home = std::env::var_os("HOME").ok_or("HOME is unset, and actions/cache reads `~` from it")?; - let paths: Vec = PATHS - .iter() - .map(|path| path.strip_prefix("~/").map_or_else(|| PathBuf::from(path), |rest| Path::new(&home).join(rest))) - .filter(|path| root.join(path).exists()) - .collect(); - let mut tar = Command::new("tar") - .args(["--posix", "-cf", "-", "-P", "--use-compress-program", "zstdmt", "-C"]) - .arg(root) - .args(&paths) - .stdout(Stdio::piped()) - .spawn() - .map_err(|e| format!("tar: {e}"))?; - let stored = std::io::copy(&mut tar.stdout.take().expect("piped"), &mut std::io::sink()) - .map_err(|e| format!("reading tar: {e}"))?; - let status = tar.wait().map_err(|e| format!("tar: {e}"))?; - if !status.success() { - return Err(format!("tar exited {status}")); - } - Ok(stored) -} - /// The commit an entry was built from, the runner, and its sources. fn parse(text: &str) -> Result<(String, String, Sources), String> { let mut lines = text.lines(); @@ -629,13 +594,22 @@ pub fn word(_: proc_macro::TokenStream) -> proc_macro::TokenStream { } } - /// An entry above the bound is refused, so the save after it never runs; - /// one at the bound is kept. + /// Targets holding one byte more than the bound between them are refused + /// at the seal, and get no manifest; at the bound they are sealed. The + /// bound's bytes are a hole `set_len` made, so no test writes them. #[test] fn an_entry_above_its_bound_is_refused() { - assert!(within(LIMIT).is_ok()); - let refusal = within(LIMIT + 1).expect_err("an entry above the bound"); + let tmp = TempDir::new("cicache-bound"); + let cold = cold_repo(&tmp, RUNNER); + write(&tmp, "kernel/target/one", "1"); + let big = tmp.join("target/debug/big"); + fs::create_dir_all(big.parent().unwrap()).unwrap(); + fs::File::create(&big).unwrap().set_len(LIMIT).unwrap(); + let refusal = seal(&tmp, &cold).expect_err("targets one byte above the bound"); assert!(refusal.starts_with(&format!("{} B", LIMIT + 1)), "{refusal}"); + assert!(!tmp.join(MANIFEST).exists(), "a refused tree carries a manifest"); + fs::remove_file(tmp.join("kernel/target/one")).unwrap(); + seal(&tmp, &cold).expect("targets at the bound"); } /// One commit of `a.rs`, read cold on `runner`. From f9d535d300566131d251774a3d887f7824c2a037 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 18:07:50 +0200 Subject: [PATCH 12/20] Every carried run bounds what its save would store, and the writer's save follows only a green run The bound sums `cicache::PATHS`, the list the save archives, with `~` taken from HOME: the three `~/.cargo` paths the save stores are counted, and a target the save does not store is not. That gives `PATHS` the production reader it lost when `stored()` went. `cicache::close` runs after the last step of every job that carries the cache, warm or cold. A cold run is bounded before it seals, so a refused tree gets no manifest; a warm run is bounded too, so a pull request whose tree holds more than `LIMIT` reds on its own run, not on the nightly after it lands. Only the nightly saves, as before. `seal` no longer counts, and is private to the module. `each_cache_has_one_writer` reds on any line naming `always()`, `cancelled()`, `failure()` or `continue-on-error` in a job that saves the host cache: either would let the save store a tree the seal refused, above `LIMIT` and with no manifest, which every pull request's read would then refuse. `carry()`'s comment loses "in an entry": `target/ci-driver`, saved under `target`, holds the driver's own incremental state. LIMIT's tie to the 10 GB, one ratio measured once on run 36878222090, is filed as issues/build/the-host-caches-limit-reaches-the-10-gb-only-through-one-measured-ratio.md with its evidence and an exit a gate can fail. The guest-cache issue records the ceiling H + 2G <= 10 GB leaves the guest entry: 4,002,930,524 B at LIMIT, 4,298,336,717 B at that run's H. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- ...and-its-writer-restores-before-it-saves.md | 6 + ...e-10-gb-only-through-one-measured-ratio.md | 32 +++++ src/ci.rs | 34 ++--- src/cicache.rs | 123 ++++++++++++------ 4 files changed, 142 insertions(+), 53 deletions(-) create mode 100644 issues/build/the-host-caches-limit-reaches-the-10-gb-only-through-one-measured-ratio.md diff --git a/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md b/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md index fce47f8f41d..7d112e0083c 100644 --- a/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md +++ b/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md @@ -13,4 +13,10 @@ host entry in the repository's 10 GB). And every guest job restores its targets under a checkout that dated every source at the checkout, so cargo calls every path crate in them stale. +The guest entry's ceiling is what H + 2G ≤ 10 GB leaves it, and that sum binds +every night: 4,002,930,524 B at the host's `LIMIT` (`src/cicache.rs`), or +4,298,336,717 B at run 36878222090's H. Above it, `tcg`'s save evicts that +night's host entry unless a pull request has read the entry since the guest +restore; every pull request then runs cold, and nothing reds. + Done when the guest entry is written cold, read by content, and bounded. diff --git a/issues/build/the-host-caches-limit-reaches-the-10-gb-only-through-one-measured-ratio.md b/issues/build/the-host-caches-limit-reaches-the-10-gb-only-through-one-measured-ratio.md new file mode 100644 index 00000000000..8cf13ba0ebc --- /dev/null +++ b/issues/build/the-host-caches-limit-reaches-the-10-gb-only-through-one-measured-ratio.md @@ -0,0 +1,32 @@ +--- +status: open +kind: tooling +opened: 2026-10-01 +--- + +# The host cache's limit reaches the 10 GB only through one measured ratio + +`src/cicache.rs` refuses a tree whose `PATHS` hold more than `LIMIT`, +8,000,000,000 B, uncompressed. The repository evicts its caches past 10 GB of +what actions/cache stores, compressed, and both two host entries beside a guest +one (2H + G) and one beside two (H + 2G) must fit. + +One ratio ties `LIMIT` to H, measured once. Run 36878222090 (e0ced587c) sealed +5369 MiB of targets, at least 5,629,804,544 B. That head's own archive of the +cache's paths, made with the runner's `tar` and `zstdmt`, held 1,403,326,566 B: +a ratio of 4.01. At that ratio `LIMIT` stores at most 1,994,138,951 B. With the +guest entry's 3,281,375,938 B (run 36696295750's `tcg`), 2H + G is +7,269,653,840 B and H + 2G is 8,556,890,827 B. The ratio may fall to 2.38 +before 2H + G reaches 10 GB. The lowest measured is 3.08: run 36844536500 +sealed 9553 MiB on macOS and saved 3,250,736,567 B. + +No gate reads a stored size, and the tree no longer runs that `tar` or +`zstdmt`. If the targets compress worse, H moves toward the floor and nothing +reds. + +Owner: the host cache (`src/cicache.rs`). + +**Exit condition.** A gate reds on the stored size itself: after nightly's +`host` saves, a step reads that entry's stored bytes and the newest guest +entry's from the repository's cache list, and fails when 2H + G or H + 2G +passes 10 GB. diff --git a/src/ci.rs b/src/ci.rs index 09236436eb0..d6393868e51 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -28,7 +28,7 @@ use std::path::{Path, PathBuf}; use std::process::Command; use crate::arch::{Accel, Arch}; -use crate::cicache::{self, Start}; +use crate::cicache; use crate::userlandhost::{Host, Os, Program}; use crate::{flags, release, sdkversion, sync}; @@ -468,9 +468,9 @@ fn run_control(root: &Path, control: &Control) -> Result { /// host triple for the same reason. /// /// In a job that carries the cache ([`cicache::carried`]) the restored entry is -/// read before any step, and a run that starts cold seals its tree after the -/// last and refuses an entry above its bound; a developer's tree keeps the -/// dates its edits gave it. +/// read before any step, and after the last what its save would store is +/// refused above its bound, warm or cold, and a tree that started cold is +/// sealed; a developer's tree keeps the dates its edits gave it. fn host(root: &Path) -> Vec { let tmp = toyos_tmpdir::TempDir::new("ci-host"); let short = Path::new(toyos_tmpdir::SHORT_BASE); @@ -480,20 +480,17 @@ fn host(root: &Path) -> Vec { std::env::set_var("TMPDIR", tmp.path()); let host_triple = crate::toolchain::host_triple(); let mut steps = Vec::new(); - let mut cold = None; + let mut start = None; if cicache::carried(root, &std::env::current_exe().expect("the driver's own path")) { carry(); - let mut start = None; steps.push(step("the cache entry, read by content", || { let (found, said) = cicache::read(root)?; start = Some(found); Ok(said) })); - match start { - // Every step after an unreadable entry would be judged against it. - None => return steps, - Some(Start::Cold(found)) => cold = Some(found), - Some(Start::Warm) => {} + // Every step after an unreadable entry would be judged against it. + if start.is_none() { + return steps; } } steps.extend([ @@ -568,8 +565,8 @@ fn host(root: &Path) -> Vec { cargo(root, &["test", "--manifest-path", "toyos/Cargo.toml", "--target", &host_triple]) })); steps.push(step("nothing left in $TMPDIR or /tmp", || left_behind(&tmp, short, &before))); - if let Some(cold) = cold { - steps.push(step("the tree, sealed as a cache entry", || cicache::seal(root, &cold))); + if let Some(start) = start { + steps.push(step("the tree, bounded as a cache entry", || cicache::close(root, &start))); } steps } @@ -580,8 +577,8 @@ fn carry() { // The job's `CARGO_TARGET_DIR` names the driver's target, and no step // builds there. std::env::remove_var("CARGO_TARGET_DIR"); - // No incremental state in an entry: it is most of an entry's bytes, and - // after a read by content it helps only a crate whose bytes changed. + // No incremental state: it is most of an entry's bytes, and after a read + // by content it helps only a crate whose bytes changed. std::env::set_var("CARGO_INCREMENTAL", "0"); // Line tables alone: a backtrace in a step's log reads them, and nothing // reads the rest of the debuginfo, of which a Linux link copies every @@ -1173,7 +1170,9 @@ mod tests { /// [`cicache::DRIVER`], so it carries the cache, names [`cicache::PATHS`], /// and takes no step by an alias or lends one, which this reader cannot /// follow; and the one that saves it restores nothing: its run is cold, and - /// a cold run is green only once its tree is sealed ([`cicache`]). + /// a cold run is green only once its tree is sealed ([`cicache`]). No line + /// of that job names a status function that runs a step past a red one, or + /// `continue-on-error`, so its save follows only a green run. #[test] fn each_cache_has_one_writer() { let dir = repo_root().join(".github/workflows"); @@ -1209,6 +1208,9 @@ mod tests { assert_eq!(paths, &cicache::PATHS, "{name} {job}: the host cache's paths"); assert!(lines.contains(&carries.as_str()), "{name} {job}: the host cache without `{}`", carries.trim()); assert!(!save || caches.iter().all(|(s, ..)| *s), "{name} {job}: the host cache's writer restores"); + let past_a_red = ["always()", "cancelled()", "failure()", "continue-on-error"]; + let past = lines.iter().find(|l| past_a_red.iter().any(|word| l.contains(word))); + assert!(!save || past.is_none(), "{name} {job}: the host cache saved past a red step: {past:?}"); let named = |l: &&str| ["- *", "- &"].iter().any(|by| l.trim_start().starts_with(by)); assert!(!lines.iter().any(named), "{name} {job}: a step by an alias, or lent to one"); } diff --git a/src/cicache.rs b/src/cicache.rs index ff1e33ee7d2..4afe0423015 100644 --- a/src/cicache.rs +++ b/src/cicache.rs @@ -34,8 +34,11 @@ //! would make every crate depending on them stale. A job that builds it //! anywhere else runs as a developer's tree does. //! -//! **A tree whose targets hold more than [`LIMIT`] is refused at the seal**, so -//! the save, which follows only a green run, never stores it. +//! **Every run that carries the cache refuses, after its last step, a tree +//! whose [`PATHS`] hold more than [`LIMIT`]**, `~` being `HOME` ([`close`]): a +//! cold one before it seals, so the save, which follows only a green run, +//! never stores it, and a warm one so that a pull request whose tree holds +//! more reds on its own run. use std::collections::{BTreeMap, BTreeSet}; use std::fs; @@ -62,10 +65,10 @@ pub const PATHS: [&str; 8] = [ "bootloader/target", ]; -/// The most an entry's targets may hold, uncompressed. The repository's caches -/// are evicted by last access past 10 GB, and a night whose guest jobs restore -/// after this entry is saved holds two host entries beside a guest one, then -/// one beside two guest ones. +/// The most the files under [`PATHS`] may hold, uncompressed. The repository's +/// caches are evicted by last access past 10 GB, and a night whose guest jobs +/// restore after this entry is saved holds two host entries beside a guest +/// one, then one beside two guest ones. const LIMIT: u64 = 8_000_000_000; /// 2001-09-09T01:46:40Z: older than any build, so a file dated so is never @@ -189,10 +192,39 @@ fn cold(root: &Path, runner: &str, sources: Sources, why: String) -> Result<(Sta Ok((Start::Cold(Cold { runner: runner.to_string(), sources }), said)) } -/// After the last step of a run that started [`Start::Cold`]: every file under -/// every target dated [`built`], then the manifest, which a tree holding more -/// than [`LIMIT`] never gets. -pub fn seal(root: &Path, cold: &Cold) -> Result { +/// After the last step of a job that carries the cache: what its save would +/// store, refused above [`LIMIT`], and a tree that started cold sealed. +pub fn close(root: &Path, start: &Start) -> Result { + let home = std::env::var_os("HOME").ok_or("HOME is unset, and the cache's paths name it as `~`")?; + close_at(root, Path::new(&home), start) +} + +fn close_at(root: &Path, home: &Path, start: &Start) -> Result { + let (mut files, mut bytes) = (0u64, 0u64); + for path in PATHS { + let dir = match path.strip_prefix("~/") { + Some(under) => home.join(under), + None => root.join(path), + }; + size(&dir, &mut files, &mut bytes)?; + } + if bytes > LIMIT { + return Err(format!( + "{bytes} B in {files} files under the cache's paths, above the {LIMIT} B an entry may \ + hold: saved, it could evict the guest entry or the next host one" + )); + } + let stored = format!("{files} files, {bytes} B of the {LIMIT} B an entry may hold"); + match start { + Start::Warm => Ok(format!("{stored}; started warm, so not sealed")), + Start::Cold(cold) => Ok(format!("{stored}; {}", seal(root, cold)?)), + } +} + +/// After the last step of a run that started [`Start::Cold`], once [`close`] +/// has bounded its tree: every file under every target dated [`built`], then +/// the manifest. +fn seal(root: &Path, cold: &Cold) -> Result { let now = sources(root)?; let moved: Vec<&String> = cold .sources @@ -206,15 +238,8 @@ pub fn seal(root: &Path, cold: &Cold) -> Result { if !moved.is_empty() { return Err(format!("a step wrote tracked sources, which no entry can describe: {moved:?}")); } - let (mut files, mut bytes) = (0u64, 0u64); for target in targets(root)? { - age(&target, &mut files, &mut bytes)?; - } - if bytes > LIMIT { - return Err(format!( - "{bytes} B in {files} files under the targets, above the {LIMIT} B an entry may hold: \ - saved, it could evict the guest entry or the next host one" - )); + age(&target)?; } let head = crate::sync::git(root, &["rev-parse", "HEAD"])?; let mut text = format!("{head}\n{}\n", cold.runner); @@ -222,12 +247,7 @@ pub fn seal(root: &Path, cold: &Cold) -> Result { text.push_str(&format!("{hash} {path}\n")); } fs::write(root.join(MANIFEST), text).map_err(|e| format!("write {MANIFEST}: {e}"))?; - Ok(format!( - "{} sources, built on {}; {files} files, {bytes} B of the {LIMIT} B an entry may hold, \ - dated as built", - cold.sources.len(), - cold.runner, - )) + Ok(format!("sealed: {} sources, built on {}, every target dated as built", cold.sources.len(), cold.runner)) } /// The commit an entry was built from, the runner, and its sources. @@ -333,19 +353,38 @@ fn restored(root: &Path) -> Result, String> { /// Date everything under `dir`, and `dir`, as [`built`]. A symbolic link is /// left alone: dating one dates whatever it names. -fn age(dir: &Path, files: &mut u64, bytes: &mut u64) -> Result<(), String> { +fn age(dir: &Path) -> Result<(), String> { for entry in fs::read_dir(dir).map_err(|e| format!("read {}: {e}", dir.display()))? { let entry = entry.map_err(|e| format!("read {}: {e}", dir.display()))?; let meta = entry.metadata().map_err(|e| format!("{}: {e}", entry.path().display()))?; if meta.is_dir() { - age(&entry.path(), files, bytes)?; + age(&entry.path())?; } else if meta.is_file() { date(&entry.path(), built())?; + } + } + date(dir, built()) +} + +/// Count the files under `dir` and their bytes as an archive of it holds them: +/// a symbolic link is stored as a link, and a path that does not exist as +/// nothing. +fn size(dir: &Path, files: &mut u64, bytes: &mut u64) -> Result<(), String> { + let entries = match fs::read_dir(dir) { + Err(e) if e.kind() == ErrorKind::NotFound => return Ok(()), + entries => entries.map_err(|e| format!("read {}: {e}", dir.display()))?, + }; + for entry in entries { + let entry = entry.map_err(|e| format!("read {}: {e}", dir.display()))?; + let meta = entry.metadata().map_err(|e| format!("{}: {e}", entry.path().display()))?; + if meta.is_dir() { + size(&entry.path(), files, bytes)?; + } else if meta.is_file() { *files += 1; *bytes += meta.len(); } } - date(dir, built()) + Ok(()) } fn date(path: &Path, when: SystemTime) -> Result<(), String> { @@ -594,22 +633,32 @@ pub fn word(_: proc_macro::TokenStream) -> proc_macro::TokenStream { } } - /// Targets holding one byte more than the bound between them are refused - /// at the seal, and get no manifest; at the bound they are sealed. The - /// bound's bytes are a hole `set_len` made, so no test writes them. + /// What a save would store is the files under [`PATHS`], `~` being the + /// home: one byte above the bound, between a target and the home's + /// registry, is refused whether the run started warm or cold, and a cold + /// tree refused gets no manifest; at the bound both close, and a target no + /// save stores is not counted. The bound's bytes are a hole `set_len` + /// made, so no test writes them. #[test] - fn an_entry_above_its_bound_is_refused() { + fn what_a_save_would_store_is_refused_above_the_bound_warm_or_cold() { let tmp = TempDir::new("cicache-bound"); - let cold = cold_repo(&tmp, RUNNER); - write(&tmp, "kernel/target/one", "1"); + let home = tmp.join("home"); + let cold = Start::Cold(cold_repo(&tmp, RUNNER)); + write(&home, ".cargo/registry/cache/one", "1"); + write(&tmp, "tests/target/unsaved", "1"); let big = tmp.join("target/debug/big"); fs::create_dir_all(big.parent().unwrap()).unwrap(); fs::File::create(&big).unwrap().set_len(LIMIT).unwrap(); - let refusal = seal(&tmp, &cold).expect_err("targets one byte above the bound"); - assert!(refusal.starts_with(&format!("{} B", LIMIT + 1)), "{refusal}"); + for start in [&Start::Warm, &cold] { + let refusal = close_at(&tmp, &home, start).expect_err("one byte above the bound"); + assert!(refusal.starts_with(&format!("{} B", LIMIT + 1)), "{refusal}"); + } assert!(!tmp.join(MANIFEST).exists(), "a refused tree carries a manifest"); - fs::remove_file(tmp.join("kernel/target/one")).unwrap(); - seal(&tmp, &cold).expect("targets at the bound"); + fs::remove_file(home.join(".cargo/registry/cache/one")).unwrap(); + for start in [&Start::Warm, &cold] { + close_at(&tmp, &home, start).expect("at the bound"); + } + assert!(tmp.join(MANIFEST).exists(), "a tree at the bound is sealed"); } /// One commit of `a.rs`, read cold on `runner`. From 79230510927ca7cf2565ae0c6e3f1f1bc68824fb Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 18:59:37 +0200 Subject: [PATCH 13/20] The seal alone bounds the tree, and a job that names the host cache runs the driver and nothing else A warm tree keeps each unit its build wrote under a new name beside the one it replaced: a moved fork pin or a lock bump renames every unit above it, and cargo deletes none. So a warm run's sum could red a pull request whose cold tree fits, and nothing in that pull request could clear it; and a pull request's run is never saved, so its bound protected nothing. `cicache::close` and its warm arm go. The bound moves into `cicache::seal`, which takes the `Cold` only a cold start makes, so a warm run has no bound to call. A landing that takes the cold tree past LIMIT is first refused by the next nightly's seal; the LIMIT issue records that, with an exit a gate can fail. Its "the tree no longer runs that tar or zstdmt" goes: main never ran them. `each_cache_has_one_writer` held the writer's job to four words, and neither `|| true` on its run line nor `if: success() || true` on its save names one: GitHub adds its implicit `success() &&` only to an `if:` that names no status function. In every job that names the host cache, ci.yml's `host` included, `the_driver_alone` now holds each line to an allow-list: every `run:` is `cargo run -- --ci host`, and no step has an `if:` of its own, a `shell:` or `continue-on-error`; no file holding such a job names `defaults`. Every line must be a plain key, a comment or a path of `path: |`, which refuses a folded continuation line, a quoted key, an alias and an anchor, so the separate alias check goes. `guest` and `tcg`, which name the guest cache, are not held to it: their `deps` step is the shell issues/build/the-build-runs-host-tools-outside-rust-and-qemu.md declares, and their serial-log upload runs on failure. `size` repeated `age`'s walk: `each_under` is the one walker, and the seal hands it a visitor that counts and one that dates. "As an archive holds them" goes with `size`: cargo hard-links each uplifted binary on Linux, and the sum counts every name whole. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- ...e-10-gb-only-through-one-measured-ratio.md | 23 ++-- src/ci.rs | 52 +++++--- src/cicache.rs | 118 ++++++++---------- 3 files changed, 103 insertions(+), 90 deletions(-) diff --git a/issues/build/the-host-caches-limit-reaches-the-10-gb-only-through-one-measured-ratio.md b/issues/build/the-host-caches-limit-reaches-the-10-gb-only-through-one-measured-ratio.md index 8cf13ba0ebc..3a259c864c3 100644 --- a/issues/build/the-host-caches-limit-reaches-the-10-gb-only-through-one-measured-ratio.md +++ b/issues/build/the-host-caches-limit-reaches-the-10-gb-only-through-one-measured-ratio.md @@ -20,13 +20,22 @@ guest entry's 3,281,375,938 B (run 36696295750's `tcg`), 2H + G is before 2H + G reaches 10 GB. The lowest measured is 3.08: run 36844536500 sealed 9553 MiB on macOS and saved 3,250,736,567 B. -No gate reads a stored size, and the tree no longer runs that `tar` or -`zstdmt`. If the targets compress worse, H moves toward the floor and nothing -reds. +No gate reads a stored size. If the targets compress worse, H moves toward the +floor and nothing reds. + +`LIMIT` is checked only where a tree is sealed, by a cold run. A pull +request's run is warm whenever an entry its runner can use exists, and a warm +tree is not bounded: it keeps each unit rebuilt under a new name beside the +one it replaced, so its sum could red a pull request whose cold tree fits. So +a landing that takes the cold tree past `LIMIT` is first refused by the next +nightly's seal, loudly: that run is red and saves nothing. Until an entry is +sealed again, every pull request restores the last sealed one. Owner: the host cache (`src/cicache.rs`). -**Exit condition.** A gate reds on the stored size itself: after nightly's -`host` saves, a step reads that entry's stored bytes and the newest guest -entry's from the repository's cache list, and fails when 2H + G or H + 2G -passes 10 GB. +**Exit condition.** Two gates that red: +- after nightly's `host` saves, a step reads that entry's stored bytes and the + newest guest entry's from the repository's cache list, and fails when + 2H + G or H + 2G passes 10 GB; +- the merge queue's `host` reds a landing whose cold tree passes `LIMIT`, + before `main` moves. diff --git a/src/ci.rs b/src/ci.rs index d6393868e51..ad5f496366d 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -468,8 +468,7 @@ fn run_control(root: &Path, control: &Control) -> Result { /// host triple for the same reason. /// /// In a job that carries the cache ([`cicache::carried`]) the restored entry is -/// read before any step, and after the last what its save would store is -/// refused above its bound, warm or cold, and a tree that started cold is +/// read before any step, and after the last a tree that started cold is /// sealed; a developer's tree keeps the dates its edits gave it. fn host(root: &Path) -> Vec { let tmp = toyos_tmpdir::TempDir::new("ci-host"); @@ -565,8 +564,8 @@ fn host(root: &Path) -> Vec { cargo(root, &["test", "--manifest-path", "toyos/Cargo.toml", "--target", &host_triple]) })); steps.push(step("nothing left in $TMPDIR or /tmp", || left_behind(&tmp, short, &before))); - if let Some(start) = start { - steps.push(step("the tree, bounded as a cache entry", || cicache::close(root, &start))); + if let Some(cicache::Start::Cold(cold)) = &start { + steps.push(step("the tree, bounded as a cache entry", || cicache::seal(root, cold))); } steps } @@ -1164,15 +1163,41 @@ mod tests { jobs } + /// Refuses a job's line unless it is a comment, a path of `path: |`, or a + /// plain key: so no alias, anchor, quoted key or folded line hides a step + /// from this reader. Every `run:` is the driver, and no step has an `if:` + /// of its own, a `shell:` or `continue-on-error`. + fn the_driver_alone(name: &str, job: &str, lines: &[&str]) { + let mut paths = None; + for line in lines { + let indent = line.len() - line.trim_start().len(); + let text = line.trim_start(); + if text.is_empty() || text.starts_with('#') || paths.is_some_and(|at| indent > at) { + continue; + } + let entry = text.strip_prefix("- ").unwrap_or(text); + let column = indent + text.len() - entry.len(); + let (key, value) = entry.split_once(": ").or(entry.strip_suffix(':').map(|key| (key, ""))).unwrap_or_default(); + let plain = !key.is_empty() && key.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'-' || b == b'_'); + assert!(plain, "{name} {job}: a line this reader cannot judge: {line:?}"); + paths = (key == "path" && value == "|").then_some(column); + match key { + "run" => assert_eq!(value, "cargo run -- --ci host", "{name} {job}"), + "if" => assert_eq!(column, 4, "{name} {job}: a step's own `if:`: {line:?}"), + "shell" | "continue-on-error" => panic!("{name} {job}: {line:?}"), + _ => {} + } + } + } + /// Exactly one job writes each cache, on the nightly, so what a pull request /// restores is one run's tree and never a race between two writers. A job /// that restores or saves the host cache builds the driver in - /// [`cicache::DRIVER`], so it carries the cache, names [`cicache::PATHS`], - /// and takes no step by an alias or lends one, which this reader cannot - /// follow; and the one that saves it restores nothing: its run is cold, and - /// a cold run is green only once its tree is sealed ([`cicache`]). No line - /// of that job names a status function that runs a step past a red one, or - /// `continue-on-error`, so its save follows only a green run. + /// [`cicache::DRIVER`], so it carries the cache, and names + /// [`cicache::PATHS`]; the one that saves it restores nothing: its run is + /// cold, and a cold run is green only once its tree is sealed + /// ([`cicache`]). Such a job's verdict is the driver's + /// ([`the_driver_alone`]), so its save follows only a green run. #[test] fn each_cache_has_one_writer() { let dir = repo_root().join(".github/workflows"); @@ -1208,11 +1233,8 @@ mod tests { assert_eq!(paths, &cicache::PATHS, "{name} {job}: the host cache's paths"); assert!(lines.contains(&carries.as_str()), "{name} {job}: the host cache without `{}`", carries.trim()); assert!(!save || caches.iter().all(|(s, ..)| *s), "{name} {job}: the host cache's writer restores"); - let past_a_red = ["always()", "cancelled()", "failure()", "continue-on-error"]; - let past = lines.iter().find(|l| past_a_red.iter().any(|word| l.contains(word))); - assert!(!save || past.is_none(), "{name} {job}: the host cache saved past a red step: {past:?}"); - let named = |l: &&str| ["- *", "- &"].iter().any(|by| l.trim_start().starts_with(by)); - assert!(!lines.iter().any(named), "{name} {job}: a step by an alias, or lent to one"); + assert!(!text.contains("defaults"), "{name}: a shell or directory for every step"); + the_driver_alone(&name, job, &lines); } if *save { writers.push((name.clone(), prefix.clone())); diff --git a/src/cicache.rs b/src/cicache.rs index 4afe0423015..e2f000b8e65 100644 --- a/src/cicache.rs +++ b/src/cicache.rs @@ -34,11 +34,11 @@ //! would make every crate depending on them stale. A job that builds it //! anywhere else runs as a developer's tree does. //! -//! **Every run that carries the cache refuses, after its last step, a tree -//! whose [`PATHS`] hold more than [`LIMIT`]**, `~` being `HOME` ([`close`]): a -//! cold one before it seals, so the save, which follows only a green run, -//! never stores it, and a warm one so that a pull request whose tree holds -//! more reds on its own run. +//! **A seal refuses a tree whose [`PATHS`] hold more than [`LIMIT`]**, `~` +//! being `HOME`, so the save, which follows only a green run, never stores +//! one. A warm tree is not bounded: it keeps each unit its build wrote under a +//! new name beside the one it replaced, so it sums more than a cold build of +//! its sources seals. use std::collections::{BTreeMap, BTreeSet}; use std::fs; @@ -192,21 +192,32 @@ fn cold(root: &Path, runner: &str, sources: Sources, why: String) -> Result<(Sta Ok((Start::Cold(Cold { runner: runner.to_string(), sources }), said)) } -/// After the last step of a job that carries the cache: what its save would -/// store, refused above [`LIMIT`], and a tree that started cold sealed. -pub fn close(root: &Path, start: &Start) -> Result { +/// After the last step of a run that started [`Start::Cold`]: the tree refused +/// if its [`PATHS`] hold more than [`LIMIT`], `~` being `HOME`, and otherwise +/// every file under every target dated [`built`], then the manifest. +pub fn seal(root: &Path, cold: &Cold) -> Result { let home = std::env::var_os("HOME").ok_or("HOME is unset, and the cache's paths name it as `~`")?; - close_at(root, Path::new(&home), start) + seal_at(root, Path::new(&home), cold) } -fn close_at(root: &Path, home: &Path, start: &Start) -> Result { +fn seal_at(root: &Path, home: &Path, cold: &Cold) -> Result { let (mut files, mut bytes) = (0u64, 0u64); for path in PATHS { let dir = match path.strip_prefix("~/") { Some(under) => home.join(under), None => root.join(path), }; - size(&dir, &mut files, &mut bytes)?; + // A path the save finds nothing at stores nothing. + if !dir.try_exists().map_err(|e| format!("{}: {e}", dir.display()))? { + continue; + } + each_under(&dir, &mut |_, meta| { + if meta.is_file() { + files += 1; + bytes += meta.len(); + } + Ok(()) + })?; } if bytes > LIMIT { return Err(format!( @@ -214,17 +225,6 @@ fn close_at(root: &Path, home: &Path, start: &Start) -> Result { hold: saved, it could evict the guest entry or the next host one" )); } - let stored = format!("{files} files, {bytes} B of the {LIMIT} B an entry may hold"); - match start { - Start::Warm => Ok(format!("{stored}; started warm, so not sealed")), - Start::Cold(cold) => Ok(format!("{stored}; {}", seal(root, cold)?)), - } -} - -/// After the last step of a run that started [`Start::Cold`], once [`close`] -/// has bounded its tree: every file under every target dated [`built`], then -/// the manifest. -fn seal(root: &Path, cold: &Cold) -> Result { let now = sources(root)?; let moved: Vec<&String> = cold .sources @@ -239,7 +239,8 @@ fn seal(root: &Path, cold: &Cold) -> Result { return Err(format!("a step wrote tracked sources, which no entry can describe: {moved:?}")); } for target in targets(root)? { - age(&target)?; + each_under(&target, &mut |path, _| date(path, built()))?; + date(&target, built())?; } let head = crate::sync::git(root, &["rev-parse", "HEAD"])?; let mut text = format!("{head}\n{}\n", cold.runner); @@ -247,7 +248,12 @@ fn seal(root: &Path, cold: &Cold) -> Result { text.push_str(&format!("{hash} {path}\n")); } fs::write(root.join(MANIFEST), text).map_err(|e| format!("write {MANIFEST}: {e}"))?; - Ok(format!("sealed: {} sources, built on {}, every target dated as built", cold.sources.len(), cold.runner)) + Ok(format!( + "{files} files, {bytes} B of the {LIMIT} B an entry may hold; sealed: {} sources, built on {}, \ + every target dated as built", + cold.sources.len(), + cold.runner + )) } /// The commit an entry was built from, the runner, and its sources. @@ -351,37 +357,18 @@ fn restored(root: &Path) -> Result, String> { Ok(found) } -/// Date everything under `dir`, and `dir`, as [`built`]. A symbolic link is -/// left alone: dating one dates whatever it names. -fn age(dir: &Path) -> Result<(), String> { +/// Hands `visit` every file and directory under `dir`, with its metadata. A +/// symbolic link is neither followed nor visited: dating one dates whatever it +/// names. +fn each_under(dir: &Path, visit: &mut impl FnMut(&Path, &fs::Metadata) -> Result<(), String>) -> Result<(), String> { for entry in fs::read_dir(dir).map_err(|e| format!("read {}: {e}", dir.display()))? { let entry = entry.map_err(|e| format!("read {}: {e}", dir.display()))?; let meta = entry.metadata().map_err(|e| format!("{}: {e}", entry.path().display()))?; if meta.is_dir() { - age(&entry.path())?; - } else if meta.is_file() { - date(&entry.path(), built())?; + each_under(&entry.path(), visit)?; } - } - date(dir, built()) -} - -/// Count the files under `dir` and their bytes as an archive of it holds them: -/// a symbolic link is stored as a link, and a path that does not exist as -/// nothing. -fn size(dir: &Path, files: &mut u64, bytes: &mut u64) -> Result<(), String> { - let entries = match fs::read_dir(dir) { - Err(e) if e.kind() == ErrorKind::NotFound => return Ok(()), - entries => entries.map_err(|e| format!("read {}: {e}", dir.display()))?, - }; - for entry in entries { - let entry = entry.map_err(|e| format!("read {}: {e}", dir.display()))?; - let meta = entry.metadata().map_err(|e| format!("{}: {e}", entry.path().display()))?; - if meta.is_dir() { - size(&entry.path(), files, bytes)?; - } else if meta.is_file() { - *files += 1; - *bytes += meta.len(); + if meta.is_dir() || meta.is_file() { + visit(&entry.path(), &meta)?; } } Ok(()) @@ -472,7 +459,7 @@ mod tests { panic!("a tree with no target is cold") }; build(writer); - seal(writer, &cold).unwrap(); + seal_at(writer, &writer.join("home"), &cold).unwrap(); } /// The entry at `writer`, restored under a fresh checkout of `origin`. @@ -635,29 +622,24 @@ pub fn word(_: proc_macro::TokenStream) -> proc_macro::TokenStream { /// What a save would store is the files under [`PATHS`], `~` being the /// home: one byte above the bound, between a target and the home's - /// registry, is refused whether the run started warm or cold, and a cold - /// tree refused gets no manifest; at the bound both close, and a target no - /// save stores is not counted. The bound's bytes are a hole `set_len` - /// made, so no test writes them. + /// registry, is refused and gets no manifest; at the bound the tree is + /// sealed, and a target no save stores is not counted. The bound's bytes + /// are a hole `set_len` made, so no test writes them. #[test] - fn what_a_save_would_store_is_refused_above_the_bound_warm_or_cold() { + fn a_seal_refuses_what_its_save_would_store_above_the_bound() { let tmp = TempDir::new("cicache-bound"); let home = tmp.join("home"); - let cold = Start::Cold(cold_repo(&tmp, RUNNER)); + let cold = cold_repo(&tmp, RUNNER); write(&home, ".cargo/registry/cache/one", "1"); write(&tmp, "tests/target/unsaved", "1"); let big = tmp.join("target/debug/big"); fs::create_dir_all(big.parent().unwrap()).unwrap(); fs::File::create(&big).unwrap().set_len(LIMIT).unwrap(); - for start in [&Start::Warm, &cold] { - let refusal = close_at(&tmp, &home, start).expect_err("one byte above the bound"); - assert!(refusal.starts_with(&format!("{} B", LIMIT + 1)), "{refusal}"); - } + let refusal = seal_at(&tmp, &home, &cold).expect_err("one byte above the bound"); + assert!(refusal.starts_with(&format!("{} B", LIMIT + 1)), "{refusal}"); assert!(!tmp.join(MANIFEST).exists(), "a refused tree carries a manifest"); fs::remove_file(home.join(".cargo/registry/cache/one")).unwrap(); - for start in [&Start::Warm, &cold] { - close_at(&tmp, &home, start).expect("at the bound"); - } + seal_at(&tmp, &home, &cold).expect("at the bound"); assert!(tmp.join(MANIFEST).exists(), "a tree at the bound is sealed"); } @@ -697,14 +679,14 @@ pub fn word(_: proc_macro::TokenStream) -> proc_macro::TokenStream { let cold = cold_repo(&tmp, RUNNER); write(&tmp, "target/debug/deps/x", "x"); write(&tmp, "userland/target/y", "y"); - seal(&tmp, &cold).unwrap(); + seal_at(&tmp, &tmp.join("home"), &cold).unwrap(); for file in ["target/debug/deps/x", "target/debug", "userland/target/y"] { assert_eq!(modified(&tmp.join(file)).unwrap(), built(), "{file}"); } fs::remove_file(tmp.join(MANIFEST)).unwrap(); for bytes in ["b\n", "a\n"] { write(&tmp, "a.rs", bytes); - let refusal = seal(&tmp, &cold).unwrap_err(); + let refusal = seal_at(&tmp, &tmp.join("home"), &cold).unwrap_err(); assert!(refusal.contains("a.rs"), "{bytes:?}: {refusal}"); } } @@ -730,7 +712,7 @@ pub fn word(_: proc_macro::TokenStream) -> proc_macro::TokenStream { let cold = cold_repo(&tmp, &runner_in(&image).unwrap()); write(&tmp, "target/debug/x", "x"); write(&tmp, "kernel/target/y", "y"); - seal(&tmp, &cold).unwrap(); + seal_at(&tmp, &tmp.join("home"), &cold).unwrap(); let mut other = image.clone(); other.insert(name, "another"); let (start, said) = open(&tmp, &runner_in(&other).unwrap(), SystemTime::now()).unwrap(); @@ -749,7 +731,7 @@ pub fn word(_: proc_macro::TokenStream) -> proc_macro::TokenStream { let tmp = TempDir::new("cicache-clock"); let cold = cold_repo(&tmp, RUNNER); write(&tmp, "target/debug/x", "x"); - seal(&tmp, &cold).unwrap(); + seal_at(&tmp, &tmp.join("home"), &cold).unwrap(); let refusal = open(&tmp, RUNNER, built()).err().expect("a clock at the entry's date"); assert!(refusal.contains("clock"), "{refusal}"); } From 41398684021949669ce337edb7e7a3951cbc0be6 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 20:05:24 +0200 Subject: [PATCH 14/20] Round 9: the workflows are read as YAML, and only the job that saves the host entry seals it `each_cache_has_one_writer` read the workflows as text, and three rounds found spellings that escaped it: a quoted key, a comment after `jobs:`, an unknown job key, a runner variable in `env:`, an anchor and an alias, a deleted run step. - `src/workflow.rs` reads every workflow with yaml-rust2's event parser. An alias is resolved as GitHub resolves it, and every node an anchor, an alias or a tag made is marked. A key given twice, a key that is not a plain scalar, and anything but one document are refused. It is compiled for tests alone, beside its two tests, and its accessors are what another gate over `.github/workflows/` calls. - yaml-rust2 0.13, as a dev-dependency with its encoding feature off: the pure-Rust YAML 1.2 parser with 61,444,840 downloads on crates.io, whose events carry the anchors, aliases and tags a loader resolves away. - The gate works on that structure. Every cache step's key and restore keys must begin, up to their first expression, with a known cache's prefix, and the combined action is refused. The jobs that name the host cache must be exactly ci.yml's `host`, the reader, and nightly.yml's `host`, the writer. Each is held to a closed allow-list: its workflow's keys, its own keys, its `if:`, `env:` with `CARGO_TARGET_DIR` alone, and its steps in order as checkout, restore and the run, or checkout, the run and save. No step has a condition of its own, no anchor, alias or tag is in the job, and the host cache's paths, key and restore key are exact. - Only the writer seals. `cargo run -- --ci seal` is `host` from a cold tree, which it refuses if anything was restored, then sealed and bounded by `LIMIT`. nightly.yml's `host` runs it, and ci.yml's `host` runs `--ci host`, whose read never makes a `Cold` and so never seals. A pull request's run, warm or cold, is never refused by `LIMIT`. - The LIMIT issue's residual says what is true now. The guest-cache issue records that its jobs are not held to the allow-list, and its exit names that allow-list. A new issue records the two workflow gates that still read text, with the two patches that pass them. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- .github/workflows/nightly.yml | 8 +- Cargo.lock | 35 +++ Cargo.toml | 3 + ...and-its-writer-restores-before-it-saves.md | 8 +- ...e-10-gb-only-through-one-measured-ratio.md | 14 +- ...rkflow-gates-read-the-workflows-as-text.md | 17 ++ src/ci.rs | 269 +++++++++++------- src/cicache.rs | 120 ++++---- src/lib.rs | 3 + src/workflow.rs | 223 +++++++++++++++ 10 files changed, 536 insertions(+), 164 deletions(-) create mode 100644 issues/build/two-workflow-gates-read-the-workflows-as-text.md create mode 100644 src/workflow.rs diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 1c93b0fabb1..5e340472273 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -17,9 +17,9 @@ concurrency: jobs: # The host cache's writer: only main's entries are readable from every - # branch. It restores nothing, so its run is cold, and a cold run is green - # only once src/cicache.rs has sealed its tree. On a branch it would only - # repeat that branch's pull request. + # branch. It restores nothing, and `seal` is green only once src/cicache.rs + # has sealed the tree the save stores. On a branch it would only repeat that + # branch's pull request. host: if: github.ref == 'refs/heads/main' runs-on: ubuntu-24.04 @@ -33,7 +33,7 @@ jobs: with: fetch-depth: 0 - - run: cargo run -- --ci host + - run: cargo run -- --ci seal - uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 with: diff --git a/Cargo.lock b/Cargo.lock index cc05a45c729..6065e78cb97 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -38,6 +38,12 @@ version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +[[package]] +name = "arraydeque" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d902e3d592a523def97af8f317b08ce16b7ab854c1985a0c671e6f15cebc236" + [[package]] name = "autocfg" version = "1.5.0" @@ -375,6 +381,24 @@ dependencies = [ "foldhash 0.2.0", ] +[[package]] +name = "hashbrown" +version = "0.17.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" +dependencies = [ + "foldhash 0.2.0", +] + +[[package]] +name = "hashlink" +version = "0.12.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a596f1b20ed2cc5ecac41a164aaebc7258057060f06c0cf7a2ba3991ee7990fb" +dependencies = [ + "hashbrown 0.17.1", +] + [[package]] name = "heck" version = "0.5.0" @@ -965,6 +989,7 @@ dependencies = [ "toyos-wallclock", "toyos-xhci", "uuid", + "yaml-rust2", ] [[package]] @@ -1589,6 +1614,16 @@ dependencies = [ "wasmparser", ] +[[package]] +name = "yaml-rust2" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57e5b818a27a4cd30884ea380857a5e56f7ec3ba24a3990a3cc0b95af3238e18" +dependencies = [ + "arraydeque", + "hashlink", +] + [[package]] name = "zerocopy" version = "0.8.47" diff --git a/Cargo.toml b/Cargo.toml index f7859848977..0f9ffb5001c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -177,6 +177,9 @@ toyos-sched = { path = "toyos-sched" } # The Bulk-Only phases, so the harness judging a wedge reads the word # `toyos_xhci::bot::Phase` declares instead of spelling it a second time. toyos-xhci = { path = "toyos-xhci" } +# The workflows' YAML, parsed to events that keep every anchor, alias and tag, +# for the gates over `.github/workflows/` (`src/workflow.rs`). +yaml-rust2 = { version = "0.13", default-features = false } [patch.crates-io] loom = { git = "https://github.com/ToyOSOrg/loom", branch = "toyos" } diff --git a/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md b/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md index 7d112e0083c..6b9a856c72b 100644 --- a/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md +++ b/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md @@ -19,4 +19,10 @@ every night: 4,002,930,524 B at the host's `LIMIT` (`src/cicache.rs`), or night's host entry unless a pull request has read the entry since the guest restore; every pull request then runs cold, and nothing reds. -Done when the guest entry is written cold, read by content, and bounded. +Its jobs are not held to the allow-list `src/ci.rs`'s +`each_cache_has_one_writer` holds the host cache's jobs to: `|| true` on +`tcg`'s run line would let its save store a red run's tree, and the gate stays +green. + +Done when the guest entry is written cold, read by content, and bounded, and +its jobs are held to that allow-list, which #671 carries. diff --git a/issues/build/the-host-caches-limit-reaches-the-10-gb-only-through-one-measured-ratio.md b/issues/build/the-host-caches-limit-reaches-the-10-gb-only-through-one-measured-ratio.md index 3a259c864c3..5e314564c12 100644 --- a/issues/build/the-host-caches-limit-reaches-the-10-gb-only-through-one-measured-ratio.md +++ b/issues/build/the-host-caches-limit-reaches-the-10-gb-only-through-one-measured-ratio.md @@ -23,13 +23,13 @@ sealed 9553 MiB on macOS and saved 3,250,736,567 B. No gate reads a stored size. If the targets compress worse, H moves toward the floor and nothing reds. -`LIMIT` is checked only where a tree is sealed, by a cold run. A pull -request's run is warm whenever an entry its runner can use exists, and a warm -tree is not bounded: it keeps each unit rebuilt under a new name beside the -one it replaced, so its sum could red a pull request whose cold tree fits. So -a landing that takes the cold tree past `LIMIT` is first refused by the next -nightly's seal, loudly: that run is red and saves nothing. Until an entry is -sealed again, every pull request restores the last sealed one. +`LIMIT` is checked only by nightly's `host`, the one job that saves an entry, +before it seals its tree. A pull request's run and the merge queue's, warm or +cold, never seal and are never refused by `LIMIT`. So a landing that takes the +cold tree past `LIMIT` is first refused by the next nightly's seal, loudly: +that run is red and saves nothing. Until an entry is sealed again, a pull +request restores the last sealed one, or runs cold once the runner image has +moved; either way its verdict is its steps'. Owner: the host cache (`src/cicache.rs`). diff --git a/issues/build/two-workflow-gates-read-the-workflows-as-text.md b/issues/build/two-workflow-gates-read-the-workflows-as-text.md new file mode 100644 index 00000000000..c9bbc068cb6 --- /dev/null +++ b/issues/build/two-workflow-gates-read-the-workflows-as-text.md @@ -0,0 +1,17 @@ +--- +status: open +kind: tooling +opened: 2026-10-01 +--- + +# Two workflow gates read the workflows as text + +`src/ci.rs`'s `workflows_run_against_main_on_hosted_runners` reads `runs-on:` +and `pull_request:` off single lines, and +`the_required_check_is_a_job_on_every_pull_request` finds ci.yml's triggers and +its `host` by substring. A spelling YAML reads the same way passes them. Each +patch below, alone on `publish.yml`, left both tests green (EXIT 0): +- `runs-on:` with its label on the next line, `- self-hosted`; +- `pull_request: {branches: [dev]}` beside `workflow_dispatch`. + +Done when both read `src/workflow.rs`'s structure. diff --git a/src/ci.rs b/src/ci.rs index ad5f496366d..7d90228b261 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -36,6 +36,8 @@ const USAGE: &str = "cargo run -- --ci , where is one of: host every host test: the build system, the harness's own checks, the host workspace, the licences of what ships, clippy, the model controls, userland and the SDK (ci.yml, nightly) + seal `host` from a cold tree, then that tree sealed as the host + cache's entry, which the step after it saves (nightly) toolchain publish this tree's toolchain if nobody has (nightly) guest the guest suite (nightly) publish put main's SDK crates on crates.io (publish.yml)"; @@ -43,6 +45,7 @@ const USAGE: &str = "cargo run -- --ci , where is one of: #[derive(Debug, PartialEq, Eq)] enum Job { Host, + Seal, Toolchain, Guest, Publish, @@ -51,6 +54,7 @@ enum Job { fn parse(words: &[String]) -> Result { let job = match words.first().map(String::as_str) { Some("host") => Job::Host, + Some("seal") => Job::Seal, Some("toolchain") => Job::Toolchain, Some("guest") => Job::Guest, Some("publish") => Job::Publish, @@ -69,7 +73,8 @@ pub fn dispatch(root: &Path, args: &[String]) { std::process::exit(2); }); let steps = match &job { - Job::Host => host(root), + Job::Host => host(root, false), + Job::Seal => host(root, true), Job::Toolchain => vec![step("the toolchain release", || release::ensure_published(root))], Job::Guest => guest(root, &suite_args(&["--jobs", "1"])), Job::Publish => vec![step("the SDK crates on crates.io", || publish(root))], @@ -468,9 +473,16 @@ fn run_control(root: &Path, control: &Control) -> Result { /// host triple for the same reason. /// /// In a job that carries the cache ([`cicache::carried`]) the restored entry is -/// read before any step, and after the last a tree that started cold is -/// sealed; a developer's tree keeps the dates its edits gave it. -fn host(root: &Path) -> Vec { +/// read before any step. `seal`'s job, the one that saves the entry, instead +/// starts from a cold tree and seals it after the last step. A developer's +/// tree keeps the dates its edits gave it. +fn host(root: &Path, seals: bool) -> Vec { + let carried = cicache::carried(root, &std::env::current_exe().expect("the driver's own path")); + if seals && !carried { + return vec![step("a cold tree, for the entry", || { + Err(format!("only a job whose driver is built in {} seals its tree", cicache::DRIVER)) + })]; + } let tmp = toyos_tmpdir::TempDir::new("ci-host"); let short = Path::new(toyos_tmpdir::SHORT_BASE); let before = toyos_tmpdir::gone_roots(short); @@ -479,16 +491,20 @@ fn host(root: &Path) -> Vec { std::env::set_var("TMPDIR", tmp.path()); let host_triple = crate::toolchain::host_triple(); let mut steps = Vec::new(); - let mut start = None; - if cicache::carried(root, &std::env::current_exe().expect("the driver's own path")) { + let mut cold = None; + if carried { carry(); - steps.push(step("the cache entry, read by content", || { - let (found, said) = cicache::read(root)?; - start = Some(found); - Ok(said) - })); + steps.push(if seals { + step("a cold tree, for the entry", || { + let (found, said) = cicache::cold(root)?; + cold = Some(found); + Ok(said) + }) + } else { + step("the cache entry, read by content", || cicache::read(root)) + }); // Every step after an unreadable entry would be judged against it. - if start.is_none() { + if steps[0].verdict.is_err() { return steps; } } @@ -564,8 +580,8 @@ fn host(root: &Path) -> Vec { cargo(root, &["test", "--manifest-path", "toyos/Cargo.toml", "--target", &host_triple]) })); steps.push(step("nothing left in $TMPDIR or /tmp", || left_behind(&tmp, short, &before))); - if let Some(cicache::Start::Cold(cold)) = &start { - steps.push(step("the tree, bounded as a cache entry", || cicache::seal(root, cold))); + if let Some(cold) = &cold { + steps.push(step("the tree, sealed as the host cache's entry", || cicache::seal(root, cold))); } steps } @@ -918,6 +934,7 @@ fn at_tip(ls_remote: &str, head: &str) -> Result<(), String> { #[cfg(test)] mod tests { use super::*; + use crate::workflow::{self, Node}; /// A deterministic control on `host`'s `std::env::set_var("TMPDIR", ...)`: /// delete that line and every child writes to the real `$TMPDIR` instead of @@ -1041,6 +1058,7 @@ mod tests { #[test] fn a_job_is_named_and_takes_nothing_after_it() { assert_eq!(parse(&words("host")), Ok(Job::Host)); + assert_eq!(parse(&words("seal")), Ok(Job::Seal)); assert_eq!(parse(&words("guest")), Ok(Job::Guest)); assert!(parse(&words("guest 3/12")).is_err()); assert!(parse(&words("tcg")).is_err()); @@ -1151,103 +1169,160 @@ mod tests { assert_eq!(seen, 3, "ci.yml, nightly.yml and publish.yml"); } - /// Each job of a workflow: its name and its lines. - fn jobs(text: &str) -> Vec<(&str, Vec<&str>)> { - let mut jobs: Vec<(&str, Vec<&str>)> = Vec::new(); - for line in text.split_once("\njobs:\n").map_or("", |(_, jobs)| jobs).lines() { - match line.strip_prefix(" ").and_then(|l| l.strip_suffix(':')) { - Some(name) if !name.starts_with([' ', '#']) => jobs.push((name, Vec::new())), - _ => jobs.last_mut().into_iter().for_each(|(_, lines)| lines.push(line)), - } + /// Each cache, by what every key and restore key of a step that names it + /// begins with, up to its first expression. + const CACHES: [&str; 2] = [cicache::SEALED, "guest-"]; + + /// What the host cache's key and its reader's restore key say after + /// [`cicache::SEALED`]: one entry per run, found by the runner's OS and + /// architecture. + const HOST_KEYS: [&str; 2] = ["${{ runner.os }}-${{ runner.arch }}-${{ github.run_id }}", "${{ runner.os }}-${{ runner.arch }}-"]; + + /// The jobs that name the host cache, its one reader and its one writer: + /// the condition each runs under, since a skipped job is a green check, and + /// its steps. The reader restores before its run; the writer restores + /// nothing and saves after its run. + const HOST_CACHE: [(&str, &str, &str, [&str; 3]); 2] = [ + ("ci.yml", "host", "github.event_name == 'merge_group' || github.event.pull_request.draft == false", [ + "actions/checkout", + "actions/cache/restore", + "run: cargo run -- --ci host", + ]), + ("nightly.yml", "host", "github.ref == 'refs/heads/main'", [ + "actions/checkout", + "run: cargo run -- --ci seal", + "actions/cache/save", + ]), + ]; + + /// Every key of `node` is one of `allowed`. + fn keys(at: &str, node: &Node, allowed: &[&str]) { + for (key, value) in node.map().expect(at) { + assert!(allowed.contains(&key.as_str()), "{at}: line {}: `{key}`, where only {allowed:?} may be", value.line); } - jobs } - /// Refuses a job's line unless it is a comment, a path of `path: |`, or a - /// plain key: so no alias, anchor, quoted key or folded line hides a step - /// from this reader. Every `run:` is the driver, and no step has an `if:` - /// of its own, a `shell:` or `continue-on-error`. - fn the_driver_alone(name: &str, job: &str, lines: &[&str]) { - let mut paths = None; - for line in lines { - let indent = line.len() - line.trim_start().len(); - let text = line.trim_start(); - if text.is_empty() || text.starts_with('#') || paths.is_some_and(|at| indent > at) { - continue; - } - let entry = text.strip_prefix("- ").unwrap_or(text); - let column = indent + text.len() - entry.len(); - let (key, value) = entry.split_once(": ").or(entry.strip_suffix(':').map(|key| (key, ""))).unwrap_or_default(); - let plain = !key.is_empty() && key.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'-' || b == b'_'); - assert!(plain, "{name} {job}: a line this reader cannot judge: {line:?}"); - paths = (key == "path" && value == "|").then_some(column); - match key { - "run" => assert_eq!(value, "cargo run -- --ci host", "{name} {job}"), - "if" => assert_eq!(column, 4, "{name} {job}: a step's own `if:`: {line:?}"), - "shell" | "continue-on-error" => panic!("{name} {job}: {line:?}"), - _ => {} + /// The action a step uses, without its version and in GitHub's case: + /// GitHub does not tell `Actions/Cache` from `actions/cache`. + fn action(at: &str, step: &Node) -> Option { + let uses = step.get("uses").expect(at)?; + Some(uses.str().expect(at).split('@').next().unwrap_or_default().to_ascii_lowercase()) + } + + /// Each step of `job` that restores or saves a cache: the cache its keys + /// name, and whether it saves. A key that names no cache, a step whose + /// keys name two, and the action that restores and saves in one are + /// refused. + fn caches(at: &str, job: &Node) -> Vec<(&'static str, bool)> { + let Some(steps) = job.get("steps").expect(at) else { return Vec::new() }; + let mut found = Vec::new(); + for step in steps.seq().expect(at) { + let save = match action(at, step).as_deref() { + Some("actions/cache") => panic!("{at}: line {}: the combined action saves too", step.line), + Some("actions/cache/restore") => false, + Some("actions/cache/save") => true, + _ => continue, + }; + let with = step.get("with").expect(at).unwrap_or_else(|| panic!("{at}: line {}: a cache step names no key", step.line)); + let mut keys = vec![with.get("key").expect(at).unwrap_or_else(|| panic!("{at}: a cache step names no key"))]; + keys.extend(with.get("restore-keys").expect(at)); + let heads: Vec<&str> = keys + .iter() + .flat_map(|key| key.str().expect(at).lines().map(str::trim).filter(|key| !key.is_empty())) + .map(|key| key.split("${{").next().unwrap_or_default()) + .collect(); + let cache = CACHES + .into_iter() + .find(|cache| heads.iter().all(|head| head == cache)) + .unwrap_or_else(|| panic!("{at}: line {}: keys {heads:?}, where each names one of {CACHES:?}", step.line)); + found.push((cache, save)); + } + found + } + + /// One step of a job that names the host cache, held to the keys it may + /// have: its action, or `run:` and its line. + fn host_step(at: &str, step: &Node) -> String { + if let Some(run) = step.get("run").expect(at) { + keys(at, step, &["run"]); + return format!("run: {}", run.str().expect(at)); + } + keys(at, step, &["uses", "with"]); + let action = action(at, step).unwrap_or_else(|| panic!("{at}: line {}: a step with no `run:` or `uses:`", step.line)); + let with = step.get("with").expect(at); + let restore = match action.as_str() { + "actions/checkout" => { + with.into_iter().for_each(|with| keys(at, with, &["fetch-depth"])); + return action; } + "actions/cache/restore" => Some(HOST_KEYS[1]), + "actions/cache/save" => None, + other => panic!("{at}: line {}: `{other}`", step.line), + }; + let with = with.unwrap_or_else(|| panic!("{at}: line {}: a cache step with no `with:`", step.line)); + keys(at, with, &["path", "key", "restore-keys"]); + let text = |key: &str| with.get(key).expect(at).map(|node| node.str().expect(at).to_string()); + let paths = text("path").unwrap_or_default(); + let paths: Vec<&str> = paths.lines().map(str::trim).filter(|path| !path.is_empty()).collect(); + assert_eq!(paths, cicache::PATHS, "{at}: line {}: the host cache's paths", with.line); + let sealed = |rest: &str| Some(format!("{}{rest}", cicache::SEALED)); + assert_eq!(text("key"), sealed(HOST_KEYS[0]), "{at}: line {}: the host cache's key", with.line); + assert_eq!(text("restore-keys"), restore.and_then(sealed), "{at}: line {}: its restore key", with.line); + action + } + + /// What a job that names the host cache may be: one of [`HOST_CACHE`], its + /// keys and its workflow's in a closed allow-list, with no anchor, alias or + /// tag in it. No step has a condition of its own, so the writer's save has + /// GitHub's, `success()`, and follows only a green run; the job's verdict + /// is the driver's. + fn the_driver_alone(at: &str, workflow: &Node, job: &Node, condition: &str, steps: [&str; 3]) { + keys(at, workflow, &["name", "on", "concurrency", "jobs"]); + if let Some(line) = job.mark() { + panic!("{at}: line {line}: an anchor, an alias or a tag"); } + keys(at, job, &["if", "runs-on", "timeout-minutes", "env", "steps"]); + let condition_given = job.get("if").expect(at).map(|node| node.str().expect(at)); + assert_eq!(condition_given, Some(condition), "{at}: the condition it runs under"); + let env = job.get("env").expect(at).unwrap_or_else(|| panic!("{at}: no `env:`")); + let env: Vec<(&str, &str)> = env.map().expect(at).iter().map(|(k, v)| (k.as_str(), v.str().expect(at))).collect(); + assert_eq!(env, [("CARGO_TARGET_DIR", cicache::DRIVER)], "{at}: the job's environment"); + let given = job.get("steps").expect(at).unwrap_or_else(|| panic!("{at}: no `steps:`")); + let given: Vec = given.seq().expect(at).iter().map(|step| host_step(at, step)).collect(); + assert_eq!(given, steps, "{at}: its steps"); } /// Exactly one job writes each cache, on the nightly, so what a pull request - /// restores is one run's tree and never a race between two writers. A job - /// that restores or saves the host cache builds the driver in - /// [`cicache::DRIVER`], so it carries the cache, and names - /// [`cicache::PATHS`]; the one that saves it restores nothing: its run is - /// cold, and a cold run is green only once its tree is sealed - /// ([`cicache`]). Such a job's verdict is the driver's - /// ([`the_driver_alone`]), so its save follows only a green run. + /// restores is one run's tree and never a race between two writers. The + /// host cache is named by ci.yml's `host`, which reads it, and nightly.yml's + /// `host`, which writes it, and by no other job; each builds the driver in + /// [`cicache::DRIVER`], so it carries the cache, and runs the driver alone + /// ([`the_driver_alone`]). The writer restores nothing and its run is + /// `seal`'s, so it saves only a tree that `seal` began cold and sealed. #[test] fn each_cache_has_one_writer() { - let dir = repo_root().join(".github/workflows"); - let carries = format!(" CARGO_TARGET_DIR: {}", cicache::DRIVER); + let mut host = Vec::new(); let mut writers = Vec::new(); - for entry in std::fs::read_dir(&dir).expect(".github/workflows is readable").flatten() { - let text = std::fs::read_to_string(entry.path()).expect("a readable workflow"); - let name = entry.file_name().to_string_lossy().into_owned(); - assert!(!text.contains("actions/cache@"), "{name}: the combined action saves too"); - for (job, lines) in jobs(&text) { - let caches: Vec<(bool, String, Vec<&str>)> = lines - .iter() - .enumerate() - .filter(|(_, l)| l.contains("actions/cache/restore@") || l.contains("actions/cache/save@")) - .map(|(at, l)| { - let key = lines[at..] - .iter() - .find_map(|l| l.trim_start().strip_prefix("key: ")) - .expect("a cache step names its key"); - let paths = lines[at..] - .iter() - .skip_while(|l| l.trim() != "path: |") - .skip(1) - .take_while(|l| l.starts_with(" ")) - .map(|l| l.trim()) - .collect(); - (l.contains("actions/cache/save@"), key.split('$').next().unwrap_or("").to_string(), paths) - }) - .collect(); - for (save, prefix, paths) in &caches { - if prefix.starts_with("host-") { - assert_eq!(prefix, cicache::SEALED, "{name} {job}"); - assert_eq!(paths, &cicache::PATHS, "{name} {job}: the host cache's paths"); - assert!(lines.contains(&carries.as_str()), "{name} {job}: the host cache without `{}`", carries.trim()); - assert!(!save || caches.iter().all(|(s, ..)| *s), "{name} {job}: the host cache's writer restores"); - assert!(!text.contains("defaults"), "{name}: a shell or directory for every step"); - the_driver_alone(&name, job, &lines); - } - if *save { - writers.push((name.clone(), prefix.clone())); - } + for (file, workflow) in workflow::all(&repo_root()).expect("the workflows") { + let jobs = workflow.get("jobs").expect(&file).unwrap_or_else(|| panic!("{file}: no `jobs:`")); + for (job, node) in jobs.map().expect(&file) { + let at = format!("{file} {job}"); + let caches = caches(&at, node); + if caches.iter().any(|(cache, _)| *cache == cicache::SEALED) { + let held = HOST_CACHE.iter().find(|(f, j, ..)| *f == file.as_str() && *j == job.as_str()); + let &(.., condition, steps) = held.unwrap_or_else(|| panic!("{at}: names the host cache")); + the_driver_alone(&at, &workflow, node, condition, steps); + host.push((file.clone(), job.clone())); } + writers.extend(caches.into_iter().filter(|(_, save)| *save).map(|(cache, _)| (cache, at.clone()))); } } - assert!(writers.iter().any(|(_, p)| p == cicache::SEALED), "no job writes the host cache: {writers:?}"); - writers.sort(); - let mut prefixes: Vec<&String> = writers.iter().map(|(_, p)| p).collect(); - prefixes.dedup(); - assert_eq!(prefixes.len(), writers.len(), "a cache with two writers: {writers:?}"); - assert!(writers.iter().all(|(f, _)| f == "nightly.yml"), "{writers:?}"); + let expected: Vec<(String, String)> = HOST_CACHE.iter().map(|(f, j, ..)| (f.to_string(), j.to_string())).collect(); + assert_eq!(host, expected, "the jobs that name the host cache"); + for cache in CACHES { + let theirs: Vec<&String> = writers.iter().filter(|(c, _)| *c == cache).map(|(_, at)| at).collect(); + assert!(theirs.len() == 1 && theirs[0].starts_with("nightly.yml "), "{cache}: written by {theirs:?}"); + } } #[test] diff --git a/src/cicache.rs b/src/cicache.rs index e2f000b8e65..8e3dc9fc127 100644 --- a/src/cicache.rs +++ b/src/cicache.rs @@ -22,11 +22,11 @@ //! names neither. The cache key cannot carry the image, because no workflow //! expression sees `ImageOS` or `ImageVersion`. //! -//! **Only a run that restored nothing seals an entry** ([`Start::Cold`], -//! [`seal`]): a warm run's targets hold units none of its steps rebuilt, -//! compiled from sources no manifest it could write describes. [`read`] deletes -//! the manifest it reads, so a warm tree carries none and is never saved, and -//! targets restored without one are refused. +//! **Only the writer seals**: the job that saves the entry restores nothing, +//! starts [`cold`] and [`seal`]s its tree after the last step. A reader's run +//! is never saved and never sealed. [`read`] deletes the manifest it reads, +//! since a warm tree holds units none of its steps rebuilt, and targets +//! restored without one are refused. //! //! **A job carries the cache when its workflow builds the driver in //! [`DRIVER`]** ([`carried`]): cargo builds the driver before this runs, so its @@ -36,9 +36,7 @@ //! //! **A seal refuses a tree whose [`PATHS`] hold more than [`LIMIT`]**, `~` //! being `HOME`, so the save, which follows only a green run, never stores -//! one. A warm tree is not bounded: it keeps each unit its build wrote under a -//! new name beside the one it replaced, so it sums more than a cold build of -//! its sources seals. +//! one. use std::collections::{BTreeMap, BTreeSet}; use std::fs; @@ -80,15 +78,7 @@ fn built() -> SystemTime { /// Every file git tracks, with the SHA-256 of its bytes on disk. type Sources = BTreeMap; -/// What a job found before its first step. -pub enum Start { - /// An entry, read and its manifest consumed. - Warm, - /// No entry this runner can use: every source is dated [`built`]. - Cold(Cold), -} - -/// What [`seal`] holds a cold run's tree to at the end. +/// What [`seal`] holds the writer's tree to at the end. pub struct Cold { runner: String, sources: Sources, @@ -105,12 +95,19 @@ pub fn carried(root: &Path, exe: &Path) -> bool { } } -/// Before the first step of a job that carries the cache: the entry restored +/// Before the first step of a job that reads the cache: the entry restored /// here, read by content. -pub fn read(root: &Path) -> Result<(Start, String), String> { +pub fn read(root: &Path) -> Result { open(root, &runner(|name| std::env::var(name).ok())?, SystemTime::now()) } +/// Before the first step of the writer's run: a tree that holds nothing but +/// its checkout and the driver, with every source dated [`built`], so that the +/// seal sees a step that writes one. +pub fn cold(root: &Path) -> Result<(Cold, String), String> { + start(root, &runner(|name| std::env::var(name).ok())?) +} + /// The runner, as the variables a hosted runner sets name it: its OS, its /// architecture and its image. fn runner(var: impl Fn(&str) -> Option) -> Result { @@ -120,8 +117,7 @@ fn runner(var: impl Fn(&str) -> Option) -> Result { Ok(values.collect::, _>>()?.join(" ")) } -fn open(root: &Path, runner: &str, now: SystemTime) -> Result<(Start, String), String> { - let current = sources(root)?; +fn open(root: &Path, runner: &str, now: SystemTime) -> Result { let manifest = root.join(MANIFEST); let text = match fs::read_to_string(&manifest) { Ok(text) => text, @@ -130,10 +126,10 @@ fn open(root: &Path, runner: &str, now: SystemTime) -> Result<(Start, String), S if !restored.is_empty() { return Err(format!( "{} restored without {MANIFEST}, so nothing says what they were built from", - restored.iter().map(|p| p.display().to_string()).collect::>().join(", ") + listed(&restored) )); } - return cold(root, runner, current, "none restored".into()); + return Ok("none restored: the run is cold".into()); } Err(e) => return Err(format!("read {MANIFEST}: {e}")), }; @@ -144,11 +140,12 @@ fn open(root: &Path, runner: &str, now: SystemTime) -> Result<(Start, String), S let gone = if path.is_dir() { fs::remove_dir_all(&path) } else { fs::remove_file(&path) }; gone.map_err(|e| format!("remove {}: {e}", path.display()))?; } - return cold(root, runner, current, format!("{commit}'s entry, built on {built_on}, deleted")); + return Ok(format!("{commit}'s entry, built on {built_on}, deleted: the run is cold")); } if now <= built() { return Err(format!("this runner's clock reads {now:?}, which no change dated now is newer than")); } + let current = sources(root)?; let mut dirty: BTreeSet> = current .iter() .filter(|(path, hash)| entry.get(*path) != Some(*hash)) @@ -173,28 +170,33 @@ fn open(root: &Path, runner: &str, now: SystemTime) -> Result<(Start, String), S let changed = current.iter().filter(|(p, h)| entry.get(*p).is_some_and(|e| e != *h)).count(); let added = current.keys().filter(|p| !entry.contains_key(*p)).count(); let removed = entry.keys().filter(|p| !current.contains_key(*p)).count(); - Ok(( - Start::Warm, - format!( - "built from {commit}: {same} of {} sources dated as built; {changed} changed, {added} \ - added, {removed} removed, in {packages} packages; {build_time} packages run code at \ - build time", - current.len(), - ), + Ok(format!( + "built from {commit}: {same} of {} sources dated as built; {changed} changed, {added} added, \ + {removed} removed, in {packages} packages; {build_time} packages run code at build time", + current.len(), )) } -fn cold(root: &Path, runner: &str, sources: Sources, why: String) -> Result<(Start, String), String> { +fn start(root: &Path, runner: &str) -> Result<(Cold, String), String> { + let restored = restored(root)?; + if !restored.is_empty() { + return Err(format!("{}: an entry is one cold build, and its writer restores nothing", listed(&restored))); + } + let sources = sources(root)?; for path in sources.keys() { date(&root.join(path), built())?; } - let said = format!("{why}; {} sources dated as built, and the tree is sealed last", sources.len()); - Ok((Start::Cold(Cold { runner: runner.to_string(), sources }), said)) + let said = format!("{} sources dated as built, and the tree is sealed last", sources.len()); + Ok((Cold { runner: runner.to_string(), sources }, said)) +} + +fn listed(paths: &[PathBuf]) -> String { + paths.iter().map(|p| p.display().to_string()).collect::>().join(", ") } -/// After the last step of a run that started [`Start::Cold`]: the tree refused -/// if its [`PATHS`] hold more than [`LIMIT`], `~` being `HOME`, and otherwise -/// every file under every target dated [`built`], then the manifest. +/// After the last step of the writer's run, which [`cold`] began: the tree +/// refused if its [`PATHS`] hold more than [`LIMIT`], `~` being `HOME`, and +/// otherwise every file under every target dated [`built`], then the manifest. pub fn seal(root: &Path, cold: &Cold) -> Result { let home = std::env::var_os("HOME").ok_or("HOME is unset, and the cache's paths name it as `~`")?; seal_at(root, Path::new(&home), cold) @@ -455,9 +457,7 @@ mod tests { /// An entry at `writer`: a checkout of `origin` built cold and sealed. fn entry(origin: &Path, writer: &Path) { checkout(origin, writer); - let Start::Cold(cold) = open(writer, RUNNER, SystemTime::now()).unwrap().0 else { - panic!("a tree with no target is cold") - }; + let (cold, _) = start(writer, RUNNER).unwrap(); build(writer); seal_at(writer, &writer.join("home"), &cold).unwrap(); } @@ -468,6 +468,13 @@ mod tests { fs::rename(writer.join("target"), reader.join("target")).unwrap(); } + /// The entry restored at `reader`, read: a test of what a read serves holds + /// nothing if the read went cold. + fn read_warm(reader: &Path) { + let said = open(reader, RUNNER, SystemTime::now()).unwrap(); + assert!(said.starts_with("built from"), "{said}"); + } + /// The oracle is cargo and the program it builds: an entry serves what /// a cold build of the reader's tree would, recompiling exactly what /// changed and what depends on it — a changed source the checkout dated @@ -513,8 +520,7 @@ mod tests { // before the entry was built. date(&reader.join("leaf/src/lib.rs"), UNIX_EPOCH + Duration::from_secs(1)).unwrap(); - let said = open(&reader, RUNNER, SystemTime::now()).unwrap(); - assert!(matches!(said.0, Start::Warm), "{}", said.1); + read_warm(&reader); assert!(!reader.join(MANIFEST).exists(), "a warm tree keeps no manifest"); let fresh = build(&reader); assert_eq!(run(&reader), "two 1"); @@ -543,7 +549,7 @@ mod tests { sh(&source, &["commit", "-qm", "probed"]); let reader = tmp.join("reader"); restore(&source, &writer, &reader); - assert!(matches!(open(&reader, RUNNER, SystemTime::now()).unwrap().0, Start::Warm)); + read_warm(&reader); let out = cargo_build(&reader); let said = String::from_utf8_lossy(&out.stdout); assert!(!out.status.success() && said.contains("E0761"), "{said}"); @@ -588,7 +594,7 @@ pub fn word(_: proc_macro::TokenStream) -> proc_macro::TokenStream { sh(&source, &["commit", "-qam", "word"]); let reader = tmp.join("reader"); restore(&source, &writer, &reader); - assert!(matches!(open(&reader, RUNNER, SystemTime::now()).unwrap().0, Start::Warm)); + read_warm(&reader); build(&reader); assert_eq!(run(&reader), "two two"); } @@ -643,31 +649,36 @@ pub fn word(_: proc_macro::TokenStream) -> proc_macro::TokenStream { assert!(tmp.join(MANIFEST).exists(), "a tree at the bound is sealed"); } - /// One commit of `a.rs`, read cold on `runner`. + /// One commit of `a.rs`, begun cold on `runner`. fn cold_repo(tmp: &Path, runner: &str) -> Cold { sh(tmp, &["init", "-q"]); configure(tmp); write(tmp, "a.rs", "a\n"); sh(tmp, &["add", "-A"]); sh(tmp, &["commit", "-qm", "a"]); - let Start::Cold(cold) = open(tmp, runner, SystemTime::now()).unwrap().0 else { panic!("cold") }; - cold + start(tmp, runner).unwrap().0 } /// Targets with no manifest beside them were built from nothing anyone can - /// name; the driver's own is this job's. + /// name, so a reader refuses them; the writer, which builds from its + /// checkout alone, refuses them and an entry. The driver's own target is + /// this job's. #[test] fn targets_restored_without_a_manifest_are_refused() { let tmp = TempDir::new("cicache-foreign"); cold_repo(&tmp, RUNNER); fs::create_dir_all(tmp.join(DRIVER)).unwrap(); - assert!(matches!(open(&tmp, RUNNER, SystemTime::now()).unwrap().0, Start::Cold(_))); + open(&tmp, RUNNER, SystemTime::now()).expect("the driver's own target"); + start(&tmp, RUNNER).expect("the driver's own target"); for target in ["kernel/target", "target/debug"] { fs::create_dir_all(tmp.join(target)).unwrap(); - let refusal = open(&tmp, RUNNER, SystemTime::now()).err().expect("a target with no manifest"); - assert!(refusal.contains(target), "{refusal}"); + let refusals = [open(&tmp, RUNNER, SystemTime::now()).err(), start(&tmp, RUNNER).err()]; + assert!(refusals.iter().all(|r| r.as_ref().is_some_and(|r| r.contains(target))), "{refusals:?}"); fs::remove_dir(tmp.join(target)).unwrap(); } + write(&tmp, MANIFEST, ""); + let refusal = start(&tmp, RUNNER).err().expect("an entry, for the writer"); + assert!(refusal.contains(MANIFEST), "{refusal}"); } /// A sealed tree is dated as built throughout, and a step that wrote a @@ -715,8 +726,7 @@ pub fn word(_: proc_macro::TokenStream) -> proc_macro::TokenStream { seal_at(&tmp, &tmp.join("home"), &cold).unwrap(); let mut other = image.clone(); other.insert(name, "another"); - let (start, said) = open(&tmp, &runner_in(&other).unwrap(), SystemTime::now()).unwrap(); - assert!(matches!(start, Start::Cold(_)), "{name}: {said}"); + let said = open(&tmp, &runner_in(&other).unwrap(), SystemTime::now()).unwrap(); assert!(!tmp.join("target/debug").exists() && !tmp.join("kernel/target").exists(), "{name}: {said}"); other.remove(name); let refusal = runner_in(&other).expect_err("a runner without a variable"); @@ -732,7 +742,7 @@ pub fn word(_: proc_macro::TokenStream) -> proc_macro::TokenStream { let cold = cold_repo(&tmp, RUNNER); write(&tmp, "target/debug/x", "x"); seal_at(&tmp, &tmp.join("home"), &cold).unwrap(); - let refusal = open(&tmp, RUNNER, built()).err().expect("a clock at the entry's date"); + let refusal = open(&tmp, RUNNER, built()).expect_err("a clock at the entry's date"); assert!(refusal.contains("clock"), "{refusal}"); } diff --git a/src/lib.rs b/src/lib.rs index c4986ce72d8..77240af8e76 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -49,6 +49,9 @@ pub mod tether; pub mod toolchain; pub mod userlandhost; pub mod wallpaper; +/// Read by gates alone, so it is not compiled into the build system at all. +#[cfg(test)] +pub mod workflow; pub mod worktree; use std::path::{Path, PathBuf}; diff --git a/src/workflow.rs b/src/workflow.rs new file mode 100644 index 00000000000..0b88ae37c8c --- /dev/null +++ b/src/workflow.rs @@ -0,0 +1,223 @@ +//! A workflow under `.github/workflows/`, read as GitHub reads it: YAML, by a +//! parser, so a gate holds the structure that runs and no spelling of it. +//! +//! An alias is resolved as GitHub resolves it. The node it makes is marked, as +//! is every node an anchor or a tag names, and [`Node::mark`] finds them. A +//! key given twice, a key that is not a plain scalar, and a file that is not +//! one document are refused. + +use std::collections::HashMap; +use std::fs; +use std::path::Path; + +use yaml_rust2::parser::{Event, MarkedEventReceiver, Parser}; +use yaml_rust2::scanner::Marker; + +/// One node of a workflow, and the line it starts on. +#[derive(Clone, Debug)] +pub struct Node { + pub value: Value, + pub line: usize, + marked: bool, +} + +#[derive(Clone, Debug)] +pub enum Value { + Scalar(String), + Seq(Vec), + Map(Vec<(String, Node)>), +} + +impl Node { + /// The value under `key`, or `None` where this mapping has no such key. + pub fn get(&self, key: &str) -> Result, String> { + Ok(self.map()?.iter().find(|(k, _)| k == key).map(|(_, node)| node)) + } + + pub fn map(&self) -> Result<&[(String, Node)], String> { + match &self.value { + Value::Map(entries) => Ok(entries), + _ => Err(format!("line {}: not a mapping", self.line)), + } + } + + pub fn seq(&self) -> Result<&[Node], String> { + match &self.value { + Value::Seq(items) => Ok(items), + _ => Err(format!("line {}: not a sequence", self.line)), + } + } + + pub fn str(&self) -> Result<&str, String> { + match &self.value { + Value::Scalar(text) => Ok(text), + _ => Err(format!("line {}: not a scalar", self.line)), + } + } + + /// The line of the first node at or under this one that an anchor, an + /// alias or a tag names or made. + pub fn mark(&self) -> Option { + if self.marked { + return Some(self.line); + } + match &self.value { + Value::Scalar(_) => None, + Value::Seq(items) => items.iter().find_map(Node::mark), + Value::Map(entries) => entries.iter().find_map(|(_, node)| node.mark()), + } + } +} + +/// Every workflow, by file name in name order: each `.yml` and `.yaml` file +/// in `root`'s `.github/workflows`, the files GitHub reads. +pub fn all(root: &Path) -> Result, String> { + let dir = root.join(".github/workflows"); + let mut found = Vec::new(); + for entry in fs::read_dir(&dir).map_err(|e| format!("read {}: {e}", dir.display()))? { + let path = entry.map_err(|e| format!("read {}: {e}", dir.display()))?.path(); + if !path.extension().is_some_and(|ext| ext == "yml" || ext == "yaml") { + continue; + } + let name = path.file_name().map(|name| name.to_string_lossy().into_owned()).unwrap_or_default(); + let text = fs::read_to_string(&path).map_err(|e| format!("read {name}: {e}"))?; + found.push((name.clone(), parse(&text).map_err(|e| format!("{name}: {e}"))?)); + } + found.sort_by(|(a, _), (b, _)| a.cmp(b)); + Ok(found) +} + +/// One YAML document. +pub fn parse(text: &str) -> Result { + let mut tree = Tree::default(); + Parser::new_from_str(text).load(&mut tree, true).map_err(|e| e.to_string())?; + if let Some(refusal) = tree.refusal { + return Err(refusal); + } + let count = tree.docs.len(); + let [doc] = <[Node; 1]>::try_from(tree.docs).map_err(|_| format!("{count} documents, not one"))?; + Ok(doc) +} + +/// The parser's events, built: the documents, each collection still open with +/// its anchor and, in a mapping, the key whose value comes next, and each +/// anchored node by its anchor. +#[derive(Default)] +struct Tree { + docs: Vec, + open: Vec<(Node, usize, Option)>, + anchors: HashMap, + refusal: Option, +} + +impl MarkedEventReceiver for Tree { + fn on_event(&mut self, event: Event, mark: Marker) { + if self.refusal.is_none() { + if let Err(why) = self.take(event, mark.line()) { + self.refusal = Some(format!("line {}: {why}", mark.line())); + } + } + } +} + +impl Tree { + fn take(&mut self, event: Event, line: usize) -> Result<(), String> { + let node = |value, anchor: usize, tagged: bool| Node { value, line, marked: anchor > 0 || tagged }; + match event { + Event::SequenceStart(anchor, tag) => { + self.open.push((node(Value::Seq(Vec::new()), anchor, tag.is_some()), anchor, None)); + } + Event::MappingStart(anchor, tag) => { + self.open.push((node(Value::Map(Vec::new()), anchor, tag.is_some()), anchor, None)); + } + Event::SequenceEnd | Event::MappingEnd => { + let (done, anchor, _) = self.open.pop().expect("the parser ends only what it began"); + self.place(done, anchor)?; + } + Event::Scalar(text, _, anchor, tag) => self.place(node(Value::Scalar(text), anchor, tag.is_some()), anchor)?, + Event::Alias(anchor) => { + let mut copy = self.anchors.get(&anchor).ok_or("an alias inside the node it names")?.clone(); + copy.line = line; + copy.marked = true; + self.place(copy, 0)?; + } + Event::Nothing | Event::StreamStart | Event::StreamEnd | Event::DocumentStart | Event::DocumentEnd => {} + } + Ok(()) + } + + /// `node` into the collection open last, or as a document. + fn place(&mut self, node: Node, anchor: usize) -> Result<(), String> { + if anchor > 0 { + self.anchors.insert(anchor, node.clone()); + } + let Some((parent, _, key)) = self.open.last_mut() else { + self.docs.push(node); + return Ok(()); + }; + match (&mut parent.value, key.take()) { + (Value::Seq(items), _) => items.push(node), + (Value::Map(entries), Some(key)) => { + if entries.iter().any(|(k, _)| *k == key) { + return Err(format!("`{key}` twice in one mapping")); + } + entries.push((key, node)); + } + (Value::Map(_), None) => match node { + Node { value: Value::Scalar(text), marked: false, .. } => *key = Some(text), + _ => return Err("a key that is not a plain scalar".into()), + }, + (Value::Scalar(_), _) => unreachable!("only a collection is open"), + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// What GitHub reads is what the reader holds: a comment, a quoted key and a + /// folded line are spellings, an alias is its anchor's node, and every + /// node an anchor, an alias or a tag made is found. + #[test] + fn a_workflow_is_its_structure_and_not_its_spelling() { + let doc = parse( + "jobs: # a comment + \"host\": + steps: &steps + - run: cargo run + || true + again: + steps: *steps + tagged: + run: !!str x +", + ) + .unwrap(); + let jobs = doc.get("jobs").unwrap().unwrap(); + assert_eq!(jobs.map().unwrap().len(), 3); + let run = |job: &str| { + let steps = jobs.get(job).unwrap().unwrap().get("steps").unwrap().unwrap(); + steps.seq().unwrap()[0].get("run").unwrap().unwrap().str().unwrap().to_string() + }; + assert_eq!([run("host"), run("again")], ["cargo run || true", "cargo run || true"]); + let mark = |job: &str| jobs.get(job).unwrap().unwrap().mark(); + assert_eq!([mark("host"), mark("again"), mark("tagged")], [Some(4), Some(7), Some(9)]); + } + + #[test] + fn what_github_would_read_two_ways_is_refused() { + for (text, refusal) in [ + ("a: 1\n\"a\": 2\n", "line 2: `a` twice in one mapping"), + ("&k a: 1\n", "line 1: a key that is not a plain scalar"), + ("? [a]\n: 1\n", "a key that is not a plain scalar"), + ("a: &x [*x]\n", "an alias inside the node it names"), + ("a: 1\n---\nb: 2\n", "2 documents, not one"), + ("a: *x\n", "unknown anchor"), + ] { + let said = parse(text).expect_err(text); + assert!(said.contains(refusal), "{text:?}: {said}"); + } + } +} From f7e0bd4b5879375f8e7413e89163bb7e94618048 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 20:15:18 +0200 Subject: [PATCH 15/20] The workflow reader marks an alias through its anchor alone, and reads a workflow's extension in any case - An alias copies a node its anchor marked, so the line that marked the copy again was dead: with it deleted, `a_workflow_is_its_structure_and_not_its_spelling` was green (EXIT 0), as it is without the deletion. - `.yml` and `.yaml` are matched in any case, so the files read are a superset of those GitHub reads. - `parse` and `Value` are private: a gate reads through `all` and `Node`'s accessors. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- src/workflow.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/workflow.rs b/src/workflow.rs index 0b88ae37c8c..fac409a1a63 100644 --- a/src/workflow.rs +++ b/src/workflow.rs @@ -16,13 +16,13 @@ use yaml_rust2::scanner::Marker; /// One node of a workflow, and the line it starts on. #[derive(Clone, Debug)] pub struct Node { - pub value: Value, + value: Value, pub line: usize, marked: bool, } #[derive(Clone, Debug)] -pub enum Value { +enum Value { Scalar(String), Seq(Vec), Map(Vec<(String, Node)>), @@ -70,17 +70,19 @@ impl Node { } /// Every workflow, by file name in name order: each `.yml` and `.yaml` file -/// in `root`'s `.github/workflows`, the files GitHub reads. +/// in `root`'s `.github/workflows`, in any case, a superset of the files +/// GitHub reads. pub fn all(root: &Path) -> Result, String> { let dir = root.join(".github/workflows"); let mut found = Vec::new(); for entry in fs::read_dir(&dir).map_err(|e| format!("read {}: {e}", dir.display()))? { - let path = entry.map_err(|e| format!("read {}: {e}", dir.display()))?.path(); - if !path.extension().is_some_and(|ext| ext == "yml" || ext == "yaml") { + let entry = entry.map_err(|e| format!("read {}: {e}", dir.display()))?; + let name = entry.file_name().to_string_lossy().into_owned(); + let lower = name.to_ascii_lowercase(); + if !(lower.ends_with(".yml") || lower.ends_with(".yaml")) { continue; } - let name = path.file_name().map(|name| name.to_string_lossy().into_owned()).unwrap_or_default(); - let text = fs::read_to_string(&path).map_err(|e| format!("read {name}: {e}"))?; + let text = fs::read_to_string(entry.path()).map_err(|e| format!("read {name}: {e}"))?; found.push((name.clone(), parse(&text).map_err(|e| format!("{name}: {e}"))?)); } found.sort_by(|(a, _), (b, _)| a.cmp(b)); @@ -88,7 +90,7 @@ pub fn all(root: &Path) -> Result, String> { } /// One YAML document. -pub fn parse(text: &str) -> Result { +fn parse(text: &str) -> Result { let mut tree = Tree::default(); Parser::new_from_str(text).load(&mut tree, true).map_err(|e| e.to_string())?; if let Some(refusal) = tree.refusal { @@ -135,10 +137,10 @@ impl Tree { self.place(done, anchor)?; } Event::Scalar(text, _, anchor, tag) => self.place(node(Value::Scalar(text), anchor, tag.is_some()), anchor)?, + // The copy of a node its anchor marked, so marked too. Event::Alias(anchor) => { let mut copy = self.anchors.get(&anchor).ok_or("an alias inside the node it names")?.clone(); copy.line = line; - copy.marked = true; self.place(copy, 0)?; } Event::Nothing | Event::StreamStart | Event::StreamEnd | Event::DocumentStart | Event::DocumentEnd => {} @@ -207,7 +209,7 @@ mod tests { } #[test] - fn what_github_would_read_two_ways_is_refused() { + fn a_document_the_reader_cannot_hold_is_refused() { for (text, refusal) in [ ("a: 1\n\"a\": 2\n", "line 2: `a` twice in one mapping"), ("&k a: 1\n", "line 1: a key that is not a plain scalar"), From 038da721e7237562c158587570c5e3df8d59eee7 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 21:08:39 +0200 Subject: [PATCH 16/20] Round 10: the workflow gate becomes review rules, and the seal job is the cold start, host and the seal The workflow gate goes, all of it: src/workflow.rs, the yaml-rust2 dev-dependency and its lock entries, and src/ci.rs's `each_cache_has_one_writer` (main's text reading of it too), with `the_driver_alone`, `host_step`, `caches`, `action`, `keys` and their constants. Round 9's review found two more spellings that GitHub's reader takes and the gate's did not: a `uses:` path GitHub splits on `/` and `\` and drops empty segments of, and U+2028, U+0085 and U+2029, which YamlDotNet breaks a line at and yaml-rust2 does not. A gate that re-reads GitHub's YAML lags GitHub's reader by construction. Its rules are now sentences in `.claude/agents/reviewer.md`'s new **Caches** bullet, read off the diff: one writer per cache, in nightly.yml; the host cache's writer nightly's `host` and its reader ci.yml's `host`, on one `runs-on` and both caching `PATHS`; those jobs run checkout, their cache step and the driver alone; the save's `github.ref == 'refs/heads/main'` guard is their only step-level `if:`, and ci.yml's `host` skips only a draft; no `continue-on-error`, `shell:`, `defaults:` or cargo `runner` reaches them; nightly's `on:` is one daily schedule and `workflow_dispatch`. The last three come from the review's NOTEs: a root `.cargo/config.toml` runner, a free `runs-on`, and a free `on:` were each invisible to the gate. Code stays where reading cannot see: the seal, its bound, the manifest and its refusals in src/cicache.rs. The Tests bullet takes the owner's two principles: a mistake a type makes unrepresentable needs no test, and a rule a reviewer can check by reading lives in the prompt, a gate being code only for what reading cannot see. `--ci seal` is `ci::seal`: the cold start, then `host`, then the seal, as calls, so no bool decides whether the one writer seals. `host` reads the cache in every job that carries it, so in the seal job it reads the tree the cold start just proved empty and says the run is cold. `cicache::cold` refuses a driver built outside `target/ci-driver`, the check `host` made for the seal before. nightly.yml's save carries the main-only guard again, the only step-level `if:` the rule allows, and the job carries none: a dispatch on a branch runs the cold build and the seal, bounded, and saves nothing. `cicache::SEALED` was read by the gate alone and goes; `PATHS` is private to the bound that reads it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- .claude/agents/reviewer.md | 11 ++ .github/workflows/nightly.yml | 12 +- Cargo.lock | 35 ------ Cargo.toml | 3 - src/ci.rs | 200 +++--------------------------- src/cicache.rs | 6 +- src/lib.rs | 3 - src/workflow.rs | 225 ---------------------------------- 8 files changed, 40 insertions(+), 455 deletions(-) delete mode 100644 src/workflow.rs diff --git a/.claude/agents/reviewer.md b/.claude/agents/reviewer.md index 19330b044ab..6e8ec3af662 100644 --- a/.claude/agents/reviewer.md +++ b/.claude/agents/reviewer.md @@ -80,6 +80,13 @@ above; otherwise it is a NOTE. or `[patch]`, which cargo ignores with only a warning; a new package without a `description` saying what it is. A new cargo feature or `cfg` arm of one, and every arm a changed `src/clippy.rs` shape stops building, is shown linted in the pull request body: a `mem::forget` planted in that arm turns `cargo run -- --clippy` red. +- **Caches.** No gate reads these; a diff that breaks one is a BLOCKER. Each cache has one + writer, a nightly.yml job; the host cache's is nightly's `host`, and its one reader ci.yml's + `host`, on the same `runs-on`, both caching `src/cicache.rs`'s `PATHS`. A job that names the + host cache runs `actions/checkout`, its cache step and `cargo run -- --ci `, and nothing + else. The save's guard, `github.ref == 'refs/heads/main'`, is the only step-level `if:` in those + jobs, and ci.yml's `host` skips only a draft. No `continue-on-error`, `shell:`, `defaults:` or + cargo `runner` reaches them. nightly.yml's `on:` is one daily `schedule` and `workflow_dispatch`. - **Growth.** Every line is a responsibility, not an asset. State the branch's net lines (`git diff --shortstat origin/main...HEAD`), production and tests apart. Production code that grows needs a reason you accept; a branch that could delete more than it adds and does not goes back @@ -93,6 +100,10 @@ above; otherwise it is a NOTE. - **Tests.** The refusals and the boundary, not the happy path. Write down the partial fix or one-field mutation that would still pass, as a patch the implementer can apply. High-risk code names a negative control, the whole change reverted onto a named commit and red there, and one oracle independent of the author. + A mistake a type makes unrepresentable needs no test: a change that no longer compiles is not a + surviving mutation, and a review asks for no test of it. + A rule a reviewer can check by reading the diff lives in this prompt, not in a gate; a gate is + code only for what reading cannot see — runtime behaviour, bytes, measurements. - **Edges.** Untrusted input never panics the kernel; it is refused. Check-then-act races. A lock held across a user copy or a device wait. Arithmetic on a value the caller chooses. A short read, an exit status nobody reads. An `at_most(::MAX)` or `index(usize::MAX)` on an diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 5e340472273..6d93ba85ac1 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -16,12 +16,9 @@ concurrency: cancel-in-progress: false jobs: - # The host cache's writer: only main's entries are readable from every - # branch. It restores nothing, and `seal` is green only once src/cicache.rs - # has sealed the tree the save stores. On a branch it would only repeat that - # branch's pull request. + # The host cache's writer restores nothing, and `seal` is green only once + # src/cicache.rs has sealed the tree the save stores. host: - if: github.ref == 'refs/heads/main' runs-on: ubuntu-24.04 timeout-minutes: 90 # The driver's own target, and no step's: it says this job carries the @@ -35,7 +32,10 @@ jobs: - run: cargo run -- --ci seal - - uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + # The host cache's one writer. Only main's entries are readable from + # every branch. + - if: github.ref == 'refs/heads/main' + uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 with: path: | ~/.cargo/registry/index diff --git a/Cargo.lock b/Cargo.lock index 6065e78cb97..cc05a45c729 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -38,12 +38,6 @@ version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" -[[package]] -name = "arraydeque" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d902e3d592a523def97af8f317b08ce16b7ab854c1985a0c671e6f15cebc236" - [[package]] name = "autocfg" version = "1.5.0" @@ -381,24 +375,6 @@ dependencies = [ "foldhash 0.2.0", ] -[[package]] -name = "hashbrown" -version = "0.17.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" -dependencies = [ - "foldhash 0.2.0", -] - -[[package]] -name = "hashlink" -version = "0.12.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a596f1b20ed2cc5ecac41a164aaebc7258057060f06c0cf7a2ba3991ee7990fb" -dependencies = [ - "hashbrown 0.17.1", -] - [[package]] name = "heck" version = "0.5.0" @@ -989,7 +965,6 @@ dependencies = [ "toyos-wallclock", "toyos-xhci", "uuid", - "yaml-rust2", ] [[package]] @@ -1614,16 +1589,6 @@ dependencies = [ "wasmparser", ] -[[package]] -name = "yaml-rust2" -version = "0.13.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "57e5b818a27a4cd30884ea380857a5e56f7ec3ba24a3990a3cc0b95af3238e18" -dependencies = [ - "arraydeque", - "hashlink", -] - [[package]] name = "zerocopy" version = "0.8.47" diff --git a/Cargo.toml b/Cargo.toml index 43434b16112..318e42c520e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -177,9 +177,6 @@ toyos-sched = { path = "toyos-sched" } # The Bulk-Only phases, so the harness judging a wedge reads the word # `toyos_xhci::bot::Phase` declares instead of spelling it a second time. toyos-xhci = { path = "toyos-xhci" } -# The workflows' YAML, parsed to events that keep every anchor, alias and tag, -# for the gates over `.github/workflows/` (`src/workflow.rs`). -yaml-rust2 = { version = "0.13", default-features = false } [patch.crates-io] loom = { git = "https://github.com/ToyOSOrg/loom", branch = "toyos" } diff --git a/src/ci.rs b/src/ci.rs index 7d90228b261..5740aedeb38 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -73,8 +73,8 @@ pub fn dispatch(root: &Path, args: &[String]) { std::process::exit(2); }); let steps = match &job { - Job::Host => host(root, false), - Job::Seal => host(root, true), + Job::Host => host(root), + Job::Seal => seal(root), Job::Toolchain => vec![step("the toolchain release", || release::ensure_published(root))], Job::Guest => guest(root, &suite_args(&["--jobs", "1"])), Job::Publish => vec![step("the SDK crates on crates.io", || publish(root))], @@ -473,16 +473,9 @@ fn run_control(root: &Path, control: &Control) -> Result { /// host triple for the same reason. /// /// In a job that carries the cache ([`cicache::carried`]) the restored entry is -/// read before any step. `seal`'s job, the one that saves the entry, instead -/// starts from a cold tree and seals it after the last step. A developer's -/// tree keeps the dates its edits gave it. -fn host(root: &Path, seals: bool) -> Vec { +/// read before any step. A developer's tree keeps the dates its edits gave it. +fn host(root: &Path) -> Vec { let carried = cicache::carried(root, &std::env::current_exe().expect("the driver's own path")); - if seals && !carried { - return vec![step("a cold tree, for the entry", || { - Err(format!("only a job whose driver is built in {} seals its tree", cicache::DRIVER)) - })]; - } let tmp = toyos_tmpdir::TempDir::new("ci-host"); let short = Path::new(toyos_tmpdir::SHORT_BASE); let before = toyos_tmpdir::gone_roots(short); @@ -491,18 +484,9 @@ fn host(root: &Path, seals: bool) -> Vec { std::env::set_var("TMPDIR", tmp.path()); let host_triple = crate::toolchain::host_triple(); let mut steps = Vec::new(); - let mut cold = None; if carried { carry(); - steps.push(if seals { - step("a cold tree, for the entry", || { - let (found, said) = cicache::cold(root)?; - cold = Some(found); - Ok(said) - }) - } else { - step("the cache entry, read by content", || cicache::read(root)) - }); + steps.push(step("the cache entry, read by content", || cicache::read(root))); // Every step after an unreadable entry would be judged against it. if steps[0].verdict.is_err() { return steps; @@ -580,9 +564,20 @@ fn host(root: &Path, seals: bool) -> Vec { cargo(root, &["test", "--manifest-path", "toyos/Cargo.toml", "--target", &host_triple]) })); steps.push(step("nothing left in $TMPDIR or /tmp", || left_behind(&tmp, short, &before))); - if let Some(cold) = &cold { - steps.push(step("the tree, sealed as the host cache's entry", || cicache::seal(root, cold))); - } + steps +} + +/// [`host`] from a cold tree, then that tree sealed as the host cache's entry. +fn seal(root: &Path) -> Vec { + let mut cold = None; + let mut steps = vec![step("a cold tree, for the entry", || { + let (found, said) = cicache::cold(root)?; + cold = Some(found); + Ok(said) + })]; + let Some(cold) = cold else { return steps }; + steps.extend(host(root)); + steps.push(step("the tree, sealed as the host cache's entry", || cicache::seal(root, &cold))); steps } @@ -934,7 +929,6 @@ fn at_tip(ls_remote: &str, head: &str) -> Result<(), String> { #[cfg(test)] mod tests { use super::*; - use crate::workflow::{self, Node}; /// A deterministic control on `host`'s `std::env::set_var("TMPDIR", ...)`: /// delete that line and every child writes to the real `$TMPDIR` instead of @@ -1169,162 +1163,6 @@ mod tests { assert_eq!(seen, 3, "ci.yml, nightly.yml and publish.yml"); } - /// Each cache, by what every key and restore key of a step that names it - /// begins with, up to its first expression. - const CACHES: [&str; 2] = [cicache::SEALED, "guest-"]; - - /// What the host cache's key and its reader's restore key say after - /// [`cicache::SEALED`]: one entry per run, found by the runner's OS and - /// architecture. - const HOST_KEYS: [&str; 2] = ["${{ runner.os }}-${{ runner.arch }}-${{ github.run_id }}", "${{ runner.os }}-${{ runner.arch }}-"]; - - /// The jobs that name the host cache, its one reader and its one writer: - /// the condition each runs under, since a skipped job is a green check, and - /// its steps. The reader restores before its run; the writer restores - /// nothing and saves after its run. - const HOST_CACHE: [(&str, &str, &str, [&str; 3]); 2] = [ - ("ci.yml", "host", "github.event_name == 'merge_group' || github.event.pull_request.draft == false", [ - "actions/checkout", - "actions/cache/restore", - "run: cargo run -- --ci host", - ]), - ("nightly.yml", "host", "github.ref == 'refs/heads/main'", [ - "actions/checkout", - "run: cargo run -- --ci seal", - "actions/cache/save", - ]), - ]; - - /// Every key of `node` is one of `allowed`. - fn keys(at: &str, node: &Node, allowed: &[&str]) { - for (key, value) in node.map().expect(at) { - assert!(allowed.contains(&key.as_str()), "{at}: line {}: `{key}`, where only {allowed:?} may be", value.line); - } - } - - /// The action a step uses, without its version and in GitHub's case: - /// GitHub does not tell `Actions/Cache` from `actions/cache`. - fn action(at: &str, step: &Node) -> Option { - let uses = step.get("uses").expect(at)?; - Some(uses.str().expect(at).split('@').next().unwrap_or_default().to_ascii_lowercase()) - } - - /// Each step of `job` that restores or saves a cache: the cache its keys - /// name, and whether it saves. A key that names no cache, a step whose - /// keys name two, and the action that restores and saves in one are - /// refused. - fn caches(at: &str, job: &Node) -> Vec<(&'static str, bool)> { - let Some(steps) = job.get("steps").expect(at) else { return Vec::new() }; - let mut found = Vec::new(); - for step in steps.seq().expect(at) { - let save = match action(at, step).as_deref() { - Some("actions/cache") => panic!("{at}: line {}: the combined action saves too", step.line), - Some("actions/cache/restore") => false, - Some("actions/cache/save") => true, - _ => continue, - }; - let with = step.get("with").expect(at).unwrap_or_else(|| panic!("{at}: line {}: a cache step names no key", step.line)); - let mut keys = vec![with.get("key").expect(at).unwrap_or_else(|| panic!("{at}: a cache step names no key"))]; - keys.extend(with.get("restore-keys").expect(at)); - let heads: Vec<&str> = keys - .iter() - .flat_map(|key| key.str().expect(at).lines().map(str::trim).filter(|key| !key.is_empty())) - .map(|key| key.split("${{").next().unwrap_or_default()) - .collect(); - let cache = CACHES - .into_iter() - .find(|cache| heads.iter().all(|head| head == cache)) - .unwrap_or_else(|| panic!("{at}: line {}: keys {heads:?}, where each names one of {CACHES:?}", step.line)); - found.push((cache, save)); - } - found - } - - /// One step of a job that names the host cache, held to the keys it may - /// have: its action, or `run:` and its line. - fn host_step(at: &str, step: &Node) -> String { - if let Some(run) = step.get("run").expect(at) { - keys(at, step, &["run"]); - return format!("run: {}", run.str().expect(at)); - } - keys(at, step, &["uses", "with"]); - let action = action(at, step).unwrap_or_else(|| panic!("{at}: line {}: a step with no `run:` or `uses:`", step.line)); - let with = step.get("with").expect(at); - let restore = match action.as_str() { - "actions/checkout" => { - with.into_iter().for_each(|with| keys(at, with, &["fetch-depth"])); - return action; - } - "actions/cache/restore" => Some(HOST_KEYS[1]), - "actions/cache/save" => None, - other => panic!("{at}: line {}: `{other}`", step.line), - }; - let with = with.unwrap_or_else(|| panic!("{at}: line {}: a cache step with no `with:`", step.line)); - keys(at, with, &["path", "key", "restore-keys"]); - let text = |key: &str| with.get(key).expect(at).map(|node| node.str().expect(at).to_string()); - let paths = text("path").unwrap_or_default(); - let paths: Vec<&str> = paths.lines().map(str::trim).filter(|path| !path.is_empty()).collect(); - assert_eq!(paths, cicache::PATHS, "{at}: line {}: the host cache's paths", with.line); - let sealed = |rest: &str| Some(format!("{}{rest}", cicache::SEALED)); - assert_eq!(text("key"), sealed(HOST_KEYS[0]), "{at}: line {}: the host cache's key", with.line); - assert_eq!(text("restore-keys"), restore.and_then(sealed), "{at}: line {}: its restore key", with.line); - action - } - - /// What a job that names the host cache may be: one of [`HOST_CACHE`], its - /// keys and its workflow's in a closed allow-list, with no anchor, alias or - /// tag in it. No step has a condition of its own, so the writer's save has - /// GitHub's, `success()`, and follows only a green run; the job's verdict - /// is the driver's. - fn the_driver_alone(at: &str, workflow: &Node, job: &Node, condition: &str, steps: [&str; 3]) { - keys(at, workflow, &["name", "on", "concurrency", "jobs"]); - if let Some(line) = job.mark() { - panic!("{at}: line {line}: an anchor, an alias or a tag"); - } - keys(at, job, &["if", "runs-on", "timeout-minutes", "env", "steps"]); - let condition_given = job.get("if").expect(at).map(|node| node.str().expect(at)); - assert_eq!(condition_given, Some(condition), "{at}: the condition it runs under"); - let env = job.get("env").expect(at).unwrap_or_else(|| panic!("{at}: no `env:`")); - let env: Vec<(&str, &str)> = env.map().expect(at).iter().map(|(k, v)| (k.as_str(), v.str().expect(at))).collect(); - assert_eq!(env, [("CARGO_TARGET_DIR", cicache::DRIVER)], "{at}: the job's environment"); - let given = job.get("steps").expect(at).unwrap_or_else(|| panic!("{at}: no `steps:`")); - let given: Vec = given.seq().expect(at).iter().map(|step| host_step(at, step)).collect(); - assert_eq!(given, steps, "{at}: its steps"); - } - - /// Exactly one job writes each cache, on the nightly, so what a pull request - /// restores is one run's tree and never a race between two writers. The - /// host cache is named by ci.yml's `host`, which reads it, and nightly.yml's - /// `host`, which writes it, and by no other job; each builds the driver in - /// [`cicache::DRIVER`], so it carries the cache, and runs the driver alone - /// ([`the_driver_alone`]). The writer restores nothing and its run is - /// `seal`'s, so it saves only a tree that `seal` began cold and sealed. - #[test] - fn each_cache_has_one_writer() { - let mut host = Vec::new(); - let mut writers = Vec::new(); - for (file, workflow) in workflow::all(&repo_root()).expect("the workflows") { - let jobs = workflow.get("jobs").expect(&file).unwrap_or_else(|| panic!("{file}: no `jobs:`")); - for (job, node) in jobs.map().expect(&file) { - let at = format!("{file} {job}"); - let caches = caches(&at, node); - if caches.iter().any(|(cache, _)| *cache == cicache::SEALED) { - let held = HOST_CACHE.iter().find(|(f, j, ..)| *f == file.as_str() && *j == job.as_str()); - let &(.., condition, steps) = held.unwrap_or_else(|| panic!("{at}: names the host cache")); - the_driver_alone(&at, &workflow, node, condition, steps); - host.push((file.clone(), job.clone())); - } - writers.extend(caches.into_iter().filter(|(_, save)| *save).map(|(cache, _)| (cache, at.clone()))); - } - } - let expected: Vec<(String, String)> = HOST_CACHE.iter().map(|(f, j, ..)| (f.to_string(), j.to_string())).collect(); - assert_eq!(host, expected, "the jobs that name the host cache"); - for cache in CACHES { - let theirs: Vec<&String> = writers.iter().filter(|(c, _)| *c == cache).map(|(_, at)| at).collect(); - assert!(theirs.len() == 1 && theirs[0].starts_with("nightly.yml "), "{cache}: written by {theirs:?}"); - } - } - #[test] fn the_declared_version_is_a_version() { let declared = diff --git a/src/cicache.rs b/src/cicache.rs index 8e3dc9fc127..6fc1b782f50 100644 --- a/src/cicache.rs +++ b/src/cicache.rs @@ -48,11 +48,10 @@ use sha2::{Digest, Sha256}; const MANIFEST: &str = "target/ci-sources"; pub const DRIVER: &str = "target/ci-driver"; -pub const SEALED: &str = "host-sealed-"; /// What every step that names the host cache archives: the cache's version is /// computed from the list, so a restore whose list differs finds nothing. -pub const PATHS: [&str; 8] = [ +const PATHS: [&str; 8] = [ "~/.cargo/registry/index", "~/.cargo/registry/cache", "~/.cargo/git/db", @@ -105,6 +104,9 @@ pub fn read(root: &Path) -> Result { /// its checkout and the driver, with every source dated [`built`], so that the /// seal sees a step that writes one. pub fn cold(root: &Path) -> Result<(Cold, String), String> { + if !carried(root, &std::env::current_exe().map_err(|e| format!("the driver's own path: {e}"))?) { + return Err(format!("only a job whose driver is built in {DRIVER} seals its tree")); + } start(root, &runner(|name| std::env::var(name).ok())?) } diff --git a/src/lib.rs b/src/lib.rs index 77240af8e76..c4986ce72d8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -49,9 +49,6 @@ pub mod tether; pub mod toolchain; pub mod userlandhost; pub mod wallpaper; -/// Read by gates alone, so it is not compiled into the build system at all. -#[cfg(test)] -pub mod workflow; pub mod worktree; use std::path::{Path, PathBuf}; diff --git a/src/workflow.rs b/src/workflow.rs deleted file mode 100644 index fac409a1a63..00000000000 --- a/src/workflow.rs +++ /dev/null @@ -1,225 +0,0 @@ -//! A workflow under `.github/workflows/`, read as GitHub reads it: YAML, by a -//! parser, so a gate holds the structure that runs and no spelling of it. -//! -//! An alias is resolved as GitHub resolves it. The node it makes is marked, as -//! is every node an anchor or a tag names, and [`Node::mark`] finds them. A -//! key given twice, a key that is not a plain scalar, and a file that is not -//! one document are refused. - -use std::collections::HashMap; -use std::fs; -use std::path::Path; - -use yaml_rust2::parser::{Event, MarkedEventReceiver, Parser}; -use yaml_rust2::scanner::Marker; - -/// One node of a workflow, and the line it starts on. -#[derive(Clone, Debug)] -pub struct Node { - value: Value, - pub line: usize, - marked: bool, -} - -#[derive(Clone, Debug)] -enum Value { - Scalar(String), - Seq(Vec), - Map(Vec<(String, Node)>), -} - -impl Node { - /// The value under `key`, or `None` where this mapping has no such key. - pub fn get(&self, key: &str) -> Result, String> { - Ok(self.map()?.iter().find(|(k, _)| k == key).map(|(_, node)| node)) - } - - pub fn map(&self) -> Result<&[(String, Node)], String> { - match &self.value { - Value::Map(entries) => Ok(entries), - _ => Err(format!("line {}: not a mapping", self.line)), - } - } - - pub fn seq(&self) -> Result<&[Node], String> { - match &self.value { - Value::Seq(items) => Ok(items), - _ => Err(format!("line {}: not a sequence", self.line)), - } - } - - pub fn str(&self) -> Result<&str, String> { - match &self.value { - Value::Scalar(text) => Ok(text), - _ => Err(format!("line {}: not a scalar", self.line)), - } - } - - /// The line of the first node at or under this one that an anchor, an - /// alias or a tag names or made. - pub fn mark(&self) -> Option { - if self.marked { - return Some(self.line); - } - match &self.value { - Value::Scalar(_) => None, - Value::Seq(items) => items.iter().find_map(Node::mark), - Value::Map(entries) => entries.iter().find_map(|(_, node)| node.mark()), - } - } -} - -/// Every workflow, by file name in name order: each `.yml` and `.yaml` file -/// in `root`'s `.github/workflows`, in any case, a superset of the files -/// GitHub reads. -pub fn all(root: &Path) -> Result, String> { - let dir = root.join(".github/workflows"); - let mut found = Vec::new(); - for entry in fs::read_dir(&dir).map_err(|e| format!("read {}: {e}", dir.display()))? { - let entry = entry.map_err(|e| format!("read {}: {e}", dir.display()))?; - let name = entry.file_name().to_string_lossy().into_owned(); - let lower = name.to_ascii_lowercase(); - if !(lower.ends_with(".yml") || lower.ends_with(".yaml")) { - continue; - } - let text = fs::read_to_string(entry.path()).map_err(|e| format!("read {name}: {e}"))?; - found.push((name.clone(), parse(&text).map_err(|e| format!("{name}: {e}"))?)); - } - found.sort_by(|(a, _), (b, _)| a.cmp(b)); - Ok(found) -} - -/// One YAML document. -fn parse(text: &str) -> Result { - let mut tree = Tree::default(); - Parser::new_from_str(text).load(&mut tree, true).map_err(|e| e.to_string())?; - if let Some(refusal) = tree.refusal { - return Err(refusal); - } - let count = tree.docs.len(); - let [doc] = <[Node; 1]>::try_from(tree.docs).map_err(|_| format!("{count} documents, not one"))?; - Ok(doc) -} - -/// The parser's events, built: the documents, each collection still open with -/// its anchor and, in a mapping, the key whose value comes next, and each -/// anchored node by its anchor. -#[derive(Default)] -struct Tree { - docs: Vec, - open: Vec<(Node, usize, Option)>, - anchors: HashMap, - refusal: Option, -} - -impl MarkedEventReceiver for Tree { - fn on_event(&mut self, event: Event, mark: Marker) { - if self.refusal.is_none() { - if let Err(why) = self.take(event, mark.line()) { - self.refusal = Some(format!("line {}: {why}", mark.line())); - } - } - } -} - -impl Tree { - fn take(&mut self, event: Event, line: usize) -> Result<(), String> { - let node = |value, anchor: usize, tagged: bool| Node { value, line, marked: anchor > 0 || tagged }; - match event { - Event::SequenceStart(anchor, tag) => { - self.open.push((node(Value::Seq(Vec::new()), anchor, tag.is_some()), anchor, None)); - } - Event::MappingStart(anchor, tag) => { - self.open.push((node(Value::Map(Vec::new()), anchor, tag.is_some()), anchor, None)); - } - Event::SequenceEnd | Event::MappingEnd => { - let (done, anchor, _) = self.open.pop().expect("the parser ends only what it began"); - self.place(done, anchor)?; - } - Event::Scalar(text, _, anchor, tag) => self.place(node(Value::Scalar(text), anchor, tag.is_some()), anchor)?, - // The copy of a node its anchor marked, so marked too. - Event::Alias(anchor) => { - let mut copy = self.anchors.get(&anchor).ok_or("an alias inside the node it names")?.clone(); - copy.line = line; - self.place(copy, 0)?; - } - Event::Nothing | Event::StreamStart | Event::StreamEnd | Event::DocumentStart | Event::DocumentEnd => {} - } - Ok(()) - } - - /// `node` into the collection open last, or as a document. - fn place(&mut self, node: Node, anchor: usize) -> Result<(), String> { - if anchor > 0 { - self.anchors.insert(anchor, node.clone()); - } - let Some((parent, _, key)) = self.open.last_mut() else { - self.docs.push(node); - return Ok(()); - }; - match (&mut parent.value, key.take()) { - (Value::Seq(items), _) => items.push(node), - (Value::Map(entries), Some(key)) => { - if entries.iter().any(|(k, _)| *k == key) { - return Err(format!("`{key}` twice in one mapping")); - } - entries.push((key, node)); - } - (Value::Map(_), None) => match node { - Node { value: Value::Scalar(text), marked: false, .. } => *key = Some(text), - _ => return Err("a key that is not a plain scalar".into()), - }, - (Value::Scalar(_), _) => unreachable!("only a collection is open"), - } - Ok(()) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - /// What GitHub reads is what the reader holds: a comment, a quoted key and a - /// folded line are spellings, an alias is its anchor's node, and every - /// node an anchor, an alias or a tag made is found. - #[test] - fn a_workflow_is_its_structure_and_not_its_spelling() { - let doc = parse( - "jobs: # a comment - \"host\": - steps: &steps - - run: cargo run - || true - again: - steps: *steps - tagged: - run: !!str x -", - ) - .unwrap(); - let jobs = doc.get("jobs").unwrap().unwrap(); - assert_eq!(jobs.map().unwrap().len(), 3); - let run = |job: &str| { - let steps = jobs.get(job).unwrap().unwrap().get("steps").unwrap().unwrap(); - steps.seq().unwrap()[0].get("run").unwrap().unwrap().str().unwrap().to_string() - }; - assert_eq!([run("host"), run("again")], ["cargo run || true", "cargo run || true"]); - let mark = |job: &str| jobs.get(job).unwrap().unwrap().mark(); - assert_eq!([mark("host"), mark("again"), mark("tagged")], [Some(4), Some(7), Some(9)]); - } - - #[test] - fn a_document_the_reader_cannot_hold_is_refused() { - for (text, refusal) in [ - ("a: 1\n\"a\": 2\n", "line 2: `a` twice in one mapping"), - ("&k a: 1\n", "line 1: a key that is not a plain scalar"), - ("? [a]\n: 1\n", "a key that is not a plain scalar"), - ("a: &x [*x]\n", "an alias inside the node it names"), - ("a: 1\n---\nb: 2\n", "2 documents, not one"), - ("a: *x\n", "unknown anchor"), - ] { - let said = parse(text).expect_err(text); - assert!(said.contains(refusal), "{text:?}: {said}"); - } - } -} From 6823ea0444fcd5c78ca1c6708ecc409132040f0a Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 21:09:48 +0200 Subject: [PATCH 17/20] The workflow gates that read workflows as text become reviewer instructions or go, and each cache issue names its owner `issues/build/two-workflow-gates-read-the-workflows-as-text.md` is replaced by `issues/build/the-workflow-gates-that-read-workflows-as-text-become-reviewer-instructions-or-go.md`. It names the third gate, src/hostws.rs's `nothing_that_runs_names_a_target_directory_a_member_does_not_have`, gives each of the three a patch that reaches it, its owner (the track `issues/build/the-tooling-is-a-review-prompt-and-three-workflows.md`), and an exit a test can fail: each is deleted with its rule in the review prompt, or reds on its patch. Measured on 038da721e. Each patch was applied with `git apply --check` then `git apply`, built with `cargo test --lib --no-run` (EXIT=0), the three tests run with `--exact`, and reverted with `git apply -R`; the tree matched its state before after every one: patch hosted required member-target publish.yml `runs-on:` label on the next line 0 0 0 publish.yml `pull_request: {branches: [dev]}` 0 0 0 ci.yml `host`'s `if: false` 0 0 0 publish.yml `run: "ls userland/sshd/targe\x74"` 0 0 0 publish.yml `run: ls userland/sshd/target` 0 0 101 The last is the positive control: "publish.yml: names userland/sshd/target". Each run selected one test ("1 passed; 400 filtered out", or "1 failed"). The guest cache's issue loses its paragraph on the deleted allow-list and the exit clause that named it. The two warm-read issues and the guest cache's name their owners, as the limit's does. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- ...cro-expanded-from-a-file-it-never-named.md | 2 ++ ...er-relinks-for-a-file-only-a-flag-names.md | 2 ++ ...and-its-writer-restores-before-it-saves.md | 8 ++---- ...text-become-reviewer-instructions-or-go.md | 28 +++++++++++++++++++ ...rkflow-gates-read-the-workflows-as-text.md | 17 ----------- 5 files changed, 34 insertions(+), 23 deletions(-) create mode 100644 issues/build/the-workflow-gates-that-read-workflows-as-text-become-reviewer-instructions-or-go.md delete mode 100644 issues/build/two-workflow-gates-read-the-workflows-as-text.md diff --git a/issues/build/a-warm-host-run-keeps-what-a-registry-proc-macro-expanded-from-a-file-it-never-named.md b/issues/build/a-warm-host-run-keeps-what-a-registry-proc-macro-expanded-from-a-file-it-never-named.md index 206bfc2c6af..903aa16d908 100644 --- a/issues/build/a-warm-host-run-keeps-what-a-registry-proc-macro-expanded-from-a-file-it-never-named.md +++ b/issues/build/a-warm-host-run-keeps-what-a-registry-proc-macro-expanded-from-a-file-it-never-named.md @@ -19,5 +19,7 @@ Of the proc macros in the lockfiles of the five workspaces the host job builds, `wayland-scanner` alone reads a file it does not name, and no tracked source names it. +Owner: the host cache (`src/cicache.rs`). + Done when a warm read serves what a cold build serves for a path crate that expands a registry proc macro reading another package's file, with a test. diff --git a/issues/build/a-warm-host-run-never-relinks-for-a-file-only-a-flag-names.md b/issues/build/a-warm-host-run-never-relinks-for-a-file-only-a-flag-names.md index 1abca38f0bb..95081ea33c1 100644 --- a/issues/build/a-warm-host-run-never-relinks-for-a-file-only-a-flag-names.md +++ b/issues/build/a-warm-host-run-never-relinks-for-a-file-only-a-flag-names.md @@ -15,5 +15,7 @@ outside the package that links with it and changes alone, a warm `host` run No flag the host job passes names a file: the tracked `.cargo/config.toml` files pass none, and its steps set no `RUSTFLAGS`. +Owner: the host cache (`src/cicache.rs`). + Done when a warm read refuses a flag that names a tracked file, or dates whole the package that links with it, with a test. diff --git a/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md b/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md index 6b9a856c72b..84f6a10f7ab 100644 --- a/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md +++ b/issues/build/the-guest-cache-is-read-by-mtime-and-its-writer-restores-before-it-saves.md @@ -19,10 +19,6 @@ every night: 4,002,930,524 B at the host's `LIMIT` (`src/cicache.rs`), or night's host entry unless a pull request has read the entry since the guest restore; every pull request then runs cold, and nothing reds. -Its jobs are not held to the allow-list `src/ci.rs`'s -`each_cache_has_one_writer` holds the host cache's jobs to: `|| true` on -`tcg`'s run line would let its save store a red run's tree, and the gate stays -green. +Owner: the orchestrator. -Done when the guest entry is written cold, read by content, and bounded, and -its jobs are held to that allow-list, which #671 carries. +Done when the guest entry is written cold, read by content, and bounded. diff --git a/issues/build/the-workflow-gates-that-read-workflows-as-text-become-reviewer-instructions-or-go.md b/issues/build/the-workflow-gates-that-read-workflows-as-text-become-reviewer-instructions-or-go.md new file mode 100644 index 00000000000..0090976e132 --- /dev/null +++ b/issues/build/the-workflow-gates-that-read-workflows-as-text-become-reviewer-instructions-or-go.md @@ -0,0 +1,28 @@ +--- +status: open +kind: tooling +opened: 2026-10-01 +--- + +# The workflow gates that read workflows as text become reviewer instructions or go + +Three tests read `.github/workflows/` as text, and each passes a patch that +breaks its rule as GitHub reads the YAML. Each patch, alone, left all three +green (EXIT 0): +- `src/ci.rs`'s `workflows_run_against_main_on_hosted_runners` reads `runs-on:` + and `pull_request:` off single lines. On `publish.yml`: `runs-on:` with its + label on the next line, `- self-hosted`; and `pull_request: {branches: [dev]}` + beside `workflow_dispatch`. +- `src/ci.rs`'s `the_required_check_is_a_job_on_every_pull_request` finds + ci.yml's triggers and its `host` by substring. On `ci.yml`: `host`'s `if:` + made `false`. A skipped job reports success, so the required check is green. +- `src/hostws.rs`'s + `nothing_that_runs_names_a_target_directory_a_member_does_not_have` finds a + `/target` by substring. On `publish.yml`: a step + `run: "ls userland/sshd/targe\x74"`, which YAML reads as + `ls userland/sshd/target`. The same step spelled plainly reds it (EXIT 101). + +Owner: `issues/build/the-tooling-is-a-review-prompt-and-three-workflows.md`. + +Done when each test is deleted with its rule a sentence in +`.claude/agents/reviewer.md`, or reds on its patch above. diff --git a/issues/build/two-workflow-gates-read-the-workflows-as-text.md b/issues/build/two-workflow-gates-read-the-workflows-as-text.md deleted file mode 100644 index c9bbc068cb6..00000000000 --- a/issues/build/two-workflow-gates-read-the-workflows-as-text.md +++ /dev/null @@ -1,17 +0,0 @@ ---- -status: open -kind: tooling -opened: 2026-10-01 ---- - -# Two workflow gates read the workflows as text - -`src/ci.rs`'s `workflows_run_against_main_on_hosted_runners` reads `runs-on:` -and `pull_request:` off single lines, and -`the_required_check_is_a_job_on_every_pull_request` finds ci.yml's triggers and -its `host` by substring. A spelling YAML reads the same way passes them. Each -patch below, alone on `publish.yml`, left both tests green (EXIT 0): -- `runs-on:` with its label on the next line, `- self-hosted`; -- `pull_request: {branches: [dev]}` beside `workflow_dispatch`. - -Done when both read `src/workflow.rs`'s structure. From c282a7669ebff9968eecf001bda90844f5c8dfae Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 21:14:12 +0200 Subject: [PATCH 18/20] The Caches rule holds both host-cache jobs to the driver's target The deleted gate held each job that names the host cache to `CARGO_TARGET_DIR: target/ci-driver`, and the review sentence did not. Without it a reader is not carried: it runs its restored tree judged by cargo's mtimes alone, with no read by content and no image check, and nothing at run time says so. The writer without it is refused by `cicache::cold`, so only the reader needed the sentence, but it holds both, as the gate did. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- .claude/agents/reviewer.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/.claude/agents/reviewer.md b/.claude/agents/reviewer.md index 6e8ec3af662..e2fd18bc1a0 100644 --- a/.claude/agents/reviewer.md +++ b/.claude/agents/reviewer.md @@ -82,11 +82,12 @@ above; otherwise it is a NOTE. A new cargo feature or `cfg` arm of one, and every arm a changed `src/clippy.rs` shape stops building, is shown linted in the pull request body: a `mem::forget` planted in that arm turns `cargo run -- --clippy` red. - **Caches.** No gate reads these; a diff that breaks one is a BLOCKER. Each cache has one writer, a nightly.yml job; the host cache's is nightly's `host`, and its one reader ci.yml's - `host`, on the same `runs-on`, both caching `src/cicache.rs`'s `PATHS`. A job that names the - host cache runs `actions/checkout`, its cache step and `cargo run -- --ci `, and nothing - else. The save's guard, `github.ref == 'refs/heads/main'`, is the only step-level `if:` in those - jobs, and ci.yml's `host` skips only a draft. No `continue-on-error`, `shell:`, `defaults:` or - cargo `runner` reaches them. nightly.yml's `on:` is one daily `schedule` and `workflow_dispatch`. + `host`, on the same `runs-on`, both caching `src/cicache.rs`'s `PATHS` with its `DRIVER` as + their `CARGO_TARGET_DIR`. A job that names the host cache runs `actions/checkout`, its cache + step and `cargo run -- --ci `, and nothing else. The save's guard, + `github.ref == 'refs/heads/main'`, is the only step-level `if:` in those jobs, and ci.yml's + `host` skips only a draft. No `continue-on-error`, `shell:`, `defaults:` or cargo `runner` + reaches them. nightly.yml's `on:` is one daily `schedule` and `workflow_dispatch`. - **Growth.** Every line is a responsibility, not an asset. State the branch's net lines (`git diff --shortstat origin/main...HEAD`), production and tests apart. Production code that grows needs a reason you accept; a branch that could delete more than it adds and does not goes back From 883adf16af1a53f82c2a073d7f2aa26e322f8295 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 2 Oct 2026 09:37:03 +0200 Subject: [PATCH 19/20] The Caches rule names the writer's `if:`, the jobs' `env:` and each one's job Round 10's review names three things the deleted workflow gate pinned and the Caches bullet did not, each one a line a reviewer reads off a diff: - nightly's `host` has no job-level `if:`. An `if: false` there stops the one writer, and a skipped job is a green check. - `CARGO_TARGET_DIR` is the one variable an `env:` gives either job. `cicache::runner` reads `ImageOS` and `ImageVersion` from the environment, so one set in both jobs outlives an image move and the image check passes. - the writer runs `--ci seal` and the reader `--ci host`. A writer on `--ci host` saves no manifest, and every pull request then refuses the targets it restored. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm --- .claude/agents/reviewer.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/.claude/agents/reviewer.md b/.claude/agents/reviewer.md index 8a7eb7f4021..2f293372468 100644 --- a/.claude/agents/reviewer.md +++ b/.claude/agents/reviewer.md @@ -79,11 +79,14 @@ if it meets the bar above; otherwise it is a NOTE. - **Caches.** No gate reads these; a diff that breaks one is a BLOCKER. Each cache has one writer, a nightly.yml job; the host cache's is nightly's `host`, and its one reader ci.yml's `host`, on the same `runs-on`, both caching `src/cicache.rs`'s `PATHS` with its `DRIVER` as - their `CARGO_TARGET_DIR`. A job that names the host cache runs `actions/checkout`, its cache - step and `cargo run -- --ci `, and nothing else. The save's guard, - `github.ref == 'refs/heads/main'`, is the only step-level `if:` in those jobs, and ci.yml's - `host` skips only a draft. No `continue-on-error`, `shell:`, `defaults:` or cargo `runner` - reaches them. nightly.yml's `on:` is one daily `schedule` and `workflow_dispatch`. + their `CARGO_TARGET_DIR`, the one variable an `env:` gives either: an `ImageOS` or + `ImageVersion` set there outlives an image move. A job that names the host cache runs + `actions/checkout`, its cache step and `cargo run -- --ci `, `seal` in the writer and + `host` in the reader, and nothing else. The save's guard, `github.ref == 'refs/heads/main'`, + is the only step-level `if:` in those jobs; ci.yml's `host` skips only a draft, and nightly's + `host` has no `if:` of its own, a skipped job being a green check. No `continue-on-error`, + `shell:`, `defaults:` or cargo `runner` reaches them. nightly.yml's `on:` is one daily + `schedule` and `workflow_dispatch`. - **Growth.** Every line is a responsibility, not an asset. State the branch's net lines (`git diff --shortstat origin/main...`), production and tests apart. Production code that grows needs a reason you accept; a branch that could delete more than it adds and does not goes From 79bd4ec49f57898921c59b087dfc7698ab0f9cd5 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 2 Oct 2026 10:08:13 +0200 Subject: [PATCH 20/20] The review prompt takes the one-writer test's rule, and Caches names the keys and the order Round 11's review left two NOTEs, both on `.claude/agents/reviewer.md`. The cut of `ci::tests::each_cache_has_one_writer` stood on nothing the prompt said: Growth cut a test "only when it tests nothing, or as **Guest tests** says". The orchestrator's decision is that the test stays cut and the sentence names the case, so it gains one clause: a test is also cut when this prompt takes its rule. Caches then has to hold all the test held. It already said one writer per cache, a nightly.yml job; the test also refused the combined `actions/cache`, whose post step saves, and that is now a clause of the same sentence, since a diff that adds it shows no `save`. Caches gains the two things the deleted workflow gate pinned and the bullet did not say, each a cold pull request run behind green checks when broken: - the reader's `restore-keys` is the writer's `key` up to its run id: a head renamed on one side leaves every pull request on the old name's last entry, or on none; - the reader restores before its `--ci host` and the writer saves after its `--ci seal`: a save above the seal stores no target, and the nightly stays green. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm --- .claude/agents/reviewer.md | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/.claude/agents/reviewer.md b/.claude/agents/reviewer.md index 2f293372468..818c3a24104 100644 --- a/.claude/agents/reviewer.md +++ b/.claude/agents/reviewer.md @@ -77,16 +77,18 @@ if it meets the bar above; otherwise it is a NOTE. shape stops building, that no shape in `src/clippy.rs` lints; an `issues/` file added, changed or deleted against `issues/README.md`. - **Caches.** No gate reads these; a diff that breaks one is a BLOCKER. Each cache has one - writer, a nightly.yml job; the host cache's is nightly's `host`, and its one reader ci.yml's - `host`, on the same `runs-on`, both caching `src/cicache.rs`'s `PATHS` with its `DRIVER` as - their `CARGO_TARGET_DIR`, the one variable an `env:` gives either: an `ImageOS` or - `ImageVersion` set there outlives an image move. A job that names the host cache runs - `actions/checkout`, its cache step and `cargo run -- --ci `, `seal` in the writer and - `host` in the reader, and nothing else. The save's guard, `github.ref == 'refs/heads/main'`, - is the only step-level `if:` in those jobs; ci.yml's `host` skips only a draft, and nightly's - `host` has no `if:` of its own, a skipped job being a green check. No `continue-on-error`, - `shell:`, `defaults:` or cargo `runner` reaches them. nightly.yml's `on:` is one daily - `schedule` and `workflow_dispatch`. + writer, a nightly.yml job, and no workflow uses the combined `actions/cache`, which saves too; + the host cache's is nightly's `host`, and its one reader ci.yml's `host`, on the same + `runs-on`, both caching `src/cicache.rs`'s `PATHS` with its `DRIVER` as their + `CARGO_TARGET_DIR`, the one variable an `env:` gives either: an `ImageOS` or `ImageVersion` + set there outlives an image move. The reader's `restore-keys` is the writer's `key` up to its + run id. A job that names the host cache runs `actions/checkout`, its cache step and + `cargo run -- --ci `, `seal` in the writer and `host` in the reader, and nothing else; + the reader restores before that step, and the writer saves after it. The save's guard, + `github.ref == 'refs/heads/main'`, is the only step-level `if:` in those jobs; ci.yml's `host` + skips only a draft, and nightly's `host` has no `if:` of its own, a skipped job being a green + check. No `continue-on-error`, `shell:`, `defaults:` or cargo `runner` reaches them. + nightly.yml's `on:` is one daily `schedule` and `workflow_dispatch`. - **Growth.** Every line is a responsibility, not an asset. State the branch's net lines (`git diff --shortstat origin/main...`), production and tests apart. Production code that grows needs a reason you accept; a branch that could delete more than it adds and does not goes @@ -94,9 +96,9 @@ if it meets the bar above; otherwise it is a NOTE. reader can check: that rule is a sentence in a prompt. What could be deleted, merged into what exists, or made smaller? An abstraction with one caller, a parameter with one value, dead code, code kept "just in case" or because nobody knows whether it is needed. Size is never bought with - a weaker check: a test is cut only when it tests nothing, or as **Guest tests** says. A - compromise the branch found is removed or recorded in `issues/` with an owner, evidence and an - exit condition. + a weaker check: a test is cut only when it tests nothing, when this prompt takes its rule, or + as **Guest tests** says. A compromise the branch found is removed or recorded in `issues/` with + an owner, evidence and an exit condition. - **Tests.** The refusals and the boundary, not the happy path. - **Edges.** Untrusted input never panics the kernel; it is refused. Check-then-act races. A lock held across a user copy or a device wait. Arithmetic on a value the caller chooses. A short