* [PATCH v1] remote.c: defer remote.<name>.followRemoteHEAD validation
@ 2026-10-18 23:55 Colin Hinton
2026-10-19 15:37 ` Junio C Hamano
0 siblings, 1 reply; 2+ messages in thread
From: Colin Hinton @ 2026-10-18 23:55 UTC (permalink / raw)
To: git; +Cc: m, gitster, 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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v1] remote.c: defer remote.<name>.followRemoteHEAD validation
2026-10-18 23:55 [PATCH v1] remote.c: defer remote.<name>.followRemoteHEAD validation Colin Hinton
@ 2026-10-19 15:37 ` Junio C Hamano
0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2026-10-19 15:37 UTC (permalink / raw)
To: Colin Hinton; +Cc: git, m
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.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-19 15:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-18 23:55 [PATCH v1] remote.c: defer remote.<name>.followRemoteHEAD validation Colin Hinton
2026-10-19 15:37 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox