Re: [PATCH v2 02/12] builtin/verify-tag: stop using `the_repository`

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 20, 2025, 15:43 UTC
Message-ID: <xmqq34g8ei5x.fsf@gitster.g>
In-Reply-To: <20250219203349.787173-3-usmanakinyemi202@gmail.com>

Usman Akinyemi <usmanakinyemi202@gmail.com> writes:

Show 15 quoted lines

> @@ -23,7 +22,7 @@ static const char * const verify_tag_usage[] = {
>  int cmd_verify_tag(int argc,
>  		   const char **argv,
>  		   const char *prefix,
> -		   struct repository *repo UNUSED)
> +		   struct repository *repo)
>  {
>  	int i = 1, verbose = 0, had_error = 0;
>  	unsigned flags = 0;
> @@ -50,13 +49,13 @@ int cmd_verify_tag(int argc,
>  		flags |= GPG_VERIFY_OMIT_STATUS;
>  	}
>  
> -	git_config(git_default_config, NULL);
> +	repo_config(repo, git_default_config, NULL);

I seriously think that it is a horrible idea (but the previous step of this series is hardly the first one that commits the same sin) to move git_config() down only to deal with "repo might be NULL if run outside a repository". We should stop making such changes, and we should revert the changes we already made along that line, to solve it differently.

Wouldn't it work much better if we teach repo_config() to allow repo to be NULL to signal that we are outside any repository, and behave the same way the current git_config() works when called outside a repository? Even though the function is called repo_config(), it is *NOT* limited to read from $GIT_DIR/config but does read from the usual "repository configuration trumps per-user configuration which trumps system-side configuration" cascade, so it is natural to skip the repository configuration when called outside any repository but read the other configuration sources, which should be what happens when git_config() is called from outside the repository, no?