Skip to content

nix-prefetch-git: fix checkout_ref() for "dumb http" repositories#116289

Closed
logan12358 wants to merge 1 commit intoNixOS:masterfrom
logan12358:fix-nix-prefetch-git-dumb-http-rev
Closed

nix-prefetch-git: fix checkout_ref() for "dumb http" repositories#116289
logan12358 wants to merge 1 commit intoNixOS:masterfrom
logan12358:fix-nix-prefetch-git-dumb-http-rev

Conversation

@logan12358
Copy link
Contributor

The checkout_ref() function was written against Git 1.6.5, just before "smart http" repositories were implemented in Git 1.6.6. In 1.6.5, the --depth parameter is ignored when fetching from a dumb http repository. From 1.6.6, this scenario causes a failure. This change updates the comment and falls back to fetching without --depth.

Motivation for this change

This change causes checkout_ref to succeed in cases where I believe it is intended. I think that previously fetches from a dumb http repository would usually succeed through the checkout_hash fallback. As of #104714 checkout_hash doesn't seem to support tags (perhaps this should be changed as well?), so now nix-prefetch-git noticeably fails for tag revs on dumb http repositories.

I'm not 100% convinced about the chained ||, but I can't find a way to instruct Git to "attempt" --depth=1 or find a way to robustly fall back only if the first fetch fails due to a "dumb http" repository.

This seems to resolve #115145.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

The checkout_ref() function was written against Git 1.6.5, just before "smart
http" repositories were implemented in Git 1.6.6. In 1.6.5, the `--depth`
parameter is ignored when fetching from a dumb http repository. From 1.6.6,
this scenario causes a failure. This change updates the comment and falls back
to fetching without `--depth`.
@ofborg ofborg bot added 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Mar 14, 2021
@r-rmcgibbo
Copy link

r-rmcgibbo commented Mar 14, 2021

Result of nixpkgs-review pr 116289 at 15364cb run on aarch64-linux 1

2 packages skipped due to time constraints:
  • haskellPackages.update-nix-fetchgit
  • update-nix-fetchgit
9 packages built successfully:

Result of nixpkgs-review pr 116289 at 15364cb run on x86_64-linux 1

12 packages built successfully:

@SuperSandro2000 SuperSandro2000 requested a review from jtojnar March 14, 2021 16:40
@stale

This comment was marked as spam.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 10, 2021
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/cant-build-solvespace-from-scratch-fetching-git-source-fails/46536/2

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 5, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
@qweered qweered requested a review from ShamrockLee December 29, 2025 00:10
@nixpkgs-ci nixpkgs-ci bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 29, 2025
@ShamrockLee
Copy link
Contributor

Hi @logan12358 , could you help check if PR #283663 addressed the issue?

I refactored the code a bit in PR #465497 to

  • use if-else instead of and-or
  • avoids fetching all tags whenever backward compatible (when not leaving .git)
  • only clone again with the deep-clone-and-fetch-all-tags flags when the forst clone is not like that.

Nevertheless, it would be better if we could check if a remote support the smart protocol or not instead of retrying blindly, but I don't know how to implement such check.

@domenkozar domenkozar closed this Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fetchgit fails on a repository that can be cloned with git

6 participants