Merge branch 'jk/fetch-follow-remote-head-fix'

"git fetch [<remote>]" with only the configured fetch refspec
should be the only thing to update refs/remotes/<remote>/HEAD,
but the code was overly eager to do so in other cases.

* jk/fetch-follow-remote-head-fix:
  fetch: make set_head() call easier to read
  fetch: don't ask for remote HEAD if followRemoteHEAD is "never"
  fetch: only respect followRemoteHEAD with configured refspecs

Author: gitster

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

builtin/fetch.c

@@ -1643,9 +1643,6 @@ static int set_head(const struct ref *remote_refs, struct remote *remote)
 		string_list_append(&heads, strip_refshead(ref->name));
 	}
 
-	if (follow_remote_head == FOLLOW_REMOTE_NEVER)
-		goto cleanup;
-
 	if (!heads.nr)
 		result = 1;
 	else if (heads.nr > 1)
@@ -1691,21 +1688,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 +1702,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 +1723,12 @@ 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);
+			if (transport->remote->follow_remote_head != FOLLOW_REMOTE_NEVER)
+				do_set_head = 1;
+		}
 		if (branch_has_merge_config(branch) &&
 		    !strcmp(branch->remote_name, transport->remote->name)) {
 			int i;
@@ -1765,8 +1751,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");
 
@@ -1925,12 +1910,13 @@ 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) {
 		/*
-		 * Way too many cases where this can go wrong
-		 * so let's just fail silently for now.
+		 * Way too many cases where this can go wrong so let's just
+		 * ignore errors and fail silently for now.
 		 */
+		set_head(remote_refs, transport->remote);
+	}
 
 cleanup:
 	if (retcode) {

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
 	)
 '

t/t5510-fetch.sh

@@ -125,7 +125,10 @@ test_expect_success "fetch test followRemoteHEAD never" '
 		cd two &&
 		git update-ref --no-deref -d refs/remotes/origin/HEAD &&
 		git config set remote.origin.followRemoteHEAD "never" &&
-		git fetch &&
+		GIT_TRACE_PACKET=$PWD/trace.out git fetch &&
+		# Confirm that we do not even ask for HEAD when we are
+		# not going to act on it.
+		test_grep ! "ref-prefix HEAD" trace.out &&
 		test_must_fail git rev-parse --verify refs/remotes/origin/HEAD
 	)
 '
@@ -256,6 +259,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 &&