Skip to content

Fix git-lfs file smudging in zone source accessors#13

Merged
lilyinstarlight merged 7 commits intotecnix-gcsfrom
vw/fix-git-lfs
Mar 20, 2026
Merged

Fix git-lfs file smudging in zone source accessors#13
lilyinstarlight merged 7 commits intotecnix-gcsfrom
vw/fix-git-lfs

Conversation

@vwassan
Copy link
Copy Markdown

@vwassan vwassan commented Mar 4, 2026

Fix git-lfs file smudging in zone source accessors

Problem

Zone source accessors (getZoneStorePath, mountZoneByTreeSha, getZoneFromCheckout) were created with smudgeLfs = false, causing git-lfs pointer files to be copied into the Nix store as-is instead of being fetched and replaced with their real content. This broke builds for zones containing LFS-tracked files (e.g. //areas/platforms/query-engine), with errors like:
internal/gen/catalog/generated_schema_shopify.go:1:1: expected 'package', found version

The root cause is two-fold:

  • smudgeLfs was never enabled for zone accessors
  • Even if enabled, zone accessors are created from tree SHAs, but lfs::Fetch::shouldFetch() uses GIT_ATTR_CHECK_INCLUDE_COMMIT which requires a commit OID to resolve .gitattributes — so LFS attribute lookup would silently fail regardless

Solution

  • Add lfsCommitRev to GitAccessorOptions — when set, this commit hash is passed to lfs::Fetch instead of the tree hash, allowing correct .gitattributes lookup when the accessor is created from a tree SHA
  • Enable smudgeLfs = true on all three zone accessor call sites, passing the world commit hash (from --tectonix-git-sha) via lfsCommitRev

Note: getWorldGitAccessor() is intentionally left with smudgeLfs = false — it is only used for path validation and tree SHA computation, not for copying content into the store.

Files Changed

  • src/libfetchers/include/nix/fetchers/git-utils.hh — add lfsCommitRev to GitAccessorOptions
  • src/libfetchers/git-utils.cc — use lfsCommitRev in lfs::Fetch constructor; include in makeFingerprint
  • src/libexpr/eval.cc — enable smudgeLfs = true + lfsCommitRev on all zone accessor call sites

@vwassan
Copy link
Copy Markdown
Author

vwassan commented Mar 4, 2026

Screenshot 2026-03-04 at 16 46 26

Copy link
Copy Markdown

@lilyinstarlight lilyinstarlight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still iffy on continuing to put tree shas into the git accessor and fixing up later

I'd rather a commit sha plus path to retrieve be passed then peel back to tree sha for path early on in accessor for what it needs (and I guess fingerprint on that), and then otherwise operate on commit sha for everything. That way we'd only be trying to fixup path rather than having to fix up both commit sha and path with options everywhere

But I don't have bandwidth today/tomorrow for doing that and this is still a step towards fixing the mess, thank you!

Copy link
Copy Markdown

@lilyinstarlight lilyinstarlight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed with further testing that pulling from LFS objects gitdir for checked-out zones, fetching from git-lfs API for non-checked-out zones, and pulling from nix git-lfs cache all work correctly!

@lilyinstarlight lilyinstarlight merged commit 9405d41 into tecnix-gcs Mar 20, 2026
8 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants