[PATCH] refs: fix creation of corrupted reflogs for symrefs

From: Karthik Nayak <karthik.188@gmail.com>
Date: Jan 22, 2025, 10:03 UTC
Message-ID: <20250122100319.2280647-1-karthik.188@gmail.com>
In-Reply-To: <20250121215235.GA2753621@coredump.intra.peff.net>

The commit 297c09eabb (refs: allow multiple reflog entries for the same refname, 2024-12-16) added logic for reflogs to exit early in `lock_ref_for_update()` after obtaining the required lock. This was added as a performance optimization as it was assumed that no further processing was required for reflog only updates. However this was incorrect since for a symref's reflog entry, the update needs to be populated with the old_oid value. This is done right after the early exit.

This caused a bug in Git 2.48 where target references of symrefs being updated would create a corrupted reflog entry for the symref since the old_oid is not populated. Undo the skip in logic to fix this issue and also add a test to ensure that such an issue doesn't arise in the future.

The early exit was added as a performance optimization for reflog-only updates, but this accidentally broke symref reflog handling. Remove the optimization since it wasn't essential to the original changes.

Reported-by: Nika Layzell <nika@thelayzells.com>
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---

Hello,

This patch is based on top of 'maint' so that it can be easily backported. Sorry for the inconvenience here. This was a premature optimization which wasn't needed, and unfortunately this wasn't captured by any test.

Karthik

---
 refs/files-backend.c  |  3 ---
 t/t1400-update-ref.sh | 16 ++++++++++++++++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 5cfb8b7ca8..29f08dced4 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2615,9 +2615,6 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 
 	update->backend_data = lock;
 
-	if (update->flags & REF_LOG_ONLY)
-		goto out;
-
 	if (update->type & REF_ISSYMREF) {
 		if (update->flags & REF_NO_DEREF) {
 			/*
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index e2316f1dd4..59493dd73f 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -4,6 +4,8 @@
 #
 
 test_description='Test git update-ref and basic ref logging'
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
@@ -2068,4 +2070,18 @@ do
 
 done
 
+test_expect_success 'update-ref should also create reflog for HEAD' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A &&
+		test_commit B &&
+		git rev-parse HEAD >>expect &&
+		git update-ref --create-reflog refs/heads/main HEAD~ &&
+		git rev-parse HEAD@{1} >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_done

-- 
2.47.0