Skip to content

Pack the bare caches' refs on the sweep #470

Description

@blooop

Work

Pack the bare caches' refs on the detached sweep, per the decision on #465.

Deliberately not folded into #464, which touches the same two functions: that ticket fixes a permanent fetch wedge and should land on its own timing, and coupling a bug fix to an enhancement delays the fix. Either can land first.

  1. Git::pack_refs(bare) in clients/git.rs — git pack-refs --all, cwd the bare, ABOUT_ONE_REPO timeout. No --auto: it is a no-op on the files backend, measured declining at both 5000-loose-versus-1-packed and 100-versus-5001, so it cannot be leaned on.
  2. One call in RepositoryManager::fetch_repo, after the fetch's refusal check and before the last_fetched stamp. A packing refusal must not become a FetchRepoError: the fetch happened and the stamp must land. Give it its own arm reported as a CacheNotice, not a let _ = — this module's discipline is that every warning carries that line's data.
  3. Tests: the ref set is identical before and after, the packed-refs count rises, the argv reaches the spawn as pack-refs --all with the bare as cwd, and a packing refusal still returns Ok with the stamp written.
  4. A contract row in docs/cleanup.md. No em or en dashes (test_docs_prose.py).

Two things worth carrying into the code comments, because they are why this is here rather than on --prune. fetch_all has exactly one production caller (lazy_fetch via sweep_repo_fetches), so "after the fetch" is the detached sweep by construction, in a lock scope it already holds. And packing pays a second time in probe latency: #446's per-worktree proof is rev-list --not --all on the bare, which today reads thousands of loose ref files per probe instead of one packed-refs.

Honest cost to state rather than hide: the sweep's lock is the one a launch blocks on, so the 0.36s worst case measured lands inside a scope already bounded at 300s.

Activity

  1. self-assigned this
    on Aug 25, 2026
  2. blooop commented on Aug 25, 2026

    @blooop
    OwnerAuthor

    breadcrumb: claimed. main is at a5c0049, carrying #471 (widened unpushed probe) and #464 (forced tag refspec). #464 matters here: the sweep now prunes tags as well as heads, so pack-refs --all runs against a ref set that has just shrunk, and the loose-ref count #453 measured at 5286 is no longer the same population.

  3. blooop commented on Aug 25, 2026

    @blooop
    OwnerAuthor

    breadcrumb: build claimed, seams as the ticket names them: the argv seam on the git client (whole argv, cwd, timeout, through the fake runner) and the flow seam on the fetch (real git against a real bare, asserting the ref set is identical and the loose files are gone). Third seam is the refusal arm: a packing failure reaches the caller as a notice, never as a failed fetch.

  4. blooop commented on Aug 25, 2026

    @blooop
    OwnerAuthor

    breadcrumb: the loose refs do not come from the clone. A git clone --bare arrives packed (601 refs, zero loose files, one 33 KB packed-refs on 2.51.1); they are made by fetching, one file per ref the fetch updates, shadowing a packed-refs that keeps the pre-fetch shas. So the placement argument is stronger than "the sweep is a convenient home": the sweep is the only thing in dl that makes them in quantity, and it is the only caller of the broad refspec.

  5. blooop commented on Aug 25, 2026

    @blooop
    OwnerAuthor

    breadcrumb: re-measured after #464, since the ref population it prunes is not #453 's. Fetch that moved 601 refs: 601 loose files, 2404 KiB of blocks, against 33787 B packed. Probe (rev-list --count HEAD --not --all) over 551 refs with 301 loose: 5.3 ms to 5.5 ms loose, 2.7 ms to 2.9 ms packed. The pack itself, 23 ms. Ref sets byte-identical across the pack.

  6. blooop commented on Aug 25, 2026

    @blooop
    OwnerAuthor

    breadcrumb (principle 1): a pack that refuses is reported and never returned as a failed fetch, and the freshness stamp still lands. The fetch happened and the cache is fresh, so calling it a fetch failure is a lie about the network; withholding the stamp is the real harm, since it re-fetches the whole repository every interval forever over a representation change that did not come off. Packing is the optional half of that function and the fetch is the point of it.

  7. blooop commented on Aug 25, 2026

    @blooop
    OwnerAuthor

    breadcrumb: packed-versus-loose deletion checked, and it is a no-difference finding. 100 refs deleted upstream while packed: --prune removed all 100, rewrote packed-refs, created no loose files, left the survivors alone. A ref that was loose over a stale packed line and then retracted upstream loses both entries, so nothing is resurrected at the old sha. Packing therefore changes nothing about what a later prune may delete or keep; the only difference is cost, and it falls on the prune, which rewrites the whole file where a loose removal unlinks one path.

  8. blooop commented on Aug 25, 2026

    @blooop
    OwnerAuthor

    breadcrumb: built and pushed as PR #478. One pack-refs --all after a successful fetch, its own refusal arm as CacheNotice::RefsNotPacked, and a contract row in the cleanup page. Red first against a real bare, on the two loose refs the sweep's own fetch had just written.

  9. blooop commented on Aug 25, 2026

    @blooop
    OwnerAuthor

    breadcrumb: PR #478 green at ec9619aa9be0231fdeae2d045a73cd5e3beddd07, MERGEABLE, gate/ci/rust/e2e/public-api/packaging/prek/review/coverage all success. Ready for review.

  10. blooop commented on Aug 25, 2026

    @blooop
    OwnerAuthor

    breadcrumb: gate passed. PR #478, head ec9619aa9be0231fdeae2d045a73cd5e3beddd07, 15 checks green and MERGEABLE. Handing to a fresh reviewer, pointed at two things: the CacheNotice arm that lets a refused pack keep last_fetched (a principle-1 call on a path where the obvious reading is the wrong one), and the claim that packing changes nothing a later prune may delete.

  11. blooop commented on Aug 25, 2026

    @blooop
    OwnerAuthor

    breadcrumb (review): Request changes on PR #478 at ec9619aa9be0231fdeae2d045a73cd5e3beddd07 — #478 (review). Standards passes clean. Two blocking Spec findings, both single sentences of prose, no code defect: docs/cleanup.md:372-375 and repo_manager.rs:1417-1420 state #446's bare-side reachability probe in the present tense when it is unbuilt (the shipped probe is log --all --not --remotes on the clone, #471); and docs/cleanup.md:380 promises a stderr line for a refused pack that no user can ever see, since the sweep's only production path is a detached child with stderr(Stdio::null()). Re-verified by running rather than reading: the packed-versus-loose deletion finding holds for heads and tags including the loose-over-stale-packed resurrection case (identical outcomes packed and unpacked, nothing resurrected); pack-refs --all is safe against concurrent readers (636 clones + fsck, zero anomalies); --all is pinned by three tests, proved by mutation; and Sourcery's item-3 finding is refuted by construction (a pack that drops a ref fails repo_manager.rs:3799). Measured that a permanently refusing pack plateaus at one block per ref rather than accumulating, so principle 2 is not newly engaged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions