From 33ceea60998646e8f0b21f473d5cb799d90ca387 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Mon, 21 Jul 2025 21:10:41 -0700 Subject: [PATCH 1/3] Unpeel reference for git+file If the reference for git+file is an annotated tag, the revision will differ than when it's fetched using other fetchers such as `github:` since Github seems to automatiacally peel to the underlying commit. Turns out that rev-parse has the capability through it's syntax to request the underlying commit by "peeling" using the `^{commit}` syntax. This is safe to apply in all scenarios where the goal is to get an underlying commit. fixes #11266 --- src/libfetchers/git-utils.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index f45360f71..a758848b2 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -360,7 +360,13 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this 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); From 98858148dc8da11533d5c2bbae51f0bc9d7e6b04 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Mon, 21 Jul 2025 21:56:11 -0700 Subject: [PATCH 2/3] Add unit test --- src/libfetchers-tests/git-utils.cc | 60 +++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/src/libfetchers-tests/git-utils.cc b/src/libfetchers-tests/git-utils.cc index c2c7f9da0..bfba3d679 100644 --- a/src/libfetchers-tests/git-utils.cc +++ b/src/libfetchers-tests/git-utils.cc @@ -3,20 +3,28 @@ #include #include #include +#include #include +#include +#include #include #include "nix/util/fs-sink.hh" #include "nix/util/serialise.hh" #include "nix/fetchers/git-lfs-fetch.hh" +#include +#include + namespace nix { class GitUtilsTest : public ::testing::Test { // We use a single repository for all tests. - std::filesystem::path tmpDir; std::unique_ptr 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 From aadfb682d41881e261b929266dc3192794562a3c Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Mon, 21 Jul 2025 22:01:05 -0700 Subject: [PATCH 3/3] Fix fetchGit functional tests to peel as well --- tests/functional/fetchGit.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/fetchGit.sh b/tests/functional/fetchGit.sh index dc5d8f818..e7c9c77a5 100755 --- a/tests/functional/fetchGit.sh +++ b/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