Re: [PATCH v3 2/5] config: read protected config with `git_protected_config()`

[Junio C Hamano <gitster@pobox.com>]

Glen Choo <chooglen@google.com> writes:

> A goal in this version was to introduce as little jargon as possible, so
> - "protected config" refers to the set of config sources, and
> - "protected config only" refers to config variables/settings that are
>   only read from protected config.

OK.  Let's have such a clear pair of definitions somewhere in the
doc or at least in a proposed log message.

>
>>> - Protected config is stored in `the_repository` so that we don't need
>>>   to statically allocate it. But this might be confusing since protected
>>>   config ignores repository config by definition.
>>
>> Yes, it indeed is.  Is it because we were over-eager when we
>> introduced the "struct repository *repo" parameter to many functions
>> and the configuration system wants you to have some repository, even
>> when you know you are not reading from any repository?  
>
> Ah no, I was just trying to avoid yet-another global variable (since
> IIRC we want to move towards a more lib-like Git), and the_repository
> was a convenient global variable to (ab)use.

If this does not have to be known only inside config.c, until we
introduce a more global bag of things, which may have the current
the_repository as one of its components, I do not think it hurts to
have a file-scope static there.  Then, perhaps git_configset_get*()
helper functions can recognize cs==NULL as a sign that the caller
wants to grab from the "protected config", or something?  If we do
not want to expose the underying global variable to the public, that
is.

> As an aside, I wonder how we could get rid of all of the globals in
> environment.c in the long term. Maybe we would have yet-another all
> encompassing global, the_environment, and then figure out which
> variables belong to the repository and which belong to the environment.

I think we are on the same page, we'd probably need something called
the_world ;-)

> Instead, we can use "protected config" to refer to the config and
> "protected config only" to refer to variables. Since "protected config"
> is defined as (global + system + CLI) config, then yes, we would say
> that it is "protected config". But since we do not enforce that
> "user.name" _must_ come from only protected config, it is not "protected
> config only".

Very clear.  Thanks.