Re: [PATCH v2 2/2] maintenance: add option to register in a specific config

[Taylor Blau <me@ttaylorr.com>]

On Tue, Nov 08, 2022 at 08:38:51PM +0000, Phillip Wood wrote:
> > diff --git a/builtin/gc.c b/builtin/gc.c
> > index 24ea85c7af..1709355bce 100644
> > --- a/builtin/gc.c
> > +++ b/builtin/gc.c
> > @@ -1497,12 +1499,16 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)
> >   	if (!found) {
> >   		int rc;
> > -		char *user_config, *xdg_config;
> > -		git_global_config(&user_config, &xdg_config);
> > -		if (!user_config)
> > -			die(_("$HOME not set"));
> > +		char *user_config = NULL, *xdg_config = NULL;
> > +
> > +		if (!config_file) {
> > +			git_global_config(&user_config, &xdg_config);
> > +			config_file = user_config;
>
> Here we need to decide whether to use user_config or xdg_config as the
> config file. In builtin/config.c we do this with
>
> 		if (access_or_warn(user_config, R_OK, 0) &&
> 		    xdg_config && !access_or_warn(xdg_config, R_OK, 0)) {
> 			given_config_source.file = xdg_config;
> 			free(user_config);
> 		} else {
> 			given_config_source.file = user_config;
> 			free(xdg_config);
> 		}
>
> We need something similar here (maybe we should create a helper function to
> find the appropriate file)
>
> > +			if (!user_config)
> > +				die(_("$HOME not set"));
>
> This check needs to come before deciding which config file to use

True, but that problem existed before this series, too. So the new
behavior is no worse with respect to the XDG config stuff, and any
improvements to that behavior can be done independently on top.

Thanks,
Taylor