Re: [PATCH v2 0/7] Introduce fetch.followRemoteHEAD config variable
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 16, 2026, 23:18 UTC
- Message-ID
- <xmqqh5n213bw.fsf@gitster.g>
- In-Reply-To
- <20260616222606.1003521-1-m@lfurio.us>
Matt Hunter <m@lfurio.us> writes:
Show 6 quoted lines
> Changes in v2: > - Don't die() if the value of fetch.followRemoteHEAD is unrecognized. > - Use case-sensitive matching for fetch.followRemoteHEAD values. > - Avoid the phrase "configuration option". > - Minor documentation wording changes. > - Link to v1: https://patch.msgid.link/20260612055947.1499497-1-m@lfurio.us
Show 20 quoted lines
> @@ builtin/fetch.c: static int git_fetch_config(const char *k, const char *v,
> + if (!strcmp(k, "fetch.followremotehead")) {
> + if (!v)
> + return config_error_nonbool(k);
> -+ else if (!strcasecmp(v, "never"))
> ++ else if (!strcmp(v, "never"))
> + fetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;
> -+ else if (!strcasecmp(v, "create"))
> ++ else if (!strcmp(v, "create"))
> + fetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;
> -+ else if (!strcasecmp(v, "warn"))
> ++ else if (!strcmp(v, "warn"))
> + fetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;
> -+ else if (!strcasecmp(v, "always"))
> ++ else if (!strcmp(v, "always"))
> + fetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;
> -+ else
> -+ die(_("invalid value for '%s': '%s'"),
> -+ "fetch.followRemoteHEAD", v);
> + }Not dying on an unrecognised value is certainly better than dying, but shouldn't we at least clear fetch_config->follow_remote_head to some "unspecified" or "default" value? What does the existing parser routine for remote.*.followremotehead do?
Ideally,
(1) If the "fetch" operation ends up with not needing to consult
the value of fetch.followRemoteHEAD at all (e.g., it is a
one-shot fetch that updates no remote-tracking hierarchy, or it
has a more specific per-remote setting that this variable is
meant to serve as a mere fallback), any bogus or unknown value
will not get any warning. (2) If fetch.followRemoteHEAD ends up being _used_, and if it has
an unknown value, we should at least warn "we do not understand
what you wrote, 'awlays', and we ignore it", or die "we do not
understand 'reset', perhaps it is from a future version of Git?".I do not think customization based on git_config() callback like the above can easily implement such an ideal semantics.
And I suspect that the existing per-remote configuration that this variable is meant to serve as a fallback definition would not work in such an ideal way (i.e., even if we are doing one-shot fetch that does not touch any remote-tracking hierarchies, "git fetch" may warn if the value is not understood, and when we do need the value, the code would only warn and does not die), so in that sense this new code is not making things _worse_, even though it may be spreading the same badness more widely X-<.
Thanks.