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

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

>> 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.

>>  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.