Subject: Re: [PATCH v3 1/8] config: teach repo_config to allow `repo` to be NULL

From: Junio C Hamano <[email-masked]>
Date: Thu, 06 Mar 2025 09:53:16 -0800

Usman Akinyemi <[email-masked]> writes:

>  void repo_config(struct repository *repo, config_fn_t fn, void *data)
>  {
> +	if (!repo) {
> +		read_very_early_config(fn, data);
> +		return;
> +	}
>  	git_config_check_init(repo);
>  	configset_iter(repo->config, fn, data);
>  }
> diff --git a/config.h b/config.h
> index 5c730c4f89..1e5b22dfc4 100644
> --- a/config.h
> +++ b/config.h
> @@ -219,6 +219,9 @@ void read_very_early_config(config_fn_t cb, void *data);
>   * repo-specific one; by overwriting, the higher-priority repo-specific
>   * value is left at the end).
>   *
> + * In cases where the repository variable is NULL, repo_config() will
> + * call read_early_config().
> + *

early or very early?

I am wondering if we should describe the effect we want out of the
design more prominently than the way we try to obtain the effect
here.  In other words, instead of (rather, in addition to) saying
that we call helper X, wouldn't it be more helpful to future
developers why we call X, to convey the intent, so that they know
how to adjust when for example what X does change or X even
disappears?  E.g., 

	When repo==NULL, skip reading the per-repository
	configuration file but still use the system- and globa-
	configuration, by calling X.  Note that this ignores
	one-time configuration override "git -c var=val" given from
	the command line.  The only use case the feature to allow
	passing repo==NULL was designed for is to support handling
	"git foo -h" (which lets git.c:run_builtin() to pass NULL
	and have the cmd_foo() call repo_config() before calling
	parse_options() to notice "-h", give help and exit) for a
	command that ordinarily require a repository, so this
	limitation may be OK (but if needed you are welcome to fix
	it).

That way, folks who are planning to update read_veriy_early_config()
so that it pays attention to the "git -c var=val" in the future will
be rest assured that they won't be breaking this caller with their
planned change.

Of course I didn't spend enough brainpower to make the above comment
more concise and to the point, which the final version should be,
but hopefully you got the idea.

Thanks.