git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Nov 8, 2022, 20:38 UTC
Message-ID
<448cc6ed-c441-85a3-2780-0c07e56f53f8@dunelm.org.uk>
In-Reply-To
<20221108194930.25805-3-ronan@rjp.ie>
Hi Ronan

I think both these patches are useful improvements, thanks for working on them. I've left one comment below

On 08/11/2022 19:49, Ronan Pigott wrote:
Show 17 quoted lines
> 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
Best Wishes
Phillip
Show 93 quoted lines
> +		}
>   		rc = git_config_set_multivar_in_file_gently(
> -			user_config, "maintenance.repo", maintpath,
> +			config_file, "maintenance.repo", maintpath,
>   			CONFIG_REGEX_NONE, 0);
>   		free(user_config);
>   		free(xdg_config);
> @@ -1517,14 +1523,16 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)
>   }
>   
>   static char const * const builtin_maintenance_unregister_usage[] = {
> -	"git maintenance unregister [--force]",
> +	"git maintenance unregister [--config-file <path>] [--force]",
>   	NULL
>   };
>   
>   static int maintenance_unregister(int argc, const char **argv, const char *prefix)
>   {
>   	int force = 0;
> +	char *config_file = NULL;
>   	struct option options[] = {
> +		OPT_STRING(0, "config-file", &config_file, N_("file"), N_("use given config file")),
>   		OPT__FORCE(&force,
>   			   N_("return success even if repository was not registered"),
>   			   PARSE_OPT_NOCOMPLETE),
> @@ -1542,24 +1550,29 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
>   		usage_with_options(builtin_maintenance_unregister_usage,
>   				   options);
>   
> -	list = git_config_get_value_multi(key);
> -	if (list) {
> -		for_each_string_list_item(item, list) {
> -			if (!strcmp(maintpath, item->string)) {
> -				found = 1;
> -				break;
> +	if (!config_file) {
> +		list = git_config_get_value_multi(key);
> +		if (list) {
> +			for_each_string_list_item(item, list) {
> +				if (!strcmp(maintpath, item->string)) {
> +					found = 1;
> +					break;
> +				}
>   			}
>   		}
>   	}
>   
> -	if (found) {
> +	if (found || config_file) {
>   		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;
> +			if (!user_config)
> +				die(_("$HOME not set"));
> +		}
>   		rc = git_config_set_multivar_in_file_gently(
> -			user_config, key, NULL, maintpath,
> +			config_file, key, NULL, maintpath,
>   			CONFIG_FLAGS_MULTI_REPLACE | CONFIG_FLAGS_FIXED_VALUE);
>   		free(user_config);
>   		free(xdg_config);
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index 96bdd42045..091da683a8 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -500,6 +500,21 @@ test_expect_success 'register and unregister' '
>   	git config --global --get-all maintenance.repo >actual &&
>   	test_cmp before actual &&
>   
> +	git config --file ./other --add maintenance.repo /existing1 &&
> +	git config --file ./other --add maintenance.repo /existing2 &&
> +	git config --file ./other --get-all maintenance.repo >before &&
> +
> +	git maintenance register --config-file ./other &&
> +	test_cmp_config false maintenance.auto &&
> +	git config --file ./other --get-all maintenance.repo >between &&
> +	cp before expect &&
> +	pwd >>expect &&
> +	test_cmp expect between &&
> +
> +	git maintenance unregister --config-file ./other &&
> +	git config --file ./other --get-all maintenance.repo >actual &&
> +	test_cmp before actual &&
> +
>   	test_must_fail git maintenance unregister 2>err &&
>   	grep "is not registered" err &&
>   	git maintenance unregister --force
Previous: Ronan PigottNext: Taylor Blau
Message 4 of 10 in “git-maintenance quality-of-life improvements”
  1. 0/2 git-maintenance quality-of-life improvementsRonan Pigott, Nov 8, 2022
  2. 1/2 for-each-repo: interpolate repo path argumentsRonan Pigott, Nov 8, 2022
  3. 2/2 maintenance: add option to register in a specific configRonan Pigott, Nov 8, 2022
  4. Phillip WoodNov 8, 2022
  5. Taylor BlauNov 8, 2022
  6. Phillip WoodNov 8, 2022
  7. ronan@rjp.ieNov 8, 2022
  8. Derrick StoleeNov 9, 2022
  9. Taylor BlauNov 10, 2022
  10. Taylor BlauNov 8, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.