Re: [PATCH v2 1/3] worktree: remove "the_repository" from is_current_worktree()

[Patrick Steinhardt <ps@pks.im>]

On Sun, Mar 15, 2026 at 04:18:50PM +0000, Phillip Wood wrote:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
> is_current_worktree() compares the gitdir of the worktree to the gitdir
> of "the_repository" and returns true when they match. To get the gitdir
> of the worktree it calls get_workree_git_dir() which also depends on
> "the_repository". This has the effect that even if "wt->path" matches
> "wt->repo->worktree" is_current_worktree(wt) will return false when
> "wt->repo" is not "the_repository" which is confusing.
> 
> The use of "the_repository" in is_current_wortree() comes from
> replacing get_git_dir() with repo_get_git_dir() in 246deeac951
> (environment: make `get_git_dir()` accept a repository, 2024-09-12). In
> get_worktree_git_dir() it comes from replacing git_common_path() with
> repo_common_path() in 07242c2a5af (path: drop `git_common_path()`
> in favor of `repo_common_path()`, 2025-02-07). In both cases we have
> a repository instance available so use that instead. This means
> that a worktree "wt" is always considered current when "wt->path"
> matches "wt->repo->worktree" and so the worktree returned by
> get_worktree_from_repository() is always considered current.
> 
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  worktree.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/worktree.c b/worktree.c
> index e9ff6e6ef2e..344ad0c031b 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -58,7 +58,7 @@ static void add_head_info(struct worktree *wt)
>  
>  static int is_current_worktree(struct worktree *wt)
>  {
> -	char *git_dir = absolute_pathdup(repo_get_git_dir(the_repository));
> +	char *git_dir = absolute_pathdup(repo_get_git_dir(wt->repo));
>  	char *wt_git_dir = get_worktree_git_dir(wt);
>  	int is_current = !fspathcmp(git_dir, absolute_path(wt_git_dir));
>  	free(wt_git_dir);

Hm, okay.

> @@ -78,7 +78,7 @@ struct worktree *get_worktree_from_repository(struct repository *repo)
>  	wt->is_bare = !repo->worktree;
>  	if (fspathcmp(gitdir, commondir))
>  		wt->id = xstrdup(find_last_dir_sep(gitdir) + 1);
> -	wt->is_current = is_current_worktree(wt);
> +	wt->is_current = true;
>  	add_head_info(wt);
>  
>  	free(gitdir);

I have been staring at this code for longer than I want to admit, and I
still haven't convinced myself that this is not a change in behaviour.
I think what I'm wondering about is what `is_current_worktree()` is
actually intended to do. In other words, what _do_ we consider to be
"current"?

Naively, I would consider the worktree "current" that the Git process
has been invoked in. So if I pass a repo other than `the_repository`, or
if I pass a worktree that is not the one that Git has been started in,
then I would expect the function to return `false`. With that naive
assumption your change would be breaking the existing logic.

But I have no idea whether my assumption is correct or not, as
there is not really any documentation of what the function or of the
`struct worktree::is_current` field. And having a look at a couple of
callers doesn't really make me all the wiser.

It would be great if you could shine some light on this and then also
add a bit of documentation to either the function, the field, or both :)

Thanks!

Patrick