eb3f0bd09b56c8489a4d5690938482f6600b15fb
6
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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 |
||
 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 |