From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
The 'git maintenance unregister' subcommand has a step that removes the
current repository from the multi-valued maitenance.repo config key.
This fails if the repository is not listed in that key. This makes
running 'git maintenance unregister' twice result in a failure in the
second instance.
Make this task idempotent, since the end result is the same in both
cases: maintenance will no longer run on this repository.
Signed-off-by: Derrick Stolee <derrickstolee@xxxxxxxxxx>
---
maintenance: make unregister idempotent
I noticed this while we were updating the microsoft/git fork to include
v2.38.0-rc0. I don't think 'git maintenance unregister' was idempotent
before, but instead some change in 'scalar unregister' led to it relying
on the return code of 'git maintenance unregister'. Our functional tests
expect 'scalar unregister' to be idempotent, and I think that's a good
pattern for 'git maintenance unregister', so I'm fixing it at that
layer.
Despite finding this during the 2.38.0-rc0 integration, this isn't
critical to the release.
Perhaps an argument could be made that "failure means it wasn't
registered before", but I think that isn't terribly helpful.
Our functional tests are running the unregister subcommand to disable
maintenance in order to run tests on the object store (such as running
maintenance commands in the foreground and checking the object store
afterwards). This is a form of automation using 'unregister' as a check
that maintenance will not run at the same time, and it doesn't care if
maintenance was already disabled. I can imagine other scripting
scenarios wanting that kind of guarantee.
Thanks, -Stolee
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1358%2Fderrickstolee%2Fmaintenance-unregister-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1358/derrickstolee/maintenance-unregister-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1358
builtin/gc.c | 24 +++++++++++++++++++-----
t/t7900-maintenance.sh | 5 ++++-
2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 0accc024067..787e9c702b2 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1528,9 +1528,13 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
struct option options[] = {
OPT_END(),
};
- int rc;
+ const char *key = "maintenance.repo";
+ int rc = 0;
struct child_process config_unset = CHILD_PROCESS_INIT;
char *maintpath = get_maintpath();
+ int found = 0;
+ struct string_list_item *item;
+ const struct string_list *list = git_config_get_value_multi(key);
argc = parse_options(argc, argv, prefix, options,
builtin_maintenance_unregister_usage, 0);
@@ -1538,11 +1542,21 @@ 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);
+ for_each_string_list_item(item, list) {
+ if (!strcmp(maintpath, item->string)) {
+ found = 1;
+ break;
+ }
+ }
+
+ if (found) {
+ config_unset.git_cmd = 1;
+ strvec_pushl(&config_unset.args, "config", "--global", "--unset",
+ "--fixed-value", key, maintpath, NULL);
+
+ rc = run_command(&config_unset);
+ }
- rc = run_command(&config_unset);
free(maintpath);
return rc;
}
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 2724a44fe3e..3747e4a14f8 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -493,7 +493,10 @@ 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 &&
+
+ # Expect unregister to be idempotent.
+ git maintenance unregister
'
test_expect_success !MINGW 'register and unregister with regex metacharacters' '
base-commit: d3fa443f97e3a8d75b51341e2d5bac380b7422df
--
gitgitgadget
