Merge pull request #13521 from fzakaria/fzakaria/issue-11266

Unpeel reference for git+file

src/libfetchers-tests/git-utils.cc

@@ -3,20 +3,28 @@
 #include <gmock/gmock.h>
 #include <git2/global.h>
 #include <git2/repository.h>
+#include <git2/signature.h>
 #include <git2/types.h>
+#include <git2/object.h>
+#include <git2/tag.h>
 #include <gtest/gtest.h>
 #include "nix/util/fs-sink.hh"
 #include "nix/util/serialise.hh"
 #include "nix/fetchers/git-lfs-fetch.hh"
 
+#include <git2/blob.h>
+#include <git2/tree.h>
+
 namespace nix {
 
 class GitUtilsTest : public ::testing::Test
 {
     // We use a single repository for all tests.
-    std::filesystem::path tmpDir;
     std::unique_ptr<AutoDelete> delTmpDir;
 
+protected:
+    std::filesystem::path tmpDir;
+
 public:
     void SetUp() override
     {
@@ -115,4 +123,54 @@ TEST_F(GitUtilsTest, sink_hardlink)
     }
 };
 
+TEST_F(GitUtilsTest, peel_reference)
+{
+    // Create a commit in the repo
+    git_repository * rawRepo = nullptr;
+    ASSERT_EQ(git_repository_open(&rawRepo, tmpDir.string().c_str()), 0);
+
+    // Create a blob
+    git_oid blob_oid;
+    const char * blob_content = "hello world";
+    ASSERT_EQ(git_blob_create_from_buffer(&blob_oid, rawRepo, blob_content, strlen(blob_content)), 0);
+
+    // Create a tree with that blob
+    git_treebuilder * builder = nullptr;
+    ASSERT_EQ(git_treebuilder_new(&builder, rawRepo, nullptr), 0);
+    ASSERT_EQ(git_treebuilder_insert(nullptr, builder, "file.txt", &blob_oid, GIT_FILEMODE_BLOB), 0);
+
+    git_oid tree_oid;
+    ASSERT_EQ(git_treebuilder_write(&tree_oid, builder), 0);
+    git_treebuilder_free(builder);
+
+    git_tree * tree = nullptr;
+    ASSERT_EQ(git_tree_lookup(&tree, rawRepo, &tree_oid), 0);
+
+    // Create a commit
+    git_signature * sig = nullptr;
+    ASSERT_EQ(git_signature_now(&sig, "nix", "nix@example.com"), 0);
+
+    git_oid commit_oid;
+    ASSERT_EQ(git_commit_create_v(&commit_oid, rawRepo, "HEAD", sig, sig, nullptr, "initial commit", tree, 0), 0);
+
+    // Lookup our commit
+    git_object * commit_object = nullptr;
+    ASSERT_EQ(git_object_lookup(&commit_object, rawRepo, &commit_oid, GIT_OBJECT_COMMIT), 0);
+
+    // Create annotated tag
+    git_oid tag_oid;
+    ASSERT_EQ(git_tag_create(&tag_oid, rawRepo, "v1", commit_object, sig, "annotated tag", 0), 0);
+
+    auto repo = openRepo();
+
+    // Use resolveRef to get peeled object
+    auto resolved = repo->resolveRef("refs/tags/v1");
+
+    // Now assert that we have unpeeled it!
+    ASSERT_STREQ(resolved.gitRev().c_str(), git_oid_tostr_s(&commit_oid));
+
+    git_signature_free(sig);
+    git_repository_free(rawRepo);
+}
+
 } // namespace nix

src/libfetchers/git-utils.cc

@@ -360,7 +360,13 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this<GitRepoImpl>
     Hash resolveRef(std::string ref) override
     {
         Object object;
-        if (git_revparse_single(Setter(object), *this, ref.c_str()))
+
+        // Using the rev-parse notation which libgit2 supports, make sure we peel
+        // the ref ultimately down to the underlying commit.
+        // This is to handle the case where it may be an annotated tag which itself has
+        // an object_id.
+        std::string peeledRef = ref + "^{commit}";
+        if (git_revparse_single(Setter(object), *this, peeledRef.c_str()))
             throw Error("resolving Git reference '%s': %s", ref, git_error_last()->message);
         auto oid = git_object_id(object.get());
         return toHash(*oid);

tests/functional/fetchGit.sh

@@ -236,10 +236,10 @@ path9=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$rep
 # Specifying a ref without a rev shouldn't pick a cached rev for a different ref
 export _NIX_FORCE_HTTP=1
 rev_tag1_nix=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"refs/tags/tag1\"; }).rev")
-rev_tag1=$(git -C $repo rev-parse refs/tags/tag1)
+rev_tag1=$(git -C $repo rev-parse refs/tags/tag1^{commit})
 [[ $rev_tag1_nix = $rev_tag1 ]]
 rev_tag2_nix=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"refs/tags/tag2\"; }).rev")
-rev_tag2=$(git -C $repo rev-parse refs/tags/tag2)
+rev_tag2=$(git -C $repo rev-parse refs/tags/tag2^{commit})
 [[ $rev_tag2_nix = $rev_tag2 ]]
 unset _NIX_FORCE_HTTP