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

From: Junio C Hamano <[email-masked]>
Date: Fri, 27 May 2022 17:28:14 -0700

"Glen Choo via GitGitGadget" <[email-masked]> writes:

> From: Glen Choo <[email-masked]>
>
> Protected config is read using `read_very_early_config()`, which has
> several downsides:
>
> - Every call to `read_very_early_config()` parses global and
>   system-level config files anew, but this can be optimized by just
>   parsing them once [1].
> - Protected variables should respect "-c" because we can reasonably
>   assume that it comes from the user. But, `read_very_early_config()`
>   can't use "-c" because it is called so early that it does not have
>   access to command line arguments.

Now we are talking about protected "variable".  Is that a synonym
for "config", or are there some distinctions between them?

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

I am wondering if it is a cleaner solution *not* to hang the
protected config as a configset in the_repository, but keep the
configset as a separate global variable, perhaps static to config.c
and is meant to be only accessed via git_protected_config() and the
like.

> @@ -295,6 +295,11 @@ void repo_clear(struct repository *repo)
>  		FREE_AND_NULL(repo->remote_state);
>  	}
>  
> +	if (repo->protected_config) {
> +		git_configset_clear(repo->protected_config);
> +		FREE_AND_NULL(repo->protected_config);
> +	}
> +

This becomes necessary only because each repository instance has
protected_config, even though we need only one instance, no matter
how many repositories we are accessing in this single invocation of
Git, no?

How should "git config -l" interact with "protected config" and
"protected variables", by the way?  Should a user be able to tell
which ones are coming from protected scope?  Should we gain, next to
--global, --system, etc., --protected option to list only the
protected config/variable?

This is another thing that I find iffy on terminology.  Should a
random variable, like user.name, be a "protected config", if it is
found in $HOME/.gitconfig?  If it comes from there, surely we can
trust its value, but unlike things like safe.directory, there is no
code that wants to enforce that we pay attention only to user.name
that came from trusted scopes.  Should such a variable be called
"protected variable"?

Thanks.