git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Derrick Stolee <stolee@gmail.com>
To: Patrick Steinhardt <ps@pks.im>, git@vger.kernel.org
Subject: Re: [PATCH 5/7] builtin/maintenance: introduce "worktree-prune" task
Date: Tue, 29 Apr 2025 16:02:03 -0400	[thread overview]
Message-ID: <c629f845-180f-4ecb-949f-a99f184f812d@gmail.com> (raw)
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...

> +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

  reply	other threads:[~2025-04-29 20:02 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-25  7:29 [PATCH 0/7] builtin/maintenance: implement missing tasks compared to git-gc(1) Patrick Steinhardt
2025-04-25  7:29 ` [PATCH 1/7] builtin/gc: fix indentation of `cmd_gc()` parameters Patrick Steinhardt
2025-04-25  7:29 ` [PATCH 2/7] builtin/gc: remove global variables where it trivial to do Patrick Steinhardt
2025-04-25  7:29 ` [PATCH 3/7] builtin/gc: move pruning of worktrees into a separate function Patrick Steinhardt
2025-04-25  7:29 ` [PATCH 4/7] worktree: expose function to retrieve worktree names Patrick Steinhardt
2025-04-25  7:29 ` [PATCH 5/7] builtin/maintenance: introduce "worktree-prune" task Patrick Steinhardt
2025-04-29 20:02   ` Derrick Stolee [this message]
2025-04-30  7:08     ` Patrick Steinhardt
2025-04-25  7:29 ` [PATCH 6/7] builtin/gc: move rerere garbage collection into separate function Patrick Steinhardt
2025-04-25  7:29 ` [PATCH 7/7] builtin/maintenance: introduce "rerere-gc" task Patrick Steinhardt
2025-04-29 20:02 ` [PATCH 0/7] builtin/maintenance: implement missing tasks compared to git-gc(1) Derrick Stolee
2025-04-30  7:08   ` Patrick Steinhardt
2025-04-30 10:25 ` [PATCH v2 0/8] " Patrick Steinhardt
2025-04-30 10:25   ` [PATCH v2 1/8] builtin/gc: fix indentation of `cmd_gc()` parameters Patrick Steinhardt
2025-04-30 10:25   ` [PATCH v2 2/8] builtin/gc: remove global variables where it trivial to do Patrick Steinhardt
2025-04-30 10:25   ` [PATCH v2 3/8] builtin/gc: move pruning of worktrees into a separate function Patrick Steinhardt
2025-04-30 10:25   ` [PATCH v2 4/8] worktree: expose function to retrieve worktree names Patrick Steinhardt
2025-04-30 10:25   ` [PATCH v2 5/8] builtin/maintenance: introduce "worktree-prune" task Patrick Steinhardt
2025-04-30 10:25   ` [PATCH v2 6/8] rerere: provide function to collect stale entries Patrick Steinhardt
2025-04-30 16:58     ` Junio C Hamano
2025-05-02  8:07       ` Patrick Steinhardt
2025-05-02 16:35         ` Junio C Hamano
2025-05-05  7:22           ` Patrick Steinhardt
2025-04-30 10:25   ` [PATCH v2 7/8] builtin/gc: move rerere garbage collection into separate function Patrick Steinhardt
2025-04-30 10:25   ` [PATCH v2 8/8] builtin/maintenance: introduce "rerere-gc" task Patrick Steinhardt
2025-04-30 10:37   ` [PATCH v2 0/8] builtin/maintenance: implement missing tasks compared to git-gc(1) Derrick Stolee
2025-05-02  8:43 ` [PATCH v3 0/7] " Patrick Steinhardt
2025-05-02  8:43   ` [PATCH v3 1/7] builtin/gc: fix indentation of `cmd_gc()` parameters Patrick Steinhardt
2025-05-02  8:43   ` [PATCH v3 2/7] builtin/gc: remove global variables where it trivial to do Patrick Steinhardt
2025-05-02  8:44   ` [PATCH v3 3/7] builtin/gc: move pruning of worktrees into a separate function Patrick Steinhardt
2025-05-02  8:44   ` [PATCH v3 4/7] worktree: expose function to retrieve worktree names Patrick Steinhardt
2025-05-05  8:42     ` Eric Sunshine
2025-05-07  7:06       ` Patrick Steinhardt
2025-05-02  8:44   ` [PATCH v3 5/7] builtin/maintenance: introduce "worktree-prune" task Patrick Steinhardt
2025-05-05  8:59     ` Eric Sunshine
2025-05-07  7:06       ` Patrick Steinhardt
2025-05-02  8:44   ` [PATCH v3 6/7] builtin/gc: move rerere garbage collection into separate function Patrick Steinhardt
2025-05-02  8:44   ` [PATCH v3 7/7] builtin/maintenance: introduce "rerere-gc" task Patrick Steinhardt
2025-05-02 14:57   ` [PATCH v3 0/7] builtin/maintenance: implement missing tasks compared to git-gc(1) Derrick Stolee
2025-05-02 21:07     ` Junio C Hamano
2025-05-05  7:32       ` Patrick Steinhardt
2025-05-05  8:51 ` [PATCH v4 " Patrick Steinhardt
2025-05-05  8:51   ` [PATCH v4 1/7] builtin/gc: fix indentation of `cmd_gc()` parameters Patrick Steinhardt
2025-05-05  8:51   ` [PATCH v4 2/7] builtin/gc: remove global variables where it trivial to do Patrick Steinhardt
2025-05-06  7:44     ` Christian Couder
2025-05-07  7:06       ` Patrick Steinhardt
2025-05-05  8:51   ` [PATCH v4 3/7] builtin/gc: move pruning of worktrees into a separate function Patrick Steinhardt
2025-05-06  7:50     ` Christian Couder
2025-05-07  7:06       ` Patrick Steinhardt
2025-05-05  8:51   ` [PATCH v4 4/7] worktree: expose function to retrieve worktree names Patrick Steinhardt
2025-05-06  8:20     ` Christian Couder
2025-05-06 16:08       ` Eric Sunshine
2025-05-05  8:51   ` [PATCH v4 5/7] builtin/maintenance: introduce "worktree-prune" task Patrick Steinhardt
2025-05-06  7:40     ` Christian Couder
2025-05-07  7:06       ` Patrick Steinhardt
2025-05-05  8:51   ` [PATCH v4 6/7] builtin/gc: move rerere garbage collection into separate function Patrick Steinhardt
2025-05-06  8:39     ` Christian Couder
2025-05-05  8:51   ` [PATCH v4 7/7] builtin/maintenance: introduce "rerere-gc" task Patrick Steinhardt
2025-05-06  9:05   ` [PATCH v4 0/7] builtin/maintenance: implement missing tasks compared to git-gc(1) Christian Couder
2025-05-07  7:21 ` [PATCH v5 0/6] " Patrick Steinhardt
2025-05-07  7:21   ` [PATCH v5 1/6] builtin/gc: fix indentation of `cmd_gc()` parameters Patrick Steinhardt
2025-05-07  7:21   ` [PATCH v5 2/6] builtin/gc: remove global variables where it is trivial to do Patrick Steinhardt
2025-05-07  7:21   ` [PATCH v5 3/6] builtin/gc: move pruning of worktrees into a separate function Patrick Steinhardt
2025-05-07  7:21   ` [PATCH v5 4/6] builtin/maintenance: introduce "worktree-prune" task Patrick Steinhardt
2025-05-07  7:21   ` [PATCH v5 5/6] builtin/gc: move rerere garbage collection into separate function Patrick Steinhardt
2025-05-07  7:21   ` [PATCH v5 6/6] builtin/maintenance: introduce "rerere-gc" task Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c629f845-180f-4ecb-949f-a99f184f812d@gmail.com \
    --to=stolee@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).