Repository navigation
A worktree's fork checkout is made and moved under its build lock, and ensure_shallow_fork initialises no submodule in a linked worktree - #683
Merged
Conversation
`sysroot::fork_checkout` made a linked worktree's `rust/` and moved one behind its pin, and every build called it: the guest suite's workers reach `toolchain::ensure` on their own threads, so the first run of a new worktree, and the first after a merge moved the gitlink, ran `git worktree add` or `git checkout --detach` on one checkout from every worker at once. Measured at ec7de74 with `cargo test --test toyos-build -- screen_`, three workers, on 14 cores: - a new worktree's first run, at load average 74, exited 1 with 2 of 3 red: two workers each ran `git worktree add`, one died on `fatal: '…/rust' already exists`, and the third read the checkout the winner was still writing and died in `llvm::refuse_uncommitted_bootstrap` on every file of `src/bootstrap` as `D`; - with the checkout one commit behind its pin, at load average 23, it exited 1 with 2 of 3 red on `Unable to create '…/worktrees/rust2/index.lock': File exists`. The move is now `sysroot::make_fork_checkout`, which a process calls once where it starts, before it has a second thread: `cargo run` beside the primary's `ensure_submodules`, the harness before its first build, and the licence gate. `fork_checkout` is what a build calls, and it moves nothing: a checkout that is not at the pin or ahead of it is refused by name, which is what a build sees when the pin moves under a run. So no worker is a mover and there is nothing to lock. `a_build_reads_the_fork_checkout_and_never_moves_it` starts twelve builds at once against a checkout that is not made and against one behind its pin: each is refused and the checkout stands as it was, and all twelve read the one `make_fork_checkout` made and the one it moved. With `fork_checkout` calling `make_fork_checkout` again it exits 101, each of the twelve running `git worktree add`. Closes issues/build/a-suites-first-run-after-the-fork-pin-moves-races-its-own-checkout.md and issues/build/a-worktrees-first-wide-run-reds-on-the-fork-checkout-it-is-still-making.md, two records of this one defect. The `git submodule update --init library/backtrace` arm issues/build/the-fork-checkout-runs-git-submodule-in-a-linked-worktree.md records is the same code under the mover's new name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm
…fork checkout One process is one mover now; two are still two. Measured with the mover called where a process starts (020a97d), from an empty `rust/` stub at load average 36 of 14 cores: two `cargo test --test toyos-build` started together, one on `screen_panic_muted` and one on `screen_fatal_halt_composited`, both printed `Making …/rust a fork checkout`, and they exited 101 and 0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm
Collaborator
Author
|
Mutation: a build moves the checkout again ( --- a/src/sysroot.rs
+++ b/src/sysroot.rs
@@ -513,6 +513,7 @@
/// process builds at once, so a build moves no checkout: one that is not at the
/// commit this tree pins or ahead of it is refused by name.
pub fn fork_checkout(root: &Path) -> PathBuf {
+ make_fork_checkout(root);
let fork = root.join("rust");
match toolchain::owner(root) {
Owner::Us => {}
|
Collaborator
Author
|
Run logs, with the checkouts' parent directory written Base
|
Collaborator
Author
|
Gate logs at
|
Collaborator
Author
|
Review of Net, Evidence at the head: BLOCKER
NOTE
REMOVE
SEND BACK |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm
…t_if each The first round moved the checkout once where a process starts and had every build refuse one off its pin. The review of #683 found that this left two processes as two movers (measured there: exits 101 and 0), let a process arriving inside the `git worktree add` take a checkout still being written, and made a plain `--list` make a 61,561-file checkout, while the tree already held the mechanism for a decide-then-act defect on state a worktree owns: `Held::act_if(Scope::Worktree, …)`, which `build::invalidate_stale` uses, on the `lock` `sysroot::ensure` already receives. So `fork_checkout` is main's again and takes that lock: the making is one `act_if` and the move to the pin another. `flock` is per open file description, so twelve workers' `Held`s exclude each other as two processes do. `licence::std_library` takes the worktree's lock shared for the call, as a build does. The three `make_fork_checkout` call sites, the refusal in `fork_checkout`, the issue file the first round added and its four renames in the submodule issue are gone. The twelve-thread test of readers goes. The test that replaces it starts twelve builds at once on a checkout not made and on one behind its pin; each returns it whole and at the pin. The ahead-of-pin case now asserts the commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm
…s checkout has a record `licence::std_library` sends a fork checkout without `library/Cargo.toml` to `ensure_shallow_fork`, which is a CI runner's path. Under the worktree's lock a checkout another process is still making no longer gets there: the reader waits. One a killed `git worktree add` left still did. Measured in a fixture at a8dcbcc: `fork_checkout` returned the remains, `git submodule--helper update --init --depth=1 -- rust` ran in the linked worktree, and afterwards `git status` in the fixture primary's `rust/` exits 128 on `cannot chdir to '../../../../../../linked/rust'`. `ensure_shallow_fork` now refuses a linked worktree by name, and `a_linked_worktree_initialises_no_submodule` holds it: on what a killed maker leaves, the refusal is returned and git in the primary's fork still runs. That a build takes such remains as made is the base's too and stays true: issues/build/a-fork-checkout-a-killed-git-worktree-add-left-is-taken-as-made.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm
`git worktree add --no-checkout` leaves a checkout with `.git` and no file; a killed one leaves some. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm
Japabu
changed the title
Collaborator
Author
|
Runs in the worktree at Each log is the command's whole output with these dropped: The sequence, and the checkout after each step
|
Collaborator
Author
|
Gate logs at
|
Collaborator
Author
|
The review of The hypothesis held as written. The make and the move are each one BLOCKER
NOTE
REMOVE
|
Collaborator
Author
|
Review of Net, Evidence at the head: Round 1's BLOCKERs
Round 1's NOTE is done ( BLOCKERNone. NOTE
REMOVE
LAND AFTER NAMED CHANGES |
… issue says what the licence gate reaches Round 2's review of cfd4931 on #683, its named changes. `fork_checkout` and `ensure_submodule` still run `git submodule` in a linked worktree's fork checkout, so "no submodule is initialised in a linked worktree" is true of `ensure_shallow_fork` and not of the tree. The test is renamed to `ensure_shallow_fork_initialises_no_submodule_in_a_linked_worktree`, and the clause of its doc that claimed it of the tree is deleted. The new issue said the licence gate reds on what a killed `git worktree add` left. `licence::std_library` reaches `ensure_shallow_fork` only where the checkout has no `library/Cargo.toml`, which the fixture's fork never has and which the 30,004-file run's kill left among its 253 files; past it the gate reads what `library/` holds. The issue now says that. It also recorded one kill of two. The other is the builder's alone, with its `git worktree add` still writing and the worktree's lock freed: the issue names it as read from the code and unmeasured. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm
Collaborator
Author
|
Patches and fixture runs at Every patch was applied to the clean worktree with Mutation: the make and the move decide, then act, under no lock--- a/src/sysroot.rs
+++ b/src/sysroot.rs
@@ -522,9 +522,8 @@
Owner::Elsewhere(primary) => primary,
};
let pinned = pinned_fork(root);
- lock.act_if(
- Scope::Worktree,
- "make the fork checkout",
+ let _ = &lock;
+ unlocked(
|| (!fork.join(".git").exists()).then_some(()),
|()| {
let stub = fs::read_dir(&fork).map_or(0, |d| d.count());
@@ -555,9 +554,7 @@
}
},
);
- lock.act_if(
- Scope::Worktree,
- "move the fork checkout to its pin",
+ unlocked(
|| {
let head = git_out(&fork, &["rev-parse", "HEAD"]).trim().to_string();
let ahead = Command::new("git")
@@ -591,6 +588,13 @@
fork
}
+/// The mutation: decide, then act, under no lock.
+fn unlocked<W>(decide: impl Fn() -> Option<W>, act: impl FnOnce(W)) {
+ if let Some(work) = decide() {
+ act(work);
+ }
+}
+
/// 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 {
Mutation: a checkout ahead of its pin is moved--- a/src/sysroot.rs
+++ b/src/sysroot.rs
@@ -568,7 +568,7 @@
.current_dir(&fork)
.status()
.is_ok_and(|s| s.success());
- (head != pinned && !ahead).then_some(head)
+ (head != pinned || !ahead).then_some(head)
},
|head| {
let dirty = git_out(&fork, &["status", "--porcelain", "--ignore-submodules=none"]);The same command, once: exit 101. Mutation:
|
Japabu
changed the title
Japabu
marked this pull request as ready for review
October 3, 2026 11:49
Japabu
enabled auto-merge
October 3, 2026 11:49
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sysroot::fork_checkoutmakes a linked worktree'srust/, and moves one that is behind its pin, each as oneHeld::act_if(Scope::Worktree, …)on the build locksysroot::ensurealready receives. The decision and the act are one exclusive section, asbuild::invalidate_stale's cleans are.Before, every build decided and acted under the shared lock. The guest suite's workers reach
fork_checkouton their own threads and a second process reaches it whenever it starts, sogit worktree addorgit checkout --detachran on one checkout from several movers at once, and a build arriving inside thegit worktree addtook a checkout git was still writing.Closes
issues/build/a-suites-first-run-after-the-fork-pin-moves-races-its-own-checkout.mdandissues/build/a-worktrees-first-wide-run-reds-on-the-fork-checkout-it-is-still-making.md, two records of this one defect.git grepof both slugs over the tree atc402baf6cfinds nothing.Decisions
flockis per open file description, so twelve workers'Helds exclude each other as two processes do. No call site is added:src/main.rsandtests/toyos.rsaremain's.act_ifs. Making and moving are two decisions, each asked again under the exclusive lock, astoolchain::ensure's steps are.licence::std_libraryholds the worktree's lock shared for the call. It reads the fork'slibrary/after the call without it: nothing moves a checkout that is at its pin.ensure_shallow_forkrefuses a linked worktree. It is a CI runner's path, andstd_librarysent any fork checkout withoutlibrary/Cargo.tomlthere. The lock closes the reader that arrives while the checkout is being made. It does not close the reader that finds what a killed maker left, and what that costs is measured below: git in the primary'srust/stops working.--listmakes and moves nothing, as onmain.What it does not fix
git worktree addleft as made, as onmain:issues/build/a-fork-checkout-a-killed-git-worktree-add-left-is-taken-as-made.md. The licence gate refuses the remains by name only for a kill beforelibrary/Cargo.tomlis written:licence::std_libraryreachesensure_shallow_forkonly without that file, and past it the gate reads whatlibrary/holds and runs nogit submodule. The issue also names the builder's own kill, with itsgit worktree addstill writing and the lock freed, as read from the code and unmeasured.issues/build/the-fork-checkout-runs-git-submodule-in-a-linked-worktree.mdis untouched, and its three arms stand.Checks
Build-system concurrency over git state every worktree shares. The negative control is the whole change reverted onto
cfd4931a6:git diff cfd4931a6 origin/main -- src tests, applied, run and reversed. The oracle is git: it refuses a second writer on its own (already exists,index.lock), it records a half-made worktree itself (locked initializing), and its trace says which commands ran.The head is
c402baf6c. Everything below but the first gate was measured atcfd4931a6, which the head differs from by one host test's name, a clause deleted from that test's doc, and the new issue's record:git diff --stat cfd4931a6 c402baf6cissrc/sysroot.rs+4 −4, inside its#[cfg(test)]module, and the issue +23 −8.In this worktree, at
cfd4931a6rust/beforecfd4931a6cargo test --test toyos-build -- --listrust/still the empty stubcfd4931a6cargo test --test toyos-build -- screen_Making …; the other two workers took the lock 4.6 s later and made nothingcfd4931a6-- --listHEADnot movedcfd4931a6-- screen_checked it outcfd4931a6-- screen_panic_mutedand-- screen_fatal_halt_compositedMaking …between them; the other took the lock 5.2 s latercfd4931a6rust/.gitappeared (1 of 37 top-level entries written)waiting for the build lock (shared, test image) — held by pid … (make the fork checkout)-- screen_Unable to create '…/worktrees/rust/index.lock': File exists-- screen_fatal: '…/rust' already exists; threeMaking …After every row but the two
--listones the checkout is at its pin with an emptygit status, registered once in the primary's fork, andcore.worktreeof the primary's fork and of itslibrary/backtraceis what it was. Each row's log is in the comment headed "Runs in the worktree".In a fixture, never the real fork
A temporary host test (its patch is in the comment headed "Patches and fixture runs", #683 (comment)) builds
sysroot::tests::two_pinsand checks the fork's last file,x.py, out through a filter that waits, so a realgit worktree addstops withrust/.gitandHEADwritten andx.pynot.git submoduleranrust/afterwardscfd4931a6licence::std_library, while the maker is heldheld by pid … (make the fork checkout)), and returned the whole checkout after the releasegit submodule--helper update --init --depth=1 -- rustgit statusexits 128,cannot chdir to '../../../../../../linked/rust'cfd4931a6std_library, on what a maker killed with SIGKILL left… is a linked worktree's fork checkout and is not whole: no submodule is initialised therea8dcbccb2, the lock without the guardOkThe second row is
main's code: by it, a--ci hostwhose licences step lands inside another process'sgit worktree addin the same worktree breaks git in the primary'srust/.Host tests and mutations
sysroot::tests::twelve_builds_at_once_make_and_move_one_fork_checkoutreleases twelve threads, each holding its ownHeld, from oneBarrierintofork_checkout, on a checkout not made and then on one behind its pin. Each returns it at the pin withlibrary/backtracethere. What it sees that reading cannot: git under twelve concurrent callers, and that theHelds of one process's threads exclude each other.sysroot::tests::ensure_shallow_fork_initialises_no_submodule_in_a_linked_worktreecallsensure_shallow_forkin a linked worktree whoserust/is a checkout with no file. It asserts first that git in the primary's fork still runs, then the refusal. What it sees that reading cannot: what git does to the primary when that command runs there. Atcfd4931a6, and in the logs taken there, its name isa_linked_worktree_initialises_no_submodule.a_worktree_pinning_another_fork_commit_gets_its_own_checkoutnow assertsHEADstays at the agent's own commit when the checkout is ahead of its pin.cfd4931a6act_ifs replaced by decide-then-act under no locktwelve_builds_…head != pinned || !ahead)a_worktree_pinning_…a checkout ahead of its pin was movedensure_shallow_forkremoveda_linked_worktree_…, the head'sensure_shallow_fork_initialises_…cannot chdirEach was applied with
git apply --check, built, run and reversed, leavinggit status --porcelainempty. The patches and logs are in the comment headed "Patches and fixture runs", #683 (comment).Gates
c402baf6ccargo run -- --ci hosttest sysroot::tests::ensure_shallow_fork_initialises_no_submodule_in_a_linked_worktree ... okis its line 532cfd4931a6cargo run -- --ci hostcfd4931a6cargo test --test toyos-build, from an empty stub, 12 wideMaking …, eleven workers took the lock 4.6 to 4.7 s latercfd4931a6cargo run -- --build-only, from a checkout one commit behind its pinchecked it outThe log of the first is
~/.claude/jobs/2280e09e/tmp/scratchpad/orch/683-named/ci-host.logon the development machine; the logs atcfd4931a6are in the comment headed "Gate logs". The guest suite and the image build were not run again atc402baf6c: neither compiles a#[cfg(test)]module ofsrc/, and the head changes nothing else undersrc/ortests/.Net
git diff --shortstat origin/main...HEAD: 7 files, +195 −135.src/sysroot.rs+73 −59 above its test module, of which +26 −12 is not indentation;src/lib.rs+9 −1, the guard;src/buildlock.rs+5 −4, its header andScope::Worktree's doc;src/licence.rs+2 −1.src/sysroot.rs.Unsure of
ensure_shallow_fork, a primary checkout with norust/x.py, gained onetoolchain::ownercall and was read, not run, on this machine: no checkout here is in that state. The hostedhostandtoolchainchecks are its measurement. A draft skips them, and they run when the pull request is marked ready and again in its merge group, so a red in that arm stops this landing before it reachesmain.twelve_builds_…because at least two of twelve threads decide before one has acted, which is a race in the mutated code: red in 16 runs of 16 over the round that ended atcfd4931a6.cfd4931a6's at 19 to 21, of 14 cores.🤖 Generated with Claude Code
https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm