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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 22, 2025, 17:56 UTC
Message-ID: <xmqqsepawx4o.fsf@gitster.g>
In-Reply-To: <Z5De17-Ed3K31kzh@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 13 quoted lines

>> 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.
>
> It's a bit curious that you describe the fix here, then in the next
> paragraph describe why we have skipped the logic only to reiterate the
> fix.
>
>> 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.

Yeah, that indeed is a "bit" curious. I'd call it confusing, though ;-).

> Okay, makes sense. The error is specific to the "files" backend, which
> might be worth mentioning in the commit message.

Indeed.

Show 9 quoted lines

>>  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
>>  
>
> We could use `git symbolic-ref HEAD` to resolve the branch name instead
> of overriding the branch name here.

I agree.  That sounds like a more sensible way to go.

Thanks.