From 26c7974376fad730ba5bc1926afe7d26ba8d91e0 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 23 Feb 2021 02:31:07 -0500 Subject: [PATCH] maintenance: fix incorrect `maintenance.repo` path with bare repository The periodic maintenance tasks configured by `git maintenance start` invoke `git for-each-repo` to run `git maintenance run` on each path specified by the multi-value global configuration variable `maintenance.repo`. Because `git for-each-repo` will likely be run outside of the repositories which require periodic maintenance, it is mandatory that the repository paths specified by `maintenance.repo` are absolute. Unfortunately, however, `git maintenance register` does nothing to ensure that the paths it assigns to `maintenance.repo` are indeed absolute, and may in fact -- especially in the case of a bare repository -- assign a relative path to `maintenance.repo` instead. Fix this problem by converting all paths to absolute before assigning them to `maintenance.repo`. While at it, also fix `git maintenance unregister` to convert paths to absolute, as well, in order to ensure that it can correctly remove from `maintenance.repo` a path assigned via `git maintenance register`. Reported-by: Clement Moyroud Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/gc.c | 50 ++++++++++++++++++++++++++++-------------- t/t7900-maintenance.sh | 13 +++++++++++ 2 files changed, 46 insertions(+), 17 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 6db9cb39e6..ef7226d7bc 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1461,11 +1461,23 @@ static int maintenance_run(int argc, const char **argv, const char *prefix) return maintenance_run_tasks(&opts); } +static char *get_maintpath(void) +{ + struct strbuf sb = STRBUF_INIT; + const char *p = the_repository->worktree ? + the_repository->worktree : the_repository->gitdir; + + strbuf_realpath(&sb, p, 1); + return strbuf_detach(&sb, NULL); +} + static int maintenance_register(void) { + int rc; char *config_value; struct child_process config_set = CHILD_PROCESS_INIT; struct child_process config_get = CHILD_PROCESS_INIT; + char *maintpath = get_maintpath(); /* Disable foreground maintenance */ git_config_set("maintenance.auto", "false"); @@ -1478,40 +1490,44 @@ static int maintenance_register(void) config_get.git_cmd = 1; strvec_pushl(&config_get.args, "config", "--global", "--get", - "--fixed-value", "maintenance.repo", - the_repository->worktree ? the_repository->worktree - : the_repository->gitdir, - NULL); + "--fixed-value", "maintenance.repo", maintpath, NULL); config_get.out = -1; - if (start_command(&config_get)) - return error(_("failed to run 'git config'")); + if (start_command(&config_get)) { + rc = error(_("failed to run 'git config'")); + goto done; + } /* We already have this value in our config! */ - if (!finish_command(&config_get)) - return 0; + if (!finish_command(&config_get)) { + rc = 0; + goto done; + } config_set.git_cmd = 1; strvec_pushl(&config_set.args, "config", "--add", "--global", "maintenance.repo", - the_repository->worktree ? the_repository->worktree - : the_repository->gitdir, - NULL); + maintpath, NULL); - return run_command(&config_set); + rc = run_command(&config_set); + +done: + free(maintpath); + return rc; } static int maintenance_unregister(void) { + int rc; struct child_process config_unset = CHILD_PROCESS_INIT; + char *maintpath = get_maintpath(); config_unset.git_cmd = 1; strvec_pushl(&config_unset.args, "config", "--global", "--unset", - "--fixed-value", "maintenance.repo", - the_repository->worktree ? the_repository->worktree - : the_repository->gitdir, - NULL); + "--fixed-value", "maintenance.repo", maintpath, NULL); - return run_command(&config_unset); + rc = run_command(&config_unset); + free(maintpath); + return rc; } static const char *get_frequency(enum schedule_priority schedule) diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 286b18db3c..2412d8c5c0 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -632,4 +632,17 @@ test_expect_success 'fails when running outside of a repository' ' nongit test_must_fail git maintenance unregister ' +test_expect_success 'register and unregister bare repo' ' + test_when_finished "git config --global --unset-all maintenance.repo || :" && + test_might_fail git config --global --unset-all maintenance.repo && + git init --bare barerepo && + ( + cd barerepo && + git maintenance register && + git config --get --global --fixed-value maintenance.repo "$(pwd)" && + git maintenance unregister && + test_must_fail git config --global --get-all maintenance.repo + ) +' + test_done