remote.c: defer remote.<name>.followRemoteHEAD validation

Started by Colin Hinton on Oct 18, 2026 · 19 messages from 3 authors · Last reply Nov 8, 2026

Subsystems (2)

Maintainers (2)

HOTtoday

open the first message on its own page

  1. [PATCH v2] remote.c: defer remote.<name>.followRemoteHEAD validation
    Related patches
Message details and related content
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 <redacted>
---
 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
Message details and related content
Colin Hinton [off-list ref] 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.
quoted hunk
-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.
quoted hunk
 	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.
quoted hunk
 	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);

22 more quoted lines

Overly long line.
 		struct strbuf config_name = STRBUF_INIT;
 		strbuf_addf(&config_name,
 			"remote.%s.followremotehead", remote->name);
Thanks.
Message details and related content
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:
[off-list ref]
[off-list ref]
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.
quoted hunk
@@ -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>
quoted hunk
@@ -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?
Message details and related content
Matt & Junio
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.
I don't have a particularly strong justification for this,  and was
already on the fence about including this change or not.
I likely will revert this addition for fetch, and make it so that this
only works for remote.
quoted
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.
I also went back and forth on how valuable this is, since if it's a
typo it should be clear which variable is incorrect.
But being detailed doesn't hurt, and in the event of an empty or " "
mistake then it will be even easier to identify the configuration
error.
quoted
@@ -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>
If I am reverting the change to fetch and warn-if-not, I will need to
revert the .adoc changes too, but I can make this change back in the
remote.adoc if we believe this is an improvement.
quoted
+                             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
I will clean these up, and the ones Junio mentioned, thanks for
pointing these out.
quoted
                      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?
Given the change I am proposing in remote.c where the enum is no
longer used, I do see the value in changing the scope of the enum, I
will experiment with this and update this in v2.

-Colin Hinton
Message details and related content
On Wed Oct 21, 2026 at 12:30 PM EDT, Colin Hinton wrote:
quoted
I like the style change from $branch to <branch>
If I am reverting the change to fetch and warn-if-not, I will need to
revert the .adoc changes too, but I can make this change back in the
remote.adoc if we believe this is an improvement.
It's more consistent, which is why I'd be in favor

thanks

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

Message details and related content
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".

Moved enum follow_remote_head_settings to builtin/fetch.c as it is no
longer referenced in remote.

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

Signed-off-by: Colin Hinton <redacted>
---
 Documentation/config/remote.adoc |  4 +--
 builtin/fetch.c                  | 45 +++++++++++++++++++++-----------
 builtin/remote.c                 |  3 ++-
 remote.c                         | 28 +++-----------------
 remote.h                         | 13 ++-------
 t/t5510-fetch.sh                 | 18 +++++++++++++
 6 files changed, 58 insertions(+), 53 deletions(-)
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 3a20d0f752..d19fe88ef3 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -173,5 +173,5 @@ remote.<name>.followRemoteHEAD::
 	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`.
+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..bb59e70865 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -65,6 +65,15 @@ enum display_format {
 	DISPLAY_FORMAT_PORCELAIN,
 };
 
+#define BUILTIN_FOLLOW_REMOTE_HEAD_DFLT FOLLOW_REMOTE_CREATE
+enum follow_remote_head_settings {
+	FOLLOW_REMOTE_UNCONFIGURED = 0,
+	FOLLOW_REMOTE_NEVER,
+	FOLLOW_REMOTE_CREATE,
+	FOLLOW_REMOTE_WARN,
+	FOLLOW_REMOTE_ALWAYS,
+};
+
 struct display_state {
 	struct strbuf buf;
 
@@ -187,19 +196,22 @@ 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 *key, const char **no_warn_branch)
 {
 	if (!setting)
-		die(_("missing value for 'fetch.followRemoteHEAD'"));
+		die(_("missing value for '%s'"), key);
 	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 (no_warn_branch && 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 %s value '%s' ignored"), key, setting);
 	return BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
 }
 
@@ -1760,12 +1772,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 +1966,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,16 +1990,19 @@ 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;
-			else if (config->follow_remote_head_seen)
-				follow_remote_head = get_follow_remote_head(config->follow_remote_head_raw);
-			else
+			if (transport->remote->follow_remote_head_seen) {
+				char *key = xstrfmt("remote.%s.followRemoteHEAD", transport->remote->name);
+				follow_remote_head = get_follow_remote_head(
+					transport->remote->follow_remote_head_raw,
+					key, &no_warn_branch);
+				free(key);
+			} else if (config->follow_remote_head_seen) {
+				follow_remote_head = get_follow_remote_head(
+					config->follow_remote_head_raw,
+					"fetch.followRemoteHEAD", NULL);
+			} else {
 				follow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
+			}
 		}
 		if (branch && branch_has_merge_config(branch) &&
 		    !strcmp(branch->remote_name, transport->remote->name)) {
@@ -2195,7 +2210,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..aad92c7dcd 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1606,7 +1606,8 @@ 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..b95f00df2d 100644
--- a/remote.h
+++ b/remote.h
@@ -62,15 +62,6 @@ struct remote_state {
 void remote_state_clear(struct remote_state *remote_state);
 struct remote_state *remote_state_new(void);
 
-#define BUILTIN_FOLLOW_REMOTE_HEAD_DFLT FOLLOW_REMOTE_CREATE
-enum follow_remote_head_settings {
-	FOLLOW_REMOTE_UNCONFIGURED = 0,
-	FOLLOW_REMOTE_NEVER,
-	FOLLOW_REMOTE_CREATE,
-	FOLLOW_REMOTE_WARN,
-	FOLLOW_REMOTE_ALWAYS,
-};
-
 struct remote {
 	struct hashmap_entry ent;
 
@@ -122,8 +113,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..5b97dced12 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -290,6 +290,24 @@ 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 is not supported" '
+	git -C two rev-parse --verify refs/remotes/origin/other &&
+	git -C two remote set-head origin other &&
+	test_config -C two fetch.followRemoteHEAD "warn-if-not-main" &&
+	git -C two fetch >out 2>err &&
+	test_must_be_empty out &&
+	test_grep "unrecognized fetch.followRemoteHEAD value ${SQ}warn-if-not-main${SQ} ignored" err &&
+	echo refs/remotes/origin/other >expect &&
+	git -C two symbolic-ref refs/remotes/origin/HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "fetch test followRemoteHEAD unrecognized value names the remote" '
+   	test_config -C two remote.origin.followRemoteHEAD bogus &&
+   	git -C two fetch 2>err &&
+   	test_grep "unrecognized remote.origin.followRemoteHEAD value ${SQ}bogus${SQ} ignored" err
+'
+
 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 v2] remote.c: defer remote.<name>.followRemoteHEAD validation

Message details and related content
Colin Hinton [off-list ref] 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.

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

Moved enum follow_remote_head_settings to builtin/fetch.c as it is no
longer referenced in remote.

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

Signed-off-by: Colin Hinton <redacted>
---
 Documentation/config/remote.adoc |  4 +--
 builtin/fetch.c                  | 45 +++++++++++++++++++++-----------
 builtin/remote.c                 |  3 ++-
 remote.c                         | 28 +++-----------------
 remote.h                         | 13 ++-------
 t/t5510-fetch.sh                 | 18 +++++++++++++
 6 files changed, 58 insertions(+), 53 deletions(-)
This looks mostly good, except that I see a few places where the
indentation is a bit off.
quoted hunk
-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 *key, const char **no_warn_branch)
Perhaps indent with a tab (instead of four spaces).
quoted hunk
 {
 	if (!setting)
-		die(_("missing value for 'fetch.followRemoteHEAD'"));
+		die(_("missing value for '%s'"), key);
 	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 (no_warn_branch && 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 %s value '%s' ignored"), key, setting);
 	return BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
 }
 
@@ -1760,12 +1772,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 +1966,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) {

16 more quoted lines

This line is +l00 columns of long.  Line wrap?
quoted hunk
+test_expect_success "fetch test followRemoteHEAD unrecognized value names the remote" '
+   	test_config -C two remote.origin.followRemoteHEAD bogus &&
+   	git -C two fetch 2>err &&
+   	test_grep "unrecognized remote.origin.followRemoteHEAD value ${SQ}bogus${SQ} ignored" err
+'
These lines have space-before-tab whitespace errors.
 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 &&

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

Message details and related content
On Fri Oct 23, 2026 at 2:50 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".

Moved enum follow_remote_head_settings to builtin/fetch.c as it is no
longer referenced in remote.

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

Signed-off-by: Colin Hinton <redacted>
---
 Documentation/config/remote.adoc |  4 +--
 builtin/fetch.c                  | 45 +++++++++++++++++++++-----------
 builtin/remote.c                 |  3 ++-
 remote.c                         | 28 +++-----------------
 remote.h                         | 13 ++-------
 t/t5510-fetch.sh                 | 18 +++++++++++++
 6 files changed, 58 insertions(+), 53 deletions(-)
Besides Junio's whitespace notes, I have no additional comments.  Looks
good.

Thanks

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

Message details and related content
"Matt Hunter" [off-list ref] writes:
Besides Junio's whitespace notes, I have no additional comments.  Looks
good.
Thanks, Matt, for an extra set of eyes.

This however has some textual conflict with Harald's work to limit
the set of branches to fetch with refmap which keeps this topic out
of 'seen'.  If the hn/fetch-refmap topic is solid enough, it might
make sense to wait for it to hit 'next', and then rebase this patch
on top of the result of merging the hn/fetch-refmap topic into
'master', to avoid mismerges.

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

Message details and related content
Colin Hinton [off-list ref] writes:

One more thing.
quoted hunk
@@ -1978,16 +1990,19 @@ 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;
-			else if (config->follow_remote_head_seen)
-				follow_remote_head = get_follow_remote_head(config->follow_remote_head_raw);
-			else
+			if (transport->remote->follow_remote_head_seen) {
+				char *key = xstrfmt("remote.%s.followRemoteHEAD", transport->remote->name);
+				follow_remote_head = get_follow_remote_head(
+					transport->remote->follow_remote_head_raw,
+					key, &no_warn_branch);
+				free(key);
+			} else if (config->follow_remote_head_seen) {
+				follow_remote_head = get_follow_remote_head(
+					config->follow_remote_head_raw,
+					"fetch.followRemoteHEAD", NULL);
+			} else {
 				follow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
+			}
 		}
This part of the code becomes overly deeply indented, and more
importantly, the outer conditional around here has been reworked by
another topic and the code that learns follow_remote_head will have
to be reused elsewhere, which would lead to a nasty merge conflict.

I'd like to see this part refactored into a separate helper
function, by squashing something like this on top of this patch.

Thanks.

 builtin/fetch.c | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)
diff --git c/builtin/fetch.c w/builtin/fetch.c
index bb59e70865..77e3276cc4 100644
--- c/builtin/fetch.c
+++ w/builtin/fetch.c
@@ -1946,6 +1946,28 @@ static void collect_updated_tips(struct oidset *tips, struct ref *ref_map)
 	}
 }
 
+static int lazy_follow_remote_head(const struct transport *transport,
+				   const struct fetch_config *config,
+				   const char **no_warn_branch)
+{
+	int ret = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
+
+	if (transport->remote->follow_remote_head_seen) {
+		char *key = xstrfmt("remote.%s.followRemoteHEAD",
+				    transport->remote->name);
+		ret = get_follow_remote_head(
+			transport->remote->follow_remote_head_raw,
+			key, no_warn_branch);
+		free(key);
+	} else if (config->follow_remote_head_seen) {
+		ret = get_follow_remote_head(
+			config->follow_remote_head_raw,
+			"fetch.followRemoteHEAD", NULL);
+	}
+
+	return ret;
+}
+
 static int do_fetch(struct transport *transport,
 		    struct refspec *rs,
 		    const struct fetch_config *config,
@@ -1990,19 +2012,9 @@ static int do_fetch(struct transport *transport,
 		if (transport->remote->fetch.nr) {
 			refspec_ref_prefixes(&transport->remote->fetch,
 					     &transport_ls_refs_options.ref_prefixes);
-			if (transport->remote->follow_remote_head_seen) {
-				char *key = xstrfmt("remote.%s.followRemoteHEAD", transport->remote->name);
-				follow_remote_head = get_follow_remote_head(
-					transport->remote->follow_remote_head_raw,
-					key, &no_warn_branch);
-				free(key);
-			} else if (config->follow_remote_head_seen) {
-				follow_remote_head = get_follow_remote_head(
-					config->follow_remote_head_raw,
-					"fetch.followRemoteHEAD", NULL);
-			} else {
-				follow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
-			}
+			follow_remote_head =
+				lazy_follow_remote_head(transport, config,
+							&no_warn_branch);
 		}
 		if (branch && branch_has_merge_config(branch) &&
 		    !strcmp(branch->remote_name, transport->remote->name)) {



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

Message details and related content
This part of the code becomes overly deeply indented, and more
importantly, the outer conditional around here has been reworked by
another topic and the code that learns follow_remote_head will have
to be reused elsewhere, which would lead to a nasty merge conflict.

I'd like to see this part refactored into a separate helper
function, by squashing something like this on top of this patch.

Thanks.
Okay, I will refactor this into a helper method since yes the
intending is becoming too much, I will also keep my eye on the patch
you previously mentioned, and post a new version once Next has been
updated from Harald's work so I can rebase with their fixes. Thanks
for the feedback.

-Colin Hinton

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

Message details and related content
Colin Hinton [off-list ref] writes:
quoted
This part of the code becomes overly deeply indented, and more
importantly, the outer conditional around here has been reworked by
another topic and the code that learns follow_remote_head will have
to be reused elsewhere, which would lead to a nasty merge conflict.

I'd like to see this part refactored into a separate helper
function, by squashing something like this on top of this patch.

Thanks.
Okay, I will refactor this into a helper method since yes the
intending is becoming too much, I will also keep my eye on the patch
you previously mentioned, and post a new version once Next has been
updated from Harald's work so I can rebase with their fixes. Thanks
for the feedback.

-Colin Hinton
The 'ch/remote-defer-follow-remote-head-validation' branch at
https://github.com/gitster/git/ has a change on top that you can use
as a reference.  If you are going to make this a two-patch series, it
may make sense to first extract the code commented with "See
remote.c's handling of ..." into a helper function, and then extend
it to cover the lazy behavior in the second patch.  When viewed
that way, the helper function should not be named with a 'lazy_'
prefix as I mistakenly did.  Being lazy is not its purpose.  Its
point is to determine which setting of followRemoteHEAD is to be
used, and the helper function should be named after that concept.

Thanks.

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

Message details and related content
On Wed Oct 28, 2026 at 10:26 AM EDT, Junio C Hamano wrote:
quoted
quoted
This part of the code becomes overly deeply indented, and more
importantly, the outer conditional around here has been reworked by
another topic and the code that learns follow_remote_head will have
to be reused elsewhere, which would lead to a nasty merge conflict.

I'd like to see this part refactored into a separate helper
function, by squashing something like this on top of this patch.
[...]

The 'ch/remote-defer-follow-remote-head-validation' branch at
https://github.com/gitster/git/ has a change on top that you can use
as a reference.  If you are going to make this a two-patch series, it
may make sense to first extract the code commented with "See
remote.c's handling of ..." into a helper function, and then extend
it to cover the lazy behavior in the second patch.  When viewed
that way, the helper function should not be named with a 'lazy_'
prefix as I mistakenly did.  Being lazy is not its purpose.  Its
point is to determine which setting of followRemoteHEAD is to be
used, and the helper function should be named after that concept.
I wonder if it makes sense to incorporate such a new helper function
into the existing 'get_follow_remote_head()', since it would be the only
place using it.

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

Message details and related content
"Matt Hunter" [off-list ref] writes:
On Wed Oct 28, 2026 at 10:26 AM EDT, Junio C Hamano wrote:
quoted
quoted
quoted
This part of the code becomes overly deeply indented, and more
importantly, the outer conditional around here has been reworked by
another topic and the code that learns follow_remote_head will have
to be reused elsewhere, which would lead to a nasty merge conflict.
...
I wonder if it makes sense to incorporate such a new helper function
into the existing 'get_follow_remote_head()', since it would be the only
place using it.
I am not sure what being the only place has to do with avoiding to
split a helper function out to clarify the main logic flow, though.
And simultaneously, another topic wants to have this piece of logic
twice.

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

Message details and related content
On Fri Oct 30, 2026 at 11:12 AM EDT, Junio C Hamano wrote:
"Matt Hunter" [off-list ref] writes:
quoted
...
I wonder if it makes sense to incorporate such a new helper function
into the existing 'get_follow_remote_head()', since it would be the only
place using it.
I am not sure what being the only place has to do with avoiding to
split a helper function out to clarify the main logic flow, though.
And simultaneously, another topic wants to have this piece of logic
twice.
I should explain myself better.  I don't mean Colin should avoid
splitting out logic from the main flow in do_fetch.  But the end result
can be reached without introducing a new function.  I find needing to
hold fewer functions in my head makes the code easier to reason about,
though that is a subjective point.

The idea in my head is below and is
Signed-off-by: Matt Hunter <redacted>
Feel free to run with this or not.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index bb59e70865ff..844ff5105f91 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -196,23 +196,44 @@ 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,
-    const char *key, const char **no_warn_branch)
+static enum follow_remote_head_settings get_follow_remote_head(
+		const struct transport *transport, const struct fetch_config *config,
+		const char **no_warn_branch)
 {
-	if (!setting)
-		die(_("missing value for '%s'"), key);
-	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 (no_warn_branch && skip_prefix(setting, "warn-if-not-", no_warn_branch))
-		return FOLLOW_REMOTE_WARN;
-	else if (!strcmp(setting, "always"))
-		return FOLLOW_REMOTE_ALWAYS;
-	warning(_("unrecognized %s value '%s' ignored"), key, setting);
-	return BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
+	int ret = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
+	char *key = NULL;
+	const char *setting = NULL;
+
+	if (transport->remote->follow_remote_head_seen) {
+		key = xstrfmt("remote.%s.followRemoteHEAD", transport->remote->name);
+		setting = transport->remote->follow_remote_head_raw;
+	} else if (config->follow_remote_head_seen) {
+		key = xstrdup("fetch.followRemoteHEAD");
+		setting = config->follow_remote_head_raw;
+		no_warn_branch = NULL;
+	}
+
+	if (key) {
+		if (!setting)
+			die(_("missing value for '%s'"), key);
+		else if (!strcmp(setting, "never"))
+			ret = FOLLOW_REMOTE_NEVER;
+		else if (!strcmp(setting, "create"))
+			ret = FOLLOW_REMOTE_CREATE;
+		else if (!strcmp(setting, "warn"))
+			ret = FOLLOW_REMOTE_WARN;
+		else if (no_warn_branch &&
+				skip_prefix(setting, "warn-if-not-", no_warn_branch))
+			ret = FOLLOW_REMOTE_WARN;
+		else if (!strcmp(setting, "always"))
+			ret = FOLLOW_REMOTE_ALWAYS;
+		else
+			warning(_("unrecognized %s value '%s' ignored"), key, setting);
+
+		free(key);
+	}
+
+	return ret;
 }
 
 static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)
@@ -1990,19 +2011,9 @@ static int do_fetch(struct transport *transport,
 		if (transport->remote->fetch.nr) {
 			refspec_ref_prefixes(&transport->remote->fetch,
 					     &transport_ls_refs_options.ref_prefixes);
-			if (transport->remote->follow_remote_head_seen) {
-				char *key = xstrfmt("remote.%s.followRemoteHEAD", transport->remote->name);
-				follow_remote_head = get_follow_remote_head(
-					transport->remote->follow_remote_head_raw,
-					key, &no_warn_branch);
-				free(key);
-			} else if (config->follow_remote_head_seen) {
-				follow_remote_head = get_follow_remote_head(
-					config->follow_remote_head_raw,
-					"fetch.followRemoteHEAD", NULL);
-			} else {
-				follow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
-			}
+			follow_remote_head =
+				get_follow_remote_head(transport, config,
+						&no_warn_branch);
 		}
 		if (branch && branch_has_merge_config(branch) &&
 		    !strcmp(branch->remote_name, transport->remote->name)) {

Admittedly, I haven't been following Harald's hn/fetch-refmap topic
closely, but am more familiar with the affected areas now.

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

Message details and related content
"Matt Hunter" [off-list ref] writes:
I should explain myself better.  I don't mean Colin should avoid
splitting out logic from the main flow in do_fetch.  But the end result
can be reached without introducing a new function.  I find needing to
hold fewer functions in my head makes the code easier to reason about,
though that is a subjective point.

The idea in my head is below and is
Signed-off-by: Matt Hunter <redacted>
Feel free to run with this or not.
Hmph, interesting.  Can you rewrite Colin's patch using this, and
then merge the result into 'seen' (after reverting Colin's topic
ch/remote-defer-follow-remote-head-validation from 'seen' first) to
see how badly it crashes with other topics in flight?

Thanks.


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

Message details and related content
On Thu Nov 5, 2026 at 11:03 PM EST, Junio C Hamano wrote:
Hmph, interesting.  Can you rewrite Colin's patch using this, and
then merge the result into 'seen' (after reverting Colin's topic
ch/remote-defer-follow-remote-head-validation from 'seen' first) to
see how badly it crashes with other topics in flight?
I still have the commit sitting around.  I tried this and a replay of
the current topic to compare -- the conflict is the same in both
cases.  eg: Nothing else in flight is touching the
get_follow_remote_head() function.


static int do_fetch(struct transport *transport,
		    struct refspec *rs,
		    const struct fetch_config *config,
		    struct list_objects_filter_options *filter_options,
		    struct oidset *updated_tips)
{
	struct ref_transaction *transaction = NULL;
	struct ref *ref_map = NULL;
	struct display_state display_state = { 0 };
	int autotags = (transport->remote->fetch_tags == 1);
	int retcode = 0;
	const struct ref *remote_refs;
	struct transport_ls_refs_options transport_ls_refs_options =
		TRANSPORT_LS_REFS_OPTIONS_INIT;
	struct fetch_head fetch_head = { 0 };
	struct strbuf err = STRBUF_INIT;
	struct ref_update_display_info_array display_array = { 0 };
	struct strmap rejected_refs = STRMAP_INIT;
	int summary_width = 0;
	int follow_remote_head = FOLLOW_REMOTE_NEVER;
<<<<<<< HEAD
	bool apply_follow_remote_head_config = false;
=======
	const char *no_warn_branch = NULL;
quoted
quoted
quoted
quoted
quoted
quoted
try-merge
	if (tags == TAGS_DEFAULT) {
		if (transport->remote->fetch_tags == 2)
			tags = TAGS_SET;
		if (transport->remote->fetch_tags == -1)
			tags = TAGS_UNSET;
	}

	/* if not appending, truncate FETCH_HEAD */
	if (!append && write_fetch_head) {
		retcode = truncate_fetch_head();
		if (retcode)
			goto cleanup;
	}

	if (rs->nr) {
		refspec_ref_prefixes(rs, &transport_ls_refs_options.ref_prefixes);
	} else if (transport->remote->fetch.nr) {
		refspec_ref_prefixes(&transport->remote->fetch,
				     &transport_ls_refs_options.ref_prefixes);
		apply_follow_remote_head_config = true;

<<<<<<< HEAD
		/*
		 * The configured refspec may not cover the current
		 * branch's upstream (e.g. a narrowed -t refspec), so
		 * make sure we can still fetch it regardless.
		 */
		collect_upstream_from_remote(the_repository, transport->remote,
					      NULL, &transport_ls_refs_options.ref_prefixes);
	} else {
		/*
		 * The --refmap command line option, if given, takes
		 * precedence over remote.<name>.fetchRefmap.
		 */
		struct refspec *effective_refmap = refmap.nr ? &refmap :
			&transport->remote->refmap;

		if (effective_refmap->nr) {
			branches_tracking_remote(the_repository, transport->remote,

20 more lines

quoted
quoted
quoted
quoted
quoted
quoted
try-merge
		}

		/*
		 * If there are no refs specified to fetch, then we just
		 * fetch HEAD; mention that to narrow the advertisement.
		 */
		if (!transport_ls_refs_options.ref_prefixes.nr)
			strvec_push(&transport_ls_refs_options.ref_prefixes,
				    "HEAD");
	}

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

Message details and related content
"Matt Hunter" [off-list ref] writes:
On Thu Nov 5, 2026 at 11:03 PM EST, Junio C Hamano wrote:
quoted
Hmph, interesting.  Can you rewrite Colin's patch using this, and
then merge the result into 'seen' (after reverting Colin's topic
ch/remote-defer-follow-remote-head-validation from 'seen' first) to
see how badly it crashes with other topics in flight?
I still have the commit sitting around.  I tried this and a replay of
the current topic to compare -- the conflict is the same in both
cases.  eg: Nothing else in flight is touching the
get_follow_remote_head() function.
Thanks for investigating.  

Your approach of passing more context into get_follow_remote_head()
and have _it_ do the (lazy) conversion from string to value makes it
much easier for callers to make calls from multiple branches to the
function, because the need to "set-up" to make the call disappears.

Very cleanly done.

Colin, what's your preference?  Can we move this topic past the
finish line?

Thanks.

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

Message details and related content
I prefer Matt's approach as I agree it's cleaner and simpler by
removing the setup at the call site. I'll adopt it into my next
version and will give credit to Matt for his contribution.

I am still holding on to the re-roll until hn/fetch-refmap is accepted
into 'next', so that I can rebase and resolve any conflicts myself.
Junio, if you think there is a better branch for me to base this off
of so that I can begin the conflict resolution now please let me know.

-Colin Hinton

On Fri, Nov 6, 2026 at 8:15 AM Junio C Hamano [off-list ref] wrote:
"Matt Hunter" [off-list ref] writes:
quoted
On Thu Nov 5, 2026 at 11:03 PM EST, Junio C Hamano wrote:
quoted
Hmph, interesting.  Can you rewrite Colin's patch using this, and
then merge the result into 'seen' (after reverting Colin's topic
ch/remote-defer-follow-remote-head-validation from 'seen' first) to
see how badly it crashes with other topics in flight?
I still have the commit sitting around.  I tried this and a replay of
the current topic to compare -- the conflict is the same in both
cases.  eg: Nothing else in flight is touching the
get_follow_remote_head() function.
Thanks for investigating.

Your approach of passing more context into get_follow_remote_head()
and have _it_ do the (lazy) conversion from string to value makes it
much easier for callers to make calls from multiple branches to the
function, because the need to "set-up" to make the call disappears.

Very cleanly done.

Colin, what's your preference?  Can we move this topic past the
finish line?

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help