{"thread":{"id":"59135","subject":"[PATCH] bisect: fix \"reset\" when branch is checked out elsewhere","startedAt":"2023-01-22T01:38:16Z","lastAt":"2023-02-20T22:53:20Z","messageCount":15,"participants":["Rubén Justo","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"470855","messageId":"1c36c334-9f10-3859-c92f-3d889e226769@gmail.com","threadId":"59135","inReplyTo":null,"subject":"[PATCH] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2023-01-22T01:38:10Z","receivedAt":"2023-01-22T01:38:16Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"Since 1d0fa89 (checkout: add --ignore-other-wortrees, 2015-01-03) we\nhave a safety valve in checkout/switch to prevent the same branch from\nbeing checked out simultaneously in multiple worktrees.\n\nIf a branch is bisected in a worktree while also being checked out in\nanother worktree; when the bisection is finished, checking out the\nbranch back in the current worktree may fail.\n\nLet's teach bisect to use the \"--ignore-other-worktrees\" flag.\n\nSigned-off-by: Rubén Justo <rjusto@gmail.com>\n---\n builtin/bisect.c            |  3 ++-\n t/t6030-bisect-porcelain.sh | 23 +++++++++++++++++++++++\n 2 files changed, 25 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex cc9483e851..56da34648b 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -245,7 +245,8 @@ static int bisect_reset(const char *commit)\n \t\tstruct child_process cmd = CHILD_PROCESS_INIT;\n \n \t\tcmd.git_cmd = 1;\n-\t\tstrvec_pushl(&cmd.args, \"checkout\", branch.buf, \"--\", NULL);\n+\t\tstrvec_pushl(&cmd.args, \"checkout\", \"--ignore-other-worktrees\",\n+\t\t\t\tbranch.buf, \"--\", NULL);\n \t\tif (run_command(&cmd)) {\n \t\t\terror(_(\"could not check out original\"\n \t\t\t\t\" HEAD '%s'. Try 'git bisect\"\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 98a72ff78a..cc8acbab18 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -122,6 +122,29 @@ test_expect_success 'bisect start without -- takes unknown arg as pathspec' '\n \tgrep bar \".git/BISECT_NAMES\"\n '\n \n+test_expect_success 'bisect reset: back in a branch checked out also elsewhere' '\n+\techo \"shared\" > branch.expect &&\n+\ttest_bisect_reset() {\n+\t\tgit -C $1 bisect start &&\n+\t\tgit -C $1 bisect good $HASH1 &&\n+\t\tgit -C $1 bisect bad $HASH3 &&\n+\t\tgit -C $1 bisect reset &&\n+\t\tgit -C $1 branch --show-current > branch.output &&\n+\t\tcmp branch.expect branch.output\n+\t} &&\n+\ttest_when_finished \"\n+\t\tgit worktree remove wt1 &&\n+\t\tgit worktree remove wt2 &&\n+\t\tgit branch -d shared\n+\t\" &&\n+\tgit worktree add wt1 -b shared &&\n+\tgit worktree add wt2 -f shared &&\n+\t# we test in both worktrees to ensure that works\n+\t# as expected with \"first\" and \"next\" worktrees\n+\ttest_bisect_reset wt1 &&\n+\ttest_bisect_reset wt2\n+'\n+\n test_expect_success 'bisect reset: back in the main branch' '\n \tgit bisect reset &&\n \techo \"* main\" > branch.expect &&\n-- \n2.39.0\n"},{"id":"470895","messageId":"xmqqo7qqovp1.fsf@gitster.g","threadId":"59135","inReplyTo":"1c36c334-9f10-3859-c92f-3d889e226769@gmail.com","subject":"Re: [PATCH] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-23T02:01:46Z","receivedAt":"2023-01-23T02:01:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rubén Justo <rjusto@gmail.com> writes:\n\n> Since 1d0fa89 (checkout: add --ignore-other-wortrees, 2015-01-03) we\n> have a safety valve in checkout/switch to prevent the same branch from\n> being checked out simultaneously in multiple worktrees.\n>\n> If a branch is bisected in a worktree while also being checked out in\n> another worktree; when the bisection is finished, checking out the\n> branch back in the current worktree may fail.\n\nTrue.  But we should explain why failing is a bad thing here.  After\nall, \"git checkout foo\" to check out a branch 'foo' that is being\nused in a different worktree linked to the same repository fails,\nand that is a GOOD thing.  Is the logic behind your \"may fail and\nthat is a bad thing\" something like this?\n\n    When \"git bisect reset\" goes back to the branch, it used to error\n    out if the same branch is checked out in a different worktree.\n    Since this can happen only after the end-user deliberately checked\n    out the branch twice, erroring out does not contribute to any\n    safety.\n\nHaving said that...\n\n> @@ -245,7 +245,8 @@ static int bisect_reset(const char *commit)\n>  \t\tstruct child_process cmd = CHILD_PROCESS_INIT;\n>  \n>  \t\tcmd.git_cmd = 1;\n> -\t\tstrvec_pushl(&cmd.args, \"checkout\", branch.buf, \"--\", NULL);\n> +\t\tstrvec_pushl(&cmd.args, \"checkout\", \"--ignore-other-worktrees\",\n> +\t\t\t\tbranch.buf, \"--\", NULL);\n\n\"git bisect reset\" does take an arbitrary commit or branch name,\nwhich may not be the original branch the user was on.  If the\nuser did not have any branch checked out twice, can they do\nsomething like\n\n    $ git checkout foo\n    $ git bisect start HEAD HEAD~20\n    ... bisect session finds the first bad commit ...\n    $ git bisect reset bar\n\nwhere 'foo' is checked out only in this worktree?  What happens if\n'bar' has been checked out in a different worktree linked to the\nsame repository while this bisect was going on?  The current code\nmay fail due to the safety \"checkout\" has, but isn't that exactly\nwhat we want?  I.e. prevent 'bar' from being checked out twice by\nmistake?  Giving 'bar' on the command line of \"bisect reset\" is\nlikely because the user wants to start working on that branch,\nwithout necessarily knowing that they already have a worktree that\nchecked out the branch elsewhere---in other words, isn't that a lazy\nfolks' shorthand for \"git bisect reset && git checkout bar\"?\n\nIf we loosen the safety only when bisect_reset() receives NULL to\nits commit parameter, i.e. we are going back to the original branch,\nthe damage might be limited to narrower use cases, but I still am\nnot sure if the loosening is worth it.\n\nIIUC, the scenario that may be helped would go like this:\n\n    ... another worktree has 'foo' checked out ...\n    $ git checkout --ignore-other-worktrees foo\n    $ git bisect start HEAD HEAD~20\n    ... bisect session finds the first bad commit ...\n    $ git bisect reset\n\nThe last step wants to go back to 'foo', and it may be more\nconvenient if it did not fail to go back to the risky state the user\noriginally created.  But doesn't the error message already tell us\nhow to go back after this step refuses to recreate such a risky\nstate?  It sugests \"git bisect reset <commit>\" to switch to a\ndetached HEAD, so presumably, after seeing the above fail and\nreading the error message, the user could do\n\n    $ git bisect reset foo^{commit}\n\nto finish the bisect session and detach the head at 'foo', and then\nthe \"usual\" mantra to recreate the risky state that 'foo' is checked\nout twice can be done, i.e.\n\n    $ git checkout --ignore-other-worktrees foo\n\nSo, I am not sure if this is a good idea in general.\n\nOr do I misunderstand why you think \"checking out may fail\" is a bad\nthing?\n"},{"id":"471044","messageId":"0d04f8ed-6933-9354-1f64-24d827424c71@gmail.com","threadId":"59135","inReplyTo":"xmqqo7qqovp1.fsf@gitster.g","subject":"Re: [PATCH] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2023-01-26T02:18:23Z","receivedAt":"2023-01-26T02:18:29Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On 22-ene-2023 18:01:46, Junio C Hamano wrote:\n\n> Rubén Justo <rjusto@gmail.com> writes:\n> \n> > Since 1d0fa89 (checkout: add --ignore-other-wortrees, 2015-01-03) we\n> > have a safety valve in checkout/switch to prevent the same branch from\n> > being checked out simultaneously in multiple worktrees.\n> >\n> > If a branch is bisected in a worktree while also being checked out in\n> > another worktree; when the bisection is finished, checking out the\n> > branch back in the current worktree may fail.\n> \n> True.  But we should explain why failing is a bad thing here.  After\n> all, \"git checkout foo\" to check out a branch 'foo' that is being\n> used in a different worktree linked to the same repository fails,\n> and that is a GOOD thing.  Is the logic behind your \"may fail and\n> that is a bad thing\" something like this?\n> \n>     When \"git bisect reset\" goes back to the branch, it used to error\n>     out if the same branch is checked out in a different worktree.\n>     Since this can happen only after the end-user deliberately checked\n>     out the branch twice, erroring out does not contribute to any\n>     safety.\n> \n> Having said that...\n> \n> > @@ -245,7 +245,8 @@ static int bisect_reset(const char *commit)\n> >  \t\tstruct child_process cmd = CHILD_PROCESS_INIT;\n> >  \n> >  \t\tcmd.git_cmd = 1;\n> > -\t\tstrvec_pushl(&cmd.args, \"checkout\", branch.buf, \"--\", NULL);\n> > +\t\tstrvec_pushl(&cmd.args, \"checkout\", \"--ignore-other-worktrees\",\n> > +\t\t\t\tbranch.buf, \"--\", NULL);\n> \n> \"git bisect reset\" does take an arbitrary commit or branch name,\n> which may not be the original branch the user was on.  If the\n> user did not have any branch checked out twice, can they do\n> something like\n> \n>     $ git checkout foo\n>     $ git bisect start HEAD HEAD~20\n>     ... bisect session finds the first bad commit ...\n>     $ git bisect reset bar\n> \n> where 'foo' is checked out only in this worktree?  What happens if\n> 'bar' has been checked out in a different worktree linked to the\n> same repository while this bisect was going on?  The current code\n> may fail due to the safety \"checkout\" has, but isn't that exactly\n> what we want?  I.e. prevent 'bar' from being checked out twice by\n> mistake?  Giving 'bar' on the command line of \"bisect reset\" is\n> likely because the user wants to start working on that branch,\n> without necessarily knowing that they already have a worktree that\n> checked out the branch elsewhere---in other words, isn't that a lazy\n> folks' shorthand for \"git bisect reset && git checkout bar\"?\n> \n> If we loosen the safety only when bisect_reset() receives NULL to\n> its commit parameter, i.e. we are going back to the original branch,\n> the damage might be limited to narrower use cases, but I still am\n> not sure if the loosening is worth it.\n> \n> IIUC, the scenario that may be helped would go like this:\n> \n>     ... another worktree has 'foo' checked out ...\n>     $ git checkout --ignore-other-worktrees foo\n>     $ git bisect start HEAD HEAD~20\n>     ... bisect session finds the first bad commit ...\n>     $ git bisect reset\n> \n> The last step wants to go back to 'foo', and it may be more\n> convenient if it did not fail to go back to the risky state the user\n> originally created.  But doesn't the error message already tell us\n> how to go back after this step refuses to recreate such a risky\n> state?  It sugests \"git bisect reset <commit>\" to switch to a\n> detached HEAD, so presumably, after seeing the above fail and\n> reading the error message, the user could do\n> \n>     $ git bisect reset foo^{commit}\n> \n> to finish the bisect session and detach the head at 'foo', and then\n> the \"usual\" mantra to recreate the risky state that 'foo' is checked\n> out twice can be done, i.e.\n> \n>     $ git checkout --ignore-other-worktrees foo\n> \n> So, I am not sure if this is a good idea in general.\n> \n> Or do I misunderstand why you think \"checking out may fail\" is a bad\n> thing?\n\nMaybe we make it fail for no reason.  Thank you for thinking about this with\ndepth.\n\nI know you are aware, but for other readers;  In another thread, a bug-fix is\nbeing reviewed that, if accepted, will affect \"bisect\".  With that fix, the\nphrase \"checking out may fail\" will become \"checking out will fail\".  But does\nit need to fail?\n\nAs you said, we want to reject normal check outs of a branch when that\nbranch is already checked out in other worktrees.  Because of this, sounds\nreasonable to leave the implicit 'checkout' in 'bisect reset' to fail in those\ncases, and just add some tests to notice if this changes in the future.\n\nBut, and this is what makes me think that \"checking out will fail\" is the wrong\nchoice for \"bisect\", while bisecting, with the worktree in a detached HEAD\nstate, the branch to which \"bisect reset\" will check out back (BISECT_START),\nis still considered checked out in the working tree:\n\n\t$ git checkout -b work\n\t$ git bisect start HEAD HEAD~3\n\t... bisect detaches the current worktree ...\n\t$ git worktree add work\n\tPreparing worktree (checkout out 'work')\n\tfatal: 'work' is already checked out at ...\n\nSo, checking out back to the implicitly checked out branch sounds like it\nshould not fail.  And should be pretty secure (under the risk accepted by the\nuser) because we do not allow to normally check out the branch in another\nworktree.  We even reject the checkout in the current worktree while bisecting,\nbut let's leave that for another time.\n\nNote that due to the bug, what we are changing here is the reset on \"second\nworktrees\".  That means, \"bisect reset\" on the main worktree has been allowed\nfor some time, we are just making it official.\n\nAnd this is the less disrupting change in the usage.  Which does not justify\nthe change but does support it.\n\nThis is the reasoning I had and what makes me think that \"checking out may\nfail\" is an inconvenience for the user, without any gain.\n\nNow, if these arguments are reasonable, the next issue is what and how to\nallow.  I chose at least strict, but maybe we can do something more\nelaborate... just NULL sounds good.\n"},{"id":"471073","messageId":"xmqqzga5b4yz.fsf@gitster.g","threadId":"59135","inReplyTo":"0d04f8ed-6933-9354-1f64-24d827424c71@gmail.com","subject":"Re: [PATCH] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-26T17:06:28Z","receivedAt":"2023-01-26T17:06:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rubén Justo <rjusto@gmail.com> writes:\n\n> But, and this is what makes me think that \"checking out will fail\" is the wrong\n> choice for \"bisect\", while bisecting, with the worktree in a detached HEAD\n> state, the branch to which \"bisect reset\" will check out back (BISECT_START),\n> is still considered checked out in the working tree:\n>\n> \t$ git checkout -b work\n> \t$ git bisect start HEAD HEAD~3\n> \t... bisect detaches the current worktree ...\n> \t$ git worktree add work\n> \tPreparing worktree (checkout out 'work')\n> \tfatal: 'work' is already checked out at ...\n>\n> So, checking out back to the implicitly checked out branch sounds like it\n> should not fail.\n\nIf that is what you are aiming at, I suspect that the posted patch\nis doing it in a wrong way.  Instead, we should just declare that\nthe branch being bisected does not mean the branch cannot be checked\nout elsewhere, so that\n\n\t$ git worktree add --detach ../another HEAD^0\n\t$ git checkout -b work\n\t$ git bisect start work work~3\n        ... detaches ...\n\t$ git -C ../another checkout work\n\nshould just work, no?\n\nI admit I haven't thought things through, but I tend to be\nsympathetic to such a declaration.  After all, \"bisect\" is a\nread-only operation as far as the branch you happened to be on when\nyou started a bisect session is concerned.  Jumping around and\nmaterializing tree states recorded in various commits leading to the\ntip of the branch and inspecting them would not change anything on\nthe branch itself.  And more importantly, the branch being checked\nout in another worktree and modified there should not break the\nbisection, EXCEPT that the final \"git bisect reset\" (without\narguments) would fail if the other worktree removed the branch.\n\nSo, how about removing the is_worktree_being_bisected() check from\nfind_shared_symref(), so that not just \"worktree add\" and \"bisect\nreset\", but \"checkout\" and \"switch\" are allowed to make the branch\ncurrent even it is being bisected elsewhere?\n\nThat would affect the other topic, I suspect, as well.  It may be a\npositive change.  Or are there cases I missed, where the branch\nbeing bisected should not be touched from elsewhere, and we cannot\nremove the check from find_shared_symref()?\n\nThanks.\n\n\n\n"},{"id":"471074","messageId":"xmqqr0vhb4nx.fsf@gitster.g","threadId":"59135","inReplyTo":"xmqqzga5b4yz.fsf@gitster.g","subject":"Re: [PATCH] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-26T17:13:06Z","receivedAt":"2023-01-26T17:13:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> the branch itself.  And more importantly, the branch being checked\n> out in another worktree and modified there should not break the\n> bisection, EXCEPT that the final \"git bisect reset\" (without\n> arguments) would fail if the other worktree removed the branch.\n\nAnd \"bisect reset [<branch>]\" (with or without arguments) should not\nignore other worktrees when it runs \"checkout\" internally.  You\nmight have done\n\n    * checkout 'work' in worktree A\n    * start bisection of it there\n    * checkout 'work' in worktree B\n    * finish bisection of 'work' in worktree A\n    * \"git bisect reset\"\n\nand the third step should allow you work on 'work' in the other\nworktree, but then the last step should not allow 'work' to be\nchecked out in two places (it is OK for the user to use \"git bisect\nreset main\" and then \"git checkout --ignore-other work\" to work it\naround, of course, but the default should be safe).\n\nHmm?\n"},{"id":"471503","messageId":"a66218a3-919d-eca2-1859-41ac02aa38e7@gmail.com","threadId":"59135","inReplyTo":"xmqqzga5b4yz.fsf@gitster.g","subject":"Re: [PATCH] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2023-02-04T22:46:47Z","receivedAt":"2023-02-04T22:46:54Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On 26-ene-2023 09:06:28, Junio C Hamano wrote:\n\n> > But, and this is what makes me think that \"checking out will fail\" is the wrong\n> > choice for \"bisect\", while bisecting, with the worktree in a detached HEAD\n> > state, the branch to which \"bisect reset\" will check out back (BISECT_START),\n> > is still considered checked out in the working tree:\n> >\n> > \t$ git checkout -b work\n> > \t$ git bisect start HEAD HEAD~3\n> > \t... bisect detaches the current worktree ...\n> > \t$ git worktree add work\n> > \tPreparing worktree (checkout out 'work')\n> > \tfatal: 'work' is already checked out at ...\n> >\n> > So, checking out back to the implicitly checked out branch sounds like it\n> > should not fail.\n> \n> If that is what you are aiming at, I suspect that the posted patch\n> is doing it in a wrong way.  Instead, we should just declare that\n> the branch being bisected does not mean the branch cannot be checked\n> out elsewhere, so that\n> \n> \t$ git worktree add --detach ../another HEAD^0\n> \t$ git checkout -b work\n> \t$ git bisect start work work~3\n>         ... detaches ...\n> \t$ git -C ../another checkout work\n> \n> should just work, no?\n\nSorry, I should have been more explicit, I meant: \"checking out back to the\nimplicitly checked out branch sounds like it should not fail in the worktree\nwhere the user is bisecting\".\n\nSo, to your question: no, in another worktree should not work without\nthe --ignore-other-worktrees.  But\n\n\t$ git checkout -b work\n\t$ git worktree add -f ../another work\n\t$ git -C ../another bisect start work work~3\n\t  ... detaches ...\n\t$ git -C ../another bisect reset\n\nshould work.\n\n> So, how about removing the is_worktree_being_bisected() check from\n> find_shared_symref(), so that not just \"worktree add\" and \"bisect\n> reset\", but \"checkout\" and \"switch\" are allowed to make the branch\n> current even it is being bisected elsewhere?\n\nThe devil is in the details: \"git branch -m\", \"git branch -d\".\n\nWe're not ready to have BISECT_START pointing to a deleted branch, or\nrenaming a branch pointed by it.\n\nAlso the inconvenience that, if we allow the user to checkout normally\nthe same branch in another worktree, we are not providing any facility\nin \"git bisect reset\" to override the \"already checked out\".  We are\nforcing the user to \"git bisect reset HEAD~0 && git checkout\n--ignore-other-worktrees ...\".  Which, to me, sound as an\ninconvenience.\n\n> \n> That would affect the other topic, I suspect, as well.\n\nI'm not sure.  The other topic is somewhat independent of what we decide\nhere.\n\nThis series started because the other topic is going to affect \"git\nbisect\", not the other way around.  But this series can be\nconsidered even if the other topic is discarded.\n\nNow, \"git bisect reset\" with a branch checked out multiple times, works\nin the first worktree that has the branch checkedout (the main tree is\nalways the first), and fails in the next ones.  This is due to a bug\nthe other topic is fixing.\n\nThis series aims to make \"git bisect reset\" to the original branch, work\nin all worktrees.  Independently of the other topic.\n"},{"id":"471504","messageId":"ada28944-6e9e-d4e7-74c9-ffadaf406e1f@gmail.com","threadId":"59135","inReplyTo":"1c36c334-9f10-3859-c92f-3d889e226769@gmail.com","subject":"[PATCH v2] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2023-02-04T22:57:14Z","receivedAt":"2023-02-04T22:57:21Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"Since 1d0fa89 (checkout: add --ignore-other-wortrees, 2015-01-03) we\nhave a safety valve in checkout/switch to prevent the same branch from\nbeing checked out simultaneously in multiple worktrees.\n\nIf a branch is bisected in a worktree while also being checked out in\nanother worktree; when the bisection is finished, checking out the\nbranch back in the current worktree may fail.\n\nLet's teach bisect to use the \"--ignore-other-worktrees\" flag.\n\nSigned-off-by: Rubén Justo <rjusto@gmail.com>\n---\nRange-diff against v1:\n1:  f902db6bfb ! 1:  72e1526313 bisect: fix \"reset\" when branch is checked out elsewhere\n    @@ builtin/bisect.c: static int bisect_reset(const char *commit)\n      \n      \t\tcmd.git_cmd = 1;\n     -\t\tstrvec_pushl(&cmd.args, \"checkout\", branch.buf, \"--\", NULL);\n    -+\t\tstrvec_pushl(&cmd.args, \"checkout\", \"--ignore-other-worktrees\",\n    -+\t\t\t\tbranch.buf, \"--\", NULL);\n    ++\t\tstrvec_pushl(&cmd.args, \"checkout\", NULL);\n    ++\t\tif (!commit)\n    ++\t\t\tstrvec_pushl(&cmd.args, \"--ignore-other-worktrees\", NULL);\n    ++\t\tstrvec_pushl(&cmd.args, branch.buf, \"--\", NULL);\n      \t\tif (run_command(&cmd)) {\n      \t\t\terror(_(\"could not check out original\"\n      \t\t\t\t\" HEAD '%s'. Try 'git bisect\"\n\n builtin/bisect.c            |  5 ++++-\n t/t6030-bisect-porcelain.sh | 23 +++++++++++++++++++++++\n 2 files changed, 27 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 7301740267..46fba8db50 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -244,7 +244,10 @@ static int bisect_reset(const char *commit)\n \t\tstruct child_process cmd = CHILD_PROCESS_INIT;\n \n \t\tcmd.git_cmd = 1;\n-\t\tstrvec_pushl(&cmd.args, \"checkout\", branch.buf, \"--\", NULL);\n+\t\tstrvec_pushl(&cmd.args, \"checkout\", NULL);\n+\t\tif (!commit)\n+\t\t\tstrvec_pushl(&cmd.args, \"--ignore-other-worktrees\", NULL);\n+\t\tstrvec_pushl(&cmd.args, branch.buf, \"--\", NULL);\n \t\tif (run_command(&cmd)) {\n \t\t\terror(_(\"could not check out original\"\n \t\t\t\t\" HEAD '%s'. Try 'git bisect\"\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 3ba4fdf615..fb01bd6abc 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -122,6 +122,29 @@ test_expect_success 'bisect start without -- takes unknown arg as pathspec' '\n \tgrep bar \".git/BISECT_NAMES\"\n '\n \n+test_expect_success 'bisect reset: back in a branch checked out also elsewhere' '\n+\techo \"shared\" > branch.expect &&\n+\ttest_bisect_reset() {\n+\t\tgit -C $1 bisect start &&\n+\t\tgit -C $1 bisect good $HASH1 &&\n+\t\tgit -C $1 bisect bad $HASH3 &&\n+\t\tgit -C $1 bisect reset &&\n+\t\tgit -C $1 branch --show-current > branch.output &&\n+\t\tcmp branch.expect branch.output\n+\t} &&\n+\ttest_when_finished \"\n+\t\tgit worktree remove wt1 &&\n+\t\tgit worktree remove wt2 &&\n+\t\tgit branch -d shared\n+\t\" &&\n+\tgit worktree add wt1 -b shared &&\n+\tgit worktree add wt2 -f shared &&\n+\t# we test in both worktrees to ensure that works\n+\t# as expected with \"first\" and \"next\" worktrees\n+\ttest_bisect_reset wt1 &&\n+\ttest_bisect_reset wt2\n+'\n+\n test_expect_success 'bisect reset: back in the main branch' '\n \tgit bisect reset &&\n \techo \"* main\" > branch.expect &&\n-- \n2.39.0\n"},{"id":"471577","messageId":"xmqqwn4u7gyt.fsf@gitster.g","threadId":"59135","inReplyTo":"a66218a3-919d-eca2-1859-41ac02aa38e7@gmail.com","subject":"Re: [PATCH] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-06T19:04:58Z","receivedAt":"2023-02-06T19:05:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rubén Justo <rjusto@gmail.com> writes:\n\n> The devil is in the details: \"git branch -m\", \"git branch -d\".\n>\n> We're not ready to have BISECT_START pointing to a deleted branch, or\n> renaming a branch pointed by it.\n\nIt indicates that the callers of find_shared_symref() to see if \"is\nthis branch being actively used by checked out, bisected, or\nrebased, and I shouldn't touch it?\" need to know more than what\nfind_shared_symref() interface gives them---namely, \"can I repoint\nit to a different commit?\" and \"can I make it disappear?\" are\ndifferent conditions they need to be able to learn.\n\nUntil that distinction becomes expressible, I am actually OK with\nforbidding both operations, i.e. while a branch is being bisected\nelsewhere, we should be able to update its tip to point at a\ndifferent commit, but it is OK to forbid that because we cannot\nallow the branch be renamed away or removed.\n\nThanks.\n"},{"id":"471619","messageId":"xmqqwn4u2ztc.fsf@gitster.g","threadId":"59135","inReplyTo":"ada28944-6e9e-d4e7-74c9-ffadaf406e1f@gmail.com","subject":"Re: [PATCH v2] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-06T22:29:03Z","receivedAt":"2023-02-06T22:29:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rubén Justo <rjusto@gmail.com> writes:\n\n> Since 1d0fa89 (checkout: add --ignore-other-wortrees, 2015-01-03) we\n> have a safety valve in checkout/switch to prevent the same branch from\n> being checked out simultaneously in multiple worktrees.\n>\n> If a branch is bisected in a worktree while also being checked out in\n> another worktree; when the bisection is finished, checking out the\n> branch back in the current worktree may fail.\n\nSorry for asking possibly the same question again (which may mean\nthat the phrasing of this paragraph is misleading), but isn't it a\ngood thing if in this sequence:\n\n - I checkout 'main' and start bisecting (BISECT_HEAD says 'main');\n\n - I then checkout 'main' in another worktree; I may even make a\n   commit or two, or even rename 'main' to 'master'.\n\n - I finish bisection and \"bisect reset\" tries to take me back to\n   'main', which may notice that 'main' is checked out in the other\n   worktree, and fail.\n\nthe last one failed?  After the above sequence, I now have two\nworktrees, both checking out 'main', and it is exactly the situation\nthe safety valve tries to prevent from occuring, no?\n\nOr is the new behaviour considered better because the third step\nwould try to check out 'main' that is checked out elsewhere only if\nthe second step was forced, so the person who decided to touch\n'main' in another worktree should already be aware of the risk and\nwe should disable the safety valve in the third step automatically?\n\nI am not sure if that is a sensible argument, but if that is the\ncase, let's spell it out in the proposed log message.\n\nThanks.\n\n>  builtin/bisect.c            |  5 ++++-\n>  t/t6030-bisect-porcelain.sh | 23 +++++++++++++++++++++++\n>  2 files changed, 27 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/bisect.c b/builtin/bisect.c\n> index 7301740267..46fba8db50 100644\n> --- a/builtin/bisect.c\n> +++ b/builtin/bisect.c\n> @@ -244,7 +244,10 @@ static int bisect_reset(const char *commit)\n>  \t\tstruct child_process cmd = CHILD_PROCESS_INIT;\n>  \n>  \t\tcmd.git_cmd = 1;\n> -\t\tstrvec_pushl(&cmd.args, \"checkout\", branch.buf, \"--\", NULL);\n> +\t\tstrvec_pushl(&cmd.args, \"checkout\", NULL);\n> +\t\tif (!commit)\n> +\t\t\tstrvec_pushl(&cmd.args, \"--ignore-other-worktrees\", NULL);\n> +\t\tstrvec_pushl(&cmd.args, branch.buf, \"--\", NULL);\n\nOK, so this time around \"git bisect reset\" to go back to the\noriginal branch gets --ignore-other-worktrees but \"git bisect reset\nHEAD\" or other forms that names a branch still gets the safety.\nThat makes the blast radius smaller, but I am not 100% sure if\nloosening the safety is a good thing.\n"},{"id":"471744","messageId":"eb086063-9e47-9fdb-3644-77a843733dcf@gmail.com","threadId":"59135","inReplyTo":"xmqqwn4u2ztc.fsf@gitster.g","subject":"Re: [PATCH v2] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2023-02-08T00:30:28Z","receivedAt":"2023-02-08T00:30:37Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On 06-feb-2023 14:29:03, Junio C Hamano wrote:\n> Rubén Justo <rjusto@gmail.com> writes:\n> \n> > Since 1d0fa89 (checkout: add --ignore-other-wortrees, 2015-01-03) we\n> > have a safety valve in checkout/switch to prevent the same branch from\n> > being checked out simultaneously in multiple worktrees.\n> >\n> > If a branch is bisected in a worktree while also being checked out in\n> > another worktree; when the bisection is finished, checking out the\n> > branch back in the current worktree may fail.\n> \n> Sorry for asking possibly the same question again (which may mean\n\nNo problem.  I am sorry because I don't understand what's worrying you.\n\n> that the phrasing of this paragraph is misleading), but isn't it a\n> good thing if in this sequence:\n> \n>  - I checkout 'main' and start bisecting (BISECT_HEAD says 'main');\n> \n>  - I then checkout 'main' in another worktree; I may even make a\n>    commit or two, or even rename 'main' to 'master'.\n> \n>  - I finish bisection and \"bisect reset\" tries to take me back to\n>    'main', which may notice that 'main' is checked out in the other\n>    worktree, and fail.\n> \n> the last one failed?  After the above sequence, I now have two\n> worktrees, both checking out 'main', and it is exactly the situation\n> the safety valve tries to prevent from occuring, no?\n\nWe are considering the initial branch (BISECT_START) as a branch checked\nout _implicitly_ in the worktree that is bisecting.  Doesn't that\nprovide us and the user enough safety?  And is this not enough safety\nfor us to allow \"git bisect reset\"?  Just \"git bisect reset\", without\nany other argument.\n"},{"id":"471751","messageId":"xmqqo7q4vir8.fsf@gitster.g","threadId":"59135","inReplyTo":"eb086063-9e47-9fdb-3644-77a843733dcf@gmail.com","subject":"Re: [PATCH v2] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-08T05:16:59Z","receivedAt":"2023-02-08T05:17:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rubén Justo <rjusto@gmail.com> writes:\n\n> No problem.  I am sorry because I don't understand what's worrying you.\n>\n>> that the phrasing of this paragraph is misleading), but isn't it a\n>> good thing if in this sequence:\n>> \n>>  - I checkout 'main' and start bisecting (BISECT_HEAD says 'main');\n>> \n>>  - I then checkout 'main' in another worktree; I may even make a\n>>    commit or two, or even rename 'main' to 'master'.\n>> \n>>  - I finish bisection and \"bisect reset\" tries to take me back to\n>>    'main', which may notice that 'main' is checked out in the other\n>>    worktree, and fail.\n>> \n>> the last one failed?  After the above sequence, I now have two\n>> worktrees, both checking out 'main', and it is exactly the situation\n>> the safety valve tries to prevent from occuring, no?\n>\n> We are considering the initial branch (BISECT_START) as a branch checked\n> out _implicitly_ in the worktree that is bisecting.  Doesn't that\n> provide us and the user enough safety?\n\nIf that is a question, then the answer is no.  If that is\nrhetorical, then I just do not see how it gives us any safety.\n\nIn the end, if you allow \"bisect reset\" to check out 'main' in the\nworktree you used to run bisection, the 'main' branch is checked out\ntwice, once there, and another checkout in the other worktree.  That\nis exactly what \"git checkout 'main'\" in one worktree while 'main'\nis already checked out in another would prevent from happening, no?\n"},{"id":"471807","messageId":"d95d9505-db32-ebe6-1f23-05f7f00c2922@gmail.com","threadId":"59135","inReplyTo":"xmqqo7q4vir8.fsf@gitster.g","subject":"Re: [PATCH v2] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2023-02-08T21:54:53Z","receivedAt":"2023-02-08T21:55:01Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"\nOn 07-feb-2023 21:16:59, Junio C Hamano wrote:\n> Rubén Justo <rjusto@gmail.com> writes:\n> \n> > No problem.  I am sorry because I don't understand what's worrying you.\n> >\n> >> that the phrasing of this paragraph is misleading), but isn't it a\n> >> good thing if in this sequence:\n> >> \n> >>  - I checkout 'main' and start bisecting (BISECT_HEAD says 'main');\n> >> \n> >>  - I then checkout 'main' in another worktree; I may even make a\n> >>    commit or two, or even rename 'main' to 'master'.\n> >> \n> >>  - I finish bisection and \"bisect reset\" tries to take me back to\n> >>    'main', which may notice that 'main' is checked out in the other\n> >>    worktree, and fail.\n> >> \n> >> the last one failed?  After the above sequence, I now have two\n> >> worktrees, both checking out 'main', and it is exactly the situation\n> >> the safety valve tries to prevent from occuring, no?\n> >\n> > We are considering the initial branch (BISECT_START) as a branch checked\n> > out _implicitly_ in the worktree that is bisecting.  Doesn't that\n> > provide us and the user enough safety?\n> \n> If that is a question, then the answer is no.  If that is\n> rhetorical, then I just do not see how it gives us any safety.\n> \n> In the end, if you allow \"bisect reset\" to check out 'main' in the\n> worktree you used to run bisection, the 'main' branch is checked out\n> twice, once there, and another checkout in the other worktree.  That\n> is exactly what \"git checkout 'main'\" in one worktree while 'main'\n> is already checked out in another would prevent from happening, no?\n\nYes, but I still don't understand what you are worried about.\n\nIf we compare with \"rebase\": \"git rebase --abort\" does checkout back to\nthe original branch too.  But as \"git rebase\" is in a more evolved\n\"builtin transition\", and uses reset_head() instead of spawing a new Git\nwith \"checkout\", it avoids the \"--ignore-other-worktrees\".\n\nThe safety I'm considering with \"git bisect reset\" is that while a\nbranch is being bisected in a worktree, that branch cannot be (without\nforcing): checked out in another worktree, deleted or renamed.\n\nAnd this safety is enough, to me, to alleviate the user from having to\n\"git bisect reset\" to a \"safe\" place and then \"git checkout\n--ignore-other-worktrees\" to have the BISECT_START branch checked out\nagain.\n\nAlso, note that the aim of this patch is not to introduce a new behavior, but\nfix how \"git bisect reset\" works with multiple worktrees.\n"},{"id":"472120","messageId":"CAPig+cQPVMKui=AiXBHEfNWY5e4fMbxixdnzvZmXsSkW9ZMLyg@mail.gmail.com","threadId":"59135","inReplyTo":"ada28944-6e9e-d4e7-74c9-ffadaf406e1f@gmail.com","subject":"Re: [PATCH v2] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-02-15T04:52:08Z","receivedAt":"2023-02-15T04:52:23Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Feb 4, 2023 at 6:02 PM Rubén Justo <rjusto@gmail.com> wrote:\n> Since 1d0fa89 (checkout: add --ignore-other-wortrees, 2015-01-03) we\n> have a safety valve in checkout/switch to prevent the same branch from\n> being checked out simultaneously in multiple worktrees.\n>\n> If a branch is bisected in a worktree while also being checked out in\n> another worktree; when the bisection is finished, checking out the\n> branch back in the current worktree may fail.\n>\n> Let's teach bisect to use the \"--ignore-other-worktrees\" flag.\n>\n> Signed-off-by: Rubén Justo <rjusto@gmail.com>\n> ---\n> diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\n> @@ -122,6 +122,29 @@ test_expect_success 'bisect start without -- takes unknown arg as pathspec' '\n> +test_expect_success 'bisect reset: back in a branch checked out also elsewhere' '\n> +       echo \"shared\" > branch.expect &&\n> +       test_bisect_reset() {\n> +               git -C $1 bisect start &&\n> +               git -C $1 bisect good $HASH1 &&\n> +               git -C $1 bisect bad $HASH3 &&\n> +               git -C $1 bisect reset &&\n> +               git -C $1 branch --show-current > branch.output &&\n> +               cmp branch.expect branch.output\n> +       } &&\n> +       test_when_finished \"\n> +               git worktree remove wt1 &&\n> +               git worktree remove wt2 &&\n> +               git branch -d shared\n> +       \" &&\n\nAs mentioned in my review[1] of one of your other patches, &&-chaining\nwithin the argument to test_when_finished() is probably undesirable in\nthis case since failure of any cleanup command would cause\ntest_when_finish() to fail, which would cause the test to fail\noverall.\n\n[1]: https://lore.kernel.org/git/CAPig+cQpizjmhmDKb=HPrcYqqRq7JpvC-NZvY7B9eBbG+NrfKw@mail.gmail.com/\n\n> +       git worktree add wt1 -b shared &&\n> +       git worktree add wt2 -f shared &&\n> +       # we test in both worktrees to ensure that works\n> +       # as expected with \"first\" and \"next\" worktrees\n> +       test_bisect_reset wt1 &&\n> +       test_bisect_reset wt2\n> +'\n"},{"id":"472169","messageId":"1008f790-d2af-3b1f-50a3-6ce27f3b3df4@gmail.com","threadId":"59135","inReplyTo":"CAPig+cQPVMKui=AiXBHEfNWY5e4fMbxixdnzvZmXsSkW9ZMLyg@mail.gmail.com","subject":"Re: [PATCH v2] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2023-02-15T22:20:16Z","receivedAt":"2023-02-15T22:20:29Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On 14-feb-2023 23:52:08, Eric Sunshine wrote:\n> On Sat, Feb 4, 2023 at 6:02 PM Rubén Justo <rjusto@gmail.com> wrote:\n> > Since 1d0fa89 (checkout: add --ignore-other-wortrees, 2015-01-03) we\n> > have a safety valve in checkout/switch to prevent the same branch from\n> > being checked out simultaneously in multiple worktrees.\n> >\n> > If a branch is bisected in a worktree while also being checked out in\n> > another worktree; when the bisection is finished, checking out the\n> > branch back in the current worktree may fail.\n> >\n> > Let's teach bisect to use the \"--ignore-other-worktrees\" flag.\n> >\n> > Signed-off-by: Rubén Justo <rjusto@gmail.com>\n> > ---\n> > diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\n> > @@ -122,6 +122,29 @@ test_expect_success 'bisect start without -- takes unknown arg as pathspec' '\n> > +test_expect_success 'bisect reset: back in a branch checked out also elsewhere' '\n> > +       echo \"shared\" > branch.expect &&\n> > +       test_bisect_reset() {\n> > +               git -C $1 bisect start &&\n> > +               git -C $1 bisect good $HASH1 &&\n> > +               git -C $1 bisect bad $HASH3 &&\n> > +               git -C $1 bisect reset &&\n> > +               git -C $1 branch --show-current > branch.output &&\n> > +               cmp branch.expect branch.output\n> > +       } &&\n> > +       test_when_finished \"\n> > +               git worktree remove wt1 &&\n> > +               git worktree remove wt2 &&\n> > +               git branch -d shared\n> > +       \" &&\n> \n> As mentioned in my review[1] of one of your other patches, &&-chaining\n> within the argument to test_when_finished() is probably undesirable in\n> this case since failure of any cleanup command would cause\n> test_when_finish() to fail, which would cause the test to fail\n> overall.\n\nAs I said in my other response, if there is no argument against this\nchange, I'll reroll with it.\n\nThanks.\n\n> \n> [1]: https://lore.kernel.org/git/CAPig+cQpizjmhmDKb=HPrcYqqRq7JpvC-NZvY7B9eBbG+NrfKw@mail.gmail.com/\n> \n> > +       git worktree add wt1 -b shared &&\n> > +       git worktree add wt2 -f shared &&\n> > +       # we test in both worktrees to ensure that works\n> > +       # as expected with \"first\" and \"next\" worktrees\n> > +       test_bisect_reset wt1 &&\n> > +       test_bisect_reset wt2\n> > +'\n"},{"id":"472359","messageId":"6d70ba66-11b1-d596-ab0a-8643d6fc23a4@gmail.com","threadId":"59135","inReplyTo":"ada28944-6e9e-d4e7-74c9-ffadaf406e1f@gmail.com","subject":"[PATCH v3] bisect: fix \"reset\" when branch is checked out elsewhere","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2023-02-20T22:53:04Z","receivedAt":"2023-02-20T22:53:20Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"When the user starts a bisection in a worktree, we save the branch\ncurrently checked out in that worktree (BISECT_START) to allow the user\nto go back to that branch later on.\n\nFrom that moment and until the bisection ends, the branch is no longer\nchecked out in that worktree, but we give it the same consideration as\nif it was checked out, which means that we prevent:\n - deleting that branch and\n - checking out that branch in another worktree, unless the user force\n   the ckeck out.\n\n\"bisect\" was introduced as a helper script in 8cc6a08 ([PATCH] Making it\neasier to find which change introduced a bug, 2005-07-30), and\noriginally based its functioning in Git builtin commands.  Although\n\"bisect\" is now itself a builtin command, it still spawns Git\nsub-processes to use other builtin commands, like \"checkout\".\n\nSince 1d0fa89 (checkout: add --ignore-other-worktrees, 2015-01-03) we\nhave a safety valve in \"checkout\" and \"switch\" commands to prevent the\nuser from normally checking out simultaneously the same branch in\nmultiple worktrees.\n\nIf the user, using \"--ignore-other-worktrees\", checks out a branch\nbefore or while bisecting that same branch in another worktree, we may\nfail when the user ends the bisection using \"git bisect reset\":\n\n   $ git worktree add work ../work/\n   $ git checkout --ignore-other-worktrees work\n   $ git -C ../work bisect start HEAD HEAD~3\n   ... the user inspects some commits ...\n   $ git -C ../work bisect reset\n   fatal: 'work' is already checked out at ...\n   error: could not check out original HEAD 'work'. Try 'git bisect reset <commit>'.\n\nThanks to the special consideration we give to the branch while being\nbisected, and the safety valve introduced in 1d0fa89, we can assume the\nuser is aware and responsible of the \"multiple checked out branch\"\nsituation.  This makes sensible to allow the user to go back to the\noriginal branch using \"git bisect reset\" and avoid him a somewhat less\nintuitive sequence like: \"git bisect reset work~0 && git checkout\n--ignore-other-worktree work\".\n\nLet's teach \"bisect\" to use the \"--ignore-other-worktrees\" when the user\nwants to end the bisection.\n\nSigned-off-by: Rubén Justo <rjusto@gmail.com>\n---\n\nChanges since v2\n\n - Reworded the message to be more descriptive.\n - Using \"||:\" to chain commands in the test_when_finished block,\n   suggested by Eric.\n\nThis series started because of a bug, being fixed in another series,\nallowed \"git bisect reset\" to work in certain cases where multiple\nbranches were checked out.  Once the bug is fixed, \"git bisect reset\"\nwill no longer work in those scenarios.  Which is not bad, but taking\ninto account the different scenarios and the fact that some users may\nrely on this bug as a feature, I think that the current patch proposes a\nsensible and convenient change for the user.\n\nRange-diff against v2:\n1:  0fe0bc70dd ! 1:  3b2ac1aa8f bisect: fix \"reset\" when branch is checked out elsewhere\n    @@ Metadata\n      ## Commit message ##\n         bisect: fix \"reset\" when branch is checked out elsewhere\n     \n    -    Since 1d0fa89 (checkout: add --ignore-other-wortrees, 2015-01-03) we\n    -    have a safety valve in checkout/switch to prevent the same branch from\n    -    being checked out simultaneously in multiple worktrees.\n    +    When the user starts a bisection in a worktree, we save the branch\n    +    currently checked out in that worktree (BISECT_START) to allow the user\n    +    to go back to that branch later on.\n     \n    -    If a branch is bisected in a worktree while also being checked out in\n    -    another worktree; when the bisection is finished, checking out the\n    -    branch back in the current worktree may fail.\n    +    From that moment and until the bisection ends, the branch is no longer\n    +    checked out in that worktree, but we give it the same consideration as\n    +    if it was checked out, which means that we prevent:\n    +     - deleting that branch and\n    +     - checking out that branch in another worktree, unless the user force\n    +       the ckeck out.\n     \n    -    Let's teach bisect to use the \"--ignore-other-worktrees\" flag.\n    +    \"bisect\" was introduced as a helper script in 8cc6a08 ([PATCH] Making it\n    +    easier to find which change introduced a bug, 2005-07-30), and\n    +    originally based its functioning in Git builtin commands.  Although\n    +    \"bisect\" is now itself a builtin command, it still spawns Git\n    +    sub-processes to use other builtin commands, like \"checkout\".\n    +\n    +    Since 1d0fa89 (checkout: add --ignore-other-worktrees, 2015-01-03) we\n    +    have a safety valve in \"checkout\" and \"switch\" commands to prevent the\n    +    user from normally checking out simultaneously the same branch in\n    +    multiple worktrees.\n    +\n    +    If the user, using \"--ignore-other-worktrees\", checks out a branch\n    +    before or while bisecting that same branch in another worktree, we may\n    +    fail when the user ends the bisection using \"git bisect reset\":\n    +\n    +       $ git worktree add work ../work/\n    +       $ git checkout --ignore-other-worktrees work\n    +       $ git -C ../work bisect start HEAD HEAD~3\n    +       ... the user inspects some commits ...\n    +       $ git -C ../work bisect reset\n    +       fatal: 'work' is already checked out at ...\n    +       error: could not check out original HEAD 'work'. Try 'git bisect reset <commit>'.\n    +\n    +    Thanks to the special consideration we give to the branch while being\n    +    bisected, and the safety valve introduced in 1d0fa89, we can assume the\n    +    user is aware and responsible of the \"multiple checked out branch\"\n    +    situation.  This makes sensible to allow the user to go back to the\n    +    original branch using \"git bisect reset\" and avoid him a somewhat less\n    +    intuitive sequence like: \"git bisect reset work~0 && git checkout\n    +    --ignore-other-worktree work\".\n    +\n    +    Let's teach \"bisect\" to use the \"--ignore-other-worktrees\" when the user\n    +    wants to end the bisection.\n     \n         Signed-off-by: Rubén Justo <rjusto@gmail.com>\n     \n    @@ builtin/bisect.c: static int bisect_reset(const char *commit)\n     -\t\tstrvec_pushl(&cmd.args, \"checkout\", branch.buf, \"--\", NULL);\n     +\t\tstrvec_pushl(&cmd.args, \"checkout\", NULL);\n     +\t\tif (!commit)\n    -+\t\t\tstrvec_pushl(&cmd.args, \"--ignore-other-worktrees\", NULL);\n    ++\t\t\tstrvec_pushl(&cmd.args, \"--ignore-other-worktrees\",\n    ++\t\t\t\t     NULL);\n     +\t\tstrvec_pushl(&cmd.args, branch.buf, \"--\", NULL);\n      \t\tif (run_command(&cmd)) {\n      \t\t\terror(_(\"could not check out original\"\n    @@ t/t6030-bisect-porcelain.sh: test_expect_success 'bisect start without -- takes\n     +\t\tcmp branch.expect branch.output\n     +\t} &&\n     +\ttest_when_finished \"\n    -+\t\tgit worktree remove wt1 &&\n    -+\t\tgit worktree remove wt2 &&\n    -+\t\tgit branch -d shared\n    ++\t\tgit worktree remove wt1 ||:\n    ++\t\tgit worktree remove wt2 ||:\n    ++\t\tgit branch -d shared ||:\n     +\t\" &&\n     +\tgit worktree add wt1 -b shared &&\n     +\tgit worktree add wt2 -f shared &&\n\n builtin/bisect.c            |  6 +++++-\n t/t6030-bisect-porcelain.sh | 23 +++++++++++++++++++++++\n 2 files changed, 28 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 7301740267..011f674b08 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -244,7 +244,11 @@ static int bisect_reset(const char *commit)\n \t\tstruct child_process cmd = CHILD_PROCESS_INIT;\n \n \t\tcmd.git_cmd = 1;\n-\t\tstrvec_pushl(&cmd.args, \"checkout\", branch.buf, \"--\", NULL);\n+\t\tstrvec_pushl(&cmd.args, \"checkout\", NULL);\n+\t\tif (!commit)\n+\t\t\tstrvec_pushl(&cmd.args, \"--ignore-other-worktrees\",\n+\t\t\t\t     NULL);\n+\t\tstrvec_pushl(&cmd.args, branch.buf, \"--\", NULL);\n \t\tif (run_command(&cmd)) {\n \t\t\terror(_(\"could not check out original\"\n \t\t\t\t\" HEAD '%s'. Try 'git bisect\"\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 3ba4fdf615..1d047f1c1a 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -122,6 +122,29 @@ test_expect_success 'bisect start without -- takes unknown arg as pathspec' '\n \tgrep bar \".git/BISECT_NAMES\"\n '\n \n+test_expect_success 'bisect reset: back in a branch checked out also elsewhere' '\n+\techo \"shared\" > branch.expect &&\n+\ttest_bisect_reset() {\n+\t\tgit -C $1 bisect start &&\n+\t\tgit -C $1 bisect good $HASH1 &&\n+\t\tgit -C $1 bisect bad $HASH3 &&\n+\t\tgit -C $1 bisect reset &&\n+\t\tgit -C $1 branch --show-current > branch.output &&\n+\t\tcmp branch.expect branch.output\n+\t} &&\n+\ttest_when_finished \"\n+\t\tgit worktree remove wt1 ||:\n+\t\tgit worktree remove wt2 ||:\n+\t\tgit branch -d shared ||:\n+\t\" &&\n+\tgit worktree add wt1 -b shared &&\n+\tgit worktree add wt2 -f shared &&\n+\t# we test in both worktrees to ensure that works\n+\t# as expected with \"first\" and \"next\" worktrees\n+\ttest_bisect_reset wt1 &&\n+\ttest_bisect_reset wt2\n+'\n+\n test_expect_success 'bisect reset: back in the main branch' '\n \tgit bisect reset &&\n \techo \"* main\" > branch.expect &&\n-- \n2.34.1\n"}]}