From: Phillip Wood <phillip.wood123@gmail.com>
Date: Tue, 08 Nov 2022 21:28:25 GMT
Subject: Re: [PATCH v2 2/2] maintenance: add option to register in a specific config
Message-ID: <7730f942-7332-4408-a2c0-f6ccde3cdb3a@dunelm.org.uk>
In-Reply-To: <Y2rCsq5y9TuCfrKC@nand.local>

Hi Taylor

On 08/11/2022 20:57, Taylor Blau wrote:
> 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.

Oh yes, sorry I'd missed that

Phillip

> 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