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

From: Junio C Hamano <[email-masked]>
Date: Thu, 20 Feb 2025 07:43:06 -0800

Usman Akinyemi <[email-masked]> writes:

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