Re: [PATCH 5/6] fetch: utilize rejected ref error details

From: Karthik Nayak <karthik.188@gmail.com>
Date: Jan 15, 2026, 10:54 UTC
Message-ID: <CAOLa=ZS0i+YXfVHHAax699ME48YG7jXNZ3WOBYryS0hypMZO-A@mail.gmail.com>
In-Reply-To: <xmqqldi0f6a4.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 31 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> In 0e358de64a (fetch: use batched reference updates, 2025-05-19),
>> git-fetch(1) switched to using batched reference updates. This also
>> introduced a regression wherein instead of providing detailed error
>> messages for failed referenced updates, the users were provided generic
>> error messages based on the error type.
>>
>> Similar to the previous commit, switch to using detailed error messages
>> if present for failed reference updates to fix this regression.
>
> The same question applkies as the previous step.  That is ...
>
>> @@ -1674,9 +1674,11 @@ static void ref_transaction_rejection_handler(const char *refname,
>>  			"branches"), data->remote_name);
>>  		data->conflict_msg_shown = true;
>>  	} else {
>> -		const char *reason = ref_transaction_error_msg(err);
>> -
>> -		error(_("fetching ref %s failed: %s"), refname, reason);
>> +		if (details)
>> +			error("%s", details);
>> +		else
>> +			error(_("fetching ref %s failed: %s"),
>> +			      refname, ref_transaction_error_msg(err));
>
> ... would "details" always carry enough information to cover
> "refname" here, plus what the err code tells us?
>
> I guess ...
>

In general, yes. Here's the final detailed error we'd show:

generic availability checks (files + reftable backend):
- '%s' exists; cannot create '%s'
- cannot process '%s' and '%s' at the same time

packed-backend:
- cannot update ref '%s': reference already exists
- cannot update ref '%s': is at %s but expected %s
- cannot update ref '%s': reference is missing but expected %s

files-backend:
- cannot lock ref '%s': '%s' exists; cannot create '%s'
- cannot lock ref '%s': cannot process '%s' and '%s' at the same time
- cannot lock ref '%s': unable to resolve reference '%s'
- multiple updates for 'HEAD' (including one via its referent '%s')
are not allowed
- cannot lock ref '%s': Unable to create '%s.lock': %s.\n\n
  Another git process seems to be running in this repository, e.g.\n an
  editor opened by 'git commit'. Please make sure all processes\n are
  terminated then try again. If it still fails, a git process\n may have
  crashed in this repository earlier:\n remove the file manually to
  continue.
- cannot lock ref '%s': Unable to create '%s.lock': %s
- cannot lock ref '%s': dangling symref already exists
- cannot lock ref '%s': expected symref with target '%s': but is a regular ref
- cannot lock ref '%s': is at %s but expected %s
- cannot lock ref '%s': reference already exists
- cannot lock ref '%s': reference is missing but expected %s
- cannot lock ref '%s': there is a non-empty directory '%s' blocking
reference '%s'
- cannot lock ref '%s': unable to resolve reference '%s'
- cannot update ref '%s': trying to write non-commit object %s to branch '%s'
- cannot update ref '%s': trying to write ref '%s' with nonexistent object %s
- multiple updates for '%s' (including one via symref '%s') are not allowed
- verifying symref target: '%s': is at %s but expected %s
- verifying symref target: '%s': reference is missing but expected %s

reftable-backend:
- cannot lock ref '%s': dangling symref already exists
- cannot lock ref '%s': expected symref with target '%s': but is a
regular ref
- cannot lock ref '%s': is at %s but expected %s
- cannot lock ref '%s': reference already exists
- cannot lock ref '%s': reference is missing but expected %s
- cannot lock ref '%s': unable to resolve reference '%s'
- multiple updates for '%s' (including one via symref '%s') are not allowed
- multiple updates for 'HEAD' (including one via its referent '%s')
are not allowed
- trying to write non-commit object %s to branch '%s'
- trying to write ref '%s' with nonexistent object %s
- verifying symref target: '%s': is at %s but expected %s
- verifying symref target: '%s': reference is missing but expected %s

Show 19 quoted lines

>> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
>> index ce1c23684e..c69afb5a60 100755
>> --- a/t/t5510-fetch.sh
>> +++ b/t/t5510-fetch.sh
>> @@ -1516,7 +1516,7 @@ test_expect_success REFFILES 'existing reference lock in repo' '
>>  		git remote add origin ../base &&
>>  		touch refs/heads/foo.lock &&
>>  		test_must_fail git fetch -f origin "refs/heads/*:refs/heads/*" 2>err &&
>> -		test_grep "error: fetching ref refs/heads/foo failed: reference already exists" err &&
>> +		test_grep -e "error: cannot lock ref ${SQ}refs/heads/foo${SQ}: Unable to create" -e "refs/heads/foo.lock${SQ}: File exists." err &&
>
> ... the error only talks about our local name, and when the command
> is "git fetch origin refs/heads/foo:refs/remotes/origin/bar", we
> only complain about refs/remotes/origin/bar without ever mentioning
> refs/heads/foo on the remote side, so I think "details" has enough
> information to replace the existing message here in this case.
>
> Thanks.
>

Yup. I do think there is a good cleanup we could potentially do here, with some of the error messages and perhaps following a better pattern in general, perhaps a more structured error message. But I didn't want to tackle that in this series.

Karthik