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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 1, 2022, 15:58 UTC
Message-ID: <xmqqwne0tlxe.fsf@gitster.g>
In-Reply-To: <kl6lv8tlegy4.fsf@chooglen-macbookpro.roam.corp.google.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.

Show 13 quoted lines

>
>>> - 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 ;-)

Show 6 quoted lines

> 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.