From: Jeff King <peff@peff.net>
Date: Sat, 27 Dec 2025 07:44:11 GMT
Subject: Re: Possible regression: lost diagnostic message when pushing non-commit objects to refs/heads/*
Message-ID: <20251227074411.GB2071715@coredump.intra.peff.net>
In-Reply-To: <CAOLa=ZSOZz9aGFFeD7tiQ+PRwkMosjcoxfTSk52fQeQq0ghgaw@mail.gmail.com>

On Fri, Dec 26, 2025 at 11:48:19AM -0500, Karthik Nayak wrote:

> > And then receive-pack can either dump it via rp_error(), giving the same
> > behavior as the old version. Or it can stick it into the per-ref status
> > field. The latter feels more "right" in the sense that the error
> > messages can be reliably attached to specific ref updates in the
> > machine-readable output (rather than appearing willy-nilly on stderr or
> > sideband 2). But I'd guess it would make the output rather unwieldy.
> 
> The second option would be more useful to the user too. Since they can
> act upon that specific update.

The trouble is that the low-level code constructing the "err" buffer is
aimed at writing a human-readable message. So you get the whole string like:

  cannot update ref 'refs/heads/foo': trying to write non-commit object
  d19968fcf0d3193147b827c9e89668d619afd01e to branch 'refs/heads/foo'

That's already somewhat redundant by itself, because
lock_ref_for_update(), the intermediate caller that sticks "cannot
update ref 'foo':" on the front of the string, does not know that its
helper function write_ref_to_lockfile() has already put "foo" in the
error message is returned.

And we get one layer worse when we attach that whole thing to
machine-readable output associated with the ref "foo".

There's probably some clean-up possible here, but it will have to be
done very carefully. If we can check that all of the callers of
write_ref_to_lockfile() mention the refname in their error messages, for
example, then we can simplify what write_ref_to_lockfile() puts in its
error messages.

I'll let you decide how you want to proceed, but IMHO it would be OK to
handle the immediate regression fix by just going back to dumping the
error messages to stderr. And then further cleanup can come on top.

> Yeah, I can polish what you've send. I'll work on it and send something
> soon-ish (I'm taking some time off, but its hard to stay away from the
> laptop).

Sounds good. Enjoy your holiday!

-Peff