Re: [PATCH v8 3/3] fetch: fix failed batched updates skipping operations

From: Karthik Nayak <karthik.188@gmail.com>
Date: Dec 2, 2025, 22:35 UTC
Message-ID: <CAOLa=ZTdfkK0ty2YQfE+GTtgYZ7wrOW_04Ony8tN+x2oqoXSCg@mail.gmail.com>
In-Reply-To: <aS2Q6y_hnwBxycGk@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

[snip]

Show 39 quoted lines

>> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
>> index 4b113d7c27..a1ca4e1ac7 100755
>> --- a/t/t5510-fetch.sh
>> +++ b/t/t5510-fetch.sh
>> @@ -1639,6 +1639,94 @@ test_expect_success "backfill tags when providing a refspec" '
>>  	test_cmp expect actual
>>  '
>>
>> +test_expect_success REFFILES "FETCH_HEAD is updated even if ref updates fail" '
>> +	test_when_finished rm -rf base repo &&
>> +
>> +	git init base &&
>> +	(
>> +		cd base &&
>> +		test_commit "updated" &&
>> +
>> +		git update-ref refs/heads/foo @ &&
>> +		git update-ref refs/heads/branch @
>> +	) &&
>> +
>> +	git init --bare repo &&
>> +	(
>> +		cd repo &&
>> +		rm -f FETCH_HEAD &&
>> +		git remote add origin ../base &&
>> +		>refs/heads/foo.lock &&
>
> Hm. Is this compatible with all supported systems? We typically write
> this as:
>
>     : >refs/heads/foo.lock
>
> But I have to acknowledge that I only do this because some people that
> are more knowledgeable than I am know that we need this.
>
> Other than that I'm happy with the current state of this patch series.
> If the above turns out to be a non-issue I think it should be ready for
> 'next'.
>

I didn't know about this. The CI didn't complain about this too.

A quick search through our repo shows

$ rg --stats '^\s*>[\w/.-]+' t/ ... 969 matches 969 matched lines 285 files contained matches ...

$ rg --stats '^\s*:\s*>[\w/.-]+' t/ ... 188 matches 188 matched lines 58 files contained matches ...

So seems like we use both, meaning, this should be okay?

> Thanks!
>
> Patrick

Thanks for the review.

Since the other comment was a nit. I will hold off on sending a new version :)