Re: [PATCH] config: don't BUG when both kvi and source are set

[Glen Choo <chooglen@google.com>]

Junio C Hamano <gitster@pobox.com> 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.