Merge branch 'jc/checkout-B-branch-in-use'

"git checkout -B <branch> [<start-point>]" allowed a branch that is
in use in another worktree to be updated and checked out, which
might be a bit unexpected.  The rule has been tightened, which is a
breaking change.  "--ignore-other-worktrees" option is required to
unbreak you, if you are used to the current behaviour that "-B"
overrides the safety.

* jc/checkout-B-branch-in-use:
  checkout: forbid "-B <branch>" from touching a branch used elsewhere
  checkout: refactor die_if_checked_out() caller
maint
Junio C Hamano 2023-12-27 14:52:24 -08:00
commit f09e74175d
5 changed files with 63 additions and 11 deletions

View File

@ -63,7 +63,9 @@ $ git checkout <branch>
------------
+
that is to say, the branch is not reset/created unless "git checkout" is
successful.
successful (e.g., when the branch is in use in another worktree, not
just the current branch stays the same, but the branch is not reset to
the start-point, either).

'git checkout' --detach [<branch>]::
'git checkout' [--detach] <commit>::

View File

@ -59,13 +59,18 @@ out at most one of `A` and `B`, in which case it defaults to `HEAD`.
-c <new-branch>::
--create <new-branch>::
Create a new branch named `<new-branch>` starting at
`<start-point>` before switching to the branch. This is a
convenient shortcut for:
`<start-point>` before switching to the branch. This is the
transactional equivalent of
+
------------
$ git branch <new-branch>
$ git switch <new-branch>
------------
+
that is to say, the branch is not reset/created unless "git switch" is
successful (e.g., when the branch is in use in another worktree, not
just the current branch stays the same, but the branch is not reset to
the start-point, either).

-C <new-branch>::
--force-create <new-branch>::

View File

@ -1518,6 +1518,26 @@ static void die_if_some_operation_in_progress(void)
wt_status_state_free_buffers(&state);
}

/*
* die if attempting to checkout an existing branch that is in use
* in another worktree, unless ignore-other-wortrees option is given.
* The check is bypassed when the branch is already the current one,
* as it will not make things any worse.
*/
static void die_if_switching_to_a_branch_in_use(struct checkout_opts *opts,
const char *full_ref)
{
int flags;
char *head_ref;

if (opts->ignore_other_worktrees)
return;
head_ref = resolve_refdup("HEAD", 0, NULL, &flags);
if (head_ref && (!(flags & REF_ISSYMREF) || strcmp(head_ref, full_ref)))
die_if_checked_out(full_ref, 1);
free(head_ref);
}

static int checkout_branch(struct checkout_opts *opts,
struct branch_info *new_branch_info)
{
@ -1578,14 +1598,15 @@ static int checkout_branch(struct checkout_opts *opts,
if (!opts->can_switch_when_in_progress)
die_if_some_operation_in_progress();

if (new_branch_info->path && !opts->force_detach && !opts->new_branch &&
!opts->ignore_other_worktrees) {
int flag;
char *head_ref = resolve_refdup("HEAD", 0, NULL, &flag);
if (head_ref &&
(!(flag & REF_ISSYMREF) || strcmp(head_ref, new_branch_info->path)))
die_if_checked_out(new_branch_info->path, 1);
free(head_ref);
/* "git checkout <branch>" */
if (new_branch_info->path && !opts->force_detach && !opts->new_branch)
die_if_switching_to_a_branch_in_use(opts, new_branch_info->path);

/* "git checkout -B <branch>" */
if (opts->new_branch_force) {
char *full_ref = xstrfmt("refs/heads/%s", opts->new_branch);
die_if_switching_to_a_branch_in_use(opts, full_ref);
free(full_ref);
}

if (!new_branch_info->commit && opts->new_branch) {

View File

@ -170,8 +170,10 @@ test_expect_success 'switch back when temporarily detached and checked out elsew
# we test in both worktrees to ensure that works
# as expected with "first" and "next" worktrees
test_must_fail git -C wt1 switch shared &&
test_must_fail git -C wt1 switch -C shared &&
git -C wt1 switch --ignore-other-worktrees shared &&
test_must_fail git -C wt2 switch shared &&
test_must_fail git -C wt2 switch -C shared &&
git -C wt2 switch --ignore-other-worktrees shared
'


View File

@ -126,6 +126,28 @@ test_expect_success 'die the same branch is already checked out' '
)
'

test_expect_success 'refuse to reset a branch in use elsewhere' '
(
cd here &&

# we know we are on detached HEAD but just in case ...
git checkout --detach HEAD &&
git rev-parse --verify HEAD >old.head &&

git rev-parse --verify refs/heads/newmain >old.branch &&
test_must_fail git checkout -B newmain 2>error &&
git rev-parse --verify refs/heads/newmain >new.branch &&
git rev-parse --verify HEAD >new.head &&

grep "already used by worktree at" error &&
test_cmp old.branch new.branch &&
test_cmp old.head new.head &&

# and we must be still on the same detached HEAD state
test_must_fail git symbolic-ref HEAD
)
'

test_expect_success SYMLINKS 'die the same branch is already checked out (symlink)' '
head=$(git -C there rev-parse --git-path HEAD) &&
ref=$(git -C there symbolic-ref HEAD) &&