From: "Carlo Marcelo Arenas Belón" <carenas@gmail.com>
To: git@vger.kernel.org
Cc: pclouds@gmail.com, gitster@pobox.com,
"Carlo Marcelo Arenas Belón" <carenas@gmail.com>,
"Jinwook Jeong" <vustthat@gmail.com>,
"Rubén Justo" <rjusto@gmail.com>,
"Eric Sunshine" <sunshine@sunshineco.com>
Subject: [PATCH v3] checkout/switch: disallow checking out same branch in multiple worktrees
Date: Wed, 18 Jan 2023 21:53:24 -0800 [thread overview]
Message-ID: <20230119055325.1013-1-carenas@gmail.com> (raw)
In-Reply-To: <20230118061527.76218-1-carenas@gmail.com>
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
by expanding on the existing checks that are being used by their non
force variants.
Unlike what it is done for `git switch` and `git checkout`, that have
an historical exception to ignore other workspaces 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.
Reported-by: Jinwook Jeong <vustthat@gmail.com>
Helped-by: Rubén Justo <rjusto@gmail.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
Changes since v2
* A leak free implementation
* More details in commit as suggested by Junio
Changes since v1
* A much better commit message
* Changes to the tests as suggested by Eric
* Changes to the logic as suggested by Rubén
builtin/checkout.c | 31 +++++++++++++++++++++++--------
t/t2400-worktree-add.sh | 18 ++++++++++++++++--
2 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5963e1b74b..467e194701 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1476,7 +1476,8 @@ static void die_if_some_operation_in_progress(void)
}
static int checkout_branch(struct checkout_opts *opts,
- struct branch_info *new_branch_info)
+ struct branch_info *new_branch_info,
+ char *check_branch_path)
{
if (opts->pathspec.nr)
die(_("paths cannot be used with switching branches"));
@@ -1535,15 +1536,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) {
+ if (check_branch_path && !opts->force_detach && !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);
+ if (opts->new_branch_force || (head_ref &&
+ (!(flag & REF_ISSYMREF) || strcmp(head_ref, check_branch_path))))
+ die_if_checked_out(check_branch_path, 1);
free(head_ref);
}
+ free(check_branch_path);
if (!new_branch_info->commit && opts->new_branch) {
struct object_id rev;
@@ -1630,6 +1631,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
struct branch_info *new_branch_info)
{
int parseopt_flags = 0;
+ char *check_branch_path = NULL;
opts->overwrite_ignore = 1;
opts->prefix = prefix;
@@ -1741,6 +1743,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
!opts->new_branch;
int n = parse_branchname_arg(argc, argv, dwim_ok,
new_branch_info, opts, &rev);
+ check_branch_path = xstrdup_or_null(new_branch_info->path);
argv += n;
argc -= n;
} else if (!opts->accept_ref && opts->from_treeish) {
@@ -1753,8 +1756,18 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
opts, &rev,
opts->from_treeish);
+ check_branch_path = xstrdup_or_null(new_branch_info->path);
if (!opts->source_tree)
die(_("reference is not a tree: %s"), opts->from_treeish);
+ } else if (opts->new_branch && !opts->ignore_other_worktrees) {
+ struct object_id rev;
+
+ if (!get_oid_mb(opts->new_branch, &rev)) {
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_branchname(&buf, opts->new_branch, INTERPRET_BRANCH_LOCAL);
+ strbuf_splice(&buf, 0, 0, "refs/heads/", 11);
+ check_branch_path = strbuf_detach(&buf, NULL);
+ }
}
if (argc) {
@@ -1818,10 +1831,12 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
strbuf_release(&buf);
}
- if (opts->patch_mode || opts->pathspec.nr)
+ if (opts->patch_mode || opts->pathspec.nr) {
+ free(check_branch_path);
return checkout_paths(opts, new_branch_info);
+ }
else
- return checkout_branch(opts, new_branch_info);
+ return checkout_branch(opts, new_branch_info, check_branch_path);
}
int cmd_checkout(int argc, const char **argv, const char *prefix)
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index d587e0b20d..a66f9bb30c 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -121,7 +121,10 @@ test_expect_success '"add" worktree creating new branch' '
test_expect_success 'die the same branch is already checked out' '
(
cd here &&
- test_must_fail git checkout newmain
+ test_must_fail git checkout newmain &&
+ test_must_fail git checkout -B newmain &&
+ test_must_fail git switch newmain &&
+ test_must_fail git switch -C newmain
)
'
@@ -143,7 +146,18 @@ test_expect_success 'not die the same branch is already checked out' '
test_expect_success 'not die on re-checking out current branch' '
(
cd there &&
- git checkout newmain
+ git checkout newmain &&
+ git switch newmain
+ )
+'
+
+test_expect_success 'but die if using force without --ignore-other-worktrees' '
+ (
+ cd there &&
+ test_must_fail git checkout -B newmain &&
+ test_must_fail git switch -C newmain &&
+ git checkout --ignore-other-worktrees -B newmain &&
+ git switch --ignore-other-worktrees -C newmain
)
'
--
2.37.1 (Apple Git-137.1)
next prev parent reply other threads:[~2023-01-19 5:53 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-16 17:28 [PATCH] builtin/checkout: check the branch used in -B with worktrees Carlo Marcelo Arenas Belón
2023-01-16 22:18 ` Eric Sunshine
2023-01-17 0:53 ` Rubén Justo
2023-01-18 5:44 ` Carlo Arenas
2023-01-18 6:15 ` [PATCH v2] checkout/switch: disallow checking out same branch in multiple worktrees Carlo Marcelo Arenas Belón
2023-01-18 6:52 ` Junio C Hamano
2023-01-18 7:58 ` Carlo Arenas
2023-01-18 16:10 ` Junio C Hamano
2023-01-18 22:55 ` Junio C Hamano
2023-01-19 5:53 ` Carlo Marcelo Arenas Belón [this message]
2023-01-19 7:23 ` Re* [PATCH v3] " Junio C Hamano
2023-01-19 7:41 ` Carlo Arenas
2023-01-19 14:21 ` Phillip Wood
2023-01-20 3:10 ` Junio C Hamano
2023-01-20 3:53 ` Carlo Arenas
2023-01-20 4:39 ` Carlo Arenas
2023-01-20 11:35 ` [PATCH v4] " Carlo Marcelo Arenas Belón
2023-01-20 15:08 ` Phillip Wood
2023-01-20 22:12 ` Carlo Arenas
2023-01-27 14:46 ` Phillip Wood
2023-05-14 20:21 ` Rubén Justo
2023-03-23 0:06 ` Junio C Hamano
2023-03-24 3:49 ` Carlo Arenas
2023-05-14 20:24 ` Rubén Justo
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=20230119055325.1013-1-carenas@gmail.com \
--to=carenas@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=pclouds@gmail.com \
--cc=rjusto@gmail.com \
--cc=sunshine@sunshineco.com \
--cc=vustthat@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).