{"thread":{"id":"58791","subject":"[PATCH v2] maintenance: improve error reporting for unregister","startedAt":"2022-11-11T23:19:46Z","lastAt":"2022-11-14T22:54:02Z","messageCount":3,"participants":["Ronan Pigott","Derrick Stolee","Taylor Blau"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"467175","messageId":"20221111231910.26769-1-ronan@rjp.ie","threadId":"58791","inReplyTo":null,"subject":"[PATCH v2] maintenance: improve error reporting for unregister","fromName":"Ronan Pigott","fromEmail":"ronan@rjp.ie","sentAt":"2022-11-11T23:19:10Z","receivedAt":"2022-11-11T23:19:46Z","isPatch":true,"sender":{"key":"ronan@rjp.ie","avatar":"https://avatars.githubusercontent.com/u/12577529?v=4"},"body":"Previously the unregister command would only check the standard paths to\ndetermine if the repo was registered. We should check the provided path\nwhen available instead.\n\nSigned-off-by: Ronan Pigott \n---\nChanges in v2:\n - Moved configset declaration with the other declarations\n - Extract configset initialization from conditional so it is always\n initialized before it is cleared. It is still deferred until after\n the command arguments are sanity checked.\n\n Documentation/git-maintenance.txt | 2 +-\n builtin/gc.c | 22 ++++++++++++++--------\n t/t7900-maintenance.sh | 6 +++++-\n 3 files changed, 20 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt\nindex eb3ae9fbd599..805e5a2e3a04 100644\n--- a/Documentation/git-maintenance.txt\n+++ b/Documentation/git-maintenance.txt\n@@ -53,7 +53,7 @@ register::\n \tInitialize Git config values so any scheduled maintenance will start\n \trunning on this repository. This adds the repository to the\n \t`maintenance.repo` config variable in the current user's global config,\n-\tor the config specified by --config option, and enables some\n+\tor the config specified by --config-file option, and enables some\n \trecommended configuration values for `maintenance..schedule`. The\n \ttasks that are enabled are safe for running in the background without\n \tdisrupting foreground processes.\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 1709355bce5a..c32a5f28cca9 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1543,6 +1543,7 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n \tint found = 0;\n \tstruct string_list_item *item;\n \tconst struct string_list *list;\n+\tstruct config_set cs;\n \n \targc = parse_options(argc, argv, prefix, options,\n \t\t\t builtin_maintenance_unregister_usage, 0);\n@@ -1550,19 +1551,23 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n \t\tusage_with_options(builtin_maintenance_unregister_usage,\n \t\t\t\t options);\n \n-\tif (!config_file) {\n+\tgit_configset_init(&cs);\n+\tif (config_file) {\n+\t\tgit_configset_add_file(&cs, config_file);\n+\t\tlist = git_configset_get_value_multi(&cs, key);\n+\t} else {\n \t\tlist = git_config_get_value_multi(key);\n-\t\tif (list) {\n-\t\t\tfor_each_string_list_item(item, list) {\n-\t\t\t\tif (!strcmp(maintpath, item->string)) {\n-\t\t\t\t\tfound = 1;\n-\t\t\t\t\tbreak;\n-\t\t\t\t}\n+\t}\n+\tif (list) {\n+\t\tfor_each_string_list_item(item, list) {\n+\t\t\tif (!strcmp(maintpath, item->string)) {\n+\t\t\t\tfound = 1;\n+\t\t\t\tbreak;\n \t\t\t}\n \t\t}\n \t}\n \n-\tif (found || config_file) {\n+\tif (found) {\n \t\tint rc;\n \t\tchar *user_config = NULL, *xdg_config = NULL;\n \t\tif (!config_file) {\n@@ -1585,6 +1590,7 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n \t\tdie(_(\"repository '%s' is not registered\"), maintpath);\n \t}\n \n+\tgit_configset_clear(&cs);\n \tfree(maintpath);\n \treturn 0;\n }\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 091da683a8af..823331e44a03 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -517,7 +517,11 @@ test_expect_success 'register and unregister' '\n \n \ttest_must_fail git maintenance unregister 2>err &&\n \tgrep \"is not registered\" err &&\n-\tgit maintenance unregister --force\n+\tgit maintenance unregister --force &&\n+\n+\ttest_must_fail git maintenance unregister --config-file ./other 2>err &&\n+\tgrep \"is not registered\" err &&\n+\tgit maintenance unregister --config-file ./other --force\n '\n \n test_expect_success !MINGW 'register and unregister with regex metacharacters' '\n-- \n2.38.1\n\n"},{"id":"467253","messageId":"2c78ee6b-0ee1-8955-a692-384a238d0f64@github.com","threadId":"58791","inReplyTo":"20221111231910.26769-1-ronan@rjp.ie","subject":"Re: [PATCH v2] maintenance: improve error reporting for unregister","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-11-14T15:04:38Z","receivedAt":"2022-11-14T15:04:46Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/11/2022 6:19 PM, Ronan Pigott wrote:\n> Previously the unregister command would only check the standard paths to\n> determine if the repo was registered. We should check the provided path\n> when available instead.\n\nThank you for following up on these items. I appreciate your diligence.\n\nLGTM.\n-Stolee\n"},{"id":"467283","messageId":"Y3LHBL3PpwufjjFx@nand.local","threadId":"58791","inReplyTo":"2c78ee6b-0ee1-8955-a692-384a238d0f64@github.com","subject":"Re: [PATCH v2] maintenance: improve error reporting for unregister","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-14T22:53:56Z","receivedAt":"2022-11-14T22:54:02Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Nov 14, 2022 at 10:04:38AM -0500, Derrick Stolee wrote:\n> On 11/11/2022 6:19 PM, Ronan Pigott wrote:\n> > Previously the unregister command would only check the standard paths to\n> > determine if the repo was registered. We should check the provided path\n> > when available instead.\n>\n> Thank you for following up on these items. I appreciate your diligence.\n\nThanks, both. Will queue.\n\nThanks,\nTaylor\n"}]}