Re: [PATCH 5/7] builtin/maintenance: introduce "worktree-prune" task
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Apr 29, 2025, 20:02 UTC
- Message-ID
- <c629f845-180f-4ecb-949f-a99f184f812d@gmail.com>
- In-Reply-To
- <20250425-pks-maintenance-missing-tasks-v1-5-972ed6ab2c0d@pks.im>
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...
Show 28 quoted lines
> +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).
So I think this is a good approach. Similar thoughts apply to patch 7. No code change is needed.
Show 20 quoted lines
> +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.
Thanks, -Stolee