[PATCH v1] remote.c: defer remote.<name>.followRemoteHEAD validation

[Colin Hinton <colinlewishinton@gmail.com>]

Continuing from 86129fd (fetch.c: defer fetch.followRemoteHEAD
validation), apply the same treatment to
remote.<name>.followRemoteHEAD, which is still validated in
handle_config(). That emits a warning for any command that loads the
remote configuration, such as "git push" or "git remote -v", even
though only "git fetch" ever consults the value.

Store the raw string in struct remote instead, and resolve it lazily
in do_fetch() through the get_follow_remote_head() helper introduced
by 86129fd. This replaces the parsed follow_remote_head and
no_warn_branch members, and the NEEDSWORK comment left behind by
86129fd is removed. "git remote set-head" now compares against the
raw string to decide whether to rewrite "always" to "warn".

Because both settings now go through the same parser,
fetch.followRemoteHEAD accepts "warn-if-not-<branch>", which
previously was only recognized for remote.<name>.followRemoteHEAD.
Document it under fetch.followRemoteHEAD, drop the remote-specific
paragraph, and add tests for the new fetch-level form.

Messages for an unrecognized or missing value no longer name the
fetch. prefix, as the helper now serves both the fetch-level and
the remote-level setting.

Signed-off-by: Colin Hinton <colinlewishinton@gmail.com>
---
 Documentation/config/fetch.adoc  |  3 +++
 Documentation/config/remote.adoc |  4 ----
 builtin/fetch.c                  | 26 +++++++++++++-------------
 builtin/remote.c                 |  2 +-
 remote.c                         | 28 ++++------------------------
 remote.h                         |  4 ++--
 t/t5510-fetch.sh                 | 28 ++++++++++++++++++++++++++++
 7 files changed, 51 insertions(+), 44 deletions(-)

diff --git a/Documentation/config/fetch.adoc b/Documentation/config/fetch.adoc
index 00435e9a16..5e70949c2b 100644
--- a/Documentation/config/fetch.adoc
+++ b/Documentation/config/fetch.adoc
@@ -139,6 +139,9 @@ remove the value for the `fetch.bundleCreationToken` value before fetching.
 `warn`;;
 	Display a warning if the remote advertises a different `HEAD` than what is
 	set locally.  Behaves like "create" if the local symbolic-ref doesn't exist.
+`warn-if-not-<branch>`;;
+	Like "warn", but do not display the warning if the remote's `HEAD`
+	is `remotes/<name>/<branch>`.
 `always`;;
 	Silently update `remotes/<name>/HEAD` whenever the remote advertises a new
 	value.
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 3a20d0f752..56b036fc98 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -171,7 +171,3 @@ remote.<name>.followRemoteHEAD::
 	`remotes/<name>/HEAD` symbolic-ref.  Overrides the value of
 	`fetch.followRemoteHEAD`.  See `fetch.followRemoteHEAD` for a description of
 	accepted values.
-+
-In addition to the values supported by `fetch.followRemoteHEAD`, this setting
-may also take on the value "warn-if-not-`$branch`", which behaves like "warn",
-but ignores the warning if the remote's `HEAD` is `remotes/<name>/$branch`.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1f76ae6242..aa112f9b5a 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -187,19 +187,23 @@ static int git_fetch_config(const char *k, const char *v,
 	return git_default_config(k, v, ctx, cb);
 }
 
-static enum follow_remote_head_settings get_follow_remote_head(const char *setting)
+static enum follow_remote_head_settings get_follow_remote_head(const char *setting,
+    const char **no_warn_branch)
 {
+	*no_warn_branch = NULL;
 	if (!setting)
-		die(_("missing value for 'fetch.followRemoteHEAD'"));
+		die(_("missing value for 'followRemoteHEAD'"));
 	else if (!strcmp(setting, "never"))
 		return FOLLOW_REMOTE_NEVER;
 	else if (!strcmp(setting, "create"))
 		return FOLLOW_REMOTE_CREATE;
 	else if (!strcmp(setting, "warn"))
 		return FOLLOW_REMOTE_WARN;
+	else if (skip_prefix(setting, "warn-if-not-", no_warn_branch))
+		return FOLLOW_REMOTE_WARN;
 	else if (!strcmp(setting, "always"))
 		return FOLLOW_REMOTE_ALWAYS;
-	warning(_("unrecognized fetch.followRemoteHEAD value '%s' ignored"), setting);
+	warning(_("unrecognized followRemoteHEAD value '%s' ignored"), setting);
 	return BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
 }
 
@@ -1760,12 +1764,11 @@ static void warn_set_head(const char *remote, const char *head_name,
 }
 
 static int set_head(const struct ref *remote_refs, struct remote *remote,
-			int follow_remote_head)
+		    int follow_remote_head, const char *no_warn_branch)
 {
 	int result = 0, create_only, baremirror, was_detached;
 	struct strbuf b_head = STRBUF_INIT, b_remote_head = STRBUF_INIT,
 		      b_local_head = STRBUF_INIT;
-	const char *no_warn_branch = remote->no_warn_branch;
 	char *head_name = NULL;
 	struct ref *ref, *matches;
 	struct ref *fetch_map = NULL, **fetch_map_tail = &fetch_map;
@@ -1955,6 +1958,7 @@ static int do_fetch(struct transport *transport,
 	struct strmap rejected_refs = STRMAP_INIT;
 	int summary_width = 0;
 	int follow_remote_head = FOLLOW_REMOTE_NEVER;
+	const char *no_warn_branch = NULL;
 
 	if (tags == TAGS_DEFAULT) {
 		if (transport->remote->fetch_tags == 2)
@@ -1978,14 +1982,10 @@ static int do_fetch(struct transport *transport,
 		if (transport->remote->fetch.nr) {
 			refspec_ref_prefixes(&transport->remote->fetch,
 					     &transport_ls_refs_options.ref_prefixes);
-			/*
-			 * See remote.c's handling of remote.<name>.followRemoteHEAD
-			 * for the analogous, still-unresolved case.
-			 */
-			if (transport->remote->follow_remote_head)
-				follow_remote_head = transport->remote->follow_remote_head;
+			if (transport->remote->follow_remote_head_seen)
+				follow_remote_head = get_follow_remote_head(transport->remote->follow_remote_head_raw, &no_warn_branch);
 			else if (config->follow_remote_head_seen)
-				follow_remote_head = get_follow_remote_head(config->follow_remote_head_raw);
+				follow_remote_head = get_follow_remote_head(config->follow_remote_head_raw, &no_warn_branch);
 			else
 				follow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
 		}
@@ -2195,7 +2195,7 @@ static int do_fetch(struct transport *transport,
 		 * 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, follow_remote_head);
+		set_head(remote_refs, transport->remote, follow_remote_head, no_warn_branch);
 	}
 
 cleanup:
diff --git a/builtin/remote.c b/builtin/remote.c
index fb7e5b114f..92eaffb4a0 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1606,7 +1606,7 @@ static int set_head(int argc, const char **argv, const char *prefix,
 	}
 	if (opt_a)
 		report_set_head_auto(argv[0], head_name, &b_local_head, was_detached);
-	if (remote->follow_remote_head == FOLLOW_REMOTE_ALWAYS) {
+	if (remote->follow_remote_head_raw && !strcmp(remote->follow_remote_head_raw, "always")) {
 		struct strbuf config_name = STRBUF_INIT;
 		strbuf_addf(&config_name,
 			"remote.%s.followremotehead", remote->name);
diff --git a/remote.c b/remote.c
index 49ee2ea318..fa230005ff 100644
--- a/remote.c
+++ b/remote.c
@@ -184,6 +184,7 @@ static void remote_clear(struct remote *remote)
 	string_list_clear(&remote->server_options, 0);
 	string_list_clear(&remote->negotiation_restrict, 0);
 	string_list_clear(&remote->negotiation_include, 0);
+	free(remote->follow_remote_head_raw);
 }
 
 static void add_merge(struct branch *branch, const char *name)
@@ -581,30 +582,9 @@ static int handle_config(const char *key, const char *value,
 		return parse_transport_option(key, value,
 					      &remote->negotiation_include);
 	} else if (!strcmp(subkey, "followremotehead")) {
-		const char *no_warn_branch;
-		/*
-		 * NEEDSWORK: this is validated/warned about here, during config
-		 * parsing, regardless of whether the fetch that triggered this
-		 * parse will ever consult it for this particular remote. See
-		 * fetch.c's deferred handling of fetch.followRemoteHEAD for the
-		 * pattern this should likely follow.
-		 */
-		if (!strcmp(value, "never"))
-			remote->follow_remote_head = FOLLOW_REMOTE_NEVER;
-		else if (!strcmp(value, "create"))
-			remote->follow_remote_head = FOLLOW_REMOTE_CREATE;
-		else if (!strcmp(value, "warn")) {
-			remote->follow_remote_head = FOLLOW_REMOTE_WARN;
-			remote->no_warn_branch = NULL;
-		} else if (skip_prefix(value, "warn-if-not-", &no_warn_branch)) {
-			remote->follow_remote_head = FOLLOW_REMOTE_WARN;
-			remote->no_warn_branch = no_warn_branch;
-		} else if (!strcmp(value, "always")) {
-			remote->follow_remote_head = FOLLOW_REMOTE_ALWAYS;
-		} else {
-			warning(_("unrecognized followRemoteHEAD value '%s' ignored"),
-				value);
-		}
+		free(remote->follow_remote_head_raw);
+		remote->follow_remote_head_raw = xstrdup_or_null(value);
+		remote->follow_remote_head_seen = 1;
 	}
 	return 0;
 }
diff --git a/remote.h b/remote.h
index cca02033b9..e80beee6bb 100644
--- a/remote.h
+++ b/remote.h
@@ -122,8 +122,8 @@ struct remote {
 	struct string_list negotiation_restrict;
 	struct string_list negotiation_include;
 
-	enum follow_remote_head_settings follow_remote_head;
-	const char *no_warn_branch;
+	char *follow_remote_head_raw;
+	int follow_remote_head_seen;
 };
 
 /**
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 17c0be499c..1e5ad50912 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -290,6 +290,34 @@ test_expect_success "fetch test followRemoteHEAD warn-if-not-branch branch is di
 	test "z$head" = "z$branch"
 '
 
+test_expect_success "fetch test default followRemoteHEAD warn-if-not-branch branch is same" '
+	git -C two rev-parse --verify refs/remotes/origin/other &&
+	git -C two remote set-head origin other &&
+	git -C two rev-parse --verify refs/remotes/origin/HEAD &&
+	git -C two rev-parse --verify refs/remotes/origin/main &&
+	test_config -C two fetch.followRemoteHEAD "warn-if-not-main" &&
+	actual=$(git -C two fetch) &&
+	test "z" = "z$actual" &&
+	head=$(git -C two rev-parse refs/remotes/origin/HEAD) &&
+	branch=$(git -C two rev-parse refs/remotes/origin/other) &&
+	test "z$head" = "z$branch"
+'
+
+test_expect_success "fetch test default followRemoteHEAD warn-if-not-branch branch is different" '
+	git -C two rev-parse --verify refs/remotes/origin/other &&
+	git -C two remote set-head origin other &&
+	git -C two rev-parse --verify refs/remotes/origin/HEAD &&
+	git -C two rev-parse --verify refs/remotes/origin/main &&
+	test_config -C two fetch.followRemoteHEAD "warn-if-not-some/different-branch" &&
+	git -C two fetch >actual &&
+	echo "${SQ}HEAD${SQ} at ${SQ}origin${SQ} is ${SQ}main${SQ}," \
+		"but we have ${SQ}other${SQ} locally." >expect &&
+	test_cmp expect actual &&
+	head=$(git -C two rev-parse refs/remotes/origin/HEAD) &&
+	branch=$(git -C two rev-parse refs/remotes/origin/other) &&
+	test "z$head" = "z$branch"
+'
+
 test_expect_success "fetch test default followRemoteHEAD always" '
 	git -C two rev-parse --verify refs/remotes/origin/other &&
 	git -C two remote set-head origin other &&
-- 
2.55.0.windows.3