{"thread":{"id":"63382","subject":"[PATCH v3 1/7] builtin/gc: fix indentation of `cmd_gc()` parameters","startedAt":"2025-05-02T08:44:09Z","lastAt":"2025-05-07T07:06:47Z","messageCount":15,"participants":["Patrick Steinhardt","Derrick Stolee","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":3,"patchTotal":7},"messages":[{"id":"517091","messageId":"20250502-pks-maintenance-missing-tasks-v3-0-13e130d36640@pks.im","threadId":"63382","inReplyTo":"20250425-pks-maintenance-missing-tasks-v1-0-972ed6ab2c0d@pks.im","subject":"[PATCH v3 0/7] builtin/maintenance: implement missing tasks compared to git-gc(1)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-02T08:43:57Z","receivedAt":"2025-05-02T08:44:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis small patch series implements the last couple of remaining tasks\nthat are missing compared to the functionality git-gc(1) provides.\n\nRight now, git-maintenance(1) still executes git-gc(1). With these last\ngaps plugged though we can in theory fully replace git-gc(1) with finer\ngrained tasks without losing any functionality. The benefit is that it\nbecomes possible for users to have finer-grained control over what\nexactly the maintenance does.\n\nThis patch series doesn't do that yet, but only implements whatever is\nneeded to get there.\n\nChanges in v2:\n  - Introduce \"maintenance.worktree-prune.auto\", which controls how many\n    stale worktrees need to exist before executing `git worktree prune`.\n  - Introduce \"maintenance.rerere-gc.auto\", which controls how many\n    stale rerere entries need to exist before executing `git rerere gc`.\n  - Add tests to verify that \"gc.worktreePruneExpire\" works.\n  - Remove some fragile test logic by introducing functions that check\n    for a given maintenance subprocess.\n  - Link to v1: https://lore.kernel.org/r/20250425-pks-maintenance-missing-tasks-v1-0-972ed6ab2c0d@pks.im\n\nChanges in v3:\n  - Simplify the heuristic for \"rerere-gc\" so that we only count the\n    number of directory entries in \".git/rr-cache\", without considering\n    staleness.\n  - Link to v2: https://lore.kernel.org/r/20250430-pks-maintenance-missing-tasks-v2-0-2580b7b8ca3a@pks.im\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (7):\n      builtin/gc: fix indentation of `cmd_gc()` parameters\n      builtin/gc: remove global variables where it trivial to do\n      builtin/gc: move pruning of worktrees into a separate function\n      worktree: expose function to retrieve worktree names\n      builtin/maintenance: introduce \"worktree-prune\" task\n      builtin/gc: move rerere garbage collection into separate function\n      builtin/maintenance: introduce \"rerere-gc\" task\n\n Documentation/config/maintenance.adoc |  16 ++++\n Documentation/git-maintenance.adoc    |   8 ++\n builtin/gc.c                          | 157 +++++++++++++++++++++++++++-------\n builtin/worktree.c                    |  25 +++---\n t/t7900-maintenance.sh                | 114 ++++++++++++++++++++++++\n worktree.c                            |  30 +++++++\n worktree.h                            |   8 ++\n 7 files changed, 314 insertions(+), 44 deletions(-)\n\nRange-diff versus v2:\n\n1:  fe67cebba69 = 1:  ac3cd73438b builtin/gc: fix indentation of `cmd_gc()` parameters\n2:  623c53de24c = 2:  662dee455b6 builtin/gc: remove global variables where it trivial to do\n3:  7c6b33075ca = 3:  522d40d8da7 builtin/gc: move pruning of worktrees into a separate function\n4:  423963c589e = 4:  737df0a7dcc worktree: expose function to retrieve worktree names\n5:  a031d9aa9fc = 5:  8a24fb6547b builtin/maintenance: introduce \"worktree-prune\" task\n6:  6610b72a0f5 < -:  ----------- rerere: provide function to collect stale entries\n7:  4e6c461e0b6 = 6:  3ce62afb54f builtin/gc: move rerere garbage collection into separate function\n8:  4763c2aa09e ! 7:  90e86a52fcc builtin/maintenance: introduce \"rerere-gc\" task\n    @@ Documentation/config/maintenance.adoc: maintenance.reflog-expire.auto::\n     +\tshould be run as part of `git maintenance run --auto`. If zero, then\n     +\tthe `rerere-gc` task will not run with the `--auto` option. A negative\n     +\tvalue will force the task to run every time. Otherwise, a positive\n    -+\tvalue implies the command should run when the number of prunable rerere\n    -+\tentries exceeds the value. The default value is 20.\n    ++\tvalue implies the command should run when there are at least this many\n    ++\tdirectory entries in the \"rr-cache\" directory. The default value is 1.\n     +\n      maintenance.worktree-prune.auto::\n      \tThis integer config option controls how often the `worktree-prune` task\n    @@ builtin/gc.c: static int maintenance_task_rerere_gc(struct maintenance_run_opts\n     +static int rerere_gc_condition(struct gc_config *cfg UNUSED)\n     +{\n     +\tstruct strbuf path = STRBUF_INIT;\n    -+\tstruct string_list prunable_dirs = STRING_LIST_INIT_DUP;\n    -+\tstruct rerere_id *prunable_entries = NULL;\n    -+\tsize_t prunable_entries_nr;\n    -+\tint should_gc = 0;\n    -+\tint limit = 20;\n    ++\tint should_gc = 0, limit = 1;\n    ++\tDIR *dir = NULL;\n     +\n     +\tgit_config_get_int(\"maintenance.rerere-gc.auto\", &limit);\n     +\tif (limit <= 0) {\n    @@ builtin/gc.c: static int maintenance_task_rerere_gc(struct maintenance_run_opts\n     +\t\tgoto out;\n     +\t}\n     +\n    -+\t/* Skip garbage collecting the rerere cache in case rerere is disabled. */\n    ++\t/*\n    ++\t * We skip garbage collection in case we either have no \"rr-cache\"\n    ++\t * directory or when it doesn't contain at least as many directories as\n    ++\t * indicated by \"maintenance.rerere-gc.auto\".\n    ++\t */\n     +\trepo_git_path_replace(the_repository, &path, \"rr-cache\");\n    -+\tif (!is_directory(path.buf))\n    ++\tdir = opendir(path.buf);\n    ++\tif (!dir)\n     +\t\tgoto out;\n     +\n    -+\tif (rerere_collect_stale_entries(the_repository, &prunable_dirs,\n    -+\t\t\t\t\t &prunable_entries, &prunable_entries_nr) < 0)\n    -+\t\tgoto out;\n    ++\twhile (readdir_skip_dot_and_dotdot(dir)) {\n    ++\t\tif (--limit)\n    ++\t\t\tcontinue;\n     +\n    -+\tshould_gc = prunable_entries_nr >= limit;\n    ++\t\tshould_gc = 1;\n    ++\t\tgoto out;\n    ++\t}\n     +\n     +out:\n    -+\tstring_list_clear(&prunable_dirs, 0);\n    -+\tfree(prunable_entries);\n     +\tstrbuf_release(&path);\n    ++\tif (dir)\n    ++\t\tclosedir(dir);\n     +\treturn should_gc;\n     +}\n     +\n    @@ t/t7900-maintenance.sh: test_expect_success 'worktree-prune task honors gc.workt\n      \ttest_path_is_missing .git/worktrees/worktree\n      '\n      \n    -+setup_stale_rerere_entry () {\n    -+\trr=.git/rr-cache/\"$(printf \"%0$(test_oid hexsz)d\" \"$1\")\" &&\n    -+\tmkdir -p \"$rr\" &&\n    -+\t>\"$rr/preimage\" &&\n    -+\t>\"$rr/postimage\" &&\n    -+\n    -+\ttest-tool chmtime =\"$((-61 * 86400))\" \"$rr/preimage\" &&\n    -+\ttest-tool chmtime =\"$((-61 * 86400))\" \"$rr/postimage\"\n    -+}\n    -+\n     +test_expect_rerere_gc () {\n     +\tnegate=\n     +\tif test \"$1\" = \"!\"\n    @@ t/t7900-maintenance.sh: test_expect_success 'worktree-prune task honors gc.workt\n     +'\n     +\n     +test_expect_success 'rerere-gc task with --auto only prunes with prunable entries' '\n    ++\ttest_when_finished \"rm -rf .git/rr-cache\" &&\n     +\ttest_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&\n    -+\tfor i in $(test_seq 19)\n    -+\tdo\n    -+\t\tsetup_stale_rerere_entry $i || return 1\n    -+\tdone &&\n    ++\tmkdir .git/rr-cache &&\n     +\ttest_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&\n    -+\tsetup_stale_rerere_entry 20 &&\n    ++\t: >.git/rr-cache/entry &&\n     +\ttest_expect_rerere_gc git maintenance run --auto --task=rerere-gc\n     +'\n     +\n     +test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.auto' '\n    ++\ttest_when_finished \"rm -rf .git/rr-cache\" &&\n    ++\n     +\t# A negative value should always prune.\n     +\ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc &&\n     +\n    -+\tfor i in $(test_seq 20)\n    -+\tdo\n    -+\t\tsetup_stale_rerere_entry $i || return 1\n    -+\tdone &&\n    ++\tmkdir .git/rr-cache &&\n    ++\t: >.git/rr-cache/entry-1 &&\n    ++\t: >.git/rr-cache/entry-2 &&\n     +\n     +\t# Zero should never prune.\n     +\ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc &&\n     +\t# A positive value should require at least this many stale rerere entries.\n    -+\ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=21 maintenance run --auto --task=rerere-gc &&\n    -+\ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=10 maintenance run --auto --task=rerere-gc\n    ++\ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=3 maintenance run --auto --task=rerere-gc &&\n    ++\ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=2 maintenance run --auto --task=rerere-gc\n     +'\n     +\n      test_expect_success '--auto and --schedule incompatible' '\n\n---\nbase-commit: a2955b34f48265d240ab8c7deb0a929ec2d65fd0\nchange-id: 20250424-pks-maintenance-missing-tasks-8ffcdd596b73\n\n"},{"id":"517089","messageId":"20250502-pks-maintenance-missing-tasks-v3-1-13e130d36640@pks.im","threadId":"63382","inReplyTo":"20250502-pks-maintenance-missing-tasks-v3-0-13e130d36640@pks.im","subject":"[PATCH v3 1/7] builtin/gc: fix indentation of `cmd_gc()` parameters","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-02T08:43:58Z","receivedAt":"2025-05-02T08:44:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The parameters of `cmd_gc()` aren't indented properly. Fix this.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/gc.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex d5c75be2522..a73ec22fb18 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -724,9 +724,9 @@ static void gc_before_repack(struct maintenance_run_opts *opts,\n }\n \n int cmd_gc(int argc,\n-const char **argv,\n-const char *prefix,\n-struct repository *repo UNUSED)\n+\t   const char **argv,\n+\t   const char *prefix,\n+\t   struct repository *repo UNUSED)\n {\n \tint aggressive = 0;\n \tint quiet = 0;\n\n-- \n2.49.0.987.g0cc8ee98dc.dirty\n\n"},{"id":"517090","messageId":"20250502-pks-maintenance-missing-tasks-v3-2-13e130d36640@pks.im","threadId":"63382","inReplyTo":"20250502-pks-maintenance-missing-tasks-v3-0-13e130d36640@pks.im","subject":"[PATCH v3 2/7] builtin/gc: remove global variables where it trivial to do","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-02T08:43:59Z","receivedAt":"2025-05-02T08:44:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We use a couple of global variables to assemble command line arguments\nfor subprocesses we execute in git-gc(1). All of these variables except\nthe one for git-repack(1) are only used in a single place though, so\nthey don't really add anything but confusion.\n\nRemove those variables.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/gc.c | 31 ++++++++++++-------------------\n 1 file changed, 12 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex a73ec22fb18..ada36e210f0 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -53,15 +53,9 @@ static const char * const builtin_gc_usage[] = {\n };\n \n static timestamp_t gc_log_expire_time;\n-\n static struct strvec repack = STRVEC_INIT;\n-static struct strvec prune = STRVEC_INIT;\n-static struct strvec prune_worktrees = STRVEC_INIT;\n-static struct strvec rerere = STRVEC_INIT;\n-\n static struct tempfile *pidfile;\n static struct lock_file log_lock;\n-\n static struct string_list pack_garbage = STRING_LIST_INIT_DUP;\n \n static void clean_pack_garbage(void)\n@@ -769,9 +763,6 @@ int cmd_gc(int argc,\n \t\t\t\t\t builtin_gc_usage, builtin_gc_options);\n \n \tstrvec_pushl(&repack, \"repack\", \"-d\", \"-l\", NULL);\n-\tstrvec_pushl(&prune, \"prune\", \"--expire\", NULL);\n-\tstrvec_pushl(&prune_worktrees, \"worktree\", \"prune\", \"--expire\", NULL);\n-\tstrvec_pushl(&rerere, \"rerere\", \"gc\", NULL);\n \n \tgc_config(&cfg);\n \n@@ -897,34 +888,36 @@ int cmd_gc(int argc,\n \t\tif (cfg.prune_expire) {\n \t\t\tstruct child_process prune_cmd = CHILD_PROCESS_INIT;\n \n+\t\t\tstrvec_pushl(&prune_cmd.args, \"prune\", \"--expire\", NULL);\n \t\t\t/* run `git prune` even if using cruft packs */\n-\t\t\tstrvec_push(&prune, cfg.prune_expire);\n+\t\t\tstrvec_push(&prune_cmd.args, cfg.prune_expire);\n \t\t\tif (quiet)\n-\t\t\t\tstrvec_push(&prune, \"--no-progress\");\n+\t\t\t\tstrvec_push(&prune_cmd.args, \"--no-progress\");\n \t\t\tif (repo_has_promisor_remote(the_repository))\n-\t\t\t\tstrvec_push(&prune,\n+\t\t\t\tstrvec_push(&prune_cmd.args,\n \t\t\t\t\t    \"--exclude-promisor-objects\");\n \t\t\tprune_cmd.git_cmd = 1;\n-\t\t\tstrvec_pushv(&prune_cmd.args, prune.v);\n+\n \t\t\tif (run_command(&prune_cmd))\n-\t\t\t\tdie(FAILED_RUN, prune.v[0]);\n+\t\t\t\tdie(FAILED_RUN, prune_cmd.args.v[0]);\n \t\t}\n \t}\n \n \tif (cfg.prune_worktrees_expire) {\n \t\tstruct child_process prune_worktrees_cmd = CHILD_PROCESS_INIT;\n \n-\t\tstrvec_push(&prune_worktrees, cfg.prune_worktrees_expire);\n \t\tprune_worktrees_cmd.git_cmd = 1;\n-\t\tstrvec_pushv(&prune_worktrees_cmd.args, prune_worktrees.v);\n+\t\tstrvec_pushl(&prune_worktrees_cmd.args, \"worktree\", \"prune\", \"--expire\", NULL);\n+\t\tstrvec_push(&prune_worktrees_cmd.args, cfg.prune_worktrees_expire);\n+\n \t\tif (run_command(&prune_worktrees_cmd))\n-\t\t\tdie(FAILED_RUN, prune_worktrees.v[0]);\n+\t\t\tdie(FAILED_RUN, prune_worktrees_cmd.args.v[0]);\n \t}\n \n \trerere_cmd.git_cmd = 1;\n-\tstrvec_pushv(&rerere_cmd.args, rerere.v);\n+\tstrvec_pushl(&rerere_cmd.args, \"rerere\", \"gc\", NULL);\n \tif (run_command(&rerere_cmd))\n-\t\tdie(FAILED_RUN, rerere.v[0]);\n+\t\tdie(FAILED_RUN, rerere_cmd.args.v[0]);\n \n \treport_garbage = report_pack_garbage;\n \treprepare_packed_git(the_repository);\n\n-- \n2.49.0.987.g0cc8ee98dc.dirty\n\n"},{"id":"517092","messageId":"20250502-pks-maintenance-missing-tasks-v3-3-13e130d36640@pks.im","threadId":"63382","inReplyTo":"20250502-pks-maintenance-missing-tasks-v3-0-13e130d36640@pks.im","subject":"[PATCH v3 3/7] builtin/gc: move pruning of worktrees into a separate function","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-02T08:44:00Z","receivedAt":"2025-05-02T08:44:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Move pruning of worktrees into a separate function. This prepares for a\nsubsequent commit where we introduce a new \"worktree-prune\" task for\ngit-maintenance(1).\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/gc.c | 25 +++++++++++++++----------\n 1 file changed, 15 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex ada36e210f0..005ecc3f192 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -334,6 +334,18 @@ static int maintenance_task_reflog_expire(struct maintenance_run_opts *opts UNUS\n \treturn run_command(&cmd);\n }\n \n+static int maintenance_task_worktree_prune(struct maintenance_run_opts *opts UNUSED,\n+\t\t\t\t\t   struct gc_config *cfg)\n+{\n+\tstruct child_process prune_worktrees_cmd = CHILD_PROCESS_INIT;\n+\n+\tprune_worktrees_cmd.git_cmd = 1;\n+\tstrvec_pushl(&prune_worktrees_cmd.args, \"worktree\", \"prune\", \"--expire\", NULL);\n+\tstrvec_push(&prune_worktrees_cmd.args, cfg->prune_worktrees_expire);\n+\n+\treturn run_command(&prune_worktrees_cmd);\n+}\n+\n static int too_many_loose_objects(struct gc_config *cfg)\n {\n \t/*\n@@ -903,16 +915,9 @@ int cmd_gc(int argc,\n \t\t}\n \t}\n \n-\tif (cfg.prune_worktrees_expire) {\n-\t\tstruct child_process prune_worktrees_cmd = CHILD_PROCESS_INIT;\n-\n-\t\tprune_worktrees_cmd.git_cmd = 1;\n-\t\tstrvec_pushl(&prune_worktrees_cmd.args, \"worktree\", \"prune\", \"--expire\", NULL);\n-\t\tstrvec_push(&prune_worktrees_cmd.args, cfg.prune_worktrees_expire);\n-\n-\t\tif (run_command(&prune_worktrees_cmd))\n-\t\t\tdie(FAILED_RUN, prune_worktrees_cmd.args.v[0]);\n-\t}\n+\tif (cfg.prune_worktrees_expire &&\n+\t    maintenance_task_worktree_prune(&opts, &cfg))\n+\t\tdie(FAILED_RUN, \"worktree\");\n \n \trerere_cmd.git_cmd = 1;\n \tstrvec_pushl(&rerere_cmd.args, \"rerere\", \"gc\", NULL);\n\n-- \n2.49.0.987.g0cc8ee98dc.dirty\n\n"},{"id":"517093","messageId":"20250502-pks-maintenance-missing-tasks-v3-4-13e130d36640@pks.im","threadId":"63382","inReplyTo":"20250502-pks-maintenance-missing-tasks-v3-0-13e130d36640@pks.im","subject":"[PATCH v3 4/7] worktree: expose function to retrieve worktree names","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-02T08:44:01Z","receivedAt":"2025-05-02T08:44:14Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Introduce a function that retrieves worktree names as present in\n\".git/worktrees\". This function will be used in a subsequent commit.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/worktree.c | 25 ++++++++++++-------------\n worktree.c         | 30 ++++++++++++++++++++++++++++++\n worktree.h         |  8 ++++++++\n 3 files changed, 50 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 87ccd47794c..9b00dbf1265 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -211,27 +211,24 @@ static void prune_dups(struct string_list *l)\n \n static void prune_worktrees(void)\n {\n-\tstruct strbuf reason = STRBUF_INIT;\n \tstruct strbuf main_path = STRBUF_INIT;\n \tstruct string_list kept = STRING_LIST_INIT_DUP;\n-\tchar *path;\n-\tDIR *dir;\n-\tstruct dirent *d;\n+\tstruct strvec worktrees = STRVEC_INIT;\n+\tstruct strbuf reason = STRBUF_INIT;\n \n-\tpath = repo_git_path(the_repository, \"worktrees\");\n-\tdir = opendir(path);\n-\tfree(path);\n-\tif (!dir)\n+\tif (get_worktree_names(the_repository, &worktrees) < 0 ||\n+\t    !worktrees.nr)\n \t\treturn;\n-\twhile ((d = readdir_skip_dot_and_dotdot(dir)) != NULL) {\n+\n+\tfor (size_t i = 0; i < worktrees.nr; i++) {\n \t\tchar *path;\n+\n \t\tstrbuf_reset(&reason);\n-\t\tif (should_prune_worktree(d->d_name, &reason, &path, expire))\n-\t\t\tprune_worktree(d->d_name, reason.buf);\n+\t\tif (should_prune_worktree(worktrees.v[i], &reason, &path, expire))\n+\t\t\tprune_worktree(worktrees.v[i], reason.buf);\n \t\telse if (path)\n-\t\t\tstring_list_append_nodup(&kept, path)->util = xstrdup(d->d_name);\n+\t\t\tstring_list_append_nodup(&kept, path)->util = xstrdup(worktrees.v[i]);\n \t}\n-\tclosedir(dir);\n \n \tstrbuf_add_absolute_path(&main_path, repo_get_common_dir(the_repository));\n \t/* massage main worktree absolute path to match 'gitdir' content */\n@@ -242,6 +239,8 @@ static void prune_worktrees(void)\n \n \tif (!show_only)\n \t\tdelete_worktrees_dir_if_empty();\n+\n+\tstrvec_clear(&worktrees);\n \tstrbuf_release(&reason);\n }\n \ndiff --git a/worktree.c b/worktree.c\nindex c34b9eb74e5..947b7a82209 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -988,6 +988,36 @@ int should_prune_worktree(const char *id, struct strbuf *reason, char **wtpath,\n \treturn rc;\n }\n \n+int get_worktree_names(struct repository *repo, struct strvec *out)\n+{\n+\tchar *worktrees_dir;\n+\tstruct dirent *d;\n+\tDIR *dir;\n+\tint ret;\n+\n+\tworktrees_dir = repo_git_path(repo, \"worktrees\");\n+\tdir = opendir(worktrees_dir);\n+\tif (!dir) {\n+\t\tif (errno == ENOENT) {\n+\t\t\tret = 0;\n+\t\t\tgoto out;\n+\t\t}\n+\n+\t\tret = -1;\n+\t\tgoto out;\n+\t}\n+\n+\twhile ((d = readdir_skip_dot_and_dotdot(dir)) != NULL)\n+\t\tstrvec_push(out, d->d_name);\n+\n+\tret = 0;\n+out:\n+\tif (dir)\n+\t\tclosedir(dir);\n+\tfree(worktrees_dir);\n+\treturn ret;\n+}\n+\n static int move_config_setting(const char *key, const char *value,\n \t\t\t       const char *from_file, const char *to_file)\n {\ndiff --git a/worktree.h b/worktree.h\nindex e4bcccdc0ae..59825c37881 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -38,6 +38,14 @@ struct worktree **get_worktrees(void);\n  */\n struct worktree **get_worktrees_without_reading_head(void);\n \n+/*\n+ * Retrieve all worktree names. Not all names may correspond to a fully\n+ * functional worktree. Returns 0 on success, a negative error code on failure.\n+ * Calling the function on a repository that doesn't have any worktrees is not\n+ * considered an error.\n+ */\n+int get_worktree_names(struct repository *repo, struct strvec *out);\n+\n /*\n  * Returns 1 if linked worktrees exist, 0 otherwise.\n  */\n\n-- \n2.49.0.987.g0cc8ee98dc.dirty\n\n"},{"id":"517094","messageId":"20250502-pks-maintenance-missing-tasks-v3-5-13e130d36640@pks.im","threadId":"63382","inReplyTo":"20250502-pks-maintenance-missing-tasks-v3-0-13e130d36640@pks.im","subject":"[PATCH v3 5/7] builtin/maintenance: introduce \"worktree-prune\" task","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-02T08:44:02Z","receivedAt":"2025-05-02T08:44:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While git-gc(1) knows to prune stale worktrees, git-maintenance(1) does\nnot yet have a task for this cleanup. Introduce a new \"worktree-prune\"\ntask to plug this gap.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/config/maintenance.adoc |  8 ++++\n Documentation/git-maintenance.adoc    |  4 ++\n builtin/gc.c                          | 46 +++++++++++++++++++++++\n t/t7900-maintenance.sh                | 71 +++++++++++++++++++++++++++++++++++\n 4 files changed, 129 insertions(+)\n\ndiff --git a/Documentation/config/maintenance.adoc b/Documentation/config/maintenance.adoc\nindex 41536162a77..b36b62c1c47 100644\n--- a/Documentation/config/maintenance.adoc\n+++ b/Documentation/config/maintenance.adoc\n@@ -83,3 +83,11 @@ maintenance.reflog-expire.auto::\n \tpositive value implies the command should run when the number of\n \texpired reflog entries in the \"HEAD\" reflog is at least the value of\n \t`maintenance.loose-objects.auto`. The default value is 100.\n+\n+maintenance.worktree-prune.auto::\n+\tThis integer config option controls how often the `worktree-prune` task\n+\tshould be run as part of `git maintenance run --auto`. If zero, then\n+\tthe `worktree-prune` task will not run with the `--auto` option. A\n+\tnegative value will force the task to run every time. Otherwise, a\n+\tpositive value implies the command should run when the number of\n+\tprunable worktrees exceeds the value. The default value is 1.\ndiff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc\nindex 3a1e2a69b6b..6f085a9cf8c 100644\n--- a/Documentation/git-maintenance.adoc\n+++ b/Documentation/git-maintenance.adoc\n@@ -166,6 +166,10 @@ reflog-expire::\n \tThe `reflog-expire` task deletes any entries in the reflog older than the\n \texpiry threshold. See linkgit:git-reflog[1] for more information.\n \n+worktree-prune::\n+\tThe `worktree-prune` task deletes stale or broken worktrees. See\n+\tlinkit:git-worktree[1] for more information.\n+\n OPTIONS\n -------\n --auto::\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 005ecc3f192..93a8c856dbc 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -44,6 +44,7 @@\n #include \"hook.h\"\n #include \"setup.h\"\n #include \"trace2.h\"\n+#include \"worktree.h\"\n \n #define FAILED_RUN \"failed to run %s\"\n \n@@ -346,6 +347,45 @@ static int maintenance_task_worktree_prune(struct maintenance_run_opts *opts UNU\n \treturn run_command(&prune_worktrees_cmd);\n }\n \n+static int worktree_prune_condition(struct gc_config *cfg)\n+{\n+\tstruct strvec worktrees = STRVEC_INIT;\n+\tstruct strbuf reason = STRBUF_INIT;\n+\ttimestamp_t expiry_date;\n+\tint should_prune = 0;\n+\tint limit = 1;\n+\n+\tgit_config_get_int(\"maintenance.worktree-prune.auto\", &limit);\n+\tif (limit <= 0) {\n+\t\tshould_prune = limit < 0;\n+\t\tgoto out;\n+\t}\n+\n+\tif (parse_expiry_date(cfg->prune_worktrees_expire, &expiry_date) ||\n+\t    get_worktree_names(the_repository, &worktrees) < 0)\n+\t\tgoto out;\n+\n+\tfor (size_t i = 0; i < worktrees.nr; i++) {\n+\t\tchar *wtpath;\n+\n+\t\tstrbuf_reset(&reason);\n+\t\tif (should_prune_worktree(worktrees.v[i], &reason, &wtpath, expiry_date)) {\n+\t\t\tlimit--;\n+\n+\t\t\tif (!limit) {\n+\t\t\t\tshould_prune = 1;\n+\t\t\t\tgoto out;\n+\t\t\t}\n+\t\t}\n+\t\tfree(wtpath);\n+\t}\n+\n+out:\n+\tstrvec_clear(&worktrees);\n+\tstrbuf_release(&reason);\n+\treturn should_prune;\n+}\n+\n static int too_many_loose_objects(struct gc_config *cfg)\n {\n \t/*\n@@ -1455,6 +1495,7 @@ enum maintenance_task_label {\n \tTASK_COMMIT_GRAPH,\n \tTASK_PACK_REFS,\n \tTASK_REFLOG_EXPIRE,\n+\tTASK_WORKTREE_PRUNE,\n \n \t/* Leave as final value */\n \tTASK__COUNT\n@@ -1496,6 +1537,11 @@ static struct maintenance_task tasks[] = {\n \t\tmaintenance_task_reflog_expire,\n \t\treflog_expire_condition,\n \t},\n+\t[TASK_WORKTREE_PRUNE] = {\n+\t\t\"worktree-prune\",\n+\t\tmaintenance_task_worktree_prune,\n+\t\tworktree_prune_condition,\n+\t},\n };\n \n static int compare_tasks_by_selection(const void *a_, const void *b_)\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 9b82e11c100..530c56ae91e 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -493,6 +493,77 @@ test_expect_success 'reflog-expire task --auto only packs when exceeding limits'\n \ttest_subcommand git reflog expire --all <reflog-expire-auto.txt\n '\n \n+test_expect_worktree_prune () {\n+\tnegate=\n+\tif test \"$1\" = \"!\"\n+\tthen\n+\t\tnegate=\"!\"\n+\t\tshift\n+\tfi\n+\n+\trm -f \"worktree-prune.txt\" &&\n+\tGIT_TRACE2_EVENT=\"$(pwd)/worktree-prune.txt\" \"$@\" &&\n+\ttest_subcommand $negate git worktree prune --expire 3.months.ago <worktree-prune.txt\n+}\n+\n+test_expect_success 'worktree-prune task without --auto always prunes' '\n+\ttest_expect_worktree_prune git maintenance run --task=worktree-prune\n+'\n+\n+test_expect_success 'worktree-prune task --auto only prunes with prunable worktree' '\n+\ttest_expect_worktree_prune ! git maintenance run --auto --task=worktree-prune &&\n+\tmkdir .git/worktrees &&\n+\t: >.git/worktrees/abc &&\n+\ttest_expect_worktree_prune git maintenance run --auto --task=worktree-prune\n+'\n+\n+test_expect_success 'worktree-prune task with --auto honors maintenance.worktree-prune.auto' '\n+\t# A negative value should always prune.\n+\ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=-1 maintenance run --auto --task=worktree-prune &&\n+\n+\tmkdir .git/worktrees &&\n+\t: >.git/worktrees/first &&\n+\t: >.git/worktrees/second &&\n+\t: >.git/worktrees/third &&\n+\n+\t# Zero should never prune.\n+\ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&\n+\t# A positive value should require at least this man prunable worktrees.\n+\ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&\n+\ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune\n+'\n+\n+test_expect_success 'worktree-prune task with --auto honors maintenance.worktree-prune.auto' '\n+\t# A negative value should always prune.\n+\ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=-1 maintenance run --auto --task=worktree-prune &&\n+\n+\tmkdir .git/worktrees &&\n+\t: >.git/worktrees/first &&\n+\t: >.git/worktrees/second &&\n+\t: >.git/worktrees/third &&\n+\n+\t# Zero should never prune.\n+\ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&\n+\t# A positive value should require at least this many prunable worktrees.\n+\ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&\n+\ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune\n+'\n+\n+test_expect_success 'worktree-prune task honors gc.worktreePruneExpire' '\n+\tgit worktree add worktree &&\n+\trm -rf worktree &&\n+\n+\trm -f worktree-prune.txt &&\n+\tGIT_TRACE2_EVENT=\"$(pwd)/worktree-prune.txt\" git -c gc.worktreePruneExpire=1.week.ago maintenance run --auto --task=worktree-prune &&\n+\ttest_subcommand ! git worktree prune --expire 1.week.ago <worktree-prune.txt &&\n+\ttest_path_is_dir .git/worktrees/worktree &&\n+\n+\trm -f worktree-prune.txt &&\n+\tGIT_TRACE2_EVENT=\"$(pwd)/worktree-prune.txt\" git -c gc.worktreePruneExpire=now maintenance run --auto --task=worktree-prune &&\n+\ttest_subcommand git worktree prune --expire now <worktree-prune.txt &&\n+\ttest_path_is_missing .git/worktrees/worktree\n+'\n+\n test_expect_success '--auto and --schedule incompatible' '\n \ttest_must_fail git maintenance run --auto --schedule=daily 2>err &&\n \ttest_grep \"at most one\" err\n\n-- \n2.49.0.987.g0cc8ee98dc.dirty\n\n"},{"id":"517095","messageId":"20250502-pks-maintenance-missing-tasks-v3-6-13e130d36640@pks.im","threadId":"63382","inReplyTo":"20250502-pks-maintenance-missing-tasks-v3-0-13e130d36640@pks.im","subject":"[PATCH v3 6/7] builtin/gc: move rerere garbage collection into separate function","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-02T08:44:03Z","receivedAt":"2025-05-02T08:44:18Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Move garbage collection of cached rerere entries into a separate\nfunction. This prepares us for a subsequent commit where we introduce a\nnew \"rerere-gc\" task for git-maintenance(1).\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/gc.c | 16 +++++++++++-----\n 1 file changed, 11 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 93a8c856dbc..3dd1d07cca4 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -386,6 +386,15 @@ static int worktree_prune_condition(struct gc_config *cfg)\n \treturn should_prune;\n }\n \n+static int maintenance_task_rerere_gc(struct maintenance_run_opts *opts UNUSED,\n+\t\t\t\t      struct gc_config *cfg UNUSED)\n+{\n+\tstruct child_process rerere_cmd = CHILD_PROCESS_INIT;\n+\trerere_cmd.git_cmd = 1;\n+\tstrvec_pushl(&rerere_cmd.args, \"rerere\", \"gc\", NULL);\n+\treturn run_command(&rerere_cmd);\n+}\n+\n static int too_many_loose_objects(struct gc_config *cfg)\n {\n \t/*\n@@ -782,7 +791,6 @@ int cmd_gc(int argc,\n \tint daemonized = 0;\n \tint keep_largest_pack = -1;\n \ttimestamp_t dummy;\n-\tstruct child_process rerere_cmd = CHILD_PROCESS_INIT;\n \tstruct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;\n \tstruct gc_config cfg = GC_CONFIG_INIT;\n \tconst char *prune_expire_sentinel = \"sentinel\";\n@@ -959,10 +967,8 @@ int cmd_gc(int argc,\n \t    maintenance_task_worktree_prune(&opts, &cfg))\n \t\tdie(FAILED_RUN, \"worktree\");\n \n-\trerere_cmd.git_cmd = 1;\n-\tstrvec_pushl(&rerere_cmd.args, \"rerere\", \"gc\", NULL);\n-\tif (run_command(&rerere_cmd))\n-\t\tdie(FAILED_RUN, rerere_cmd.args.v[0]);\n+\tif (maintenance_task_rerere_gc(&opts, &cfg))\n+\t\tdie(FAILED_RUN, \"rerere\");\n \n \treport_garbage = report_pack_garbage;\n \treprepare_packed_git(the_repository);\n\n-- \n2.49.0.987.g0cc8ee98dc.dirty\n\n"},{"id":"517096","messageId":"20250502-pks-maintenance-missing-tasks-v3-7-13e130d36640@pks.im","threadId":"63382","inReplyTo":"20250502-pks-maintenance-missing-tasks-v3-0-13e130d36640@pks.im","subject":"[PATCH v3 7/7] builtin/maintenance: introduce \"rerere-gc\" task","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-02T08:44:04Z","receivedAt":"2025-05-02T08:44:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While git-gc(1) knows to garbage collect the rerere cache,\ngit-maintenance(1) does not yet have a task for this cleanup. Introduce\na new \"rerere-gc\" task to plug this gap.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/config/maintenance.adoc |  8 +++++++\n Documentation/git-maintenance.adoc    |  4 ++++\n builtin/gc.c                          | 45 +++++++++++++++++++++++++++++++++++\n t/t7900-maintenance.sh                | 43 +++++++++++++++++++++++++++++++++\n 4 files changed, 100 insertions(+)\n\ndiff --git a/Documentation/config/maintenance.adoc b/Documentation/config/maintenance.adoc\nindex b36b62c1c47..2e8299dd15a 100644\n--- a/Documentation/config/maintenance.adoc\n+++ b/Documentation/config/maintenance.adoc\n@@ -84,6 +84,14 @@ maintenance.reflog-expire.auto::\n \texpired reflog entries in the \"HEAD\" reflog is at least the value of\n \t`maintenance.loose-objects.auto`. The default value is 100.\n \n+maintenance.rerere-gc.auto::\n+\tThis integer config option controls how often the `rerere-gc` task\n+\tshould be run as part of `git maintenance run --auto`. If zero, then\n+\tthe `rerere-gc` task will not run with the `--auto` option. A negative\n+\tvalue will force the task to run every time. Otherwise, a positive\n+\tvalue implies the command should run when there are at least this many\n+\tdirectory entries in the \"rr-cache\" directory. The default value is 1.\n+\n maintenance.worktree-prune.auto::\n \tThis integer config option controls how often the `worktree-prune` task\n \tshould be run as part of `git maintenance run --auto`. If zero, then\ndiff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc\nindex 6f085a9cf8c..931f3e02e85 100644\n--- a/Documentation/git-maintenance.adoc\n+++ b/Documentation/git-maintenance.adoc\n@@ -166,6 +166,10 @@ reflog-expire::\n \tThe `reflog-expire` task deletes any entries in the reflog older than the\n \texpiry threshold. See linkgit:git-reflog[1] for more information.\n \n+rerere-gc::\n+\tThe `rerere-gc` task invokes garbage collection for stale entries in\n+\tthe rerere cache. See linkgit:git-rerere[1] for more information.\n+\n worktree-prune::\n \tThe `worktree-prune` task deletes stale or broken worktrees. See\n \tlinkit:git-worktree[1] for more information.\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 3dd1d07cca4..7bba826c496 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -16,6 +16,7 @@\n #include \"builtin.h\"\n #include \"abspath.h\"\n #include \"date.h\"\n+#include \"dir.h\"\n #include \"environment.h\"\n #include \"hex.h\"\n #include \"config.h\"\n@@ -34,6 +35,7 @@\n #include \"pack-objects.h\"\n #include \"path.h\"\n #include \"reflog.h\"\n+#include \"rerere.h\"\n #include \"blob.h\"\n #include \"tree.h\"\n #include \"promisor-remote.h\"\n@@ -395,6 +397,43 @@ static int maintenance_task_rerere_gc(struct maintenance_run_opts *opts UNUSED,\n \treturn run_command(&rerere_cmd);\n }\n \n+static int rerere_gc_condition(struct gc_config *cfg UNUSED)\n+{\n+\tstruct strbuf path = STRBUF_INIT;\n+\tint should_gc = 0, limit = 1;\n+\tDIR *dir = NULL;\n+\n+\tgit_config_get_int(\"maintenance.rerere-gc.auto\", &limit);\n+\tif (limit <= 0) {\n+\t\tshould_gc = limit < 0;\n+\t\tgoto out;\n+\t}\n+\n+\t/*\n+\t * We skip garbage collection in case we either have no \"rr-cache\"\n+\t * directory or when it doesn't contain at least as many directories as\n+\t * indicated by \"maintenance.rerere-gc.auto\".\n+\t */\n+\trepo_git_path_replace(the_repository, &path, \"rr-cache\");\n+\tdir = opendir(path.buf);\n+\tif (!dir)\n+\t\tgoto out;\n+\n+\twhile (readdir_skip_dot_and_dotdot(dir)) {\n+\t\tif (--limit)\n+\t\t\tcontinue;\n+\n+\t\tshould_gc = 1;\n+\t\tgoto out;\n+\t}\n+\n+out:\n+\tstrbuf_release(&path);\n+\tif (dir)\n+\t\tclosedir(dir);\n+\treturn should_gc;\n+}\n+\n static int too_many_loose_objects(struct gc_config *cfg)\n {\n \t/*\n@@ -1502,6 +1541,7 @@ enum maintenance_task_label {\n \tTASK_PACK_REFS,\n \tTASK_REFLOG_EXPIRE,\n \tTASK_WORKTREE_PRUNE,\n+\tTASK_RERERE_GC,\n \n \t/* Leave as final value */\n \tTASK__COUNT\n@@ -1548,6 +1588,11 @@ static struct maintenance_task tasks[] = {\n \t\tmaintenance_task_worktree_prune,\n \t\tworktree_prune_condition,\n \t},\n+\t[TASK_RERERE_GC] = {\n+\t\t\"rerere-gc\",\n+\t\tmaintenance_task_rerere_gc,\n+\t\trerere_gc_condition,\n+\t},\n };\n \n static int compare_tasks_by_selection(const void *a_, const void *b_)\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 530c56ae91e..f34322de320 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -564,6 +564,49 @@ test_expect_success 'worktree-prune task honors gc.worktreePruneExpire' '\n \ttest_path_is_missing .git/worktrees/worktree\n '\n \n+test_expect_rerere_gc () {\n+\tnegate=\n+\tif test \"$1\" = \"!\"\n+\tthen\n+\t\tnegate=\"!\"\n+\t\tshift\n+\tfi\n+\n+\trm -f \"rerere-gc.txt\" &&\n+\tGIT_TRACE2_EVENT=\"$(pwd)/rerere-gc.txt\" \"$@\" &&\n+\ttest_subcommand $negate git rerere gc <rerere-gc.txt\n+}\n+\n+test_expect_success 'rerere-gc task without --auto always collects garbage' '\n+\ttest_expect_rerere_gc git maintenance run --task=rerere-gc\n+'\n+\n+test_expect_success 'rerere-gc task with --auto only prunes with prunable entries' '\n+\ttest_when_finished \"rm -rf .git/rr-cache\" &&\n+\ttest_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&\n+\tmkdir .git/rr-cache &&\n+\ttest_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&\n+\t: >.git/rr-cache/entry &&\n+\ttest_expect_rerere_gc git maintenance run --auto --task=rerere-gc\n+'\n+\n+test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.auto' '\n+\ttest_when_finished \"rm -rf .git/rr-cache\" &&\n+\n+\t# A negative value should always prune.\n+\ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc &&\n+\n+\tmkdir .git/rr-cache &&\n+\t: >.git/rr-cache/entry-1 &&\n+\t: >.git/rr-cache/entry-2 &&\n+\n+\t# Zero should never prune.\n+\ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc &&\n+\t# A positive value should require at least this many stale rerere entries.\n+\ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=3 maintenance run --auto --task=rerere-gc &&\n+\ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=2 maintenance run --auto --task=rerere-gc\n+'\n+\n test_expect_success '--auto and --schedule incompatible' '\n \ttest_must_fail git maintenance run --auto --schedule=daily 2>err &&\n \ttest_grep \"at most one\" err\n\n-- \n2.49.0.987.g0cc8ee98dc.dirty\n\n"},{"id":"517117","messageId":"d5307e82-8e45-4a2a-a8cc-03f84c2d5670@gmail.com","threadId":"63382","inReplyTo":"20250502-pks-maintenance-missing-tasks-v3-0-13e130d36640@pks.im","subject":"Re: [PATCH v3 0/7] builtin/maintenance: implement missing tasks compared to git-gc(1)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2025-05-02T14:57:33Z","receivedAt":"2025-05-02T14:57:34Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 5/2/2025 4:43 AM, Patrick Steinhardt wrote:\n\n> Changes in v3:\n>   - Simplify the heuristic for \"rerere-gc\" so that we only count the\n>     number of directory entries in \".git/rr-cache\", without considering\n>     staleness.\n\nThe range-diff was harder to read than just re-reviewing patch 7, but I\nthink this v3 is ready to go.\n\nThanks,\n-Stolee\n\n"},{"id":"517130","messageId":"xmqqo6wau3fz.fsf@gitster.g","threadId":"63382","inReplyTo":"d5307e82-8e45-4a2a-a8cc-03f84c2d5670@gmail.com","subject":"Re: [PATCH v3 0/7] builtin/maintenance: implement missing tasks compared to git-gc(1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-02T21:07:28Z","receivedAt":"2025-05-02T21:07:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n> On 5/2/2025 4:43 AM, Patrick Steinhardt wrote:\n>\n>> Changes in v3:\n>>   - Simplify the heuristic for \"rerere-gc\" so that we only count the\n>>     number of directory entries in \".git/rr-cache\", without considering\n>>     staleness.\n>\n> The range-diff was harder to read than just re-reviewing patch 7, but I\n> think this v3 is ready to go.\n\nI still do not think \"configurable 'rerere gc' basing the decision\non the number of existing rerere entries\" adds negative value to the\nsystem.  If there is truly more than N active rerere entries\ncurrently in your repository with your workflow, such a\nconfiguration essentially decides to run 'rerere gc' every time (and\nwithout pruning enough entries to make the next 'rerere gc'\nunnecessary), and never otherwise.  It is not like pruning unneeded\nloose object files and packing the rest into a packfile, where\nrunning it once (even if it resulted in miniscule packfile due to\nmisidentification) would remove all the loose object files, which\nmakes 'repack' unneeded for some time until we accumulate more of\nthem.  After 'rerere gc', you still will have rerere entries kept.\n\nUntil we can implement a meaningful automation (which may require\nchanging the way rerere entries are stored on disk to help us\ncheaply tell if there truly are excessive number of no longer\nrelevant rerere entries; code that we can readily borrow from\n\"rerere gc\" is enough, as I said), I do not think we should add\nextra code to make such a useless decision.  Instead, I would prefer\nto see a single \"do we or do we not run `rerere gc`?\" Boolean, until\nthat happens.\n\nOther than that, I think the series is a good addition to the\nsystem.  Giving finer grained control is great, and 'git maintenance'\nis a much better framework than 'git gc' to do so.\n\nThanks, all.\n\n\n"},{"id":"517214","messageId":"aBhpn3q-sgm3WjR6@pks.im","threadId":"63382","inReplyTo":"xmqqo6wau3fz.fsf@gitster.g","subject":"Re: [PATCH v3 0/7] builtin/maintenance: implement missing tasks compared to git-gc(1)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-05T07:32:47Z","receivedAt":"2025-05-05T07:32:55Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, May 02, 2025 at 02:07:28PM -0700, Junio C Hamano wrote:\n> Derrick Stolee <stolee@gmail.com> writes:\n> \n> > On 5/2/2025 4:43 AM, Patrick Steinhardt wrote:\n> >\n> >> Changes in v3:\n> >>   - Simplify the heuristic for \"rerere-gc\" so that we only count the\n> >>     number of directory entries in \".git/rr-cache\", without considering\n> >>     staleness.\n> >\n> > The range-diff was harder to read than just re-reviewing patch 7, but I\n> > think this v3 is ready to go.\n> \n> I still do not think \"configurable 'rerere gc' basing the decision\n> on the number of existing rerere entries\" adds negative value to the\n> system.  If there is truly more than N active rerere entries\n> currently in your repository with your workflow, such a\n> configuration essentially decides to run 'rerere gc' every time (and\n> without pruning enough entries to make the next 'rerere gc'\n> unnecessary), and never otherwise.  It is not like pruning unneeded\n> loose object files and packing the rest into a packfile, where\n> running it once (even if it resulted in miniscule packfile due to\n> misidentification) would remove all the loose object files, which\n> makes 'repack' unneeded for some time until we accumulate more of\n> them.  After 'rerere gc', you still will have rerere entries kept.\n> \n> Until we can implement a meaningful automation (which may require\n> changing the way rerere entries are stored on disk to help us\n> cheaply tell if there truly are excessive number of no longer\n> relevant rerere entries; code that we can readily borrow from\n> \"rerere gc\" is enough, as I said), I do not think we should add\n> extra code to make such a useless decision.  Instead, I would prefer\n> to see a single \"do we or do we not run `rerere gc`?\" Boolean, until\n> that happens.\n> \n> Other than that, I think the series is a good addition to the\n> system.  Giving finer grained control is great, and 'git maintenance'\n> is a much better framework than 'git gc' to do so.\n\nOk, fair enough. I'll stick with the \"maintenance.rerere-gc.auto\"\nconfig as integer, but will adapt it so that any positive value will\ncause us to invoke `git rerere gc` when the \"rr-cache\" directory exists\nand has at least one entry.\n\nThe reason I want to keep it as an integer is mostly to stay consistent\nwith all the other \"maintenance.*.auto\" settings, even though it does\nnot add a lot of value right now.\n\nPatrick\n"},{"id":"517221","messageId":"CAPig+cSDDbhGrym8j=PFKBCUxBQhZPzAHXGvKy-Z6POA4Ju3sw@mail.gmail.com","threadId":"63382","inReplyTo":"20250502-pks-maintenance-missing-tasks-v3-4-13e130d36640@pks.im","subject":"Re: [PATCH v3 4/7] worktree: expose function to retrieve worktree names","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-05-05T08:42:40Z","receivedAt":"2025-05-05T08:42:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, May 2, 2025 at 4:44 AM Patrick Steinhardt <ps@pks.im> wrote:\n> Introduce a function that retrieves worktree names as present in\n> \".git/worktrees\". This function will be used in a subsequent commit.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n\nI'm not convinced that this patch or the get_worktree_names() function\nwhich it adds to worktree.[hc] adds value. Aside from the mere act of\nconsulting the directory at repo_git_path(r, \"worktrees\"), there is\nnothing about the function at all related to worktrees. It doesn't\nmake any guarantees, such as only returning entries which at least\nsuperficially look like worktree-metadata directories, or perform any\nsort of validation. I don't see how this is any better than the caller\njust implementing its own bog-standard opendir() / readdir()-loop /\nclosedir() over repo_git_path(r, \"worktrees\"). Or, if you don't want\nthe caller to implement its own readdir()-loop, I wouldn't be\nsurprised if we already have a function which does exactly this for a\nprovided path, though I haven't checked. If there isn't such a generic\nfunction, perhaps it makes more sense to add one and call it with\nrepo_git_path(r, \"worktrees\") as its input?\n\n> diff --git a/worktree.c b/worktree.c\n> @@ -988,6 +988,36 @@ int should_prune_worktree(const char *id, struct strbuf *reason, char **wtpath,\n> +int get_worktree_names(struct repository *repo, struct strvec *out)\n> +{\n> +       worktrees_dir = repo_git_path(repo, \"worktrees\");\n> +       dir = opendir(worktrees_dir);\n> +       if (!dir) {\n> +               if (errno == ENOENT) {\n> +                       ret = 0;\n> +                       goto out;\n> +               }\n> +\n> +               ret = -1;\n> +               goto out;\n> +       }\n> +\n> +       while ((d = readdir_skip_dot_and_dotdot(dir)) != NULL)\n> +               strvec_push(out, d->d_name);\n> +\n> +       ret = 0;\n> +out:\n> +       if (dir)\n> +               closedir(dir);\n> +       free(worktrees_dir);\n> +       return ret;\n> +}\n\nIt's subjective, but although we often recommend the `goto` approach\non this project to ensure proper cleanup, the above seems to place an\nunnecessarily high cognitive load on the reader. I would think that\nthe following straightforward `goto`-free approach would suffice:\n\n    worktrees_dir = repo_git_path(repo, \"worktrees\");\n    dir = opendir(worktrees_dir);\n    FREE_AND_NULL(worktrees_dir);\n    if (!dir)\n        return errno == ENOENT ? 0 : -1;\n    while ((d = readdir_skip_dot_and_dotdot(dir)) != NULL)\n        strvec_push(out, d->d_name);\n    closedir(dir);\n    return 0\n\n> diff --git a/worktree.h b/worktree.h\n> @@ -38,6 +38,14 @@ struct worktree **get_worktrees(void);\n> +/*\n> + * Retrieve all worktree names. Not all names may correspond to a fully\n> + * functional worktree. Returns 0 on success, a negative error code on failure.\n> + * Calling the function on a repository that doesn't have any worktrees is not\n> + * considered an error.\n> + */\n> +int get_worktree_names(struct repository *repo, struct strvec *out);\n\nAs I was reading through the patch, I was worried that you might have\noverlooked the fact that the names returned by the function might not\nbe fully functional worktrees or that the caller of this function\nmight not realize that, so I'm glad to see that you documented this\npotential downside.\n\nIn fact, there is no guarantee that the returned entries are even\ndirectories; they could be anything that someone happened to create in\nthe .git/worktrees directory, such as plain files, special files, etc.\nSo, aside from the objections I wrote above, the term \"names\" here is\npotentially unclear and misleading, as used both in the documentation\nand the function name itself, and the fact that I can't come up with a\nbetter term further leads me to believe that this is not a function we\nreally want to be publishing.\n"},{"id":"517230","messageId":"CAPig+cScor=E2i4w99NCrsaUd-Po=FqhkrLSyT69PSo1+h2dRw@mail.gmail.com","threadId":"63382","inReplyTo":"20250502-pks-maintenance-missing-tasks-v3-5-13e130d36640@pks.im","subject":"Re: [PATCH v3 5/7] builtin/maintenance: introduce \"worktree-prune\" task","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-05-05T08:59:14Z","receivedAt":"2025-05-05T08:59:25Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, May 2, 2025 at 4:44 AM Patrick Steinhardt <ps@pks.im> wrote:\n> While git-gc(1) knows to prune stale worktrees, git-maintenance(1) does\n> not yet have a task for this cleanup. Introduce a new \"worktree-prune\"\n> task to plug this gap.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> @@ -346,6 +347,45 @@ static int maintenance_task_worktree_prune(struct maintenance_run_opts *opts UNU\n> +static int worktree_prune_condition(struct gc_config *cfg)\n> +{\n> + [...]\n> +       git_config_get_int(\"maintenance.worktree-prune.auto\", &limit);\n> +       if (limit <= 0) {\n> +               should_prune = limit < 0;\n> +               goto out;\n> +       }\n> +\n> +       if (parse_expiry_date(cfg->prune_worktrees_expire, &expiry_date) ||\n> +           get_worktree_names(the_repository, &worktrees) < 0)\n> +               goto out;\n> +\n> +       for (size_t i = 0; i < worktrees.nr; i++) {\n> +               char *wtpath;\n> +               strbuf_reset(&reason);\n> +               if (should_prune_worktree(worktrees.v[i], &reason, &wtpath, expiry_date)) {\n\nAs I was reading the commit message, I had concerns that if this\nimplementation was rolling its own pruning logic, it might overlook\nconditions such as a worktree being locked, so I'm glad to see that\nyou're using should_prune_worktree() here rather than reinventing the\nwheel.\n\n> +                       limit--;\n> +\n> +                       if (!limit) {\n> +                               should_prune = 1;\n> +                               goto out;\n> +                       }\n> +               }\n> +               free(wtpath);\n\nThis leaks `wtpath` when it takes the `goto out` arm.\n\n> +       }\n> +\n> +out:\n> +       strvec_clear(&worktrees);\n> +       strbuf_release(&reason);\n> +       return should_prune;\n> +}\n> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\n> @@ -493,6 +493,77 @@ test_expect_success 'reflog-expire task --auto only packs when exceeding limits'\n> +test_expect_success 'worktree-prune task with --auto honors maintenance.worktree-prune.auto' '\n> +       # A negative value should always prune.\n> +       test_expect_worktree_prune git -c maintenance.worktree-prune.auto=-1 maintenance run --auto --task=worktree-prune &&\n> +\n> +       mkdir .git/worktrees &&\n> +       : >.git/worktrees/first &&\n> +       : >.git/worktrees/second &&\n> +       : >.git/worktrees/third &&\n> +\n> +       # Zero should never prune.\n> +       test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&\n> +       # A positive value should require at least this man prunable worktrees.\n\ns/man/many/\n"},{"id":"517448","messageId":"aBsGgNsNmy-t5CGK@pks.im","threadId":"63382","inReplyTo":"CAPig+cSDDbhGrym8j=PFKBCUxBQhZPzAHXGvKy-Z6POA4Ju3sw@mail.gmail.com","subject":"Re: [PATCH v3 4/7] worktree: expose function to retrieve worktree names","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-07T07:06:40Z","receivedAt":"2025-05-07T07:06:43Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, May 05, 2025 at 04:42:40AM -0400, Eric Sunshine wrote:\n> On Fri, May 2, 2025 at 4:44 AM Patrick Steinhardt <ps@pks.im> wrote:\n> > Introduce a function that retrieves worktree names as present in\n> > \".git/worktrees\". This function will be used in a subsequent commit.\n> >\n> > Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> \n> I'm not convinced that this patch or the get_worktree_names() function\n> which it adds to worktree.[hc] adds value. Aside from the mere act of\n> consulting the directory at repo_git_path(r, \"worktrees\"), there is\n> nothing about the function at all related to worktrees. It doesn't\n> make any guarantees, such as only returning entries which at least\n> superficially look like worktree-metadata directories, or perform any\n> sort of validation. I don't see how this is any better than the caller\n> just implementing its own bog-standard opendir() / readdir()-loop /\n> closedir() over repo_git_path(r, \"worktrees\"). Or, if you don't want\n> the caller to implement its own readdir()-loop, I wouldn't be\n> surprised if we already have a function which does exactly this for a\n> provided path, though I haven't checked. If there isn't such a generic\n> function, perhaps it makes more sense to add one and call it with\n> repo_git_path(r, \"worktrees\") as its input?\n\nI think this is fair criticism indeed, and open-coding this isn't even\nany more complex, either, as shown by the below patch. I'll drop this\npatch.\n\nPatrick\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 82f0fac81a4..eb4469b7858 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -351,11 +351,11 @@ static int maintenance_task_worktree_prune(struct maintenance_run_opts *opts UNU\n \n static int worktree_prune_condition(struct gc_config *cfg)\n {\n-\tstruct strvec worktrees = STRVEC_INIT;\n-\tstruct strbuf reason = STRBUF_INIT;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint should_prune = 0, limit = 1;\n \ttimestamp_t expiry_date;\n-\tint should_prune = 0;\n-\tint limit = 1;\n+\tstruct dirent *d;\n+\tDIR *dir = NULL;\n \n \tgit_config_get_int(\"maintenance.worktree-prune.auto\", &limit);\n \tif (limit <= 0) {\n@@ -363,15 +363,18 @@ static int worktree_prune_condition(struct gc_config *cfg)\n \t\tgoto out;\n \t}\n \n-\tif (parse_expiry_date(cfg->prune_worktrees_expire, &expiry_date) ||\n-\t    get_worktree_names(the_repository, &worktrees) < 0)\n+\tif (parse_expiry_date(cfg->prune_worktrees_expire, &expiry_date))\n+\t\tgoto out;\n+\n+\tdir = opendir(repo_git_path_replace(the_repository, &buf, \"worktrees\"));\n+\tif (!dir)\n \t\tgoto out;\n \n-\tfor (size_t i = 0; i < worktrees.nr; i++) {\n+\twhile ((d = readdir_skip_dot_and_dotdot(dir))) {\n \t\tchar *wtpath;\n \n-\t\tstrbuf_reset(&reason);\n-\t\tif (should_prune_worktree(worktrees.v[i], &reason, &wtpath, expiry_date)) {\n+\t\tstrbuf_reset(&buf);\n+\t\tif (should_prune_worktree(d->d_name, &buf, &wtpath, expiry_date)) {\n \t\t\tlimit--;\n \n \t\t\tif (!limit) {\n@@ -384,8 +387,9 @@ static int worktree_prune_condition(struct gc_config *cfg)\n \t}\n \n out:\n-\tstrvec_clear(&worktrees);\n-\tstrbuf_release(&reason);\n+\tif (dir)\n+\t\tclosedir(dir);\n+\tstrbuf_release(&buf);\n \treturn should_prune;\n }\n"},{"id":"517449","messageId":"aBsGg7w_4uKyQSaQ@pks.im","threadId":"63382","inReplyTo":"CAPig+cScor=E2i4w99NCrsaUd-Po=FqhkrLSyT69PSo1+h2dRw@mail.gmail.com","subject":"Re: [PATCH v3 5/7] builtin/maintenance: introduce \"worktree-prune\" task","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-07T07:06:43Z","receivedAt":"2025-05-07T07:06:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, May 05, 2025 at 04:59:14AM -0400, Eric Sunshine wrote:\n> On Fri, May 2, 2025 at 4:44 AM Patrick Steinhardt <ps@pks.im> wrote:\n> > +                       limit--;\n> > +\n> > +                       if (!limit) {\n> > +                               should_prune = 1;\n> > +                               goto out;\n> > +                       }\n> > +               }\n> > +               free(wtpath);\n> \n> This leaks `wtpath` when it takes the `goto out` arm.\n\nGood catch, fixed now.\n\n> > +       }\n> > +\n> > +out:\n> > +       strvec_clear(&worktrees);\n> > +       strbuf_release(&reason);\n> > +       return should_prune;\n> > +}\n> > diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\n> > @@ -493,6 +493,77 @@ test_expect_success 'reflog-expire task --auto only packs when exceeding limits'\n> > +test_expect_success 'worktree-prune task with --auto honors maintenance.worktree-prune.auto' '\n> > +       # A negative value should always prune.\n> > +       test_expect_worktree_prune git -c maintenance.worktree-prune.auto=-1 maintenance run --auto --task=worktree-prune &&\n> > +\n> > +       mkdir .git/worktrees &&\n> > +       : >.git/worktrees/first &&\n> > +       : >.git/worktrees/second &&\n> > +       : >.git/worktrees/third &&\n> > +\n> > +       # Zero should never prune.\n> > +       test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&\n> > +       # A positive value should require at least this man prunable worktrees.\n> \n> s/man/many/\n\nFixed, as well.\n\nPatrick\n"}]}