[PATCH 1/2] fetch: only respect followRemoteHEAD with configured refspecs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 6c915c3f85 (fetch: do not ask for
HEAD unnecessarily, 2024-12-06) and 20010b8c20 (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 6c915c3f85 and 20010b8c20, 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 3f763ddf28 (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 <peff@xxxxxxxx>
---
The test is weird with the way it calls "cd", but it matches the nearby
ones. I think that could all be simplified, but I'll leave that for
another day (or never).

 Documentation/config/remote.adoc |  3 ++-
 builtin/fetch.c                  | 25 ++++++-------------------
 t/t5505-remote.sh                |  2 +-
 t/t5510-fetch.sh                 | 14 ++++++++++++++
 4 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 25fe219d10..91e46f66f5 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -108,7 +108,8 @@ the values inherited from a lower priority configuration files (e.g.
 `$HOME/.gitconfig`).
 
 remote.<name>.followRemoteHEAD::
-	How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`.
+	How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
+	when fetching using the configured refspecs of a remote.
 	The default value is "create", which will create `remotes/<name>/HEAD`
 	if it exists on the remote, but not locally; this will not touch an
 	already existing local reference. Setting it to "warn" will print
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 02af505469..66f5ae31b6 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1691,21 +1691,6 @@ static int set_head(const struct ref *remote_refs, struct remote *remote)
 	return result;
 }
 
-static int uses_remote_tracking(struct transport *transport, struct refspec *rs)
-{
-	if (!remote_is_configured(transport->remote, 0))
-		return 0;
-
-	if (!rs->nr)
-		rs = &transport->remote->fetch;
-
-	for (int i = 0; i < rs->nr; i++)
-		if (rs->items[i].dst)
-			return 1;
-
-	return 0;
-}
-
 static int do_fetch(struct transport *transport,
 		    struct refspec *rs,
 		    const struct fetch_config *config)
@@ -1720,6 +1705,7 @@ static int do_fetch(struct transport *transport,
 		TRANSPORT_LS_REFS_OPTIONS_INIT;
 	struct fetch_head fetch_head = { 0 };
 	struct strbuf err = STRBUF_INIT;
+	int do_set_head = 0;
 
 	if (tags == TAGS_DEFAULT) {
 		if (transport->remote->fetch_tags == 2)
@@ -1740,9 +1726,11 @@ static int do_fetch(struct transport *transport,
 	} else {
 		struct branch *branch = branch_get(NULL);
 
-		if (transport->remote->fetch.nr)
+		if (transport->remote->fetch.nr) {
 			refspec_ref_prefixes(&transport->remote->fetch,
 					     &transport_ls_refs_options.ref_prefixes);
+			do_set_head = 1;
+		}
 		if (branch_has_merge_config(branch) &&
 		    !strcmp(branch->remote_name, transport->remote->name)) {
 			int i;
@@ -1765,8 +1753,7 @@ static int do_fetch(struct transport *transport,
 		strvec_push(&transport_ls_refs_options.ref_prefixes,
 			    "refs/tags/");
 
-	if (transport_ls_refs_options.ref_prefixes.nr &&
-	    uses_remote_tracking(transport, rs))
+	if (do_set_head)
 		strvec_push(&transport_ls_refs_options.ref_prefixes,
 			    "HEAD");
 
@@ -1918,7 +1905,7 @@ static int do_fetch(struct transport *transport,
 				  "you need to specify exactly one branch with the --set-upstream option"));
 		}
 	}
-	if (set_head(remote_refs, transport->remote))
+	if (do_set_head && set_head(remote_refs, transport->remote))
 		;
 		/*
 		 * Way too many cases where this can go wrong
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index bb7e0c6879..fed5823e85 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -499,7 +499,7 @@ test_expect_success 'set-head --auto has no problem w/multiple HEADs' '
 		cd test &&
 		git fetch two "refs/heads/*:refs/remotes/two/*" &&
 		git remote set-head --auto two >output 2>&1 &&
-		echo "${SQ}two/HEAD${SQ} is unchanged and points to ${SQ}main${SQ}" >expect &&
+		echo "${SQ}two/HEAD${SQ} is now created and points to ${SQ}main${SQ}" >expect &&
 		test_cmp expect output
 	)
 '
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 5f350facf5..ad23dd11ef 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -250,6 +250,20 @@ test_expect_success "fetch test followRemoteHEAD always" '
 	)
 '
 
+test_expect_success 'followRemoteHEAD does not kick in with refspecs' '
+	test_when_finished "git config unset remote.origin.followRemoteHEAD" &&
+	(
+		cd "$D" &&
+		cd two &&
+		git remote set-head origin other &&
+		git config set remote.origin.followRemoteHEAD always &&
+		git fetch origin refs/heads/main:refs/remotes/origin/main &&
+		echo refs/remotes/origin/other >expect &&
+		git symbolic-ref refs/remotes/origin/HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_expect_success 'fetch --prune on its own works as expected' '
 	cd "$D" &&
 	git clone . prune &&
-- 
2.49.0.390.gc9996a6a6c





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux