From 60c2d15f5a7db86eb79c11c4a863789d186e8650 Mon Sep 17 00:00:00 2001
From: Bouke van der Bijl <i@bou.ke>
Date: Thu, 14 Mar 2024 14:04:51 +0100
Subject: [PATCH] git fetcher: use resolveRef for getting revision of reference

* Add regression test
* Fix 'no repo' test so it doesn't succeed if the data is still in cache
* Use git_revparse_single inside git-utils instead of reimplementing the same logic.
---
 src/libfetchers/git-utils.cc | 24 ++++--------------------
 src/libfetchers/git.cc       |  2 +-
 tests/functional/fetchGit.sh |  9 +++++++++
 3 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc
index 9cae9034e..b723554cc 100644
--- a/src/libfetchers/git-utils.cc
+++ b/src/libfetchers/git-utils.cc
@@ -21,6 +21,7 @@
 #include <git2/refs.h>
 #include <git2/remote.h>
 #include <git2/repository.h>
+#include <git2/revparse.h>
 #include <git2/status.h>
 #include <git2/submodule.h>
 #include <git2/tree.h>
@@ -199,27 +200,10 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this<GitRepoImpl>
 
     Hash resolveRef(std::string ref) override
     {
-        // Handle revisions used as refs.
-        {
-            git_oid oid;
-            if (git_oid_fromstr(&oid, ref.c_str()) == 0)
-                return toHash(oid);
-        }
-
-        // Resolve short names like 'master'.
-        Reference ref2;
-        if (!git_reference_dwim(Setter(ref2), *this, ref.c_str()))
-            ref = git_reference_name(ref2.get());
-
-        // Resolve full references like 'refs/heads/master'.
-        Reference ref3;
-        if (git_reference_lookup(Setter(ref3), *this, ref.c_str()))
+        Object object;
+        if (git_revparse_single(Setter(object), *this, ref.c_str()))
             throw Error("resolving Git reference '%s': %s", ref, git_error_last()->message);
-
-        auto oid = git_reference_target(ref3.get());
-        if (!oid)
-            throw Error("cannot get OID for Git reference '%s'", git_reference_name(ref3.get()));
-
+        auto oid = git_object_id(object.get());
         return toHash(*oid);
     }
 
diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index 25eabb1dc..34cfd3f5b 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -585,7 +585,7 @@ struct GitInputScheme : InputScheme
                         repoInfo.url
                         );
             } else
-                input.attrs.insert_or_assign("rev", Hash::parseAny(chomp(readFile(localRefFile)), HashAlgorithm::SHA1).gitRev());
+                input.attrs.insert_or_assign("rev", repo->resolveRef(ref).gitRev());
 
             // cache dir lock is removed at scope end; we will only use read-only operations on specific revisions in the remainder
         }
diff --git a/tests/functional/fetchGit.sh b/tests/functional/fetchGit.sh
index 3f2d0d5fb..74d6de4e3 100644
--- a/tests/functional/fetchGit.sh
+++ b/tests/functional/fetchGit.sh
@@ -43,10 +43,18 @@ path0_=$(nix eval --impure --raw --expr "(builtins.fetchTree git+file://$TEST_RO
 export _NIX_FORCE_HTTP=1
 [[ $(tail -n 1 $path0/hello) = "hello" ]]
 
+# Nuke the cache
+rm -rf $TEST_HOME/.cache/nix
+
 # Fetch the default branch.
 path=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath")
 [[ $(cat $path/hello) = world ]]
 
+# Fetch when the cache has packed-refs
+# Regression test of #8822
+git -C $TEST_HOME/.cache/nix/gitv3/*/ pack-refs --all
+path=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath")
+
 # Fetch a rev from another branch
 git -C $repo checkout -b devtest
 echo "different file" >> $TEST_ROOT/git/differentbranch
@@ -251,6 +259,7 @@ path12=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = file://$repo
 
 # should fail if there is no repo
 rm -rf $repo/.git
+rm -rf $TEST_HOME/.cache/nix
 (! nix eval --impure --raw --expr "(builtins.fetchGit \"file://$repo\").outPath")
 
 # should succeed for a repo without commits