From: Jeff King <peff@peff.net>
Date: Wed, 14 Jan 2026 17:55:58 GMT
Subject: Re: [PATCH 4/6] update-ref: utilize rejected error details if available
Message-ID: <20260114175558.GG885771@coredump.intra.peff.net>
In-Reply-To: <xmqqpl7cf6kf.fsf@gitster.g>

On Wed, Jan 14, 2026 at 09:27:28AM -0800, Junio C Hamano wrote:

> Karthik Nayak <karthik.188@gmail.com> writes:
> 
> > @@ -573,16 +573,18 @@ static void print_rejected_refs(const char *refname,
> >  				const char *old_target,
> >  				const char *new_target,
> >  				enum ref_transaction_error err,
> > -				const char *details UNUSED,
> > +				const char *details,
> >  				void *cb_data UNUSED)
> >  {
> >  	struct strbuf sb = STRBUF_INIT;
> > -	const char *reason = ref_transaction_error_msg(err);
> >  
> > -	strbuf_addf(&sb, "rejected %s %s %s %s\n", refname,
> > -		    new_oid ? oid_to_hex(new_oid) : new_target,
> > -		    old_oid ? oid_to_hex(old_oid) : old_target,
> > -		    reason);
> > +	if (details)
> > +		strbuf_addf(&sb, "%s\n", details);
> > +	else
> > +		strbuf_addf(&sb, "rejected %s %s %s %s\n", refname,
> > +			    new_oid ? oid_to_hex(new_oid) : new_target,
> > +			    old_oid ? oid_to_hex(old_oid) : old_target,
> > +			    ref_transaction_error_msg(err));
> 
> Could "details" reported from the lower layer be less detailed than
> what we are formulating here, like updating the value of what ref
> from what old object to what new object, or what the err code tells
> the end-user?

I wondered that, too, but also: is this supposed to be machine-readable?
The "rejected ..." output looks like something that could be parsed,
and it seems to be documented in git-update-ref(1).

  Side note: if this is meant to be a stable format, surely there should
  be some coverage in the test suite? There doesn't seem to be.

So should we just be replacing the ref_transaction_error_msg() part? I
_think_ the low-level details will usually be more informative there,
but not necessarily. So possibly we'd even want to show both, though I
suspect just concatenating them would be messy.

Plus the "details" one has a lot of redundant information in it (it
mentions "refname", even though it is already on the "rejected" line).

In the short-term, I wonder if we just want:

  if (details && *details)
	error("%s", details);

That gets us back to the status quo, where the details are at least
available via stderr. And then we can consider how to combine them into
the machine-readable format separately.

-Peff