Re: [PATCH v4] checkout/switch: disallow checking out same branch in multiple worktrees

On 27-ene-2023 14:46:45, Phillip Wood wrote:

Hi Carlo

On 20/01/2023 22:12, Carlo Arenas wrote:

quoted

On Fri, Jan 20, 2023 at 7:08 AM Phillip Wood [off-list ref] wrote:

quoted

On 20/01/2023 11:35, Carlo Marcelo Arenas Belón wrote:

quoted

Commands `git switch -C` and `git checkout -B` neglect to check whether
the provided branch is already checked out in some other worktree, as
shown by the following:

    $ git worktree list
    .../foo    beefb00f [main]
    $ git worktree add ../other
    Preparing worktree (new branch 'other')
    HEAD is now at beefb00f first
    $ cd ../other
    $ git switch -C main
    Switched to and reset branch 'main'
    $ git worktree list
    .../foo    beefb00f [main]
    .../other  beefb00f [main]

Fix this problem by teaching `git switch -C` and `git checkout -B` to
check whether the branch in question is already checked out elsewhere.

Unlike what it is done for `git switch` and `git checkout`, that have
an historical exception to ignore other worktrees if the branch to
check is the current one (as required when called as part of other
tools), the logic implemented is more strict and will require the user
to invoke the command with `--ignore-other-worktrees` to explicitly
indicate they want the risky behaviour.

This matches the current behaviour of `git branch -f` and is safer; for
more details see the tests in t2400.

As I said before, it would be much easier for everyone else to
understand the changes if you wrote out what they were rather than
saying "look at the tests". I do appreciate the improved test
descriptions though - thanks for that.

Apologies on that, I tried to come up with something that would
describe the change of behaviour other than the paragraph above and
couldn't come out with a better explanation except reading the tests
(which I know is complicated by the fact they are interlinked).

The behaviour I am changing is also not documented (other than by the
test) and might have been added as a quirk to keep the scripted rebase
and bisect going when confronted with branches that were checked out
in multiple worktrees, and as such might had not be intended for
`switch`, and might not be needed anymore either.

Using`checkout` for simplicity, but also applies to `switch`,

   % git worktree list
   .../base  6a45aba [main]
   % git worktree add -f ../other main
   Preparing worktree (checking out 'main')
   HEAD is now at 6a45aba init
   % cd ../other
   % git checkout main
   Already on 'main'
   % git checkout -B main
   fatal: 'main' is already checked out at '.../base'

Thanks for explaining that. If there is no <start-point> given we don't
reset the branch so it seems a bit harsh to error out here.

We say in the documentation:

   +
   If `-B` is given, `<new-branch>` is created if it doesn't exist; otherwise, it
   is reset. This is the transactional equivalent of
   +
   ------------
   $ git branch -f <branch> [<start-point>]
   $ git checkout <branch>
   ------------
   +

and, since 55c4a67307 (Prevent force-updating of the current branch,
2011-08-20), we return with error on "git branch -f <current-branch> [...]".

Therefore, when the current branch is checked out in multiple worktrees, it
seems consequent to error on "git checkout -B <current_branch> [...]".

But we want to allow "git checkout -B <current_branch>", without <start-point>,
as a way to say "git reset --keep", see: 39bd6f7261 (Allow checkout -B
<current-branch> to update the current branch, 2011-11-26).

Your suggestion not to error is not only reasonable, but also seems desirable.

Thanks.