From 0553a6b956494ec667ee7d848da2a4e9ff662360 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 14:39:29 -0500 Subject: [PATCH] fix(hardlink): resolve an ambiguous `out/` toward unsharing, and pin the linked-output scenario on a shape that exhibits it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on #16. The `out/` discriminator keyed on Cargo's record of a build-script execution, which Cargo writes only AFTER the script exits successfully. A build script that populates OUT_DIR and then fails leaves a unit with no record at all, so its OUT_DIR read as a compile unit's artifact directory and stayed shared — a regression against the old `-name build` selection, which real-copied that state by construction. Reproduced on cargo 1.93.1 stable. An `out` directory now stays shared only when two independent signals agree: it holds an `.rlib`/`.rmeta` of its own, and its unit carries no execution record. Either one missing real-copies it. The cost is unchanged to the byte — the newly-unshared directories hold only executables and `*.d`, both already privately owned by the file rules. The live linked-test-binary scenario was built on the lib+bin probe crate, whose test binaries relink to a fresh inode — a shape gitdan-actions#17 records as measured safe. 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, and skips loudly with a warning rather than passing quietly if the toolchain does not exhibit the rewrite at all. Also: drop a clause asserting the linker writes in place "whenever the path has no other hard link", which this change's own evidence denies; move the load-bearing comment block back above `unshare_mutable_paths`; correct a superseded 99.998% figure; and record both cost rows in the README rather than only the flattering whole-tree one. --- README.md | 35 +++++++-- scripts/cache-lib.sh | 110 ++++++++++++++++++--------- scripts/hardlink-clone-selftest.sh | 115 +++++++++++++++++++++-------- 3 files changed, 187 insertions(+), 73 deletions(-) diff --git a/README.md b/README.md index 8e879d7..db8dbff 100644 --- a/README.md +++ b/README.md @@ -57,14 +57,35 @@ Layout v2 regroups everything per build unit under `.fingerprint` and no `deps` to key off and `build/` is no longer a proxy for "metadata" — it is the whole tree. The one place the two layouts genuinely differ is that under v2 a build script's `OUT_DIR` and a compile unit's rlib -are both a directory called `out`; the run unit is told apart by Cargo's record -of the execution beside it (`run/root-output` under v2, a loose `root-output` -under v1). v2 is the nightly default and stabilises in cargo 1.100.0 on -2026-11-12. +are both a directory called `out`. -Measured real-copied share of a 5.5 GB Bevy target directory: **9.0% before, -14.1% after** — the increase is the linker-output rule, not the layout work. -On a scratch crate the layout fix alone takes v2 from 99.996% to 0.2%. +**Ambiguity there resolves toward unsharing**, because over-unsharing costs +bytes and under-unsharing costs corruption. An `out` directory stays shared +only when two independent signals agree it is a compile unit's: it holds an +`.rlib`/`.rmeta` of its own, *and* its unit carries no record of a build-script +execution beside it (`run/` under v2, a loose `root-output` under v1). The +execution record alone is not enough — Cargo writes it only after the script +succeeds, so a build script that populates `OUT_DIR` and then fails leaves a +unit that reads as a compile unit. v2 is the nightly default and stabilises in +cargo 1.100.0 on 2026-11-12. + +### What it costs + +Real-copied share of a 5.5 GB Bevy target directory, before and after the +linker-output rule landed: + +| tree | before | after | +|---|---|---| +| **excluding `incremental/`** — the figure to plan against, since the quick-start below sets `CARGO_INCREMENTAL: 0` | **36.4%** | **57.0%** | +| whole tree, `incremental/` included (a local dev checkout, not CI) | 9.0% | 14.1% | + +The first row is the one a CI consumer gets. The increase is the linker-output +rule, not the layout work: on a scratch crate the layout fix alone takes v2 +from 99.996% to 0.2%. + +The copy is paid per clone and does not amortise — a fresh `cp -al` leaves +every file with `nlink >= 2`, so the `-links +1` filter cannot skip anything — +and a clone happens twice per job, once seeding and once publishing. --- diff --git a/scripts/cache-lib.sh b/scripts/cache-lib.sh index e33c31f..d898e0b 100755 --- a/scripts/cache-lib.sh +++ b/scripts/cache-lib.sh @@ -317,6 +317,73 @@ _unshare_files() { xargs -0 -r -n 64 bash -c 'rc=0; for f; do cp -p -- "$f" "$f.unshare.$$" && mv -f -- "$f.unshare.$$" "$f" || rc=1; done; exit $rc' _ } +# True when a directory holds a compiled library artifact of its own. +# +# The glob is left unquoted and unmatched-glob-safe on purpose: with nullglob +# off an unmatched pattern stays literal and the `-e` test fails, which is the +# answer wanted. +_holds_compiled_artifact() { + local f + for f in "$1"/*.rlib "$1"/*.rmeta; do + [ -e "$f" ] && return 0 + done + return 1 +} + +# The directories `unshare_mutable_paths` replaces, under either layout. +# +# All four names are pruned, so nothing selected here can contain anything else +# selected here and the caller never unshares a subtree twice. +# +# `out` is the one that needs deciding rather than naming, and it is the whole +# difficulty of layout v2: a compile unit's rlib and a build script's OUT_DIR +# are both a directory called `out`, one directory apart, and they need +# opposite treatment. +# +# AMBIGUITY RESOLVES TOWARD UNSHARING, and that direction is the rule rather +# than a default: over-unsharing costs bytes, under-unsharing costs corruption. +# So an `out` directory is left shared only when TWO independent signals agree +# it is a compile unit's artifact directory, and either one missing is enough +# to real-copy it: +# +# 1. it holds an `.rlib`/`.rmeta` of its own — the artifact whose sharing is +# the entire point of the clone; and +# 2. its unit directory has no record of a build-script execution beside it +# (`run/` under layout v2, a loose `root-output` under v1). +# +# Signal 2 alone was the first cut of this and it is NOT sufficient, because +# Cargo writes `root-output` only AFTER the script exits successfully. A build +# script that populates `OUT_DIR` and then FAILS leaves a unit with no record +# at all, which reads as "compile unit" — and the old `-name build` selection +# covered that state by real-copying `build/` wholesale, so trusting signal 2 +# alone was a regression against it. Reproduced on cargo 1.93.1 stable: the +# clone's build wrote through the shared inode into the source's OUT_DIR. +# Signal 1 closes it, because a failed build script's OUT_DIR holds no rlib. +# +# The residual is a build script that writes a file NAMED `*.rlib`/`*.rmeta` +# into `OUT_DIR` and has never once succeeded. Nothing bounds that away; it is +# simply far narrower than what it replaces. +# +# Requiring signal 1 also means a bin, test or build-script COMPILE unit's +# `out` is real-copied rather than shared — at no cost in bytes, since +# everything in one is an executable or a `*.d`, and both are privately owned +# by the file rules below either way. +_mutable_dirs() { + local root="$1" d unit + while IFS= read -r d; do + if [ "${d##*/}" = out ]; then + unit="${d%/out}" + if ! [ -d "$unit/run" ] && ! [ -e "$unit/root-output" ] \ + && _holds_compiled_artifact "$d"; then + continue + fi + fi + printf '%s\n' "$d" + done < <(find "$root" -type d \ + \( -name .fingerprint -o -name fingerprint -o -name run -o -name out \) \ + -prune -print 2>/dev/null) +} + # THE load-bearing function of this whole design. # # A hardlink clone is only safe if every write the clone's build performs @@ -403,7 +470,7 @@ _unshare_files() { # included. Bracketed locally: cargo 1.97.1 and 1.98.0-nightly write v1, # 1.100.0-nightly writes v2. # -# Until 2026-08-27 the selection below was `-name .fingerprint -o -name build`, +# Until 2026-08-27 the selection was `-name .fingerprint -o -name build`, # which under a v2 Cargo matched nothing on its first clause and the entire # tree on its second, because the artifacts moved under `build/` too. The guard # held by accident and the saving did not: on one scratch crate (serde + @@ -423,33 +490,6 @@ _unshare_files() { # and garbage-collects old sessions by unlinking directory entries — neither # of which mutates a shared inode. CI should still set CARGO_INCREMENTAL=0, # for size rather than correctness. -# The directories `unshare_mutable_paths` replaces, under either layout. -# -# All four names are pruned, so nothing selected here can contain anything else -# selected here and the caller never unshares a subtree twice. -# -# `out` is the one that needs deciding rather than naming, and it is the whole -# difficulty of layout v2: a compile unit's rlib and a build script's OUT_DIR -# are both a directory called `out`, one directory apart, and they need -# opposite treatment. The discriminator is structural — Cargo records a build -# script's execution beside its OUT_DIR and nowhere else, as `run/root-output` -# under v2 and as a loose `root-output` under v1 — so a unit directory holding -# one of those is a RUN unit and its `out` is the OUT_DIR. A unit directory -# without one is a compile unit, and its `out` holds the artifact whose sharing -# is the entire point of the clone. -_mutable_dirs() { - local root="$1" d unit - while IFS= read -r d; do - if [ "${d##*/}" = out ]; then - unit="${d%/out}" - [ -d "$unit/run" ] || [ -e "$unit/root-output" ] || continue - fi - printf '%s\n' "$d" - done < <(find "$root" -type d \ - \( -name .fingerprint -o -name fingerprint -o -name run -o -name out \) \ - -prune -print 2>/dev/null) -} - unshare_mutable_paths() { local root="$1" d [ -d "$root" ] || return 0 @@ -479,12 +519,14 @@ unshare_mutable_paths() { } # Linked outputs. Unlike an rlib or an rmeta — which rustc writes to a # temporary and renames into place — an executable or shared object is - # written by the LINKER, and the linker writes THROUGH an existing inode - # whenever the path it is given has no other hard link. Measured 2026-08-27 - # on cargo 1.98.0-nightly (a335d47ff, layout v1) and 1.100.0-nightly - # (e8cb624d5, layout v2), mold and the default linker alike: a `cargo test` - # binary in a `cp -al` clone rewrote the SOURCE's copy of itself in place, - # under both layouts. + # written by the LINKER, and the linker writes THROUGH an existing inode. + # Measured 2026-08-27 on cargo 1.93.1 stable, 1.96.0-nightly, 1.98.0-nightly + # (layout v1) and 1.100.0-nightly (e8cb624d5, layout v2), mold and the + # default linker alike: a `cargo test --no-run` binary in a `cp -al` clone + # rewrote the SOURCE's copy of itself in place, under both layouts. Hardlink + # count at link time is NOT what decides it — measured 2 in both a rewritten + # and an intact case, and two test binaries of one crate at `nlink == 1` + # behaved differently from each other. See gitdan-actions#17. # # The executable bit is the discriminator because it is the linker's own # output that is at risk, not the directory it happens to land in — `.rlib`, diff --git a/scripts/hardlink-clone-selftest.sh b/scripts/hardlink-clone-selftest.sh index 01c4846..57fb578 100755 --- a/scripts/hardlink-clone-selftest.sh +++ b/scripts/hardlink-clone-selftest.sh @@ -124,7 +124,8 @@ assert_partition() { # The cost model, asserted rather than assumed. Selecting too much is not a # correctness bug, which is exactly why nothing caught layout v2 taking the - # selection from a fifth of the tree to 99.998% of it (gitdan-actions#14): + # selection from 39.3% of one scratch crate's tree to 99.996% of it + # (gitdan-actions#14): # a hardlink clone that real-copies everything is a `cp -a` with extra steps. # The bound is loose on purpose. It is not a budget — the honest figure moves # with how much of a tree is linker output, and these fixtures are mostly @@ -186,6 +187,12 @@ mkfile "$v2/debug/build/probe/da96cf45111f80dd/out/gen.txt" 'generated from 24 b for f in invoked.timestamp root-output stdout stderr; do mkfile "$v2/debug/build/probe/da96cf45111f80dd/run/$f" "$f" done +# A build script that wrote into OUT_DIR and then FAILED: Cargo records the +# run only on success, so this unit has `out/` populated and no `run/` at all. +# Reading "no execution record" as "compile unit" left this shared, which is +# the one state the old `-name build` selection covered and the first cut of +# this one did not. +mkfile "$v2/debug/build/probe/f00ded00f00ded00/out/gen.txt" 'half-written' mkfile "$v2/debug/incremental/probe-abc/s-xyz/dep-graph.bin" "$artifact_bytes" assert_partition "layout v2" "$v2" "$(cat <<'SPEC' @@ -205,6 +212,7 @@ private|debug/build/probe/da96cf45111f80dd/run/root-output private|debug/build/probe/da96cf45111f80dd/run/stdout private|debug/build/probe/da96cf45111f80dd/run/stderr private|debug/build/probe/da96cf45111f80dd/run/invoked.timestamp +private|debug/build/probe/f00ded00f00ded00/out/gen.txt shared|debug/libprobe.rlib shared|debug/build/probe/1bf5493368dce3cd/out/libprobe-1bf5493368dce3cd.rlib shared|debug/build/probe/1bf5493368dce3cd/out/libprobe-1bf5493368dce3cd.rmeta @@ -243,6 +251,8 @@ for f in invoked.timestamp output root-output stderr; do mkfile "$v1/debug/build/probe-da96cf45111f80dd/$f" "$f" done mkfile "$v1/debug/build/probe-da96cf45111f80dd/out/gen.txt" 'generated from 24 bytes' +# The same never-succeeded build script under layout v1. +mkfile "$v1/debug/build/probe-f00ded00f00ded00/out/gen.txt" 'half-written' mkfile "$v1/debug/incremental/probe-abc/s-xyz/dep-graph.bin" "$artifact_bytes" assert_partition "layout v1" "$v1" "$(cat <<'SPEC' @@ -261,6 +271,7 @@ private|debug/build/probe-da96cf45111f80dd/output private|debug/build/probe-da96cf45111f80dd/root-output private|debug/build/probe-da96cf45111f80dd/stderr private|debug/build/probe-da96cf45111f80dd/out/gen.txt +private|debug/build/probe-f00ded00f00ded00/out/gen.txt shared|debug/libprobe.rlib shared|debug/deps/libprobe-1bf5493368dce3cd.rlib shared|debug/deps/libprobe-1bf5493368dce3cd.rmeta @@ -567,44 +578,84 @@ echo "=== a linked TEST binary, which nothing uplifts and nothing replaces ===" # The one artifact family that is NOT safe to share, and the reason # `unshare_mutable_paths` privately owns every executable. rustc writes an # rlib to a temporary and renames it in; the LINKER writes an executable -# through whatever inode is already at the path. Cargo re-creates the path -# first when it also has to uplift the result — a bin target's -# `deps/-` has a hardlink twin at `/` — which is why -# bin targets look safe. A test binary has no twin, nothing re-creates it, and -# the write lands on the source's inode. +# through whatever inode is already at the path. # -# Measured 2026-08-27 on cargo 1.98.0-nightly (a335d47ff, layout v1) and -# 1.100.0-nightly (e8cb624d5, layout v2): under BOTH layouts a `cargo test -# --no-run` inside a raw `cp -al` clone rewrote the SOURCE's own test binary. +# THE CRATE SHAPE IS LOAD-BEARING AND WAS WRONG ONCE. An earlier cut of this +# scenario reused the lib+bin probe crate above, whose test binaries relink to +# a FRESH inode — a shape gitdan-actions#17 records as measured safe. Both +# halves then passed green against the unfixed selection, on the strength of +# dep-info mutations the previous scenario already covers, and the scenario +# pinned nothing. A bin-only crate with a unit test does exhibit the rewrite, +# on cargo 1.93.1 stable and on 1.98.0-nightly and 1.100.0-nightly, so that is +# what this builds. `nlink` does not predict which is which — two test binaries +# of one crate, both at 1, behaved differently — so the shape is chosen by +# measurement rather than derived. +mkbincrate() { + local dir="$1" marker="$2" + mkdir -p "$dir/src" + cat > "$dir/Cargo.toml" <<'TOML' +[package] +name = "binprobe" +version = "0.1.0" +edition = "2021" +[workspace] +TOML + cat > "$dir/src/main.rs" < 0, true); } } +RS +} + +bin_dir="$scratch/binprobe" +mkbincrate "$bin_dir" MARKER_AAAA +cd "$bin_dir" base_exe="$scratch/base-exe"; clone_exe_ctl="$scratch/clone-exe-ctl"; clone_exe="$scratch/clone-exe" -printf '%s\n' "$CONTENT_A" > src/lib.rs -CARGO_TARGET_DIR="$base_exe" "${CARGO_BIN[@]}" test --no-run -q > /dev/null 2>&1 -before=$(snapshot_tree "$base_exe") + +# Only the executables are read here. The families the other scenarios cover +# would satisfy a "something changed" assertion on their own, which is exactly +# how the earlier cut of this passed while pinning nothing. +source_exe_digest() { + (cd "$1" && find . -type f -executable -print0 | sort -z | xargs -0 -r sha1sum) 2>/dev/null +} + +CARGO_TARGET_DIR="$base_exe" "${CARGO_BIN[@]}" test --no-run -q > /dev/null 2>&1 \ + || fail "the bin-only probe crate failed to build" +before=$(source_exe_digest "$base_exe") cp -al "$base_exe" "$clone_exe_ctl" strip_cargo_locks "$clone_exe_ctl" -printf '%s\n' "$CONTENT_B" > src/lib.rs +mkbincrate "$bin_dir" MARKER_BBBB CARGO_TARGET_DIR="$clone_exe_ctl" "${CARGO_BIN[@]}" test --no-run -q > /dev/null 2>&1 -exe_ctl_mutated=$(mutated_paths "$before" "$(snapshot_tree "$base_exe")") -[ -n "$exe_ctl_mutated" ] \ - || fail "control: a raw cp -al clone of a test build no longer mutates the source — the test can no longer tell fixed from broken" -ok "raw cp -al clone of a test build mutates the source ($(printf '%s\n' "$exe_ctl_mutated" | wc -l) paths)" -printf '%s\n' "$exe_ctl_mutated" | sed 's/^/ /' +exe_ctl_mutated=$(mutated_paths "$before" "$(source_exe_digest "$base_exe")") -# Rebuild the base from CONTENT_A so it is warm and consistent again, then do -# the same thing through the real clone. -printf '%s\n' "$CONTENT_A" > src/lib.rs -CARGO_TARGET_DIR="$base_exe" "${CARGO_BIN[@]}" test --no-run -q > /dev/null 2>&1 -before=$(snapshot_tree "$base_exe") -hardlink_clone_into "$base_exe" "$clone_exe" "selftest-exe" \ - || fail "hardlink_clone_into refused the destination" -printf '%s\n' "$CONTENT_B" > src/lib.rs -CARGO_TARGET_DIR="$clone_exe" "${CARGO_BIN[@]}" test --no-run -q > /dev/null 2>&1 -exe_mutated=$(mutated_paths "$before" "$(snapshot_tree "$base_exe")") -if [ -n "$exe_mutated" ]; then - printf '%s\n' "$exe_mutated" | sed 's/^/ /' >&2 - fail "a test build in the clone mutated the source through a shared inode" +# THREE OUTCOMES, as the freshness probe above has, and for the same reason: a +# scenario that cannot tell "the fix works" from "the hazard never fired" is +# not a gate. If this Cargo does not rewrite the source's test binary, the +# assertion below would pass for a toolchain reason rather than a code one, so +# it is skipped LOUDLY instead of passing quietly. +if [ -z "$exe_ctl_mutated" ]; then + echo "::warning::hardlink-clone-selftest: this toolchain did not rewrite the source's test binary through a raw cp -al clone, so the linked-output scenario proves nothing here and was SKIPPED. That is a statement about this Cargo, not about unshare_mutable_paths." +else + ok "control: a raw cp -al clone rewrites the source's own linked test binary" + printf '%s\n' "$exe_ctl_mutated" | sed 's/^/ /' + + # Rebuild the base from the original marker so it is warm and consistent + # again, then do the same thing through the real clone. + mkbincrate "$bin_dir" MARKER_AAAA + CARGO_TARGET_DIR="$base_exe" "${CARGO_BIN[@]}" test --no-run -q > /dev/null 2>&1 + before=$(snapshot_tree "$base_exe") + hardlink_clone_into "$base_exe" "$clone_exe" "selftest-exe" \ + || fail "hardlink_clone_into refused the destination" + mkbincrate "$bin_dir" MARKER_BBBB + CARGO_TARGET_DIR="$clone_exe" "${CARGO_BIN[@]}" test --no-run -q > /dev/null 2>&1 + exe_mutated=$(mutated_paths "$before" "$(snapshot_tree "$base_exe")") + if [ -n "$exe_mutated" ]; then + printf '%s\n' "$exe_mutated" | sed 's/^/ /' >&2 + fail "a test build in the clone mutated the source through a shared inode" + fi + ok "no file in the source changed after a full test build in the clone" fi -ok "no file in the source changed after a full test build in the clone" +cd "$crate_dir" echo if [ "$CHECKSUM_MODE" = "on" ]; then