From: Jeff King <peff@peff.net>
Date: Wed, 24 Dec 2025 08:21:16 GMT
Subject: Re: Possible regression: lost diagnostic message when pushing non-commit objects to refs/heads/*
Message-ID: <20251224082116.GA1946629@coredump.intra.peff.net>
In-Reply-To: <20251224081214.GA1879908@coredump.intra.peff.net>

On Wed, Dec 24, 2025 at 03:12:14AM -0500, Jeff King wrote:

> But in the new batched world that allows partial-batch failures, we
> throw it away. The problem (at least for the files backend) is this code
> in files_transaction_prepare():
> 
>           ret = lock_ref_for_update(refs, update, i, transaction,
>                                     head_ref, &refnames_to_check,
>                                     err);
>           if (ret) {
>                   if (ref_transaction_maybe_set_rejected(transaction, i, ret)) {
>                           strbuf_reset(err);
>                           ret = 0;
> 
>                           continue;
>                   }
>                   goto cleanup;
>           }

BTW, you found the regression via receive-pack, but as you can see here
it is really a problem for any batched ref-update caller that sets the
ALLOW_FAILURE flag. So the original sin is not from the commit you found
via bisect, but 23fc8e4f61 (refs: implement batch reference update
support, 2025-04-08). And it affects fetch, too:

  $ git.v2.50.0 fetch . v1.0.0:refs/heads/foo
  error: cannot update ref 'refs/heads/foo': trying to write non-commit object f665776185ad074b236c00751d666da7d1977dbe to branch 'refs/heads/foo'
  From .
   ! [new tag]               v1.0.0     -> foo  (unable to update local ref)

  $ git.v2.51.0 fetch . v1.0.0:refs/heads/foo
  From .
   * [new tag]               v1.0.0     -> foo
  error: fetching ref refs/heads/foo failed: invalid new value provided

Actually, I think there is another bug lurking there for fetch. We do
not even mark the failure in the status output anymore!

And I guess "update-ref --batch-updates" suffers from the same lack of
detail:

  $ echo create refs/heads/foo v1.0.0 | git update-ref --batch-updates --stdin
  rejected refs/heads/foo f665776185ad074b236c00751d666da7d1977dbe 0000000000000000000000000000000000000000 invalid new value provided

though it is not technically a regression since the option to ask for
ALLOW_FAILURE did not even exist before --batch-updates. It would be
nice if it gave more details (whether to stderr or in the
machine-readable output).

-Peff