On Fri, Nov 21, 2025 at 12:13:47PM +0100, Karthik Nayak wrote:

Fix a regression introduced with batched updates in 0e358de64a (fetch:
use batched reference updates, 2025-05-19) when fetching references. In
the `do_fetch()` function, we jump to cleanup if committing the
transaction fails, regardless of whether using batched or atomic
updates. This skips three subsequent operations:

  - Update 'FETCH_HEAD' as part of `commit_fetch_head()`.

  - Add upstream tracking information via `set_upstream()`.

  - Setting remote 'HEAD' values when `do_set_head` is true.

For atomic updates, this is expected behavior. For batched updates,
we want to continue with these operations even if some refs fail to
update.

Skipping `commit_fetch_head()` isn't actually a regression because
'FETCH_HEAD' is already updated via `append_fetch_head()` when not
using '--atomic'. However, we add a test to validate this behavior.

This raises the question what happens when this function _does_ get
executed again. But we're guarding us:

    static void commit_fetch_head(struct fetch_head *fetch_head)
    {
        if (!fetch_head->fp || !atomic_fetch)
            return;
        strbuf_write(&fetch_head->buf, fetch_head->fp);
    }

And as we only `goto cleanup` in case `retcode && atomic_fetch` we know
that the above function will exit early. So this is a no-op change
indeed.

Skipping the other two operations (upstream tracking and remote HEAD)
is a regression. Fix this by only jumping to cleanup when using
'--atomic', allowing batched updates to continue with post-fetch
operations. Add tests to prevent future regressions.

Makes sense.

Hm. Is this compatible with all supported systems? We typically write
this as:

    : >refs/heads/foo.lock

But I have to acknowledge that I only do this because some people that
are more knowledgeable than I am know that we need this.

Other than that I'm happy with the current state of this patch series.
If the above turns out to be a non-issue I think it should be ready for
'next'.

Thanks!

Patrick