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

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 in the files backend 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, and it wasn't essential to the original changes. As such, reverting it shouldn't cause any further issues.

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

Changes since the v1:
- Modified the commit message and subject to make it a little more clearer.
- Used `git symbolic-ref HEAD` in the test instead of setting
  the default branch.

Range diff:

1:  dbb1186512 ! 1:  9326fb08a9 reflog: fix bug which didn't resolve symref reflogs
    @@ Metadata
     Author: Karthik Nayak <karthik.188@gmail.com>
     
      ## Commit message ##
    -    reflog: fix bug which didn't resolve symref reflogs
    +    refs: fix creation of corrupted reflogs for symrefs
     
         The commit 297c09eabb (refs: allow multiple reflog entries for the same
    -    refname, 2024-12-16) added logic to skip the flow for reflogs in
    -    `lock_ref_for_update()` after obtaining the required lock. This was done
    -    because it was assumed that the flow ends there for reflogs. However
    -    this was incorrect since for a symref's reflog entry, we need to
    -    populate the old_oid value which is done right after.
    +    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.
    +    This caused a bug in Git 2.48 in the files backend 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, and it wasn't essential to the original changes. As such,
    +    reverting it shouldn't cause any further issues.
     
         Reported-by: Nika Layzell <nika@thelayzells.com>
         Co-authored-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Jeff King <peff@peff.net>
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
      ## refs/files-backend.c ##
    @@ refs/files-backend.c: static int lock_ref_for_update(struct files_ref_store *ref
      			/*
     
      ## t/t1400-update-ref.sh ##
    -@@
    - #
    - 
    - 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
    - 
     @@ t/t1400-update-ref.sh: 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_commit to-rewind &&
    ++	git rev-parse HEAD >expect &&
    ++	head=$(git symbolic-ref HEAD) &&
    ++	git update-ref --create-reflog "$head" HEAD~ &&
    ++	git rev-parse HEAD@{1} >actual &&
    ++	test_cmp expect actual
     +'
     +
      test_done

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