public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Karthik Nayak <karthik.188@gmail•com>
To: git@vger•kernel.org
Cc: Karthik Nayak <karthik.188@gmail•com>,
	jltobler@gmail•com, ps@pks•im,  gitster@pobox•com,
	David Bohman <debohman@gmail•com>
Subject: [PATCH v5 2/2] fetch: fix non-conflicting tags not being committed
Date: Thu, 13 Nov 2025 14:38:37 +0100	[thread overview]
Message-ID: <20251113-fix-tags-not-fetching-v5-2-371ea7ec638d@gmail.com> (raw)
In-Reply-To: <20251113-fix-tags-not-fetching-v5-0-371ea7ec638d@gmail.com>

The commit 0e358de64a (fetch: use batched reference updates, 2025-05-19)
updated the 'git-fetch(1)' command to use batched updates. This batches
updates to gain performance improvements. When fetching references, each
update is added to the transaction. Finally, when committing, individual
updates are allowed to fail with reason, while the transaction itself
succeeds.

One scenario which was missed here, was fetching tags. When fetching
conflicting tags, the `fetch_and_consume_refs()` function returns '1',
which skipped committing the transaction and directly jumped to the
cleanup section. This mean that no updates were applied. This also
extends to backfilling tags which is done when fetching specific
refspecs which contains tags in their history.

Fix this by committing the transaction when we have an error code and
not using an atomic transaction. This ensures other references are
applied even when some updates fail.

The cleanup section is reached with `retcode` set in several scenarios:

   - `truncate_fetch_head()`, `open_fetch_head()` and `prune_refs()` set
     `retcode` before the transaction is created, so no commit is
     attempted.

   - `fetch_and_consume_refs()` and `backfill_tags()` are the primary
     cases this fix targets, both setting a positive `retcode` to
     trigger the committing of the transaction.

This simplifies error handling and ensures future modifications to
`do_fetch()` don't need special handling for batched updates.

Add tests to check for this regression. While here, add a missing
cleanup from previous test.

Reported-by: David Bohman <debohman@gmail•com>
Helped-by: Patrick Steinhardt <ps@pks•im>
Signed-off-by: Karthik Nayak <karthik.188@gmail•com>
---
 builtin/fetch.c  |  8 ++++++++
 t/t5510-fetch.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index f90179040b..b19fa8e966 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1957,6 +1957,14 @@ static int do_fetch(struct transport *transport,
 	}
 
 cleanup:
+	/*
+	 * When using batched updates, we want to commit the non-rejected
+	 * updates and also handle the rejections.
+	 */
+	if (retcode && !atomic_fetch && transaction)
+		commit_ref_transaction(&transaction, false,
+				       transport->remote->name, &err);
+
 	if (retcode) {
 		if (err.len) {
 			error("%s", err.buf);
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index b7059cccaa..4b113d7c27 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1552,6 +1552,7 @@ test_expect_success CASE_INSENSITIVE_FS,REFFILES 'D/F conflict on case insensiti
 '
 
 test_expect_success REFFILES 'D/F conflict on case sensitive filesystem with lock' '
+	test_when_finished rm -rf base repo &&
 	(
 		git init --ref-format=reftable base &&
 		cd base &&
@@ -1577,6 +1578,67 @@ test_expect_success REFFILES 'D/F conflict on case sensitive filesystem with loc
 	)
 '
 
+test_expect_success 'fetch --tags fetches existing tags' '
+	test_when_finished rm -rf base repo &&
+
+	git init base &&
+	git -C base commit --allow-empty -m "empty-commit" &&
+
+	git clone --bare base repo &&
+
+	git -C base tag tag-1 &&
+	git -C repo for-each-ref >out &&
+	test_grep ! "tag-1" out &&
+	git -C repo fetch --tags &&
+	git -C repo for-each-ref >out &&
+	test_grep "tag-1" out
+'
+
+test_expect_success 'fetch --tags fetches non-conflicting tags' '
+	test_when_finished rm -rf base repo &&
+
+	git init base &&
+	git -C base commit --allow-empty -m "empty-commit" &&
+	git -C base tag tag-1 &&
+
+	git clone --bare base repo &&
+
+	git -C base tag tag-2 &&
+	git -C repo for-each-ref >out &&
+	test_grep ! "tag-2" out &&
+
+	git -C base commit --allow-empty -m "second empty-commit" &&
+	git -C base tag -f tag-1 &&
+
+	test_must_fail git -C repo fetch --tags 2>out &&
+	test_grep "tag-1  (would clobber existing tag)" out &&
+	git -C repo for-each-ref >out &&
+	test_grep "tag-2" out
+'
+
+test_expect_success "backfill tags when providing a refspec" '
+	test_when_finished rm -rf source target &&
+
+	git init source &&
+	git -C source commit --allow-empty --message common &&
+	git clone file://"$(pwd)"/source target &&
+	(
+	    cd source &&
+	    test_commit history &&
+	    test_commit fetch-me
+	) &&
+
+	# The "history" tag is backfilled eventhough we requested
+	# to only fetch HEAD
+	git -C target fetch origin HEAD:branch &&
+	git -C target tag -l >actual &&
+	cat >expect <<-\EOF &&
+	fetch-me
+	history
+	EOF
+	test_cmp expect actual
+'
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 

-- 
2.51.0


  parent reply	other threads:[~2025-11-13 13:38 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   ` Karthik Nayak [this message]
2025-11-13 21:23     ` [PATCH v5 2/2] fetch: fix non-conflicting tags not being committed 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
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=20251113-fix-tags-not-fetching-v5-2-371ea7ec638d@gmail.com \
    --to=karthik.188@gmail$(echo .)com \
    --cc=debohman@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=jltobler@gmail$(echo .)com \
    --cc=ps@pks$(echo .)im \
    /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