[PATCH v3 0/2] fetch: fix non-conflicting tags not being committed
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Nov 8, 2025, 21:34 UTC
- Message-ID
- <20251108-fix-tags-not-fetching-v3-0-a12ab6c4daef@gmail.com>
- 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 committedRange-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 committedbase-commit: a99f379adf116d53eb11957af5bab5214915f91d change-id: 20251103-fix-tags-not-fetching-0f1621a474d4
Thanks - Karthik