diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt index 9c630efe19..bb888690e4 100644 --- a/Documentation/git-maintenance.txt +++ b/Documentation/git-maintenance.txt @@ -11,7 +11,7 @@ SYNOPSIS [verse] 'git maintenance' run [] 'git maintenance' start [--scheduler=] -'git maintenance' (stop|register|unregister) +'git maintenance' (stop|register|unregister) [] DESCRIPTION @@ -79,6 +79,10 @@ unregister:: Remove the current repository from background maintenance. This only removes the repository from the configured list. It does not stop the background maintenance processes from running. ++ +The `unregister` subcommand will report an error if the current repository +is not already registered. Use the `--force` option to return success even +when the current repository is not registered. TASKS ----- diff --git a/builtin/gc.c b/builtin/gc.c index 2753bd15a5..7a585f0b71 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1470,11 +1470,12 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) struct option options[] = { OPT_END(), }; - int rc; + int found = 0; + const char *key = "maintenance.repo"; char *config_value; - struct child_process config_set = CHILD_PROCESS_INIT; - struct child_process config_get = CHILD_PROCESS_INIT; char *maintpath = get_maintpath(); + struct string_list_item *item; + const struct string_list *list; argc = parse_options(argc, argv, prefix, options, builtin_maintenance_register_usage, 0); @@ -1491,46 +1492,56 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) else git_config_set("maintenance.strategy", "incremental"); - config_get.git_cmd = 1; - strvec_pushl(&config_get.args, "config", "--global", "--get", - "--fixed-value", "maintenance.repo", maintpath, NULL); - config_get.out = -1; - - if (start_command(&config_get)) { - rc = error(_("failed to run 'git config'")); - goto done; + list = git_config_get_value_multi(key); + if (list) { + for_each_string_list_item(item, list) { + if (!strcmp(maintpath, item->string)) { + found = 1; + break; + } + } } - /* We already have this value in our config! */ - if (!finish_command(&config_get)) { - rc = 0; - goto done; + if (!found) { + int rc; + char *user_config, *xdg_config; + git_global_config(&user_config, &xdg_config); + if (!user_config) + die(_("$HOME not set")); + rc = git_config_set_multivar_in_file_gently( + user_config, "maintenance.repo", maintpath, + CONFIG_REGEX_NONE, 0); + free(user_config); + free(xdg_config); + + if (rc) + die(_("unable to add '%s' value of '%s'"), + key, maintpath); } - config_set.git_cmd = 1; - strvec_pushl(&config_set.args, "config", "--add", "--global", "maintenance.repo", - maintpath, NULL); - - rc = run_command(&config_set); - -done: free(maintpath); - return rc; + return 0; } static char const * const builtin_maintenance_unregister_usage[] = { - "git maintenance unregister", + "git maintenance unregister [--force]", NULL }; static int maintenance_unregister(int argc, const char **argv, const char *prefix) { + int force = 0; struct option options[] = { + OPT__FORCE(&force, + N_("return success even if repository was not registered"), + PARSE_OPT_NOCOMPLETE), OPT_END(), }; - int rc; - struct child_process config_unset = CHILD_PROCESS_INIT; + const char *key = "maintenance.repo"; char *maintpath = get_maintpath(); + int found = 0; + struct string_list_item *item; + const struct string_list *list; argc = parse_options(argc, argv, prefix, options, builtin_maintenance_unregister_usage, 0); @@ -1538,13 +1549,38 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi usage_with_options(builtin_maintenance_unregister_usage, options); - config_unset.git_cmd = 1; - strvec_pushl(&config_unset.args, "config", "--global", "--unset", - "--fixed-value", "maintenance.repo", maintpath, NULL); + 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) { + int rc; + char *user_config, *xdg_config; + git_global_config(&user_config, &xdg_config); + if (!user_config) + die(_("$HOME not set")); + rc = git_config_set_multivar_in_file_gently( + user_config, key, NULL, maintpath, + CONFIG_FLAGS_MULTI_REPLACE | CONFIG_FLAGS_FIXED_VALUE); + free(user_config); + free(xdg_config); + + if (rc && + (!force || rc == CONFIG_NOTHING_SET)) + die(_("unable to unset '%s' value of '%s'"), + key, maintpath); + } else if (!force) { + die(_("repository '%s' is not registered"), maintpath); + } - rc = run_command(&config_unset); free(maintpath); - return rc; + return 0; } static const char *get_frequency(enum schedule_priority schedule) diff --git a/scalar.c b/scalar.c index c5c1ce6891..6de9c0ee52 100644 --- a/scalar.c +++ b/scalar.c @@ -207,7 +207,10 @@ static int set_recommended_config(int reconfigure) static int toggle_maintenance(int enable) { - return run_git("maintenance", enable ? "start" : "unregister", NULL); + return run_git("maintenance", + enable ? "start" : "unregister", + enable ? NULL : "--force", + NULL); } static int add_or_remove_enlistment(int add) diff --git a/string-list.h b/string-list.h index d5a744e143..c7b0d5d000 100644 --- a/string-list.h +++ b/string-list.h @@ -141,7 +141,12 @@ void string_list_clear_func(struct string_list *list, string_list_clear_func_t c int for_each_string_list(struct string_list *list, string_list_each_func_t func, void *cb_data); -/** Iterate over each item, as a macro. */ +/** + * Iterate over each item, as a macro. + * + * Be sure that 'list' is non-NULL. The macro cannot perform NULL + * checks due to -Werror=address errors. + */ #define for_each_string_list_item(item,list) \ for (item = (list)->items; \ item && item < (list)->items + (list)->nr; \ diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 2724a44fe3..96bdd42045 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -480,6 +480,11 @@ test_expect_success 'maintenance.strategy inheritance' ' test_expect_success 'register and unregister' ' test_when_finished git config --global --unset-all maintenance.repo && + + test_must_fail git maintenance unregister 2>err && + grep "is not registered" err && + git maintenance unregister --force && + git config --global --add maintenance.repo /existing1 && git config --global --add maintenance.repo /existing2 && git config --global --get-all maintenance.repo >before && @@ -493,7 +498,11 @@ test_expect_success 'register and unregister' ' git maintenance unregister && git config --global --get-all maintenance.repo >actual && - test_cmp before actual + test_cmp before actual && + + test_must_fail git maintenance unregister 2>err && + grep "is not registered" err && + git maintenance unregister --force ' test_expect_success !MINGW 'register and unregister with regex metacharacters' ' diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh index 14ca575a21..be51a8bb7a 100755 --- a/t/t9210-scalar.sh +++ b/t/t9210-scalar.sh @@ -116,7 +116,10 @@ test_expect_success 'scalar unregister' ' test_must_fail git config --get --global --fixed-value \ maintenance.repo "$(pwd)/vanish/src" && scalar list >scalar.repos && - ! grep -F "$(pwd)/vanish/src" scalar.repos + ! grep -F "$(pwd)/vanish/src" scalar.repos && + + # scalar unregister should be idempotent + scalar unregister vanish ' test_expect_success 'set up repository to clone' '