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

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

[Colin Hinton]

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

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

[Junio C Hamano]

Colin Hinton <colinlewishinton@gmail.com> writes:

> 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.

Yeah, I remember that topic.  Making these two sources of the
setting that controls the same behaviour consistent is a good thing
to do.

> 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

And the approach does sound familiar ;-).

> 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.

Obviously, when remote.origin.followRemoteHEAD has a bogus value, it
is wrong to report that the user has fetch.followRemoteHEAD
misconfigured (and vice versa), so not hardcoding "fetch." is of
course the right thing to do.  As long as it is clear to the user
what exact variable has problematic value that needs fixing, it is a
good change.  If not, the two calling sites should pass what prefix
to use so that the user sees the variable name in full.

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

This needs to be corrected, if we are reusing the function to report
for both fetch.followRemoteHEAD and remote.origin.followRemoteHEAD,
by having the caller pass what prefix to use, or what variable name
to report.

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

Same here.

>  	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")) {

Overly long line.

>  		struct strbuf config_name = STRBUF_INIT;
>  		strbuf_addf(&config_name,
>  			"remote.%s.followremotehead", remote->name);

Thanks.

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

[Matt Hunter]

On Sun Oct 18, 2026 at 7:55 PM EDT, Colin Hinton wrote:
> 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.

I think a better justification is needed for this.  The intent behind
'warn-if-not-<branch>' makes much less sense in the context of the fetch
variable imo, and if it turns out to be problematic, reverting support
later will be tricky.

See previous discussion:
<20260611060123.GA2187173@coredump.intra.peff.net>
<DJ6IBPYNOTTY.3QKEZQ28P713V@ferdinandy.com>

>
> 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.

I agree with Junio that this should be addressed.

>
> @@ -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.

I like the style change from $branch to <branch>

> @@ -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);

A couple more overly-long lines

>  			else
>  				follow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
>  		}

By the way, I had previously made a comment about moving the definition
of 'enum follow_remote_head_settings' over to the fetch file since that
is now the only place it is relevant.  Do you think it's worth changing?