From 1dc363cd6623c3f08422f3605fe0870079ef948d Mon Sep 17 00:00:00 2001 From: John Rinehart Date: Sat, 26 Oct 2024 12:23:25 -0700 Subject: [PATCH] fix: better support dangling revisions The submodules branch generates a checkout which depends on only the references, not all revisions. If there are dangling revisions in the bare fetcher clone (a correctness issue itself) then the subsequent temporary directory clone will disastrously fail with messages like ``` fatal: reference is not a tree: 2da0785fa32ce4c628d501c8743c88be00835b50 ``` You should be able to reproduce this behavior with ```bash this_nixpkgs='github:nixos/nixpkgs?rev=36ac8d7e411eeb11ac0998d5a39e329c1226e541'; this_git=$(nix build --print-out-paths "$this_nixpkgs#git")/bin/git; export GIT_AUTHOR_NAME=foo \ GIT_AUTHOR_EMAIL=foo@foo.foo \ GIT_AUTHOR_DATE=100000000 \ GIT_COMMITER_NAME=foo \ GIT_COMMITER_EMAIL=foo@foo.foo \ GIT_COMMITTER_DATE=100000000; tmpgit=$(mktemp -d); echo "Repo'll be at $tmpgit"; "$this_git" init "$tmpgit"; "$this_git" -C "$tmpgit" commit --allow-empty -m "foo"; # 8c2146823865b76da067b3bb458611a0a19ede3b "$this_git" -C "$tmpgit" commit --allow-empty -m "foo"; # 2da0785fa32ce4c628d501c8743c88be00835b50 "$this_git" -C "$tmpgit" reset --hard HEAD~1; # 8c2146823865b76da067b3bb458611a0a19ede3b for version in 18 19 20 21 22 23 24; do echo "Checking nix version 2.$version"; this_nix=$(nix build "$this_nixpkgs#nixVersions.nix_2_$version^out" --print-out-paths)/bin/nix; url="file:///$tmpgit"; # populate fetcher-v1.sqlite and the gitv3/ dir "$this_nix-build" --expr $'builtins.fetchGit { url = \"'"$url"$'\"; rev = \"8c2146823865b76da067b3bb458611a0a19ede3b\"; submodules = true; }'; # put the "corrupted" git dir in place of the fetched one cache_dir="${XDG_CONFIG_HOME:-$HOME}"/.cache/nix/gitv3; tmp=$(mktemp); printf "%s" "$url" >"$tmp"; fetched_dir="$cache_dir"/$(printf "%s" "$url" | $this_nix --extra-experimental-features nix-command hash file --type sha256 --base32 "$tmp"); rm -rf "$fetched_dir"; git clone --bare "$tmpgit" "$fetched_dir"; # See if we can realise the derivation pointing to the dangling commit _NIX_FORCE_HTTP=1 "$this_nix-build" --impure --expr $' { pkgs ? (import { }) }: pkgs.stdenv.mkDerivation { pname = "foo"; version = "1.0.0"; dontUnpack = true; '" src = builtins.fetchGit { url = \"file:///$tmpgit\"; rev = \"2da0785fa32ce4c628d501c8743c88be00835b50\"; submodules = true; }; buildPhase = '' touch \$out; exit 0; ''; } " done ``` This change set also reports such an inconsistency (printTalkative) with an associated attempt to redress the inconsistency with a fresh fetch. It also avoids this issue in the submodules path by simply checking out the specified revision (in addition to all references to preserve the original behavior - although, I think _only_ the revision is necessary). --- src/libfetchers/git.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index f8d89ab2fcdd..c12970397015 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -529,7 +529,7 @@ struct GitInputScheme : InputScheme if (input.getRev()) { try { runProgram("git", true, { "-C", repoDir, "--git-dir", gitDir, "cat-file", "-e", input.getRev()->gitRev() }); - doFetch = false; + doFetch = false; } catch (ExecError & e) { if (WIFEXITED(e.status)) { doFetch = true; @@ -630,7 +630,7 @@ struct GitInputScheme : InputScheme // everything to ensure we get the rev. Activity act(*logger, lvlTalkative, actUnknown, fmt("making temporary clone of '%s'", repoDir)); runProgram("git", true, { "-C", tmpDir, "fetch", "--quiet", "--force", - "--update-head-ok", "--", repoDir, "refs/*:refs/*" }, {}, true); + "--update-head-ok", "--", repoDir, "refs/*:refs/*", input.getRev()->gitRev() }, {}, true); } runProgram("git", true, { "-C", tmpDir, "checkout", "--quiet", input.getRev()->gitRev() });