Skip to content

Commit c834d1a

Browse files
peffgitster
authored andcommitted
fetch: only respect followRemoteHEAD with configured refspecs
The new followRemoteHEAD feature is triggered for almost every fetch, causing us to ask the server about the remote "HEAD" and to consider updating our local tracking HEAD symref. This patch limits the feature only to the case when we are fetching a remote using its configured refspecs (typically into its refs/remotes/ hierarchy). There are two reasons for this. One is efficiency. E.g., the fixes in 6c915c3 (fetch: do not ask for HEAD unnecessarily, 2024-12-06) and 20010b8 (fetch: avoid ls-refs only to ask for HEAD symref update, 2025-03-08) were aimed at reducing the work we do when we would not be able to update HEAD anyway. But they do not quite cover all cases. The remaining one is: git fetch origin refs/heads/foo:refs/remotes/origin/foo which _sometimes_ can update HEAD, but usually not. And that leads us to the second point, which is being simple and explainable. The code for updating the tracking HEAD symref requires both that we learned which ref the remote HEAD points at, and that the server advertised that ref to us. But because the v2 protocol narrows the server's advertisement, the command above would not typically update HEAD at all, unless it happened to point to the "foo" branch. Or even weirder, it probably _would_ update if the server is very old and supports only the v0 protocol, which always gives a full advertisement. This creates confusing behavior for the user: sometimes we may try to update HEAD and sometimes not, depending on vague rules. One option here would be to loosen the update code to accept the remote HEAD even if the server did not advertise that ref. I think that could work, but it may also lead to interesting corner cases (e.g., creating a dangling symref locally, even though the branch is not unborn on the server, if we happen not to have fetched it). So let's instead simplify the rules: we'll only consider updating the tracking HEAD symref when we're doing a full fetch of the remote's configured refs. This is easy to implement; we can just set a flag at the moment we realize we're using the configured refspecs. And we can drop the special case code added by 6c915c3 and 20010b8, since this covers those cases. The existing tests from those commits still pass. In t5505, an incidental call to "git fetch <remote> <refspec>" updated HEAD, which caused us to adjust the test in 3f763dd (fetch: set remote/HEAD if it does not exist, 2024-11-22). We can now adjust that back to how it was before the feature was added. Even though t5505 is incidentally testing our new desired behavior, we'll add an explicit test in t5510 to make sure it is covered. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 1a0413a commit c834d1a

File tree

4 files changed

+23
-21
lines changed

4 files changed

+23
-21
lines changed

Documentation/config/remote.adoc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@ the values inherited from a lower priority configuration files (e.g.
108108
`$HOME/.gitconfig`).
109109

110110
remote.<name>.followRemoteHEAD::
111-
How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`.
111+
How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
112+
when fetching using the configured refspecs of a remote.
112113
The default value is "create", which will create `remotes/<name>/HEAD`
113114
if it exists on the remote, but not locally; this will not touch an
114115
already existing local reference. Setting it to "warn" will print

builtin/fetch.c

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1691,21 +1691,6 @@ static int set_head(const struct ref *remote_refs, struct remote *remote)
16911691
return result;
16921692
}
16931693

1694-
static int uses_remote_tracking(struct transport *transport, struct refspec *rs)
1695-
{
1696-
if (!remote_is_configured(transport->remote, 0))
1697-
return 0;
1698-
1699-
if (!rs->nr)
1700-
rs = &transport->remote->fetch;
1701-
1702-
for (int i = 0; i < rs->nr; i++)
1703-
if (rs->items[i].dst)
1704-
return 1;
1705-
1706-
return 0;
1707-
}
1708-
17091694
static int do_fetch(struct transport *transport,
17101695
struct refspec *rs,
17111696
const struct fetch_config *config)
@@ -1720,6 +1705,7 @@ static int do_fetch(struct transport *transport,
17201705
TRANSPORT_LS_REFS_OPTIONS_INIT;
17211706
struct fetch_head fetch_head = { 0 };
17221707
struct strbuf err = STRBUF_INIT;
1708+
int do_set_head = 0;
17231709

17241710
if (tags == TAGS_DEFAULT) {
17251711
if (transport->remote->fetch_tags == 2)
@@ -1740,9 +1726,11 @@ static int do_fetch(struct transport *transport,
17401726
} else {
17411727
struct branch *branch = branch_get(NULL);
17421728

1743-
if (transport->remote->fetch.nr)
1729+
if (transport->remote->fetch.nr) {
17441730
refspec_ref_prefixes(&transport->remote->fetch,
17451731
&transport_ls_refs_options.ref_prefixes);
1732+
do_set_head = 1;
1733+
}
17461734
if (branch_has_merge_config(branch) &&
17471735
!strcmp(branch->remote_name, transport->remote->name)) {
17481736
int i;
@@ -1765,8 +1753,7 @@ static int do_fetch(struct transport *transport,
17651753
strvec_push(&transport_ls_refs_options.ref_prefixes,
17661754
"refs/tags/");
17671755

1768-
if (transport_ls_refs_options.ref_prefixes.nr &&
1769-
uses_remote_tracking(transport, rs))
1756+
if (do_set_head)
17701757
strvec_push(&transport_ls_refs_options.ref_prefixes,
17711758
"HEAD");
17721759

@@ -1918,7 +1905,7 @@ static int do_fetch(struct transport *transport,
19181905
"you need to specify exactly one branch with the --set-upstream option"));
19191906
}
19201907
}
1921-
if (set_head(remote_refs, transport->remote))
1908+
if (do_set_head && set_head(remote_refs, transport->remote))
19221909
;
19231910
/*
19241911
* Way too many cases where this can go wrong

t/t5505-remote.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ test_expect_success 'set-head --auto has no problem w/multiple HEADs' '
499499
cd test &&
500500
git fetch two "refs/heads/*:refs/remotes/two/*" &&
501501
git remote set-head --auto two >output 2>&1 &&
502-
echo "${SQ}two/HEAD${SQ} is unchanged and points to ${SQ}main${SQ}" >expect &&
502+
echo "${SQ}two/HEAD${SQ} is now created and points to ${SQ}main${SQ}" >expect &&
503503
test_cmp expect output
504504
)
505505
'

t/t5510-fetch.sh

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,20 @@ test_expect_success "fetch test followRemoteHEAD always" '
250250
)
251251
'
252252

253+
test_expect_success 'followRemoteHEAD does not kick in with refspecs' '
254+
test_when_finished "git config unset remote.origin.followRemoteHEAD" &&
255+
(
256+
cd "$D" &&
257+
cd two &&
258+
git remote set-head origin other &&
259+
git config set remote.origin.followRemoteHEAD always &&
260+
git fetch origin refs/heads/main:refs/remotes/origin/main &&
261+
echo refs/remotes/origin/other >expect &&
262+
git symbolic-ref refs/remotes/origin/HEAD >actual &&
263+
test_cmp expect actual
264+
)
265+
'
266+
253267
test_expect_success 'fetch --prune on its own works as expected' '
254268
cd "$D" &&
255269
git clone . prune &&

0 commit comments

Comments
 (0)