Skip to content

The bare cache becomes the repo's LFS store; workspaces hardlink from it #163

Closed

Description

@blooop

Question

_materialize_lfs (workspace_clone.py:150-170) runs git lfs pull origin per workspace, so every workspace of an LFS repo pays a full network download and 2× the payload on disk (.git/lfs/objects plus the worktree copy).

Per the #154 decision, make <bare>/lfs the repo's shared LFS store. Inside _materialize_lfs (new bare_path argument, already available as bare_repo_path in _prepare_workspace):

  1. Run git -c lfs.fetchrecentrefsdays=0 -c lfs.fetchrecentcommitsdays=0 lfs fetch origin <ref> with the bare as cwd.
  2. Then git lfs pull "file://<bare_path>" in the workspace — measured to hardlink the objects (same inodes, zero extra disk) and to succeed with origin pointed at a dead URL (zero network).
  3. Keep today's git lfs pull origin only as a second phase, entered when _has_lfs_pointers still holds.

Keep the existing RuntimeError-and-retry contract and the deliberate non-gating on is_new_workspace (:350-352).

Nothing host-specific may be persisted into the clone: no lfs.storage in .git/config, no extra remote. The clone directory is bind-mounted into the devcontainer (devpod up <clone path>, dl.py:1136) and .bare is not, so a persisted host path would break every in-container git checkout of an LFS repo. Note also that -c lfs.storage was measured to break against local-path remotes, because GIT_CONFIG_PARAMETERS is inherited by the remote-side git-lfs child.

Tests (all offline, local fixture remote):

  • integration — two workspaces of an LFS repo, remote broken before the second; the second materializes real content and its .git/lfs/objects inodes equal the bare's.
  • integration — after materialization, git config --local --get lfs.storage is empty and git remote is just origin.
  • unit (fake subprocess.run argv, as in test/test_workspace_clone.py:296-388) — the bare-side fetch runs in the bare cwd with the recent-refs knobs and a named ref; git lfs pull origin is issued only when the cache phase leaves pointers; a repo with no LFS attributes issues no git lfs command at all.

Add git-lfs to the pixi test-feature dependencies — it is absent today, so these tests would silently skip.

Sequencing: land after PR #148 / #142 (same file, inherits its attributes gate) and after #150 (adjacent lines at :298-353). Touch nothing in repo_manager.py's fetch flow.

Decided on #154.


Amendment (2026-08-09, forced by the final review of PR #148). Wherever this ticket says the work "inherits its attributes gate", that wording is stale — the shipped gate does not read gitattributes at all, so there is no attributes gate to inherit, and any test planned against one would be testing a mechanism that no longer exists.

What actually shipped on #142: the predicate asks can any tracked file here be an unmaterialized pointer, enumerating the true HEAD ∪ index union with a single git ls-files -z --with-tree=HEAD, then reading the 23-byte pointer prefix. It never consults .gitattributes, core.attributesFile, /etc/gitattributes, or .git/info/attributes — three earlier attempts to reason from declarations were each falsified against real git, which is why the declaration-reading code was deleted rather than corrected.

Three consequences bind this ticket:

  • It answers correctly in a linked worktree, because it no longer reads .git/ as a directory. That was previously called out as a hazard for this ticket; it is now a property you can rely on.
  • The soundness claim is "HEAD or index", not "tracked". The stronger phrasing was proven false (git ls-files reads the index; git lfs ls-files reads HEAD ∪ index) and is corrected in the code, CHANGELOG and PR body. Do not restate the stronger form.
  • Re-examine the predicate under this ticket's own design. Make clone object sharing deliberate and extend it to LFS #154 settled that LFS is fetched once into <bare>/lfs and materialized per workspace with git lfs pull "file://<bare>". The final reviewer flagged that the union claim is empirically true but stated as definitional — --with-tree is documented only for --error-unmatch, and git-lfs never documents reading the index. Under a local-store materialization path, satisfy yourself the predicate still holds rather than assuming it transfers.

Also inherited from that review, and worth pinning here rather than rediscovering: stub_git in the unit tests is blind to --with-tree=HEAD — only the real-git integration tests kill that mutant. Any test this ticket adds against the union behaviour must use real git, or it will pass for the wrong reason.

Activity

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

    @blooop
    OwnerAuthor

    breadcrumb: claimed, and the ticket's three measured premises re-verified on this
    machine before writing anything (git 2.5x, git-lfs 3.7.1, ext4), against a local
    fixture remote built exactly the way the suite builds one:

    • a bare clone of an LFS repo arrives with an empty LFS store, so the bare-side
      fetch is genuinely the thing that fills it;
    • git lfs fetch origin <ref> with the bare as cwd puts the object in <bare>/lfs/objects;
    • git lfs pull "file://<bare>" in a skip-smudge workspace materializes the real
      content with origin pointed at a dead URL — zero network — and the workspace's
      .git/lfs/objects entry is the same inode and same device as the bare's, not a
      copy. Afterwards git config --local --get lfs.storage is empty and git remote
      is just origin; the only thing written to the clone's config is
      lfs.repositoryformatversion, which is not host-specific.

    Also measured, because the failure arms decide the shape: an empty bare store makes
    the file:// pull exit 2 with "remote missing object", and a dead origin makes both
    the bare-side fetch and git lfs pull origin exit 2. So neither cache-phase step can
    be fatal — each degrades to the origin pull, which keeps today's RuntimeError-and-retry
    contract as the only thing that raises.

    Seams I will test at, all offline:

    1. integration, real git and real git-lfs — two workspaces of one LFS repo with the
      fixture remote deleted before the second; the second materializes real content
      and its LFS objects are (st_dev, st_ino)-identical to the bare's.
    2. integration — after materialization the clone persists nothing host-specific:
      no lfs.storage, and git remote is just origin still pointing at the real
      remote rather than at the file:// bare. This is the bind-mount hazard the ticket
      names, and it is the assertion that goes red on the -c lfs.storage shortcut.
    3. unit, at the subprocess.run argv boundary — the bare-side fetch runs in the
      bare's cwd with both recent-refs knobs and a named ref; git lfs pull origin is
      issued only when the cache phase leaves pointers behind, and is not issued when
      the cache phase materializes them; a repo with no pointers issues no git-lfs
      command at all.

    On the ticket's amendment: I am not testing an attributes gate, and the union claim
    is left exactly as #142 shipped it. Under this design the predicate still holds for
    the reason the amendment asks me to check rather than assume — the cache phase changes
    only where bytes come from, and the predicate reads bytes already on disk in the
    worktree, so it is indifferent to the source. It is also now asked twice per
    materialization, and the second ask is what decides whether the network is touched at
    all; the integration tests above exercise both asks against real git.

  3. blooop commented on Aug 14, 2026

    @blooop
    OwnerAuthor

    breadcrumb: built and open as PR #209.

    The slice: _materialize_lfs gained the bare and the ref and runs in two phases —
    fill <bare>/lfs with the bare as cwd and the recent-refs knobs zeroed, materialize
    the workspace with git lfs pull "file://<bare>", and only then, if a pointer
    survived, today's git lfs pull origin unchanged. The second phase is entered on
    surviving pointers rather than on the cache commands' exit codes, because git-lfs
    exits zero having fetched only some objects; neither cache step can fail a launch.

    Two things worth carrying forward past this ticket:

    • The host-specific guard is invisible without its test. Adding a cache remote
      to the clone instead of naming it as a file:// argument leaves every disk and
      network assertion green — the sharing works, the content is right. Only the
      one-remote assertion goes red. The breakage it stands in for happens nowhere near
      here: it is an in-container git checkout, in a directory .bare is not mounted
      into. That guard is the whole of the protection.
    • On the amendment's question, checked rather than assumed. The predicate holds
      unchanged under this design and for a reason that survives it: it reads pointer
      bytes already on disk in the worktree, so it is indifferent to where those bytes
      came from. What did change is that it is now asked twice per materialization,
      and the second ask is what decides whether the network is touched at all. Both asks
      run against real git in the new integration tests, so the amendment's warning about
      stub_git being blind to --with-tree=HEAD is respected — nothing new leans on the
      fake for the union behaviour.

    Ten mutations checked, all caught; the full table is in the PR body. pixi run ci
    exits 0, pylint 10.00/10, 1624 passed. One declared deviation: pixi.lock moved v6→v7
    because that is what the pinned pixi writes, and the package set is provably identical
    apart from the added git-lfs.

    Stopping here — review is a separate stage in fresh context.

  4. blooop commented on Aug 14, 2026

    @blooop
    OwnerAuthor

    wf-review verdict on #209: Approve — both axes clean; Spec axis independently reproduced the mutation split and union-path behavior with real git-lfs; pixi.lock v6→v7 rewrite verified mechanically by both axes (package set identical + git-lfs only). Non-blocking riders in the review (two stale comments at workspace_clone.py:590/:625, fixture naming). Merging.

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