{"thread":{"id":"66363","subject":"[PATCH] fetch.c: defer fetch.followRemoteHEAD validation","startedAt":"2026-09-22T04:01:02Z","lastAt":"2026-10-06T03:22:58Z","messageCount":21,"participants":["Colin Hinton","Junio C Hamano","Matt Hunter"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"552966","messageId":"20260922040047.2567-1-colinlewishinton@gmail.com","threadId":"66363","inReplyTo":null,"subject":"[PATCH] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Colin Hinton","fromEmail":"colinlewishinton@gmail.com","sentAt":"2026-09-22T04:00:47Z","receivedAt":"2026-09-22T04:01:02Z","isPatch":true,"body":"Previously, fetch.followRemoteHEAD was validated and any invalid\nvalue was warned about unconditionally during config parsing.\n\nNow store the raw config string instead, and resolve/validate it lazily\nat the one call in do_fetch(), so an irrelevant fetch no longer warns about an unrelated\nconfig value it never needed.\n\nSigned-off-by: Colin Hinton \n---\n builtin/fetch.c | 57 ++++++++++++++++++++++++-------------------------\n 1 file changed, 28 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex ab7db2be06..64ad26f5d4 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -103,7 +103,7 @@ static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;\n \n struct fetch_config {\n \tenum display_format display_format;\n-\tenum follow_remote_head_settings follow_remote_head;\n+\tchar *follow_remote_head_raw;\n \tint all;\n \tint prune;\n \tint prune_tags;\n@@ -178,22 +178,29 @@ static int git_fetch_config(const char *k, const char *v,\n \tif (!strcmp(k, \"fetch.followremotehead\")) {\n \t\tif (!v)\n \t\t\treturn config_error_nonbool(k);\n-\t\telse if (!strcmp(v, \"never\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;\n-\t\telse if (!strcmp(v, \"create\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;\n-\t\telse if (!strcmp(v, \"warn\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;\n-\t\telse if (!strcmp(v, \"always\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;\n-\t\telse\n-\t\t\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), v);\n+\t\tfree(fetch_config->follow_remote_head_raw);\n+\t\tfetch_config->follow_remote_head_raw = xstrdup(v);\n+\t\t\n \t\treturn 0;\n \t}\n \n \treturn git_default_config(k, v, ctx, cb);\n }\n \n+static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n+{\n+\tif (!strcmp(setting, \"never\"))\n+\t\treturn FOLLOW_REMOTE_NEVER;\n+\telse if (!strcmp(setting, \"create\"))\n+\t\treturn FOLLOW_REMOTE_CREATE;\n+\telse if (!strcmp(setting, \"warn\"))\n+\t\treturn FOLLOW_REMOTE_WARN;\n+\telse if (!strcmp(setting, \"always\"))\n+\t\treturn FOLLOW_REMOTE_ALWAYS;\n+\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n+\treturn FOLLOW_REMOTE_UNCONFIGURED;\n+}\n+\n static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)\n {\n \tBUG_ON_OPT_NEG(unset);\n@@ -1922,7 +1929,7 @@ static int do_fetch(struct transport *transport,\n \tstruct ref_update_display_info_array display_array = { 0 };\n \tstruct strmap rejected_refs = STRMAP_INIT;\n \tint summary_width = 0;\n-\tint follow_remote_head;\n+\tint follow_remote_head = 0;\n \n \tif (tags == TAGS_DEFAULT) {\n \t\tif (transport->remote->fetch_tags == 2)\n@@ -1938,22 +1945,6 @@ static int do_fetch(struct transport *transport,\n \t\t\tgoto cleanup;\n \t}\n \n-\t/*\n-\t * NEEDSWORK: By the time this function executes, we have already parsed\n-\t * all such followRemoteHEAD values from the external configuration,\n-\t * potentially emitting warning messages for bogus values. Ideally, if\n-\t * this fetch ends up not needing to consult these values, then git would\n-\t * not ever output a value warning. (eg: when pulling from a URL directly -\n-\t * rather than a configured remote, or when a remote's followRemoteHEAD\n-\t * overrides the fallback fetch setting)\n-\t */\n-\tif (transport->remote->follow_remote_head)\n-\t\tfollow_remote_head = transport->remote->follow_remote_head;\n-\telse if (config->follow_remote_head)\n-\t\tfollow_remote_head = config->follow_remote_head;\n-\telse\n-\t\tfollow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n-\n \tif (rs->nr) {\n \t\trefspec_ref_prefixes(rs, &transport_ls_refs_options.ref_prefixes);\n \t} else {\n@@ -1962,6 +1953,14 @@ static int do_fetch(struct transport *transport,\n \t\tif (transport->remote->fetch.nr) {\n \t\t\trefspec_ref_prefixes(&transport->remote->fetch,\n \t\t\t\t\t &transport_ls_refs_options.ref_prefixes);\n+\n+\t\t\tif (transport->remote->follow_remote_head)\n+\t\t\t\tfollow_remote_head = transport->remote->follow_remote_head;\n+\t\t\telse if (config->follow_remote_head_raw)\n+\t\t\t\tfollow_remote_head = get_follow_remote_head(config->follow_remote_head_raw);\n+\t\t\telse\n+\t\t\t\tfollow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n+\t\t\t\n \t\t\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n \t\t\t\tdo_set_head = 1;\n \t\t}\n@@ -2509,7 +2508,7 @@ int cmd_fetch(int argc,\n {\n \tstruct fetch_config config = {\n \t\t.display_format = DISPLAY_FORMAT_FULL,\n-\t\t.follow_remote_head = FOLLOW_REMOTE_UNCONFIGURED,\n+\t\t.follow_remote_head_raw = NULL,\n \t\t.prune = -1,\n \t\t.prune_tags = -1,\n \t\t.show_forced_updates = 1,\n-- \n2.55.0.windows.3\n\n"},{"id":"552970","messageId":"xmqqwlsdhmvk.fsf@gitster.g","threadId":"66363","inReplyTo":"20260922040047.2567-1-colinlewishinton@gmail.com","subject":"Re: [PATCH] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-22T05:32:31Z","receivedAt":"2026-09-22T05:32:34Z","isPatch":true,"body":"Colin Hinton writes:\n\n> Previously, fetch.followRemoteHEAD was validated and any invalid\n> value was warned about unconditionally during config parsing.\n\nEarly paragraphs that make observation on how the current system\nworks should be written in present tense. It is the status quo, so\nwe shouldn't say \"previously\" and we do not need to say \"currently\".\n\n The value of configuration variable \"fetch.followRemoteHEAD is\n validated while the configuration file is being parsed, which\n lead to a warning, even when we do not need to know the value.\n\n> Now store the raw config string instead, and resolve/validate it lazily\n> at the one call in do_fetch(), so an irrelevant fetch no longer warns about an unrelated\n> config value it never needed.\n\nWell written, except that \"an irrelevant fetch\" is a bit awkward.\n\"Irrelevant how, for whom, and why?\" is a set of natural questions\nthat come to readers' minds. I am guessing that you wanted to say\nthat \"git fetch\" does not always need to know the value of the\nfetch.followRemoteHEAD configuration variable, perhaps because a\nparticular invocation of \"git fetch\" receives specific refspec.\nYou'd need to find a concise way to say that and replace the\n\"irrelevant\" there.\n\nIn any case, it is a very good discipline to avoid dying or making\nnoises while reading the configuration file and instead complain\nonly when we know we will use the bad value.\n\n> struct fetch_config {\n> \tenum display_format display_format;\n> -\tenum follow_remote_head_settings follow_remote_head;\n> +\tchar *follow_remote_head_raw;\n\nOK. So this is the read the value and keep it as-is.\n\n> \tint all;\n> \tint prune;\n> \tint prune_tags;\n> @@ -178,22 +178,29 @@ static int git_fetch_config(const char *k, const char *v,\n> \tif (!strcmp(k, \"fetch.followremotehead\")) {\n> \t\tif (!v)\n> \t\t\treturn config_error_nonbool(k);\n\nThis error still triggers even when the configuration variable is\nirrelevant (e.g, \"git fetch origin master\", i.e., rs->nr != 0).\nDealing with it is well within the scope of the topic, isn't it?\nYou may be ignoring\n\n\t[fetch]\n\t\tfollowremotehead = bogus\n\nwhen the user runs \"git fetch https://over.there/repo master\" with\nthis patch, which may be an improvement, but if the user has a\nvalueless truth\n\n\t[fetch]\n\t\tfollowremotehead\n\nthen the same command would die while parsing the configuration\nvariable, which is not what you wanted to see, right?\n\n> -\t\telse if (!strcmp(v, \"never\"))\n> -\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;\n> -\t\telse if (!strcmp(v, \"create\"))\n> -\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;\n> -\t\telse if (!strcmp(v, \"warn\"))\n> -\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;\n> -\t\telse if (!strcmp(v, \"always\"))\n> -\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;\n> -\t\telse\n> -\t\t\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), v);\n> +\t\tfree(fetch_config->follow_remote_head_raw);\n> +\t\tfetch_config->follow_remote_head_raw = xstrdup(v);\n\nGood to see that the code is prepared to see the same variable\ndefined multiple times in the configuration stream without leaking\nearlier values.\n\n> +static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n> +{\n> +\tif (!strcmp(setting, \"never\"))\n> +\t\treturn FOLLOW_REMOTE_NEVER;\n> +\telse if (!strcmp(setting, \"create\"))\n> +\t\treturn FOLLOW_REMOTE_CREATE;\n> +\telse if (!strcmp(setting, \"warn\"))\n> +\t\treturn FOLLOW_REMOTE_WARN;\n> +\telse if (!strcmp(setting, \"always\"))\n> +\t\treturn FOLLOW_REMOTE_ALWAYS;\n> +\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n> +\treturn FOLLOW_REMOTE_UNCONFIGURED;\n> +}\n\nOK. So unrecognised are treated as unconfigured, just like before.\n\n> static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)\n> {\n> \tBUG_ON_OPT_NEG(unset);\n> @@ -1922,7 +1929,7 @@ static int do_fetch(struct transport *transport,\n> \tstruct ref_update_display_info_array display_array = { 0 };\n> \tstruct strmap rejected_refs = STRMAP_INIT;\n> \tint summary_width = 0;\n> -\tint follow_remote_head;\n> +\tint follow_remote_head = 0;\n> \n> \tif (tags == TAGS_DEFAULT) {\n> \t\tif (transport->remote->fetch_tags == 2)\n> @@ -1938,22 +1945,6 @@ static int do_fetch(struct transport *transport,\n> \t\t\tgoto cleanup;\n> \t}\n> \n> -\t/*\n> -\t * NEEDSWORK: By the time this function executes, we have already parsed\n> -\t * all such followRemoteHEAD values from the external configuration,\n> -\t * potentially emitting warning messages for bogus values. Ideally, if\n> -\t * this fetch ends up not needing to consult these values, then git would\n> -\t * not ever output a value warning. (eg: when pulling from a URL directly -\n> -\t * rather than a configured remote, or when a remote's followRemoteHEAD\n> -\t * overrides the fallback fetch setting)\n> -\t */\n\nGood write-up. We should be able to steal some in our own description.\n\n> @@ -1962,6 +1953,14 @@ static int do_fetch(struct transport *transport,\n> \t\tif (transport->remote->fetch.nr) {\n> \t\t\trefspec_ref_prefixes(&transport->remote->fetch,\n> \t\t\t\t\t &transport_ls_refs_options.ref_prefixes);\n> +\n> +\t\t\tif (transport->remote->follow_remote_head)\n> +\t\t\t\tfollow_remote_head = transport->remote->follow_remote_head;\n\nThe code assumes that remote.*.followRemoteHEAD has been pre-parsed.\nDoesn't the code to do so in remote.c::handle_config() share exactly\nthe same problem as you are fixing here?\n\n> +\t\t\telse if (config->follow_remote_head_raw)\n> +\t\t\t\tfollow_remote_head = get_follow_remote_head(config->follow_remote_head_raw);\n> +\t\t\telse\n> +\t\t\t\tfollow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n\nMake a mental note that do_set_head is flipped on ONLY here in this\nfunction.\n\n> \t\t\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n> \t\t\t\tdo_set_head = 1;\n> \t\t}\n\nAnd later, do_set_head is referenced twice. Once when preparing the\ntransport options to first discover what refs they have (ls-refs)\n\n\tif (do_set_head)\n\t\tstrvec_push(&transport_ls_refs_options.ref_prefixes,\n\t\t\t \"HEAD\");\n\nand then once more to make a set-head call using follow_remote_head.\n\n\tif (do_set_head) {\n\t\t/*\n\t\t * Way too many cases where this can go wrong so let's just\n\t\t * ignore errors and fail silently for now.\n\t\t */\n\t\tset_head(remote_refs, transport->remote, follow_remote_head);\n\t}\n\nIncidentally, after that \"lazily turn configuration string into\nfollow_remote_head variable\" block is left, this is the only place\nthat follow_remote_head variable is referenced.\n\nWhich suggests to me that we can get rid of do_set_head variable, we\ncan initialize follow_remote_head variable to FOLLOW_REMOTE_NEVER,\nand replace these two \n\n\tif (do_set_head)\n\nwith\n\n\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n\nand the resulting code may become a tad easier to follow.\n\nHmmm?\n"},{"id":"553031","messageId":"CAHeTm9OMLba_h0B2jRh_-GhogQXuwROBpX2jE__BPJ0GHq9P1A@mail.gmail.com","threadId":"66363","inReplyTo":"xmqqwlsdhmvk.fsf@gitster.g","subject":"Re: [PATCH] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Colin Hinton","fromEmail":"colinlewishinton@gmail.com","sentAt":"2026-09-23T05:03:57Z","receivedAt":"2026-09-23T05:04:10Z","isPatch":true,"body":"On Mon, Sep 21, 2026 at 10:32 PM Junio C Hamano wrote:\n>\n> Colin Hinton writes:\n>\n> > Previously, fetch.followRemoteHEAD was validated and any invalid\n> > value was warned about unconditionally during config parsing.\n>\n> Early paragraphs that make observation on how the current system\n> works should be written in present tense. It is the status quo, so\n> we shouldn't say \"previously\" and we do not need to say \"currently\".\n>\n> The value of configuration variable \"fetch.followRemoteHEAD is\n> validated while the configuration file is being parsed, which\n> lead to a warning, even when we do not need to know the value.\n>\n> > Now store the raw config string instead, and resolve/validate it lazily\n> > at the one call in do_fetch(), so an irrelevant fetch no longer warns about an unrelated\n> > config value it never needed.\n>\n> Well written, except that \"an irrelevant fetch\" is a bit awkward.\n> \"Irrelevant how, for whom, and why?\" is a set of natural questions\n> that come to readers' minds. I am guessing that you wanted to say\n> that \"git fetch\" does not always need to know the value of the\n> fetch.followRemoteHEAD configuration variable, perhaps because a\n> particular invocation of \"git fetch\" receives specific refspec.\n> You'd need to find a concise way to say that and replace the\n> \"irrelevant\" there.\n\nOkay, for my next patch I will clear this up, and will change my\ntenses to all be in present tense.\n\n>\n> In any case, it is a very good discipline to avoid dying or making\n> noises while reading the configuration file and instead complain\n> only when we know we will use the bad value.\n>\n> > struct fetch_config {\n> > enum display_format display_format;\n> > - enum follow_remote_head_settings follow_remote_head;\n> > + char *follow_remote_head_raw;\n>\n> OK. So this is the read the value and keep it as-is.\n>\n> > int all;\n> > int prune;\n> > int prune_tags;\n> > @@ -178,22 +178,29 @@ static int git_fetch_config(const char *k, const char *v,\n> > if (!strcmp(k, \"fetch.followremotehead\")) {\n> > if (!v)\n> > return config_error_nonbool(k);\n>\n> This error still triggers even when the configuration variable is\n> irrelevant (e.g, \"git fetch origin master\", i.e., rs->nr != 0).\n> Dealing with it is well within the scope of the topic, isn't it?\n> You may be ignoring\n>\n> [fetch]\n> followremotehead = bogus\n>\n> when the user runs \"git fetch https://over.there/repo master\" with\n> this patch, which may be an improvement, but if the user has a\n> valueless truth\n>\n> [fetch]\n> followremotehead\n>\n> then the same command would die while parsing the configuration\n> variable, which is not what you wanted to see, right?\n\nI Agree on this, I will defer this check to the same point of use that\nI have abstracted in get_follow_remote_head. It would make sense to\nonly report a valueless fetch.followRemoteHEAD when the value is\nneeded. I also assume that if followRemoteHEAD is valueless, that we\nshould call die(...) rather than config_error_nonbool() to match\ncurrent behavior, so I will implement this in my next patch unless\nthere is something additional I should consider.\n>\n> > - else if (!strcmp(v, \"never\"))\n> > - fetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;\n> > - else if (!strcmp(v, \"create\"))\n> > - fetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;\n> > - else if (!strcmp(v, \"warn\"))\n> > - fetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;\n> > - else if (!strcmp(v, \"always\"))\n> > - fetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;\n> > - else\n> > - warning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), v);\n> > + free(fetch_config->follow_remote_head_raw);\n> > + fetch_config->follow_remote_head_raw = xstrdup(v);\n>\n> Good to see that the code is prepared to see the same variable\n> defined multiple times in the configuration stream without leaking\n> earlier values.\n>\n> > +static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n> > +{\n> > + if (!strcmp(setting, \"never\"))\n> > + return FOLLOW_REMOTE_NEVER;\n> > + else if (!strcmp(setting, \"create\"))\n> > + return FOLLOW_REMOTE_CREATE;\n> > + else if (!strcmp(setting, \"warn\"))\n> > + return FOLLOW_REMOTE_WARN;\n> > + else if (!strcmp(setting, \"always\"))\n> > + return FOLLOW_REMOTE_ALWAYS;\n> > + warning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n> > + return FOLLOW_REMOTE_UNCONFIGURED;\n> > +}\n>\n> OK. So unrecognised are treated as unconfigured, just like before.\n>\n> > static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)\n> > {\n> > BUG_ON_OPT_NEG(unset);\n> > @@ -1922,7 +1929,7 @@ static int do_fetch(struct transport *transport,\n> > struct ref_update_display_info_array display_array = { 0 };\n> > struct strmap rejected_refs = STRMAP_INIT;\n> > int summary_width = 0;\n> > - int follow_remote_head;\n> > + int follow_remote_head = 0;\n> >\n> > if (tags == TAGS_DEFAULT) {\n> > if (transport->remote->fetch_tags == 2)\n> > @@ -1938,22 +1945,6 @@ static int do_fetch(struct transport *transport,\n> > goto cleanup;\n> > }\n> >\n> > - /*\n> > - * NEEDSWORK: By the time this function executes, we have already parsed\n> > - * all such followRemoteHEAD values from the external configuration,\n> > - * potentially emitting warning messages for bogus values. Ideally, if\n> > - * this fetch ends up not needing to consult these values, then git would\n> > - * not ever output a value warning. (eg: when pulling from a URL directly -\n> > - * rather than a configured remote, or when a remote's followRemoteHEAD\n> > - * overrides the fallback fetch setting)\n> > - */\n>\n> Good write-up. We should be able to steal some in our own description.\n>\n> > @@ -1962,6 +1953,14 @@ static int do_fetch(struct transport *transport,\n> > if (transport->remote->fetch.nr) {\n> > refspec_ref_prefixes(&transport->remote->fetch,\n> > &transport_ls_refs_options.ref_prefixes);\n> > +\n> > + if (transport->remote->follow_remote_head)\n> > + follow_remote_head = transport->remote->follow_remote_head;\n>\n> The code assumes that remote.*.followRemoteHEAD has been pre-parsed.\n> Doesn't the code to do so in remote.c::handle_config() share exactly\n> the same problem as you are fixing here?\n>\nI agree that the same problem that is being addressed here is present\nin remote.c as well. The only difference being, that there is no\nreturn call in the followremotehead block in remote.c, and it at most\nonly throws a warning if no valid value is present. I think this\nshould be addressed, but I am uncertain if this is within the scope of\nthis issue and should be resolved now, or if this requires its own\ninvestigation and should be resolved in a future patch. Regardless I\nam eager to work on it, but would like some guidance as to what is\nmost appropriate for a change in remote.c.\n\n> > + else if (config->follow_remote_head_raw)\n> > + follow_remote_head = get_follow_remote_head(config->follow_remote_head_raw);\n> > + else\n> > + follow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n>\n> Make a mental note that do_set_head is flipped on ONLY here in this\n> function.\n>\n> > if (follow_remote_head != FOLLOW_REMOTE_NEVER)\n> > do_set_head = 1;\n> > }\n>\n> And later, do_set_head is referenced twice. Once when preparing the\n> transport options to first discover what refs they have (ls-refs)\n>\n> if (do_set_head)\n> strvec_push(&transport_ls_refs_options.ref_prefixes,\n> \"HEAD\");\n>\n> and then once more to make a set-head call using follow_remote_head.\n>\n> if (do_set_head) {\n> /*\n> * Way too many cases where this can go wrong so let's just\n> * ignore errors and fail silently for now.\n> */\n> set_head(remote_refs, transport->remote, follow_remote_head);\n> }\n>\n> Incidentally, after that \"lazily turn configuration string into\n> follow_remote_head variable\" block is left, this is the only place\n> that follow_remote_head variable is referenced.\n>\n> Which suggests to me that we can get rid of do_set_head variable, we\n> can initialize follow_remote_head variable to FOLLOW_REMOTE_NEVER,\n> and replace these two\n>\n> if (do_set_head)\n>\n> with\n>\n> if (follow_remote_head != FOLLOW_REMOTE_NEVER)\n>\n> and the resulting code may become a tad easier to follow.\n>\n> Hmmm?\n\nCertainly would be an improvement, and one I will be implementing in\nmy next patch.\n"},{"id":"553149","messageId":"DLNDRT2L2DYK.3LCFVZ6URY4G6@lfurio.us","threadId":"66363","inReplyTo":"xmqqwlsdhmvk.fsf@gitster.g","subject":"Re: [PATCH] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Matt Hunter","fromEmail":"m@lfurio.us","sentAt":"2026-09-24T07:50:09Z","receivedAt":"2026-09-24T07:58:12Z","isPatch":true,"body":"Colin - thanks for picking this up.\n\nJust wanted to make note of some lingering thoughts of mine from when\nthis config was added. I wouldn't consider these necessary for your\npatch, though you might agree with the ideas.\n\nOn Tue Sep 22, 2026 at 1:32 AM EDT, Junio C Hamano wrote:\n> Colin Hinton writes:\n>\n>> +static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n>> +{\n>> +\tif (!strcmp(setting, \"never\"))\n>> +\t\treturn FOLLOW_REMOTE_NEVER;\n>> +\telse if (!strcmp(setting, \"create\"))\n>> +\t\treturn FOLLOW_REMOTE_CREATE;\n>> +\telse if (!strcmp(setting, \"warn\"))\n>> +\t\treturn FOLLOW_REMOTE_WARN;\n>> +\telse if (!strcmp(setting, \"always\"))\n>> +\t\treturn FOLLOW_REMOTE_ALWAYS;\n>> +\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n>> +\treturn FOLLOW_REMOTE_UNCONFIGURED;\n>> +}\n>\n> OK. So unrecognised are treated as unconfigured, just like before.\n\nThere was an idea I raised in [1] that didn't really get discussed.\nThat being that we should effectively act like FOLLOW_REMOTE_NEVER is\nset when the configured value is unrecognized.\n\nThe situation I envision is a user porting their .gitconfig file to a\nsystem running an older git, that doesn't know about their preferred\nsetting. Given that _something_ is configured, the user obviously\ndoesn't want the default behavior, but that's what they'll get when\nFOLLOW_REMOTE_UNCONFIGURED is returned.\n\nFOLLOW_REMOTE_NEVER seems like the least suprising action to take when\nwe don't understand the request. And I think this reasoning could apply\nto remote.foo.followRemoteHEAD as well, if you think it's worth doing\nhere.\n\n>\n> Make a mental note that do_set_head is flipped on ONLY here in this\n> function.\n>\n>> \t\t\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n>> \t\t\t\tdo_set_head = 1;\n>> \t\t}\n>\n> And later, do_set_head is referenced twice. Once when preparing the\n> transport options to first discover what refs they have (ls-refs)\n>\n> \tif (do_set_head)\n> \t\tstrvec_push(&transport_ls_refs_options.ref_prefixes,\n> \t\t\t \"HEAD\");\n>\n> and then once more to make a set-head call using follow_remote_head.\n>\n> \tif (do_set_head) {\n> \t\t/*\n> \t\t * Way too many cases where this can go wrong so let's just\n> \t\t * ignore errors and fail silently for now.\n> \t\t */\n> \t\tset_head(remote_refs, transport->remote, follow_remote_head);\n> \t}\n>\n> Incidentally, after that \"lazily turn configuration string into\n> follow_remote_head variable\" block is left, this is the only place\n> that follow_remote_head variable is referenced.\n>\n> Which suggests to me that we can get rid of do_set_head variable, we\n> can initialize follow_remote_head variable to FOLLOW_REMOTE_NEVER,\n> and replace these two \n>\n> \tif (do_set_head)\n>\n> with\n>\n> \tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n>\n> and the resulting code may become a tad easier to follow.\n\nIt occurred to me a while ago that there's another case in which we\nmight want to skip querying the remote for its HEAD - when\nFOLLOW_REMOTE_CREATE is in effect, and the remote already has a local\nHEAD symref. Given that 'create' is the default mode for this setting,\nit would probably be a valuable save on network and server overhead.\n\nOf course, I think this should probably be its own topic, separate from\nwhat this patch is addressing. But if the condition of that 'if'\nstatement is to become more complicated, it might be a good reason to\nnot start duplicating it here.\n\n>\n> Hmmm?\n\n1: https://lore.kernel.org/git/DJBVYP58YNTU.LQ7VXFIQE84H@lfurio.us/\n"},{"id":"553150","messageId":"DLNDRU6GIB30.1F5G8Z3JIR67W@lfurio.us","threadId":"66363","inReplyTo":"CAHeTm9OMLba_h0B2jRh_-GhogQXuwROBpX2jE__BPJ0GHq9P1A@mail.gmail.com","subject":"Re: [PATCH] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Matt Hunter","fromEmail":"m@lfurio.us","sentAt":"2026-09-24T07:50:12Z","receivedAt":"2026-09-24T07:58:12Z","isPatch":true,"body":"On Wed Sep 23, 2026 at 1:03 AM EDT, Colin Hinton wrote:\n>> > @@ -1962,6 +1953,14 @@ static int do_fetch(struct transport *transport,\n>> > if (transport->remote->fetch.nr) {\n>> > refspec_ref_prefixes(&transport->remote->fetch,\n>> > &transport_ls_refs_options.ref_prefixes);\n>> > +\n>> > + if (transport->remote->follow_remote_head)\n>> > + follow_remote_head = transport->remote->follow_remote_head;\n>>\n>> The code assumes that remote.*.followRemoteHEAD has been pre-parsed.\n>> Doesn't the code to do so in remote.c::handle_config() share exactly\n>> the same problem as you are fixing here?\n>>\n> I agree that the same problem that is being addressed here is present\n> in remote.c as well. The only difference being, that there is no\n> return call in the followremotehead block in remote.c,\n\nI'm not exactly sure why the config parsing in remote.c doesn't end with\na fallback 'return git_default_config(...)', though the followremotehead\ncase piggybacking the common 'return 0' at the end should be no problem.\n\n> and it at most only throws a warning if no valid value is present.\n\nwhich _was_ the case for fetch.followRemoteHEAD as well. So, we should\nkeep the two in sync right?\n\n> I think this\n> should be addressed, but I am uncertain if this is within the scope of\n> this issue and should be resolved now, or if this requires its own\n> investigation and should be resolved in a future patch. Regardless I\n> am eager to work on it, but would like some guidance as to what is\n> most appropriate for a change in remote.c.\n\nI spent some time drafting up what changes to remote.c could look like,\nbased on your work so far. This follow-up patch also has extra changes\nto builtin/fetch.c to accommodate the same allowed functionality as\nbefore. There are two awkward bits to this patch as-is, though:\n\nbuiltin/remote.c::set_head()\n\n012bc566bad7 (remote set-head: set followRemoteHEAD to \"warn\" if \"always\")\nadded this behavior to overrule a remote's \"always\" setting if the user\never modified their HEAD manually. So, this file needs to know about the\nfollowRemoteHEAD values, but parsing into the enums is currently confined\nto fetch.c. This just adds another bit of string parsing.\n\nbuiltin/fetch.c::get_follow_remote_head()\n\nis updated to serve double-duty for both the fetch and remote configs,\nand needs a better warning message if a bad value is detected. Perhaps\nadd another parameter to the function?\n\nWith this patch below, it's arguable whether the enum definition for the\nfollowRemoteHEAD values now better fits in fetch.c instead of remote.h.\n\n\nSigned-off-by: Matt Hunter \n---\n builtin/fetch.c | 54 ++++++++++++++++++++++++++++++++----------------\n builtin/remote.c | 3 ++-\n remote.c | 19 ++---------------\n remote.h | 3 +--\n 4 files changed, 41 insertions(+), 38 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 83074c48150b..5a4c9fb9309c 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -187,18 +187,34 @@ static int git_fetch_config(const char *k, const char *v,\n \treturn git_default_config(k, v, ctx, cb);\n }\n \n-static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n+/* TODO might be worth considering a better name for this */\n+struct follow_remote_head_target {\n+\tenum follow_remote_head_settings mode;\n+\tconst char *no_warn_branch;\n+};\n+\n+static struct follow_remote_head_target get_follow_remote_head(const char *setting,\n+\t\tint allow_warn_if_not_branch)\n {\n+\tstruct follow_remote_head_target frh = { 0 };\n+\n \tif (!strcmp(setting, \"never\"))\n-\t\treturn FOLLOW_REMOTE_NEVER;\n+\t\tfrh.mode = FOLLOW_REMOTE_NEVER;\n \telse if (!strcmp(setting, \"create\"))\n-\t\treturn FOLLOW_REMOTE_CREATE;\n+\t\tfrh.mode = FOLLOW_REMOTE_CREATE;\n \telse if (!strcmp(setting, \"warn\"))\n-\t\treturn FOLLOW_REMOTE_WARN;\n+\t\tfrh.mode = FOLLOW_REMOTE_WARN;\n+\telse if (skip_prefix(setting, \"warn-if-not-\", &frh.no_warn_branch)\n+\t\t\t&& allow_warn_if_not_branch)\n+\t\tfrh.mode = FOLLOW_REMOTE_WARN;\n \telse if (!strcmp(setting, \"always\"))\n-\t\treturn FOLLOW_REMOTE_ALWAYS;\n-\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n-\treturn FOLLOW_REMOTE_UNCONFIGURED;\n+\t\tfrh.mode = FOLLOW_REMOTE_ALWAYS;\n+\telse\n+\t\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n+\t\t/* TODO this also parses remote..followRemoteHEAD,\n+\t\t * but the warning string says fetch.followRemoteHEAD */\n+\n+\treturn frh;\n }\n \n static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)\n@@ -1758,12 +1774,11 @@ static void warn_set_head(const char *remote, const char *head_name,\n }\n \n static int set_head(const struct ref *remote_refs, struct remote *remote,\n-\t\t\tint follow_remote_head)\n+\t\t\tstruct follow_remote_head_target follow_remote_head)\n {\n \tint result = 0, create_only, baremirror, was_detached;\n \tstruct strbuf b_head = STRBUF_INIT, b_remote_head = STRBUF_INIT,\n \t\t b_local_head = STRBUF_INIT;\n-\tconst char *no_warn_branch = remote->no_warn_branch;\n \tchar *head_name = NULL;\n \tstruct ref *ref, *matches;\n \tstruct ref *fetch_map = NULL, **fetch_map_tail = &fetch_map;\n@@ -1793,7 +1808,7 @@ static int set_head(const struct ref *remote_refs, struct remote *remote,\n \tif (!head_name)\n \t\tgoto cleanup;\n \tbaremirror = is_bare_repository(the_repository) && remote->mirror;\n-\tcreate_only = follow_remote_head == FOLLOW_REMOTE_ALWAYS ? 0 : !baremirror;\n+\tcreate_only = follow_remote_head.mode == FOLLOW_REMOTE_ALWAYS ? 0 : !baremirror;\n \tif (baremirror) {\n \t\tstrbuf_addstr(&b_head, \"HEAD\");\n \t\tstrbuf_addf(&b_remote_head, \"refs/heads/%s\", head_name);\n@@ -1813,8 +1828,9 @@ static int set_head(const struct ref *remote_refs, struct remote *remote,\n \t\tgoto cleanup;\n \t}\n \tif (verbosity >= 0 &&\n-\t\tfollow_remote_head == FOLLOW_REMOTE_WARN &&\n-\t\t(!no_warn_branch || strcmp(no_warn_branch, head_name)))\n+\t\tfollow_remote_head.mode == FOLLOW_REMOTE_WARN &&\n+\t\t(!follow_remote_head.no_warn_branch ||\n+\t\t strcmp(follow_remote_head.no_warn_branch, head_name)))\n \t\twarn_set_head(remote->name, head_name, &b_local_head, was_detached);\n \n cleanup:\n@@ -1929,7 +1945,7 @@ static int do_fetch(struct transport *transport,\n \tstruct ref_update_display_info_array display_array = { 0 };\n \tstruct strmap rejected_refs = STRMAP_INIT;\n \tint summary_width = 0;\n-\tint follow_remote_head = 0;\n+\tstruct follow_remote_head_target follow_remote_head = { 0 };\n \n \tif (tags == TAGS_DEFAULT) {\n \t\tif (transport->remote->fetch_tags == 2)\n@@ -1954,14 +1970,16 @@ static int do_fetch(struct transport *transport,\n \t\t\trefspec_ref_prefixes(&transport->remote->fetch,\n \t\t\t\t\t &transport_ls_refs_options.ref_prefixes);\n \n-\t\t\tif (transport->remote->follow_remote_head)\n-\t\t\t\tfollow_remote_head = transport->remote->follow_remote_head;\n+\t\t\tif (transport->remote->follow_remote_head_raw)\n+\t\t\t\tfollow_remote_head = get_follow_remote_head(\n+\t\t\t\t\t\ttransport->remote->follow_remote_head_raw, 1);\n \t\t\telse if (config->follow_remote_head_raw)\n-\t\t\t\tfollow_remote_head = get_follow_remote_head(config->follow_remote_head_raw);\n+\t\t\t\tfollow_remote_head = get_follow_remote_head(\n+\t\t\t\t\t\tconfig->follow_remote_head_raw, 0);\n \t\t\telse\n-\t\t\t\tfollow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n+\t\t\t\tfollow_remote_head.mode = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n \t\t\t\n-\t\t\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n+\t\t\tif (follow_remote_head.mode != FOLLOW_REMOTE_NEVER)\n \t\t\t\tdo_set_head = 1;\n \t\t}\n \t\tif (branch && branch_has_merge_config(branch) &&\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex de989ea3ba96..89ac1f0daa82 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -1606,7 +1606,8 @@ static int set_head(int argc, const char **argv, const char *prefix,\n \t}\n \tif (opt_a)\n \t\treport_set_head_auto(argv[0], head_name, &b_local_head, was_detached);\n-\tif (remote->follow_remote_head == FOLLOW_REMOTE_ALWAYS) {\n+\tif (remote->follow_remote_head_raw &&\n+\t\t\t!strcmp(remote->follow_remote_head_raw, \"always\")) {\n \t\tstruct strbuf config_name = STRBUF_INIT;\n \t\tstrbuf_addf(&config_name,\n \t\t\t\"remote.%s.followremotehead\", remote->name);\ndiff --git a/remote.c b/remote.c\nindex fe6206846356..5fdcadfbdbf0 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -581,23 +581,8 @@ static int handle_config(const char *key, const char *value,\n \t\treturn parse_transport_option(key, value,\n \t\t\t\t\t &remote->negotiation_include);\n \t} else if (!strcmp(subkey, \"followremotehead\")) {\n-\t\tconst char *no_warn_branch;\n-\t\tif (!strcmp(value, \"never\"))\n-\t\t\tremote->follow_remote_head = FOLLOW_REMOTE_NEVER;\n-\t\telse if (!strcmp(value, \"create\"))\n-\t\t\tremote->follow_remote_head = FOLLOW_REMOTE_CREATE;\n-\t\telse if (!strcmp(value, \"warn\")) {\n-\t\t\tremote->follow_remote_head = FOLLOW_REMOTE_WARN;\n-\t\t\tremote->no_warn_branch = NULL;\n-\t\t} else if (skip_prefix(value, \"warn-if-not-\", &no_warn_branch)) {\n-\t\t\tremote->follow_remote_head = FOLLOW_REMOTE_WARN;\n-\t\t\tremote->no_warn_branch = no_warn_branch;\n-\t\t} else if (!strcmp(value, \"always\")) {\n-\t\t\tremote->follow_remote_head = FOLLOW_REMOTE_ALWAYS;\n-\t\t} else {\n-\t\t\twarning(_(\"unrecognized followRemoteHEAD value '%s' ignored\"),\n-\t\t\t\tvalue);\n-\t\t}\n+\t\tfree(remote->follow_remote_head_raw);\n+\t\tremote->follow_remote_head_raw = xstrdup(value);\n \t}\n \treturn 0;\n }\ndiff --git a/remote.h b/remote.h\nindex cca02033b9d7..cd97df017454 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -122,8 +122,7 @@ struct remote {\n \tstruct string_list negotiation_restrict;\n \tstruct string_list negotiation_include;\n \n-\tenum follow_remote_head_settings follow_remote_head;\n-\tconst char *no_warn_branch;\n+\tchar *follow_remote_head_raw;\n };\n \n /**\n-- \n2.55.0\n\n"},{"id":"553309","messageId":"CAHeTm9M9c7D71QUA-Dy9o_YD2PGbNYRa9LnNWKrYSaHEoUtpSQ@mail.gmail.com","threadId":"66363","inReplyTo":"DLNDRU6GIB30.1F5G8Z3JIR67W@lfurio.us","subject":"Re: [PATCH] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Colin Hinton","fromEmail":"colinlewishinton@gmail.com","sentAt":"2026-09-25T18:48:38Z","receivedAt":"2026-09-25T18:48:52Z","isPatch":true,"body":"On Thu, Sep 24, 2026 at 12:50 AM Matt Hunter wrote:\n>\n> On Wed Sep 23, 2026 at 1:03 AM EDT, Colin Hinton wrote:\n> >> > @@ -1962,6 +1953,14 @@ static int do_fetch(struct transport *transport,\n> >> > if (transport->remote->fetch.nr) {\n> >> > refspec_ref_prefixes(&transport->remote->fetch,\n> >> > &transport_ls_refs_options.ref_prefixes);\n> >> > +\n> >> > + if (transport->remote->follow_remote_head)\n> >> > + follow_remote_head = transport->remote->follow_remote_head;\n> >>\n> >> The code assumes that remote.*.followRemoteHEAD has been pre-parsed.\n> >> Doesn't the code to do so in remote.c::handle_config() share exactly\n> >> the same problem as you are fixing here?\n> >>\n> > I agree that the same problem that is being addressed here is present\n> > in remote.c as well. The only difference being, that there is no\n> > return call in the followremotehead block in remote.c,\n>\n> I'm not exactly sure why the config parsing in remote.c doesn't end with\n> a fallback 'return git_default_config(...)', though the followremotehead\n> case piggybacking the common 'return 0' at the end should be no problem.\n>\n> > and it at most only throws a warning if no valid value is present.\n>\n> which _was_ the case for fetch.followRemoteHEAD as well. So, we should\n> keep the two in sync right?\n>\nTo respond to both of your emails, I agree that the two should be kept in sync,\nas Junio pointed out, a valueless followremotehead will currently\nresult in a die,\nyet I agree with your point from your [1] that it would be more\nsensible to warn,\nand treat a valueless or bogus followremotehead as FOLLOW_REMOTE_NEVER.\nFor this patch, I will keep the behavior similar, but for a follow-on patch\nand with some approval I agree with this change.\n\n> > I think this\n> > should be addressed, but I am uncertain if this is within the scope of\n> > this issue and should be resolved now, or if this requires its own\n> > investigation and should be resolved in a future patch. Regardless I\n> > am eager to work on it, but would like some guidance as to what is\n> > most appropriate for a change in remote.c.\n>\n> I spent some time drafting up what changes to remote.c could look like,\n> based on your work so far. This follow-up patch also has extra changes\n> to builtin/fetch.c to accommodate the same allowed functionality as\n> before. There are two awkward bits to this patch as-is, though:\n>\n> builtin/remote.c::set_head()\n>\n> 012bc566bad7 (remote set-head: set followRemoteHEAD to \"warn\" if \"always\")\n> added this behavior to overrule a remote's \"always\" setting if the user\n> ever modified their HEAD manually. So, this file needs to know about the\n> followRemoteHEAD values, but parsing into the enums is currently confined\n> to fetch.c. This just adds another bit of string parsing.\n>\n> builtin/fetch.c::get_follow_remote_head()\n>\n> is updated to serve double-duty for both the fetch and remote configs,\n> and needs a better warning message if a bad value is detected. Perhaps\n> add another parameter to the function?\n>\n> With this patch below, it's arguable whether the enum definition for the\n> followRemoteHEAD values now better fits in fetch.c instead of remote.h.\n>\nThank you very much for this. I think this is a great starting point\nfor a follow up patch.\n\n-Colin Hinton\n"},{"id":"553313","messageId":"20260925192658.1166-1-colinlewishinton@gmail.com","threadId":"66363","inReplyTo":"20260922040047.2567-1-colinlewishinton@gmail.com","subject":"[PATCH v2] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Colin Hinton","fromEmail":"colinlewishinton@gmail.com","sentAt":"2026-09-25T19:26:58Z","receivedAt":"2026-09-25T19:27:15Z","isPatch":true,"body":"The value of the fetch.followRemoteHEAD configuration variable is\nvalidated while the configuration file is being parsed, which\nproduces a warning even when this particular \"git fetch\" invocation\nwill never consult it\n\nStore the raw config string instead, and resolve/validate it lazily\nat the one place in do_fetch() that actually uses it, so a stale or\nmistyped fetch.followRemoteHEAD value only produces a warning when\nthis fetch would have consulted it.\n\nSigned-off-by: Colin Hinton \n---\n builtin/fetch.c | 67 +++++++++++++++++++++++--------------------------\n 1 file changed, 31 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex ab7db2be06..6a5254a9bc 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -103,7 +103,7 @@ static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;\n \n struct fetch_config {\n \tenum display_format display_format;\n-\tenum follow_remote_head_settings follow_remote_head;\n+\tchar *follow_remote_head_raw;\n \tint all;\n \tint prune;\n \tint prune_tags;\n@@ -176,24 +176,31 @@ static int git_fetch_config(const char *k, const char *v,\n \t}\n \n \tif (!strcmp(k, \"fetch.followremotehead\")) {\n-\t\tif (!v)\n-\t\t\treturn config_error_nonbool(k);\n-\t\telse if (!strcmp(v, \"never\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;\n-\t\telse if (!strcmp(v, \"create\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;\n-\t\telse if (!strcmp(v, \"warn\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;\n-\t\telse if (!strcmp(v, \"always\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;\n-\t\telse\n-\t\t\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), v);\n+\t\tfree(fetch_config->follow_remote_head_raw);\n+\t\tfetch_config->follow_remote_head_raw = xstrdup(v);\n+\t\t\n \t\treturn 0;\n \t}\n \n \treturn git_default_config(k, v, ctx, cb);\n }\n \n+static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n+{\n+\tif (!setting)\n+\t\tdie(_(\"missing value for 'fetch.followRemoteHEAD'\"));\n+\telse if (!strcmp(setting, \"never\"))\n+\t\treturn FOLLOW_REMOTE_NEVER;\n+\telse if (!strcmp(setting, \"create\"))\n+\t\treturn FOLLOW_REMOTE_CREATE;\n+\telse if (!strcmp(setting, \"warn\"))\n+\t\treturn FOLLOW_REMOTE_WARN;\n+\telse if (!strcmp(setting, \"always\"))\n+\t\treturn FOLLOW_REMOTE_ALWAYS;\n+\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n+\treturn FOLLOW_REMOTE_UNCONFIGURED;\n+}\n+\n static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)\n {\n \tBUG_ON_OPT_NEG(unset);\n@@ -1918,11 +1925,10 @@ static int do_fetch(struct transport *transport,\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n \tstruct fetch_head fetch_head = { 0 };\n \tstruct strbuf err = STRBUF_INIT;\n-\tint do_set_head = 0;\n \tstruct ref_update_display_info_array display_array = { 0 };\n \tstruct strmap rejected_refs = STRMAP_INIT;\n \tint summary_width = 0;\n-\tint follow_remote_head;\n+\tint follow_remote_head = FOLLOW_REMOTE_NEVER;\n \n \tif (tags == TAGS_DEFAULT) {\n \t\tif (transport->remote->fetch_tags == 2)\n@@ -1938,22 +1944,6 @@ static int do_fetch(struct transport *transport,\n \t\t\tgoto cleanup;\n \t}\n \n-\t/*\n-\t * NEEDSWORK: By the time this function executes, we have already parsed\n-\t * all such followRemoteHEAD values from the external configuration,\n-\t * potentially emitting warning messages for bogus values. Ideally, if\n-\t * this fetch ends up not needing to consult these values, then git would\n-\t * not ever output a value warning. (eg: when pulling from a URL directly -\n-\t * rather than a configured remote, or when a remote's followRemoteHEAD\n-\t * overrides the fallback fetch setting)\n-\t */\n-\tif (transport->remote->follow_remote_head)\n-\t\tfollow_remote_head = transport->remote->follow_remote_head;\n-\telse if (config->follow_remote_head)\n-\t\tfollow_remote_head = config->follow_remote_head;\n-\telse\n-\t\tfollow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n-\n \tif (rs->nr) {\n \t\trefspec_ref_prefixes(rs, &transport_ls_refs_options.ref_prefixes);\n \t} else {\n@@ -1962,8 +1952,13 @@ static int do_fetch(struct transport *transport,\n \t\tif (transport->remote->fetch.nr) {\n \t\t\trefspec_ref_prefixes(&transport->remote->fetch,\n \t\t\t\t\t &transport_ls_refs_options.ref_prefixes);\n-\t\t\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n-\t\t\t\tdo_set_head = 1;\n+\n+\t\t\tif (transport->remote->follow_remote_head)\n+\t\t\t\tfollow_remote_head = transport->remote->follow_remote_head;\n+\t\t\telse if (config->follow_remote_head_raw)\n+\t\t\t\tfollow_remote_head = get_follow_remote_head(config->follow_remote_head_raw);\n+\t\t\telse\n+\t\t\t\tfollow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n \t\t}\n \t\tif (branch && branch_has_merge_config(branch) &&\n \t\t !strcmp(branch->remote_name, transport->remote->name)) {\n@@ -1987,7 +1982,7 @@ static int do_fetch(struct transport *transport,\n \t\tstrvec_push(&transport_ls_refs_options.ref_prefixes,\n \t\t\t \"refs/tags/\");\n \n-\tif (do_set_head)\n+\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n \t\tstrvec_push(&transport_ls_refs_options.ref_prefixes,\n \t\t\t \"HEAD\");\n \n@@ -2164,7 +2159,7 @@ static int do_fetch(struct transport *transport,\n \t\t\t\t \"you need to specify exactly one branch with the --set-upstream option\"));\n \t\t}\n \t}\n-\tif (do_set_head) {\n+\tif (follow_remote_head != FOLLOW_REMOTE_NEVER) {\n \t\t/*\n \t\t * Way too many cases where this can go wrong so let's just\n \t\t * ignore errors and fail silently for now.\n@@ -2509,7 +2504,7 @@ int cmd_fetch(int argc,\n {\n \tstruct fetch_config config = {\n \t\t.display_format = DISPLAY_FORMAT_FULL,\n-\t\t.follow_remote_head = FOLLOW_REMOTE_UNCONFIGURED,\n+\t\t.follow_remote_head_raw = NULL,\n \t\t.prune = -1,\n \t\t.prune_tags = -1,\n \t\t.show_forced_updates = 1,\n-- \n2.55.0.windows.3\n\n"},{"id":"553317","messageId":"xmqqo6dlt906.fsf@gitster.g","threadId":"66363","inReplyTo":"20260925192658.1166-1-colinlewishinton@gmail.com","subject":"Re: [PATCH v2] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-25T19:40:25Z","receivedAt":"2026-09-25T19:40:28Z","isPatch":true,"body":"Colin Hinton writes:\n\n> struct fetch_config {\n> \tenum display_format display_format;\n> -\tenum follow_remote_head_settings follow_remote_head;\n> +\tchar *follow_remote_head_raw;\n> \tint all;\n> \tint prune;\n> \tint prune_tags;\n> @@ -176,24 +176,31 @@ static int git_fetch_config(const char *k, const char *v,\n> \t}\n> \n> \tif (!strcmp(k, \"fetch.followremotehead\")) {\n> -\t\tif (!v)\n> -\t\t\treturn config_error_nonbool(k);\n> -\t\telse if (!strcmp(v, \"never\"))\n> -\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;\n> -\t\telse if (!strcmp(v, \"create\"))\n> -\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;\n> -\t\telse if (!strcmp(v, \"warn\"))\n> -\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;\n> -\t\telse if (!strcmp(v, \"always\"))\n> -\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;\n> -\t\telse\n> -\t\t\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), v);\n> +\t\tfree(fetch_config->follow_remote_head_raw);\n> +\t\tfetch_config->follow_remote_head_raw = xstrdup(v);\n\nThis will segfault when !v, so\n\n\tfetch_config->follow_remote_head_raw = xstrdup_or_null(v);\n\nWith that change,\n\n> +static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n> +{\n> +\tif (!setting)\n> +\t\tdie(_(\"missing value for 'fetch.followRemoteHEAD'\"));\n> +\telse if (!strcmp(setting, \"never\"))\n> +\t\treturn FOLLOW_REMOTE_NEVER;\n> +\telse if (!strcmp(setting, \"create\"))\n> +\t\treturn FOLLOW_REMOTE_CREATE;\n> +\telse if (!strcmp(setting, \"warn\"))\n> +\t\treturn FOLLOW_REMOTE_WARN;\n> +\telse if (!strcmp(setting, \"always\"))\n> +\t\treturn FOLLOW_REMOTE_ALWAYS;\n> +\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n> +\treturn FOLLOW_REMOTE_UNCONFIGURED;\n> +}\n\nThis would do a reasonable job.\n\nWe should do something similar to what remote.c parses for\nconsistency, but other than that, it seems this topic is moving in\nthe right direction.\n\nThanks.\n"},{"id":"553321","messageId":"CAHeTm9Pb-fb-ZS_m4UVNZxfp+ENQwBUGDvP1E24dEDTZy5RFFw@mail.gmail.com","threadId":"66363","inReplyTo":"xmqqo6dlt906.fsf@gitster.g","subject":"Re: [PATCH v2] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Colin Hinton","fromEmail":"colinlewishinton@gmail.com","sentAt":"2026-09-25T20:30:54Z","receivedAt":"2026-09-25T20:31:12Z","isPatch":true,"body":"On Fri, Sep 25, 2026 at 12:40 PM Junio C Hamano wrote:\n>\n> Colin Hinton writes:\n>\n> > struct fetch_config {\n> > enum display_format display_format;\n> > - enum follow_remote_head_settings follow_remote_head;\n> > + char *follow_remote_head_raw;\n> > int all;\n> > int prune;\n> > int prune_tags;\n> > @@ -176,24 +176,31 @@ static int git_fetch_config(const char *k, const char *v,\n> > }\n> >\n> > if (!strcmp(k, \"fetch.followremotehead\")) {\n> > - if (!v)\n> > - return config_error_nonbool(k);\n> > - else if (!strcmp(v, \"never\"))\n> > - fetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;\n> > - else if (!strcmp(v, \"create\"))\n> > - fetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;\n> > - else if (!strcmp(v, \"warn\"))\n> > - fetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;\n> > - else if (!strcmp(v, \"always\"))\n> > - fetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;\n> > - else\n> > - warning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), v);\n> > + free(fetch_config->follow_remote_head_raw);\n> > + fetch_config->follow_remote_head_raw = xstrdup(v);\n>\n> This will segfault when !v, so\n>\n> fetch_config->follow_remote_head_raw = xstrdup_or_null(v);\n\nWill change shortly.\n>\n> With that change,\n>\n> > +static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n> > +{\n> > + if (!setting)\n> > + die(_(\"missing value for 'fetch.followRemoteHEAD'\"));\n> > + else if (!strcmp(setting, \"never\"))\n> > + return FOLLOW_REMOTE_NEVER;\n> > + else if (!strcmp(setting, \"create\"))\n> > + return FOLLOW_REMOTE_CREATE;\n> > + else if (!strcmp(setting, \"warn\"))\n> > + return FOLLOW_REMOTE_WARN;\n> > + else if (!strcmp(setting, \"always\"))\n> > + return FOLLOW_REMOTE_ALWAYS;\n> > + warning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n> > + return FOLLOW_REMOTE_UNCONFIGURED;\n> > +}\n>\n> This would do a reasonable job.\n>\n> We should do something similar to what remote.c parses for\n> consistency, but other than that, it seems this topic is moving in\n> the right direction.\n>\n> Thanks.\n\nThe only critical difference I see in the configuration parse between\nremote.c and fetch.c is the case for \"warn-if-not-$branch\". From\nreading the git-config manpage, this is only a setting for a remote\nand not for fetch directly so I do not see a reason to check this in\nfetch.c. Perhaps I am missing something else to make this more\nconsistent, or perhaps there is an argument to support the\nconfiguration for fetch to \"warn-if-not-$branch\" in which case can be\nadded to this patch.\n\n-Colin Hinton\n"},{"id":"553338","messageId":"20260925230621.179649-1-colinlewishinton@gmail.com","threadId":"66363","inReplyTo":"20260925192658.1166-1-colinlewishinton@gmail.com","subject":"[PATCH v3] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Colin Hinton","fromEmail":"colinlewishinton@gmail.com","sentAt":"2026-09-25T23:06:21Z","receivedAt":"2026-09-25T23:06:28Z","isPatch":true,"body":"The value of the fetch.followRemoteHEAD configuration variable is\nvalidated while the configuration file is being parsed, which\nproduces a warning even when this particular \"git fetch\" invocation\nwill never consult it\n\nStore the raw config string instead, and resolve/validate it lazily\nat the one place in do_fetch() that actually uses it, so a stale or\nmistyped fetch.followRemoteHEAD value only produces a warning when\nthis fetch would have consulted it.\n\nSigned-off-by: Colin Hinton \n---\n builtin/fetch.c | 67 +++++++++++++++++++++++--------------------------\n 1 file changed, 31 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex ab7db2be06..85e3d1ca4b 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -103,7 +103,7 @@ static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;\n \n struct fetch_config {\n \tenum display_format display_format;\n-\tenum follow_remote_head_settings follow_remote_head;\n+\tchar *follow_remote_head_raw;\n \tint all;\n \tint prune;\n \tint prune_tags;\n@@ -176,24 +176,31 @@ static int git_fetch_config(const char *k, const char *v,\n \t}\n \n \tif (!strcmp(k, \"fetch.followremotehead\")) {\n-\t\tif (!v)\n-\t\t\treturn config_error_nonbool(k);\n-\t\telse if (!strcmp(v, \"never\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;\n-\t\telse if (!strcmp(v, \"create\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;\n-\t\telse if (!strcmp(v, \"warn\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;\n-\t\telse if (!strcmp(v, \"always\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;\n-\t\telse\n-\t\t\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), v);\n+\t\tfree(fetch_config->follow_remote_head_raw);\n+\t\tfetch_config->follow_remote_head_raw = xstrdup_or_null(v);\n+\t\t\n \t\treturn 0;\n \t}\n \n \treturn git_default_config(k, v, ctx, cb);\n }\n \n+static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n+{\n+\tif (!setting)\n+\t\tdie(_(\"missing value for 'fetch.followRemoteHEAD'\"));\n+\telse if (!strcmp(setting, \"never\"))\n+\t\treturn FOLLOW_REMOTE_NEVER;\n+\telse if (!strcmp(setting, \"create\"))\n+\t\treturn FOLLOW_REMOTE_CREATE;\n+\telse if (!strcmp(setting, \"warn\"))\n+\t\treturn FOLLOW_REMOTE_WARN;\n+\telse if (!strcmp(setting, \"always\"))\n+\t\treturn FOLLOW_REMOTE_ALWAYS;\n+\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n+\treturn FOLLOW_REMOTE_UNCONFIGURED;\n+}\n+\n static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)\n {\n \tBUG_ON_OPT_NEG(unset);\n@@ -1918,11 +1925,10 @@ static int do_fetch(struct transport *transport,\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n \tstruct fetch_head fetch_head = { 0 };\n \tstruct strbuf err = STRBUF_INIT;\n-\tint do_set_head = 0;\n \tstruct ref_update_display_info_array display_array = { 0 };\n \tstruct strmap rejected_refs = STRMAP_INIT;\n \tint summary_width = 0;\n-\tint follow_remote_head;\n+\tint follow_remote_head = FOLLOW_REMOTE_NEVER;\n \n \tif (tags == TAGS_DEFAULT) {\n \t\tif (transport->remote->fetch_tags == 2)\n@@ -1938,22 +1944,6 @@ static int do_fetch(struct transport *transport,\n \t\t\tgoto cleanup;\n \t}\n \n-\t/*\n-\t * NEEDSWORK: By the time this function executes, we have already parsed\n-\t * all such followRemoteHEAD values from the external configuration,\n-\t * potentially emitting warning messages for bogus values. Ideally, if\n-\t * this fetch ends up not needing to consult these values, then git would\n-\t * not ever output a value warning. (eg: when pulling from a URL directly -\n-\t * rather than a configured remote, or when a remote's followRemoteHEAD\n-\t * overrides the fallback fetch setting)\n-\t */\n-\tif (transport->remote->follow_remote_head)\n-\t\tfollow_remote_head = transport->remote->follow_remote_head;\n-\telse if (config->follow_remote_head)\n-\t\tfollow_remote_head = config->follow_remote_head;\n-\telse\n-\t\tfollow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n-\n \tif (rs->nr) {\n \t\trefspec_ref_prefixes(rs, &transport_ls_refs_options.ref_prefixes);\n \t} else {\n@@ -1962,8 +1952,13 @@ static int do_fetch(struct transport *transport,\n \t\tif (transport->remote->fetch.nr) {\n \t\t\trefspec_ref_prefixes(&transport->remote->fetch,\n \t\t\t\t\t &transport_ls_refs_options.ref_prefixes);\n-\t\t\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n-\t\t\t\tdo_set_head = 1;\n+\n+\t\t\tif (transport->remote->follow_remote_head)\n+\t\t\t\tfollow_remote_head = transport->remote->follow_remote_head;\n+\t\t\telse if (config->follow_remote_head_raw)\n+\t\t\t\tfollow_remote_head = get_follow_remote_head(config->follow_remote_head_raw);\n+\t\t\telse\n+\t\t\t\tfollow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n \t\t}\n \t\tif (branch && branch_has_merge_config(branch) &&\n \t\t !strcmp(branch->remote_name, transport->remote->name)) {\n@@ -1987,7 +1982,7 @@ static int do_fetch(struct transport *transport,\n \t\tstrvec_push(&transport_ls_refs_options.ref_prefixes,\n \t\t\t \"refs/tags/\");\n \n-\tif (do_set_head)\n+\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n \t\tstrvec_push(&transport_ls_refs_options.ref_prefixes,\n \t\t\t \"HEAD\");\n \n@@ -2164,7 +2159,7 @@ static int do_fetch(struct transport *transport,\n \t\t\t\t \"you need to specify exactly one branch with the --set-upstream option\"));\n \t\t}\n \t}\n-\tif (do_set_head) {\n+\tif (follow_remote_head != FOLLOW_REMOTE_NEVER) {\n \t\t/*\n \t\t * Way too many cases where this can go wrong so let's just\n \t\t * ignore errors and fail silently for now.\n@@ -2509,7 +2504,7 @@ int cmd_fetch(int argc,\n {\n \tstruct fetch_config config = {\n \t\t.display_format = DISPLAY_FORMAT_FULL,\n-\t\t.follow_remote_head = FOLLOW_REMOTE_UNCONFIGURED,\n+\t\t.follow_remote_head_raw = NULL,\n \t\t.prune = -1,\n \t\t.prune_tags = -1,\n \t\t.show_forced_updates = 1,\n-- \n2.55.0.windows.3\n\n"},{"id":"553668","messageId":"DLSD3JY380Q4.2VPO95D7M213G@lfurio.us","threadId":"66363","inReplyTo":"CAHeTm9Pb-fb-ZS_m4UVNZxfp+ENQwBUGDvP1E24dEDTZy5RFFw@mail.gmail.com","subject":"Re: [PATCH v2] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Matt Hunter","fromEmail":"m@lfurio.us","sentAt":"2026-09-30T04:21:48Z","receivedAt":"2026-09-30T04:22:00Z","isPatch":true,"body":"On Fri Sep 25, 2026 at 4:30 PM EDT, Colin Hinton wrote:\n> On Fri, Sep 25, 2026 at 12:40 PM Junio C Hamano wrote:\n>>\n>> We should do something similar to what remote.c parses for\n>> consistency, but other than that, it seems this topic is moving in\n>> the right direction.\n>>\n>> Thanks.\n>\n> The only critical difference I see in the configuration parse between\n> remote.c and fetch.c is the case for \"warn-if-not-$branch\". From\n> reading the git-config manpage, this is only a setting for a remote\n> and not for fetch directly so I do not see a reason to check this in\n> fetch.c. Perhaps I am missing something else to make this more\n> consistent,\n\nI agree with this assessment. However, I wonder if Junio meant\n\n We should (do something similar) to (what remote.c parses) ...\n\ninstead of\n\n We should do (something similar to what remote.c parses) ...\n\nas the issue in the NEEDSWORK _does_ apply to both sides.\n\nPerhaps at a minimum, this patch should leave the comment intact (or\nreworded) if not yet addressing remote.c. v3 otherwise is looking good\nto me, and functionality seems to work.\n"},{"id":"553742","messageId":"xmqqpkxua87a.fsf@gitster.g","threadId":"66363","inReplyTo":"DLSD3JY380Q4.2VPO95D7M213G@lfurio.us","subject":"Re: [PATCH v2] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-30T18:46:17Z","receivedAt":"2026-09-30T18:46:20Z","isPatch":true,"body":"\"Matt Hunter\" writes:\n\n> I agree with this assessment. However, I wonder if Junio meant\n>\n> We should (do something similar) to (what remote.c parses) ...\n>\n> instead of\n>\n> We should do (something similar to what remote.c parses) ...\n>\n> as the issue in the NEEDSWORK _does_ apply to both sides.\n\nExactly.\n\n> Perhaps at a minimum, this patch should leave the comment intact (or\n> reworded) if not yet addressing remote.c. v3 otherwise is looking good\n> to me, and functionality seems to work.\n\nTo end users, the annoyance factor due to an irrelevant incorrect\nsetting in fetch.followRemoteHEAD and remote.*.followRemoteHEAD\nvariables killing their \"git fetch\" are the same. Correcting one\nmay be better than correcting none, but until both gets corrected,\nwe cannot claim we helped users.\n\nThanks.\n"},{"id":"554043","messageId":"CAHeTm9MBx_ndL1XkdyjtjFaGvoF69AHwP6DLYsirpBVsiNRfAA@mail.gmail.com","threadId":"66363","inReplyTo":"xmqqpkxua87a.fsf@gitster.g","subject":"Re: [PATCH v2] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Colin Hinton","fromEmail":"colinlewishinton@gmail.com","sentAt":"2026-10-03T04:50:41Z","receivedAt":"2026-10-03T04:50:52Z","isPatch":true,"body":"> > Perhaps at a minimum, this patch should leave the comment intact (or\n> > reworded) if not yet addressing remote.c. v3 otherwise is looking good\n> > to me, and functionality seems to work.\n>\n> To end users, the annoyance factor due to an irrelevant incorrect\n> setting in fetch.followRemoteHEAD and remote.*.followRemoteHEAD\n> variables killing their \"git fetch\" are the same. Correcting one\n> may be better than correcting none, but until both gets corrected,\n> we cannot claim we helped users.\n>\nAll good points, I will add the NEEDSWORK back into this patch\nas this is a half measure to the entire problem; however, rather than\nleaving the NEEDSWORK in fetch.c, I will move it to remote.c near the\nremaining defect in handle_config(), and maybe add a short comment in\nfetch.c for context of this fix. Unless there are any concerns, V4\nshould be released soon.\n\n-Colin Hinton\n"},{"id":"554090","messageId":"20261003231422.6004-1-colinlewishinton@gmail.com","threadId":"66363","inReplyTo":"20260925230621.179649-1-colinlewishinton@gmail.com","subject":"[PATCH v4] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Colin Hinton","fromEmail":"colinlewishinton@gmail.com","sentAt":"2026-10-03T23:14:22Z","receivedAt":"2026-10-03T23:14:25Z","isPatch":true,"body":"The value of the fetch.followRemoteHEAD configuration variable is\nvalidated while the configuration file is being parsed, which\nproduces a warning even when this particular \"git fetch\" invocation\nwill never consult it.\n\nStore the raw config string instead, and resolve/validate it lazily\nat the one place in do_fetch() that actually uses it, so a mistyped\nvalue only warns, and a missing value only dies, when this fetch\nwould have consulted it.\n\nremote.c's handle_config() has the same problem for\nremote..followRemoteHEAD, but is left unaddressed here since\nit touches shared remote-parsing infrastructure used well beyond\nfetch. Leave NEEDSWORK comments at both the now unresolved call site\nin do_fetch() and at the actual defect in handle_config(), so the\nremaining scope is easy to find for a follow-up patch.\n\nSigned-off-by: Colin Hinton \n---\n builtin/fetch.c | 68 ++++++++++++++++++++++++-------------------------\n remote.c | 7 +++++\n 2 files changed, 41 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 533fdfe7d8..2cb0bcca8b 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -103,7 +103,7 @@ static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;\n \n struct fetch_config {\n \tenum display_format display_format;\n-\tenum follow_remote_head_settings follow_remote_head;\n+\tchar *follow_remote_head_raw;\n \tint all;\n \tint prune;\n \tint prune_tags;\n@@ -176,24 +176,33 @@ static int git_fetch_config(const char *k, const char *v,\n \t}\n \n \tif (!strcmp(k, \"fetch.followremotehead\")) {\n+\t\tfree(fetch_config->follow_remote_head_raw);\n \t\tif (!v)\n-\t\t\treturn config_error_nonbool(k);\n-\t\telse if (!strcmp(v, \"never\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;\n-\t\telse if (!strcmp(v, \"create\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;\n-\t\telse if (!strcmp(v, \"warn\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;\n-\t\telse if (!strcmp(v, \"always\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;\n+\t\t\tfetch_config->follow_remote_head_raw = xstrdup(\"\");\n \t\telse\n-\t\t\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), v);\n+\t\t\tfetch_config->follow_remote_head_raw = xstrdup(v);\n \t\treturn 0;\n \t}\n \n \treturn git_default_config(k, v, ctx, cb);\n }\n \n+static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n+{\n+\tif (!setting || !*setting)\n+\t\tdie(_(\"missing value for 'fetch.followRemoteHEAD'\"));\n+\telse if (!strcmp(setting, \"never\"))\n+\t\treturn FOLLOW_REMOTE_NEVER;\n+\telse if (!strcmp(setting, \"create\"))\n+\t\treturn FOLLOW_REMOTE_CREATE;\n+\telse if (!strcmp(setting, \"warn\"))\n+\t\treturn FOLLOW_REMOTE_WARN;\n+\telse if (!strcmp(setting, \"always\"))\n+\t\treturn FOLLOW_REMOTE_ALWAYS;\n+\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n+\treturn BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n+}\n+\n static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)\n {\n \tBUG_ON_OPT_NEG(unset);\n@@ -1918,11 +1927,10 @@ static int do_fetch(struct transport *transport,\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n \tstruct fetch_head fetch_head = { 0 };\n \tstruct strbuf err = STRBUF_INIT;\n-\tint do_set_head = 0;\n \tstruct ref_update_display_info_array display_array = { 0 };\n \tstruct strmap rejected_refs = STRMAP_INIT;\n \tint summary_width = 0;\n-\tint follow_remote_head;\n+\tint follow_remote_head = FOLLOW_REMOTE_NEVER;\n \n \tif (tags == TAGS_DEFAULT) {\n \t\tif (transport->remote->fetch_tags == 2)\n@@ -1938,22 +1946,6 @@ static int do_fetch(struct transport *transport,\n \t\t\tgoto cleanup;\n \t}\n \n-\t/*\n-\t * NEEDSWORK: By the time this function executes, we have already parsed\n-\t * all such followRemoteHEAD values from the external configuration,\n-\t * potentially emitting warning messages for bogus values. Ideally, if\n-\t * this fetch ends up not needing to consult these values, then git would\n-\t * not ever output a value warning. (eg: when pulling from a URL directly -\n-\t * rather than a configured remote, or when a remote's followRemoteHEAD\n-\t * overrides the fallback fetch setting)\n-\t */\n-\tif (transport->remote->follow_remote_head)\n-\t\tfollow_remote_head = transport->remote->follow_remote_head;\n-\telse if (config->follow_remote_head)\n-\t\tfollow_remote_head = config->follow_remote_head;\n-\telse\n-\t\tfollow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n-\n \tif (rs->nr) {\n \t\trefspec_ref_prefixes(rs, &transport_ls_refs_options.ref_prefixes);\n \t} else {\n@@ -1962,8 +1954,16 @@ static int do_fetch(struct transport *transport,\n \t\tif (transport->remote->fetch.nr) {\n \t\t\trefspec_ref_prefixes(&transport->remote->fetch,\n \t\t\t\t\t &transport_ls_refs_options.ref_prefixes);\n-\t\t\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n-\t\t\t\tdo_set_head = 1;\n+\t\t\t/*\n+\t\t\t * See remote.c's handling of remote..followRemoteHEAD\n+\t\t\t * for the analogous, still-unresolved case.\n+\t\t\t */\n+\t\t\tif (transport->remote->follow_remote_head)\n+\t\t\t\tfollow_remote_head = transport->remote->follow_remote_head;\n+\t\t\telse if (config->follow_remote_head_raw)\n+\t\t\t\tfollow_remote_head = get_follow_remote_head(config->follow_remote_head_raw);\n+\t\t\telse\n+\t\t\t\tfollow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n \t\t}\n \t\tif (branch && branch_has_merge_config(branch) &&\n \t\t !strcmp(branch->remote_name, transport->remote->name)) {\n@@ -1987,7 +1987,7 @@ static int do_fetch(struct transport *transport,\n \t\tstrvec_push(&transport_ls_refs_options.ref_prefixes,\n \t\t\t \"refs/tags/\");\n \n-\tif (do_set_head)\n+\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n \t\tstrvec_push(&transport_ls_refs_options.ref_prefixes,\n \t\t\t \"HEAD\");\n \n@@ -2164,7 +2164,7 @@ static int do_fetch(struct transport *transport,\n \t\t\t\t \"you need to specify exactly one branch with the --set-upstream option\"));\n \t\t}\n \t}\n-\tif (do_set_head) {\n+\tif (follow_remote_head != FOLLOW_REMOTE_NEVER) {\n \t\t/*\n \t\t * Way too many cases where this can go wrong so let's just\n \t\t * ignore errors and fail silently for now.\n@@ -2509,7 +2509,7 @@ int cmd_fetch(int argc,\n {\n \tstruct fetch_config config = {\n \t\t.display_format = DISPLAY_FORMAT_FULL,\n-\t\t.follow_remote_head = FOLLOW_REMOTE_UNCONFIGURED,\n+\t\t.follow_remote_head_raw = NULL,\n \t\t.prune = -1,\n \t\t.prune_tags = -1,\n \t\t.show_forced_updates = 1,\ndiff --git a/remote.c b/remote.c\nindex fe62068463..58f3436222 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -582,6 +582,13 @@ static int handle_config(const char *key, const char *value,\n \t\t\t\t\t &remote->negotiation_include);\n \t} else if (!strcmp(subkey, \"followremotehead\")) {\n \t\tconst char *no_warn_branch;\n+\t\t/*\n+\t\t * NEEDSWORK: this is validated/warned about here, during config\n+\t\t * parsing, regardless of whether the fetch that triggered this\n+\t\t * parse will ever consult it for this particular remote. See\n+\t\t * fetch.c's deferred handling of fetch.followRemoteHEAD for the\n+\t\t * pattern this should likely follow.\n+\t\t */\n \t\tif (!strcmp(value, \"never\"))\n \t\t\tremote->follow_remote_head = FOLLOW_REMOTE_NEVER;\n \t\telse if (!strcmp(value, \"create\"))\n-- \n2.55.0.windows.3\n\n"},{"id":"554112","messageId":"xmqqa4otvbnk.fsf@gitster.g","threadId":"66363","inReplyTo":"20261003231422.6004-1-colinlewishinton@gmail.com","subject":"Re: [PATCH v4] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-04T13:27:27Z","receivedAt":"2026-10-04T13:27:30Z","isPatch":true,"body":"Colin Hinton writes:\n\n> \tif (!strcmp(k, \"fetch.followremotehead\")) {\n> +\t\tfree(fetch_config->follow_remote_head_raw);\n> \t\tif (!v)\n> +\t\t\tfetch_config->follow_remote_head_raw = xstrdup(\"\");\n> \t\telse\n> +\t\t\tfetch_config->follow_remote_head_raw = xstrdup(v);\n\nHmph, this means that the code cannot distinguish between\n\n\t[fetch] followremotehead\n\n\t[fetch] followremotehead = \"\"\n\nIt would be less code and more expressive if you lost the\nconditional, i.e.,\n\n\tif (!strcmp(k, \"fetch.followremotehead\"))\n\t\tfree(fetch_config->follow_remote_head_raw);\n\t\tfetch_config->follow_remote_head_raw = xstrdup_or_null(v);\n\t}\n\n> +static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n> +{\n> +\tif (!setting || !*setting)\n> +\t\tdie(_(\"missing value for 'fetch.followRemoteHEAD'\"));\n\nThen you can differenciate\n\n\tif (!setting)\n\t\t... we got '[fetch] followRemoteHEAD' ...\n\t\tdie() as before, complaining that the this is not a Bool.\n\telse if (!*setting)\n\t\t... we got '[fetch] followRemoteHEAD = \"\"' ...\n\nif we wanted to. It probably do not need to check for an empty\nstring as it will fall through the \"else if\" cascade below and\neventually end up with the warning + default. \n\n> +\telse if (!strcmp(setting, \"never\"))\n> +\t\treturn FOLLOW_REMOTE_NEVER;\n> +\telse if (!strcmp(setting, \"create\"))\n> +\t\treturn FOLLOW_REMOTE_CREATE;\n> +\telse if (!strcmp(setting, \"warn\"))\n> +\t\treturn FOLLOW_REMOTE_WARN;\n> +\telse if (!strcmp(setting, \"always\"))\n> +\t\treturn FOLLOW_REMOTE_ALWAYS;\n> +\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n> +\treturn BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n> +}\n"},{"id":"554115","messageId":"CAHeTm9PK=sc4ajmf53rhurd532OST0qYfEaS-Kc5kpGZf1Zw2A@mail.gmail.com","threadId":"66363","inReplyTo":"xmqqa4otvbnk.fsf@gitster.g","subject":"Re: [PATCH v4] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Colin Hinton","fromEmail":"colinlewishinton@gmail.com","sentAt":"2026-10-04T15:40:13Z","receivedAt":"2026-10-04T15:40:26Z","isPatch":true,"body":"My rationale for this recent change came from when I was evaluating\nwhat calls get_follow_remote_head() in my patch.\n\nWith the current design, get_follow_remote_head() is only called in\ndo_fetch() in this conditional else if\n(config->follow_remote_head_raw).\n\nIf followRemoteHEAD is now NULL because we set it as such in\nfetch_config->follow_remote_head_raw = xstrdup_or_null(v); Then this\nconditional is skipped, and we will never call the die(), and alert\nthe user that their value is blank.\n\nTo fully fix based on your suggestion, I suppose the design question\nis, should empty string warn or die?\n\nIf empty string should warn, I likely will need to add some value in\nthe fetch_config struct such as follow_remote_head_seen, and use this\nas our conditional in do_fetch() rather than the\nfollow_remote_head_raw, to account for when followRemoteHEAD was set\nto anything. Then when the check in get_follow_remote_head() occurs,\nwe know to die or warn based on NULL, or bogus.\n\nVisually, it would look something like this.\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 2cb0bcca8b..af22f63954 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -104,6 +104,7 @@ static struct string_list negotiation_include =\nSTRING_LIST_INIT_NODUP;\n struct fetch_config {\n enum display_format display_format;\n char *follow_remote_head_raw;\n+ int follow_remote_head_seen;\n int all;\n int prune;\n int prune_tags;\n@@ -177,10 +178,8 @@ static int git_fetch_config(const char *k, const char *v,\n\n if (!strcmp(k, \"fetch.followremotehead\")) {\n free(fetch_config->follow_remote_head_raw);\n- if (!v)\n- fetch_config->follow_remote_head_raw = xstrdup(\"\");\n- else\n- fetch_config->follow_remote_head_raw = xstrdup(v);\n+ fetch_config->follow_remote_head_raw = xstrdup_or_null(v);\n+ follow_remote_head_seen = 1;\n return 0;\n }\n\n@@ -189,7 +188,7 @@ static int git_fetch_config(const char *k, const char *v,\n\n static enum follow_remote_head_settings get_follow_remote_head(const\nchar *setting)\n {\n- if (!setting || !*setting)\n+ if (!setting) /*!*setting would return true on \"\" removing to\nwarn instead*/\n die(_(\"missing value for 'fetch.followRemoteHEAD'\"));\n else if (!strcmp(setting, \"never\"))\n return FOLLOW_REMOTE_NEVER;\n@@ -1960,7 +1959,7 @@ static int do_fetch(struct transport *transport,\n */\n if (transport->remote->follow_remote_head)\n follow_remote_head =\ntransport->remote->follow_remote_head;\n- else if (config->follow_remote_head_raw)\n+ else if (config->follow_remote_head_seen)\n follow_remote_head =\nget_follow_remote_head(config->follow_remote_head_raw);\n else\n follow_remote_head =\nBUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n@@ -2510,6 +2509,7 @@ int cmd_fetch(int argc,\n struct fetch_config config = {\n .display_format = DISPLAY_FORMAT_FULL,\n .follow_remote_head_raw = NULL,\n+ .follow_remote_head_seen = 0,\n .prune = -1,\n .prune_tags = -1,\n .show_forced_updates = 1,\n\nLet me know if this sounds right, and I will add this in for v5.\n-Colin Hinton\n\nOn Sun, Oct 4, 2026 at 6:27 AM Junio C Hamano wrote:\n>\n> Colin Hinton writes:\n>\n> > if (!strcmp(k, \"fetch.followremotehead\")) {\n> > + free(fetch_config->follow_remote_head_raw);\n> > if (!v)\n> > + fetch_config->follow_remote_head_raw = xstrdup(\"\");\n> > else\n> > + fetch_config->follow_remote_head_raw = xstrdup(v);\n>\n> Hmph, this means that the code cannot distinguish between\n>\n> [fetch] followremotehead\n>\n> [fetch] followremotehead = \"\"\n>\n> It would be less code and more expressive if you lost the\n> conditional, i.e.,\n>\n> if (!strcmp(k, \"fetch.followremotehead\"))\n> free(fetch_config->follow_remote_head_raw);\n> fetch_config->follow_remote_head_raw = xstrdup_or_null(v);\n> }\n>\n> > +static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n> > +{\n> > + if (!setting || !*setting)\n> > + die(_(\"missing value for 'fetch.followRemoteHEAD'\"));\n>\n> Then you can differenciate\n>\n> if (!setting)\n> ... we got '[fetch] followRemoteHEAD' ...\n> die() as before, complaining that the this is not a Bool.\n> else if (!*setting)\n> ... we got '[fetch] followRemoteHEAD = \"\"' ...\n>\n> if we wanted to. It probably do not need to check for an empty\n> string as it will fall through the \"else if\" cascade below and\n> eventually end up with the warning + default.\n>\n> > + else if (!strcmp(setting, \"never\"))\n> > + return FOLLOW_REMOTE_NEVER;\n> > + else if (!strcmp(setting, \"create\"))\n> > + return FOLLOW_REMOTE_CREATE;\n> > + else if (!strcmp(setting, \"warn\"))\n> > + return FOLLOW_REMOTE_WARN;\n> > + else if (!strcmp(setting, \"always\"))\n> > + return FOLLOW_REMOTE_ALWAYS;\n> > + warning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n> > + return BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n> > +}\n"},{"id":"554121","messageId":"xmqqh5j1s6q0.fsf@gitster.g","threadId":"66363","inReplyTo":"CAHeTm9PK=sc4ajmf53rhurd532OST0qYfEaS-Kc5kpGZf1Zw2A@mail.gmail.com","subject":"Re: [PATCH v4] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-04T17:42:15Z","receivedAt":"2026-10-04T17:42:17Z","isPatch":true,"body":"Colin Hinton writes:\n\n> My rationale for this recent change came from when I was evaluating\n> what calls get_follow_remote_head() in my patch.\n>\n> With the current design, get_follow_remote_head() is only called in\n> do_fetch() in this conditional else if\n> (config->follow_remote_head_raw).\n>\n> If followRemoteHEAD is now NULL because we set it as such in\n> fetch_config->follow_remote_head_raw = xstrdup_or_null(v); Then this\n> conditional is skipped, and we will never call the die(), and alert\n> the user that their value is blank.\n>\n> To fully fix based on your suggestion, I suppose the design question\n> is, should empty string warn or die?\n\nI think we should behave the same when we see \"nvere\". We do not\nunderstand what they wanted us to do in either case, so we should\nbehave the same way, be it warn-and-ignore or complain-and-die.\n\nGiven that we now check the validity of the value only after we\ndetermine that we need it, I think it is OK to tighten the rules\nto die() instead of warn(). The historical behavior of not dying,\nand instead warning and ignoring, was a weak excuse for leaving\nconfiguration parsing broken and checking the validity of the value\nin the wrong place.\n\nThis patch rectifies the situation, which is a very good step\ntoward doing the right thing. It is perfectly fine to tighten\nthe rules as a separate topic after this patch lands and things\nstabilize, but this patch lays the groundwork for us to move in\nthat direction.\n\n> If empty string should warn, I likely will need to add some value in\n> the fetch_config struct such as follow_remote_head_seen, and use this\n> as our conditional in do_fetch() rather than the\n> follow_remote_head_raw, to account for when followRemoteHEAD was set\n> to anything. Then when the check in get_follow_remote_head() occurs,\n> we know to die or warn based on NULL, or bogus.\n\n... because? Ah, because then you lose distinction between \"the\nconfiguration variable not set at all\" and \"the configuration\nvariable is set to the valueless true\"?\n\nIf so, you'd need to be able to tell _three_ cases. The empty\nstring you use as a stand in for \"valueless true\" should be\ndistinguishable from the empty string the user set (by mistake).\n\n> Visually, it would look something like this.\n>\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index 2cb0bcca8b..af22f63954 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -104,6 +104,7 @@ static struct string_list negotiation_include =\n> STRING_LIST_INIT_NODUP;\n> struct fetch_config {\n> enum display_format display_format;\n> char *follow_remote_head_raw;\n> + int follow_remote_head_seen;\n\nThat would certainly work, and might be easier to work with than\nwhat I would have done, which is not to bother with this extra\nvariable and instead to have a\n\n\tstatic const char *valueless_true = \"true\";\n\nin the file scope. Then use that ...\n\n> int all;\n> int prune;\n> int prune_tags;\n> @@ -177,10 +178,8 @@ static int git_fetch_config(const char *k, const char *v,\n>\n> if (!strcmp(k, \"fetch.followremotehead\")) {\n> free(fetch_config->follow_remote_head_raw);\n> - if (!v)\n> - fetch_config->follow_remote_head_raw = xstrdup(\"\");\n> - else\n> - fetch_config->follow_remote_head_raw = xstrdup(v);\n> + fetch_config->follow_remote_head_raw = xstrdup_or_null(v);\n\n... here like so:\n\n\t\tif (fetch_config->follow_remote_head_raw != valueless_true)\n\t\t\tfree(fetch_config->follow_remote_head_raw);\n\t\tif (!v)\n\t\t\tfetch_config->follow_remote_head_raw = valueless_true;\n\t\telse\n\t\t\t...\n\n\n> @@ -189,7 +188,7 @@ static int git_fetch_config(const char *k, const char *v,\n>\n> static enum follow_remote_head_settings get_follow_remote_head(const\n> char *setting)\n> {\n> - if (!setting || !*setting)\n> + if (!setting) /*!*setting would return true on \"\" removing to\n> warn instead*/\n> die(_(\"missing value for 'fetch.followRemoteHEAD'\"));\n> else if (!strcmp(setting, \"never\"))\n> return FOLLOW_REMOTE_NEVER;\n\n... and deal with the setting that is equal to valueless_true here.\n\nI think either way would work, and the way you outlined would be\nbetter.\n\n"},{"id":"554129","messageId":"20261004201428.5210-1-colinlewishinton@gmail.com","threadId":"66363","inReplyTo":"20261003231422.6004-1-colinlewishinton@gmail.com","subject":"[PATCH v5] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Colin Hinton","fromEmail":"colinlewishinton@gmail.com","sentAt":"2026-10-04T20:14:27Z","receivedAt":"2026-10-04T20:14:32Z","isPatch":true,"body":"The value of the fetch.followRemoteHEAD configuration variable is\nvalidated while the configuration file is being parsed, which\nproduces a warning even when this particular \"git fetch\" invocation\nwill never consult it.\n\nStore the raw config string instead, and resolve/validate it lazily\nat the one place in do_fetch() that actually uses it, so a mistyped\nvalue only warns, and a missing value only dies, when this fetch\nwould have consulted it.\n\nremote.c's handle_config() has the same problem for\nremote..followRemoteHEAD, but is left unaddressed here since\nit touches shared remote-parsing infrastructure used well beyond\nfetch. Leave NEEDSWORK comments at both the now unresolved call site\nin do_fetch() and at the actual defect in handle_config(), so the\nremaining scope is easy to find for a follow-up patch.\n\nSigned-off-by: Colin Hinton \n---\n builtin/fetch.c | 72 ++++++++++++++++++++++++-------------------------\n remote.c | 7 +++++\n 2 files changed, 43 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 533fdfe7d8..e0b4394fea 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -103,7 +103,8 @@ static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;\n \n struct fetch_config {\n \tenum display_format display_format;\n-\tenum follow_remote_head_settings follow_remote_head;\n+\tchar *follow_remote_head_raw;\n+\tint follow_remote_head_seen;\n \tint all;\n \tint prune;\n \tint prune_tags;\n@@ -176,24 +177,31 @@ static int git_fetch_config(const char *k, const char *v,\n \t}\n \n \tif (!strcmp(k, \"fetch.followremotehead\")) {\n-\t\tif (!v)\n-\t\t\treturn config_error_nonbool(k);\n-\t\telse if (!strcmp(v, \"never\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;\n-\t\telse if (!strcmp(v, \"create\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;\n-\t\telse if (!strcmp(v, \"warn\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;\n-\t\telse if (!strcmp(v, \"always\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;\n-\t\telse\n-\t\t\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), v);\n+\t\tfree(fetch_config->follow_remote_head_raw);\n+\t\tfetch_config->follow_remote_head_raw = xstrdup_or_null(v);\n+\t\tfetch_config->follow_remote_head_seen = 1;\n \t\treturn 0;\n \t}\n \n \treturn git_default_config(k, v, ctx, cb);\n }\n \n+static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n+{\n+\tif (!setting)\n+\t\tdie(_(\"missing value for 'fetch.followRemoteHEAD'\"));\n+\telse if (!strcmp(setting, \"never\"))\n+\t\treturn FOLLOW_REMOTE_NEVER;\n+\telse if (!strcmp(setting, \"create\"))\n+\t\treturn FOLLOW_REMOTE_CREATE;\n+\telse if (!strcmp(setting, \"warn\"))\n+\t\treturn FOLLOW_REMOTE_WARN;\n+\telse if (!strcmp(setting, \"always\"))\n+\t\treturn FOLLOW_REMOTE_ALWAYS;\n+\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n+\treturn BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n+}\n+\n static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)\n {\n \tBUG_ON_OPT_NEG(unset);\n@@ -1918,11 +1926,10 @@ static int do_fetch(struct transport *transport,\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n \tstruct fetch_head fetch_head = { 0 };\n \tstruct strbuf err = STRBUF_INIT;\n-\tint do_set_head = 0;\n \tstruct ref_update_display_info_array display_array = { 0 };\n \tstruct strmap rejected_refs = STRMAP_INIT;\n \tint summary_width = 0;\n-\tint follow_remote_head;\n+\tint follow_remote_head = FOLLOW_REMOTE_NEVER;\n \n \tif (tags == TAGS_DEFAULT) {\n \t\tif (transport->remote->fetch_tags == 2)\n@@ -1938,22 +1945,6 @@ static int do_fetch(struct transport *transport,\n \t\t\tgoto cleanup;\n \t}\n \n-\t/*\n-\t * NEEDSWORK: By the time this function executes, we have already parsed\n-\t * all such followRemoteHEAD values from the external configuration,\n-\t * potentially emitting warning messages for bogus values. Ideally, if\n-\t * this fetch ends up not needing to consult these values, then git would\n-\t * not ever output a value warning. (eg: when pulling from a URL directly -\n-\t * rather than a configured remote, or when a remote's followRemoteHEAD\n-\t * overrides the fallback fetch setting)\n-\t */\n-\tif (transport->remote->follow_remote_head)\n-\t\tfollow_remote_head = transport->remote->follow_remote_head;\n-\telse if (config->follow_remote_head)\n-\t\tfollow_remote_head = config->follow_remote_head;\n-\telse\n-\t\tfollow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n-\n \tif (rs->nr) {\n \t\trefspec_ref_prefixes(rs, &transport_ls_refs_options.ref_prefixes);\n \t} else {\n@@ -1962,8 +1953,16 @@ static int do_fetch(struct transport *transport,\n \t\tif (transport->remote->fetch.nr) {\n \t\t\trefspec_ref_prefixes(&transport->remote->fetch,\n \t\t\t\t\t &transport_ls_refs_options.ref_prefixes);\n-\t\t\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n-\t\t\t\tdo_set_head = 1;\n+\t\t\t/*\n+\t\t\t * See remote.c's handling of remote..followRemoteHEAD\n+\t\t\t * for the analogous, still-unresolved case.\n+\t\t\t */\n+\t\t\tif (transport->remote->follow_remote_head)\n+\t\t\t\tfollow_remote_head = transport->remote->follow_remote_head;\n+\t\t\telse if (config->follow_remote_head_seen)\n+\t\t\t\tfollow_remote_head = get_follow_remote_head(config->follow_remote_head_raw);\n+\t\t\telse\n+\t\t\t\tfollow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n \t\t}\n \t\tif (branch && branch_has_merge_config(branch) &&\n \t\t !strcmp(branch->remote_name, transport->remote->name)) {\n@@ -1987,7 +1986,7 @@ static int do_fetch(struct transport *transport,\n \t\tstrvec_push(&transport_ls_refs_options.ref_prefixes,\n \t\t\t \"refs/tags/\");\n \n-\tif (do_set_head)\n+\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n \t\tstrvec_push(&transport_ls_refs_options.ref_prefixes,\n \t\t\t \"HEAD\");\n \n@@ -2164,7 +2163,7 @@ static int do_fetch(struct transport *transport,\n \t\t\t\t \"you need to specify exactly one branch with the --set-upstream option\"));\n \t\t}\n \t}\n-\tif (do_set_head) {\n+\tif (follow_remote_head != FOLLOW_REMOTE_NEVER) {\n \t\t/*\n \t\t * Way too many cases where this can go wrong so let's just\n \t\t * ignore errors and fail silently for now.\n@@ -2509,7 +2508,8 @@ int cmd_fetch(int argc,\n {\n \tstruct fetch_config config = {\n \t\t.display_format = DISPLAY_FORMAT_FULL,\n-\t\t.follow_remote_head = FOLLOW_REMOTE_UNCONFIGURED,\n+\t\t.follow_remote_head_raw = NULL,\n+\t\t.follow_remote_head_seen = 0,\n \t\t.prune = -1,\n \t\t.prune_tags = -1,\n \t\t.show_forced_updates = 1,\ndiff --git a/remote.c b/remote.c\nindex fe62068463..58f3436222 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -582,6 +582,13 @@ static int handle_config(const char *key, const char *value,\n \t\t\t\t\t &remote->negotiation_include);\n \t} else if (!strcmp(subkey, \"followremotehead\")) {\n \t\tconst char *no_warn_branch;\n+\t\t/*\n+\t\t * NEEDSWORK: this is validated/warned about here, during config\n+\t\t * parsing, regardless of whether the fetch that triggered this\n+\t\t * parse will ever consult it for this particular remote. See\n+\t\t * fetch.c's deferred handling of fetch.followRemoteHEAD for the\n+\t\t * pattern this should likely follow.\n+\t\t */\n \t\tif (!strcmp(value, \"never\"))\n \t\t\tremote->follow_remote_head = FOLLOW_REMOTE_NEVER;\n \t\telse if (!strcmp(value, \"create\"))\n-- \n2.55.0.windows.3\n\n"},{"id":"554156","messageId":"DLWS9R3ZM2Z0.MVT71MWE19LN@lfurio.us","threadId":"66363","inReplyTo":"20261004201428.5210-1-colinlewishinton@gmail.com","subject":"Re: [PATCH v5] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Matt Hunter","fromEmail":"m@lfurio.us","sentAt":"2026-10-05T09:05:51Z","receivedAt":"2026-10-05T09:06:05Z","isPatch":true,"body":"Hi Colin,\n\nJust a couple comments and 'thinking aloud' on the design of the patch\nfrom me. Unfortunately I don't have time at the moment to test this new\nrevision.\n\nthanks\n\nOn Sun Oct 4, 2026 at 4:14 PM EDT, Colin Hinton wrote:\n> The value of the fetch.followRemoteHEAD configuration variable is\n> validated while the configuration file is being parsed, which\n> produces a warning even when this particular \"git fetch\" invocation\n> will never consult it.\n>\n> Store the raw config string instead, and resolve/validate it lazily\n> at the one place in do_fetch() that actually uses it, so a mistyped\n> value only warns, and a missing value only dies, when this fetch\n> would have consulted it.\n>\n> remote.c's handle_config() has the same problem for\n> remote..followRemoteHEAD, but is left unaddressed here since\n> it touches shared remote-parsing infrastructure used well beyond\n> fetch. Leave NEEDSWORK comments at both the now unresolved call site\n\nThe new comment in fetch.c doesn't actually have the NEEDSWORK label. I\nwould probably suggest placing one there, instead of adjusting this\nsentence.\n\n> @@ -103,7 +103,8 @@ static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;\n> \n> struct fetch_config {\n> \tenum display_format display_format;\n> -\tenum follow_remote_head_settings follow_remote_head;\n> +\tchar *follow_remote_head_raw;\n> +\tint follow_remote_head_seen;\n> \tint all;\n> \tint prune;\n> \tint prune_tags;\n> @@ -176,24 +177,31 @@ static int git_fetch_config(const char *k, const char *v,\n> \t}\n> \n> \tif (!strcmp(k, \"fetch.followremotehead\")) {\n> -\t\tif (!v)\n> -\t\t\treturn config_error_nonbool(k);\n> -\t\telse if (!strcmp(v, \"never\"))\n> -\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;\n> -\t\telse if (!strcmp(v, \"create\"))\n> -\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;\n> -\t\telse if (!strcmp(v, \"warn\"))\n> -\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;\n> -\t\telse if (!strcmp(v, \"always\"))\n> -\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;\n> -\t\telse\n> -\t\t\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), v);\n> +\t\tfree(fetch_config->follow_remote_head_raw);\n> +\t\tfetch_config->follow_remote_head_raw = xstrdup_or_null(v);\n> +\t\tfetch_config->follow_remote_head_seen = 1;\n> \t\treturn 0;\n> \t}\n\nWe now have 'follow_remote_head_seen', which tells us whether to trust\nthe 'raw' string ptr or if the variable hasn't been set by the user.\n\nTherefore, when 'seen' is true, the raw string is:\n\n - NULL when a valueless true was specified\n - \"\" when an actual empty string was specified\n - any other string for a normal value\n\nBecause of this ...\n\n> \n> \treturn git_default_config(k, v, ctx, cb);\n> }\n> \n> +static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n> +{\n> +\tif (!setting)\n> +\t\tdie(_(\"missing value for 'fetch.followRemoteHEAD'\"));\n\n... this case is now meaningful, as we couldn't previously reach this if\n'setting' was NULL.\n\n> +\telse if (!strcmp(setting, \"never\"))\n> +\t\treturn FOLLOW_REMOTE_NEVER;\n> +\telse if (!strcmp(setting, \"create\"))\n> +\t\treturn FOLLOW_REMOTE_CREATE;\n> +\telse if (!strcmp(setting, \"warn\"))\n> +\t\treturn FOLLOW_REMOTE_WARN;\n> +\telse if (!strcmp(setting, \"always\"))\n> +\t\treturn FOLLOW_REMOTE_ALWAYS;\n> +\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n\nAnd the empty string case is handled here, which makes sense.\n\nI believe Junio had mentioned tightening this warning to a die. I'm not\nsure if it was _just_ this case or something else. Personally, I think the\nwarning is still the better call for some \"f@KeValue\". But it _could_\nmake sense to die on \"\", since that is more obviously mis-configured.\n\nHaving typed the above out, I now realize that is actually how the\nprevious v4 behaved (but in slightly less code). So, we're getting into\nopinionated details here... Though, as-is I think this v5 implementation\nis reasonable.\n\n> @@ -2509,7 +2508,8 @@ int cmd_fetch(int argc,\n> {\n> \tstruct fetch_config config = {\n> \t\t.display_format = DISPLAY_FORMAT_FULL,\n> -\t\t.follow_remote_head = FOLLOW_REMOTE_UNCONFIGURED,\n> +\t\t.follow_remote_head_raw = NULL,\n> +\t\t.follow_remote_head_seen = 0,\n\nand the 'seen' variable is initialized to false, good.\n\n> \t\t.prune = -1,\n> \t\t.prune_tags = -1,\n> \t\t.show_forced_updates = 1,\n> diff --git a/remote.c b/remote.c\n> index fe62068463..58f3436222 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -582,6 +582,13 @@ static int handle_config(const char *key, const char *value,\n> \t\t\t\t\t &remote->negotiation_include);\n> \t} else if (!strcmp(subkey, \"followremotehead\")) {\n> \t\tconst char *no_warn_branch;\n> +\t\t/*\n> +\t\t * NEEDSWORK: this is validated/warned about here, during config\n> +\t\t * parsing, regardless of whether the fetch that triggered this\n> +\t\t * parse will ever consult it for this particular remote. See\n> +\t\t * fetch.c's deferred handling of fetch.followRemoteHEAD for the\n> +\t\t * pattern this should likely follow.\n> +\t\t */\n> \t\tif (!strcmp(value, \"never\"))\n> \t\t\tremote->follow_remote_head = FOLLOW_REMOTE_NEVER;\n> \t\telse if (!strcmp(value, \"create\"))\n"},{"id":"554171","messageId":"xmqqqzi4nvn4.fsf@gitster.g","threadId":"66363","inReplyTo":"20261004201428.5210-1-colinlewishinton@gmail.com","subject":"Re: [PATCH v5] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-05T13:07:27Z","receivedAt":"2026-10-05T13:07:30Z","isPatch":true,"body":"Colin Hinton writes:\n\nI see there are only two minor things remaining in this iteration.\n\n> fetch. Leave NEEDSWORK comments at both the now unresolved call site\n> in do_fetch() and at the actual defect in handle_config(), so the\n> remaining scope is easy to find for a follow-up patch.\n\nHere is one of the two. There is only one NEEDSWORK, not \"at both\".\n\n\tLeave a NEEDSWORK comment at remote.c:handle_config() that\n\thas a defect similar to what is fixed by this patch, so ...\n\nshould be sufficient.\n\nAnother is that \n\n int cmd_fetch(int argc,\n const char **argv,\n const char *prefix,\n struct repository *repo UNUSED)\n {\n struct fetch_config config = {\n .display_format = DISPLAY_FORMAT_FULL,\n .follow_remote_head_raw = NULL,\n .follow_remote_head_seen = 0,\n .prune = -1,\n .prune_tags = -1,\n .show_forced_updates = 1,\n .recurse_submodules = RECURSE_SUBMODULES_DEFAULT,\n .parallel = 1,\n .submodule_fetch_jobs = -1,\n };\n\nwill hold onto a copy of config.follow_remote_head_seen that was\nread from the configuration and never frees it, so when cmd_fetch()\nleaves, it technically leaks a string.\n\nOther than these two points, this round looks very good.\n\nThanks.\n"},{"id":"554231","messageId":"20261006032258.6561-1-colinlewishinton@gmail.com","threadId":"66363","inReplyTo":"20261004201428.5210-1-colinlewishinton@gmail.com","subject":"[PATCH v6] fetch.c: defer fetch.followRemoteHEAD validation","fromName":"Colin Hinton","fromEmail":"colinlewishinton@gmail.com","sentAt":"2026-10-06T03:22:58Z","receivedAt":"2026-10-06T03:22:58Z","isPatch":true,"body":"The value of the fetch.followRemoteHEAD configuration variable is\nvalidated while the configuration file is being parsed, which\nproduces a warning even when this particular \"git fetch\" invocation\nwill never consult it.\n\nStore the raw config string instead, and resolve/validate it lazily\nat the one place in do_fetch() that actually uses it, so a mistyped\nvalue only warns, and a missing value only dies, when this fetch\nwould have consulted it.\n\nremote.c's handle_config() has the same problem for\nremote..followRemoteHEAD, but is left unaddressed here since\nit touches shared remote-parsing infrastructure used well beyond\nfetch. Leave a NEEDSWORK comment at remote.c:handle_config()\nthat has a defect similar to what is fixed by this patch,\nso the remaining scope is easy to find for a follow-up patch.\n\nSigned-off-by: Colin Hinton \n---\n builtin/fetch.c | 73 +++++++++++++++++++++++++------------------------\n remote.c | 7 +++++\n 2 files changed, 44 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 533fdfe7d8..d800978c38 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -103,7 +103,8 @@ static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;\n \n struct fetch_config {\n \tenum display_format display_format;\n-\tenum follow_remote_head_settings follow_remote_head;\n+\tchar *follow_remote_head_raw;\n+\tint follow_remote_head_seen;\n \tint all;\n \tint prune;\n \tint prune_tags;\n@@ -176,24 +177,31 @@ static int git_fetch_config(const char *k, const char *v,\n \t}\n \n \tif (!strcmp(k, \"fetch.followremotehead\")) {\n-\t\tif (!v)\n-\t\t\treturn config_error_nonbool(k);\n-\t\telse if (!strcmp(v, \"never\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;\n-\t\telse if (!strcmp(v, \"create\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;\n-\t\telse if (!strcmp(v, \"warn\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;\n-\t\telse if (!strcmp(v, \"always\"))\n-\t\t\tfetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;\n-\t\telse\n-\t\t\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), v);\n+\t\tfree(fetch_config->follow_remote_head_raw);\n+\t\tfetch_config->follow_remote_head_raw = xstrdup_or_null(v);\n+\t\tfetch_config->follow_remote_head_seen = 1;\n \t\treturn 0;\n \t}\n \n \treturn git_default_config(k, v, ctx, cb);\n }\n \n+static enum follow_remote_head_settings get_follow_remote_head(const char *setting)\n+{\n+\tif (!setting)\n+\t\tdie(_(\"missing value for 'fetch.followRemoteHEAD'\"));\n+\telse if (!strcmp(setting, \"never\"))\n+\t\treturn FOLLOW_REMOTE_NEVER;\n+\telse if (!strcmp(setting, \"create\"))\n+\t\treturn FOLLOW_REMOTE_CREATE;\n+\telse if (!strcmp(setting, \"warn\"))\n+\t\treturn FOLLOW_REMOTE_WARN;\n+\telse if (!strcmp(setting, \"always\"))\n+\t\treturn FOLLOW_REMOTE_ALWAYS;\n+\twarning(_(\"unrecognized fetch.followRemoteHEAD value '%s' ignored\"), setting);\n+\treturn BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n+}\n+\n static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)\n {\n \tBUG_ON_OPT_NEG(unset);\n@@ -1918,11 +1926,10 @@ static int do_fetch(struct transport *transport,\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n \tstruct fetch_head fetch_head = { 0 };\n \tstruct strbuf err = STRBUF_INIT;\n-\tint do_set_head = 0;\n \tstruct ref_update_display_info_array display_array = { 0 };\n \tstruct strmap rejected_refs = STRMAP_INIT;\n \tint summary_width = 0;\n-\tint follow_remote_head;\n+\tint follow_remote_head = FOLLOW_REMOTE_NEVER;\n \n \tif (tags == TAGS_DEFAULT) {\n \t\tif (transport->remote->fetch_tags == 2)\n@@ -1938,22 +1945,6 @@ static int do_fetch(struct transport *transport,\n \t\t\tgoto cleanup;\n \t}\n \n-\t/*\n-\t * NEEDSWORK: By the time this function executes, we have already parsed\n-\t * all such followRemoteHEAD values from the external configuration,\n-\t * potentially emitting warning messages for bogus values. Ideally, if\n-\t * this fetch ends up not needing to consult these values, then git would\n-\t * not ever output a value warning. (eg: when pulling from a URL directly -\n-\t * rather than a configured remote, or when a remote's followRemoteHEAD\n-\t * overrides the fallback fetch setting)\n-\t */\n-\tif (transport->remote->follow_remote_head)\n-\t\tfollow_remote_head = transport->remote->follow_remote_head;\n-\telse if (config->follow_remote_head)\n-\t\tfollow_remote_head = config->follow_remote_head;\n-\telse\n-\t\tfollow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n-\n \tif (rs->nr) {\n \t\trefspec_ref_prefixes(rs, &transport_ls_refs_options.ref_prefixes);\n \t} else {\n@@ -1962,8 +1953,16 @@ static int do_fetch(struct transport *transport,\n \t\tif (transport->remote->fetch.nr) {\n \t\t\trefspec_ref_prefixes(&transport->remote->fetch,\n \t\t\t\t\t &transport_ls_refs_options.ref_prefixes);\n-\t\t\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n-\t\t\t\tdo_set_head = 1;\n+\t\t\t/*\n+\t\t\t * See remote.c's handling of remote..followRemoteHEAD\n+\t\t\t * for the analogous, still-unresolved case.\n+\t\t\t */\n+\t\t\tif (transport->remote->follow_remote_head)\n+\t\t\t\tfollow_remote_head = transport->remote->follow_remote_head;\n+\t\t\telse if (config->follow_remote_head_seen)\n+\t\t\t\tfollow_remote_head = get_follow_remote_head(config->follow_remote_head_raw);\n+\t\t\telse\n+\t\t\t\tfollow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;\n \t\t}\n \t\tif (branch && branch_has_merge_config(branch) &&\n \t\t !strcmp(branch->remote_name, transport->remote->name)) {\n@@ -1987,7 +1986,7 @@ static int do_fetch(struct transport *transport,\n \t\tstrvec_push(&transport_ls_refs_options.ref_prefixes,\n \t\t\t \"refs/tags/\");\n \n-\tif (do_set_head)\n+\tif (follow_remote_head != FOLLOW_REMOTE_NEVER)\n \t\tstrvec_push(&transport_ls_refs_options.ref_prefixes,\n \t\t\t \"HEAD\");\n \n@@ -2164,7 +2163,7 @@ static int do_fetch(struct transport *transport,\n \t\t\t\t \"you need to specify exactly one branch with the --set-upstream option\"));\n \t\t}\n \t}\n-\tif (do_set_head) {\n+\tif (follow_remote_head != FOLLOW_REMOTE_NEVER) {\n \t\t/*\n \t\t * Way too many cases where this can go wrong so let's just\n \t\t * ignore errors and fail silently for now.\n@@ -2509,7 +2508,8 @@ int cmd_fetch(int argc,\n {\n \tstruct fetch_config config = {\n \t\t.display_format = DISPLAY_FORMAT_FULL,\n-\t\t.follow_remote_head = FOLLOW_REMOTE_UNCONFIGURED,\n+\t\t.follow_remote_head_raw = NULL,\n+\t\t.follow_remote_head_seen = 0,\n \t\t.prune = -1,\n \t\t.prune_tags = -1,\n \t\t.show_forced_updates = 1,\n@@ -2929,5 +2929,6 @@ int cmd_fetch(int argc,\n cleanup:\n \tstring_list_clear(&list, 0);\n \tlist_objects_filter_release(&filter_options);\n+\tfree(config.follow_remote_head_raw);\n \treturn result;\n }\ndiff --git a/remote.c b/remote.c\nindex fe62068463..58f3436222 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -582,6 +582,13 @@ static int handle_config(const char *key, const char *value,\n \t\t\t\t\t &remote->negotiation_include);\n \t} else if (!strcmp(subkey, \"followremotehead\")) {\n \t\tconst char *no_warn_branch;\n+\t\t/*\n+\t\t * NEEDSWORK: this is validated/warned about here, during config\n+\t\t * parsing, regardless of whether the fetch that triggered this\n+\t\t * parse will ever consult it for this particular remote. See\n+\t\t * fetch.c's deferred handling of fetch.followRemoteHEAD for the\n+\t\t * pattern this should likely follow.\n+\t\t */\n \t\tif (!strcmp(value, \"never\"))\n \t\t\tremote->follow_remote_head = FOLLOW_REMOTE_NEVER;\n \t\telse if (!strcmp(value, \"create\"))\n-- \n2.55.0.windows.3\n\n\n"}]}