Re: `git update-ref` fails to set reflog old_oid in 2.48

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:
>
> On Tue, Jan 21, 2025 at 03:40:06PM -0500, Nika Layzell wrote:
>
>> In git 2.48.1, the `git update-ref` subcommand no longer correctly
>> updates the reflog in some cases. Specifically, it appears that the
>> `old_oid` field will not be updated when modifying a branch referenced
>> by another symbolic ref (e.g. HEAD). This doesn't break the `git
>> reflog` subcommand, but does break references like `HEAD@{1}`, which
>> appear to read the `old_oid` field.
>>
>> STR (in a fresh directory):
>> ```
>> git init -b main
>> git commit --allow-empty -m "A"
>> git commit --allow-empty -m "B"
>> git update-ref -m "reason" refs/heads/main HEAD~ HEAD
>> ```
>> [...]
>> The `old_oid` field is empty (all zeroes). This is only the case in
>> derived reflogs (in this case .git/logs/HEAD). The reflog for
>> refs/heads/main appears to be updated correctly.
>
> Thanks for an easy reproduction recipe. Looks like it bisects to
> 297c09eabb (refs: allow multiple reflog entries for the same refname,
> 2024-12-16). Author cc'd.
>
> Just looking at the diff, the early return from lock_ref_for_update()
> seems suspicious to me. Doing this:
>
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 8953d1c6d3..1c0e24a855 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -2611,9 +2611,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) {
>  			/*
>
> makes the problem go away, and doesn't fail any tests. But that is just
> me poking at it without understanding why those lines were there in the
> first place.
>

The reason for that addition was that there was assumed the flow of
`lock_ref_for_update()` for reflog only updates was to capture the lock
only [1]. But this is wrong since this misses the old_oid population. As
such your fix should do the trick. Thanks!

[1]: https://lore.kernel.org/git/CAOLa=ZQqQTEquJ0e5rG168-CVADR8K-uYma7Z8yiDCptyPgBQg@xxxxxxxxxxxxxx/

> -Peff

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux