From: Patrick Steinhardt <ps@pks•im>
To: Karthik Nayak <karthik.188@gmail•com>
Cc: git@vger•kernel.org, jltobler@gmail•com, gitster@pobox•com,
sunshine@sunshineco•com
Subject: Re: [PATCH v8 3/3] fetch: fix failed batched updates skipping operations
Date: Mon, 1 Dec 2025 13:58:19 +0100 [thread overview]
Message-ID: <aS2Q6y_hnwBxycGk@pks.im> (raw)
In-Reply-To: <20251121-fix-tags-not-fetching-v8-3-23b53a8a8334@gmail.com>
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.
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index 4b113d7c27..a1ca4e1ac7 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -1639,6 +1639,94 @@ test_expect_success "backfill tags when providing a refspec" '
> test_cmp expect actual
> '
>
> +test_expect_success REFFILES "FETCH_HEAD is updated even if ref updates fail" '
> + test_when_finished rm -rf base repo &&
> +
> + git init base &&
> + (
> + cd base &&
> + test_commit "updated" &&
> +
> + git update-ref refs/heads/foo @ &&
> + git update-ref refs/heads/branch @
> + ) &&
> +
> + git init --bare repo &&
> + (
> + cd repo &&
> + rm -f FETCH_HEAD &&
> + git remote add origin ../base &&
> + >refs/heads/foo.lock &&
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
next prev parent reply other threads:[~2025-12-01 12:58 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-03 13:49 [PATCH] fetch: fix non-conflicting tags not being committed Karthik Nayak
2025-11-03 17:53 ` Eric Sunshine
2025-11-03 21:22 ` Karthik Nayak
2025-11-03 20:52 ` Justin Tobler
2025-11-06 8:39 ` [PATCH v2] " Karthik Nayak
2025-11-06 11:50 ` Patrick Steinhardt
2025-11-06 18:56 ` Junio C Hamano
2025-11-07 13:15 ` Karthik Nayak
2025-11-07 14:07 ` Patrick Steinhardt
2025-11-07 15:13 ` Karthik Nayak
2025-11-06 22:10 ` Justin Tobler
2025-11-07 14:01 ` Karthik Nayak
2025-11-08 21:34 ` [PATCH v3 0/2] " Karthik Nayak
2025-11-08 21:34 ` [PATCH v3 1/2] fetch: extract out reference committing logic Karthik Nayak
2025-11-10 7:34 ` Patrick Steinhardt
2025-11-10 13:11 ` Karthik Nayak
2025-11-08 21:34 ` [PATCH v3 2/2] fetch: fix non-conflicting tags not being committed Karthik Nayak
2025-11-10 7:34 ` Patrick Steinhardt
2025-11-10 13:23 ` Karthik Nayak
2025-11-11 13:27 ` [PATCH v4 0/2] " Karthik Nayak
2025-11-11 13:27 ` [PATCH v4 1/2] fetch: extract out reference committing logic Karthik Nayak
2025-11-11 13:27 ` [PATCH v4 2/2] fetch: fix non-conflicting tags not being committed Karthik Nayak
2025-11-12 6:16 ` Patrick Steinhardt
2025-11-12 8:52 ` Karthik Nayak
2025-11-12 16:34 ` Junio C Hamano
2025-11-13 13:38 ` [PATCH v5 0/2] " Karthik Nayak
2025-11-13 13:38 ` [PATCH v5 1/2] fetch: extract out reference committing logic Karthik Nayak
2025-11-13 13:38 ` [PATCH v5 2/2] fetch: fix non-conflicting tags not being committed Karthik Nayak
2025-11-13 21:23 ` Junio C Hamano
2025-11-15 22:16 ` Karthik Nayak
2025-11-17 0:02 ` Junio C Hamano
2025-11-17 15:38 ` Karthik Nayak
2025-11-18 11:27 ` [PATCH v6 0/3] " Karthik Nayak
2025-11-18 11:27 ` [PATCH v6 1/3] fetch: extract out reference committing logic Karthik Nayak
2025-11-18 11:27 ` [PATCH v6 2/3] fetch: fix non-conflicting tags not being committed Karthik Nayak
2025-11-18 11:27 ` [PATCH v6 3/3] fetch: fix failed batched updates skipping operations Karthik Nayak
2025-11-18 18:03 ` Junio C Hamano
2025-11-19 8:59 ` Karthik Nayak
2025-11-19 21:46 ` [PATCH v7 0/3] fetch: fix non-conflicting tags not being committed Karthik Nayak
2025-11-19 21:46 ` [PATCH v7 1/3] fetch: extract out reference committing logic Karthik Nayak
2025-11-19 21:46 ` [PATCH v7 2/3] fetch: fix non-conflicting tags not being committed Karthik Nayak
2025-11-19 21:46 ` [PATCH v7 3/3] fetch: fix failed batched updates skipping operations Karthik Nayak
2025-11-19 22:20 ` Eric Sunshine
2025-11-19 23:08 ` Junio C Hamano
2025-11-21 11:00 ` Karthik Nayak
2025-11-21 11:13 ` [PATCH v8 0/3] fetch: fix non-conflicting tags not being committed Karthik Nayak
2025-11-21 11:13 ` [PATCH v8 1/3] fetch: extract out reference committing logic Karthik Nayak
2025-11-21 11:13 ` [PATCH v8 2/3] fetch: fix non-conflicting tags not being committed Karthik Nayak
2025-12-01 12:58 ` Patrick Steinhardt
2025-12-02 22:26 ` Karthik Nayak
2025-11-21 11:13 ` [PATCH v8 3/3] fetch: fix failed batched updates skipping operations Karthik Nayak
2025-12-01 12:58 ` Patrick Steinhardt [this message]
2025-12-02 22:35 ` Karthik Nayak
2025-11-21 19:58 ` [PATCH v8 0/3] fetch: fix non-conflicting tags not being committed Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aS2Q6y_hnwBxycGk@pks.im \
--to=ps@pks$(echo .)im \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=jltobler@gmail$(echo .)com \
--cc=karthik.188@gmail$(echo .)com \
--cc=sunshine@sunshineco$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox