Re: [PATCH] worktree: teach find_shared_symref to ignore current worktree

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 17, 2023, 23:27 UTC
Message-ID: <xmqqilh491y6.fsf@gitster.g>
In-Reply-To: <eeb0c778-af0a-235c-f009-bca3abafdb15@gmail.com>

Rubén Justo <rjusto@gmail.com> writes:

Show 8 quoted lines

> We prevent some operations from being executed on a branch checked out
> in a worktree other than the current one.  An example of this was
> introduced in b5cabb4 (rebase: refuse to switch to branch already
> checked out elsewhere, 2020-02-23).
>
> "find_shared_symref()" is sometimes used to find the worktree in which a
> branch is checked out.  It performs its search starting with the current
> worktree.

"starting with the current" may be a correct statement of the fact, but it is totally unclear what the relevance it has to the problem being solved. Rather, it is unclear what problem you are solving.

Is it

 - We search through the worktrees, starting with the current one,
   and stop at the first one found.

 - If the current branch the the current worktree is checked out in
   a different worktree, we get the current worktree back.

 - There are callers that want to know ONLY about other worktrees;
   they check the returned value and when they see it is the current
   one, they happily ignores the fact that it might be checked out
   elsewhere as well.

Show 10 quoted lines

> As we allow to have the same branch checked out in multiple worktrees
> simultaneously...
>
> 	$ git worktree add foo
> 	$ git worktree add -f bar foo
> 	$ git checkout --ignore-other-worktrees foo
>
> ... if the branch checked out in the current worktree is also checked
> out in another worktree, with "find_shared_symref()" we will not notice
> this "other" working tree.

It is somewhat disturbing that your solution only needs to "ignore" the current one. Whatever problem you are seeing by the current code not ignoring the current worktree, wouldn't we have a similar problem if two non-current worktrees checked out the same branch? Would it not be a problem because any non-current worktree returned by the function triggers the "already checked-out" safety mechanism?