fix(hardlink): name the mutable set directly, under both build-dir layouts #16
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: daniel/gitdan-actions#16
Reference in New Issue
Block a user
Read this first: this PR materially reduces what the hardlink clone saves
On a real 5.5 GB Bevy target directory, the share of bytes the clone real-copies instead of sharing:
incremental/— the CI-shaped figure, since CI setsCARGO_INCREMENTAL=0incremental/included (a local dev checkout)The CI number is the one consumers feel, and the whole-tree number understates it. Both rows are now in the README too, not just here. Three repos' CI inherits this at
@v1the moment it merges — and so does a fourth consumer that is not CI at all: dotfiles'scripts/seed-worktree-cache.shhardlink tier, which seeds from a live developer worktree.The cost is unconditional per clone and does not amortise: a fresh
cp -alleaves every file withnlink >= 2, so_unshare_files'-links +1filter cannot skip anything, and a clone happens twice per job — once seeding (seed-target-dir.sh), once publishing (publish-snapshot.sh). On a zemyna-sized tree that is roughly +282 MB copied per clone, +564 MB per job, every run, whether or not the build touches any of it.None of that cost is the layout work. The layout fix alone takes the v2 figure to 0.2%. Every point above that is the second finding below, which is a correctness fix.
Summary
unshare_mutable_paths()no longer selects by container (.fingerprint/build). It names the mutable set directly, so it holds under both of Cargo's build-dir layouts.OUT_DIRis told apart from a compile unit's artifactout/structurally, not by name.hardlink-clone-selftest.shgains two compiler-free layout fixtures that pin the partition in both directions, plus a live scenario that relinks a test binary.Why the two land together
They are separable as tickets and not as a change. Under v2 today, the
-name buildclause matching the entire tree accidentally protects linked outputs: at 99.996% real-copied, nothing is shared, test binaries included. Fixing the collapse on its own would make executables shared under v2 for the first time, on a code path this PR's own measurements show being written in place — shipping a fresh corruption bug in the same diff that cites the evidence for it. The only defensible split is the other ordering (executables first, layout second), and it buys nothing.Part 1 — the layout-v2 selection (#14)
Layout v2 (
-Z build-dir-new-layout, nightly default since cargo 1.99, stabilising in 1.100.0 on 2026-11-12) removes<profile>/.fingerprintand<profile>/depsand regroups everything per build unit under<profile>/build/<pkg>/<hash>/{fingerprint,out,run}/— artifacts included. So-name .fingerprintmatched nothing and-name buildmatched the entire tree. Correctness held by accident; the saving did not.Measured on one scratch crate (serde + serde_json + regex plus a build script), same sources both ways:
The
out/discriminatorUnder v2 a build script's
OUT_DIRand a compile unit's rlib are both a directory calledout, one directory apart, and they need opposite treatment. Verified shapes (2026-08-27, underbuild/probe/):The discriminator is Cargo's own record of the execution: it sits beside the OUT_DIR and nowhere else —
run/root-outputunder v2, a looseroot-outputunder v1. A unit directory holding one is a build-script run unit and itsoutis the OUT_DIR; a unit directory without one is a compile unit and itsoutholds the artifact whose sharing is the entire point. No name matching onbuild_script_*, and no layout detection anywhere: one selection covers both.All four selected directory names (
.fingerprint,fingerprint,run,out) are pruned, so no selected directory can contain another and nothing is unshared twice.Ambiguity resolves toward unsharing
The execution record alone is not a sufficient signal, and keying on it alone was a regression against
main. Cargo writesroot-outputonly after the script exits successfully, so a build script that populatesOUT_DIRand then fails leaves a unit with no record at all — which read as "compile unit" and stayed shared.mainreal-copied all ofbuild/unconditionally and covered that state by construction; this was the one place the new v1 selection was not a superset of the old one. Reproduced on cargo 1.93.1 stable: the clone's build wrote through the shared inode into the source'sgen.txt.An
outdirectory now stays shared only when two independent signals agree it is a compile unit's, and either one missing real-copies it:.rlib/.rmetaof its own — the artifact whose sharing is the point; andOver-unsharing costs bytes; under-unsharing costs corruption. The residual is a build script that writes a file named
*.rlib/*.rmetaintoOUT_DIRand has never once succeeded.That residual deliberately lives in a code comment and not in a ticket. It is stated at the selection it qualifies, where anyone changing the rule reads it; a ticket for a case this narrow would sit unactioned and make the backlog less informative, not more. Flagging it as a decision rather than an omission.
This costs nothing. Requiring signal 1 also real-copies a bin, test or build-script compile unit's
out, but everything in one is an executable or a*.d, both already privately owned by the file rules — the measured figures below are byte-identical before and after this change.Part 2 — linked outputs are written in place (#17)
rustc writes an
.rlib/.rmetato a temporary and renames it into place. A linked executable is written through whatever inode is already at its path. Acargo test --no-runinside a rawcp -alclone therefore rewrote the source's own test binary:debug/deps/bprobe-0ecbea0cebf59df5debug/deps/bprobe-6a091d813b2be60ddebug/deps/bprobe-6a091d813b2be60ddebug/build/bprobe/…/out/bprobe-…Stable included, both layouts, and
deps/is shared today — so this is live now and owes nothing to layout v2. Base's fingerprints stay privately owned and therefore intact, so base's next build reportsFresh, does not relink, and runs the other branch's binary.No materially narrower rule is available
This is not a conservative default chosen for want of evidence. The obvious narrower candidate — executable AND no in-tree hardlink twin, which #17 names — was measured on the real zemyna tree and would unshare exactly the same 283 MB, recovering none of this PR's cost.
The reason is that the twinned executables are the wrong 489 MB. Of the tree's 776 MB of executable bytes, the 489 MB that do have a twin are build-script binaries (
build_script_build-<hash>/build-script-buildpairs), which live underbuild/and were therefore already 100% real-copied by the old selection. Exempting them would not reduce this PR's cost at all; it would reduce the status quo cost, on files about whose safety no evidence exists — the same "ship a fresh exposure" move this PR argues against for the layout-only split.The delta set is 36 files / 283 MB, of which 35 are executables and not one has an in-tree twin: every one is
nlink == 1in the source,nlink == 2aftercp -al, so-links +1matches all of them on every clone.Note what this argument does not rest on. An earlier revision of this body and of #17 claimed
nlinkwas ruled out as a discriminator, citing a lib+bin crate whose two test binaries were bothnlink == 1and appeared to behave differently. Review re-checked it: the "intact" one had not been rebuilt at all — same content, same inode, the unit was never exercised — and forcing both to rebuild rewrote both. That claim asserted more than the measurement showed and is withdrawn, here, in the code comments and in #17. This PR exists because a claim outlived its evidence, so it should not ship one.What was actually observed is narrower: every executable measured intact had an uplift hardlink twin Cargo must re-create anyway; every one measured rewritten had none. Whether the twin is the mechanism or a correlate was not determined — and the rule does not depend on the answer, because the arithmetic above holds either way. Output size growth and the linker in use (mold via the machine's global
~/.cargo/config.toml, and the default linker) were genuinely ruled out.The remaining trigger question is recorded as a named limitation in #17; measuring it further was deliberately out of scope. A rule that recovers real bytes would have to share some untwinned executable — precisely the set every rewrite was measured in — so it needs the mechanism established first, not another filesystem heuristic.
Measurements after
incremental/(CI-shaped)Bevy figures are a read-only scan of
~/.cache/cargo-target/zemyna/zemyna(5.54 GB, layout v1); nothing was built into it.Red proof
New assertions, against the selection on
main(the v2 fixture is the first thing the suite checks now):and with the per-file assertions relaxed so the byte gate is the sole witness:
After the fix:
The failed-build-script hole, against the first cut of this PR (execution record as the sole signal):
The linked-test-binary scenario, against
main'scache-lib.shwith the fixtures neutered so the run reaches it. The earlier cut of this scenario passed green here; it now fails, and the path it names is the executable:The fixtures are mostly synthetic "executables" by construction, which is why their percentages are high; the byte gate is a floor under "still a hardlink clone at all", not a budget.
Test surface
The two fixtures need no compiler, which is the point: every live scenario runs whichever layout the installed Cargo writes, and gitdan-ci's is v1 — so before this PR nothing on any machine could have caught a v2 regression. They also assert the partition in both directions; #14 notes that nothing previously checked sharing beyond
shared > 0.Also added: a live
cargo test --no-runscenario covering a linked test binary. The crate shape is load-bearing — the first cut reused the lib+bin probe crate, whose test binaries relink to a fresh inode, so both halves passed green against the unfixed selection on dep-info mutations the previous scenario already covers. It now builds a bin-only crate with a unit test, reads only executables when deciding whether the hazard fired, and — like the freshness probe above — has three outcomes rather than two: if the toolchain does not exhibit the rewrite, it emits a::warning::and skips rather than passing quietly on a hazard that never fired.Files affected
scripts/cache-lib.sh— new_mutable_dirs();unshare_mutable_paths()gains the v1 run-metadata and linked-output file rules; the selection comment block corrected with dated toolchain anchors.scripts/hardlink-clone-selftest.sh— two layout fixtures, artifact-sharing assertions, the test-binary scenario, a bin target on the probe crate.scripts/publish-snapshot-selftest.sh— one comment clause this change falsified ("Cargo and rustc REPLACE an artifact" is now true of rustc's renamed outputs only).README.md— the cost-model section (now carrying both rows, with the CI-shaped one first, beside the quick-start that setsCARGO_INCREMENTAL: 0), the two-signalout/rule, and the selftest table row.Test plan
Both from committed state, after merging
origin/main. Beyond that, the measurement work above was run by hand across four toolchains in a temp directory — no build touched a shared cache or gitdan-ci, and the Bevy-tree numbers are a read-only scan.Nothing here is user-visible, so there is no visual smoke to run.
Closes #14.
Closes #17.
VERDICT: REQUEST-CHANGES
Two findings block, both narrow and both fixable without touching the trade-off this PR is asking consumers to accept. The trade-off itself I verified and it holds — see "What checks out" below, which is more supportive of the broad rule than the PR body's own argument is.
1. The live "linked TEST binary" scenario is not red-proven, and pins nothing
The PR's Test-surface section claims: "a live
cargo test --no-runscenario (control + fix) covering a linked test binary". It does not cover it. I ran the PR'shardlink-clone-selftest.shagainstmain'scache-lib.shwith the two fixtures neutered:Both halves pass green against the unfixed selection. Not one of the 7 control paths is an executable — the control's
[ -n "$exe_ctl_mutated" ]is satisfied entirely by dep-info and fingerprint mutations that the previous control scenario already covers, and the fix half then passes because on this Cargo no executable was written through its inode at all.The reason is in this PR's own ticket: the probe crate is lib+bin, and #17 lists a lib+bin crate's test binaries as one of the shapes that relinked to a fresh inode. The scenario was built on a shape the ticket says does not exhibit the hazard.
A bin-only crate with a unit test does exhibit it, on the same stable toolchain — I reproduced it independently (below). Adding that shape, or switching the scenario's crate to it, makes the ~50 lines actually pin what they name. As it stands this is
gitdan#66's lesson again: a fixture that cannot fail is not coverage.The two compiler-free layout fixtures are fine — I confirmed both are genuinely red against
mainand assert the partition in both directions:and the v2 fixture fails on
mainwith exactly thelibprobe-….rlib was real-copiedassertion the PR body quotes.2. The
out/discriminator has a hole: a build script that has never succeeded_mutable_dirs()classifies<unit>/outas an OUT_DIR only when<unit>/run/or<unit>/root-outputexists. Cargo writesroot-outputafter the script exits successfully. A build script that writes intoOUT_DIRand then fails leaves the unit without/populated and noroot-output— so the OUT_DIR is classified as a compile unit's artifactoutand left shared. Reproduced end to end on cargo 1.93.1 stable (layout v1):This is a regression against
main, which real-copied all ofbuild/unconditionally and so covered this state by construction. It is the one place the new v1 selection is not a superset of the old (everywhere else it is — measured, below).Two things bound it, which is why this is REQUEST-CHANGES and not BLOCK:
root-outputsurvives a later failure — verified: green build, then force the script to panic,root-outputstill present, discriminator still says RUN-unit.OUT_DIRitself (SOURCE2in my run). So it is transient corruption, not the silent-wrong-artifact-reuse class that #17 is about.Reachability is further limited by
cargo-cache-publish'spublishmode being documented as the post-green call. Buthardlink_clone_intohas a fourth consumer that is not CI at all — dotfiles'scripts/seed-worktree-cache.shhardlink tier, seeding from a live developer worktree, which is exactly where a failed build script sits around. Worth closing rather than reasoning about.3. A comment asserts a mechanism this PR's own evidence denies
cache-lib.sh, the linked-outputs block:#17 explicitly records hardlink count at link time as ruled out by measurement ("2 in both a rewritten and an intact case"). My own runs agree that it is not predictive. Delete the
whenever …clause — the sentence is correct without it, and this repo shipped #15 today about precisely this failure mode.4. README states the cost model the PR body calls the understated one
README now says "9.0% before, 14.1% after" and nothing else, while README line 124 recommends
CARGO_INCREMENTAL: 0— the configuration for which the real figure is 36.4% → 57.0%. The PR body says outright that the CI number "is the one consumers feel, and the whole-tree number understates it"; the durable doc records only the flattering one. Perdocs-ship-with-codethe README is where the cost model lives, so it should carry both rows.Nits (non-blocking)
hardlink-clone-selftest.sh:127still cites the supersededa fifth of the tree → 99.998%pair (the cargo 1.97.1 measurement from #14); the PR's own updated figures are 39.3% → 99.996%.THE load-bearing function of this whole designcomment block now sits above_mutable_dirs, not aboveunshare_mutable_paths, which is left with no header.@v1" — dotfiles'seed-worktree-cache.shis a fourth consumer of the same function.What checks out — including the load-bearing justification, more strongly than stated
#17's mechanism, reproduced independently (bin crate + unit test,
cargo test --no-run,cp -al, change source, build in clone):debug/deps/bprobe-18ce56843dfcdd70debug/deps/bprobe-52a15e0acc18b2cddebug/build/bprobe/…/out/bprobe-…The consequence chain, reproduced end to end on stable — the part that sets the severity:
Base reports
Fresh, does not relink, and keeps the other branch's code in the binary that runs. The chain does not break anywhere.Cost figures reproduce exactly, read-only scan of
~/.cache/cargo-target/zemyna/zemyna(5.54 GB, 6045 files, 1.37 GB excludingincremental/):The
nlink >= 2amortisation claim is correct, and stronger than argued: the delta set is 36 files / 283 MB, of which 35 are executables and not one has an in-tree hardlink twin — so every one of them hasnlink == 1in the source,nlink == 2aftercp -al, and-links +1matches all of them on every clone. The PR's "+282 MB per clone" is right.No materially narrower rule is available, and the reason is sharper than the PR states. The candidate #17 names — executable AND no in-tree hardlink twin — would unshare exactly the same 283 MB, recovering none of the new cost. Of the tree's 776 MB of executable bytes, the 489 MB that do have a twin are build-script binaries (
build_script_build-<hash>/build-script-buildpairs), which live underbuild/and were therefore already 100% real-copied by the old selection. Exempting them would not reduce this PR's cost; it would reduce the status quo cost, on files about whose safety no evidence exists — the same "ship a fresh exposure" argument the PR correctly makes about the layout-only split. I also confirmed nlink is not predictive at all: two test binaries in one lib+bin crate, bothnlink == 1in the source, both rewritten. So the broad rule is the right call, and this is the argument for it.Under v1 the new selection is a strict superset of the old on that real tree — 0 bytes move from real-copied to shared — except for finding 2. Every file in the 106 v1 build-unit dirs is covered by name (
output,root-output,stderr,invoked.timestamp,*.d), by the run-unitout/rule, or by the executable rule. Noout,runorfingerprintdirectory exists anywhere outsidebuild/in that tree, so no name collision."Why the two land together" holds. Under v2 the
-name buildclause real-copies the whole tree, so executables are private today by accident; the layout fix alone would share them for the first time on a path measured as written in place. Shipping the layout fix on its own would be a fresh corruption bug.Sanity, from committed state:
shellcheck -x --source-path=scripts scripts/*.shclean;bash scripts/selftest.sh— all 6 suites pass (10 assertions inhardlink-clone-selftest.sh).Housekeeping: exactly two closing directives (
Closes #14,Closes #17), both well-formed, none in the commit message. #17 carriesbug, applied in the create call (label event timestamp == issuecreated_at), and records the unenumerable trigger as a named limitation rather than a footnote.I edited nothing in the worktree; all experiments ran in a scratch directory with their own temp target dirs, and the zemyna tree was read only.
out/toward unsharing, and pin the linked-output scenario on a shape that exhibits itVERDICT: APPROVE-WITH-NITS
Re-review at
0553a6b. Both blocking findings are closed, and I re-verified each independently rather than reading the diff for intent. Nothing new blocks.Finding 2 — the
out/hole is closed, three-way provenSame repro as last round (build script writes
OUT_DIR, then panics; clone; build in the clone; read the source'sgen.txt), run against three selections in turn:Closed, and closed back to
main's behaviour rather than to something adjacent._holds_compiled_artifact's glob — verified rather than assumed, acrossdefault,nullglobanddotglob, on an empty dir, an rlib-only dir, an rmeta-only dir, an executable-only dir, and a path containing a space:Correct under all three. The comment reasons from "nullglob off", which is true (nothing in
scripts/or eitheraction.ymlrunsshoptat all) but narrower than the actual guarantee — with nullglob on the loop simply does not execute and still returns 1. Onlyfailglobwould break it, and nothing sets it.Strict superset restored. Re-scanned
~/.cache/cargo-target/zemyna/zemynaread-only with the new selection: 0 bytes / 0 files move from real-copied to shared versusmain. No remaining gap on v1.Incidental, worth knowing: under layout v2 this state was never exposed — cargo creates
run/for a build script that fails, so signal 2 already covered v2 and the hole was v1-only. Signal 1 is what makes it uniform."This costs nothing" checks out on a real v2 tree. I built a v2 workspace (proc-macro + lib + bin + test + build script, cargo 1.100.0-nightly) and enumerated every
out/:Every newly real-copied compile-unit
out/holds only executables and*.d, both already privately owned by the file rules; the only non-exe content in a real-copiedoutis the build script's own OUT_DIR, which is the intended case. So the v2 figure is genuinely unmoved.Finding 1 — the scenario now pins what it names
Red-proven against
main'scache-lib.shwith the fixtures neutered:The fix half now fails against the unfixed selection — last round it passed green — and the control's mutated set is a single executable, which was the specific defect.
source_exe_digest'sfind -type f -executableis what forecloses the old failure mode: dep-info and fingerprint mutations can no longer satisfy the control on their own.The loud three-outcome skip (
::warning::when a toolchain does not exhibit the rewrite, rather than a quiet pass) is the right shape and matches the freshness probe's precedent in the same file.The new fixture case is red against the previous commit, which is the right thing for it to be red against:
Cost table
Re-derived read-only on the same 5.54 GB tree with the new selection — unchanged, as the body claims:
README now carries both rows with the CI-shaped one first and ties it to the quick-start's own
CARGO_INCREMENTAL: 0, which was finding 4. Good.Narrower-rule analysis
Landed at "Ambiguity resolves toward unsharing" / "the safe cases are not enumerable", and every figure in it matches what I measured: 776 MB of executable bytes, 489 MB twinned and all build-script binaries under
build/already fully copied by the old selection, 283 MB / 36 files / 35 executables / zero twins in the delta. The framing — that exempting twins would cut the status quo rather than this PR's cost, on files with no safety evidence — is stated accurately.Nits (non-blocking)
nlink == 1in the source, behaved differently from each other — one rewritten, one not." I tried to reproduce that pair. In my run the intact one turned out not to have been rebuilt at all — identical content in base and clone, still one inode — so it is a not-exercised unit rather than a demonstrated safe one. When I forced both to rebuild (a#[cfg(test)]module inmain.rsas well aslib.rs), both were rewritten. The conclusion is untouched — the twin analysis carries the argument on its own, and #17's separate "2 in both" measurement is yours, not something I can refute — but consider softening that clause to what it demonstrates.*.rlib/*.rmetaintoOUT_DIRthat has never once succeeded) is honestly stated in both the code and the body, but is not tracked anywhere in Gitea. Given how narrow it is, a line in the comment is arguably the right home — flagging only so the choice is deliberate.Verified housekeeping
Nits 1–3 from last round all landed: the superseded
99.998%figure at:127is now39.3% → 99.996%; theTHE load-bearing functionheader sits directly aboveunshare_mutable_pathsagain; and the fourth consumer (dotfiles'scripts/seed-worktree-cache.shhardlink tier) is named in the body. Thewhenever the path it is given has no other hard linkclause is gone and replaced with the correct "hardlink count is NOT what decides it", citing #17.shellcheck -x --source-path=scripts scripts/*.shclean.bash scripts/selftest.sh— all 6 suites pass, from committed state; the fixture byte figures in the body (34% of 25086 B (8675 B),43% of 28920 B (12509 B)) match my run exactly.Two closing directives,
Closes #14andCloses #17, both well-formed; none in either commit message. PR is still correctly draft.I edited nothing in the worktree; every experiment ran in a scratch directory with its own temp target dirs, and the zemyna tree was read only.