Re: [PATCH v6 3/3] fetch: fix failed batched updates skipping operations
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 18, 2025, 18:03 UTC
- Message-ID
- <xmqqv7j7dxqy.fsf@gitster.g>
- In-Reply-To
- <20251118-fix-tags-not-fetching-v6-3-2a2f15fc137e@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 24 quoted lines
> 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. > > 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.
Other than the usual "unless you care about timestamps, do not use 'touch' only to create a file" applies, but other than that the added tests look quite sensible.
About the second new test piece, it is a bit surprising that we didn't have test for --set-upstream on successful fetch. It does not need REFFILES prerequisite, does it?
Thanks.
Show 97 quoted lines
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh > index 4b113d7c27..f5c87d81fe 100755 > --- a/t/t5510-fetch.sh > +++ b/t/t5510-fetch.sh > @@ -1639,6 +1639,93 @@ 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 && > + ! test -f FETCH_HEAD && > + git remote add origin ../base && > + touch refs/heads/foo.lock && > + test_must_fail git fetch -f origin "refs/heads/*:refs/heads/*" 2>err && > + test_grep "error: fetching ref refs/heads/foo failed: reference already exists" err && > + test -f FETCH_HEAD > + ) > +' > + > +test_expect_success REFFILES "upstream tracking info is added with --set-upstream" ' > + test_when_finished rm -rf base repo && > + > + git init --initial-branch=main base && > + test_commit -C base "updated" && > + > + git init --bare --initial-branch=main repo && > + ( > + cd repo && > + git remote add origin ../base && > + git fetch origin --set-upstream main && > + git config get branch.main.remote >actual && > + echo "origin" >expect && > + test_cmp expect actual > + ) > +' > + > +test_expect_success REFFILES "upstream tracking info is added even with conflicts" ' > + test_when_finished rm -rf base repo && > + > + git init --initial-branch=main base && > + test_commit -C base "updated" && > + > + git init --bare --initial-branch=main repo && > + ( > + cd repo && > + git remote add origin ../base && > + test_must_fail git config get branch.main.remote && > + > + mkdir -p refs/remotes/origin && > + touch refs/remotes/origin/main.lock && > + test_must_fail git fetch origin --set-upstream main && > + git config get branch.main.remote >actual && > + echo "origin" >expect && > + test_cmp expect actual > + ) > +' > + > +test_expect_success REFFILES "HEAD is updated even with conflicts" ' > + 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 && > + git remote add origin ../base && > + > + ! test -f refs/remotes/origin/HEAD && > + mkdir -p refs/remotes/origin && > + touch refs/remotes/origin/branch.lock && > + test_must_fail git fetch origin && > + test -f refs/remotes/origin/HEAD > + ) > +' > + > . "$TEST_DIRECTORY"/lib-httpd.sh > start_httpd