Re: [PATCH 1/2] wt-status: avoid passing NULL worktree

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 17, 2026, 18:47 UTC
Message-ID: <xmqqa4x7cile.fsf@gitster.g>
In-Reply-To: <409871a7d521b76c9eb811d3c49747e04de8defc.1771258688.git.phillip.wood@dunelm.org.uk>

Phillip Wood <phillip.wood123@gmail.com> writes:

Show 6 quoted lines

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> In preparation for removing the repository argument from
> worktree_git_path() add a function to construct a "struct worktree"
> from a "struct repository" and use that to avoid passing a NULL
> worktree to wt_status_check_bisect() and wt_status_check_rebase().

Hmph, I am afraid that

    "Construct a struct worktree from a struct repository"

is not quite sufficient. A repository can have more than one worktrees, so if you give a repository as a parameter, there needs a way for the implementation of this helper function to identify which one of them to construct a struct worktree for, and more importantly for you as the caller to be able to expect which one the implementation would pick, and what that particular worktree among many _means_ to you.

I know that the implementation uses repo->worktree but what does that path mean in the world-view of the worktree API set?

I am guessing that it is what the worktree API calls "current", but if so, perhaps the function should be explained with that word in it, and the function name should also contain that word, no?

Show 9 quoted lines

> +struct worktree *get_worktree_from_repository(struct repository *repo)
> +{
> +	struct worktree *wt = xcalloc(1, sizeof(*wt));
> +	char *gitdir = absolute_pathdup(repo->gitdir);
> +	char *commondir = absolute_pathdup(repo->commondir);
> +
> +	wt->repo = repo;
> +	if (repo->worktree)
> +		wt->path = absolute_pathdup(repo->worktree);

So, if the repository instance knows where the worktree is, we use that to wt->path. Otherwise wt->path is left NULL.

> +	wt->is_bare = !!repo->worktree;

I may be confused but don't we have one ! too many? If we have a worktree directory, "git checkout" would check the files there, and that is not quite a "bare" repository, no?

> +	if (fspathcmp(gitdir, commondir))
> +		wt->id = xstrdup(find_last_dir_sep(commondir) + 1);

OK. So gitdir and commondir would be the same for the primary and for everybody else we'd have "id" as the last directory component of the commondir.

> +	wt->is_current = is_current_worktree(wt);

Oh, so I guessed wrong and this is not about "current" worktree? What does the directory pointed at by repo->worktree mean to the callers of this function? I somehow thought that is_current would be always 1 here,.