From 31145c39bcf37b9664486164e558e78064658af4 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 10:22:06 +0200 Subject: [PATCH 1/8] The primary's fork checkout is moved to the commit its tree pins The primary built std, and decided whether to bootstrap, from whatever commit its `rust/` held: `fork_checkout` returned the primary's checkout as it stood (since 86035593c), and `toolchain::ensure` decided the bootstrap before asking it. `git pull` moves the gitlink and never the submodule, so after #702 the primary's `rust/` at 95960d6c compiled against a `toyos-abi` that had moved on, and the owner's build of `main` broke. The licence gate read the same stale `library/`. Now the primary's checkout is moved to the pin whenever its `HEAD` differs, ahead or behind: it is no workspace, so nothing ahead is kept. It is refused by name when it has local changes, or does not hold the pinned commit (the refusal names the `git fetch` that fetches it); no fetch is made for it. The move holds the worktree lock and then the global lock exclusively, in `buildlock.rs`'s declared order, because every worktree's compiler and LLVM are built from that checkout. An uninitialised `rust/` (a runner's) is left to whoever initialises it: git there would walk up into the superproject. `toolchain::ensure` moves the checkout before it decides the bootstrap. Two latent gaps the same defect left in CI: `ensure_shallow_fork` accepted any `rust/` with an `x.py`, at whatever commit, and `release::bootstrap` keyed its layers from `rust/` as it stood. The first now refuses a checkout off the pin, and the second runs it first. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- src/buildlock.rs | 7 ++ src/compiler.rs | 1 + src/lib.rs | 12 ++- src/release.rs | 1 + src/sysroot.rs | 202 +++++++++++++++++++++++++++++++++-------------- src/toolchain.rs | 2 + 6 files changed, 162 insertions(+), 63 deletions(-) diff --git a/src/buildlock.rs b/src/buildlock.rs index d4a95e7af6f..9968ef74a78 100644 --- a/src/buildlock.rs +++ b/src/buildlock.rs @@ -208,6 +208,13 @@ pub fn compiler_shared(root: &Path, what: &str) -> Guard { acquire(&git_lock_dir(root), LOCK_SH, what, BUILD) } +/// The global lock exclusively: what the primary holds, inside its worktree +/// lock held exclusively, while it moves its fork checkout, which every +/// worktree's compiler is built from. +pub fn global_exclusive(root: &Path, what: &str) -> Guard { + acquire(&git_lock_dir(root), LOCK_EX, what, BUILD) +} + /// Exclusive lock over the shared cargo artifact paths. /// /// Cargo keys an artifact path on (crate, target, profile) and nothing else, so diff --git a/src/compiler.rs b/src/compiler.rs index a1d2de33046..2377bbb6384 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -431,6 +431,7 @@ pub(crate) mod tests { write(&fork.join("src/bootstrap/src/lib.rs"), "fn main() {}\n"); write(&fork.join("src/stage0"), "compiler_version=beta\n"); write(&fork.join("Cargo.lock"), "# lock\n"); + write(&fork.join("x.py"), "\n"); write(&fork.join("library/std/src/lib.rs"), "pub fn a() {}\n"); write(&fork.join(".gitignore"), "/build\n"); git(&fork, &["add", "-A"]); diff --git a/src/lib.rs b/src/lib.rs index 83d3029567a..3b94b39f6c8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -120,12 +120,18 @@ pub fn ensure_submodules(repo_dir: &Path) { /// `rust/` at the commit this tree pins and with no history, where a CI /// runner's checkout has none: what reading the fork's sources and building -/// the toolchain from them need. Refused in a linked worktree, whose `rust/` is +/// the toolchain from them need. One already there at another commit is +/// refused. Refused in a linked worktree, whose `rust/` is /// `sysroot::fork_checkout`'s to make: `git submodule` there rewrites the /// `core.worktree` of the primary's fork. pub fn ensure_shallow_fork(root: &Path) -> Result<(), String> { - if root.join("rust/x.py").exists() { - return Ok(()); + let fork = root.join("rust"); + if fork.join("x.py").exists() { + let head = sysroot::git_out(&fork, &["rev-parse", "HEAD"]).trim().to_string(); + let pinned = sysroot::pinned_fork(root); + return (head == pinned) + .then_some(()) + .ok_or_else(|| format!("{} is at {head}, and this tree pins the fork at {pinned}", fork.display())); } if let toolchain::Owner::Elsewhere(_) = toolchain::owner(root) { return Err(format!( diff --git a/src/release.rs b/src/release.rs index 2ba36b343ec..1ff7c20e2a8 100644 --- a/src/release.rs +++ b/src/release.rs @@ -196,6 +196,7 @@ fn outputs(layers: &[Layer]) -> String { /// than its build does, and so is one the build left not whole under its key. pub fn bootstrap(root: &Path) -> Result { let file = step_outputs()?; + crate::ensure_shallow_fork(root)?; let layers = layers(root); let restored: Vec = layers.iter().map(|layer| root.join(&layer.paths[0]).exists()).collect(); whole(&layers, &restored, |layer| defect(root, layer)).map_err(|why| format!("restored, {why}"))?; diff --git a/src/sysroot.rs b/src/sysroot.rs index b1e608fc4e3..720a8289c5e 100644 --- a/src/sysroot.rs +++ b/src/sysroot.rs @@ -19,7 +19,8 @@ //! it, and a crate built against a sysroot learns which targets' libraries //! moved ([`Identity`]). Each build refuses dep-info that says otherwise. //! -//! **Each worktree builds std in its own fork checkout.** The primary builds in its `rust/`; +//! **Each worktree builds std in its own fork checkout.** The primary builds in its `rust/`, +//! moved to the commit its tree pins; //! a linked worktree in its own `rust/`, made on first need as a git worktree of //! the primary's fork repository at the commit this tree pins ([`fork_checkout`]). //! `library/std` names `toyos-abi` and `toyos` as `../../../`, so each @@ -485,7 +486,7 @@ fn key_of(root: &Path, freestanding: &Key, build: &str) -> Key { /// The commit this checkout's tree pins the std fork at: the index's, so a /// staged gitlink counts as the tree's. -fn pinned_fork(root: &Path) -> String { +pub(crate) fn pinned_fork(root: &Path) -> String { let entry = git_out(root, &["ls-files", "-s", "--", "rust"]); let mut words = entry.split_whitespace(); match (words.next(), words.next()) { @@ -494,21 +495,23 @@ fn pinned_fork(root: &Path) -> String { } } -/// The fork checkout `root`'s std is built in. +/// The fork checkout `root`'s std is built in, at the commit `root`'s tree pins. /// -/// The primary's is its own `rust/`. A linked worktree's `rust/` starts as the -/// empty stub `git worktree add` leaves; it is made here, the first time it is -/// needed, as a git worktree of the primary's fork repository at the commit -/// this tree pins, sharing its objects — and `library/backtrace` the same way -/// from the primary's, or by git's own clone where the primary does not hold -/// that commit. +/// The primary's is its own `rust/`, used where it is not initialised yet, which +/// whoever initialises it does at the pin. It is no workspace, and every +/// worktree's compiler and LLVM are built from it, so one whose `HEAD` is not the +/// pin is moved there, under the global lock held exclusively; refused by name +/// if it has local changes, or does not hold the pinned commit. /// -/// A checkout that exists is used as it stands, which is where an agent edits -/// the fork; one whose `HEAD` is neither the pinned commit nor ahead of it is -/// moved there itself, fetching the commit from the primary's repository first -/// if the checkout does not already hold it, unless the checkout has local -/// changes, in which case it is refused by name rather than moved out from -/// under whoever made them. +/// A linked worktree's `rust/` starts as the empty stub `git worktree add` +/// leaves; it is made here, the first time it is needed, as a git worktree of +/// the primary's fork repository at the pin, sharing its objects — and +/// `library/backtrace` the same way from the primary's, or by git's own clone +/// where the primary does not hold that commit. It is where an agent edits the +/// fork, so one ahead of the pin is used as it stands; one neither at the pin +/// nor ahead of it is moved there, fetching the commit from the primary's +/// repository first if it does not hold it, and refused by name if it has local +/// changes rather than moved out from under whoever made them. /// /// Every build in a worktree asks this at once, so the making and the move are /// each decided and done under the worktree's lock held exclusively @@ -517,63 +520,68 @@ fn pinned_fork(root: &Path) -> String { pub fn fork_checkout(root: &Path, lock: &mut Held) -> PathBuf { let fork = root.join("rust"); let primary = match toolchain::owner(root) { - Owner::Us => return fork, + Owner::Us if !fork.join(".git").exists() => return fork, + Owner::Us => None, Owner::Installed => panic!("an installed toolchain has no fork checkout to build std in"), - Owner::Elsewhere(primary) => primary, + Owner::Elsewhere(primary) => Some(primary), }; let pinned = pinned_fork(root); - lock.act_if( - Scope::Worktree, - "make the fork checkout", - || (!fork.join(".git").exists()).then_some(()), - |()| { - let stub = fs::read_dir(&fork).map_or(0, |d| d.count()); - assert!( - stub == 0, - "{} is neither a fork checkout nor the empty stub a worktree starts with", - fork.display() - ); - let _ = fs::remove_dir(&fork); - eprintln!("Making {} a fork checkout at {pinned} (a git worktree of the primary's)", fork.display()); - git_run(&primary.join("rust"), &["worktree", "add", "--detach", path_str(&fork), &pinned]); - let backtrace = git_out(&fork, &["ls-tree", "HEAD", "library/backtrace"]); - let commit = backtrace.split_whitespace().nth(2).unwrap_or_else(|| { - panic!("{} pins no library/backtrace: {backtrace:?}", fork.display()) - }); - let theirs = primary.join("rust/library/backtrace"); - let held = Command::new("git") - .args(["cat-file", "-e", &format!("{commit}^{{commit}}")]) - .current_dir(&theirs) - .status() - .is_ok_and(|s| s.success()); - let at = fork.join("library/backtrace"); - if held { - let _ = fs::remove_dir(&at); - git_run(&theirs, &["worktree", "add", "--detach", path_str(&at), commit]); - } else { - git_run(&fork, &["submodule", "update", "--init", "library/backtrace"]); - } - }, - ); + if let Some(primary) = &primary { + lock.act_if( + Scope::Worktree, + "make the fork checkout", + || (!fork.join(".git").exists()).then_some(()), + |()| { + let stub = fs::read_dir(&fork).map_or(0, |d| d.count()); + assert!( + stub == 0, + "{} is neither a fork checkout nor the empty stub a worktree starts with", + fork.display() + ); + let _ = fs::remove_dir(&fork); + eprintln!("Making {} a fork checkout at {pinned} (a git worktree of the primary's)", fork.display()); + git_run(&primary.join("rust"), &["worktree", "add", "--detach", path_str(&fork), &pinned]); + let backtrace = git_out(&fork, &["ls-tree", "HEAD", "library/backtrace"]); + let commit = backtrace.split_whitespace().nth(2).unwrap_or_else(|| { + panic!("{} pins no library/backtrace: {backtrace:?}", fork.display()) + }); + let theirs = primary.join("rust/library/backtrace"); + let held = Command::new("git") + .args(["cat-file", "-e", &format!("{commit}^{{commit}}")]) + .current_dir(&theirs) + .status() + .is_ok_and(|s| s.success()); + let at = fork.join("library/backtrace"); + if held { + let _ = fs::remove_dir(&at); + git_run(&theirs, &["worktree", "add", "--detach", path_str(&at), commit]); + } else { + git_run(&fork, &["submodule", "update", "--init", "library/backtrace"]); + } + }, + ); + } lock.act_if( Scope::Worktree, "move the fork checkout to its pin", || { let head = git_out(&fork, &["rev-parse", "HEAD"]).trim().to_string(); - let ahead = Command::new("git") - .args(["merge-base", "--is-ancestor", &pinned, &head]) - .current_dir(&fork) - .status() - .is_ok_and(|s| s.success()); + let ahead = primary.is_some() + && Command::new("git") + .args(["merge-base", "--is-ancestor", &pinned, &head]) + .current_dir(&fork) + .status() + .is_ok_and(|s| s.success()); (head != pinned && !ahead).then_some(head) }, |head| { + let _global = primary.is_none().then(|| buildlock::global_exclusive(root, "move the fork checkout to its pin")); let dirty = git_out(&fork, &["status", "--porcelain", "--ignore-submodules=none"]); assert!( dirty.is_empty(), - "{} is at {head} with uncommitted work, and this tree pins the fork at {pinned}, which \ - that is not ahead of: a build here would compile a std this tree does not name, and \ - moving the checkout would lose that work.\n{dirty}", + "{} is at {head} with uncommitted work, and this tree pins the fork at {pinned}: a build \ + here would compile a std this tree does not name, and moving the checkout would lose \ + that work.\n{dirty}", fork.display(), ); let held = Command::new("git") @@ -582,10 +590,18 @@ pub fn fork_checkout(root: &Path, lock: &mut Held) -> PathBuf { .status() .is_ok_and(|s| s.success()); if !held { - git_run(&fork, &["fetch", path_str(&primary.join("rust")), &pinned]); + match &primary { + Some(primary) => git_run(&fork, &["fetch", path_str(&primary.join("rust")), &pinned]), + None => panic!( + "{} is at {head}, and this tree pins the fork at {pinned}, which it does not hold: \ + `git -C {} fetch origin {pinned}` fetches it", + fork.display(), + fork.display(), + ), + } } git_run(&fork, &["checkout", "--detach", "-q", &pinned]); - eprintln!("{} was at {head}, behind this tree's pin {pinned}: checked it out", fork.display()); + eprintln!("{} was at {head}, and this tree pins {pinned}: checked it out", fork.display()); }, ); fork @@ -1527,6 +1543,72 @@ mod tests { assert!(refused.as_ref().is_err_and(|why| why.contains("linked worktree")), "{refused:?}"); } + /// **The primary's fork checkout is the commit its tree pins**: one behind + /// the pin, as a `git pull` leaves it, and one ahead of it are moved there; + /// one with local changes, or pinned at a commit it does not hold, is + /// refused by name and not moved; one not initialised yet is left to + /// whoever initialises it, and git is run in no other repository. + #[test] + fn the_primary_s_fork_checkout_is_moved_to_its_pin() { + let base = TempDir::new("fork-primary"); + let (primary, _linked, c1, c2) = two_pins(&base); + let fork = primary.join("rust"); + let pin = |commit: &str| { + git(&primary, &["update-index", "--cacheinfo", &format!("160000,{commit},rust")]); + git(&primary, &["commit", "-qm", "a pin"]); + }; + let head = || git(&fork, &["rev-parse", "HEAD"]); + let mut lock = buildlock::shared(&primary, "a build"); + + pin(&c2); + assert_eq!(fork_checkout(&primary, &mut lock), fork); + assert_eq!(head(), c2, "a checkout behind its pin was not moved to it"); + + pin(&c1); + assert_eq!(fork_checkout(&primary, &mut lock), fork); + assert_eq!(head(), c1, "a checkout ahead of its pin was kept"); + + pin(&c2); + write(&fork.join("library/std/src/lib.rs"), "pub fn uncommitted() {}\n"); + let said = refusal(|| drop(fork_checkout(&primary, &mut lock))); + assert!(said.contains(&format!("is at {c1} with uncommitted work")) && said.contains(&c2), "{said}"); + assert_eq!(head(), c1, "a checkout with local changes was moved"); + git(&fork, &["checkout", "-q", "--", "."]); + + let upstream = base.join("fork-src"); + write(&upstream.join("library/std/src/lib.rs"), "pub fn c() {}\n"); + git(&upstream, &["commit", "-qam", "C3"]); + let c3 = git(&upstream, &["rev-parse", "HEAD"]); + pin(&c3); + let said = refusal(|| drop(fork_checkout(&primary, &mut lock))); + assert!(said.contains(&format!("`git -C {} fetch origin {c3}` fetches it", fork.display())), "{said}"); + assert_eq!(head(), c1, "a checkout was moved to a commit it does not hold"); + + fs::remove_dir_all(&fork).unwrap(); + fs::create_dir(&fork).unwrap(); + let superproject = git(&primary, &["rev-parse", "HEAD"]); + assert_eq!(fork_checkout(&primary, &mut lock), fork); + assert_eq!(git(&primary, &["rev-parse", "HEAD"]), superproject, "git ran in the superproject"); + } + + /// **The primary's bootstrap decides from the `compiler/` its tree pins**: + /// a fork checkout left at a commit naming another `compiler/` reads as a + /// stale compiler, and once it is moved to the pin the compiler the primary + /// built is current again. + #[test] + fn the_primary_s_bootstrap_decides_from_the_pinned_compiler() { + let scratch = TempDir::new("fork-primary-compiler"); + let (primary, rust_dir, [_, a, _]) = crate::compiler::tests::estate(&scratch); + let pinned = git(&rust_dir, &["rev-parse", "HEAD"]); + git(&rust_dir, &["checkout", "-q", "--detach", &git(&a.join("rust"), &["rev-parse", "HEAD"])]); + assert!(!compiler::primary_is_current(&rust_dir), "another compiler/ read as the one built"); + + let mut lock = buildlock::shared(&primary, "a build"); + assert_eq!(fork_checkout(&primary, &mut lock), rust_dir); + assert_eq!(git(&rust_dir, &["rev-parse", "HEAD"]), pinned); + assert!(compiler::primary_is_current(&rust_dir), "the pinned compiler/ read as stale"); + } + /// The primary's compiler under `base`: `rustc` and `rust-lld`, and the C /// toolchain `src/clang.rs` provisions beside them if `clang`; no cargo. fn primary_compiler(base: &Path, clang: bool) -> Compiler { diff --git a/src/toolchain.rs b/src/toolchain.rs index b7a00883cc3..736ad0f7d5d 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -560,6 +560,8 @@ pub fn ensure(root: &Path, lock: &mut buildlock::Held, hosted_rustc: bool) -> Sy Owner::Us => {} } + // The bootstrap decides from the `compiler/` this tree pins. + sysroot::fork_checkout(root, lock); let hosted_stamp = stamps_dir.join("hosted-rustc.stamp"); lock.act_if( Scope::Global, From 72621e9cb2a1284492d2185ff77a18d967dd2751 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 10:24:00 +0200 Subject: [PATCH 2/8] The move waits for the global lock, and a shallow fork off its pin is refused Two tests the first commit's mutations showed it lacked: removing the global lock from the primary's move, and accepting a shallow fork at any commit, each stayed green. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- src/buildlock.rs | 14 ++++++++++++++ src/sysroot.rs | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/src/buildlock.rs b/src/buildlock.rs index 9968ef74a78..5d36317eb4e 100644 --- a/src/buildlock.rs +++ b/src/buildlock.rs @@ -636,6 +636,16 @@ pub(crate) mod tests { Elsewhere::hold("buildlock::tests::child_role", &env) } + /// The primary's compiler of `root`, read by a process of its own. + pub(crate) fn compiler_read_elsewhere(root: &Path) -> Elsewhere { + held_elsewhere(root, "read-compiler") + } + + /// Whether an exclusive acquirer of `root`'s global lock is queued for it. + pub(crate) fn global_queued(root: &Path) -> bool { + !try_lock(&open_lock_file(&git_lock_dir(root).join("intent")), LOCK_SH) + } + /// What `role` of [`child_role`] takes in `root`, held by a process of its own. fn held_elsewhere(root: &Path, role: &str) -> Elsewhere { Elsewhere::hold("buildlock::tests::child_role", &[(ROLE, OsStr::new(role)), (ROOT, root.as_os_str())]) @@ -774,6 +784,10 @@ pub(crate) mod tests { let _using = keyed_using(&root, Keyed::Sysroot, &Key::parse(&std::env::var(KEY).unwrap()).unwrap()); hold_until_released(); } + "read-compiler" => { + let _reading = compiler_shared(&root, "child"); + hold_until_released(); + } "clean" | "clean-unlocked" => { touch(&root.join("cleaner-ready")); assert!(appeared(&root.join("builder-mid"), Duration::from_secs(20))); diff --git a/src/sysroot.rs b/src/sysroot.rs index 720a8289c5e..5cd9cd43121 100644 --- a/src/sysroot.rs +++ b/src/sysroot.rs @@ -1591,6 +1591,45 @@ mod tests { assert_eq!(git(&primary, &["rev-parse", "HEAD"]), superproject, "git ran in the superproject"); } + /// **The primary's fork checkout moves only while nobody reads its + /// compiler**: a sysroot being made in another worktree holds the global + /// lock shared, and the move queues for it and moves nothing until it is + /// let go. + #[test] + fn the_primary_s_fork_checkout_moves_only_while_nobody_reads_its_compiler() { + let base = TempDir::new("fork-primary-global"); + let (primary, _linked, c1, c2) = two_pins(&base); + let fork = primary.join("rust"); + git(&primary, &["update-index", "--cacheinfo", &format!("160000,{c2},rust")]); + git(&primary, &["commit", "-qm", "pins C2"]); + let reader = buildlock::tests::compiler_read_elsewhere(&primary); + std::thread::scope(|scope| { + let mover = scope.spawn(|| drop(fork_checkout(&primary, &mut buildlock::shared(&primary, "a build")))); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20); + while !buildlock::tests::global_queued(&primary) { + assert!(!mover.is_finished(), "the move took no global lock"); + assert!(std::time::Instant::now() < deadline, "the move never queued for the global lock"); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert_eq!(git(&fork, &["rev-parse", "HEAD"]), c1, "the checkout moved while a sysroot build read its compiler"); + reader.release(); + mover.join().unwrap(); + }); + assert_eq!(git(&fork, &["rev-parse", "HEAD"]), c2); + } + + /// **A shallow fork is the commit its tree pins**: one already there is + /// accepted at the pin and refused, naming both commits, anywhere else. + #[test] + fn ensure_shallow_fork_refuses_a_fork_off_its_pin() { + let base = TempDir::new("fork-shallow-pin"); + let (primary, _linked, c1, c2) = two_pins(&base); + assert_eq!(crate::ensure_shallow_fork(&primary), Ok(())); + git(&primary.join("rust"), &["checkout", "-q", &c2]); + let refused = crate::ensure_shallow_fork(&primary); + assert!(refused.as_ref().is_err_and(|why| why.contains(&format!("is at {c2}")) && why.contains(&c1)), "{refused:?}"); + } + /// **The primary's bootstrap decides from the `compiler/` its tree pins**: /// a fork checkout left at a commit naming another `compiler/` reads as a /// stale compiler, and once it is moved to the pin the compiler the primary From a34c2b7d3c3b6ef2e4d231c8723c573a62c2553b Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 10:45:04 +0200 Subject: [PATCH 3/8] File virt_el1_smp's stall on counters_read under a loaded host Seen in this branch's whole-suite run at 72621e9cb, which changes no guest byte; the host's load is recorded with it, as root CLAUDE.md asks of a red seen under load. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...smp-stalled-on-counters-read-under-load.md | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 issues/kernel/virt-el1-smp-stalled-on-counters-read-under-load.md diff --git a/issues/kernel/virt-el1-smp-stalled-on-counters-read-under-load.md b/issues/kernel/virt-el1-smp-stalled-on-counters-read-under-load.md new file mode 100644 index 00000000000..3b39ae8b06c --- /dev/null +++ b/issues/kernel/virt-el1-smp-stalled-on-counters-read-under-load.md @@ -0,0 +1,33 @@ +--- +status: open +kind: defect +opened: 2026-10-04 +--- + +# `virt_el1_smp` stalled waiting for `test_rs_counters_read` to end, under a loaded host + +**What was seen.** `cargo test`, the whole guest suite, dev host, on +`wt/toyos-forkpin` at `72621e9cb`, a branch that changes only the build +system's fork checkout and no guest byte: + +``` +FAIL virt_el1_smp: STALLED: waiting for the job test_rs_counters_read to end — it went quiet + STALL virt_el1_smp (33s) +test result: FAILED. 25 passed, 1 failed, 0 invalidated, 26 total +``` + +The serial the harness printed ends with the guest powering off on its own: +`[kernel 2.408 cpu4] spawn: /system/bin/shutdown`, the supervisor's +`power: the machine stops ... (Shutdown)`, then `[kernel 2.419 cpu0] Shutting +down.` — so the guest did not hang; the job's end never reached the harness +before the machine stopped. The printed serial elides its middle, and +`target/red-run-serial/` kept no capture of this guest, so whether +`counters_read` printed its verdict is not known. In the same run `virt_smp` +(entered at EL2) passed with `counters_read: every cpu answered for itself`. + +Host load at the end of the run, from `uptime`: `17.65 28.91 33.61` on 14 +cores. + +**Exit**: a capture of a red `virt_el1_smp` whole enough to say whether +`counters_read` ended before the shutdown, and the cause it names fixed with +a test that reds without it. From dd27e9f32262a9dc95179e0910f99ad3d28c7bcc Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 12:00:34 +0200 Subject: [PATCH 4/8] Answer review round 1: nested submodules move with the fork checkout, and the unreachable CI refusals go - The move checks out every initialised nested submodule at the commit the new pin records, with `git checkout` in it and never `git submodule`, which run in a linked fork checkout rewrites the primary's nested `core.worktree`. It refuses by name, before moving anything, a nested commit not held. A nested gitlink moved and nothing more is no longer read as uncommitted work. - `two_pins`' C1 and C2 now record different `library/backtrace` commits, so the primary and linked tests reach the nested move. - The global-lock test holds a `Scope::Global` phase in another process, as a bootstrap does; that is what the lock keeps the move out of. - `toolchain::ensure` takes the primary's `rust_dir` from `fork_checkout`. - `ensure_shallow_fork`'s pin refusal, `release::bootstrap`'s call to it and their test go: `toolchain.yml` initialises `rust/` at the pin in the step before, and nothing between moves it. - The virt_el1_smp stall issue goes: wt/toyos-counterstall carries it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...smp-stalled-on-counters-read-under-load.md | 33 ---- src/buildlock.rs | 16 +- src/lib.rs | 12 +- src/release.rs | 1 - src/sysroot.rs | 146 ++++++++++++------ src/toolchain.rs | 14 +- 6 files changed, 117 insertions(+), 105 deletions(-) delete mode 100644 issues/kernel/virt-el1-smp-stalled-on-counters-read-under-load.md diff --git a/issues/kernel/virt-el1-smp-stalled-on-counters-read-under-load.md b/issues/kernel/virt-el1-smp-stalled-on-counters-read-under-load.md deleted file mode 100644 index 3b39ae8b06c..00000000000 --- a/issues/kernel/virt-el1-smp-stalled-on-counters-read-under-load.md +++ /dev/null @@ -1,33 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-04 ---- - -# `virt_el1_smp` stalled waiting for `test_rs_counters_read` to end, under a loaded host - -**What was seen.** `cargo test`, the whole guest suite, dev host, on -`wt/toyos-forkpin` at `72621e9cb`, a branch that changes only the build -system's fork checkout and no guest byte: - -``` -FAIL virt_el1_smp: STALLED: waiting for the job test_rs_counters_read to end — it went quiet - STALL virt_el1_smp (33s) -test result: FAILED. 25 passed, 1 failed, 0 invalidated, 26 total -``` - -The serial the harness printed ends with the guest powering off on its own: -`[kernel 2.408 cpu4] spawn: /system/bin/shutdown`, the supervisor's -`power: the machine stops ... (Shutdown)`, then `[kernel 2.419 cpu0] Shutting -down.` — so the guest did not hang; the job's end never reached the harness -before the machine stopped. The printed serial elides its middle, and -`target/red-run-serial/` kept no capture of this guest, so whether -`counters_read` printed its verdict is not known. In the same run `virt_smp` -(entered at EL2) passed with `counters_read: every cpu answered for itself`. - -Host load at the end of the run, from `uptime`: `17.65 28.91 33.61` on 14 -cores. - -**Exit**: a capture of a red `virt_el1_smp` whole enough to say whether -`counters_read` ended before the shutdown, and the cause it names fixed with -a test that reds without it. diff --git a/src/buildlock.rs b/src/buildlock.rs index 5d36317eb4e..4b72a25aea1 100644 --- a/src/buildlock.rs +++ b/src/buildlock.rs @@ -209,8 +209,8 @@ pub fn compiler_shared(root: &Path, what: &str) -> Guard { } /// The global lock exclusively: what the primary holds, inside its worktree -/// lock held exclusively, while it moves its fork checkout, which every -/// worktree's compiler is built from. +/// lock held exclusively, while it moves its fork checkout, which its +/// [`Scope::Global`] phases build from with the worktree lock put down. pub fn global_exclusive(root: &Path, what: &str) -> Guard { acquire(&git_lock_dir(root), LOCK_EX, what, BUILD) } @@ -636,9 +636,9 @@ pub(crate) mod tests { Elsewhere::hold("buildlock::tests::child_role", &env) } - /// The primary's compiler of `root`, read by a process of its own. - pub(crate) fn compiler_read_elsewhere(root: &Path) -> Elsewhere { - held_elsewhere(root, "read-compiler") + /// A [`Scope::Global`] phase of `root`, as a bootstrap holds it, in a process of its own. + pub(crate) fn global_phase_elsewhere(root: &Path) -> Elsewhere { + held_elsewhere(root, "global-phase") } /// Whether an exclusive acquirer of `root`'s global lock is queued for it. @@ -784,9 +784,9 @@ pub(crate) mod tests { let _using = keyed_using(&root, Keyed::Sysroot, &Key::parse(&std::env::var(KEY).unwrap()).unwrap()); hold_until_released(); } - "read-compiler" => { - let _reading = compiler_shared(&root, "child"); - hold_until_released(); + "global-phase" => { + let mut held = shared(&root, "child"); + held.act_if(Scope::Global, "child global phase", || Some(()), |()| hold_until_released()); } "clean" | "clean-unlocked" => { touch(&root.join("cleaner-ready")); diff --git a/src/lib.rs b/src/lib.rs index 3b94b39f6c8..83d3029567a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -120,18 +120,12 @@ pub fn ensure_submodules(repo_dir: &Path) { /// `rust/` at the commit this tree pins and with no history, where a CI /// runner's checkout has none: what reading the fork's sources and building -/// the toolchain from them need. One already there at another commit is -/// refused. Refused in a linked worktree, whose `rust/` is +/// the toolchain from them need. Refused in a linked worktree, whose `rust/` is /// `sysroot::fork_checkout`'s to make: `git submodule` there rewrites the /// `core.worktree` of the primary's fork. pub fn ensure_shallow_fork(root: &Path) -> Result<(), String> { - let fork = root.join("rust"); - if fork.join("x.py").exists() { - let head = sysroot::git_out(&fork, &["rev-parse", "HEAD"]).trim().to_string(); - let pinned = sysroot::pinned_fork(root); - return (head == pinned) - .then_some(()) - .ok_or_else(|| format!("{} is at {head}, and this tree pins the fork at {pinned}", fork.display())); + if root.join("rust/x.py").exists() { + return Ok(()); } if let toolchain::Owner::Elsewhere(_) = toolchain::owner(root) { return Err(format!( diff --git a/src/release.rs b/src/release.rs index 1ff7c20e2a8..2ba36b343ec 100644 --- a/src/release.rs +++ b/src/release.rs @@ -196,7 +196,6 @@ fn outputs(layers: &[Layer]) -> String { /// than its build does, and so is one the build left not whole under its key. pub fn bootstrap(root: &Path) -> Result { let file = step_outputs()?; - crate::ensure_shallow_fork(root)?; let layers = layers(root); let restored: Vec = layers.iter().map(|layer| root.join(&layer.paths[0]).exists()).collect(); whole(&layers, &restored, |layer| defect(root, layer)).map_err(|why| format!("restored, {why}"))?; diff --git a/src/sysroot.rs b/src/sysroot.rs index 5cd9cd43121..8f248eba7f3 100644 --- a/src/sysroot.rs +++ b/src/sysroot.rs @@ -486,7 +486,7 @@ fn key_of(root: &Path, freestanding: &Key, build: &str) -> Key { /// The commit this checkout's tree pins the std fork at: the index's, so a /// staged gitlink counts as the tree's. -pub(crate) fn pinned_fork(root: &Path) -> String { +fn pinned_fork(root: &Path) -> String { let entry = git_out(root, &["ls-files", "-s", "--", "rust"]); let mut words = entry.split_whitespace(); match (words.next(), words.next()) { @@ -498,10 +498,10 @@ pub(crate) fn pinned_fork(root: &Path) -> String { /// The fork checkout `root`'s std is built in, at the commit `root`'s tree pins. /// /// The primary's is its own `rust/`, used where it is not initialised yet, which -/// whoever initialises it does at the pin. It is no workspace, and every -/// worktree's compiler and LLVM are built from it, so one whose `HEAD` is not the -/// pin is moved there, under the global lock held exclusively; refused by name -/// if it has local changes, or does not hold the pinned commit. +/// whoever initialises it does at the pin. It is no workspace, so one whose +/// `HEAD` is not the pin is moved there, under the global lock held exclusively, +/// which the primary's toolchain phases hold while they build from it; refused +/// by name if it has local changes, or does not hold the pinned commit. /// /// A linked worktree's `rust/` starts as the empty stub `git worktree add` /// leaves; it is made here, the first time it is needed, as a git worktree of @@ -513,6 +513,10 @@ pub(crate) fn pinned_fork(root: &Path) -> String { /// repository first if it does not hold it, and refused by name if it has local /// changes rather than moved out from under whoever made them. /// +/// Every nested submodule checked out in it moves with it to the commit the pin +/// records there, refused by name where it does not hold that commit: git leaves +/// one where it was, and the next move would read it as work. +/// /// Every build in a worktree asks this at once, so the making and the move are /// each decided and done under the worktree's lock held exclusively /// ([`Held::act_if`]): one build does it, and none reads a checkout another is @@ -546,13 +550,8 @@ pub fn fork_checkout(root: &Path, lock: &mut Held) -> PathBuf { panic!("{} pins no library/backtrace: {backtrace:?}", fork.display()) }); let theirs = primary.join("rust/library/backtrace"); - let held = Command::new("git") - .args(["cat-file", "-e", &format!("{commit}^{{commit}}")]) - .current_dir(&theirs) - .status() - .is_ok_and(|s| s.success()); let at = fork.join("library/backtrace"); - if held { + if holds(&theirs, commit) { let _ = fs::remove_dir(&at); git_run(&theirs, &["worktree", "add", "--detach", path_str(&at), commit]); } else { @@ -576,20 +575,18 @@ pub fn fork_checkout(root: &Path, lock: &mut Held) -> PathBuf { }, |head| { let _global = primary.is_none().then(|| buildlock::global_exclusive(root, "move the fork checkout to its pin")); - let dirty = git_out(&fork, &["status", "--porcelain", "--ignore-submodules=none"]); + // A nested gitlink checked out at another commit, and nothing more, is no work: it moves with the checkout. + let status = git_out(&fork, &["status", "--porcelain=v2", "--ignore-submodules=none"]); + let work: Vec<&str> = status.lines().filter(|entry| !entry.starts_with("1 .M SC.. ")).collect(); assert!( - dirty.is_empty(), + work.is_empty(), "{} is at {head} with uncommitted work, and this tree pins the fork at {pinned}: a build \ here would compile a std this tree does not name, and moving the checkout would lose \ - that work.\n{dirty}", + that work.\n{}", fork.display(), + work.join("\n"), ); - let held = Command::new("git") - .args(["cat-file", "-e", &format!("{pinned}^{{commit}}")]) - .current_dir(&fork) - .status() - .is_ok_and(|s| s.success()); - if !held { + if !holds(&fork, &pinned) { match &primary { Some(primary) => git_run(&fork, &["fetch", path_str(&primary.join("rust")), &pinned]), None => panic!( @@ -600,13 +597,41 @@ pub fn fork_checkout(root: &Path, lock: &mut Held) -> PathBuf { ), } } + let nested: Vec<(PathBuf, String)> = git_out(&fork, &["ls-tree", "-r", &pinned]) + .lines() + .filter_map(|entry| { + let (meta, path) = entry.split_once('\t')?; + match meta.split_whitespace().collect::>().as_slice() { + ["160000", "commit", commit] => Some((fork.join(path), commit.to_string())), + _ => None, + } + }) + .filter(|(at, commit)| at.join(".git").exists() && git_out(at, &["rev-parse", "HEAD"]).trim() != commit) + .collect(); + for (at, commit) in &nested { + assert!( + holds(at, commit), + "{} does not hold {commit}, which the fork's {pinned} records there: \ + `git -C {} fetch origin {commit}` fetches it", + at.display(), + at.display(), + ); + } git_run(&fork, &["checkout", "--detach", "-q", &pinned]); + for (at, commit) in &nested { + git_run(at, &["checkout", "--detach", "-q", commit]); + } eprintln!("{} was at {head}, and this tree pins {pinned}: checked it out", fork.display()); }, ); fork } +/// Whether the repository at `dir` holds `commit`. +fn holds(dir: &Path, commit: &str) -> bool { + git_try(dir, &["cat-file", "-e", &format!("{commit}^{{commit}}")]).is_ok() +} + /// What a sysroot's [`SOURCES`] says: its key, the fork checkout its std was /// built in, and the witness of the sources it was built from. fn sources_text(key: &Key, fork: &Path, witness: &str) -> String { @@ -1412,7 +1437,8 @@ mod tests { } /// A primary with the fork as its `rust` submodule at `C1`, the fork's `C2` - /// one library change later, and a linked worktree whose tree pins `C2`. + /// one library change and one `library/backtrace` commit later, and a linked + /// worktree whose tree pins `C2`. fn two_pins(base: &Path) -> (PathBuf, PathBuf, String, String) { let bt = base.join("backtrace-src"); fs::create_dir_all(&bt).unwrap(); @@ -1420,6 +1446,9 @@ mod tests { write(&bt.join("lib.rs"), "pub fn trace() {}\n"); git(&bt, &["add", "-A"]); git(&bt, &["commit", "-qm", "backtrace"]); + let b1 = git(&bt, &["rev-parse", "HEAD"]); + write(&bt.join("lib.rs"), "pub fn trace_more() {}\n"); + git(&bt, &["commit", "-qam", "backtrace, later"]); let fork = base.join("fork-src"); fs::create_dir_all(&fork).unwrap(); @@ -1428,10 +1457,12 @@ mod tests { write(&fork.join("compiler/lib.rs"), "\n"); write(&fork.join("x.py"), "\n"); git(&fork, &["submodule", "add", "-q", bt.to_str().unwrap(), "library/backtrace"]); + git(&fork.join("library/backtrace"), &["checkout", "-q", &b1]); git(&fork, &["add", "-A"]); git(&fork, &["commit", "-qm", "C1"]); let c1 = git(&fork, &["rev-parse", "HEAD"]); write(&fork.join("library/std/src/lib.rs"), "pub fn b() {}\n"); + git(&fork.join("library/backtrace"), &["checkout", "-q", "-"]); git(&fork, &["commit", "-qam", "C2"]); let c2 = git(&fork, &["rev-parse", "HEAD"]); @@ -1490,6 +1521,16 @@ mod tests { git(&fork, &["checkout", "-q", &c1]); assert_eq!(fork_checkout(&linked, &mut lock), fork); assert_eq!(git(&fork, &["rev-parse", "HEAD"]), c2, "a checkout behind its pin was not moved to it"); + let behind = git(&fork, &["rev-parse", &format!("{c1}:library/backtrace")]); + git(&fork.join("library/backtrace"), &["checkout", "-q", "--detach", &behind]); + git(&fork, &["checkout", "-q", &c1]); + assert_eq!(fork_checkout(&linked, &mut lock), fork); + assert_eq!( + git(&fork.join("library/backtrace"), &["rev-parse", "HEAD"]), + git(&fork, &["rev-parse", "HEAD:library/backtrace"]), + "a nested submodule was left behind its pin" + ); + assert_eq!(git(&primary.join("rust"), &["status", "--porcelain"]), before, "the primary's nested submodule moved"); // One with local changes is never moved out from under whoever made them. git(&fork, &["checkout", "-q", &c1]); @@ -1544,30 +1585,44 @@ mod tests { } /// **The primary's fork checkout is the commit its tree pins**: one behind - /// the pin, as a `git pull` leaves it, and one ahead of it are moved there; - /// one with local changes, or pinned at a commit it does not hold, is - /// refused by name and not moved; one not initialised yet is left to - /// whoever initialises it, and git is run in no other repository. + /// the pin, as a `git pull` leaves it, and one ahead of it are moved there, + /// each nested submodule checked out in it with it; one whose nested + /// gitlink alone is moved is no work and moves too; one with local changes, + /// or pinned at a commit it or a nested submodule does not hold, is refused + /// by name and not moved; one not initialised yet is left to whoever + /// initialises it, and git is run in no other repository. #[test] fn the_primary_s_fork_checkout_is_moved_to_its_pin() { let base = TempDir::new("fork-primary"); let (primary, _linked, c1, c2) = two_pins(&base); let fork = primary.join("rust"); + let nested = fork.join("library/backtrace"); let pin = |commit: &str| { git(&primary, &["update-index", "--cacheinfo", &format!("160000,{commit},rust")]); git(&primary, &["commit", "-qm", "a pin"]); }; let head = || git(&fork, &["rev-parse", "HEAD"]); + let recorded = |commit: &str| git(&fork, &["rev-parse", &format!("{commit}:library/backtrace")]); + let nested_head = || git(&nested, &["rev-parse", "HEAD"]); let mut lock = buildlock::shared(&primary, "a build"); pin(&c2); assert_eq!(fork_checkout(&primary, &mut lock), fork); assert_eq!(head(), c2, "a checkout behind its pin was not moved to it"); + assert_eq!(nested_head(), recorded(&c2), "a nested submodule was left behind its pin"); pin(&c1); assert_eq!(fork_checkout(&primary, &mut lock), fork); assert_eq!(head(), c1, "a checkout ahead of its pin was kept"); + assert_eq!(nested_head(), recorded(&c1), "a nested submodule was left ahead of its pin"); + git(&nested, &["checkout", "-q", "--detach", &recorded(&c2)]); + pin(&c2); + assert_eq!(fork_checkout(&primary, &mut lock), fork); + assert_eq!((head(), nested_head()), (c2.clone(), recorded(&c2)), "a moved nested gitlink was read as work"); + + pin(&c1); + assert_eq!(fork_checkout(&primary, &mut lock), fork); pin(&c2); write(&fork.join("library/std/src/lib.rs"), "pub fn uncommitted() {}\n"); let said = refusal(|| drop(fork_checkout(&primary, &mut lock))); @@ -1584,6 +1639,19 @@ mod tests { assert!(said.contains(&format!("`git -C {} fetch origin {c3}` fetches it", fork.display())), "{said}"); assert_eq!(head(), c1, "a checkout was moved to a commit it does not hold"); + let bt = base.join("backtrace-src"); + write(&bt.join("lib.rs"), "pub fn trace_most() {}\n"); + git(&bt, &["commit", "-qam", "backtrace, latest"]); + let b3 = git(&bt, &["rev-parse", "HEAD"]); + git(&upstream.join("library/backtrace"), &["pull", "-q"]); + git(&upstream, &["commit", "-qam", "C4"]); + let c4 = git(&upstream, &["rev-parse", "HEAD"]); + git(&fork, &["-c", "fetch.recurseSubmodules=false", "fetch", "-q", path_str(&upstream), &c4]); + pin(&c4); + let said = refusal(|| drop(fork_checkout(&primary, &mut lock))); + assert!(said.contains(&format!("`git -C {} fetch origin {b3}` fetches it", nested.display())), "{said}"); + assert_eq!((head(), nested_head()), (c1.clone(), recorded(&c1)), "a move half-made while a nested commit was missing"); + fs::remove_dir_all(&fork).unwrap(); fs::create_dir(&fork).unwrap(); let superproject = git(&primary, &["rev-parse", "HEAD"]); @@ -1591,18 +1659,18 @@ mod tests { assert_eq!(git(&primary, &["rev-parse", "HEAD"]), superproject, "git ran in the superproject"); } - /// **The primary's fork checkout moves only while nobody reads its - /// compiler**: a sysroot being made in another worktree holds the global - /// lock shared, and the move queues for it and moves nothing until it is - /// let go. + /// **The primary's fork checkout moves only while no toolchain phase builds + /// from it**: another primary process's bootstrap holds the global lock + /// exclusively with the worktree lock put down, and the move queues for it + /// and moves nothing until it is let go. #[test] - fn the_primary_s_fork_checkout_moves_only_while_nobody_reads_its_compiler() { + fn the_primary_s_fork_checkout_moves_only_while_no_toolchain_phase_builds_from_it() { let base = TempDir::new("fork-primary-global"); let (primary, _linked, c1, c2) = two_pins(&base); let fork = primary.join("rust"); git(&primary, &["update-index", "--cacheinfo", &format!("160000,{c2},rust")]); git(&primary, &["commit", "-qm", "pins C2"]); - let reader = buildlock::tests::compiler_read_elsewhere(&primary); + let bootstrap = buildlock::tests::global_phase_elsewhere(&primary); std::thread::scope(|scope| { let mover = scope.spawn(|| drop(fork_checkout(&primary, &mut buildlock::shared(&primary, "a build")))); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20); @@ -1611,25 +1679,13 @@ mod tests { assert!(std::time::Instant::now() < deadline, "the move never queued for the global lock"); std::thread::sleep(std::time::Duration::from_millis(5)); } - assert_eq!(git(&fork, &["rev-parse", "HEAD"]), c1, "the checkout moved while a sysroot build read its compiler"); - reader.release(); + assert_eq!(git(&fork, &["rev-parse", "HEAD"]), c1, "the checkout moved under a running bootstrap"); + bootstrap.release(); mover.join().unwrap(); }); assert_eq!(git(&fork, &["rev-parse", "HEAD"]), c2); } - /// **A shallow fork is the commit its tree pins**: one already there is - /// accepted at the pin and refused, naming both commits, anywhere else. - #[test] - fn ensure_shallow_fork_refuses_a_fork_off_its_pin() { - let base = TempDir::new("fork-shallow-pin"); - let (primary, _linked, c1, c2) = two_pins(&base); - assert_eq!(crate::ensure_shallow_fork(&primary), Ok(())); - git(&primary.join("rust"), &["checkout", "-q", &c2]); - let refused = crate::ensure_shallow_fork(&primary); - assert!(refused.as_ref().is_err_and(|why| why.contains(&format!("is at {c2}")) && why.contains(&c1)), "{refused:?}"); - } - /// **The primary's bootstrap decides from the `compiler/` its tree pins**: /// a fork checkout left at a commit naming another `compiler/` reads as a /// stale compiler, and once it is moved to the pin the compiler the primary diff --git a/src/toolchain.rs b/src/toolchain.rs index 736ad0f7d5d..d5395d9f27a 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -532,14 +532,12 @@ fn runs(stage2: &Path) -> bool { /// inside it, and the loser dies compiling `core` with `couldn't create a temp /// dir: No such file or directory`. pub fn ensure(root: &Path, lock: &mut buildlock::Held, hosted_rustc: bool) -> Sysroot { - let rust_dir = rust_dir(root); let stamps_dir = root.join("target/stamps"); fs::create_dir_all(&stamps_dir).ok(); - let owner = owner(root); - - match owner { + let rust_dir = match owner(root) { Owner::Elsewhere(primary) => { + let rust_dir = primary.join("rust"); assert!( stage2(&rust_dir).join("bin/rustc").exists(), "there is no compiler to build with: {} does not exist.\n\ @@ -550,6 +548,7 @@ pub fn ensure(root: &Path, lock: &mut buildlock::Held, hosted_rustc: bool) -> Sy return sysroot::ensure(root, &rust_dir, lock); } Owner::Installed => { + let rust_dir = root.join("rust"); check_installed_toolchain(root, &rust_dir); let release = manifest_path(&rust_dir); let release = fs::read_to_string(&release).unwrap_or_else(|e| { @@ -557,11 +556,8 @@ pub fn ensure(root: &Path, lock: &mut buildlock::Held, hosted_rustc: bool) -> Sy }); return Sysroot::installed(stage2(&rust_dir), &release); } - Owner::Us => {} - } - - // The bootstrap decides from the `compiler/` this tree pins. - sysroot::fork_checkout(root, lock); + Owner::Us => sysroot::fork_checkout(root, lock), + }; let hosted_stamp = stamps_dir.join("hosted-rustc.stamp"); lock.act_if( Scope::Global, From a9cf973d6dafbf0480ad0efb5ad55f8eab996404 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 12:08:49 +0200 Subject: [PATCH 5/8] The fork checkout's own HEAD moves last, after its nested submodules A move killed between the nested checkouts and the top-level one then leaves `HEAD` off the pin, so the next build asks for the move again and finishes it. Moved the other way round, a killed move left nested submodules behind a `HEAD` already at the pin, which no build re-checks. In a linked worktree, x.py's own sync of those submodules would then run `git submodule update` there, which rewrites the primary's nested `core.worktree`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- src/sysroot.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sysroot.rs b/src/sysroot.rs index 8f248eba7f3..a0df0ada755 100644 --- a/src/sysroot.rs +++ b/src/sysroot.rs @@ -617,10 +617,11 @@ pub fn fork_checkout(root: &Path, lock: &mut Held) -> PathBuf { at.display(), ); } - git_run(&fork, &["checkout", "--detach", "-q", &pinned]); for (at, commit) in &nested { git_run(at, &["checkout", "--detach", "-q", commit]); } + // Last, so a move killed before it is asked for again: `HEAD` alone decides. + git_run(&fork, &["checkout", "--detach", "-q", &pinned]); eprintln!("{} was at {head}, and this tree pins {pinned}: checked it out", fork.display()); }, ); From 4546e0cd77c7c63df594e40d7912b209d9a24495 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 12:52:27 +0200 Subject: [PATCH 6/8] Answer review round 2: a nested commit nothing records refuses the move A nested submodule's gitlink at another commit was let through as no work whatever that commit was, so a commit an agent made inside linked/rust/library/backtrace, not yet recorded by a gitlink, was detached by the next move and left reachable only from that submodule's reflog. An `SC..` entry now goes through only where the nested HEAD is the commit the pin records there or an ancestor of it (`git merge-base --is-ancestor`); any other is refused, naming both. The check moves after the pin's and the nested commits' held checks, since it reads the commit the pin records. The doc's "the next move would read it as work" goes: the filter made it false. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- src/sysroot.rs | 62 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 15 deletions(-) diff --git a/src/sysroot.rs b/src/sysroot.rs index a0df0ada755..247dfea17b0 100644 --- a/src/sysroot.rs +++ b/src/sysroot.rs @@ -514,8 +514,8 @@ fn pinned_fork(root: &Path) -> String { /// changes rather than moved out from under whoever made them. /// /// Every nested submodule checked out in it moves with it to the commit the pin -/// records there, refused by name where it does not hold that commit: git leaves -/// one where it was, and the next move would read it as work. +/// records there, refused by name where it does not hold that commit, or is at +/// a commit neither that one nor behind it, which only its own `HEAD` records. /// /// Every build in a worktree asks this at once, so the making and the move are /// each decided and done under the worktree's lock held exclusively @@ -575,17 +575,6 @@ pub fn fork_checkout(root: &Path, lock: &mut Held) -> PathBuf { }, |head| { let _global = primary.is_none().then(|| buildlock::global_exclusive(root, "move the fork checkout to its pin")); - // A nested gitlink checked out at another commit, and nothing more, is no work: it moves with the checkout. - let status = git_out(&fork, &["status", "--porcelain=v2", "--ignore-submodules=none"]); - let work: Vec<&str> = status.lines().filter(|entry| !entry.starts_with("1 .M SC.. ")).collect(); - assert!( - work.is_empty(), - "{} is at {head} with uncommitted work, and this tree pins the fork at {pinned}: a build \ - here would compile a std this tree does not name, and moving the checkout would lose \ - that work.\n{}", - fork.display(), - work.join("\n"), - ); if !holds(&fork, &pinned) { match &primary { Some(primary) => git_run(&fork, &["fetch", path_str(&primary.join("rust")), &pinned]), @@ -597,15 +586,19 @@ pub fn fork_checkout(root: &Path, lock: &mut Held) -> PathBuf { ), } } - let nested: Vec<(PathBuf, String)> = git_out(&fork, &["ls-tree", "-r", &pinned]) + let recorded: Vec<(String, String)> = git_out(&fork, &["ls-tree", "-r", &pinned]) .lines() .filter_map(|entry| { let (meta, path) = entry.split_once('\t')?; match meta.split_whitespace().collect::>().as_slice() { - ["160000", "commit", commit] => Some((fork.join(path), commit.to_string())), + ["160000", "commit", commit] => Some((path.to_string(), commit.to_string())), _ => None, } }) + .collect(); + let nested: Vec<(PathBuf, String)> = recorded + .iter() + .map(|(path, commit)| (fork.join(path), commit.clone())) .filter(|(at, commit)| at.join(".git").exists() && git_out(at, &["rev-parse", "HEAD"]).trim() != commit) .collect(); for (at, commit) in &nested { @@ -617,6 +610,32 @@ pub fn fork_checkout(root: &Path, lock: &mut Held) -> PathBuf { at.display(), ); } + // A nested submodule checked out at the commit the pin records there, or behind it, and nothing more, is + // no work: it moves with the checkout. One at any other commit holds a commit nothing else records. + let status = git_out(&fork, &["status", "--porcelain=v2", "--ignore-submodules=none"]); + let work: Vec = status + .lines() + .filter_map(|entry| { + let path = entry.strip_prefix("1 .M SC.. ").and_then(|rest| rest.splitn(6, ' ').nth(5)); + let Some((path, commit)) = path.and_then(|path| recorded.iter().find(|(p, _)| p == path)) else { + return Some(entry.to_string()); + }; + let at = fork.join(path); + let at_head = git_out(&at, &["rev-parse", "HEAD"]).trim().to_string(); + let behind = git_try(&at, &["merge-base", "--is-ancestor", &at_head, commit]).is_ok(); + (!behind).then(|| { + format!("{} is at {at_head}, which is neither {commit}, what {pinned} records there, nor behind it", at.display()) + }) + }) + .collect(); + assert!( + work.is_empty(), + "{} is at {head} with uncommitted work, and this tree pins the fork at {pinned}: a build \ + here would compile a std this tree does not name, and moving the checkout would lose \ + that work.\n{}", + fork.display(), + work.join("\n"), + ); for (at, commit) in &nested { git_run(at, &["checkout", "--detach", "-q", commit]); } @@ -1538,6 +1557,19 @@ mod tests { write(&fork.join("library/std/src/lib.rs"), "pub fn uncommitted() {}\n"); let message = refusal(|| drop(fork_checkout(&linked, &mut lock))); assert!(message.contains(&c1) && message.contains(&c2), "{message}"); + git(&fork, &["checkout", "-q", "--", "."]); + + // Nor one whose nested submodule holds a commit of the agent's that no gitlink records yet. + let nested = fork.join("library/backtrace"); + git(&nested, &["checkout", "-q", "--detach", &behind]); + write(&nested.join("lib.rs"), "pub fn trace_mine() {}\n"); + git(&nested, &["commit", "-qam", "the agent's own, not yet recorded"]); + let mine = git(&nested, &["rev-parse", "HEAD"]); + let recorded = git(&fork, &["rev-parse", &format!("{c2}:library/backtrace")]); + let message = refusal(|| drop(fork_checkout(&linked, &mut lock))); + assert!(message.contains(&mine) && message.contains(&recorded), "{message}"); + assert_eq!(git(&nested, &["rev-parse", "HEAD"]), mine, "a nested commit nothing records was moved off"); + assert_eq!(git(&fork, &["rev-parse", "HEAD"]), c1, "a checkout over a nested commit nothing records was moved"); } /// **Twelve builds starting at once in a worktree make its fork checkout From bb5eeceab0dc57e7b054342564e6d89a7029ef9d Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 13:08:12 +0200 Subject: [PATCH 7/8] Refuse a nested commit on top of the pin's record, in a test Review round 3's BLOCKER: swapping the arguments to `merge-base --is-ancestor` passed every case. The new case commits in the linked checkout's nested submodule on top of what C2 records there, with `rust/` at C1, and asserts the refusal names that commit and the record and that neither HEAD moves. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- src/sysroot.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/sysroot.rs b/src/sysroot.rs index 247dfea17b0..f7a03565b12 100644 --- a/src/sysroot.rs +++ b/src/sysroot.rs @@ -1570,6 +1570,16 @@ mod tests { assert!(message.contains(&mine) && message.contains(&recorded), "{message}"); assert_eq!(git(&nested, &["rev-parse", "HEAD"]), mine, "a nested commit nothing records was moved off"); assert_eq!(git(&fork, &["rev-parse", "HEAD"]), c1, "a checkout over a nested commit nothing records was moved"); + + // Nor when that commit sits on top of what the pin records there. + git(&nested, &["checkout", "-q", "--detach", &recorded]); + write(&nested.join("lib.rs"), "pub fn trace_mine_later() {}\n"); + git(&nested, &["commit", "-qam", "the agent's own, on the pin's record"]); + let mine = git(&nested, &["rev-parse", "HEAD"]); + let message = refusal(|| drop(fork_checkout(&linked, &mut lock))); + assert!(message.contains(&mine) && message.contains(&recorded), "{message}"); + assert_eq!(git(&nested, &["rev-parse", "HEAD"]), mine, "a nested commit on the pin's record was moved off"); + assert_eq!(git(&fork, &["rev-parse", "HEAD"]), c1, "a checkout over a nested commit on the pin's record was moved"); } /// **Twelve builds starting at once in a worktree make its fork checkout From bf7c87b0d36c7ddbb405fa11cec7427f3cb387b4 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 13:09:20 +0200 Subject: [PATCH 8/8] A nested submodule passes the move only at the pin's own record Review round 3's NOTE: the arm that let through a nested HEAD strictly behind the pin's record was reached by no case. The only state the move itself leaves there is a nested submodule at the record, after a move killed before its last checkout; anything else is refused by name, which fails loudly. Equality also drops a `merge-base` call per entry. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- src/sysroot.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/sysroot.rs b/src/sysroot.rs index f7a03565b12..62768c8be5c 100644 --- a/src/sysroot.rs +++ b/src/sysroot.rs @@ -515,7 +515,8 @@ fn pinned_fork(root: &Path) -> String { /// /// Every nested submodule checked out in it moves with it to the commit the pin /// records there, refused by name where it does not hold that commit, or is at -/// a commit neither that one nor behind it, which only its own `HEAD` records. +/// a commit that neither the checkout's `HEAD` nor the pin records there, which +/// only its own `HEAD` may record. /// /// Every build in a worktree asks this at once, so the making and the move are /// each decided and done under the worktree's lock held exclusively @@ -610,8 +611,8 @@ pub fn fork_checkout(root: &Path, lock: &mut Held) -> PathBuf { at.display(), ); } - // A nested submodule checked out at the commit the pin records there, or behind it, and nothing more, is - // no work: it moves with the checkout. One at any other commit holds a commit nothing else records. + // A nested submodule checked out at the commit the pin records there, and nothing more, is no work: a move + // killed before its last step leaves it so. One at any other commit may hold a commit nothing else records. let status = git_out(&fork, &["status", "--porcelain=v2", "--ignore-submodules=none"]); let work: Vec = status .lines() @@ -622,9 +623,8 @@ pub fn fork_checkout(root: &Path, lock: &mut Held) -> PathBuf { }; let at = fork.join(path); let at_head = git_out(&at, &["rev-parse", "HEAD"]).trim().to_string(); - let behind = git_try(&at, &["merge-base", "--is-ancestor", &at_head, commit]).is_ok(); - (!behind).then(|| { - format!("{} is at {at_head}, which is neither {commit}, what {pinned} records there, nor behind it", at.display()) + (at_head != *commit).then(|| { + format!("{} is at {at_head}, not at {commit}, what {pinned} records there", at.display()) }) }) .collect();