{"thread":{"id":"60547","subject":"git checkout -B lets you checkout a branch that is already checked out in another worktree Inbox","startedAt":"2023-11-22T19:08:49Z","lastAt":"2024-01-30T22:30:17Z","messageCount":17,"participants":["Willem Verstraeten","Junio C Hamano","Phillip Wood","Eric Sunshine","Andy Koppe","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"485050","messageId":"CAGX9RpFMCVLQV7RbK2u9AabusvkZD+RZNv_UD=R00cSUrjutBg@mail.gmail.com","threadId":"60547","inReplyTo":null,"subject":"git checkout -B lets you checkout a branch that is already checked out in another worktree Inbox","fromName":"Willem Verstraeten","fromEmail":"willem.verstraeten@gmail.com","sentAt":"2023-11-22T19:08:36Z","receivedAt":"2023-11-22T19:08:49Z","isPatch":false,"sender":{"key":"willem.verstraeten@gmail.com","avatar":null},"body":"# What did you do before the bug happened? (Steps to reproduce your issue)\n\nClone a repo\nCreate an additional worktree for that clone\nUse `git checkout -B branch-of-primary-clone ...` to checkout the\nbranch that is already checked out in the primary clone\n\nFor example, with the pathfinder repo on GitHub:\n\n git clone https://github.com/servo/pathfinder.git primary\n cd primary\n git worktree add -b metal ../secondary origin/metal\n cd ../secondary\n git checkout -b main #reports a fatal error, as expected\n git checkout -f main origin/main #also reports a fatal error, as expected\n git checkout -B main origin/main # ----> this succeeds, which is\nunexpected <----\n\n# What did you expect to happen? (Expected behavior)\n\nI expected a fatal error stating that the branch could not be checked\nout since it was already checked out in my primary worktree\n\nIn `git checkout --help`, it is documented that `git checkout -B` is\nthe atomic equivalent of `git branch -f ; git\ncheckout ` :\n\n> If -B is given, is created if it doesn’t exist; otherwise, it is reset. This is the transactional equivalent of\n>\n> $ git branch -f []\n> $ git checkout \n\n# What happened instead? (Actual behavior)\n\nThe branch was checked out in the secondary worktree. If I then work\nand make commits in this secondary worktree, the status of my primary\nworktree gets clobbered as well.\n\nWhat's different between what you expected and what actually happened?\n\nThe checkout in the secondary worktree is allowed, but it shouldn't be\n\n\n[System Info]\ngit version:\ngit version 2.41.0\ncpu: arm64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nfeature: fsmonitor--daemon\nuname: Darwin 22.6.0 Darwin Kernel Version 22.6.0: Wed Jul 5 22:21:53\nPDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6020 arm64\ncompiler info: clang: 14.0.3 (clang-1403.0.22.14.1)\nlibc info: no libc information available\n$SHELL (typically, interactive shell): /bin/zsh\n\n\n[Enabled Hooks]\n"},{"id":"485056","messageId":"xmqqjzq9cl70.fsf@gitster.g","threadId":"60547","inReplyTo":"CAGX9RpFMCVLQV7RbK2u9AabusvkZD+RZNv_UD=R00cSUrjutBg@mail.gmail.com","subject":"Re: git checkout -B lets you checkout a branch that is already checked out in another worktree Inbox","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-23T01:28:19Z","receivedAt":"2023-11-23T01:28:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Willem Verstraeten writes:\n\n> git checkout -b main #reports a fatal error, as expected\n\nThis is expected because \"main\" already exists, not because \"main\"\nis checked out elsewhere.\n\n> git checkout -f main origin/main #also reports a fatal error, as expected\n\nThis is expected because origin/main is taken as pathspec, and it is\na request to checkout the paths that match the pathspec out of the\nnamed tree-ish (i.e., \"main\"), even when these paths have local\nchanges, but you do not have paths that match \"origin/main\". The\nfailure is not because \"main\" is checked out elsewhere.\n\nA slight variant of the command\n\n git checkout -f -b main origin/main\n\nstill fails for the same reason as the first of your examples above.\n\nIt is a tangent, but I suspect this failure may be a bit unexpected.\nIn this example, \"-f\"orce could be overriding the usual failure from\n\"-b\" to switch to a branch that already exists, but that is what\n\"-B\" does, and \"-f -b\" does not work as a synonym for \"-B\".\n\nIn any case, these example you marked \"fail as expected\" do fail as\nexpected, but they fail for reasons that have nothing to do with the\nprotection of branches that are used in other worktrees.\n\n> git checkout -B main origin/main # ----> this succeeds, which is\n> unexpected <----\n\nI agree this may be undesirable.\n\nBut it makes sort of sense, because \"-B\" is a forced form of \"-b\"\n(i.e., it tells git: even when \"-b\" would fail, take necessary\nmeasures to make it work), and we can view that it is part of\n\"forcing\" to override the protection over branches that are used\nelsewhere.\n\nI guess we could change the behaviour so that\n\n git checkout -B []\n\nfails when is an existing branch that is in use in another\nworktree, and allow \"-f\" to be used to override the safety, i.e.,\n\n git checkout -f -B []\n\nwould allow the to be repointed to (or HEAD)\neven when it is used elsewhere.\n\nThoughts, whether they agree or disagree with what I just said, by\nother experienced contributors are very much welcome, before I can\nsay \"patches welcome\" ;-).\n\nWillem, thanks for raising the issue.\n\n\n\n"},{"id":"485059","messageId":"xmqqv89tau3r.fsf@gitster.g","threadId":"60547","inReplyTo":"xmqqjzq9cl70.fsf@gitster.g","subject":"Re: git checkout -B lets you checkout a branch that is already checked out in another worktree Inbox","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-23T05:58:48Z","receivedAt":"2023-11-23T05:58:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I guess we could change the behaviour so that\n>\n> git checkout -B []\n>\n> fails when is an existing branch that is in use in another\n> worktree, and allow \"-f\" to be used to override the safety, i.e.,\n>\n> git checkout -f -B []\n>\n> would allow the to be repointed to (or HEAD)\n> even when it is used elsewhere.\n\nIt turns out that for some reason \"-f\" is not how we decided to\noverride this one---there is \"--ignore-other-worktrees\" option.\n\nI'll attach the first step (preparatory refactoring) to this message\nbelow, and follow it up with the second step to implement and test\nthe change.\n\n--- >8 ---\nFrom: Junio C Hamano \nDate: Thu, 23 Nov 2023 14:11:41 +0900\nSubject: [PATCH 1/2] checkout: refactor die_if_checked_out() caller\n\nThere is a bit dense logic to make a call to \"die_if_checked_out()\"\nwhile trying to check out a branch. Extract it into a helper\nfunction and give it a bit of comment to describe what is going on.\n\nThe most important part of the refactoring is the separation of the\nguarding logic before making the call to die_if_checked_out() into\nthe caller specific part (e.g., the logic that decides that the\ncaller is trying to check out an existing branch) and the bypass due\nto the \"--ignore-other-worktrees\" option. The latter will be common\nno matter how the current or future callers decides they need this\nprotection.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/checkout.c | 32 +++++++++++++++++++++++---------\n 1 file changed, 23 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex f02434bc15..b4ab972c5a 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1516,6 +1516,26 @@ static void die_if_some_operation_in_progress(void)\n \twt_status_state_free_buffers(&state);\n }\n \n+/*\n+ * die if attempting to checkout an existing branch that is in use\n+ * in another worktree, unless ignore-other-wortrees option is given.\n+ * The check is bypassed when the branch is already the current one,\n+ * as it will not make things any worse.\n+ */\n+static void die_if_switching_to_a_branch_in_use(struct checkout_opts *opts,\n+\t\t\t\t\t\tconst char *full_ref)\n+{\n+\tint flags;\n+\tchar *head_ref;\n+\n+\tif (opts->ignore_other_worktrees)\n+\t\treturn;\n+\thead_ref = resolve_refdup(\"HEAD\", 0, NULL, &flags);\n+\tif (head_ref && (!(flags & REF_ISSYMREF) || strcmp(head_ref, full_ref)))\n+\t\tdie_if_checked_out(full_ref, 1);\n+\tfree(head_ref);\n+}\n+\n static int checkout_branch(struct checkout_opts *opts,\n \t\t\t struct branch_info *new_branch_info)\n {\n@@ -1576,15 +1596,9 @@ static int checkout_branch(struct checkout_opts *opts,\n \tif (!opts->can_switch_when_in_progress)\n \t\tdie_if_some_operation_in_progress();\n \n-\tif (new_branch_info->path && !opts->force_detach && !opts->new_branch &&\n-\t !opts->ignore_other_worktrees) {\n-\t\tint flag;\n-\t\tchar *head_ref = resolve_refdup(\"HEAD\", 0, NULL, &flag);\n-\t\tif (head_ref &&\n-\t\t (!(flag & REF_ISSYMREF) || strcmp(head_ref, new_branch_info->path)))\n-\t\t\tdie_if_checked_out(new_branch_info->path, 1);\n-\t\tfree(head_ref);\n-\t}\n+\t/* \"git checkout \" */\n+\tif (new_branch_info->path && !opts->force_detach && !opts->new_branch)\n+\t\tdie_if_switching_to_a_branch_in_use(opts, new_branch_info->path);\n \n \tif (!new_branch_info->commit && opts->new_branch) {\n \t\tstruct object_id rev;\n-- \n2.43.0\n\n"},{"id":"485060","messageId":"xmqqpm01au0w.fsf_-_@gitster.g","threadId":"60547","inReplyTo":"xmqqv89tau3r.fsf@gitster.g","subject":"[PATCH 2/2] checkout: forbid \"-B \" from touching a branch used elsewhere","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-23T06:00:31Z","receivedAt":"2023-11-23T06:00:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git checkout -B []\", being a \"forced\" version\nof \"-b\", switches to the , after optionally resetting its\ntip to the , even if the is in use in another\nworktree, which is somewhat unexpected.\n\nProtect the using the same logic that forbids \"git checkout\n\" from touching a branch that is in use elsewhere.\n\nThis is a breaking change that may deserve backward compatibliity\nwarning in the Release Notes. The \"--ignore-other-worktrees\" option\ncan be used as an escape hatch if the finger memory of existing\nusers depend on the current behaviour of \"-B\".\n\nReported-by: Willem Verstraeten \nSigned-off-by: Junio C Hamano \n---\n\n * The documentation might also need updates, but I didn't look at.\n\n builtin/checkout.c | 7 +++++++\n t/t2060-switch.sh | 2 ++\n t/t2400-worktree-add.sh | 8 ++++++++\n 3 files changed, 17 insertions(+)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex b4ab972c5a..8a8ad23e98 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1600,6 +1600,13 @@ static int checkout_branch(struct checkout_opts *opts,\n \tif (new_branch_info->path && !opts->force_detach && !opts->new_branch)\n \t\tdie_if_switching_to_a_branch_in_use(opts, new_branch_info->path);\n \n+\t/* \"git checkout -B \" */\n+\tif (opts->new_branch_force) {\n+\t\tchar *full_ref = xstrfmt(\"refs/heads/%s\", opts->new_branch);\n+\t\tdie_if_switching_to_a_branch_in_use(opts, full_ref);\n+\t\tfree(full_ref);\n+\t}\n+\n \tif (!new_branch_info->commit && opts->new_branch) {\n \t\tstruct object_id rev;\n \t\tint flag;\ndiff --git a/t/t2060-switch.sh b/t/t2060-switch.sh\nindex e247a4735b..c91c4db936 100755\n--- a/t/t2060-switch.sh\n+++ b/t/t2060-switch.sh\n@@ -170,8 +170,10 @@ test_expect_success 'switch back when temporarily detached and checked out elsew\n \t# we test in both worktrees to ensure that works\n \t# as expected with \"first\" and \"next\" worktrees\n \ttest_must_fail git -C wt1 switch shared &&\n+\ttest_must_fail git -C wt1 switch -C shared &&\n \tgit -C wt1 switch --ignore-other-worktrees shared &&\n \ttest_must_fail git -C wt2 switch shared &&\n+\ttest_must_fail git -C wt2 switch -C shared &&\n \tgit -C wt2 switch --ignore-other-worktrees shared\n '\n \ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex df4aff7825..bbcb2d3419 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -126,6 +126,14 @@ test_expect_success 'die the same branch is already checked out' '\n \t)\n '\n \n+test_expect_success 'refuse to reset a branch in use elsewhere' '\n+\t(\n+\t\tcd here &&\n+\t\ttest_must_fail git checkout -B newmain 2>actual &&\n+\t\tgrep \"already used by worktree at\" actual\n+\t)\n+'\n+\n test_expect_success SYMLINKS 'die the same branch is already checked out (symlink)' '\n \thead=$(git -C there rev-parse --git-path HEAD) &&\n \tref=$(git -C there symbolic-ref HEAD) &&\n-- \n2.43.0\n\n"},{"id":"485063","messageId":"CAGX9RpHOOu71LJa_Z_29b6hwy7s_+oxGv0U1kdt+CJ_Ztg1iSw@mail.gmail.com","threadId":"60547","inReplyTo":"xmqqjzq9cl70.fsf@gitster.g","subject":"Re: git checkout -B lets you checkout a branch that is already checked out in another worktree Inbox","fromName":"Willem Verstraeten","fromEmail":"willem.verstraeten@gmail.com","sentAt":"2023-11-23T12:12:46Z","receivedAt":"2023-11-23T12:13:00Z","isPatch":false,"sender":{"key":"willem.verstraeten@gmail.com","avatar":null},"body":"> > git checkout -f main origin/main #also reports a fatal error, as expected\n>\n> This is expected because origin/main is taken as pathspec, and it is\n> a request to checkout the paths that match the pathspec out of the\n> named tree-ish (i.e., \"main\"), even when these paths have local\n> changes, but you do not have paths that match \"origin/main\". The\n> failure is not because \"main\" is checked out elsewhere.\n>\n\nMy mistake: I meant to do `git branch -f main origin/main`, as\ndocumented for `git checkout -B main origin/main`\n\nSo, for completeness' sake, this is my revised reproduction scenario\nthat actually demonstrates the problem I have:\n\n ~/temp> git clone https://github.com/servo/pathfinder.git primary\n Cloning into 'primary'...\n ....\n\n ~/temp> cd primary\n\n ~/temp/primary> git worktree add -b metal ../secondary origin/metal\n Preparing worktree (new branch 'metal')\n branch 'metal' set up to track 'origin/metal'.\n HEAD is now at 1cdcc209 wip\n\n ~/temp/primary> cd ..\\secondary\\\n\n ~/temp/secondary> git checkout main\n fatal: 'main' is already used by worktree at ../primary'\n\n ~/temp/secondary> git branch -f main origin/main\n fatal: cannot force update the branch 'main' used by worktree at\n'../primary'\n\n ~/temp/secondary> git checkout -B main origin/main\n Switched to and reset branch 'main'\n branch 'main' set up to track 'origin/main'.\n Your branch is up to date with 'origin/main'.\n\nI would expect that last `git checkout -B ...` to fail with a similar\nerror as the `git branch -f ...` command right before that, since the\ndocumentation for `git checkout -B ` states that\nit is the atomic equivalent of `git branch -f ;\ngit checkout `\n\n\n> I guess we could change the behaviour so that\n>\n> git checkout -B []\n>\n> fails when is an existing branch that is in use in another\n> worktree, and allow \"-f\" to be used to override the safety, i.e.,\n>\n> git checkout -f -B []\n\nI would be very much in favor of that, indeed.\n\nHowever, as you noted in your follow-up mail, the\n--ignore-other-worktrees option would be better suited than the -f\nflag.\n\n> It turns out that for some reason \"-f\" is not how we decided to\n> override this one---there is \"--ignore-other-worktrees\" option.\n\nThis means it would look like this then, if you decide to tackle this?\n\n ~/temp/secondary> git checkout -B metal origin/metal\n fatal: cannot force update the branch 'main' used by worktree at\n'../primary'\n\n ~/temp/secondary> git checkout --ignore-other-worktrees -B metal\norigin/metal\n Switched to and reset branch 'metal'\n branch 'metal' set up to track 'origin/metal'.\n Your branch is up to date with 'origin/metal'.\n\nMy thoughts as an experienced user, though not an experienced\ncontributor, admittedly :)\n\nKind regards,\nWillem Verstraeten\n\nOn Thu, 23 Nov 2023 at 02:28, Junio C Hamano wrote:\n>\n> Willem Verstraeten writes:\n>\n> > git checkout -b main #reports a fatal error, as expected\n>\n> This is expected because \"main\" already exists, not because \"main\"\n> is checked out elsewhere.\n>\n> > git checkout -f main origin/main #also reports a fatal error, as expected\n>\n> This is expected because origin/main is taken as pathspec, and it is\n> a request to checkout the paths that match the pathspec out of the\n> named tree-ish (i.e., \"main\"), even when these paths have local\n> changes, but you do not have paths that match \"origin/main\". The\n> failure is not because \"main\" is checked out elsewhere.\n>\n> A slight variant of the command\n>\n> git checkout -f -b main origin/main\n>\n> still fails for the same reason as the first of your examples above.\n>\n> It is a tangent, but I suspect this failure may be a bit unexpected.\n> In this example, \"-f\"orce could be overriding the usual failure from\n> \"-b\" to switch to a branch that already exists, but that is what\n> \"-B\" does, and \"-f -b\" does not work as a synonym for \"-B\".\n>\n> In any case, these example you marked \"fail as expected\" do fail as\n> expected, but they fail for reasons that have nothing to do with the\n> protection of branches that are used in other worktrees.\n>\n> > git checkout -B main origin/main # ----> this succeeds, which is\n> > unexpected <----\n>\n> I agree this may be undesirable.\n>\n> But it makes sort of sense, because \"-B\" is a forced form of \"-b\"\n> (i.e., it tells git: even when \"-b\" would fail, take necessary\n> measures to make it work), and we can view that it is part of\n> \"forcing\" to override the protection over branches that are used\n> elsewhere.\n>\n> I guess we could change the behaviour so that\n>\n> git checkout -B []\n>\n> fails when is an existing branch that is in use in another\n> worktree, and allow \"-f\" to be used to override the safety, i.e.,\n>\n> git checkout -f -B []\n>\n> would allow the to be repointed to (or HEAD)\n> even when it is used elsewhere.\n>\n> Thoughts, whether they agree or disagree with what I just said, by\n> other experienced contributors are very much welcome, before I can\n> say \"patches welcome\" ;-).\n>\n> Willem, thanks for raising the issue.\n>\n>\n>\n"},{"id":"485066","messageId":"bf848477-b4dd-49d3-8e4b-de0fc3948570@gmail.com","threadId":"60547","inReplyTo":"xmqqpm01au0w.fsf_-_@gitster.g","subject":"Re: [PATCH 2/2] checkout: forbid \"-B \" from touching a branch used elsewhere","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-11-23T16:33:11Z","receivedAt":"2023-11-23T16:33:14Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 23/11/2023 06:00, Junio C Hamano wrote:\n> \"git checkout -B []\", being a \"forced\" version\n> of \"-b\", switches to the , after optionally resetting its\n> tip to the , even if the is in use in another\n> worktree, which is somewhat unexpected.\n> \n> Protect the using the same logic that forbids \"git checkout\n> \" from touching a branch that is in use elsewhere.\n> \n> This is a breaking change that may deserve backward compatibliity\n> warning in the Release Notes. The \"--ignore-other-worktrees\" option\n> can be used as an escape hatch if the finger memory of existing\n> users depend on the current behaviour of \"-B\".\n\nI think this change makes sense and I found the implementation here much \neasier to understand than a previous attempt at \nhttps://lore.kernel.org/git/20230120113553.24655-1-carenas@gmail.com/\n\n> Reported-by: Willem Verstraeten \n> Signed-off-by: Junio C Hamano \n> ---\n> \n> * The documentation might also need updates, but I didn't look at.\n\nThis option is documented as an atomic version of\n\n\tgit branch -f []\n\tgit checkout \n\nHowever \"git branch -f \" will fail if the branch is checked out \nin the current worktree whereas \"git checkout -B\" succeeds. I think \nallowing the checkout in that case makes sense for \"git checkout -B\" but \nit does mean that description is not strictly accurate. I'm not sure it \nmatters that much though.\n\nThe documentation for \"switch -C\" is a bit lacking compared to \"checkout \n-B\" but that is a separate problem.\n\n> \n> builtin/checkout.c | 7 +++++++\n> t/t2060-switch.sh | 2 ++\n> t/t2400-worktree-add.sh | 8 ++++++++\n> 3 files changed, 17 insertions(+)\n> \n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index b4ab972c5a..8a8ad23e98 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -1600,6 +1600,13 @@ static int checkout_branch(struct checkout_opts *opts,\n> \tif (new_branch_info->path && !opts->force_detach && !opts->new_branch)\n> \t\tdie_if_switching_to_a_branch_in_use(opts, new_branch_info->path);\n> \n> +\t/* \"git checkout -B \" */\n> +\tif (opts->new_branch_force) {\n> +\t\tchar *full_ref = xstrfmt(\"refs/heads/%s\", opts->new_branch);\n> +\t\tdie_if_switching_to_a_branch_in_use(opts, full_ref);\n> +\t\tfree(full_ref);\n\nAt the moment this is academic as neither of the test scripts changed by \nthis patch are leak free and so I don't think we need to worry about it \nbut it raises an interesting question about how we should handle memory \nleaks when dying. Leaving the leak when dying means that a test script \nthat tests an expected failure will never be leak free but using \nUNLEAK() would mean we miss a leak being introduced in the successful \ncase should the call to \"free()\" ever be removed. We could of course \nrename die_if_checked_out() to error_if_checked_out() and return an \nerror instead of dying but that seems like a lot of churn just to keep \nthe leak checker happy.\n\nBest Wishes\n\nPhillip\n\n> +\t}\n> +\n> \tif (!new_branch_info->commit && opts->new_branch) {\n> \t\tstruct object_id rev;\n> \t\tint flag;\n> diff --git a/t/t2060-switch.sh b/t/t2060-switch.sh\n> index e247a4735b..c91c4db936 100755\n> --- a/t/t2060-switch.sh\n> +++ b/t/t2060-switch.sh\n> @@ -170,8 +170,10 @@ test_expect_success 'switch back when temporarily detached and checked out elsew\n> \t# we test in both worktrees to ensure that works\n> \t# as expected with \"first\" and \"next\" worktrees\n> \ttest_must_fail git -C wt1 switch shared &&\n> +\ttest_must_fail git -C wt1 switch -C shared &&\n> \tgit -C wt1 switch --ignore-other-worktrees shared &&\n> \ttest_must_fail git -C wt2 switch shared &&\n> +\ttest_must_fail git -C wt2 switch -C shared &&\n> \tgit -C wt2 switch --ignore-other-worktrees shared\n> '\n> \n> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\n> index df4aff7825..bbcb2d3419 100755\n> --- a/t/t2400-worktree-add.sh\n> +++ b/t/t2400-worktree-add.sh\n> @@ -126,6 +126,14 @@ test_expect_success 'die the same branch is already checked out' '\n> \t)\n> '\n> \n> +test_expect_success 'refuse to reset a branch in use elsewhere' '\n> +\t(\n> +\t\tcd here &&\n> +\t\ttest_must_fail git checkout -B newmain 2>actual &&\n> +\t\tgrep \"already used by worktree at\" actual\n> +\t)\n> +'\n> +\n> test_expect_success SYMLINKS 'die the same branch is already checked out (symlink)' '\n> \thead=$(git -C there rev-parse --git-path HEAD) &&\n> \tref=$(git -C there symbolic-ref HEAD) &&\n"},{"id":"485067","messageId":"CAPig+cRdQW-DG8PFb-P0U_44pFWxskVoOtjbGfD_OiHHDk8DdA@mail.gmail.com","threadId":"60547","inReplyTo":"bf848477-b4dd-49d3-8e4b-de0fc3948570@gmail.com","subject":"Re: [PATCH 2/2] checkout: forbid \"-B \" from touching a branch used elsewhere","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-11-23T17:09:38Z","receivedAt":"2023-11-23T17:09:50Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Nov 23, 2023 at 11:33 AM Phillip Wood wrote:\n> On 23/11/2023 06:00, Junio C Hamano wrote:\n> > \"git checkout -B []\", being a \"forced\" version\n> > of \"-b\", switches to the , after optionally resetting its\n> > tip to the , even if the is in use in another\n> > worktree, which is somewhat unexpected.\n> >\n> > Protect the using the same logic that forbids \"git checkout\n> > \" from touching a branch that is in use elsewhere.\n> >\n> > This is a breaking change that may deserve backward compatibliity\n> > warning in the Release Notes. The \"--ignore-other-worktrees\" option\n> > can be used as an escape hatch if the finger memory of existing\n> > users depend on the current behaviour of \"-B\".\n>\n> I think this change makes sense and I found the implementation here much\n> easier to understand than a previous attempt at\n> https://lore.kernel.org/git/20230120113553.24655-1-carenas@gmail.com/\n\nThanks for digging up this link. Upon reading the problem report, I\nfelt certain that we had seen this issue reported previously and that\npatches had been proposed, but I was unable to find the conversation\n(despite having taken part in it).\n\nI agree, also, that this two-patch series is simple to digest.\n"},{"id":"485080","messageId":"c8f3fcdc-a10c-4c24-b954-899cf413837e@gmail.com","threadId":"60547","inReplyTo":"xmqqv89tau3r.fsf@gitster.g","subject":"Re: git checkout -B lets you checkout a branch that is already checked out in another worktree Inbox","fromName":"Andy Koppe","fromEmail":"andy.koppe@gmail.com","sentAt":"2023-11-23T22:03:42Z","receivedAt":"2023-11-23T22:03:45Z","isPatch":false,"sender":{"key":"andy.koppe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223411?v=4"},"body":"On 23/11/2023 05:58, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n>> I guess we could change the behaviour so that\n>>\n>> git checkout -B []\n>>\n>> fails when is an existing branch that is in use in another\n>> worktree, and allow \"-f\" to be used to override the safety, i.e.,\n>>\n>> git checkout -f -B []\n>>\n>> would allow the to be repointed to (or HEAD)\n>> even when it is used elsewhere.\n> \n> It turns out that for some reason \"-f\" is not how we decided to\n> override this one---there is \"--ignore-other-worktrees\" option.\n\nPresumably that's because -f means throwing away any local changes that \nare in the way of checking out the new HEAD, which you wouldn't \nnecessarily want when trying to replace an existing branch.\n\nAndy\n"},{"id":"485083","messageId":"xmqqh6lc9cdg.fsf@gitster.g","threadId":"60547","inReplyTo":"CAPig+cRdQW-DG8PFb-P0U_44pFWxskVoOtjbGfD_OiHHDk8DdA@mail.gmail.com","subject":"Re: [PATCH 2/2] checkout: forbid \"-B \" from touching a branch used elsewhere","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-24T01:19:23Z","receivedAt":"2023-11-24T01:19:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> Thanks for digging up this link. Upon reading the problem report, I\n> felt certain that we had seen this issue reported previously and that\n> patches had been proposed, but I was unable to find the conversation\n> (despite having taken part in it).\n\nI am surprised that I did not remember that old discussion at all,\nand I am doubly surprised that I still do not, even though I clearly\nrecognise my writing in the thread.\n\n> I agree, also, that this two-patch series is simple to digest.\n\nOK.\n"},{"id":"485164","messageId":"xmqqwmu42ccb.fsf@gitster.g","threadId":"60547","inReplyTo":"bf848477-b4dd-49d3-8e4b-de0fc3948570@gmail.com","subject":"Re: [PATCH 2/2] checkout: forbid \"-B \" from touching a branch used elsewhere","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-27T01:51:00Z","receivedAt":"2023-11-27T01:51:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n>> diff --git a/builtin/checkout.c b/builtin/checkout.c\n>> index b4ab972c5a..8a8ad23e98 100644\n>> --- a/builtin/checkout.c\n>> +++ b/builtin/checkout.c\n>> @@ -1600,6 +1600,13 @@ static int checkout_branch(struct checkout_opts *opts,\n>> \tif (new_branch_info->path && !opts->force_detach && !opts->new_branch)\n>> \t\tdie_if_switching_to_a_branch_in_use(opts, new_branch_info->path);\n>> +\t/* \"git checkout -B \" */\n>> +\tif (opts->new_branch_force) {\n>> +\t\tchar *full_ref = xstrfmt(\"refs/heads/%s\", opts->new_branch);\n>> +\t\tdie_if_switching_to_a_branch_in_use(opts, full_ref);\n>> +\t\tfree(full_ref);\n>\n> At the moment this is academic as neither of the test scripts changed\n> by this patch are leak free and so I don't think we need to worry\n> about it but it raises an interesting question about how we should\n> handle memory leaks when dying. Leaving the leak when dying means that\n> a test script that tests an expected failure will never be leak free\n> but using UNLEAK() would mean we miss a leak being introduced in the\n> successful case should the call to \"free()\" ever be removed.\n\nIs there a leak here? The piece of memory is pointed at by an on-stack\nvariable full_ref when leak sanitizer starts scanning the heap and\nthe stack just before the process exits due to die, so I do not see\na reason to worry about this particular variable over all the other\non stack variables we accumulated before the control reached this\npoint of the code.\n\nAre you worried about optimizing compilers that behave more cleverly\nthan their own good to somehow lose the on-stack reference to\nfull_ref while calling die_if_switching_to_a_branch_in_use()? We\nmight need to squelch them with UNLEAK() but that does not mean we\nhave to remove the free() we see above, and I suspect a more\nproductive use of our time to solve that issue is ensure that our\nleak-sanitizing build will not triger such an unwanted optimization\nanyway.\n\nThanks.\n"},{"id":"485180","messageId":"20231127213115.GB87495@coredump.intra.peff.net","threadId":"60547","inReplyTo":"xmqqwmu42ccb.fsf@gitster.g","subject":"Re: [PATCH 2/2] checkout: forbid \"-B \" from touching a branch used elsewhere","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-27T21:31:15Z","receivedAt":"2023-11-27T21:31:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 27, 2023 at 10:51:00AM +0900, Junio C Hamano wrote:\n\n> >> +\tif (opts->new_branch_force) {\n> >> +\t\tchar *full_ref = xstrfmt(\"refs/heads/%s\", opts->new_branch);\n> >> +\t\tdie_if_switching_to_a_branch_in_use(opts, full_ref);\n> >> +\t\tfree(full_ref);\n> >\n> > At the moment this is academic as neither of the test scripts changed\n> > by this patch are leak free and so I don't think we need to worry\n> > about it but it raises an interesting question about how we should\n> > handle memory leaks when dying. Leaving the leak when dying means that\n> > a test script that tests an expected failure will never be leak free\n> > but using UNLEAK() would mean we miss a leak being introduced in the\n> > successful case should the call to \"free()\" ever be removed.\n> \n> Is there a leak here? The piece of memory is pointed at by an on-stack\n> variable full_ref when leak sanitizer starts scanning the heap and\n> the stack just before the process exits due to die, so I do not see\n> a reason to worry about this particular variable over all the other\n> on stack variables we accumulated before the control reached this\n> point of the code.\n\nRight, I think the only reasonable approach here is to not consider this\na leak. We've discussed this in the past. Here's a link into a relevant\nthread for reference, but I don't think it's really worth anybody's\ntime to re-visit:\n\n https://lore.kernel.org/git/Y0+i1G5ybdhUGph2@coredump.intra.peff.net/\n\n> Are you worried about optimizing compilers that behave more cleverly\n> than their own good to somehow lose the on-stack reference to\n> full_ref while calling die_if_switching_to_a_branch_in_use()? We\n> might need to squelch them with UNLEAK() but that does not mean we\n> have to remove the free() we see above, and I suspect a more\n> productive use of our time to solve that issue is ensure that our\n> leak-sanitizing build will not triger such an unwanted optimization\n> anyway.\n\nWe did have that problem, but it should no longer be the case after\nd3775de074 (Makefile: force -O0 when compiling with SANITIZE=leak,\n2022-10-18). If that is not sufficient for some compiler/code combo, I'd\nbe interested to hear about it.\n\n-Peff\n"},{"id":"485269","messageId":"b3532261-3cf4-4666-9cbd-4ce668cd2e49@gmail.com","threadId":"60547","inReplyTo":"xmqqwmu42ccb.fsf@gitster.g","subject":"Re: [PATCH 2/2] checkout: forbid \"-B \" from touching a branch used elsewhere","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-11-30T15:22:32Z","receivedAt":"2023-11-30T15:22:35Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 27/11/2023 01:51, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> At the moment this is academic as neither of the test scripts changed\n>> by this patch are leak free and so I don't think we need to worry\n>> about it but it raises an interesting question about how we should\n>> handle memory leaks when dying. Leaving the leak when dying means that\n>> a test script that tests an expected failure will never be leak free\n>> but using UNLEAK() would mean we miss a leak being introduced in the\n>> successful case should the call to \"free()\" ever be removed.\n> \n> Is there a leak here? The piece of memory is pointed at by an on-stack\n> variable full_ref when leak sanitizer starts scanning the heap and\n> the stack just before the process exits due to die, so I do not see\n> a reason to worry about this particular variable over all the other\n> on stack variables we accumulated before the control reached this\n> point of the code.\n\nOh, good point. I was thinking \"we exit without calling free() so it is \nleaked\" but as you say the leak checker (thankfully) does not consider \nit a leak as there is still a reference to the allocation on the stack.\n\nSorry for the noise\n\nPhillip\n\n> Are you worried about optimizing compilers that behave more cleverly\n> than their own good to somehow lose the on-stack reference to\n> full_ref while calling die_if_switching_to_a_branch_in_use()? We\n> might need to squelch them with UNLEAK() but that does not mean we\n> have to remove the free() we see above, and I suspect a more\n> productive use of our time to solve that issue is ensure that our\n> leak-sanitizing build will not triger such an unwanted optimization\n> anyway.\n> \n> Thanks.\n"},{"id":"485348","messageId":"CAGX9RpH0RJfBADQwJ=c7PCHU955vOqd0Wdc7Yi7XUuAQQW_FNQ@mail.gmail.com","threadId":"60547","inReplyTo":"b3532261-3cf4-4666-9cbd-4ce668cd2e49@gmail.com","subject":"Re: [PATCH 2/2] checkout: forbid \"-B \" from touching a branch used elsewhere","fromName":"Willem Verstraeten","fromEmail":"willem.verstraeten@gmail.com","sentAt":"2023-12-04T12:20:54Z","receivedAt":"2023-12-04T12:21:06Z","isPatch":true,"sender":{"key":"willem.verstraeten@gmail.com","avatar":null},"body":"Hi everyone,\n\nIt's not clear for me from the email thread what the status is of this\nbug report, and whether there is still something expected from me.\n\nIs the current consensus that this is a real issue that needs fixing?\nIf so, does the current patch-set fix the issue, and how does the fix\nget into (one of) the next release(s)?\n\nDo I still need to do something?\n\nKind regards,\nWillem\n\nOn Thu, 30 Nov 2023 at 16:22, Phillip Wood wrote:\n>\n> Hi Junio\n>\n> On 27/11/2023 01:51, Junio C Hamano wrote:\n> > Phillip Wood writes:\n> >\n> >> At the moment this is academic as neither of the test scripts changed\n> >> by this patch are leak free and so I don't think we need to worry\n> >> about it but it raises an interesting question about how we should\n> >> handle memory leaks when dying. Leaving the leak when dying means that\n> >> a test script that tests an expected failure will never be leak free\n> >> but using UNLEAK() would mean we miss a leak being introduced in the\n> >> successful case should the call to \"free()\" ever be removed.\n> >\n> > Is there a leak here? The piece of memory is pointed at by an on-stack\n> > variable full_ref when leak sanitizer starts scanning the heap and\n> > the stack just before the process exits due to die, so I do not see\n> > a reason to worry about this particular variable over all the other\n> > on stack variables we accumulated before the control reached this\n> > point of the code.\n>\n> Oh, good point. I was thinking \"we exit without calling free() so it is\n> leaked\" but as you say the leak checker (thankfully) does not consider\n> it a leak as there is still a reference to the allocation on the stack.\n>\n> Sorry for the noise\n>\n> Phillip\n>\n> > Are you worried about optimizing compilers that behave more cleverly\n> > than their own good to somehow lose the on-stack reference to\n> > full_ref while calling die_if_switching_to_a_branch_in_use()? We\n> > might need to squelch them with UNLEAK() but that does not mean we\n> > have to remove the free() we see above, and I suspect a more\n> > productive use of our time to solve that issue is ensure that our\n> > leak-sanitizing build will not triger such an unwanted optimization\n> > anyway.\n> >\n> > Thanks.\n"},{"id":"485355","messageId":"CAPig+cSGF+vQrnD0f99cbdpQOOC7X6ULa9tFe+FwVrG0SF4PGg@mail.gmail.com","threadId":"60547","inReplyTo":"CAGX9RpH0RJfBADQwJ=c7PCHU955vOqd0Wdc7Yi7XUuAQQW_FNQ@mail.gmail.com","subject":"Re: [PATCH 2/2] checkout: forbid \"-B \" from touching a branch used elsewhere","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-12-04T21:06:50Z","receivedAt":"2023-12-04T21:07:02Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Dec 4, 2023 at 7:21 AM Willem Verstraeten\n wrote:\n> It's not clear for me from the email thread what the status is of this\n> bug report, and whether there is still something expected from me.\n>\n> Is the current consensus that this is a real issue that needs fixing?\n> If so, does the current patch-set fix the issue, and how does the fix\n> get into (one of) the next release(s)?\n>\n> Do I still need to do something?\n\nAccording to Junio's latest \"What's cooking\"[1], the status of this\npatch series is:\n\n * jc/checkout-B-branch-in-use (2023-11-23) 2 commits\n - checkout: forbid \"-B \" from touching a branch used elsewhere\n - checkout: refactor die_if_checked_out() caller\n\n \"git checkout -B []\" allowed a branch that is\n in use in another worktree to be updated and checked out, which\n might be a bit unexpected. The rule has been tightened, which is a\n breaking change. \"--ignore-other-worktrees\" option is required to\n unbreak you, if you are used to the current behaviour that \"-B\"\n overrides the safety.\n\n Needs review and documentation updates.\n\nI'm not sure if the \"Needs review\" comment is still applicable since\nthe patch did get some review comments, however, the mentioned\ndocumentation update is probably still needed for this series to\ngraduate. I can't speak for what Junio had in mind, but perhaps\nsufficient would be to add a side-note to the description of the -B\noption saying that it historically (accidentally) would succeed even\nif the named branch was checked out in another worktree, but now\nrequires --ignore-other-worktrees.\n\nTo move the series forward, someone will probably need to make the\nnecessary documentation update. That someone could be you, if you're\ninterested, either by rerolling Junio's series and modifying patch\n[2/2] to also make the necessary documentation update, or by posting a\npatch, [3/2] atop his series which updates the documentation.\n\n[1]: https://lore.kernel.org/git/xmqq8r6j1dgt.fsf@gitster.g/\n"},{"id":"485472","messageId":"xmqqsf4c39e9.fsf@gitster.g","threadId":"60547","inReplyTo":"CAPig+cSGF+vQrnD0f99cbdpQOOC7X6ULa9tFe+FwVrG0SF4PGg@mail.gmail.com","subject":"Re: [PATCH 2/2] checkout: forbid \"-B \" from touching a branch used elsewhere","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-12-08T17:13:18Z","receivedAt":"2023-12-08T17:13:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> Needs review and documentation updates.\n>\n> I'm not sure if the \"Needs review\" comment is still applicable since\n> the patch did get some review comments, however, the mentioned\n> documentation update is probably still needed for this series to\n> graduate.\n\nThanks. I think \"-B\" being defined as \"branch -f \" followed\nby \"checkout \" makes it technically unnecessary to add any\nnew documentation (because \"checkout \" will refuse, so it\nnaturally follows that \"checkout -B \" should), but giving\nthe failure mode a bit more explicit mention would be more helpful\nto readers.\n\nHere is to illustrate what I have in mind. The mention of the\n\"transactional\" was already in the documentation for the \"checkout\"\nback when switch was described at d787d311 (checkout: split part of\nit to new command 'switch', 2019-03-29), but somehow was left out in\nthe documentation of the \"switch\". While it is not incorrect to say\nthat it is a convenient short-cut, it is more important to say what\nhappens when one of them fails, so I am tempted to port that\ndescription over to the \"switch\" command, and give the \"used elsewhere\"\nas a sample failure mode.\n\nThe test has been also enhanced to check the \"transactional\" nature.\n\n Documentation/git-checkout.txt | 4 +++-\n Documentation/git-switch.txt | 9 +++++++--\n t/t2400-worktree-add.sh | 18 ++++++++++++++++--\n 3 files changed, 26 insertions(+), 5 deletions(-)\n\ndiff --git c/Documentation/git-checkout.txt w/Documentation/git-checkout.txt\nindex 240c54639e..55a50b5b23 100644\n--- c/Documentation/git-checkout.txt\n+++ w/Documentation/git-checkout.txt\n@@ -63,7 +63,9 @@ $ git checkout \n ------------\n +\n that is to say, the branch is not reset/created unless \"git checkout\" is\n-successful.\n+successful (e.g., when the branch is in use in another worktree, not\n+just the current branch stays the same, but the branch is not reset to\n+the start-point, either).\n \n 'git checkout' --detach []::\n 'git checkout' [--detach] ::\ndiff --git c/Documentation/git-switch.txt w/Documentation/git-switch.txt\nindex c60fc9c138..6137421ede 100644\n--- c/Documentation/git-switch.txt\n+++ w/Documentation/git-switch.txt\n@@ -59,13 +59,18 @@ out at most one of `A` and `B`, in which case it defaults to `HEAD`.\n -c ::\n --create ::\n \tCreate a new branch named `` starting at\n-\t`` before switching to the branch. This is a\n-\tconvenient shortcut for:\n+\t`` before switching to the branch. This is the\n+\ttransactional equivalent of\n +\n ------------\n $ git branch \n $ git switch \n ------------\n++\n+that is to say, the branch is not reset/created unless \"git switch\" is\n+successful (e.g., when the branch is in use in another worktree, not\n+just the current branch stays the same, but the branch is not reset to\n+the start-point, either).\n \n -C ::\n --force-create ::\ndiff --git c/t/t2400-worktree-add.sh w/t/t2400-worktree-add.sh\nindex bbcb2d3419..5d5064e63d 100755\n--- c/t/t2400-worktree-add.sh\n+++ w/t/t2400-worktree-add.sh\n@@ -129,8 +129,22 @@ test_expect_success 'die the same branch is already checked out' '\n test_expect_success 'refuse to reset a branch in use elsewhere' '\n \t(\n \t\tcd here &&\n-\t\ttest_must_fail git checkout -B newmain 2>actual &&\n-\t\tgrep \"already used by worktree at\" actual\n+\n+\t\t# we know we are on detached HEAD but just in case ...\n+\t\tgit checkout --detach HEAD &&\n+\t\tgit rev-parse --verify HEAD >old.head &&\n+\n+\t\tgit rev-parse --verify refs/heads/newmain >old.branch &&\n+\t\ttest_must_fail git checkout -B newmain 2>error &&\n+\t\tgit rev-parse --verify refs/heads/newmain >new.branch &&\n+\t\tgit rev-parse --verify HEAD >new.head &&\n+\n+\t\tgrep \"already used by worktree at\" error &&\n+\t\ttest_cmp old.branch new.branch &&\n+\t\ttest_cmp old.head new.head &&\n+\n+\t\t# and we must be still on the same detached HEAD state\n+\t\ttest_must_fail git symbolic-ref HEAD\n \t)\n '\n \n"},{"id":"487598","messageId":"CAGX9RpF=tvPwiLO6UYA+uR5f2oqOLUSNaDL-jfn=T=BQ9FNtkQ@mail.gmail.com","threadId":"60547","inReplyTo":"xmqqsf4c39e9.fsf@gitster.g","subject":"Re: [PATCH 2/2] checkout: forbid \"-B \" from touching a branch used elsewhere","fromName":"Willem Verstraeten","fromEmail":"willem.verstraeten@gmail.com","sentAt":"2024-01-30T12:37:26Z","receivedAt":"2024-01-30T12:37:39Z","isPatch":true,"sender":{"key":"willem.verstraeten@gmail.com","avatar":null},"body":"Hi all,\n\nSorry for dropping out of the conversation, but I see that the changes\nlanded on the master, ready for the 2.44.0 release.\n\nThank you very much!\n\nOn Fri, 8 Dec 2023 at 18:13, Junio C Hamano wrote:\n>\n> Eric Sunshine writes:\n>\n> > Needs review and documentation updates.\n> >\n> > I'm not sure if the \"Needs review\" comment is still applicable since\n> > the patch did get some review comments, however, the mentioned\n> > documentation update is probably still needed for this series to\n> > graduate.\n>\n> Thanks. I think \"-B\" being defined as \"branch -f \" followed\n> by \"checkout \" makes it technically unnecessary to add any\n> new documentation (because \"checkout \" will refuse, so it\n> naturally follows that \"checkout -B \" should), but giving\n> the failure mode a bit more explicit mention would be more helpful\n> to readers.\n>\n> Here is to illustrate what I have in mind. The mention of the\n> \"transactional\" was already in the documentation for the \"checkout\"\n> back when switch was described at d787d311 (checkout: split part of\n> it to new command 'switch', 2019-03-29), but somehow was left out in\n> the documentation of the \"switch\". While it is not incorrect to say\n> that it is a convenient short-cut, it is more important to say what\n> happens when one of them fails, so I am tempted to port that\n> description over to the \"switch\" command, and give the \"used elsewhere\"\n> as a sample failure mode.\n>\n> The test has been also enhanced to check the \"transactional\" nature.\n>\n> Documentation/git-checkout.txt | 4 +++-\n> Documentation/git-switch.txt | 9 +++++++--\n> t/t2400-worktree-add.sh | 18 ++++++++++++++++--\n> 3 files changed, 26 insertions(+), 5 deletions(-)\n>\n> diff --git c/Documentation/git-checkout.txt w/Documentation/git-checkout.txt\n> index 240c54639e..55a50b5b23 100644\n> --- c/Documentation/git-checkout.txt\n> +++ w/Documentation/git-checkout.txt\n> @@ -63,7 +63,9 @@ $ git checkout \n> ------------\n> +\n> that is to say, the branch is not reset/created unless \"git checkout\" is\n> -successful.\n> +successful (e.g., when the branch is in use in another worktree, not\n> +just the current branch stays the same, but the branch is not reset to\n> +the start-point, either).\n>\n> 'git checkout' --detach []::\n> 'git checkout' [--detach] ::\n> diff --git c/Documentation/git-switch.txt w/Documentation/git-switch.txt\n> index c60fc9c138..6137421ede 100644\n> --- c/Documentation/git-switch.txt\n> +++ w/Documentation/git-switch.txt\n> @@ -59,13 +59,18 @@ out at most one of `A` and `B`, in which case it defaults to `HEAD`.\n> -c ::\n> --create ::\n> Create a new branch named `` starting at\n> - `` before switching to the branch. This is a\n> - convenient shortcut for:\n> + `` before switching to the branch. This is the\n> + transactional equivalent of\n> +\n> ------------\n> $ git branch \n> $ git switch \n> ------------\n> ++\n> +that is to say, the branch is not reset/created unless \"git switch\" is\n> +successful (e.g., when the branch is in use in another worktree, not\n> +just the current branch stays the same, but the branch is not reset to\n> +the start-point, either).\n>\n> -C ::\n> --force-create ::\n> diff --git c/t/t2400-worktree-add.sh w/t/t2400-worktree-add.sh\n> index bbcb2d3419..5d5064e63d 100755\n> --- c/t/t2400-worktree-add.sh\n> +++ w/t/t2400-worktree-add.sh\n> @@ -129,8 +129,22 @@ test_expect_success 'die the same branch is already checked out' '\n> test_expect_success 'refuse to reset a branch in use elsewhere' '\n> (\n> cd here &&\n> - test_must_fail git checkout -B newmain 2>actual &&\n> - grep \"already used by worktree at\" actual\n> +\n> + # we know we are on detached HEAD but just in case ...\n> + git checkout --detach HEAD &&\n> + git rev-parse --verify HEAD >old.head &&\n> +\n> + git rev-parse --verify refs/heads/newmain >old.branch &&\n> + test_must_fail git checkout -B newmain 2>error &&\n> + git rev-parse --verify refs/heads/newmain >new.branch &&\n> + git rev-parse --verify HEAD >new.head &&\n> +\n> + grep \"already used by worktree at\" error &&\n> + test_cmp old.branch new.branch &&\n> + test_cmp old.head new.head &&\n> +\n> + # and we must be still on the same detached HEAD state\n> + test_must_fail git symbolic-ref HEAD\n> )\n> '\n>\n"},{"id":"487616","messageId":"xmqqeddya1zn.fsf@gitster.g","threadId":"60547","inReplyTo":"CAGX9RpF=tvPwiLO6UYA+uR5f2oqOLUSNaDL-jfn=T=BQ9FNtkQ@mail.gmail.com","subject":"Re: [PATCH 2/2] checkout: forbid \"-B \" from touching a branch used elsewhere","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-30T22:30:04Z","receivedAt":"2024-01-30T22:30:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Willem Verstraeten writes:\n\n> Sorry for dropping out of the conversation, but I see that the changes\n> landed on the master, ready for the 2.44.0 release.\n>\n> Thank you very much!\n\nThanks for your initial report that led to the update.\n"}]}