{"thread":{"id":"58785","subject":"[PATCH] maintenance: improve error reporting for unregister","startedAt":"2022-11-10T22:53:34Z","lastAt":"2022-11-11T22:46:05Z","messageCount":4,"participants":["Ronan Pigott","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"467095","messageId":"20221110225310.7488-1-ronan@rjp.ie","threadId":"58785","inReplyTo":null,"subject":"[PATCH] maintenance: improve error reporting for unregister","fromName":"Ronan Pigott","fromEmail":"ronan@rjp.ie","sentAt":"2022-11-10T22:53:10Z","receivedAt":"2022-11-10T22:53:34Z","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 <ronan@rjp.ie>\n---\n\nThese are the changes from v3 of the --config-file patchset in patch\nform instead, as requested.\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.<task>.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..56b107e7f0b9 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1550,19 +1550,24 @@ 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+\tstruct config_set cs;\n+\tif (config_file) {\n+\t\tgit_configset_init(&cs);\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":"467116","messageId":"Y224GSbupvWD8mu/@nand.local","threadId":"58785","inReplyTo":"20221110225310.7488-1-ronan@rjp.ie","subject":"Re: [PATCH] maintenance: improve error reporting for unregister","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-11T02:48:57Z","receivedAt":"2022-11-11T02:50:28Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Nov 10, 2022 at 03:53:10PM -0700, Ronan Pigott wrote:\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\nThanks, this looks pretty reasonable to me. Let's make sure Stolee has a\nlook, too.\n\nThanks,\nTaylor\n"},{"id":"467173","messageId":"Y27QJNZWF41Kp04W@nand.local","threadId":"58785","inReplyTo":"Y224GSbupvWD8mu/@nand.local","subject":"Re: [PATCH] maintenance: improve error reporting for unregister","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-11T22:43:48Z","receivedAt":"2022-11-11T22:43:54Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Nov 10, 2022 at 09:48:57PM -0500, Taylor Blau wrote:\n> On Thu, Nov 10, 2022 at 03:53:10PM -0700, Ronan Pigott wrote:\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>\n> Thanks, this looks pretty reasonable to me. Let's make sure Stolee has a\n> look, too.\n\nActually, scratch that -- I missed a couple of important things that\nI'll point out inline in the patch above. Sorry about that.\n\n\nThanks,\nTaylor\n"},{"id":"467174","messageId":"Y27Qpz7jWafy3FCI@nand.local","threadId":"58785","inReplyTo":"20221110225310.7488-1-ronan@rjp.ie","subject":"Re: [PATCH] maintenance: improve error reporting for unregister","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-11T22:45:59Z","receivedAt":"2022-11-11T22:46:05Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Nov 10, 2022 at 03:53:10PM -0700, Ronan Pigott wrote:\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 1709355bce5a..56b107e7f0b9 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -1550,19 +1550,24 @@ 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> +\tstruct config_set cs;\n\nThis declaration should go at the beginning of the function.\n\n> +\tif (config_file) {\n> +\t\tgit_configset_init(&cs);\n> +\t\tgit_configset_add_file(&cs, config_file);\n> +\t\tlist = git_configset_get_value_multi(&cs, key);\n\nAnd these assignments conflict with 'ab/config-multi-and-nonbool', but\nthat is a problem for me to figure out, not you.\n\nThanks,\nTaylor\n"}]}