From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 14 Jan 2026 17:33:39 GMT
Subject: Re: [PATCH 5/6] fetch: utilize rejected ref error details
Message-ID: <xmqqldi0f6a4.fsf@gitster.g>
In-Reply-To: <20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-5-f5f8b173c501@gmail.com>

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

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