Re: [PATCH v3 4/7] worktree: expose function to retrieve worktree names

From

Patrick Steinhardt <ps@pks.im>

Date

May 7, 2025, 07:06 UTC

Message-ID

<aBsGgNsNmy-t5CGK@pks.im>

In-Reply-To

<CAPig+cSDDbhGrym8j=PFKBCUxBQhZPzAHXGvKy-Z6POA4Ju3sw@mail.gmail.com>

On Mon, May 05, 2025 at 04:42:40AM -0400, Eric Sunshine wrote:

> On Fri, May 2, 2025 at 4:44 AM Patrick Steinhardt <ps@pks.im> wrote:
> > Introduce a function that retrieves worktree names as present in
> > ".git/worktrees". This function will be used in a subsequent commit.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> 
> I'm not convinced that this patch or the get_worktree_names() function
> which it adds to worktree.[hc] adds value. Aside from the mere act of
> consulting the directory at repo_git_path(r, "worktrees"), there is
> nothing about the function at all related to worktrees. It doesn't
> make any guarantees, such as only returning entries which at least
> superficially look like worktree-metadata directories, or perform any
> sort of validation. I don't see how this is any better than the caller
> just implementing its own bog-standard opendir() / readdir()-loop /
> closedir() over repo_git_path(r, "worktrees"). Or, if you don't want
> the caller to implement its own readdir()-loop, I wouldn't be
> surprised if we already have a function which does exactly this for a
> provided path, though I haven't checked. If there isn't such a generic
> function, perhaps it makes more sense to add one and call it with
> repo_git_path(r, "worktrees") as its input?

Patrick

diff --git a/builtin/gc.c b/builtin/gc.c
index 82f0fac81a4..eb4469b7858 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -351,11 +351,11 @@ static int maintenance_task_worktree_prune(struct maintenance_run_opts *opts UNU
 
 static int worktree_prune_condition(struct gc_config *cfg)
 {
-	struct strvec worktrees = STRVEC_INIT;
-	struct strbuf reason = STRBUF_INIT;
+	struct strbuf buf = STRBUF_INIT;
+	int should_prune = 0, limit = 1;
 	timestamp_t expiry_date;
-	int should_prune = 0;
-	int limit = 1;
+	struct dirent *d;
+	DIR *dir = NULL;
 
 	git_config_get_int("maintenance.worktree-prune.auto", &limit);
 	if (limit <= 0) {
@@ -363,15 +363,18 @@ static int worktree_prune_condition(struct gc_config *cfg)
 		goto out;
 	}
 
-	if (parse_expiry_date(cfg->prune_worktrees_expire, &expiry_date) ||
-	    get_worktree_names(the_repository, &worktrees) < 0)
+	if (parse_expiry_date(cfg->prune_worktrees_expire, &expiry_date))
+		goto out;
+
+	dir = opendir(repo_git_path_replace(the_repository, &buf, "worktrees"));
+	if (!dir)
 		goto out;
 
-	for (size_t i = 0; i < worktrees.nr; i++) {
+	while ((d = readdir_skip_dot_and_dotdot(dir))) {
 		char *wtpath;
 
-		strbuf_reset(&reason);
-		if (should_prune_worktree(worktrees.v[i], &reason, &wtpath, expiry_date)) {
+		strbuf_reset(&buf);
+		if (should_prune_worktree(d->d_name, &buf, &wtpath, expiry_date)) {
 			limit--;
 
 			if (!limit) {
@@ -384,8 +387,9 @@ static int worktree_prune_condition(struct gc_config *cfg)
 	}
 
 out:
-	strvec_clear(&worktrees);
-	strbuf_release(&reason);
+	if (dir)
+		closedir(dir);
+	strbuf_release(&buf);
 	return should_prune;
 }