Improve errors when we are trying to access a git repository with partial history (+ fix fetchGit on these repos) #13265
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This can never work and leads to a crash bug.
github-actions
bot
added
with-tests
Mic92
force-pushed
the
fix-shallow-clone-subset
branch
2 times, most recently
from
3ed12b2 to
3110805
Compare
edolstra
reviewed
May 26, 2025
Comment on lines
+801
to
+804
| if (!getShallowAttr(input)) { | ||
| input.attrs.insert_or_assign("revCount", | ||
| rev == nullRev ? 0 : getRevCount(*input.settings, repoInfo, repoPath, rev)); | ||
| } |
There was a problem hiding this comment.
Was this intended to be included in this PR? Because this seems like a behaviour change.
There was a problem hiding this comment.
This doesn't change behavior. If we do a builtins.fetchGit with shallow = true; than it tries to compute the revCount and crashes. This code never worked for shallow clones.
There was a problem hiding this comment.
This is the error before this change:
error:
… while calling the 'fetchGit' builtin
at «string»:1:2:
1| (builtins.fetchGit { url = "file:///tmp/nix-test/main/fetchGitShallow/shallow-worktree"; shallow = true; }).rev
| ^
… while fetching the input 'git+file:///tmp/nix-test/main/fetchGitShallow/shallow-worktree?exportIgnore=1&shallow=1'
error: getting parent of Git commit 'b84683a868d74413ba54bd3c7da8bd57a94a8c8a': object not found - no match for id (cfad7cc96b05c722c83ea7c41269a6910c0a2020)
There was a problem hiding this comment.
All other code paths for shallow clones also seem to not include revCount as expected.
| [[ $(nix eval --impure --expr "(builtins.fetchTree { type = \"git\"; url = \"file://$TEST_ROOT/shallow\"; ref = \"dev\"; shallow = true; }).revCount or 123") == 123 ]] | ||
|
|
||
| expectStderr 1 nix eval --expr 'builtins.fetchTree { type = "git"; url = "file:///foo"; }' | grepQuiet "'fetchTree' will not fetch unlocked input" | ||
|
|
There was a problem hiding this comment.
Not sure if these tests should be removed in this PR?
There was a problem hiding this comment.
They are not removed but moved to fetchGitShallow.sh
Before we got something like this but only inside the VM test: vm-test-run-functional-tests-on-nixos_user> machine # fetchGit.sh: line 286: unexpected EOF while looking for matching `)' We now try to do not too much in a single line, so that the bash parser does not get confused. This also seems more readable and better quoted.
Mic92
force-pushed
the
fix-shallow-clone-subset
branch
from
3110805 to
5419d82
Compare
Mic92
added
backport 2.28-maintenance
edolstra
approved these changes
May 26, 2025
This was referenced May 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
This is a subset of #13260 but doesn't change the default behavior for flake urls, so it can be backported.
Error fixed by this PR:
Context
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.