From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Tue, 07 Dec 2021 04:43:32 GMT
Subject: Re: [PATCH v5 4/7] repo-settings: prepare_repo_settings only in git repos
Message-ID: <211207.86r1apow9f.gmgdl@evledraar.gmail.com>
In-Reply-To: <4d8d58c473b51cac6645a7d71336e995c1f3d28b.1638566165.git.gitgitgadget@gmail.com>


On Fri, Dec 03 2021, Lessley Dennington via GitGitGadget wrote:

> From: Lessley Dennington <lessleydennington@gmail.com>
>
> Check whether git directory exists before adding any repo settings. If it
> does not exist, BUG with the message that one cannot add settings for an
> uninitialized repository. If it does exist, proceed with adding repo
> settings.
>
> Signed-off-by: Lessley Dennington <lessleydennington@gmail.com>
> ---
>  repo-settings.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/repo-settings.c b/repo-settings.c
> index b93e91a212e..00ca5571a1a 100644
> --- a/repo-settings.c
> +++ b/repo-settings.c
> @@ -17,6 +17,9 @@ void prepare_repo_settings(struct repository *r)
>  	char *strval;
>  	int manyfiles;
>  
> +	if (!r->gitdir)
> +		BUG("Cannot add settings for uninitialized repository");

nit: start BUG(), error() etc. messages with lower-case.

> +
>  	if (r->settings.initialized++)
>  		return;

Our config doesn't require us to have a repo, and most of what
prepare_repo_settings() is doing is reading global config.

I think that *currently* this won't break things, but e.g. if we ever
want to have "feature.experimental" or whatever change the behavior of a
a command that doesn't require a repository we'd need to untangle this
(currently everything it changes requires a repo AFAICT).

Perhaps this is fine, and if we ever need such a "global config" point
we should stick it closer to common-main.c...