git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] fetch.c: defer fetch.followRemoteHEAD validation

From
Matt Hunter <m@lfurio.us>
Date
Sep 24, 2026, 07:50 UTC
Message-ID
<DLNDRU6GIB30.1F5G8Z3JIR67W@lfurio.us>
In-Reply-To
<CAHeTm9OMLba_h0B2jRh_-GhogQXuwROBpX2jE__BPJ0GHq9P1A@mail.gmail.com>
On Wed Sep 23, 2026 at 1:03 AM EDT, Colin Hinton wrote:
Show 15 quoted lines
>> > @@ -1962,6 +1953,14 @@ 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)
>> > +                             follow_remote_head = transport->remote->follow_remote_head;
>>
>> The code assumes that remote.*.followRemoteHEAD has been pre-parsed.
>> Doesn't the code to do so in remote.c::handle_config() share exactly
>> the same problem as you are fixing here?
>>
> I agree that the same problem that is being addressed here is present
> in remote.c as well. The only difference being, that there is no
> return call in the followremotehead block in remote.c,

I'm not exactly sure why the config parsing in remote.c doesn't end with a fallback 'return git_default_config(...)', though the followremotehead case piggybacking the common 'return 0' at the end should be no problem.

> and it at most only throws a warning if no valid value is present.

which _was_ the case for fetch.followRemoteHEAD as well. So, we should keep the two in sync right?

Show 6 quoted lines
> I think this
> should be addressed, but I am uncertain if this is within the scope of
> this issue and should be resolved now, or if this requires its own
> investigation and should be resolved in a future patch. Regardless I
> am eager to work on it, but would like some guidance as to what is
> most appropriate for a change in remote.c.

I spent some time drafting up what changes to remote.c could look like, based on your work so far. This follow-up patch also has extra changes to builtin/fetch.c to accommodate the same allowed functionality as before. There are two awkward bits to this patch as-is, though:

builtin/remote.c::set_head()

012bc566bad7 (remote set-head: set followRemoteHEAD to "warn" if "always") added this behavior to overrule a remote's "always" setting if the user ever modified their HEAD manually. So, this file needs to know about the followRemoteHEAD values, but parsing into the enums is currently confined to fetch.c. This just adds another bit of string parsing.

builtin/fetch.c::get_follow_remote_head()

is updated to serve double-duty for both the fetch and remote configs, and needs a better warning message if a bad value is detected. Perhaps add another parameter to the function?

With this patch below, it's arguable whether the enum definition for the followRemoteHEAD values now better fits in fetch.c instead of remote.h.

Signed-off-by: Matt Hunter <m@lfurio.us>
---
 builtin/fetch.c  | 54 ++++++++++++++++++++++++++++++++----------------
 builtin/remote.c |  3 ++-
 remote.c         | 19 ++---------------
 remote.h         |  3 +--
 4 files changed, 41 insertions(+), 38 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 83074c48150b..5a4c9fb9309c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -187,18 +187,34 @@ 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)
+/* TODO might be worth considering a better name for this */
+struct follow_remote_head_target {
+	enum follow_remote_head_settings mode;
+	const char *no_warn_branch;
+};
+
+static struct follow_remote_head_target get_follow_remote_head(const char *setting,
+		int allow_warn_if_not_branch)
 {
+	struct follow_remote_head_target frh = { 0 };
+
 	if (!strcmp(setting, "never"))
-		return FOLLOW_REMOTE_NEVER;
+		frh.mode = FOLLOW_REMOTE_NEVER;
 	else if (!strcmp(setting, "create"))
-		return FOLLOW_REMOTE_CREATE;
+		frh.mode = FOLLOW_REMOTE_CREATE;
 	else if (!strcmp(setting, "warn"))
-		return FOLLOW_REMOTE_WARN;
+		frh.mode = FOLLOW_REMOTE_WARN;
+	else if (skip_prefix(setting, "warn-if-not-", &frh.no_warn_branch)
+			&& allow_warn_if_not_branch)
+		frh.mode = FOLLOW_REMOTE_WARN;
 	else if (!strcmp(setting, "always"))
-		return FOLLOW_REMOTE_ALWAYS;
-	warning(_("unrecognized fetch.followRemoteHEAD value '%s' ignored"), setting);
-	return FOLLOW_REMOTE_UNCONFIGURED;
+		frh.mode = FOLLOW_REMOTE_ALWAYS;
+	else
+		warning(_("unrecognized fetch.followRemoteHEAD value '%s' ignored"), setting);
+		/* TODO this also parses remote.<name>.followRemoteHEAD,
+		 * but the warning string says fetch.followRemoteHEAD */
+
+	return frh;
 }
 
 static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)
@@ -1758,12 +1774,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)
+			struct follow_remote_head_target follow_remote_head)
 {
 	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;
@@ -1793,7 +1808,7 @@ static int set_head(const struct ref *remote_refs, struct remote *remote,
 	if (!head_name)
 		goto cleanup;
 	baremirror = is_bare_repository(the_repository) && remote->mirror;
-	create_only = follow_remote_head == FOLLOW_REMOTE_ALWAYS ? 0 : !baremirror;
+	create_only = follow_remote_head.mode == FOLLOW_REMOTE_ALWAYS ? 0 : !baremirror;
 	if (baremirror) {
 		strbuf_addstr(&b_head, "HEAD");
 		strbuf_addf(&b_remote_head, "refs/heads/%s", head_name);
@@ -1813,8 +1828,9 @@ static int set_head(const struct ref *remote_refs, struct remote *remote,
 		goto cleanup;
 	}
 	if (verbosity >= 0 &&
-		follow_remote_head == FOLLOW_REMOTE_WARN &&
-		(!no_warn_branch || strcmp(no_warn_branch, head_name)))
+		follow_remote_head.mode == FOLLOW_REMOTE_WARN &&
+		(!follow_remote_head.no_warn_branch ||
+		 strcmp(follow_remote_head.no_warn_branch, head_name)))
 		warn_set_head(remote->name, head_name, &b_local_head, was_detached);
 
 cleanup:
@@ -1929,7 +1945,7 @@ static int do_fetch(struct transport *transport,
 	struct ref_update_display_info_array display_array = { 0 };
 	struct strmap rejected_refs = STRMAP_INIT;
 	int summary_width = 0;
-	int follow_remote_head = 0;
+	struct follow_remote_head_target follow_remote_head = { 0 };
 
 	if (tags == TAGS_DEFAULT) {
 		if (transport->remote->fetch_tags == 2)
@@ -1954,14 +1970,16 @@ static int do_fetch(struct transport *transport,
 			refspec_ref_prefixes(&transport->remote->fetch,
 					     &transport_ls_refs_options.ref_prefixes);
 
-			if (transport->remote->follow_remote_head)
-				follow_remote_head = transport->remote->follow_remote_head;
+			if (transport->remote->follow_remote_head_raw)
+				follow_remote_head = get_follow_remote_head(
+						transport->remote->follow_remote_head_raw, 1);
 			else if (config->follow_remote_head_raw)
-				follow_remote_head = get_follow_remote_head(config->follow_remote_head_raw);
+				follow_remote_head = get_follow_remote_head(
+						config->follow_remote_head_raw, 0);
 			else
-				follow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
+				follow_remote_head.mode = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
 			
-			if (follow_remote_head != FOLLOW_REMOTE_NEVER)
+			if (follow_remote_head.mode != FOLLOW_REMOTE_NEVER)
 				do_set_head = 1;
 		}
 		if (branch && branch_has_merge_config(branch) &&
diff --git a/builtin/remote.c b/builtin/remote.c
index de989ea3ba96..89ac1f0daa82 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 fe6206846356..5fdcadfbdbf0 100644
--- a/remote.c
+++ b/remote.c
@@ -581,23 +581,8 @@ 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;
-		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(value);
 	}
 	return 0;
 }
diff --git a/remote.h b/remote.h
index cca02033b9d7..cd97df017454 100644
--- a/remote.h
+++ b/remote.h
@@ -122,8 +122,7 @@ 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;
 };
 
 /**

-- 
2.55.0
Previous: Colin HintonNext: Colin Hinton
Message 4 of 22 in “fetch.c: defer fetch.followRemoteHEAD validation”
  1. fetch.c: defer fetch.followRemoteHEAD validationColin Hinton, Sep 22, 2026
  2. Junio C HamanoSep 22, 2026
  3. Colin HintonSep 23, 2026
  4. Matt HunterSep 24, 2026
  5. Colin HintonSep 25, 2026
  6. Matt HunterSep 24, 2026
  7. fetch.c: defer fetch.followRemoteHEAD validationColin Hinton, Sep 25, 2026
  8. Junio C HamanoSep 25, 2026
  9. Colin HintonSep 25, 2026
  10. Matt HunterSep 30, 2026
  11. Junio C HamanoSep 30, 2026
  12. Colin HintonOct 3, 2026
  13. fetch.c: defer fetch.followRemoteHEAD validationColin Hinton, Sep 25, 2026
  14. fetch.c: defer fetch.followRemoteHEAD validationColin Hinton, Oct 3, 2026
  15. Junio C HamanoOct 4, 2026
  16. Colin HintonOct 4, 2026
  17. Junio C HamanoOct 4, 2026
  18. fetch.c: defer fetch.followRemoteHEAD validationColin Hinton, Oct 4, 2026
  19. Matt HunterOct 5, 2026
  20. Junio C HamanoOct 5, 2026
  21. fetch.c: defer fetch.followRemoteHEAD validationColin Hinton, Oct 6, 2026
  22. Junio C HamanoOct 7, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.