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

My rationale for this recent change came from when I was evaluating
what calls get_follow_remote_head() in my patch.

With the current design, get_follow_remote_head() is only called in
do_fetch() in this conditional else if
(config->follow_remote_head_raw).

If followRemoteHEAD is now NULL because we set it as such in
fetch_config->follow_remote_head_raw = xstrdup_or_null(v); Then this
conditional is skipped, and we will never call the die(), and alert
the user that their value is blank.

To fully fix based on your suggestion, I suppose the design question
is, should empty string warn or die?

If empty string should warn, I likely will need to add some value in
the fetch_config struct such as follow_remote_head_seen, and use this
as our conditional in do_fetch() rather than the
follow_remote_head_raw, to account for when followRemoteHEAD was set
to anything. Then when the check in get_follow_remote_head() occurs,
we know to die or warn based on NULL, or bogus.

Visually, it would look something like this.

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 2cb0bcca8b..af22f63954 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c

@@ -104,6 +104,7 @@ static struct string_list negotiation_include =

STRING_LIST_INIT_NODUP;
 struct fetch_config {
        enum display_format display_format;
        char *follow_remote_head_raw;
+       int follow_remote_head_seen;
        int all;
        int prune;
        int prune_tags;

@@ -177,10 +178,8 @@ static int git_fetch_config(const char *k, const char *v,

        if (!strcmp(k, "fetch.followremotehead")) {
                free(fetch_config->follow_remote_head_raw);
-               if (!v)
-                       fetch_config->follow_remote_head_raw = xstrdup("");
-               else
-                       fetch_config->follow_remote_head_raw = xstrdup(v);
+               fetch_config->follow_remote_head_raw = xstrdup_or_null(v);
+               follow_remote_head_seen = 1;
                return 0;
        }

@@ -189,7 +188,7 @@ static int git_fetch_config(const char *k, const char *v,

 static enum follow_remote_head_settings get_follow_remote_head(const

char *setting)
 {
-       if (!setting || !*setting)
+       if (!setting) /*!*setting would return true on "" removing to
warn instead*/
                die(_("missing value for 'fetch.followRemoteHEAD'"));
        else if (!strcmp(setting, "never"))
                return FOLLOW_REMOTE_NEVER;

@@ -1960,7 +1959,7 @@ static int do_fetch(struct transport *transport,
                         */
                        if (transport->remote->follow_remote_head)
                                follow_remote_head =

transport->remote->follow_remote_head;
-                       else if (config->follow_remote_head_raw)
+                       else if (config->follow_remote_head_seen)
                                follow_remote_head =
get_follow_remote_head(config->follow_remote_head_raw);
                        else
                                follow_remote_head =
BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;

@@ -2510,6 +2509,7 @@ int cmd_fetch(int argc,
        struct fetch_config config = {
                .display_format = DISPLAY_FORMAT_FULL,
                .follow_remote_head_raw = NULL,
+               .follow_remote_head_seen = 0,
                .prune = -1,
                .prune_tags = -1,
                .show_forced_updates = 1,

Let me know if this sounds right, and I will add this in for v5.
-Colin Hinton

On Sun, Oct 4, 2026 at 6:27 AM Junio C Hamano [off-list ref] wrote:

Colin Hinton [off-list ref] writes:

quoted

      if (!strcmp(k, "fetch.followremotehead")) {
+             free(fetch_config->follow_remote_head_raw);
              if (!v)
+                     fetch_config->follow_remote_head_raw = xstrdup("");
              else
+                     fetch_config->follow_remote_head_raw = xstrdup(v);

Hmph, this means that the code cannot distinguish between

        [fetch] followremotehead

        [fetch] followremotehead = ""

It would be less code and more expressive if you lost the
conditional, i.e.,

        if (!strcmp(k, "fetch.followremotehead"))
                free(fetch_config->follow_remote_head_raw);
                fetch_config->follow_remote_head_raw = xstrdup_or_null(v);
        }

quoted

+static enum follow_remote_head_settings get_follow_remote_head(const char *setting)
+{
+     if (!setting || !*setting)
+             die(_("missing value for 'fetch.followRemoteHEAD'"));

Then you can differenciate

        if (!setting)
                ... we got '[fetch] followRemoteHEAD' ...
                die() as before, complaining that the this is not a Bool.
        else if (!*setting)
                ... we got '[fetch] followRemoteHEAD = ""' ...

if we wanted to.  It probably do not need to check for an empty
string as it will fall through the "else if" cascade below and
eventually end up with the warning + default.

quoted

+     else if (!strcmp(setting, "never"))
+             return FOLLOW_REMOTE_NEVER;
+     else if (!strcmp(setting, "create"))
+             return FOLLOW_REMOTE_CREATE;
+     else if (!strcmp(setting, "warn"))
+             return FOLLOW_REMOTE_WARN;
+     else if (!strcmp(setting, "always"))
+             return FOLLOW_REMOTE_ALWAYS;
+     warning(_("unrecognized fetch.followRemoteHEAD value '%s' ignored"), setting);
+     return BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
+}