Subject: Re: [PATCH 5/7] builtin/maintenance: introduce "worktree-prune" task

From: Derrick Stolee <[email-masked]>
Date: Tue, 29 Apr 2025 16:02:03 -0400

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).

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.

Thanks,
-Stolee