From 662078caacd450f93faa72e0e7d8580c42621415 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Justo?= Date: Sat, 25 Feb 2023 15:21:51 +0100 Subject: [PATCH 1/4] worktree: introduce is_shared_symref() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new function, is_shared_symref(), which contains the heart of find_shared_symref(). Refactor find_shared_symref() to use the new function is_shared_symref(). Soon, we will use is_shared_symref() to search for symref beyond the first worktree that matches. Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano --- worktree.c | 62 +++++++++++++++++++++++++++--------------------------- worktree.h | 6 ++++++ 2 files changed, 37 insertions(+), 31 deletions(-) diff --git a/worktree.c b/worktree.c index aa43c64119..40cb9874b7 100644 --- a/worktree.c +++ b/worktree.c @@ -403,44 +403,44 @@ int is_worktree_being_bisected(const struct worktree *wt, * bisect). New commands that do similar things should update this * function as well. */ +int is_shared_symref(const struct worktree *wt, const char *symref, + const char *target) +{ + const char *symref_target; + struct ref_store *refs; + int flags; + + if (wt->is_bare) + return 0; + + if (wt->is_detached && !strcmp(symref, "HEAD")) { + if (is_worktree_being_rebased(wt, target)) + return 1; + if (is_worktree_being_bisected(wt, target)) + return 1; + } + + refs = get_worktree_ref_store(wt); + symref_target = refs_resolve_ref_unsafe(refs, symref, 0, + NULL, &flags); + if ((flags & REF_ISSYMREF) && + symref_target && !strcmp(symref_target, target)) + return 1; + + return 0; +} + const struct worktree *find_shared_symref(struct worktree **worktrees, const char *symref, const char *target) { - const struct worktree *existing = NULL; - int i = 0; - for (i = 0; worktrees[i]; i++) { - struct worktree *wt = worktrees[i]; - const char *symref_target; - struct ref_store *refs; - int flags; - - if (wt->is_bare) - continue; - - if (wt->is_detached && !strcmp(symref, "HEAD")) { - if (is_worktree_being_rebased(wt, target)) { - existing = wt; - break; - } - if (is_worktree_being_bisected(wt, target)) { - existing = wt; - break; - } - } - - refs = get_worktree_ref_store(wt); - symref_target = refs_resolve_ref_unsafe(refs, symref, 0, - NULL, &flags); - if ((flags & REF_ISSYMREF) && - symref_target && !strcmp(symref_target, target)) { - existing = wt; - break; - } + for (int i = 0; worktrees[i]; i++) { + if (is_shared_symref(worktrees[i], symref, target)) + return worktrees[i]; } - return existing; + return NULL; } int submodule_uses_worktrees(const char *path) diff --git a/worktree.h b/worktree.h index 9dcea6fc8c..7889c4761d 100644 --- a/worktree.h +++ b/worktree.h @@ -149,6 +149,12 @@ const struct worktree *find_shared_symref(struct worktree **worktrees, const char *symref, const char *target); +/* + * Returns true if a symref points to a ref in a worktree. + */ +int is_shared_symref(const struct worktree *wt, + const char *symref, const char *target); + /* * Similar to head_ref() for all HEADs _except_ one from the current * worktree, which is covered by head_ref(). From faa4d5983bc1739351f49269660285a2628a3d72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Justo?= Date: Sat, 25 Feb 2023 15:22:02 +0100 Subject: [PATCH 2/4] branch: fix die_if_checked_out() when ignore_current_worktree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In 8d9fdd7 (worktree.c: check whether branch is rebased in another worktree, 2016-04-22) die_if_checked_out() learned a new option ignore_current_worktree, to modify the operation from "die() if the branch is checked out in any worktree" to "die() if the branch is checked out in any worktree other than the current one". Unfortunately we implemented it by checking the flag is_current in the worktree that find_shared_symref() returns. When the same branch is checked out in several worktrees simultaneously, find_shared_symref() will return the first matching worktree in the list composed by get_worktrees(). If one of the worktrees with the checked out branch is the current worktree, find_shared_symref() may or may not return it, depending on the order in the list. Instead of find_shared_symref(), let's do the search using use the recently introduced API is_shared_symref(), and consider ignore_current_worktree when necessary. Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano --- branch.c | 14 +++++++++----- worktree.c | 3 +-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/branch.c b/branch.c index d182756827..e04710b2b5 100644 --- a/branch.c +++ b/branch.c @@ -820,12 +820,16 @@ void remove_branch_state(struct repository *r, int verbose) void die_if_checked_out(const char *branch, int ignore_current_worktree) { struct worktree **worktrees = get_worktrees(); - const struct worktree *wt; - wt = find_shared_symref(worktrees, "HEAD", branch); - if (wt && (!ignore_current_worktree || !wt->is_current)) { - skip_prefix(branch, "refs/heads/", &branch); - die(_("'%s' is already checked out at '%s'"), branch, wt->path); + for (int i = 0; worktrees[i]; i++) { + if (worktrees[i]->is_current && ignore_current_worktree) + continue; + + if (is_shared_symref(worktrees[i], "HEAD", branch)) { + skip_prefix(branch, "refs/heads/", &branch); + die(_("'%s' is already checked out at '%s'"), + branch, worktrees[i]->path); + } } free_worktrees(worktrees); diff --git a/worktree.c b/worktree.c index 40cb9874b7..34043d8fe0 100644 --- a/worktree.c +++ b/worktree.c @@ -435,10 +435,9 @@ const struct worktree *find_shared_symref(struct worktree **worktrees, const char *target) { - for (int i = 0; worktrees[i]; i++) { + for (int i = 0; worktrees[i]; i++) if (is_shared_symref(worktrees[i], symref, target)) return worktrees[i]; - } return NULL; } From 279f42fa27b4ffd0b3ac7c9378043eeb413f0f5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Justo?= Date: Sat, 25 Feb 2023 15:22:13 +0100 Subject: [PATCH 3/4] rebase: refuse to switch to a branch already checked out elsewhere (test) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In b5cabb4a9 (rebase: refuse to switch to branch already checked out elsewhere, 2020-02-23) we add a condition to prevent a rebase operation involving a switch to a branch that is already checked out in another worktree. A bug has recently been fixed that caused this to not work as expected. Let's add a test to notice if this changes in the future. Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano --- t/t3400-rebase.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index d5a8ee39fc..3ce918fdb8 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -388,6 +388,20 @@ test_expect_success 'switch to branch checked out here' ' git rebase main main ' +test_expect_success 'switch to branch checked out elsewhere fails' ' + test_when_finished " + git worktree remove wt1 && + git worktree remove wt2 && + git branch -d shared + " && + git worktree add wt1 -b shared && + git worktree add wt2 -f shared && + # we test in both worktrees to ensure that works + # as expected with "first" and "next" worktrees + test_must_fail git -C wt1 rebase shared shared && + test_must_fail git -C wt2 rebase shared shared +' + test_expect_success 'switch to branch not checked out' ' git checkout main && git branch other && From 894ea945095b74542c6c4f4aefbe20f5b68be437 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Justo?= Date: Sat, 25 Feb 2023 15:22:23 +0100 Subject: [PATCH 4/4] switch: reject if the branch is already checked out elsewhere (test) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since 5883034 (checkout: reject if the branch is already checked out elsewhere) in normal use, we do not allow multiple worktrees having the same checked out branch. A bug has recently been fixed that caused this to not work as expected. Let's add a test to notice if this changes in the future. Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano --- t/t2060-switch.sh | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/t/t2060-switch.sh b/t/t2060-switch.sh index 5a7caf958c..e247a4735b 100755 --- a/t/t2060-switch.sh +++ b/t/t2060-switch.sh @@ -146,4 +146,33 @@ test_expect_success 'tracking info copied with autoSetupMerge=inherit' ' test_cmp_config "" --default "" branch.main2.merge ' +test_expect_success 'switch back when temporarily detached and checked out elsewhere ' ' + test_when_finished " + git worktree remove wt1 ||: + git worktree remove wt2 ||: + git checkout - ||: + git branch -D shared ||: + " && + git checkout -b shared && + test_commit shared-first && + HASH1=$(git rev-parse --verify HEAD) && + test_commit shared-second && + test_commit shared-third && + HASH2=$(git rev-parse --verify HEAD) && + git worktree add wt1 -f shared && + git -C wt1 bisect start && + git -C wt1 bisect good $HASH1 && + git -C wt1 bisect bad $HASH2 && + git worktree add wt2 -f shared && + git -C wt2 bisect start && + git -C wt2 bisect good $HASH1 && + git -C wt2 bisect bad $HASH2 && + # we test in both worktrees to ensure that works + # as expected with "first" and "next" worktrees + test_must_fail git -C wt1 switch shared && + git -C wt1 switch --ignore-other-worktrees shared && + test_must_fail git -C wt2 switch shared && + git -C wt2 switch --ignore-other-worktrees shared +' + test_done