On Tue, Apr 29, 2025 at 04:02:03PM -0400, Derrick Stolee wrote:
> On 4/25/2025 3:29 AM, Patrick Steinhardt wrote:
> > 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.
>
> I initially thought that this could merge down into patch 3 (move pruning
> of worktrees into a separate function), but...
>
> > +static int worktree_prune_condition(struct gc_config *cfg)
> > +{
> > + struct strvec worktrees = STRVEC_INIT;
> > + struct strbuf reason = STRBUF_INIT;
> > + timestamp_t expiry_date;
> > + int should_prune = 0;
> > +
> > + 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)) {
> > + should_prune = 1;
> > + goto out;
> > + }
> > + free(wtpath);
> > + }
> > +
> > +out:
> > + strvec_clear(&worktrees);
> > + strbuf_release(&reason);
> > + return should_prune;
> > +}
> > +
>
> ...this implementation is new and nice to have in a separate patch. I
> initially wondered if this condition needed to exist in the maintenance
> builtin or could be relied upon by the 'git worktree prune' command that
> is called by this implementation.
>
> If we are trying to match the behavior of 'git gc --auto', then it was
> running 'git worktree prune --expire...' every time that the generic
> --auto condition was satisfied. But when 'git maintenance run --auto' is
> executed, each task is checked to see if it should run. If we can avoid a
> child process startup, then that is very valuable (especially on Windows
> where process creation is expensive).
Yup, exactly. In theory, we could even make the condition configurable
via "maintenance.worktree-prune.auto" so that we treat it as a limit of
how many worktrees need to be prunable before we execute `git worktree
prune`. Maybe I'll do just that in the next iteration.
> So I think this is a good approach. Similar thoughts apply to patch 7. No
> code change is needed.
>
> > +test_expect_success 'worktree-prune task' '
> > + GIT_TRACE2_EVENT="$(pwd)/worktree-prune.txt" \
> > + git maintenance run --task=worktree-prune &&
> > + test_subcommand git worktree prune --expire 3.months.ago <worktree-prune.txt
> > +'
> > +
> > +test_expect_success 'worktree-prune task --auto only prunes with prunable worktree' '
> > + GIT_TRACE2_EVENT="$(pwd)/worktree-prune-auto.txt" \
> > + git maintenance run --auto --task=worktree-prune &&
> > + test_subcommand ! git worktree prune --expire 3.months.ago <worktree-prune-auto.txt &&
> > + mkdir .git/worktrees &&
> > + : >.git/worktrees/abc &&
> > + GIT_TRACE2_EVENT="$(pwd)/worktree-prune-auto.txt" \
> > + git maintenance run --auto --task=worktree-prune &&
> > + test_subcommand git worktree prune --expire 3.months.ago <worktree-prune-auto.txt
> > +'
> > +
> > test_expect_success '--auto and --schedule incompatible' '
> > test_must_fail git maintenance run --auto --schedule=daily 2>err &&
> > test_grep "at most one" err
>
> It may be good to double-check that the gc.worktreePruneExpire config value
> is being used here, especially since the prune condition is operating on
> that value.
Fair, will do.
Patrick
