Re: [PATCH 6/7] fetch: add configuration option fetch.followRemoteHEAD

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 12, 2026, 14:17 UTC
Message-ID: <xmqqik7nj11i.fsf@gitster.g>
In-Reply-To: <20260612055947.1499497-7-m@lfurio.us>

Matt Hunter <m@lfurio.us> writes:

I haven't been following the discussion, so I will not comment on the idea, i.e., if it makes sense to add such a new option and configuration, but if we were to add such a thing, I have some comments on the mechanics.

By the way, do not call a "configuration variable" a "configuration option". Let's keep the vocabulary forcused without using random synonyms.

Show 31 quoted lines

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 3cc7efdd83a0..a21bb82274d4 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -103,6 +103,7 @@ static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;
>  
>  struct fetch_config {
>  	enum display_format display_format;
> +	enum follow_remote_head_settings follow_remote_head;
>  	int all;
>  	int prune;
>  	int prune_tags;
> @@ -173,6 +174,22 @@ static int git_fetch_config(const char *k, const char *v,
>  			    "fetch.output", v);
>  	}
>  
> +	if (!strcmp(k, "fetch.followremotehead")) {
> +		if (!v)
> +			return config_error_nonbool(k);
> +		else if (!strcasecmp(v, "never"))
> +			fetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;
> +		else if (!strcasecmp(v, "create"))
> +			fetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;
> +		else if (!strcasecmp(v, "warn"))
> +			fetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;
> +		else if (!strcasecmp(v, "always"))
> +			fetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;
> +		else
> +			die(_("invalid value for '%s': '%s'"),
> +				"fetch.followRemoteHEAD", v);
> +	}

I think these uses of strcasecmp() are unnecessary and actively harms end-user experience. This is especially true because the value given to remote.<name>.followRemoteHEAD is case sensitive.

Instead of saying "if you want X to happen, set this variable to 'create'", you have to say "'create', or any other case variations thereof like 'CrEAte'" somehow, for very dubious gain to the end users. If you use strcmp(), and document only all lowercase forms, it would guarantee to avoid confusing a newbie who read the variable to be set to 'never' on one blog and 'Never' on another and wonder if 'NEVER' would work or not.

Admittedly values to some existing configuration variables may be parsed case insensitively but we should aim to fix the mistake in the longer term, and we should certainly not add more of them.

Is it sensible to die() here? If you are fetching from somewhere without keeping a set of remote-tracking branches for it (i.e., a single shot "git fetch https://github.com/gitster/git master"), you do not care what garbage value is in fetch.followRemoteHEAD. Perhaps the version of Git that is slightly newer than the version that ships with this patch defined new valid values that this patch does not know about, and such a user who is doing a single-shot fetch may have that setting to help them working with their usual non-single shot repositories, but they use a newer version of Git for such regular work, and they are using slightly old version of Git to perform this single-shot fetch. The point is that the configured value will *NOT* be used for such a user, and dying only because this piece of code does not understand the configuration that will not be used is of dubious value.