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

[Christian Couder <christian.couder@gmail.com>]

On Mon, May 5, 2025 at 10:52 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>
> ---
>  builtin/worktree.c | 25 ++++++++++++-------------
>  worktree.c         | 30 ++++++++++++++++++++++++++++++
>  worktree.h         |  8 ++++++++
>  3 files changed, 50 insertions(+), 13 deletions(-)
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 87ccd47794c..9b00dbf1265 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -211,27 +211,24 @@ static void prune_dups(struct string_list *l)
>
>  static void prune_worktrees(void)
>  {
> -       struct strbuf reason = STRBUF_INIT;
>         struct strbuf main_path = STRBUF_INIT;
>         struct string_list kept = STRING_LIST_INIT_DUP;
> -       char *path;
> -       DIR *dir;
> -       struct dirent *d;
> +       struct strvec worktrees = STRVEC_INIT;
> +       struct strbuf reason = STRBUF_INIT;
>
> -       path = repo_git_path(the_repository, "worktrees");
> -       dir = opendir(path);
> -       free(path);
> -       if (!dir)
> +       if (get_worktree_names(the_repository, &worktrees) < 0 ||
> +           !worktrees.nr)
>                 return;
> -       while ((d = readdir_skip_dot_and_dotdot(dir)) != NULL) {
> +
> +       for (size_t i = 0; i < worktrees.nr; i++) {
>                 char *path;
> +
>                 strbuf_reset(&reason);
> -               if (should_prune_worktree(d->d_name, &reason, &path, expire))
> -                       prune_worktree(d->d_name, reason.buf);
> +               if (should_prune_worktree(worktrees.v[i], &reason, &path, expire))
> +                       prune_worktree(worktrees.v[i], reason.buf);
>                 else if (path)
> -                       string_list_append_nodup(&kept, path)->util = xstrdup(d->d_name);
> +                       string_list_append_nodup(&kept, path)->util = xstrdup(worktrees.v[i]);
>         }
> -       closedir(dir);
>
>         strbuf_add_absolute_path(&main_path, repo_get_common_dir(the_repository));
>         /* massage main worktree absolute path to match 'gitdir' content */
> @@ -242,6 +239,8 @@ static void prune_worktrees(void)
>
>         if (!show_only)
>                 delete_worktrees_dir_if_empty();
> +
> +       strvec_clear(&worktrees);
>         strbuf_release(&reason);
>  }
>
> diff --git a/worktree.c b/worktree.c
> index c34b9eb74e5..947b7a82209 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -988,6 +988,36 @@ int should_prune_worktree(const char *id, struct strbuf *reason, char **wtpath,
>         return rc;
>  }
>
> +int get_worktree_names(struct repository *repo, struct strvec *out)
> +{
> +       char *worktrees_dir;
> +       struct dirent *d;
> +       DIR *dir;
> +       int ret;
> +
> +       worktrees_dir = repo_git_path(repo, "worktrees");
> +       dir = opendir(worktrees_dir);
> +       if (!dir) {
> +               if (errno == ENOENT) {
> +                       ret = 0;
> +                       goto out;
> +               }
> +
> +               ret = -1;
> +               goto out;
> +       }
> +
> +       while ((d = readdir_skip_dot_and_dotdot(dir)) != NULL)
> +               strvec_push(out, d->d_name);
> +
> +       ret = 0;
> +out:
> +       if (dir)
> +               closedir(dir);
> +       free(worktrees_dir);
> +       return ret;
> +}

Nit: this function seems to use "goto out" a bit too much for me. What
about something like:

int get_worktree_names(struct repository *repo, struct strvec *out)
{
    int ret = 0;
    char *worktrees_dir = repo_git_path(repo, "worktrees");
    DIR *dir = opendir(worktrees_dir);

    if (dir) {
        struct dirent *d;
        while ((d = readdir_skip_dot_and_dotdot(dir)) != NULL)
            strvec_push(out, d->d_name);
        closedir(dir);
    } else {
        if (errno != ENOENT)
            ret = -1;
    }

    free(worktrees_dir);
    return ret;
}

?