Junio C Hamano <gitster@xxxxxxxxx> writes: >> Therefore, fix the bug by removing the BUG() check. We're reverting to >> an older, less safe state, but that's generally okay since >> key_value_info is always preferentially read, so we'd always read the >> correct values when we iterate a config set in the middle of a config >> parse (like we are here). > > I wonder if the source being pushed and config_kvi value at this > point have some particular relationship (like "if kvi exists, the > source must match kvi's source" or something) that we can cheaply > use to avoid "reverting to an older less safe state"? Not at all. In this case, the source should reflect .gitmodules, but the config_kvi should reflect the promisor config (aka the full repo config). config_source implements stack semantics, so we could co-opt it by e.g. converting config_kvi into a fake config_source and pushing it onto the stack (at which point, we could just get rid of config_kvi altogether too), but that's really way too much work for something that will _hopefully_ go away soon. >> The reverse would be wrong, but extremely >> unlikely to happen since very few callers parse config without going >> through a config set. > > Sorry, but I do not quite get this comment. Ah, I meant that this bug occurred because most users of config use git_config()/repo_config() (a wrapper around config sets), so it's very easy to accidentally read repo config, e.g. in the middle of parsing config (config file -> config set). I'd imagine it might also be quite easy to read repo config while reading repo config (config set -> config set), which would make current_config_* return the wrong thing, but at least it doesn't BUG(). The "reverse" case (config set -> config file) is very _unlikely_ because very few places need to know about config files, so it's unlikely that we'd have an explicit call to parse a config file, especially in the middle of reading repo config.
