| Message ID | 20221110225310.7488-1-ronan@rjp.ie (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | maintenance: improve error reporting for unregister | expand |
On Thu, Nov 10, 2022 at 03:53:10PM -0700, Ronan Pigott wrote: > Documentation/git-maintenance.txt | 2 +- > builtin/gc.c | 22 ++++++++++++++-------- > t/t7900-maintenance.sh | 6 +++++- > 3 files changed, 20 insertions(+), 10 deletions(-) Thanks, this looks pretty reasonable to me. Let's make sure Stolee has a look, too. Thanks, Taylor
On Thu, Nov 10, 2022 at 09:48:57PM -0500, Taylor Blau wrote: > On Thu, Nov 10, 2022 at 03:53:10PM -0700, Ronan Pigott wrote: > > Documentation/git-maintenance.txt | 2 +- > > builtin/gc.c | 22 ++++++++++++++-------- > > t/t7900-maintenance.sh | 6 +++++- > > 3 files changed, 20 insertions(+), 10 deletions(-) > > Thanks, this looks pretty reasonable to me. Let's make sure Stolee has a > look, too. Actually, scratch that -- I missed a couple of important things that I'll point out inline in the patch above. Sorry about that. Thanks, Taylor
On Thu, Nov 10, 2022 at 03:53:10PM -0700, Ronan Pigott wrote: > diff --git a/builtin/gc.c b/builtin/gc.c > index 1709355bce5a..56b107e7f0b9 100644 > --- a/builtin/gc.c > +++ b/builtin/gc.c > @@ -1550,19 +1550,24 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi > usage_with_options(builtin_maintenance_unregister_usage, > options); > > - if (!config_file) { > + struct config_set cs; This declaration should go at the beginning of the function. > + if (config_file) { > + git_configset_init(&cs); > + git_configset_add_file(&cs, config_file); > + list = git_configset_get_value_multi(&cs, key); And these assignments conflict with 'ab/config-multi-and-nonbool', but that is a problem for me to figure out, not you. Thanks, Taylor
diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt index eb3ae9fbd599..805e5a2e3a04 100644 --- a/Documentation/git-maintenance.txt +++ b/Documentation/git-maintenance.txt @@ -53,7 +53,7 @@ register:: Initialize Git config values so any scheduled maintenance will start running on this repository. This adds the repository to the `maintenance.repo` config variable in the current user's global config, - or the config specified by --config option, and enables some + or the config specified by --config-file option, and enables some recommended configuration values for `maintenance.<task>.schedule`. The tasks that are enabled are safe for running in the background without disrupting foreground processes. diff --git a/builtin/gc.c b/builtin/gc.c index 1709355bce5a..56b107e7f0b9 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1550,19 +1550,24 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi usage_with_options(builtin_maintenance_unregister_usage, options); - if (!config_file) { + struct config_set cs; + if (config_file) { + git_configset_init(&cs); + git_configset_add_file(&cs, config_file); + list = git_configset_get_value_multi(&cs, key); + } else { 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 (list) { + for_each_string_list_item(item, list) { + if (!strcmp(maintpath, item->string)) { + found = 1; + break; } } } - if (found || config_file) { + if (found) { int rc; char *user_config = NULL, *xdg_config = NULL; if (!config_file) { @@ -1585,6 +1590,7 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi die(_("repository '%s' is not registered"), maintpath); } + git_configset_clear(&cs); free(maintpath); return 0; } diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 091da683a8af..823331e44a03 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -517,7 +517,11 @@ test_expect_success 'register and unregister' ' test_must_fail git maintenance unregister 2>err && grep "is not registered" err && - git maintenance unregister --force + git maintenance unregister --force && + + test_must_fail git maintenance unregister --config-file ./other 2>err && + grep "is not registered" err && + git maintenance unregister --config-file ./other --force ' test_expect_success !MINGW 'register and unregister with regex metacharacters' '
Previously the unregister command would only check the standard paths to determine if the repo was registered. We should check the provided path when available instead. Signed-off-by: Ronan Pigott <ronan@rjp.ie> --- These are the changes from v3 of the --config-file patchset in patch form instead, as requested. Documentation/git-maintenance.txt | 2 +- builtin/gc.c | 22 ++++++++++++++-------- t/t7900-maintenance.sh | 6 +++++- 3 files changed, 20 insertions(+), 10 deletions(-)