From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Willem Verstraeten <willem.verstraeten@gmail.com>,
phillip.wood@dunelm.org.uk, git@vger.kernel.org,
Jeff King <peff@peff.net>
Subject: Re: [PATCH 2/2] checkout: forbid "-B <branch>" from touching a branch used elsewhere
Date: Sat, 09 Dec 2023 02:13:18 +0900 [thread overview]
Message-ID: <xmqqsf4c39e9.fsf@gitster.g> (raw)
In-Reply-To: <CAPig+cSGF+vQrnD0f99cbdpQOOC7X6ULa9tFe+FwVrG0SF4PGg@mail.gmail.com> (Eric Sunshine's message of "Mon, 4 Dec 2023 16:06:50 -0500")
Eric Sunshine <sunshine@sunshineco.com> writes:
> Needs review and documentation updates.
>
> I'm not sure if the "Needs review" comment is still applicable since
> the patch did get some review comments, however, the mentioned
> documentation update is probably still needed for this series to
> graduate.
Thanks. I think "-B" being defined as "branch -f <branch>" followed
by "checkout <branch>" makes it technically unnecessary to add any
new documentation (because "checkout <branch>" will refuse, so it
naturally follows that "checkout -B <branch>" should), but giving
the failure mode a bit more explicit mention would be more helpful
to readers.
Here is to illustrate what I have in mind. The mention of the
"transactional" was already in the documentation for the "checkout"
back when switch was described at d787d311 (checkout: split part of
it to new command 'switch', 2019-03-29), but somehow was left out in
the documentation of the "switch". While it is not incorrect to say
that it is a convenient short-cut, it is more important to say what
happens when one of them fails, so I am tempted to port that
description over to the "switch" command, and give the "used elsewhere"
as a sample failure mode.
The test has been also enhanced to check the "transactional" nature.
Documentation/git-checkout.txt | 4 +++-
Documentation/git-switch.txt | 9 +++++++--
t/t2400-worktree-add.sh | 18 ++++++++++++++++--
3 files changed, 26 insertions(+), 5 deletions(-)
diff --git c/Documentation/git-checkout.txt w/Documentation/git-checkout.txt
index 240c54639e..55a50b5b23 100644
--- c/Documentation/git-checkout.txt
+++ w/Documentation/git-checkout.txt
@@ -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>::
diff --git c/Documentation/git-switch.txt w/Documentation/git-switch.txt
index c60fc9c138..6137421ede 100644
--- c/Documentation/git-switch.txt
+++ w/Documentation/git-switch.txt
@@ -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>::
diff --git c/t/t2400-worktree-add.sh w/t/t2400-worktree-add.sh
index bbcb2d3419..5d5064e63d 100755
--- c/t/t2400-worktree-add.sh
+++ w/t/t2400-worktree-add.sh
@@ -129,8 +129,22 @@ test_expect_success 'die the same branch is already checked out' '
test_expect_success 'refuse to reset a branch in use elsewhere' '
(
cd here &&
- test_must_fail git checkout -B newmain 2>actual &&
- grep "already used by worktree at" actual
+
+ # 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
)
'
next prev parent reply other threads:[~2023-12-08 17:13 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-22 19:08 git checkout -B <branch> lets you checkout a branch that is already checked out in another worktree Inbox Willem Verstraeten
2023-11-23 1:28 ` Junio C Hamano
2023-11-23 5:58 ` Junio C Hamano
2023-11-23 6:00 ` [PATCH 2/2] checkout: forbid "-B <branch>" from touching a branch used elsewhere Junio C Hamano
2023-11-23 16:33 ` Phillip Wood
2023-11-23 17:09 ` Eric Sunshine
2023-11-24 1:19 ` Junio C Hamano
2023-11-27 1:51 ` Junio C Hamano
2023-11-27 21:31 ` Jeff King
2023-11-30 15:22 ` Phillip Wood
2023-12-04 12:20 ` Willem Verstraeten
2023-12-04 21:06 ` Eric Sunshine
2023-12-08 17:13 ` Junio C Hamano [this message]
2024-01-30 12:37 ` Willem Verstraeten
2024-01-30 22:30 ` Junio C Hamano
2023-11-23 22:03 ` git checkout -B <branch> lets you checkout a branch that is already checked out in another worktree Inbox Andy Koppe
2023-11-23 12:12 ` Willem Verstraeten
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqsf4c39e9.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=phillip.wood@dunelm.org.uk \
--cc=sunshine@sunshineco.com \
--cc=willem.verstraeten@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.