diff --git a/issues/build/a-fork-checkout-a-killed-git-worktree-add-left-is-taken-as-made.md b/issues/build/a-fork-checkout-a-killed-git-worktree-add-left-is-taken-as-made.md new file mode 100644 index 00000000000..533165a4474 --- /dev/null +++ b/issues/build/a-fork-checkout-a-killed-git-worktree-add-left-is-taken-as-made.md @@ -0,0 +1,53 @@ +--- +status: open +kind: tooling +opened: 2026-10-03 +--- + +# A fork checkout a killed `git worktree add` left is taken as made + +`sysroot::fork_checkout` (`src/sysroot.rs`) takes a linked worktree's `rust/` as +made where `rust/.git` exists and `HEAD` is the pinned commit or ahead of it. A +`git worktree add` killed while it writes its files leaves both, so every build +after it is handed a checkout with files missing. Git records the state itself: +the worktree stays `locked` with the reason `initializing`, beside a stale +`index.lock`. + +## Measured + +In a fixture whose fork checks its last file, `x.py`, out through a filter that +waits, `git worktree add --detach` was killed with SIGKILL once `rust/.git`, +`HEAD` and the files before `x.py` were written. `fork_checkout` returned what +it left: `rust/.git`, `HEAD` at the pin, no `x.py`, `locked` reading +`initializing`. `ensure_shallow_fork` refuses it by name. The licence gate +reaches that refusal only for a kill before `library/Cargo.toml` is written, +a file this fixture's fork never has: `licence::std_library` calls +`ensure_shallow_fork` only without it, and past it the gate reads what +`library/` holds and runs no `git submodule`. + +In a fixture fork of 30,004 files the same kill left 253 of them, +`library/Cargo.toml` among them. Afterwards `git worktree add` at that path +exits 128 on `already exists`, `git worktree remove --force` exits 128 on the +lock, and `git worktree remove --force --force` exits 255 on +`Directory not empty` and unregisters the worktree. + +## Read, not measured + +Both measurements kill git. The kill `src/buildlock.rs`'s header calls routine +is the builder's alone. Then the kernel frees the worktree's lock at once, and +`git worktree add` goes on writing: it is the builder's child and holds no +descriptor of the lock, `git_run` being `Command::status` and the lock file +opened close-on-exec. A build that starts then finds `rust/.git`, `HEAD` at the +pin and the lock free. When that git ends, the checkout has no +`library/backtrace`, the state +`issues/build/the-fork-checkout-runs-git-submodule-in-a-linked-worktree.md` +records of a `fork_checkout` that stopped before adding it. + +## Owner + +The toolchain item of +`issues/build/the-tooling-is-a-review-prompt-and-three-workflows.md`, whose +stores are published by an atomic rename. + +**Exit**: a build that finds a fork checkout git records as `locked` makes it +again or refuses it by name. diff --git a/issues/build/a-suites-first-run-after-the-fork-pin-moves-races-its-own-checkout.md b/issues/build/a-suites-first-run-after-the-fork-pin-moves-races-its-own-checkout.md deleted file mode 100644 index 19139ad49dd..00000000000 --- a/issues/build/a-suites-first-run-after-the-fork-pin-moves-races-its-own-checkout.md +++ /dev/null @@ -1,40 +0,0 @@ ---- -status: open -kind: tooling -opened: 2026-10-03 ---- - -# A suite's first run after the fork pin moves races its own checkout - -`sysroot::fork_checkout` (`src/sysroot.rs`) moves a linked worktree's `rust/` -to the commit the tree pins when the checkout is behind it, with -`git checkout --detach`. The guest suite boots its tests on twelve threads and -each reaches `fork_checkout` on its own, so the first run after a merge that -moved the gitlink runs that checkout in every thread at once. The same unlocked -function reds a new worktree's first run while `git worktree add` is still -writing: -`issues/build/a-worktrees-first-wide-run-reds-on-the-fork-checkout-it-is-still-making.md`. - -## Measured - -A worktree at `bf28c1e38` whose `rust/` stood at `c4c65e3e8` under a pin of -`95960d6c2`, after a merge of `origin/main`: `cargo test --test toyos-build` -exited 1 with 18 of 26 red in 177 s. Every red is one of two sentences. One is -`git ["checkout", "--detach", "-q", "95960d6c2…"] in …/rust failed`, under -git's `Unable to create '…/modules/rust/worktrees/rust1/index.lock': File -exists`. The other is `…/rust is at c4c65e3e8… with uncommitted work`, from a -thread that read the checkout's status while another thread was moving it. The -checkout ended at the pin, and the next run of the same command did not repeat -either. - -## Owner - -The toolchain stage of -`issues/build/the-tooling-is-a-review-prompt-and-three-workflows.md`. - -## What would close it - -One mover: the checkout is moved under a lock every thread of the process -takes, or once before the suite starts its workers, and -`cargo test --test toyos-build` on a worktree whose checkout is behind its pin -moves it and boots its tests. diff --git a/issues/build/a-worktrees-first-wide-run-reds-on-the-fork-checkout-it-is-still-making.md b/issues/build/a-worktrees-first-wide-run-reds-on-the-fork-checkout-it-is-still-making.md deleted file mode 100644 index b3a0dac0624..00000000000 --- a/issues/build/a-worktrees-first-wide-run-reds-on-the-fork-checkout-it-is-still-making.md +++ /dev/null @@ -1,24 +0,0 @@ ---- -status: open -kind: tooling -opened: 2026-10-02 ---- - -# A worktree's first wide run reds on the fork checkout it is still making - -`sysroot::fork_checkout` makes a new worktree's `rust/` with `git worktree add` -under no lock a second thread of the same process waits on: it tests -`rust/.git`, which exists from the first moment of the checkout, and every -other worker then takes the checkout as made while git is still writing its -files. - -The first `cargo test --test toyos-build -- screen_` in a new worktree, three -wide, on a host at load average 35 of 14 cores: one worker printed `Making -…/rust a fork checkout`, and within 0.4 s the other two panicked in -`llvm::refuse_uncommitted_bootstrap` (`src/llvm.rs`) on `rust/src/bootstrap -holds changes no commit does`, listing every file of `src/bootstrap` as `D`. -`screen_panic_muted` and `screen_fatal_halt_composited` red with that -sentence, the run exited 1, and the same command passed once the checkout -existed. - -**Exit**: the first guest run of a new worktree is green at any width. diff --git a/src/buildlock.rs b/src/buildlock.rs index cdf7ed00ec0..d4a95e7af6f 100644 --- a/src/buildlock.rs +++ b/src/buildlock.rs @@ -15,8 +15,8 @@ //! - **shared** — "I am building against the state as it stands". Any number //! at once. //! - **exclusive** — "I am replacing it": the rust bootstrap, this worktree's -//! std build, the `cargo clean`s. One at a time, and never while a build -//! holds the shared mode. +//! std build, the `cargo clean`s, the making or moving of its fork checkout. +//! One at a time, and never while a build holds the shared mode. //! //! And two [`Scope`]s: a crate target directory is shared by the builds in one //! worktree, while the primary's `rust/build` — the compiler every sysroot is @@ -94,8 +94,9 @@ pub enum Scope { /// State every worktree shares: the primary's `rust/` build tree — the /// compiler every sysroot is made with — and the machine-global rustup link. Global, - /// State this worktree alone owns — its crate target directories. Two - /// worktrees cleaning their own have nothing to say to each other. + /// State this worktree alone owns — its crate target directories and its + /// fork checkout. Two worktrees cleaning their own have nothing to say to + /// each other. Worktree, } diff --git a/src/lib.rs b/src/lib.rs index a5ef9ce72e3..83d3029567a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -120,11 +120,19 @@ 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. +/// 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> { if root.join("rust/x.py").exists() { return Ok(()); } + if let toolchain::Owner::Elsewhere(_) = toolchain::owner(root) { + return Err(format!( + "{} is a linked worktree's fork checkout and is not whole: no submodule is initialised there", + root.join("rust").display() + )); + } let status = Command::new("git") .args(["submodule", "update", "--init", "--depth", "1", "rust"]) .current_dir(root) diff --git a/src/licence.rs b/src/licence.rs index baf8c6ef70a..cc0b165562c 100644 --- a/src/licence.rs +++ b/src/licence.rs @@ -1174,7 +1174,8 @@ fn metadata( /// 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); + let mut lock = crate::buildlock::shared(root, "the licences of what ships"); + let fork = crate::sysroot::fork_checkout(root, &mut lock); if !fork.join("library/Cargo.toml").exists() { crate::ensure_shallow_fork(root)?; } diff --git a/src/sysroot.rs b/src/sysroot.rs index ca973dbfe65..b1e608fc4e3 100644 --- a/src/sysroot.rs +++ b/src/sysroot.rs @@ -51,7 +51,7 @@ use std::process::Command; use sha2::{Digest, Sha256}; use crate::arch::Arch; -use crate::buildlock::{self, Guard, Held, Keyed}; +use crate::buildlock::{self, Guard, Held, Keyed, Scope}; use crate::compiler::{self, Compiler}; use crate::identity; use crate::keystore::{self, Key}; @@ -509,7 +509,12 @@ fn pinned_fork(root: &Path) -> String { /// 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. -pub fn fork_checkout(root: &Path) -> PathBuf { +/// +/// 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 +/// still writing. +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, @@ -517,63 +522,72 @@ pub fn fork_checkout(root: &Path) -> PathBuf { Owner::Elsewhere(primary) => primary, }; let pinned = pinned_fork(root); - if !fork.join(".git").exists() { - 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"]); - } - return fork; - } - let head = git_out(&fork, &["rev-parse", "HEAD"]); - let head = head.trim(); - let ahead = Command::new("git") - .args(["merge-base", "--is-ancestor", &pinned, head]) - .current_dir(&fork) - .status() - .is_ok_and(|s| s.success()); - if head == pinned || ahead { - return fork; - } - 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}", - fork.display(), + 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()); + (head != pinned && !ahead).then_some(head) + }, + |head| { + 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}", + fork.display(), + ); + let held = Command::new("git") + .args(["cat-file", "-e", &format!("{pinned}^{{commit}}")]) + .current_dir(&fork) + .status() + .is_ok_and(|s| s.success()); + if !held { + git_run(&fork, &["fetch", path_str(&primary.join("rust")), &pinned]); + } + git_run(&fork, &["checkout", "--detach", "-q", &pinned]); + eprintln!("{} was at {head}, behind this tree's pin {pinned}: checked it out", fork.display()); + }, ); - let held = Command::new("git") - .args(["cat-file", "-e", &format!("{pinned}^{{commit}}")]) - .current_dir(&fork) - .status() - .is_ok_and(|s| s.success()); - if !held { - git_run(&fork, &["fetch", path_str(&primary.join("rust")), &pinned]); - } - git_run(&fork, &["checkout", "--detach", "-q", &pinned]); - eprintln!("{} was at {head}, behind this tree's pin {pinned}: checked it out", fork.display()); fork } @@ -618,7 +632,7 @@ fn held(root: &Path, key: &Key, dir: &Path, make: impl FnMut()) -> Guard { /// The sysroot this worktree's sources name, made if nobody has made it, and /// held in use for as long as the returned value lives. pub fn ensure(root: &Path, rust_dir: &Path, lock: &mut Held) -> Sysroot { - let fork = fork_checkout(root); + let fork = fork_checkout(root, lock); let compiler = compiler::resolve(root, rust_dir, &fork, lock); let keys = Keys::of(root, &compiler, &fork); let dir = sysroots_dir(rust_dir).join(&keys.sysroot); @@ -1409,6 +1423,7 @@ mod tests { fs::create_dir_all(&primary).unwrap(); git(&primary, &["init", "-q"]); write(&primary.join("toyos-abi/src/lib.rs"), "pub struct A;\n"); + write(&primary.join(".gitignore"), ".build-locks/\n"); git(&primary, &["submodule", "add", "-q", fork.to_str().unwrap(), "rust"]); git(&primary.join("rust"), &["checkout", "-q", &c1]); git(&primary, &["add", "-A"]); @@ -1431,8 +1446,9 @@ mod tests { let base = TempDir::new("fork-pins"); let (primary, linked, c1, c2) = two_pins(&base); let before = git(&primary.join("rust"), &["status", "--porcelain"]); + let mut lock = buildlock::shared(&linked, "a build"); - let fork = fork_checkout(&linked); + let fork = fork_checkout(&linked, &mut lock); assert_eq!(fork, linked.join("rust")); assert_eq!(git(&fork, &["rev-parse", "HEAD"]), c2); @@ -1450,22 +1466,67 @@ mod tests { // Work on the fork in the worktree's own checkout is what it builds. write(&fork.join("library/std/src/lib.rs"), "pub fn c() {}\n"); git(&fork, &["commit", "-qam", "C3, the agent's own"]); - assert_eq!(fork_checkout(&linked), fork); + let c3 = git(&fork, &["rev-parse", "HEAD"]); + assert_eq!(fork_checkout(&linked, &mut lock), fork); + assert_eq!(git(&fork, &["rev-parse", "HEAD"]), c3, "a checkout ahead of its pin was moved"); // A clean checkout behind what the tree pins is moved to the pin itself. git(&fork, &["checkout", "-q", &c1]); - assert_eq!(fork_checkout(&linked), fork); + 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"); // One with local changes is never moved out from under whoever made them. git(&fork, &["checkout", "-q", &c1]); write(&fork.join("library/std/src/lib.rs"), "pub fn uncommitted() {}\n"); - let refused = std::panic::catch_unwind(|| fork_checkout(&linked)) - .expect_err("a fork checkout with local changes was moved out from under them"); - let message = refused.downcast::().expect("a formatted refusal"); + let message = refusal(|| drop(fork_checkout(&linked, &mut lock))); assert!(message.contains(&c1) && message.contains(&c2), "{message}"); } + /// **Twelve builds starting at once in a worktree make its fork checkout + /// once, and move one behind its pin once**: each holds the worktree's lock + /// as a process of its own does, and each returns the checkout whole and at + /// the pin. + #[test] + fn twelve_builds_at_once_make_and_move_one_fork_checkout() { + let base = TempDir::new("fork-builds"); + let (_primary, linked, c1, c2) = two_pins(&base); + let fork = linked.join("rust"); + let twelve_build = || { + let start = std::sync::Barrier::new(12); + std::thread::scope(|builds| { + for _ in 0..12 { + builds.spawn(|| { + let mut lock = buildlock::shared(&linked, "a build"); + start.wait(); + assert_eq!(fork_checkout(&linked, &mut lock), fork); + assert_eq!(git(&fork, &["rev-parse", "HEAD"]), c2); + assert!(fork.join("library/backtrace/lib.rs").is_file(), "a build read a checkout still being made"); + }); + } + }); + }; + twelve_build(); + git(&fork, &["checkout", "-q", &c1]); + twelve_build(); + } + + /// Where a linked worktree's `rust/` is a fork checkout that is not whole, + /// [`crate::ensure_shallow_fork`] refuses, and git in the primary's fork + /// still runs. + #[test] + fn ensure_shallow_fork_initialises_no_submodule_in_a_linked_worktree() { + let base = TempDir::new("fork-shallow"); + let (primary, linked, c1, c2) = two_pins(&base); + let fork = linked.join("rust"); + fs::remove_dir(&fork).unwrap(); + git(&primary.join("rust"), &["worktree", "add", "-q", "--detach", "--no-checkout", path_str(&fork), &c2]); + + let refused = crate::ensure_shallow_fork(&linked); + + assert_eq!(git(&primary.join("rust"), &["rev-parse", "HEAD"]), c1); + assert!(refused.as_ref().is_err_and(|why| why.contains("linked worktree")), "{refused:?}"); + } + /// 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 {