Re: [PATCH v3 5/7] builtin/maintenance: introduce "worktree-prune" task
From: Eric Sunshine <hidden>
Date: 2025-05-05 08:59:25
On Fri, May 2, 2025 at 4:44 AM Patrick Steinhardt [off-list ref] wrote:
quoted hunk ↗ jump to hunk
While git-gc(1) knows to prune stale worktrees, git-maintenance(1) does not yet have a task for this cleanup. Introduce a new "worktree-prune" task to plug this gap. Signed-off-by: Patrick Steinhardt <redacted> ---diff --git a/builtin/gc.c b/builtin/gc.c@@ -346,6 +347,45 @@ static int maintenance_task_worktree_prune(struct maintenance_run_opts *opts UNU +static int worktree_prune_condition(struct gc_config *cfg) +{ + [...] + git_config_get_int("maintenance.worktree-prune.auto", &limit); + if (limit <= 0) { + should_prune = limit < 0; + goto out; + } + + if (parse_expiry_date(cfg->prune_worktrees_expire, &expiry_date) || + get_worktree_names(the_repository, &worktrees) < 0) + goto out; + + for (size_t i = 0; i < worktrees.nr; i++) { + char *wtpath; + strbuf_reset(&reason); + if (should_prune_worktree(worktrees.v[i], &reason, &wtpath, expiry_date)) {
As I was reading the commit message, I had concerns that if this implementation was rolling its own pruning logic, it might overlook conditions such as a worktree being locked, so I'm glad to see that you're using should_prune_worktree() here rather than reinventing the wheel.
+ limit--;
+
+ if (!limit) {
+ should_prune = 1;
+ goto out;
+ }
+ }
+ free(wtpath);This leaks `wtpath` when it takes the `goto out` arm.
quoted hunk ↗ jump to hunk
+ } + +out: + strvec_clear(&worktrees); + strbuf_release(&reason); + return should_prune; +}diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh@@ -493,6 +493,77 @@ test_expect_success 'reflog-expire task --auto only packs when exceeding limits' +test_expect_success 'worktree-prune task with --auto honors maintenance.worktree-prune.auto' ' + # A negative value should always prune. + test_expect_worktree_prune git -c maintenance.worktree-prune.auto=-1 maintenance run --auto --task=worktree-prune && + + mkdir .git/worktrees && + : >.git/worktrees/first && + : >.git/worktrees/second && + : >.git/worktrees/third && + + # Zero should never prune. + test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune && + # A positive value should require at least this man prunable worktrees.
s/man/many/