Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > On Tue, May 31 2022, Glen Choo via GitGitGadget wrote: > >> From: Glen Choo <chooglen@xxxxxxxxxx> >> >> Branch names can't be empty, so config keys with an empty branch name, >> e.g. "branch..remote", are silently ignored. >> >> Since these config keys will never be useful, make it a fatal error when >> remote.c finds a key that starts with "branch." and has an empty >> subsection. > > Perhaps this is fine, but I think this commit message (and I checked the > CL too) really needs to work a bit harder to convince us that this is > safe to do. Fair. > Are we confident that this is just bizarro config that nobody would have > had in practice? In that case I think it's fine to start dying on it. > > But as I understand we previously just ignored this, then if there's any > doubt about that perhaps we should start with a warning? > > Or are we really confident that this is an edge case not worth worrying > about in that way, and that we can go straight to die()? The case I want to make is even stronger than that - this is an edge case that _we_ shouldn't worry about, and we should tell the _user_ that their config is bogus. It truly makes no sense because `branch..remote` fits the schema of `branch.<name>.remote` where <name> is "", but "" isn't a valid branch name (and it never has been AFAIK). So such a key would never be useful to Git, and it would be extremely hacky for a non-Git tool to use such a key. I'm not sure how a user would generate such a key in the wild (e.g. [1]). Maybe it was a typo, but more worryingly (I don't have evidence for this, but it could happen), it might be misbehavior from `git [branch|config]` that we never noticed because the bogus keys have flown under the radar. If there really is a bug elsewhere, erroring out when we see such keys might also alert us to the bug. Perhaps I need to capture all of this in the commit message? [1] https://lore.kernel.org/git/24f547-6285e280-59-40303580@48243747/
