Skip to content

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

Open
vwassan wants to merge 3 commits intotecnix-gcsfrom
vw/fix-git-lfs
Open

Fix git-lfs file smudging in zone source accessors#13
vwassan wants to merge 3 commits intotecnix-gcsfrom
vw/fix-git-lfs

Conversation

@vwassan
Copy link

@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
Author

vwassan commented Mar 4, 2026

Screenshot 2026-03-04 at 16 46 26

Copy link

@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 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!

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