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, sunshine@sunshineco•com,
gitster@pobox•com, David Bohman <debohman@gmail•com>
Subject: [PATCH v3 0/2] fetch: fix non-conflicting tags not being committed
Date: Sat, 08 Nov 2025 22:34:42 +0100 [thread overview]
Message-ID: <20251108-fix-tags-not-fetching-v3-0-a12ab6c4daef@gmail.com> (raw)
In-Reply-To: <20251103-fix-tags-not-fetching-v1-1-e63caeb6c113@gmail.com>
This fixes the bug reported by David Bohman [1].
The 'git-fetch(1)' uses batched updates to perform reference updates
when not using 'atomic' transactions. 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.
The first commit, extracts out common code for committing a reference
transaction and handling rejected updates. The second commit ensures
any failures would also commit pending updates.
[1]: id:CAB9xhmPcHnB2+i6WeA3doAinv7RAeGs04+n0fHLGToJq=UKUNw@mail•gmail.com
Signed-off-by: Karthik Nayak <karthik.188@gmail•com>
---
Changes in v3:
- Split the patch into two commits. One for extracting out existing code
into a new commit and the other to perform the fix.
- Add back error handling when commit via the normal flow.
- Instead of calling the commit function at every failure, make it part
of the cleanup code.
- Link to v2: https://patch.msgid.link/20251106-fix-tags-not-fetching-v2-1-610cb4b0e7c8@gmail.com
Changes in v2:
- Add a comment to explain the purpose of `commit_ref_transaction()` and
how it works.
- Also extend the same logic towards backfilling tags. While I was able
to add a test for the happy path, I couldn't figure out how to test
when `backfill_tags()` tags would fail.
Tangentially, this flow seems to only be triggered when using the now
deprecated 'branches/' remote format.
- Remove unneeded subshells from the tests.
- Link to v1: https://patch.msgid.link/20251103-fix-tags-not-fetching-v1-1-e63caeb6c113@gmail.com
---
builtin/fetch.c | 73 ++++++++++++++++++++++++++++++++++++--------------------
t/t5510-fetch.sh | 62 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 109 insertions(+), 26 deletions(-)
Karthik Nayak (2):
fetch: extract out reference committing logic
fetch: fix non-conflicting tags not being committed
Range-diff versus v2:
1: 703593ef40 ! 1: 8a1efdd999 fetch: fix non-conflicting tags not being committed
@@ Metadata
Author: Karthik Nayak <karthik.188@gmail•com>
## Commit message ##
- fetch: fix non-conflicting tags not being committed
+ fetch: extract out reference committing logic
- 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.
+ The `do_fetch()` function contains the core of the `git-fetch(1)` logic.
+ Part of this is to fetch and store references. This is done by
- 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 when using the now deprecated 'branches/'
- format for remotes.
+ 1. Creating a reference transaction (non-atomic mode uses batched
+ updates).
+ 2. Adding individual reference updates to the transaction.
+ 3. Committing the transaction.
+ 4. When using batched updates, handling the rejected updates.
- Fix this by committing the transaction even when we have an error code.
- This ensures other references are applied. Do this by extracting out the
- transaction commit code into a new `commit_ref_transaction()` function
- and using that.
+ The following commit, will fix a bug wherein fetching tags with
+ conflicts was causing other reference updates to fail. Fixing this
+ requires utilizing this logic in different regions of the function.
- Add tests to check for this regression. While here, add a missing
- cleanup from previous test.
+ In preparation of the follow up commit, extract the committing and
+ rejection handling logic into a separate function called
+ `commit_ref_transaction()`.
- Reported-by: David Bohman <debohman@gmail•com>
Signed-off-by: Karthik Nayak <karthik.188@gmail•com>
## builtin/fetch.c ##
@@ builtin/fetch.c: static void ref_transaction_rejection_handler(const char *refna
struct refspec *rs,
const struct fetch_config *config)
@@ builtin/fetch.c: static int do_fetch(struct transport *transport,
-
- if (fetch_and_consume_refs(&display_state, transport, transaction, ref_map,
- &fetch_head, config)) {
-+ /* As we're using batched updates, commit any pending updates. */
-+ if (!atomic_fetch)
-+ commit_ref_transaction(&transaction, false,
-+ transport->remote->name, &err);
- retcode = 1;
- goto cleanup;
- }
-@@ builtin/fetch.c: static int do_fetch(struct transport *transport,
- * the transaction and don't commit anything.
- */
- if (backfill_tags(&display_state, transport, transaction, tags_ref_map,
-- &fetch_head, config))
-+ &fetch_head, config)) {
-+ if (!atomic_fetch)
-+ commit_ref_transaction(&transaction, false,
-+ transport->remote->name, &err);
- retcode = 1;
-+ }
- }
-
- free_refs(tags_ref_map);
-@@ builtin/fetch.c: static int do_fetch(struct transport *transport,
if (retcode)
goto cleanup;
@@ builtin/fetch.c: static int do_fetch(struct transport *transport,
- */
- ref_transaction_free(transaction);
- transaction = NULL;
-- goto cleanup;
++ retcode = commit_ref_transaction(&transaction, atomic_fetch,
++ transport->remote->name, &err);
++ if (retcode)
+ goto cleanup;
- }
-
- if (!atomic_fetch) {
@@ builtin/fetch.c: static int do_fetch(struct transport *transport,
- goto cleanup;
- }
- }
-+ retcode = commit_ref_transaction(&transaction, atomic_fetch,
-+ transport->remote->name, &err);
commit_fetch_head(&fetch_head);
-
- ## t/t5510-fetch.sh ##
-@@ t/t5510-fetch.sh: 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 &&
-@@ t/t5510-fetch.sh: 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 with branches remote format' '
-+ test_when_finished rm -rf base repo &&
-+
-+ git init base &&
-+ git -C base commit --allow-empty -m "empty-commit" &&
-+ git -C base tag tag1 &&
-+
-+ git clone --no-tags base repo &&
-+
-+ git -C repo remote remove origin &&
-+ mkdir -p repo/.git/branches &&
-+ echo "$(cd base && pwd)#master" >repo/.git/branches/origin &&
-+
-+ git -C base commit --allow-empty -m "second empty-commit" &&
-+ git -C base tag tag2 &&
-+
-+ git -C repo fetch origin &&
-+ git -C repo for-each-ref refs/tags >out &&
-+ test_grep "tag1" out &&
-+ test_grep "tag2" out
-+'
-+
- . "$TEST_DIRECTORY"/lib-httpd.sh
- start_httpd
-
-: ---------- > 2: 1de8d8b953 fetch: fix non-conflicting tags not being committed
base-commit: a99f379adf116d53eb11957af5bab5214915f91d
change-id: 20251103-fix-tags-not-fetching-0f1621a474d4
Thanks
- Karthik
next prev parent reply other threads:[~2025-11-08 21:34 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 ` Karthik Nayak [this message]
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
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=20251108-fix-tags-not-fetching-v3-0-a12ab6c4daef@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 \
--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