Skip to content

Fix issue where git archive fails on repositories with Git LFS#8186

Closed
chadac wants to merge 2 commits intoNixOS:masterfrom
chadac:hotfix/support-lfs
Closed

Fix issue where git archive fails on repositories with Git LFS#8186
chadac wants to merge 2 commits intoNixOS:masterfrom
chadac:hotfix/support-lfs

Conversation

@chadac
Copy link

@chadac chadac commented Apr 7, 2023

Motivation

Add a flag -c remote.origin.lfsurl=<actualUrl> to git.cc to support LFS calls end-to-end, unblocking issues where fetchGit fails on Git repositories using LFS.

Context

Resolves: #4623

Currently fetchGit is incapable of calling git-lfs. Fortunately, it almost works perfectly as git properly hands off necessary calls to a locally installed git-lfs executable, but there is a minor difference in expectations between what is available in the bare Git repository and what lfs needs. The error message from LFS is not very descriptive.

In particular, git-lfs attempts to discover the remote URL that it needs for fetching artifact URLs using the remote.origin.url Git config option, which is not present in the current bare Git repository.

While I think setting the remote.origin.url might have undesirable side-effects, LFS accepts an alternative URL via the LFS-specific configuration option remote.origin.lfsurl.

Since git.cc uses a bare checkout when pulling from repositories using submodules, Git LFS should work in those cases. It's only for repositories without submodules (i.e. when the archive command is invoked) that this configuration option is necessary. Thus, I added it directly as a configuration option on the archive command rather than updating the repository's config file.

One uncertainty that I have: I'm not exactly sure how easy this will be to test. Git LFS usually operates by making an HTTP request to an asset server and I'm not sure if it's feasible to run a test for LFS without needing a lot of extra infrastructure. It also sounds like such tests may be fundamentally changed with this new libgit approach. I can provide a sample repository with LFS files to manually verify this change, but I don't know if there is an easy way to test LFS going forward.

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

@chadac chadac requested a review from edolstra as a code owner April 7, 2023 21:42
@edolstra
Copy link
Member

AFAIK, libgit2 does not have support for LFS, so that makes me reluctant to add LFS support right now.

If we do want to support LFS, it could be tested using the VM infrastructure, see e.g. tests/nixos/github-flakes.nix for a test of the github input type that simulates a GitHub server.

@chadac
Copy link
Author

chadac commented Apr 12, 2023

I think libgit2 does support custom filter hooks, although it would likely need to shell out to git-lfs separately which means custom code eventually. It would definitely still be helpful to our organization, which needs LFS to build a large number of our internal packages.

If y'all are open to adding LFS in, I'd be willing to adding integration tests in with this PR as a POC.

@bouk
Copy link
Member

bouk commented Sep 14, 2023

@chadac We could also just set remote.origin.url in the bare repo, even when we don't have submodules. This should fix it too, right?

@chadac
Copy link
Author

chadac commented Oct 2, 2023

@bouk It should be possible setting this as well. I used the more specific in case there were side effects y'all were avoiding. Can run a test to confirm.

@lucia3e8
Copy link
Contributor

This used to work if you have latest master git-lfs, since we changed git-lfs to look for remotes in the FETCH_HEAD file (git archive looks there by default)

Notwithstanding, git-lfs broke again in the git fetcher overhaul (specifically the move to libgit2). I'll try to fix it again though

@xokdvium
Copy link
Contributor

Closing, since this has been completed in #10153

@xokdvium xokdvium closed this Oct 13, 2025
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.

Support for Git LFS in private repositories

5 participants