4bb880b7a76f7e7e590281d892de76d2f835fc8f
31
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 claude
|
fb3c72aa28
|
docs(hardlink): withdraw the nlink claim, which asserted more than was measured
CI / shellcheck + selftests (pull_request) Skipped
The comments said hardlink count at link time was ruled out as the discriminator between a rewritten executable and an intact one, citing a lib+bin crate whose two test binaries were both `nlink == 1` and appeared to behave differently. Re-checked on review: the intact one had not been rebuilt at all — same content, same inode — so it demonstrated nothing, and forcing both to rebuild rewrote both. What was actually observed is narrower and now says so: every executable measured intact had an uplift hardlink twin Cargo must re-create anyway, every one measured rewritten had none, and whether the twin is the mechanism or a correlate was not determined. The rule does not rest on the answer — exempting twinned executables would recover none of the bytes this change newly copies. This PR exists because a claim outlived its evidence; it should not ship one. |
||
|
claude
|
0553a6b956
|
fix(hardlink): resolve an ambiguous out/ toward unsharing, and pin the linked-output scenario on a shape that exhibits it
CI / shellcheck + selftests (pull_request) Skipped
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. |
||
|
claude
|
e5b26a9368
|
fix(hardlink): name the mutable set directly, under both build-dir layouts
CI / shellcheck + selftests (pull_request) Skipped
`unshare_mutable_paths` selected `.fingerprint` and `build` directories. Under Cargo's build-dir layout v2 the first clause matches nothing and the second matches the whole tree, because v2 regroups artifacts under `build/` alongside the metadata. Measured on one scratch crate: 39.3% of the tree real-copied under v1, 99.996% under v2. The selection now names the mutable set rather than the container it used to live in: fingerprint directories under either spelling, layout v2's `run/` directories, layout v1's loose build-script run metadata, and the `out` directories that are a build script's OUT_DIR rather than a compile unit's artifact directory. The two are told apart structurally, by Cargo's record of the build-script execution sitting beside the OUT_DIR and nowhere else. Verifying that turned up a second, layout-independent hazard: a linked executable is written through whatever inode is already at its path, so a `cargo test --no-run` inside a `cp -al` clone rewrites the source's own test binary. Reproduced on cargo 1.93.1 stable, 1.96.0-nightly, 1.98.0-nightly and 1.100.0-nightly, under both layouts. Every executable is now real-copied; `.rlib`, `.rmeta` and `incremental/` are what stay shared. hardlink-clone-selftest.sh gains two file-only layout fixtures that pin the partition in both directions without a compiler, and a live scenario that relinks a test binary. |
||
|
claude
|
554310186f
|
fix(hardlink): content freshness moved switches, it was not withdrawn
CI / shellcheck + selftests (pull_request) Successful in 1m17s
`unshare_mutable_paths`' comment recorded that upstream had stopped rewriting `dep-<target>` in place, on a measurement taken against 1.100.0-nightly. It had not. Two unrelated cargo changes landed within four days of each other and between them moved the switch that turns the behaviour on and the path it writes to: - cargo PR #17382 (2026-08-22) demoted `-Z checksum-freshness` to a gate and gave `build.fingerprint` the choice, defaulting to `mtime`. Setting only the gate is accepted and does nothing, which is exactly the result that was read as a withdrawal. - build-dir layout v2 (cargo PR #17354, stable 1.100.0 on 2026-11-12, nightly default since 1.99) moved the file from `.fingerprint/<unit>/dep-*` to `build/<pkg>/<hash>/fingerprint/dep-*`. Measured 2026-08-26 on 1.100.0-nightly (e8cb624d5): same toolchain, same clone procedure, one env var apart — with the gate alone a `cp -al` clone mutates only the build/ and *.d families; add `CARGO_BUILD_FINGERPRINT=content` and the source's dep-info file is mutated through the shared inode again. The hazard is intact. So the suite now exports both switches, and its strongest scenario runs again on current nightlies — verified passing against both layouts. Its control note learned the v2 path too: it looked for the v1 path only, and so printed "does NOT rewrite ... in place" three lines beneath a listing that showed the rewrite. Every claim these comments make is now dated and cited, because the defect being fixed is a comment that cited one measurement and silently stopped reproducing. Layout v2 also drags the artifacts under `build/`, which collapses this function's real-copy set from 21.3% of a target dir to 99.998%. That is a live cost, not a correctness problem, and it is filed as gitdan-actions#14 rather than fixed here. Part of daniel/gitdan#62. |
||
|
claude
|
e94c46f2ec
|
test(hardlink): move the probe's INACTIVE answer off bash's error codes
CI / shellcheck + selftests (pull_request) Successful in 1m21s
Review finding on #13, and the last change to this function. A typo'd variable name on a line that HAS its `|| exit 2` guard — `$CONTNET_B` for `$CONTENT_B` — fails in EXPANSION, before the command runs, so neither the guard nor the ERR trap ever sees it. Under `set -u` that exits 1, which was the code meaning "measured, and the answer is mtime". Not a live defect: the probe references only set variables today, and the 1 gitdan-ci reports is a genuine measurement. The defect is that the answer codes and the error codes overlapped at all. Three rounds on this function each found a narrower way for a shell-generated 1 to be read as a measurement — an unguarded command, then a guarded line whose guard could not fire — and each was closed by narrowing the failure surface, which is a game with no last move. INACTIVE is now 3, and anything that is not 0 or 3 is NOT MEASURED. Bash generates 1, 2, 126, 127 and 128+n for its own errors and never 3, so "not an answer" is decided by a property of the shell rather than by an enumeration of the ways a step can go wrong. A step added later without a guard, or with a guard that cannot fire, lands on NOT MEASURED by construction. The guards and the trap stay, with their job restated: reporting rather than correctness. They put a failed step on 2 with both probe logs printed instead of on some incidental status — nicer to debug, and the same destination either way. The `set +e` bracket at the call site keeps its own note, since a `set -e` that cannot fire is the kind of thing a reader assumes works. The not-measured message now quotes the actual exit code, so the two ways to reach it are distinguishable in a log without reading this file. Red-proven with the reviewer's own mutation, A/B: with INACTIVE at 1 the typo reports `mode: off — resolves freshness by mtime`, a false measurement; at 3 it reports `mode: unmeasured — the probe exited 1`. All four paths re-verified — unmodified 4 assertions; env stripped inside the probe, measured INACTIVE with the mtime reason; a broken probe build, unmeasured via a guard; an unguarded `false`, unmeasured via the trap. |
||
|
claude
|
5b46986cd7
|
test(hardlink): make the errexit invariant real, and stop gpg deciding a gate
CI / shellcheck + selftests (pull_request) Successful in 1m23s
Two review findings on #13. THE STATED INVARIANT WAS NOT IN FORCE. The header claimed a `set -e` abort could never be mistaken for the `1` that means "measured, and the answer is mtime". It could not fire at all: `checksum_freshness_probe || probe_rc=$?` runs the left side with errexit suppressed, and that suppression propagates into the subshell. An unguarded step there fell through to `exit 0` and reported ACTIVE — a fourth outcome the header said was impossible. The guard audit was complete, so nothing was broken; the wrong mechanism had the credit, in a comment inviting the next editor to add an unguarded step and rely on it. The suggested fix — `set -e` as the first line of the subshell — does not work, and measured on bash 5.3 it makes things worse rather than not-better: set -e inside a subshell called via `||` or `if` still falls through: rc=0 set -e inside, called with errexit off at the site aborts, but with the FAILING COMMAND's status — `false` gives 1, which is exactly the value that means "mtime" So both halves are needed and neither is decoration: `set +e` around the call site, so the subshell can arm its own errexit at all, and `trap 'exit 2' ERR` inside it, so an abort lands on "not measured" instead of on an answer. The explicit `|| exit 2` guards stay as the first line of defence. All of that is now written down where the false claim was. Red-proven with one unguarded `false` between the `touch` and the second build, on a box where freshness is live: without the fix, mode `on` and 4 assertions — the reviewer's fourth outcome, reproduced; with it, mode `unmeasured`, the warning, 3 assertions; unmodified, mode `on` and 4. Recorded in the same block, since the next person to hit a `2` will ask: why it skips rather than fails. The gated scenario is the only thing here that depends on freshness mode, and failing would turn a statement about one machine into a red gate reading "the hardlink scheme is broken" across three consuming repos. What would change the answer is `2` becoming the everyday CI outcome, and run 2583 measures that it is not — gitdan-ci reports `1`. GPG. The two suites that commit in scratch repos inherited the developer's GLOBAL `commit.gpgsign`, so whether this gate passes could depend on their gpg agent — seen as a red `restore-mtimes-selftest.sh` caused by a full disk breaking gpg, in a suite with nothing to say about either. Pinned off locally, in the throwaway repos only: `git config commit.gpgsign false` beside the identity the scratch repo already sets, and `-c commit.gpgsign=false` on prune-cache-selftest's three commits, matching its existing `-c` style. Red-proven under a GIT_CONFIG_GLOBAL with gpgsign on and a nonexistent gpg program: without it `fatal: failed to write commit object`, with it both suites pass. |
||
|
claude
|
5a90d5c829
|
test(hardlink): let the freshness probe report that it could not measure
CI / shellcheck + selftests (pull_request) Successful in 1m21s
Review finding on #13. `checksum_freshness_active()` returned non-zero for any reason — including its own `cargo build` failing — and the caller's `else` branch then announced "this nightly accepts -Z checksum-freshness but still resolves freshness by mtime" regardless. On a machine where content freshness IS live, breaking only the probe's build produced that sentence, which is false, and silently dropped the scenario, which is lost coverage. Reachable, not theoretical: it needs a half-installed toolchain, which is exactly the state a probe exists to notice. The whole point of the preceding commit was to settle this by experiment rather than by asking, and an experiment that cannot tell a negative result from a failed measurement is not settling it. "This toolchain resolves freshness by mtime" is a statement about Cargo; "a probe build failed" is a statement about this machine. They are not interchangeable. Three outcomes now, carried in an exit code: 0 measured ACTIVE both builds ran, the backdated rebuild recompiled 1 measured INACTIVE both builds ran, the backdated rebuild was Fresh 2 NOT MEASURED a probe step failed; nothing was learned Every step that could fail for a reason other than the experiment's own outcome exits 2 explicitly, so a `set -e` abort cannot be mistaken for the 1 that means "measured, and the answer is mtime". Outcome 2 emits a ::warning:: saying in those words that this is a failure to measure and not a finding about Cargo, and prints the tail of both probe logs. The scenario still skips for 1 and 2 alike — everything else the suite asserts is independent of freshness mode — but the reason is now carried to the skip line, so the two are distinguishable at a glance. Verified all three states on a machine where freshness is live: unmodified, mode `on` and 4 assertions; probe build broken with a bogus flag, mode `unmeasured` with the warning and 3 assertions, and no claim about mtime; CARGO_UNSTABLE_CHECKSUM_FRESHNESS stripped inside the probe — CI's condition — mode `off` naming mtime, and 3 assertions. |
||
|
claude
|
6a80423bd0
|
test(hardlink): settle checksum freshness by experiment, not by asking
CI / shellcheck + selftests (pull_request) Successful in 1m17s
Third toolchain probe in this branch, and the first one that asks the
question the suite actually depends on. The two before it each let the suite
assert a property the toolchain did not have:
cargo +nightly -V answers "did a proxy called with
+nightly exit 0". `-V` short-circuits
before `-Z` is parsed at all.
cargo +nightly -Z checksum-freshness answers "is the flag still accepted".
locate-project 1.100.0-nightly (2026-08-25) accepts it
and resolves freshness by mtime anyway,
which is how CI reached the final
scenario and failed there.
The final scenario depends on exactly one property: that changed content with
an OLDER mtime rebuilds. Under mtime freshness the correct answer is Fresh, so
under mtime freshness that scenario asserts a bug — which is what CI reported.
So the probe performs that experiment, on its own crate and its own target
dir, with no clone anywhere near it. That separation is also what keeps it a
control rather than a restatement: the probe establishes that the toolchain
rebuilds on content, the scenario establishes that a hardlink clone did not
take that away.
Verified both ways locally: with a real content-freshness nightly, mode on and
4 assertions; with the env var stripped inside the probe — the runner's
condition, faithfully — the probe says so by name, mode goes off, the scenario
is skipped loudly and the remaining 3 assertions pass.
|
||
|
claude
|
5e8e773f78
|
test(hardlink): report the mutation families, don't assert one of them
CI / shellcheck + selftests (pull_request) Failing after 1m21s
CI's nightly is 1.100.0-nightly (2026-08-25); the machine this suite was written on had 1.96.0-nightly (2026-02-24). On the newer one the control's `cp -al` clone mutates only the build/ and *.d families — upstream appears to have stopped rewriting `.fingerprint/*/dep-*` in place under checksum freshness — so the suite went red on the *absence* of a hazard. That is the wrong shape for a gate. The control's job is to prove the hazard exists at all, which a non-empty mutated set already does; naming one family as mandatory makes the suite red whenever upstream stops doing something we never wanted it to do, and red in the CONTROL, where a failure reads as "the hazard is gone" rather than "upstream changed". It is now a note either way. Nothing is given up. The fix scenario asserts the source is byte-identical after a full rebuild in the clone, which covers every family the running Cargo has, named or not — and the checksum-freshness scenario after it tests the stale-reuse hazard directly. The dep-* line only ever documented which family was in play. `unshare_mutable_paths` keeps unsharing dep-* regardless, and its measurement block now records both observations with their versions: 22 MB of a 6.9 GB tree against a failure mode that is a wrong answer rather than a slow one. |
||
|
claude
|
3f97d3d1e7
|
fix(selftest): make the two compiler-backed suites survive a CI runner
CI / shellcheck + selftests (pull_request) Failing after 1m36s
The new gate's first run went red on both suites that drive a real Cargo.
Neither failure was a defect in what they test; both were assumptions about
the machine, which had only ever been a dev box with a nightly installed and
no colour forcing. Fixed here rather than waived — turning a gate on is what
obliges fixing what it finds.
COLOUR. Every assertion in restore-mtimes-selftest.sh, and one in
hardlink-clone-selftest.sh, reads cargo's own words out of a build log
(`Compiling libdep`, `Fresh probe`). gitdan-ci's runner image forces colour, so
cargo wrote `Compiling\e[0m libdep` into the log and `grep -q "Compiling
libdep"` stopped matching. The suite then reported the opposite of what
happened — the failure printed the log, and the log plainly said `Compiling
libdep`. Both suites now pin CARGO_TERM_COLOR=never, which is the format their
assertions are written against.
Red-proven: with the export removed and CARGO_TERM_COLOR=always,
restore-mtimes-selftest reproduces the CI failure verbatim ("expected to find:
Compiling libdep"); with it, 14/14 pass under the same forced colour.
THE NIGHTLY PROBE ASKED THE WRONG QUESTION. `cargo +nightly -V` answers "did a
cargo proxy called with +nightly exit 0", which is not "will this build have
checksum freshness": `-V` short-circuits before `-Z` is validated at all. So
the probe said "on" on a runner where the flag was not in effect, and the
suite asserted the checksum-freshness mutation family against a build that
never had it — failing in the CONTROL, where a failure reads as "the hazard is
gone" rather than "the toolchain is wrong". It now probes the capability:
`cargo +nightly -Z checksum-freshness locate-project`, the narrowest command
that actually parses the flag. It rejects the stable channel and an unknown
flag name alike, needs no network and builds nothing.
Red-proven: the old channel probe, pointed at a cargo without the flag,
reproduces the CI failure exactly.
AND THE OFF PATH DID NOT WORK EITHER. The suite's header claimed that without
checksum freshness "the test still covers the build/ and *.d families". It did
not: the final scenario backdates the source to 2001 and asserts the rebuild
is not Fresh, which is checksum-freshness-only reasoning. Under Cargo's
ordinary mtime freshness a 2001 source IS older than the artifact and Fresh is
the correct answer, so the scenario asserted a bug. It is now gated on the
mode and skipped loudly, like the control's dep-* assertion already was: 5
assertions with a nightly, 3 without.
CI installs a nightly as well as stable (nightly first, so stable stays the
default) — that hazard is the one this whole scheme exists to close, and a CI
that skips it is checking the cheap half.
|
||
|
claude
|
fb7a788c90
|
feat(cache): give same-ref jobs separate build directories via cache-lineage
CI / shellcheck + selftests (pull_request) Failing after 1m19s
A cache key names a REF. What a target directory holds is the product of a ref
and a build configuration, and emowheel builds the same ref twice on every
push — once for the host, once for wasm32, in two jobs that start together.
Keyed on the ref alone, `cargo-cache@v1` handed both the same
CARGO_TARGET_DIR, and Cargo's target-directory lock is exclusive: the second
job sat on "Blocking waiting for file lock on build directory" for the length
of the first while holding a runner capacity slot, so a third repo's queued
job waited behind a job doing nothing.
`cache-lineage` is that second dimension. It names ONE directory level under
the cache root:
<cache-root>/target-<key> no lineage (unchanged)
<cache-root>/<lineage>/target-<key> a lineage
Nesting, not a suffix on the key, and that is the whole design decision.
`prune-cache.sh`'s liveness pass classifies a directory by recomputing
`target-<cache_key(branch)>` for every branch on origin and evicting whatever
does not match — a `target-<key>-wasm32` matches nothing, so it would be
classified dead and evicted unconditionally on every run. daniel/gitdan's
host-level arbiter reads the same shape (BRANCH_DIR_RE); a suffixed name falls
out of that too, so those caches would never be reclaim candidates and a whole
lineage would go missing from the shared disk budget. Nesting leaves both
matchers reading exactly the names they already read, one level down — which
is a layout that arbiter already walks (CI_CACHE_MAX_DEPTH is 2, and its own
suite pins the depth-2 case).
Every interacting part, checked rather than assumed:
- SEED: `seed-target-dir.sh` takes the root as an argument, so a PR branch in
a lineage layers over THAT lineage's base snapshot. Asserted.
- PUBLISH: `publish-snapshot.sh` derives both ends of the swap from the root.
The publish action now takes the root from the `CARGO_CACHE_ROOT` the
consume step exported, and CHECKS its own inputs against it — a publish step
left at the default while its consume step nested would otherwise republish
a different lineage's live target dir over that lineage's snapshot, on every
push, silently. `mode: release-lock` is exempt: it releases a lock on
`$CARGO_TARGET_DIR` and never touches a root.
- WATERMARK: per target dir, so it follows the lineage. Unchanged.
- PRUNE and LIVENESS: scoped to the root they are given, so a pass in one
lineage neither evicts nor sees a sibling's caches, or the flat layout's.
Liveness keeps resolving real branch names, which is what a key suffix would
have broken.
- ci_cache_reclaim: verified by dry-run against a fixture in this layout —
all six nested and flat dirs collected as candidates, protection resolved
correctly on the nested ones, and a `.stage-` stranded inside the lineage
found by the leftover sweep.
Refused lineage names are refused at resolve time, each rejection naming the
reader that imposes it: a path separator (the arbiter's depth budget), a
Cargo profile name (its no-descend list), a `target-`/`snapshot-` prefix (this
repo's own prune globs), a hex suffix (its per-branch-dir shape), a dot prefix
(the leftover-naming contract). None of these fails visibly on its own — each
produces a working directory that some pass silently stops seeing.
Setting no lineage resolves to the cache root byte for byte, so lublub, zemyna
and emowheel's `ci` job keep the exact directories they have on the volume.
New suite `cache-root-selftest.sh` (19 assertions), red-proven against three
deliberate breakages: a `cache_root_for` that ignores the lineage, a disabled
validator, and a `verify` that never rejects a mismatch.
|
||
|
claude
|
f76789358d
|
ci(gitea): gate scripts/ with shellcheck and the selftests
This repo had eight scripts, six selftest suites and nothing that ran any of
them. It is a composite action consumed at `@v1` — a moving tag — by three
repos' CI, so a bad edit here reaches zemyna, emowheel and lublub at once and
is discovered by whichever of them builds next.
One job: `shellcheck -x --source-path=scripts scripts/*.sh`, then
`bash scripts/selftest.sh`. `-x` follows the `. cache-lib.sh` every script
sources, which is where most of the logic being checked lives; without it
shellcheck reports SC1091 and analyses each file with a hole in it.
The full suite, not `--fast`: `hardlink-clone-selftest.sh` and
`restore-mtimes-selftest.sh` are the two that drive a real Cargo rather than a
fixture, and selftest.sh's own header says `--fast` is for iterating, not for
signing off a change. So the job installs a stable toolchain. The scratch
workspaces they build use path dependencies only, so nothing reaches
crates.io, and the job references no credentials at all.
Modelled on daniel/gitdan's workflow, including the two clauses of the
draft-skip guard (`github.event_name != 'pull_request' ||
!github.event.pull_request.draft` — the first is what stops the second from
also skipping pushes to `main`, where there is no `pull_request` context) and
the per-commit concurrency group for `push`.
No `container.volumes:` entry, deliberately: the suites want a cold target dir
every run, since "did this rebuild?" is exactly what restore-mtimes-selftest
asks. This job takes no share of the shared CI cache disk budget.
Turning the gate on surfaced three pre-existing findings, all fixed here
rather than suppressed or waived:
- `restore-mtimes-selftest.sh` SC2038: `find | xargs touch` -> `-print0 | xargs -0`.
- `hardlink-clone-selftest.sh` SC2295: `${f#$base_fix/}` -> `${f#"$base_fix"/}`.
- `cache-lib.sh` SC2016: a per-line disable with the rationale. The single
quotes are load-bearing — the program is for the inner shell, where `$f` is
its loop variable, `$rc` its accumulator and `$$` its pid — so this is the
documented-false-positive case, not a stopgap.
|
||
|
claude
|
1aca90b461 |
test(seed): pin reader_lock_acquire in hardlink_clone_into (#12)
Co-authored-by: Claude <claude@gitdan.com> Co-committed-by: Claude <claude@gitdan.com> |
||
|
claude
|
df6f1b91fb |
docs(cache): producer half of the depended-upon names contract (#11)
Co-authored-by: Claude <claude@gitdan.com> Co-committed-by: Claude <claude@gitdan.com> |
||
|
claude
|
a9e9190e5a | Merge pull request 'test(seed): pin the two unguarded terms of the torn-clone condition' (#9) from test/pin-clone-guards into main | ||
|
claude
|
eb7878b822
|
docs(readme): correct the scenario census and the #5 citation
Review of #9 found the methodology section falsified by that PR and owned by nobody — the scoping that fenced it off was wrong, #8 never touches these paragraphs. Three fixes: * The PATH-stub paragraph named 8a and 8b as the seed suite's stubs. It is now four scenarios on two commands: 8a/8b/8c stub `cp` at the clone, 10 stubs it one level down at the per-file unshare, and 8d stubs `stat` — a mechanism the paragraph did not mention at all, and the only way to make an identity that could not be READ the sole witness. * The assert-which-guard-fired paragraph cited issue #5 as a live example of a surviving mutation. #5 is the issue this PR closes, so a reader following that citation landed on "removing it leaves every suite green", which is no longer true. Scenario 9's own sentence stands — the matrix confirms it survives every mutant — so it now says WHY it survives (an unreadable source leaves the staging dir at mode 000, and the unshare pass aborts the clone before the copy's exit status is consulted) instead of citing a closed issue. * Added the mutual-masking hazard the sweep turned up, since it is the general lesson rather than a fact about two particular terms: two guards that can each catch the same fault make each other unnecessary, so no fixture built around that fault pins either one. Also names scenario 8d for what it is in its own comment — a regression guard on a defensive term, not a reproduction of a reachable state. Every route to the state it constructs is closed off (a rotation hands the witness to 8a, a genuinely absent source hands it to 8c), which is the reason it is worth pinning rather than a reason to doubt it. |
||
|
claude
|
bd60b430e0
|
test(seed): pin the two unguarded terms of the torn-clone condition
hardlink_clone_into's torn-clone detection is a four-term condition, and a mutation sweep found two of the four unpinned: removing either `[ "$cp_rc" -eq 0 ]` (issue #5) or `[ "$i_before" != missing ]` left all five suites green. They were unpinned for the same reason — they mask each other. A source that vanishes mid-clone reads as `missing` at both ends AND fails `cp -al`, so with both terms present either one catches it and neither is individually necessary. Isolating them needs a state each term alone can see: 8c cp reports failure over a tree that is in fact whole. Neither inference sees anything — 0 entries short, one unchanged inode — so the exit status is the only witness. Forced with the PATH stub 8a/8b already use, on the consumer's own top-level clone. 8d both identity reads fail while the copy succeeds. `_dir_inode` folds every stat failure into the string `missing`, so two failed reads compare equal TO EACH OTHER; without the sentinel term the tree is published on the strength of two errors. Stubs `stat` narrowly — only the `%i` reads of this clone's own source — because taking the source away would fail `cp -al` too and pin 8c's property over again. The sweep also found `_unshare_files`'s xargs status unpinned, which is the guard that stops a staging tree whose dep-info files still point at the SOURCE's inodes from being renamed into place — not a tear, so all four clone checks pass it, and exactly the silent cross-branch stale-reuse the scheme exists to prevent. Scenario 10 pins it by refusing the dep-info unshares and asserting the clone discards rather than publishes. assert_tear gains an `unreadable` identity expectation, and its `same` case now demands a READ identity rather than two equal strings — `missing` equals `missing`, which is the exact confusion 8d exists to pin. Each of the five pinning scenarios was run in isolation against each mutation; the result is a clean diagonal, so every scenario fails only for its own term. Closes #5 |
||
|
claude
|
e3869c5920
|
docs(publish): address review nits on the contract block
Four corrections from the review of #8, all in the files this PR already touches: - A local signal on the line that creates .publish-new-. The other four shapes each got a note at their producing line, which is the whole premise of #7 — someone renaming TMP_DST reads its own comment block and would never see the contract note 45 lines up at OLD. - '.publish-old- is milder' understated it. Milder is true; bounded is not. A key whose branch is merged, deleted or renamed is never published again, so its rotated generation stays until something outside this repo takes it — which is the case gitdan#30 itself makes, two lines away. - The drift check now says the prefix constants ci-cache-reclaim.sh DECLARES, not the ones it enumerates. Those are different numbers: collect_entries() globs .stage- and .evicting- only, because .reading- is read and never swept. Only the declared reading makes the five-against-five count work, and a countability check that needs a coin flip to count is not one. - publish-snapshot.sh's own header had two stale names ten lines above the stale pointer this PR fixes: step 1 staged at .stage-<tag> (that is hardlink_clone_into's inner path; the staged snapshot is .publish-new-<tag>) and step 2 named .publish-old-<tag> without the key. Pre-existing and outside both ACs, but #6's thesis is that a plausible-looking wrong name is the worst kind, and these are in the file the PR is about. Comments and docs only. With comments and blank lines stripped, all three scripts hash identically to origin/main. |
||
|
claude
|
0118c28f01
|
docs(cache): state the .publish-* gap as a ticket, not a present state
The contract block asserted that neither side reclaims .publish-new- 'today'. True when written and about to stop being true: gitdan#30 tracks adding both .publish-* prefixes to the arbiter's enumeration, and a sibling track is landing it this round. A comment that dates itself against a merge in flight is worse than no comment. Rewords all four sites (cache-lib.sh, publish-snapshot.sh, and README's table row and prose) to reference gitdan#30 and keep the mechanism that made the shape worth catching — .publish-new- is tagged per job per run exactly as .stage- is — rather than the arbiter's momentary contents. The rule itself is unchanged; it is the durable part, and it is what found this. Adds the counting check while there: the five names here and the prefixes ci-cache-reclaim.sh enumerates are meant to be the same length, so a mismatch is the cheapest signal that one side gained a shape without telling the other. |
||
|
claude
|
0cf6cc5973
|
docs(cache): write down the producer half of the leftover naming contract
daniel/gitdan's host-level arbiter (scripts/ci-cache-reclaim.sh) reclaims the dot-prefixed trees this repo's scripts strand inside the cache volumes it scans, and reads this repo's reader markers to decide whether one is still live. That arrangement was documented only on the consuming side: a contributor here could add or rename a dot-prefixed shape with no local signal that anything outside the repo depended on the spelling, and the arbiter enumerates by explicit prefix — deliberately, so it never sees a .reading-* marker as a candidate — which makes an unannounced shape invisible to it rather than conservatively handled. Adds the producing side's half at the sites someone changing a name will actually be looking at, pointing at gitdan's LEFTOVER NAMING CONTRACT block as canonical rather than restating it: - cache-lib.sh gains a header block naming every shape this repo creates under a cache root, its producing function, and how each strands; plus the rule that adding a shape obliges the same matching prefix over there as renaming one does. - Site notes at .stage-'s and .reading-'s producing lines, and at .evicting-'s in prune-cache.sh. - publish-snapshot.sh's .publish-old- / .publish-new- pair is documented as the shapes that are NOT in the arbiter's list today, with .publish-new- called out as the one that strands exactly as .stage- does and that neither side reclaims. - The staleness direction: CACHE_READ_STALE_SECONDS and STALE_LOCK_SECONDS are mirrored there and the mirrors must be >= ours, because raising ours alone makes the arbiter delete a tree under an in-flight clone (its minimum-age guard does not back-stop that case). Lowering ours is safe in any order. - README gains a short section a newcomer meets before adding a scratch directory under a cache root, cross-linked from the cache-layout block. Comments and docs only; no behaviour change. Closes #7 |
||
|
claude
|
1f42064d20
|
docs(publish): point the torn-clone note at the suite that reproduces it
publish-snapshot.sh's header described the silent-truncation failure mode as the one "this script's own selftest (scenario 8)" reproduces. Wrong twice: the scenario that reproduces a truncated clone is in seed-target-dir-selftest.sh, and since #4 split the old scenario 8 into 8a and 8b it is 8b. publish-snapshot-selftest.sh does have a scenario 8 — the abandoned-marker case — so the pointer landed on a real scenario with a plausible number that tests something else. Names the suite as well as the number, and says what the local scenario 8 actually is so the collision cannot re-form. Closes #6 |
||
|
claude
|
5551e994da
|
docs(readme): describe how the concurrency scenarios actually work now
README's Development section stated the repo's methodology for writing concurrency scenarios as "gate the interfering step on observed progress of the step it interferes with, so the window is hit deterministically". That described the progress poll scenario 8 used, which this branch removes — and the property it claims is precisely what issue #3 records as false: observing that a walk has started says nothing about where it will be when the interference lands. Left standing it would tell the next contributor to build the next scenario the way this one had to be rewritten. Replaced with what the suites do: stub, on PATH, a command the code under test calls at a known point, so placement is a fact rather than a scheduling outcome; assert the stub fired; and assert which guard caught the fault where more than one could. Also: the seed suite's table row now names both tear modes, and publish-snapshot-selftest.sh's cross-reference points at 8a and 8b rather than a scenario 8 that no longer exists. (publish-snapshot.sh's similar mislabel predates this branch and is left alone.) The stub directory and the real-cp lookup move up next to seed_with_stub, so 8b no longer depends on setup buried in 8a's block and either scenario can be run or mutated alone. |
||
|
claude
|
dc0fef71f8
|
test(seed): force scenario 8's interleaving instead of racing for it
Scenario 8 started a real publisher once a consumer's clone was observed
past a fraction of the tree, then asserted the consumer ended up holding
the NEW generation. That premise is racy: when the publish lands after the
consumer's last identity read, the consumer legitimately completes a whole,
consistent generation 1 — and the assertion called that "the seeded tree is
truncated". The one gate this repo has was failing intermittently in the
most alarming direction available.
Replaced with two scenarios that place the interference deterministically,
the way prune-cache-selftest.sh scenario 12 places a marker inside the
check-to-unlink window: the consumer's own `cp` is stubbed, so whatever the
stub does happens strictly after hardlink_clone_into read the source's
entry count and inode and strictly before it reads that inode again.
8a the stub runs the real copy, then the real publish-snapshot.sh
concurrently, and returns only once the swap is on disk. The copy
succeeds and the staging tree is whole, so the source's identity is
the only witness.
8b the stub renames a subtree out of the source's listing before the
walk starts and back before the retry. The copy exits 0 and the
source's identity never changes, so the entry count is the only
witness — the silent truncation this whole guard exists for.
Each asserts WHICH of the clone's checks reported the tear (assert_tear),
so neither stays green if the check it exercises is deleted and another
happens to fire in its place.
Closes #3
|
||
|
claude
|
b0d63c807b
|
style(prune-cache): rewrap the settle-window comment; drop an overclaim
Review nits on #2, both cosmetic. The sentence added last round left its paragraph at 127 characters in a file that otherwise wraps comments at 78-79 — the rewrap after the insert simply did not happen. `awk 'length($0)>80'` over both files now reports one comment line, the pre-existing 93-character usage string at :4. Scenario 14's header called its fixture "the only shape that can tell the two timestamps apart". The property it needs is old mtime with fresh ctime; an old directory renamed a moment ago is the production instance of that, not the only construction of it. "Production's shape" was already carrying the argument. No behaviour change and no assertion change. |
||
|
claude
|
dcd73dd82d
|
test(prune-cache): build scenario 14's aside the way the pass builds one
Review finding on #2. Swapping the settle window's `stat -c %Z` for `%Y` — the exact substitution the comment beside it calls the wrong signal — left the suite green at 37/37. The scenario built its aside with `mkdir`, so the fixture's mtime was also ~now and the two timestamps agreed; a fixture whose clocks agree cannot tell them apart. Every real aside is the opposite shape: a cache last written days ago, renamed a moment ago. Under `%Y` the window would never fire for one, the sweeper would silently return to reclaiming asides another pass is still deciding about, and nothing would say so — the mechanism guarded by a comment again, which is what the previous finding was about. So the fixture is now built the way the pass builds one: an old directory `mv`d into the aside name. `%Z` -> `%Y` now fails scenario 14, as does deleting the guard outright. Also sharpens why the PID alternative was rejected: the `$$` in an aside's name was that pass's PID inside its own job container, so testing it from another one is not unreliable, it is meaningless. No change to prune-cache.sh's behaviour; the assertions are untouched. |
||
|
claude
|
65f0782233
|
fix(prune-cache): stop the aside sweeper depending on timing it cannot see
Review findings on #2. The first is the one that mattered: the sweeper this PR added had the shape the PR exists to remove. Pass A renames a candidate aside; pass B's sweep sees an aside with no readers and reclaims it; A then finds a reader and restores. `rm -rf` traverses fd-relative, so the rename does not stop it and A can republish a half-emptied tree under a live cache name. `capacity: 1` bounds it today, which is exactly the kind of reason this PR was written to stop relying on. The unlink itself was never the problem — the ordering proof covers it under any interleaving, since the aside name only exists after the evicting pass's rename. What was missing is that an aside with no readers is indistinguishable from one a pass has just created and not yet decided about. The sweeper now leaves an aside alone until it has settled (EVICTION_ASIDE_SETTLE_SECONDS, default 60), which separates the two without having to identify the pass that created it — a PID is meaningless across the job containers these passes run in, and recycles. Read from ctime, not mtime: rename(2) updates the first and leaves the second at whenever the cache was last written, which is the signal list_by_lru wants and the wrong one here. That is a bound, not a construction, and both the code comment and the README now say which of the two properties is which instead of asserting the broader one. Also from the review: a pass that declined every dead cache it found no longer signs off with "no dead-branch caches found", and evict_dir no longer promises a later reclamation of an aside that is already gone. Scenario 14 covers the settle window against the script's own default, with nothing faked — the directory really was set aside a moment ago. Scenario 4 gains the summary assertion. 31 -> 37 assertions; each new gate verified red by defeating it alone in a scratch copy. |
||
|
claude
|
88ab063b64
|
fix(prune-cache): close the reader-marker check-then-delete window
Both eviction sites checked for a consumer's `.reading-` marker and then, seconds later — `usage_gb` runs `du -sk` over a multi-GB tree between the two — unlinked the directory. A consumer that started a clone inside that gap had its source removed mid-walk, which `cp -al` does not report: a subtree unlinked before its parent is listed is silently omitted. Unreachable today, and only by policy: snapshots belong to protected refs and protected refs never reach the marker check. `cargo-cache` and `cargo-cache-publish` take that ref list as two independent inputs, so a workflow listing a publisher in one and not the other arms this with no code change at all. Closed structurally, with publish-snapshot.sh's rotation rather than a new mechanism: the candidate is renamed aside and only then re-examined, so the scan the unlink rests on happens strictly after the rename. A consumer that resolved the directory published its marker before that scan and cannot be missed; one arriving after cannot resolve the path and starts cold, the same degrade the publisher's swap window already produces. Renaming disturbs no clone in flight — no entry is unlinked and the inode is unchanged — so a declined eviction costs a deferred eviction and nothing else. A reprieved cache is put back under its own name; one whose name a concurrent seed has retaken is left aside and swept by a later pass once its readers drain, since nothing else globs a dotted name. prune-cache-selftest gains three scenarios (21 -> 31 assertions). Scenario 12 is the one that bites: the `du` the pass runs on its candidate publishes the marker, placing it strictly after the check and strictly before the unlink. Against check-then-delete, 1-11 pass and 12 fails; breaking only the second look and leaving the rename fails it too. |
||
 claudeandClaude Opus 5
|
3b2dec6a50
|
test(cargo-cache): cover the stale-reader-marker sweep and its bound
The staleness path decides whether a publisher may reclaim disk, so getting it
wrong means an abandoned marker pins a snapshot generation forever — the exact
outcome the bound exists to prevent. It was previously covered only by
analogy to prune-cache.sh's .ci-lock-* staleness, which is not the bar.
New publish-snapshot-selftest.sh scenario 8 asserts both directions against
the SAME backdated marker, which is what separates "honours the bound" from
"ignores anything that looks old":
* under CACHE_READ_STALE_SECONDS=86400 a three-hour-old marker is left alone
and still defers reclamation, exactly as a live reader does;
* under the 7200s default the same marker is swept, reported as swept, and the
generation it was pinning — plus the one deferred by the first half — is
reclaimed.
Backdated with `touch -d`, not slept for; the suite stays fast.
Red-proven by mutation rather than against the pre-fix scripts, since the
whole mechanism is new there and "it does not exist yet" proves nothing about
the threshold logic. Mutating live_reader_count's bound test to `true` (never
sweep) fails scenario 8:
ASSERTION FAILED: the stale marker was not reported as swept
and to `false` (sweep everything, bound ignored) fails scenario 6 instead,
which is the right blast radius — ignoring the bound means unlinking under a
LIVE reader:
ASSERTION FAILED: the previous generation was unlinked while a reader
still held it
Also documents the entry-count check's measured cost in the README: on ext4
with a warm cache over 78,554 entries, 44 ms per metadata walk against 3,126 ms
for the `cp -al` it guards — about 2.8%. Not a perf-claiming change; the number
is there so the next reader does not have to wonder.
The reviewer's other nit — scenarios 8/9 of the seed suite exercising
publish-snapshot.sh's interlock — was already covered by the cross-reference
in this file's header, so no move.
Verification: `bash scripts/selftest.sh` — 5 suites, exit 0, 88 assertions
(82 before this commit, 63 at baseline). shellcheck: no new findings.
Refs: daniel/gitdan#11, zemyna#911
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sqh2vscfzisk83VuPVQX9L
|
||
|
claudeandClaude Opus 5
|
719475831b
|
fix(cargo-cache): scope the safety claim to what the code actually prevents
Follow-up to |
||
|
claudeandClaude Opus 5
|
f57e2a6013
|
fix(cargo-cache): close the seed-vs-republish race the design claimed to close
The shared action's justification over zemyna's and emowheel's schemes was that hardlink-cloning from a published snapshot closes gitdan #911 "by construction, not by the single job slot". Review disproved that. This makes the claim true, and corrects the README where it could only be bounded. Finding 1 (verdict-level) — silent partial clone ------------------------------------------------ `hardlink_clone_into` ran `cp -al` with no exit-status check, and both call sites invoked it as a condition, which suppresses `set -e` for the whole call. A publisher's `rm -rf` of the generation it rotated away therefore unlinked entries beneath an in-flight consumer walk, and the truncated tree was renamed into place and reported as success. Both layers are fixed: * The consumer verifies its own clone. Every attempt checks `cp -al`'s status explicitly, the source directory's inode before and after (a wholesale replacement mid-walk splices two generations), and the entry count — the only signal for a subtree unlinked before its parent was listed, since `cp -al` reports no error for one it never saw. Any failure discards the staging tree and retries; exhausting the attempts returns a distinct status 2 and fails the job rather than seeding a partial cache. `unshare_subtree` / `_unshare_files` now propagate failure too — a swallowed unshare leaves the clone aliasing its source, the exact corruption that step exists to prevent. * The publisher does not unlink under a reader. A consumer publishes a `.reading-<snapshot>-<tag>` marker before it resolves the snapshot path; the publisher scans for markers after its first rename. A consumer holding the old generation therefore published its marker before that scan and cannot be missed; one arriving after the scan necessarily resolves to the new generation. The publisher waits for readers to drain and, on timeout, DEFERS reclamation rather than forcing it — the old generation is left as `.publish-old-<key>-<tag>` and swept by a later publish. So correctness is closed by construction; disk reclamation is bounded, not immediate. The residual is capped at one deferred generation per publisher ref, and the README now says exactly that instead of the disproved claim. Finding 2 — restore-mtimes.sh ran with no errexit ------------------------------------------------- `set -euo pipefail` was glued to the end of a comment (`# soundness.set -euo pipefail`), so it was entirely commented out: a partial failure of the `git log | awk` pipeline would have produced wrong mtimes across the whole restore instead of failing loudly. Moved to its own line. Audited every other script for the same defect — this was the only instance. Independent confirmation: shellcheck's two SC2164 warnings on this file's `cd "$repo_root"` disappear now that errexit is actually in effect. Finding 3 — lock-acquire window ------------------------------- A just-seeded directory was unlocked until a later action step, so a concurrent job's prune pass could evict it. `seed-target-dir.sh` now takes an optional lock-id and writes the lock marker on every path out of the script, including into the staging tree before its rename, so the directory carries a lock the instant it appears under its final name. The action's acquire step stays (it is idempotent and stamps the LRU marker). Also hardened `prune-cache.sh` to treat a directory with live reader markers as locked. Today no reachable configuration prunes a snapshot — only protected refs publish them and protected refs are excluded from every pass — so this is redundant by policy; it is here so that stops being the reason it is safe. Verification ------------ New selftest scenario 8 races a real seed against a real publish rotation, gating the rotation on the seed's *observed* clone progress so the window is hit deterministically rather than on a fast machine's coin flip. Red-proven against the unguarded scripts, three consecutive runs: ASSERTION FAILED: the seeded tree is truncated: 15443 entries against the snapshot's 493 (was 48805 before the rotation) (15443 / 16986 / 16498) Green after the fix, six consecutive runs, catching the clone mid-walk at ~10.5k of 48805 entries each time. Scenario 9 covers deferred reclamation and its later sweep; scenario 10 covers an unreadable source failing loudly. `bash scripts/selftest.sh`: 5 suites, exit 0, 75 assertions (was 63). shellcheck over `scripts/`: no new findings, two SC2164 warnings resolved. Docs: README's republish-safety paragraph replaced with what the code now guarantees, including the bounded disk residual stated explicitly; new `read-grace-seconds` / `reader-stale-seconds` inputs documented in the `cargo-cache-publish` table; the selftest table names the new race. Refs: daniel/gitdan#11, zemyna#911 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sqh2vscfzisk83VuPVQX9L |
||
|
claudeandClaude Opus 5
|
248af3061e
|
feat(cargo-cache): hardlink-clone a per-ref Cargo cache from a published snapshot
Replaces the phase-0 resolution probe with the real actions, merging the two independent per-branch Cargo cache implementations on this forge into the design neither of them had. ## The merge - zemyna seeds a PR branch by `cp -al` hardlink clone (near-free: cost scales with inode count, not bytes) from the base branch's LIVE target dir — a torn read waiting for a second job slot (its own #911). - emowheel seeds from a PUBLISHED IMMUTABLE SNAPSHOT (no race by construction) but with `cp -a`, duplicating ~35 GB per branch. This ships hardlink-clone FROM a published snapshot: zemyna's cost profile, emowheel's soundness, and #911 closed structurally rather than by the runner happening to have one execution slot. ## The bug both implementations have A build inside a `cp -al` clone DOES mutate the directory it was cloned from. Cargo replaces real artifacts, but writes its metadata — and build scripts write their OUT_DIR — with a plain truncating write, straight through the shared inode. Measured set: `.fingerprint/<unit>/dep-<target>` (under CARGO_UNSTABLE_CHECKSUM_FRESHNESS), `build/<pkg>/{output,root-output,out/**}`, `deps/*.d` and `<profile>/*.d`. The checksum-freshness case is a wrong answer, not a slow build: a PR clone rewrites the base's dep-info to describe the PR's sources while the base's cache still holds the artifact built from the base's; once the PR merges, the base's next run finds the checksums match, reports `Fresh`, and links a binary built from the pre-merge code. Reproduced end to end. Fix: hardlink the artifacts (the GB), real-copy the metadata (the MB) — about 3.7% of a 6.9 GB Bevy target dir, against 100% for a full copy. ## Contents - `cargo-cache/action.yml` — consume: resolve keys, seed from the base's snapshot via staging + one atomic rename, strip Cargo lock files, unshare the mutable paths, restore mtimes from git history, lock, prune. - `cargo-cache-publish/action.yml` — publish: record the build watermark, atomically republish the snapshot on a protected branch, release the lock (`mode: release-lock` for the `if: always()` step). - `scripts/` — all logic, so it is testable standalone; the YAML is wiring. - `scripts/*selftest.sh` + `selftest.sh` — five suites, 63 assertions, every fix paired with a control that reproduces the bug. All green locally. Eviction merges emowheel's liveness pass (dead branches pruned unconditionally, not gated on disk pressure) with LRU-under-pressure, but inverts the order within the pressure pass: `target-*` before `snapshot-*`, because a snapshot is hardlinked to everything cloned from it, so evicting one frees almost no real bytes while costing every future PR its warm start. restore-mtimes.sh is ported from emowheel (the watermark variant, which closes the merge hazard zemyna's copy still has) with its provenance de-projectised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sqh2vscfzisk83VuPVQX9L |