From dd279acf50f3d09ba1bc7248328ddeaef47ffd4b 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 solution attempts to avoid the 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 f8d89ab2fcd..c1297039701 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() });