{"thread":{"id":"52844","subject":"git rebase allows branches to be checked out in multiple worktrees","startedAt":"2020-02-20T00:29:42Z","lastAt":"2020-02-24T09:20:08Z","messageCount":5,"participants":["Mike Hommey","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"392132","messageId":"20200220002932.5jws6qpnivlvxckw@glandium.org","threadId":"52844","inReplyTo":null,"subject":"git rebase allows branches to be checked out in multiple worktrees","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2020-02-20T00:29:32Z","receivedAt":"2020-02-20T00:29:42Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"Hi,\n\nI just noticed a quirk with git rebase vs. worktrees. While using e.g.\ngit checkout will prevent a same branch from being checked out from\nmultiple worktrees, that's not the case for git-rebase, so the following\nscenario ends up with two checkouts of the same branch:\n\n$ git checkout -b foo HEAD~\n$ git commit --allow-empty -m foo \n$ git worktree add /tmp/bar master\n$ cd /tmp/bar\n$ git rebase master foo\n\nMike\n"},{"id":"392351","messageId":"20200223101407.77036-2-sunshine@sunshineco.com","threadId":"52844","inReplyTo":"20200223101407.77036-1-sunshine@sunshineco.com","subject":"[PATCH 1/2] t3400: make test clean up after itself","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-02-23T10:14:06Z","receivedAt":"2020-02-23T10:15:08Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This test intentionally creates a file which causes rebase to fail, thus\nit is important that this file be deleted before subsequent tests are\nrun which are not expecting such a failure. In the past, the common way\nto ensure cleanup (regardless of whether the test succeeded or failed)\nwas either for the next test to perform the previous test's cleanup as\nits first step or to do the cleanup at global scope outside of any\ntests. With the introduction of 'test_when_finished', however, tests can\nbe responsible for their own cleanup. Therefore, update this test to\nclean up after itself.\n\nA bit of history: This 'rm' invocation was moved from within the body of\nthe following test to global scope by bffd750adf (rebase: improve error\nmessage when upstream argument is missing, 2010-05-31), which postdates,\nby about a month, introduction of 'test_when_finished' in 3bf7886705\n(test-lib: Let tests specify commands to be run at end of test,\n2010-05-02).\n---\n t/t3400-rebase.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 221b35f2df..6e746dca00 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -143,11 +143,11 @@ test_expect_success 'setup: recover' '\n \n test_expect_success 'Show verbose error when HEAD could not be detached' '\n \t>B &&\n+\ttest_when_finished \"rm -f B\" &&\n \ttest_must_fail git rebase topic 2>output.err >output.out &&\n \ttest_i18ngrep \"The following untracked working tree files would be overwritten by checkout:\" output.err &&\n \ttest_i18ngrep B output.err\n '\n-rm -f B\n \n test_expect_success 'fail when upstream arg is missing and not on branch' '\n \tgit checkout topic &&\n-- \n2.25.1.526.gf05a752211\n\n"},{"id":"392352","messageId":"20200223101407.77036-1-sunshine@sunshineco.com","threadId":"52844","inReplyTo":"20200220002932.5jws6qpnivlvxckw@glandium.org","subject":"[PATCH 0/2] git-rebase: refuse to switch to branch checked out elsewhere","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-02-23T10:14:05Z","receivedAt":"2020-02-23T10:15:08Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"git-switch, git-checkout, and git-worktree all refuse to switch to a\nbranch already checked out in some other worktree. Mike Hommey\nreported[1] that git-rebase is not careful like those other commands.\nThis patch series fixes that shortcoming.\n\n[1]: https://lore.kernel.org/git/20200220002932.5jws6qpnivlvxckw@glandium.org/\n\nEric Sunshine (2):\n t3400: make test clean up after itself\n rebase: refuse to switch to branch already checked out elsewhere\n\n builtin/rebase.c | 5 +++--\n t/t3400-rebase.sh | 20 +++++++++++++++++++-\n 2 files changed, 22 insertions(+), 3 deletions(-)\n\n-- \n2.25.1.526.gf05a752211\n\n"},{"id":"392353","messageId":"20200223101407.77036-3-sunshine@sunshineco.com","threadId":"52844","inReplyTo":"20200223101407.77036-1-sunshine@sunshineco.com","subject":"[PATCH 2/2] rebase: refuse to switch to branch already checked out elsewhere","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-02-23T10:14:07Z","receivedAt":"2020-02-23T10:15:11Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The invocation \"git rebase \" switches to \nbefore performing the rebase operation. However, unlike git-switch,\ngit-checkout, and git-worktree which all refuse to switch to a branch\nthat is already checked out in some other worktree, git-rebase switches\nto unconditionally. Curb this careless behavior by making\ngit-rebase also refuse to switch to a branch checked out elsewhere.\n\nReported-by: Mike Hommey \nSigned-off-by: Eric Sunshine \n---\n builtin/rebase.c | 5 +++--\n t/t3400-rebase.sh | 18 ++++++++++++++++++\n 2 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 6154ad8fa5..41b9372c0b 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1945,10 +1945,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\t/* Is it a local branch? */\n \t\tstrbuf_reset(&buf);\n \t\tstrbuf_addf(&buf, \"refs/heads/%s\", branch_name);\n-\t\tif (!read_ref(buf.buf, &options.orig_head))\n+\t\tif (!read_ref(buf.buf, &options.orig_head)) {\n+\t\t\tdie_if_checked_out(buf.buf, 1);\n \t\t\toptions.head_name = xstrdup(buf.buf);\n \t\t/* If not is it a valid ref (branch or commit)? */\n-\t\telse if (!get_oid(branch_name, &options.orig_head))\n+\t\t} else if (!get_oid(branch_name, &options.orig_head))\n \t\t\toptions.head_name = NULL;\n \t\telse\n \t\t\tdie(_(\"fatal: no such branch/commit '%s'\"),\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 6e746dca00..9aa5268a06 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -377,4 +377,22 @@ test_expect_success 'rebase -c rebase.useBuiltin=false warning' '\n \ttest_must_be_empty err\n '\n \n+test_expect_success 'switch to branch checked out here' '\n+\tgit checkout master &&\n+\tgit rebase master master\n+'\n+\n+test_expect_success 'switch to branch not checked out' '\n+\tgit checkout master &&\n+\tgit branch other &&\n+\tgit rebase master other\n+'\n+\n+test_expect_success 'refuse to switch to branch checked out elsewhere' '\n+\tgit checkout master &&\n+\tgit worktree add wt &&\n+\ttest_must_fail git -C wt rebase master master 2>err &&\n+\ttest_i18ngrep \"already checked out\" err\n+'\n+\n test_done\n-- \n2.25.1.526.gf05a752211\n\n"},{"id":"392401","messageId":"CAPig+cQBSCius8a2eDsyOZb3-p8nd-aNqgECgxM9+iTmBSGW5g@mail.gmail.com","threadId":"52844","inReplyTo":"20200223101407.77036-2-sunshine@sunshineco.com","subject":"Re: [PATCH 1/2] t3400: make test clean up after itself","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-02-24T09:19:54Z","receivedAt":"2020-02-24T09:20:08Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Feb 23, 2020 at 5:15 AM Eric Sunshine wrote:\n> This test intentionally creates a file which causes rebase to fail, thus\n> it is important that this file be deleted before subsequent tests are\n> run which are not expecting such a failure. In the past, the common way\n> to ensure cleanup (regardless of whether the test succeeded or failed)\n> was either for the next test to perform the previous test's cleanup as\n> its first step or to do the cleanup at global scope outside of any\n> tests. With the introduction of 'test_when_finished', however, tests can\n> be responsible for their own cleanup. Therefore, update this test to\n> clean up after itself.\n>\n> A bit of history: This 'rm' invocation was moved from within the body of\n> the following test to global scope by bffd750adf (rebase: improve error\n> message when upstream argument is missing, 2010-05-31), which postdates,\n> by about a month, introduction of 'test_when_finished' in 3bf7886705\n> (test-lib: Let tests specify commands to be run at end of test,\n> 2010-05-02).\n> ---\n\nMissing sign-off, so here it is:\n\nSigned-off-by: Eric Sunshine \n"}]}