Junio C Hamano <gitster@xxxxxxxxx> writes: > Patrick Steinhardt <ps@xxxxxx> writes: > >> I think the current version of this patch series is fine as-is then. It >> might be a good idea to adapt this in a follow-up series, unless there >> is a good reason not to. > > Sounds good. We may want to tighten the rules a bit to reject > obvious misconfigurations, but that is outside the scope of this > topic. This reminds me of a slightly related tangent. There are pathname-valued ones that we added deliberately as an optional configuration variable, IIRC, and fsck.skiplist may be one of them. In other words, they mean "In a repository that I want the configuration to affect, I'll have the configuration file at this path, but it is merely optional. If it is missing, pretend as if the configuration variable does not even exist". In retrospect, it was a mistake to define that this variable is required and triggers an error when misconfigured, and that variable is optional and triggers only a warning when misconfigured. That is one more thing for the user to remember and does not scale. We probably should have done something like - the value given to all pathname valued configuration variables and command name options MUST correctly name a path that the command can read, and a misconfigured configuration variable or an invalid command line option should trigger an error when the command needs to access it. - the value (not the variable) can say "I am optional--if the path does not appear on this system, or if it is unreadable, pretend as if this configuration variable or the command line option were not given". The "when the command needs to" part is important. Ideally, "git log" should not fail when core.hookspath is misconfigured, because it does not trigger any hook, but it currently dies while parsing the configuration files in git_default_config(): $ git -c core.hookspath log error: missing value for 'core.hookspath' fatal: unable to parse 'core.hookspath' from command-line config. which we may want to fix. Unsurprisingly there are other variables that do behave correctly, like $ git -c core.pager --no-pager log -20 >/dev/null $ git -c core.pager log -20 >/dev/null error: missing value for 'core.pager' fatal: unable to parse command-line config where a misconfigured core.pager does not cause any trouble when the pager is not in use. Any of the above are not within the scope of this topic, obviously. cf. <20241014204427.1712182-1-gitster@xxxxxxxxx>
