git: skip default-branch discovery for exact revisions - #16494
Conversation
Keep ref as HEAD when a revision is supplied without an explicit ref and allRefs is false. This avoids ls-remote even when the commit is already cached. Newly resolved locked inputs record HEAD instead of the remote's current default branch. Preserve default-branch discovery and caching for allRefs, since fetching all branches can make the cache's initial HEAD target resolve to the wrong branch for a later unpinned fetch. Add regressions for cold and warm fetches, the transition to unpinned fetches, nested submodules, and generated and existing lockfiles. Assisted-by: Codex (GPT-6)
github-actions
Bot
added
with-tests
On the one hand, this does seem to be a bit more pure. But I'm worried it would be hella confusing to people. |
Mb we can ommit ref entirely in that case? Cause currently it says master, which is also not very useful if rev is from the other branch. But it will make patch bigger |
| bool usesDefaultRef = !originalRef && (!origRev || getAllRefsAttr(input)); | ||
| bool shallow = getShallowAttr(input); | ||
| auto ref = originalRef ? *originalRef : getDefaultRef(settings, repoInfo, shallow); | ||
| auto ref = originalRef ? *originalRef : usesDefaultRef ? getDefaultRef(settings, repoInfo, shallow) : "HEAD"; |
There was a problem hiding this comment.
a consideration here it to use a sentinel other than HEAD... but it isn't clear if there would be any benefit... or any drawback of the current one.
3 participants
Motivation
Even with the negotiation-ref fix, fetching an already-cached commit triggered
git ls-remoteagainst GitHub before Nix checked whether the commit was present locally.Each call transferred about 35 MiB.
The revision is already specified, so looking up the remote's default branch is unnecessary.
This PR skips that lookup when an exact
revis supplied,refis omitted, andallRefsis false.Newly generated lockfiles record
ref = "HEAD"instead of the remote's current symbolic default branch.This changes the recorded ref, but not the selected revision or source tree.
Existing locks with a concrete ref remain valid and are not rewritten during normal reuse.
The
allRefspath retains its existing behavior.Regression tests cover cold and cached exact-revision fetches, a subsequent unpinned fetch, nested submodules using
branch = ".", and generated and existing lockfiles.Context
In an earlier PR, I tried to improve Git cache reuse during exact-revision fetches.
The negotiation issue was fixed separately in #16432.
I then wanted to see how much traffic and disk space Git could save compared to tarballs when updating nixpkgs.
I put together a reproducible VM benchmark covering daily, weekly and monthly updates, repeated CI fetches, and switching between pins.
The published benchmark predates this PR and used a broader experimental patch: it read the cached symbolic HEAD, restricted the
ls-remoteoutput, and kept Git maintenance in the foreground.The numbers below are not an isolated A/B measurement of this diff.
For daily stable + unstable updates, the benchmark measured:
For the persistent CI workload, the numbers were 1,254 / 3,493 / 159 MiB.
The experimental run transferred zero bytes across 72 repeated-pin fetches.
Cold Git fetches remained more expensive than tarballs.
Raw results, source revisions, and the measured patch are in the benchmark repository.
Testing also exposed a separate issue: Git's background auto-maintenance could overlap the next fetch.
The benchmark patch keeps maintenance in the foreground; I will address that in a separate PR.