[PATCH v3] bisect: fix "reset" when branch is checked out elsewhere

When the user starts a bisection in a worktree, we save the branch currently checked out in that worktree (BISECT_START) to allow the user to go back to that branch later on.

From that moment and until the bisection ends, the branch is no longer
checked out in that worktree, but we give it the same consideration as
if it was checked out, which means that we prevent:
 - deleting that branch and
 - checking out that branch in another worktree, unless the user force
   the ckeck out.

"bisect" was introduced as a helper script in 8cc6a08 ([PATCH] Making it easier to find which change introduced a bug, 2005-07-30), and originally based its functioning in Git builtin commands. Although "bisect" is now itself a builtin command, it still spawns Git sub-processes to use other builtin commands, like "checkout".

Since 1d0fa89 (checkout: add --ignore-other-worktrees, 2015-01-03) we have a safety valve in "checkout" and "switch" commands to prevent the user from normally checking out simultaneously the same branch in multiple worktrees.

If the user, using "--ignore-other-worktrees", checks out a branch before or while bisecting that same branch in another worktree, we may fail when the user ends the bisection using "git bisect reset":

   $ git worktree add work ../work/
   $ git checkout --ignore-other-worktrees work
   $ git -C ../work bisect start HEAD HEAD~3
   ... the user inspects some commits ...
   $ git -C ../work bisect reset
   fatal: 'work' is already checked out at ...
   error: could not check out original HEAD 'work'. Try 'git bisect reset <commit>'.

Thanks to the special consideration we give to the branch while being bisected, and the safety valve introduced in 1d0fa89, we can assume the user is aware and responsible of the "multiple checked out branch" situation. This makes sensible to allow the user to go back to the original branch using "git bisect reset" and avoid him a somewhat less intuitive sequence like: "git bisect reset work~0 && git checkout --ignore-other-worktree work".

Let's teach "bisect" to use the "--ignore-other-worktrees" when the user wants to end the bisection.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
---

Changes since v2

 - Reworded the message to be more descriptive.
 - Using "||:" to chain commands in the test_when_finished block,
   suggested by Eric.

This series started because of a bug, being fixed in another series, allowed "git bisect reset" to work in certain cases where multiple branches were checked out. Once the bug is fixed, "git bisect reset" will no longer work in those scenarios. Which is not bad, but taking into account the different scenarios and the fact that some users may rely on this bug as a feature, I think that the current patch proposes a sensible and convenient change for the user.

Range-diff against v2:
1:  0fe0bc70dd ! 1:  3b2ac1aa8f bisect: fix "reset" when branch is checked out elsewhere
    @@ Metadata
      ## Commit message ##
         bisect: fix "reset" when branch is checked out elsewhere
     
    -    Since 1d0fa89 (checkout: add --ignore-other-wortrees, 2015-01-03) we
    -    have a safety valve in checkout/switch to prevent the same branch from
    -    being checked out simultaneously in multiple worktrees.
    +    When the user starts a bisection in a worktree, we save the branch
    +    currently checked out in that worktree (BISECT_START) to allow the user
    +    to go back to that branch later on.
     
    -    If a branch is bisected in a worktree while also being checked out in
    -    another worktree; when the bisection is finished, checking out the
    -    branch back in the current worktree may fail.
    +    From that moment and until the bisection ends, the branch is no longer
    +    checked out in that worktree, but we give it the same consideration as
    +    if it was checked out, which means that we prevent:
    +     - deleting that branch and
    +     - checking out that branch in another worktree, unless the user force
    +       the ckeck out.
     
    -    Let's teach bisect to use the "--ignore-other-worktrees" flag.
    +    "bisect" was introduced as a helper script in 8cc6a08 ([PATCH] Making it
    +    easier to find which change introduced a bug, 2005-07-30), and
    +    originally based its functioning in Git builtin commands.  Although
    +    "bisect" is now itself a builtin command, it still spawns Git
    +    sub-processes to use other builtin commands, like "checkout".
    +
    +    Since 1d0fa89 (checkout: add --ignore-other-worktrees, 2015-01-03) we
    +    have a safety valve in "checkout" and "switch" commands to prevent the
    +    user from normally checking out simultaneously the same branch in
    +    multiple worktrees.
    +
    +    If the user, using "--ignore-other-worktrees", checks out a branch
    +    before or while bisecting that same branch in another worktree, we may
    +    fail when the user ends the bisection using "git bisect reset":
    +
    +       $ git worktree add work ../work/
    +       $ git checkout --ignore-other-worktrees work
    +       $ git -C ../work bisect start HEAD HEAD~3
    +       ... the user inspects some commits ...
    +       $ git -C ../work bisect reset
    +       fatal: 'work' is already checked out at ...
    +       error: could not check out original HEAD 'work'. Try 'git bisect reset <commit>'.
    +
    +    Thanks to the special consideration we give to the branch while being
    +    bisected, and the safety valve introduced in 1d0fa89, we can assume the
    +    user is aware and responsible of the "multiple checked out branch"
    +    situation.  This makes sensible to allow the user to go back to the
    +    original branch using "git bisect reset" and avoid him a somewhat less
    +    intuitive sequence like: "git bisect reset work~0 && git checkout
    +    --ignore-other-worktree work".
    +
    +    Let's teach "bisect" to use the "--ignore-other-worktrees" when the user
    +    wants to end the bisection.
     
         Signed-off-by: Rubén Justo <rjusto@gmail.com>
     
    @@ builtin/bisect.c: static int bisect_reset(const char *commit)
     -		strvec_pushl(&cmd.args, "checkout", branch.buf, "--", NULL);
     +		strvec_pushl(&cmd.args, "checkout", NULL);
     +		if (!commit)
    -+			strvec_pushl(&cmd.args, "--ignore-other-worktrees", NULL);
    ++			strvec_pushl(&cmd.args, "--ignore-other-worktrees",
    ++				     NULL);
     +		strvec_pushl(&cmd.args, branch.buf, "--", NULL);
      		if (run_command(&cmd)) {
      			error(_("could not check out original"
    @@ t/t6030-bisect-porcelain.sh: test_expect_success 'bisect start without -- takes
     +		cmp branch.expect branch.output
     +	} &&
     +	test_when_finished "
    -+		git worktree remove wt1 &&
    -+		git worktree remove wt2 &&
    -+		git branch -d shared
    ++		git worktree remove wt1 ||:
    ++		git worktree remove wt2 ||:
    ++		git branch -d shared ||:
     +	" &&
     +	git worktree add wt1 -b shared &&
     +	git worktree add wt2 -f shared &&

 builtin/bisect.c            |  6 +++++-
 t/t6030-bisect-porcelain.sh | 23 +++++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)