{"thread":{"id":"13335","subject":"[PATCH] git-bisect.sh: don't accidentally override existing branch \"bisect\"","startedAt":"2008-04-30T16:46:13Z","lastAt":"2008-05-06T06:20:59Z","messageCount":11,"participants":["Gerrit Pape","Christian Couder","Richard Quirk","Karl Hasselström","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"75689","messageId":"20080430164613.28314.qmail@b31db398e1accc.315fe32.mid.smarden.org","threadId":"13335","inReplyTo":null,"subject":"[PATCH] git-bisect.sh: don't accidentally override existing branch \"bisect\"","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2008-04-30T16:46:13Z","receivedAt":"2008-04-30T16:46:13Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"If a branch named \"bisect\" or \"new-bisect\" already was created in the\nrepo by other means than git bisect, doing a git bisect used to override\nthe branch without a warning.  Now if the branch \"bisect\" or\n\"new-bisect\" already exists, and it was not created by git bisect itself,\ngit bisect start fails with an appropriate error message.  Additionally,\nif checking out a new bisect state fails due to a merge problem, git\nbisect cleans up the temporary branch \"new-bisect\".\n\nThe accidental override has been noticed by Andres Salomon, reported\nthrough\n http://bugs.debian.org/478647\n\nSigned-off-by: Gerrit Pape <pape@smarden.org>\n---\n Documentation/git-bisect.txt |    2 +-\n git-bisect.sh                |   20 ++++++++++++++------\n t/t6030-bisect-porcelain.sh  |   18 ++++++++++++++++++\n 3 files changed, 33 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 698ffde..1c7e38d 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -85,7 +85,7 @@ Oh, and then after you want to reset to the original head, do a\n $ git bisect reset\n ------------------------------------------------\n \n-to get back to the master branch, instead of being in one of the\n+to get back to the original branch, instead of being in one of the\n bisection branches (\"git bisect start\" will do that for you too,\n actually: it will reset the bisection state, and before it does that\n it checks that you're not using some old bisection branch).\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex d8d9bfd..48d81d5 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -69,14 +69,19 @@ bisect_start() {\n \thead=$(GIT_DIR=\"$GIT_DIR\" git symbolic-ref -q HEAD) ||\n \thead=$(GIT_DIR=\"$GIT_DIR\" git rev-parse --verify HEAD) ||\n \tdie \"Bad HEAD - I need a HEAD\"\n+\t#\n+\t# Check that we either already have BISECT_START, or that the\n+\t# branches bisect, new-bisect don't exist, to not override them.\n+\t#\n+\ttest -s \"$GIT_DIR/BISECT_START\" ||\n+\t\tif git show-ref bisect > /dev/null ||\n+\t\t    git show-ref new-bisect > /dev/null; then\n+\t\t\tdie 'The branches \"bisect\" and \"new-bisect\" must not exist.'\n+\t\tfi\n \tstart_head=''\n \tcase \"$head\" in\n \trefs/heads/bisect)\n-\t\tif [ -s \"$GIT_DIR/BISECT_START\" ]; then\n-\t\t    branch=`cat \"$GIT_DIR/BISECT_START\"`\n-\t\telse\n-\t\t    branch=master\n-\t\tfi\n+\t\tbranch=`cat \"$GIT_DIR/BISECT_START\"`\n \t\tgit checkout $branch || exit\n \t\t;;\n \trefs/heads/*|$_x40)\n@@ -329,7 +334,10 @@ bisect_next() {\n \n \techo \"Bisecting: $bisect_nr revisions left to test after this\"\n \tgit branch -f new-bisect \"$bisect_rev\"\n-\tgit checkout -q new-bisect || exit\n+\tgit checkout -q new-bisect || {\n+\t\tgit branch -d new-bisect\n+\t\texit\n+\t}\n \tgit branch -M new-bisect bisect\n \tgit show-branch \"$bisect_rev\"\n }\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 5e3e544..05f1e15 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -284,6 +284,24 @@ test_expect_success 'bisect starting with a detached HEAD' '\n \n '\n \n+test_expect_success 'bisect refuses to start if branch bisect exists' '\n+\tgit bisect reset &&\n+\tgit branch bisect &&\n+\ttest_must_fail git bisect start &&\n+\tgit branch -d bisect &&\n+\tgit checkout -b bisect &&\n+\ttest_must_fail git bisect start &&\n+\tgit checkout master &&\n+\tgit branch -d bisect\n+'\n+\n+test_expect_success 'bisect refuses to start if branch new-bisect exists' '\n+\tgit bisect reset &&\n+\tgit branch new-bisect &&\n+\ttest_must_fail git bisect start &&\n+\tgit branch -d new-bisect\n+'\n+\n #\n #\n test_done\n-- \n1.5.5.1\n"},{"id":"75716","messageId":"200804302330.18354.chriscool@tuxfamily.org","threadId":"13335","inReplyTo":"20080430164613.28314.qmail@b31db398e1accc.315fe32.mid.smarden.org","subject":"Re: [PATCH] git-bisect.sh: don't accidentally override existing branch \"bisect\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-04-30T21:30:18Z","receivedAt":"2008-04-30T21:30:18Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le mercredi 30 avril 2008, Gerrit Pape a écrit :\n> If a branch named \"bisect\" or \"new-bisect\" already was created in the\n> repo by other means than git bisect, doing a git bisect used to override\n> the branch without a warning.  Now if the branch \"bisect\" or\n> \"new-bisect\" already exists, and it was not created by git bisect itself,\n> git bisect start fails with an appropriate error message.  Additionally,\n> if checking out a new bisect state fails due to a merge problem, git\n> bisect cleans up the temporary branch \"new-bisect\".\n>\n> The accidental override has been noticed by Andres Salomon, reported\n> through\n>  http://bugs.debian.org/478647\n>\n> Signed-off-by: Gerrit Pape <pape@smarden.org>\n> ---\n>  Documentation/git-bisect.txt |    2 +-\n>  git-bisect.sh                |   20 ++++++++++++++------\n>  t/t6030-bisect-porcelain.sh  |   18 ++++++++++++++++++\n>  3 files changed, 33 insertions(+), 7 deletions(-)\n>\n> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\n> index 698ffde..1c7e38d 100644\n> --- a/Documentation/git-bisect.txt\n> +++ b/Documentation/git-bisect.txt\n> @@ -85,7 +85,7 @@ Oh, and then after you want to reset to the original\n> head, do a $ git bisect reset\n>  ------------------------------------------------\n>\n> -to get back to the master branch, instead of being in one of the\n> +to get back to the original branch, instead of being in one of the\n>  bisection branches (\"git bisect start\" will do that for you too,\n>  actually: it will reset the bisection state, and before it does that\n>  it checks that you're not using some old bisection branch).\n> diff --git a/git-bisect.sh b/git-bisect.sh\n> index d8d9bfd..48d81d5 100755\n> --- a/git-bisect.sh\n> +++ b/git-bisect.sh\n> @@ -69,14 +69,19 @@ bisect_start() {\n>  \thead=$(GIT_DIR=\"$GIT_DIR\" git symbolic-ref -q HEAD) ||\n>  \thead=$(GIT_DIR=\"$GIT_DIR\" git rev-parse --verify HEAD) ||\n>  \tdie \"Bad HEAD - I need a HEAD\"\n> +\t#\n> +\t# Check that we either already have BISECT_START, or that the\n> +\t# branches bisect, new-bisect don't exist, to not override them.\n> +\t#\n> +\ttest -s \"$GIT_DIR/BISECT_START\" ||\n> +\t\tif git show-ref bisect > /dev/null ||\n> +\t\t    git show-ref new-bisect > /dev/null; then\n> +\t\t\tdie 'The branches \"bisect\" and \"new-bisect\" must not exist.'\n> +\t\tfi\n\nMinor nitpick: you may use:\n\ngit show-ref -q {new-,}bisect\n\ninstead of:\n\ngit show-ref bisect > /dev/null ||\n\tgit show-ref new-bisect > /dev/null\n\nThat would give something like:\n\n\ttest -s \"$GIT_DIR/BISECT_START\" ||\n\t\tgit show-ref -q {new-,}bisect &&\n\t\t\tdie 'The branches \"bisect\" and \"new-bisect\" must not exist.'\n\n>  \tstart_head=''\n>  \tcase \"$head\" in\n>  \trefs/heads/bisect)\n> -\t\tif [ -s \"$GIT_DIR/BISECT_START\" ]; then\n> -\t\t    branch=`cat \"$GIT_DIR/BISECT_START\"`\n> -\t\telse\n> -\t\t    branch=master\n> -\t\tfi\n> +\t\tbranch=`cat \"$GIT_DIR/BISECT_START\"`\n>  \t\tgit checkout $branch || exit\n>  \t\t;;\n>  \trefs/heads/*|$_x40)\n> @@ -329,7 +334,10 @@ bisect_next() {\n>\n>  \techo \"Bisecting: $bisect_nr revisions left to test after this\"\n>  \tgit branch -f new-bisect \"$bisect_rev\"\n> -\tgit checkout -q new-bisect || exit\n> +\tgit checkout -q new-bisect || {\n> +\t\tgit branch -d new-bisect\n> +\t\texit\n\nHere we \"exit 0\" if \"git branch -d new-bisect\" succeeds.\nThat seems wrong.\n\n> +\t}\n>  \tgit branch -M new-bisect bisect\n>  \tgit show-branch \"$bisect_rev\"\n>  }\n> diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\n> index 5e3e544..05f1e15 100755\n> --- a/t/t6030-bisect-porcelain.sh\n> +++ b/t/t6030-bisect-porcelain.sh\n> @@ -284,6 +284,24 @@ test_expect_success 'bisect starting with a detached\n> HEAD' '\n>\n>  '\n>\n> +test_expect_success 'bisect refuses to start if branch bisect exists' '\n> +\tgit bisect reset &&\n> +\tgit branch bisect &&\n> +\ttest_must_fail git bisect start &&\n> +\tgit branch -d bisect &&\n> +\tgit checkout -b bisect &&\n> +\ttest_must_fail git bisect start &&\n> +\tgit checkout master &&\n> +\tgit branch -d bisect\n> +'\n> +\n> +test_expect_success 'bisect refuses to start if branch new-bisect\n> exists' ' +\tgit bisect reset &&\n> +\tgit branch new-bisect &&\n> +\ttest_must_fail git bisect start &&\n> +\tgit branch -d new-bisect\n> +'\n> +\n>  #\n>  #\n>  test_done\n\nOtherwise the patch looks good.\n\nThanks,\nChristian.\n"},{"id":"75748","messageId":"cac9e4380805010515h783dcf74h39fcc522c78885d3@mail.gmail.com","threadId":"13335","inReplyTo":"200804302330.18354.chriscool@tuxfamily.org","subject":"Re: [PATCH] git-bisect.sh: don't accidentally override existing branch \"bisect\"","fromName":"Richard Quirk","fromEmail":"richard.quirk@gmail.com","sentAt":"2008-05-01T12:15:12Z","receivedAt":"2008-05-01T12:15:12Z","isPatch":true,"sender":{"key":"richard.quirk@gmail.com","avatar":null},"body":"On Wed, Apr 30, 2008 at 11:30 PM, Christian Couder\n<chriscool@tuxfamily.org> wrote:\n\n>  Minor nitpick: you may use:\n>\n>  git show-ref -q {new-,}bisect\n>\n>  instead of:\n>\n>\n>  git show-ref bisect > /dev/null ||\n>         git show-ref new-bisect > /dev/null\n\nCareful with that - it's a bashism and would fail if /bin/sh is dash.\nie it would say that a branch called literally \"{new-,}bisect\" does\nnot exist, even if new-bisect and bisect do.\n\n(this time reply-all instead of just to Christian!)\n"},{"id":"75750","messageId":"200805011427.22901.chriscool@tuxfamily.org","threadId":"13335","inReplyTo":"cac9e4380805010515h783dcf74h39fcc522c78885d3@mail.gmail.com","subject":"Re: [PATCH] git-bisect.sh: don't accidentally override existing branch \"bisect\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-05-01T12:27:22Z","receivedAt":"2008-05-01T12:27:22Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le jeudi 1 mai 2008, Richard Quirk a écrit :\n> On Wed, Apr 30, 2008 at 11:30 PM, Christian Couder\n>\n> <chriscool@tuxfamily.org> wrote:\n> >  Minor nitpick: you may use:\n> >\n> >  git show-ref -q {new-,}bisect\n> >\n> >  instead of:\n> >\n> >\n> >  git show-ref bisect > /dev/null ||\n> >         git show-ref new-bisect > /dev/null\n>\n> Careful with that - it's a bashism and would fail if /bin/sh is dash.\n> ie it would say that a branch called literally \"{new-,}bisect\" does\n> not exist, even if new-bisect and bisect do.\n\nYou are right. Thanks.\nSo what about a plain:\n\ngit show-ref -q bisect new-bisect\n\nRegards,\nChristian.\n"},{"id":"75812","messageId":"20080502082232.GA20020@diana.vm.bytemark.co.uk","threadId":"13335","inReplyTo":"20080430164613.28314.qmail@b31db398e1accc.315fe32.mid.smarden.org","subject":"Re: [PATCH] git-bisect.sh: don't accidentally override existing branch \"bisect\"","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-05-02T08:22:32Z","receivedAt":"2008-05-02T08:22:32Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-04-30 16:46:13 +0000, Gerrit Pape wrote:\n\n> If a branch named \"bisect\" or \"new-bisect\" already was created in\n> the repo by other means than git bisect, doing a git bisect used to\n> override the branch without a warning. Now if the branch \"bisect\" or\n> \"new-bisect\" already exists, and it was not created by git bisect\n> itself, git bisect start fails with an appropriate error message.\n> Additionally, if checking out a new bisect state fails due to a\n> merge problem, git bisect cleans up the temporary branch\n> \"new-bisect\".\n\nMakes me wonder why bisect has to use a branch at all, and not just a\ndetached HEAD ... I seem to recall this having been discussed before,\nbut I can't find it now.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"75813","messageId":"20080502085620.3361.qmail@17e811992e6d42.315fe32.mid.smarden.org","threadId":"13335","inReplyTo":"200804302330.18354.chriscool@tuxfamily.org","subject":"[PATCH amend] git-bisect.sh: don't accidentally override existing branch \"bisect\"","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2008-05-02T08:56:20Z","receivedAt":"2008-05-02T08:56:20Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"If a branch named \"bisect\" or \"new-bisect\" already was created in the\nrepo by other means than git bisect, doing a git bisect used to override\nthe branch without a warning.  Now if the branch \"bisect\" or\n\"new-bisect\" already exists, and it was not created by git bisect itself,\ngit bisect start fails with an appropriate error message.  Additionally,\nif checking out a new bisect state fails due to a merge problem, git\nbisect cleans up the temporary branch \"new-bisect\".\n\nThe accidental override has been noticed by Andres Salomon, reported\nthrough\n http://bugs.debian.org/478647\n\nSigned-off-by: Gerrit Pape <pape@smarden.org>\n---\n\nOn Wed, Apr 30, 2008 at 11:30:18PM +0200, Christian Couder wrote:\n> >     echo \"Bisecting: $bisect_nr revisions left to test after this\"\n> >     git branch -f new-bisect \"$bisect_rev\"\n> > -   git checkout -q new-bisect || exit\n> > +   git checkout -q new-bisect || {\n> > +           git branch -d new-bisect\n> > +           exit\n>\n> Here we \"exit 0\" if \"git branch -d new-bisect\" succeeds.\n> That seems wrong.\n\nThanks, I changed it to first delete the new-bisect branch, and then use\ngit checkout to create the branch and do the checkout.  So we have the\nexit code from git branch if it fails.\n\nOn Thu, May 01, 2008 at 02:27:22PM +0200, Christian Couder wrote:\n> Le jeudi 1 mai 2008, Richard Quirk a écrit :\n> > Careful with that - it's a bashism and would fail if /bin/sh is\n> > dash.\n> > ie it would say that a branch called literally \"{new-,}bisect\" does\n> > not exist, even if new-bisect and bisect do.\n>\n> You are right. Thanks.\n> So what about a plain:\n>\n> git show-ref -q bisect new-bisect\n\nI'm not sure we can rely on this, the exit code of that command if one\nof the branches exists, but not the other, isn't documented.  The patch\nnow uses --verify -q on each branch, which is documented and should be\nreliable.\n\n\n Documentation/git-bisect.txt |    2 +-\n git-bisect.sh                |   19 ++++++++++++-------\n t/t6030-bisect-porcelain.sh  |   18 ++++++++++++++++++\n 3 files changed, 31 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 698ffde..1c7e38d 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -85,7 +85,7 @@ Oh, and then after you want to reset to the original head, do a\n $ git bisect reset\n ------------------------------------------------\n \n-to get back to the master branch, instead of being in one of the\n+to get back to the original branch, instead of being in one of the\n bisection branches (\"git bisect start\" will do that for you too,\n actually: it will reset the bisection state, and before it does that\n it checks that you're not using some old bisection branch).\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex d8d9bfd..f8c411a 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -69,14 +69,19 @@ bisect_start() {\n \thead=$(GIT_DIR=\"$GIT_DIR\" git symbolic-ref -q HEAD) ||\n \thead=$(GIT_DIR=\"$GIT_DIR\" git rev-parse --verify HEAD) ||\n \tdie \"Bad HEAD - I need a HEAD\"\n+\t#\n+\t# Check that we either already have BISECT_START, or that the\n+\t# branches bisect, new-bisect don't exist, to not override them.\n+\t#\n+\ttest -s \"$GIT_DIR/BISECT_START\" ||\n+\t\tif git show-ref --verify -q refs/heads/bisect ||\n+\t\t    git show-ref --verify -q refs/heads/new-bisect; then\n+\t\t\tdie 'The branches \"bisect\" and \"new-bisect\" must not exist.'\n+\t\tfi\n \tstart_head=''\n \tcase \"$head\" in\n \trefs/heads/bisect)\n-\t\tif [ -s \"$GIT_DIR/BISECT_START\" ]; then\n-\t\t    branch=`cat \"$GIT_DIR/BISECT_START\"`\n-\t\telse\n-\t\t    branch=master\n-\t\tfi\n+\t\tbranch=`cat \"$GIT_DIR/BISECT_START\"`\n \t\tgit checkout $branch || exit\n \t\t;;\n \trefs/heads/*|$_x40)\n@@ -328,8 +333,8 @@ bisect_next() {\n \texit_if_skipped_commits \"$bisect_rev\"\n \n \techo \"Bisecting: $bisect_nr revisions left to test after this\"\n-\tgit branch -f new-bisect \"$bisect_rev\"\n-\tgit checkout -q new-bisect || exit\n+\tgit branch -D new-bisect\n+\tgit checkout -q -b new-bisect \"$bisect_rev\" || exit\n \tgit branch -M new-bisect bisect\n \tgit show-branch \"$bisect_rev\"\n }\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 5e3e544..05f1e15 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -284,6 +284,24 @@ test_expect_success 'bisect starting with a detached HEAD' '\n \n '\n \n+test_expect_success 'bisect refuses to start if branch bisect exists' '\n+\tgit bisect reset &&\n+\tgit branch bisect &&\n+\ttest_must_fail git bisect start &&\n+\tgit branch -d bisect &&\n+\tgit checkout -b bisect &&\n+\ttest_must_fail git bisect start &&\n+\tgit checkout master &&\n+\tgit branch -d bisect\n+'\n+\n+test_expect_success 'bisect refuses to start if branch new-bisect exists' '\n+\tgit bisect reset &&\n+\tgit branch new-bisect &&\n+\ttest_must_fail git bisect start &&\n+\tgit branch -d new-bisect\n+'\n+\n #\n #\n test_done\n-- \n1.5.5.1\n"},{"id":"75856","messageId":"7v8wysy5bz.fsf@gitster.siamese.dyndns.org","threadId":"13335","inReplyTo":"20080502082232.GA20020@diana.vm.bytemark.co.uk","subject":"Re: [PATCH] git-bisect.sh: don't accidentally override existing branch \"bisect\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-02T17:38:08Z","receivedAt":"2008-05-02T17:38:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karl Hasselström <kha@treskal.com> writes:\n\n> On 2008-04-30 16:46:13 +0000, Gerrit Pape wrote:\n>\n>> If a branch named \"bisect\" or \"new-bisect\" already was created in\n>> the repo by other means than git bisect, doing a git bisect used to\n>> override the branch without a warning. Now if the branch \"bisect\" or\n>> \"new-bisect\" already exists, and it was not created by git bisect\n>> itself, git bisect start fails with an appropriate error message.\n>> Additionally, if checking out a new bisect state fails due to a\n>> merge problem, git bisect cleans up the temporary branch\n>> \"new-bisect\".\n>\n> Makes me wonder why bisect has to use a branch at all, and not just a\n> detached HEAD ... I seem to recall this having been discussed before,\n> but I can't find it now.\n\nOnly because the mechanism predates detached HEAD and no other reason.\nWhoever wants to update it to use detached HEAD needs to design what\nshould happen when the bisection was started while the HEAD is detached\n(should we come back to the same HEAD?  how? ...), but other than that I\ndo not offhand see fundamental difficulties.\n"},{"id":"75905","messageId":"200805031042.32190.chriscool@tuxfamily.org","threadId":"13335","inReplyTo":"20080502085620.3361.qmail@17e811992e6d42.315fe32.mid.smarden.org","subject":"Re: [PATCH amend] git-bisect.sh: don't accidentally override existing branch \"bisect\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-05-03T08:42:31Z","receivedAt":"2008-05-03T08:42:31Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le vendredi 2 mai 2008, Gerrit Pape a écrit :\n\n[...]\n> @@ -328,8 +333,8 @@ bisect_next() {\n>  \texit_if_skipped_commits \"$bisect_rev\"\n>\n>  \techo \"Bisecting: $bisect_nr revisions left to test after this\"\n> -\tgit branch -f new-bisect \"$bisect_rev\"\n> -\tgit checkout -q new-bisect || exit\n> +\tgit branch -D new-bisect\n\nDoesn't this output an error if the branch \"new-bisect\" does not exists ?\n\n$ git branch -D new-bisect\nerror: branch 'new-bisect' not found.\n\n> +\tgit checkout -q -b new-bisect \"$bisect_rev\" || exit\n>  \tgit branch -M new-bisect bisect\n>  \tgit show-branch \"$bisect_rev\"\n>  }\n\nThanks,\nChristian.\n"},{"id":"75919","messageId":"alpine.DEB.1.00.0805031347460.30431@racer","threadId":"13335","inReplyTo":"7v8wysy5bz.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-bisect.sh: don't accidentally override existing branch \"bisect\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-03T12:48:52Z","receivedAt":"2008-05-03T12:48:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 2 May 2008, Junio C Hamano wrote:\n\n> Karl Hasselström <kha@treskal.com> writes:\n> \n> > On 2008-04-30 16:46:13 +0000, Gerrit Pape wrote:\n> >\n> >> If a branch named \"bisect\" or \"new-bisect\" already was created in the \n> >> repo by other means than git bisect, doing a git bisect used to \n> >> override the branch without a warning. Now if the branch \"bisect\" or \n> >> \"new-bisect\" already exists, and it was not created by git bisect \n> >> itself, git bisect start fails with an appropriate error message. \n> >> Additionally, if checking out a new bisect state fails due to a merge \n> >> problem, git bisect cleans up the temporary branch \"new-bisect\".\n> >\n> > Makes me wonder why bisect has to use a branch at all, and not just a \n> > detached HEAD ... I seem to recall this having been discussed before, \n> > but I can't find it now.\n> \n> Only because the mechanism predates detached HEAD and no other reason. \n> Whoever wants to update it to use detached HEAD needs to design what \n> should happen when the bisection was started while the HEAD is detached \n> (should we come back to the same HEAD?  how? ...), but other than that I \n> do not offhand see fundamental difficulties.\n\nIMO it should behave as the rebase machinery does: record $(git rev-parse \nHEAD) in the case of a detached HEAD, and go back to that.  It is dead \neasy.\n\nCiao,\nDscho\n"},{"id":"76398","messageId":"20080505074300.5555.qmail@11d6801606c1fe.315fe32.mid.smarden.org","threadId":"13335","inReplyTo":"200805031042.32190.chriscool@tuxfamily.org","subject":"[PATCH amend] git-bisect.sh: don't accidentally override existing branch \"bisect\"","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2008-05-05T07:43:00Z","receivedAt":"2008-05-05T07:43:00Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"If a branch named \"bisect\" or \"new-bisect\" already was created in the\nrepo by other means than git bisect, doing a git bisect used to override\nthe branch without a warning.  Now if the branch \"bisect\" or\n\"new-bisect\" already exists, and it was not created by git bisect itself,\ngit bisect start fails with an appropriate error message.  Additionally,\nif checking out a new bisect state fails due to a merge problem, git\nbisect cleans up the temporary branch \"new-bisect\".\n\nThe accidental override has been noticed by Andres Salomon, reported\nthrough\n http://bugs.debian.org/478647\n\nSigned-off-by: Gerrit Pape <pape@smarden.org>\n---\n\nOn Sat, May 03, 2008 at 10:42:31AM +0200, Christian Couder wrote:\n> Le vendredi 2 mai 2008, Gerrit Pape a ?crit :\n> > -   git branch -f new-bisect \"$bisect_rev\"\n> > -   git checkout -q new-bisect || exit\n> > +   git branch -D new-bisect\n> \n> Doesn't this output an error if the branch \"new-bisect\" does not\n> exists ?\n\nIt does, thanks.  Another amend, direct stderr to /dev/null.\n\n\n Documentation/git-bisect.txt |    2 +-\n git-bisect.sh                |   19 ++++++++++++-------\n t/t6030-bisect-porcelain.sh  |   18 ++++++++++++++++++\n 3 files changed, 31 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 698ffde..1c7e38d 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -85,7 +85,7 @@ Oh, and then after you want to reset to the original head, do a\n $ git bisect reset\n ------------------------------------------------\n \n-to get back to the master branch, instead of being in one of the\n+to get back to the original branch, instead of being in one of the\n bisection branches (\"git bisect start\" will do that for you too,\n actually: it will reset the bisection state, and before it does that\n it checks that you're not using some old bisection branch).\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex d8d9bfd..b5171c9 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -69,14 +69,19 @@ bisect_start() {\n \thead=$(GIT_DIR=\"$GIT_DIR\" git symbolic-ref -q HEAD) ||\n \thead=$(GIT_DIR=\"$GIT_DIR\" git rev-parse --verify HEAD) ||\n \tdie \"Bad HEAD - I need a HEAD\"\n+\t#\n+\t# Check that we either already have BISECT_START, or that the\n+\t# branches bisect, new-bisect don't exist, to not override them.\n+\t#\n+\ttest -s \"$GIT_DIR/BISECT_START\" ||\n+\t\tif git show-ref --verify -q refs/heads/bisect ||\n+\t\t    git show-ref --verify -q refs/heads/new-bisect; then\n+\t\t\tdie 'The branches \"bisect\" and \"new-bisect\" must not exist.'\n+\t\tfi\n \tstart_head=''\n \tcase \"$head\" in\n \trefs/heads/bisect)\n-\t\tif [ -s \"$GIT_DIR/BISECT_START\" ]; then\n-\t\t    branch=`cat \"$GIT_DIR/BISECT_START\"`\n-\t\telse\n-\t\t    branch=master\n-\t\tfi\n+\t\tbranch=`cat \"$GIT_DIR/BISECT_START\"`\n \t\tgit checkout $branch || exit\n \t\t;;\n \trefs/heads/*|$_x40)\n@@ -328,8 +333,8 @@ bisect_next() {\n \texit_if_skipped_commits \"$bisect_rev\"\n \n \techo \"Bisecting: $bisect_nr revisions left to test after this\"\n-\tgit branch -f new-bisect \"$bisect_rev\"\n-\tgit checkout -q new-bisect || exit\n+\tgit branch -D new-bisect 2> /dev/null\n+\tgit checkout -q -b new-bisect \"$bisect_rev\" || exit\n \tgit branch -M new-bisect bisect\n \tgit show-branch \"$bisect_rev\"\n }\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 5e3e544..05f1e15 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -284,6 +284,24 @@ test_expect_success 'bisect starting with a detached HEAD' '\n \n '\n \n+test_expect_success 'bisect refuses to start if branch bisect exists' '\n+\tgit bisect reset &&\n+\tgit branch bisect &&\n+\ttest_must_fail git bisect start &&\n+\tgit branch -d bisect &&\n+\tgit checkout -b bisect &&\n+\ttest_must_fail git bisect start &&\n+\tgit checkout master &&\n+\tgit branch -d bisect\n+'\n+\n+test_expect_success 'bisect refuses to start if branch new-bisect exists' '\n+\tgit bisect reset &&\n+\tgit branch new-bisect &&\n+\ttest_must_fail git bisect start &&\n+\tgit branch -d new-bisect\n+'\n+\n #\n #\n test_done\n-- \n1.5.5.1\n"},{"id":"76176","messageId":"200805060820.59138.chriscool@tuxfamily.org","threadId":"13335","inReplyTo":"20080505074300.5555.qmail@11d6801606c1fe.315fe32.mid.smarden.org","subject":"Re: [PATCH amend] git-bisect.sh: don't accidentally override existing branch \"bisect\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-05-06T06:20:59Z","receivedAt":"2008-05-06T06:20:59Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le lundi 5 mai 2008, Gerrit Pape a écrit :\n> If a branch named \"bisect\" or \"new-bisect\" already was created in the\n> repo by other means than git bisect, doing a git bisect used to override\n> the branch without a warning.  Now if the branch \"bisect\" or\n> \"new-bisect\" already exists, and it was not created by git bisect itself,\n> git bisect start fails with an appropriate error message.  Additionally,\n> if checking out a new bisect state fails due to a merge problem, git\n> bisect cleans up the temporary branch \"new-bisect\".\n>\n> The accidental override has been noticed by Andres Salomon, reported\n> through\n>  http://bugs.debian.org/478647\n>\n> Signed-off-by: Gerrit Pape <pape@smarden.org>\n\nTested-by: Christian Couder <chriscool@tuxfamily.org>\n\nThis one looks good to me.\n\nThanks,\nChristian.\n\n> ---\n>\n> On Sat, May 03, 2008 at 10:42:31AM +0200, Christian Couder wrote:\n> > Le vendredi 2 mai 2008, Gerrit Pape a ?crit :\n> > > -   git branch -f new-bisect \"$bisect_rev\"\n> > > -   git checkout -q new-bisect || exit\n> > > +   git branch -D new-bisect\n> >\n> > Doesn't this output an error if the branch \"new-bisect\" does not\n> > exists ?\n>\n> It does, thanks.  Another amend, direct stderr to /dev/null.\n>\n>\n>  Documentation/git-bisect.txt |    2 +-\n>  git-bisect.sh                |   19 ++++++++++++-------\n>  t/t6030-bisect-porcelain.sh  |   18 ++++++++++++++++++\n>  3 files changed, 31 insertions(+), 8 deletions(-)\n>\n> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\n> index 698ffde..1c7e38d 100644\n> --- a/Documentation/git-bisect.txt\n> +++ b/Documentation/git-bisect.txt\n> @@ -85,7 +85,7 @@ Oh, and then after you want to reset to the original\n> head, do a $ git bisect reset\n>  ------------------------------------------------\n>\n> -to get back to the master branch, instead of being in one of the\n> +to get back to the original branch, instead of being in one of the\n>  bisection branches (\"git bisect start\" will do that for you too,\n>  actually: it will reset the bisection state, and before it does that\n>  it checks that you're not using some old bisection branch).\n> diff --git a/git-bisect.sh b/git-bisect.sh\n> index d8d9bfd..b5171c9 100755\n> --- a/git-bisect.sh\n> +++ b/git-bisect.sh\n> @@ -69,14 +69,19 @@ bisect_start() {\n>  \thead=$(GIT_DIR=\"$GIT_DIR\" git symbolic-ref -q HEAD) ||\n>  \thead=$(GIT_DIR=\"$GIT_DIR\" git rev-parse --verify HEAD) ||\n>  \tdie \"Bad HEAD - I need a HEAD\"\n> +\t#\n> +\t# Check that we either already have BISECT_START, or that the\n> +\t# branches bisect, new-bisect don't exist, to not override them.\n> +\t#\n> +\ttest -s \"$GIT_DIR/BISECT_START\" ||\n> +\t\tif git show-ref --verify -q refs/heads/bisect ||\n> +\t\t    git show-ref --verify -q refs/heads/new-bisect; then\n> +\t\t\tdie 'The branches \"bisect\" and \"new-bisect\" must not exist.'\n> +\t\tfi\n>  \tstart_head=''\n>  \tcase \"$head\" in\n>  \trefs/heads/bisect)\n> -\t\tif [ -s \"$GIT_DIR/BISECT_START\" ]; then\n> -\t\t    branch=`cat \"$GIT_DIR/BISECT_START\"`\n> -\t\telse\n> -\t\t    branch=master\n> -\t\tfi\n> +\t\tbranch=`cat \"$GIT_DIR/BISECT_START\"`\n>  \t\tgit checkout $branch || exit\n>  \t\t;;\n>  \trefs/heads/*|$_x40)\n> @@ -328,8 +333,8 @@ bisect_next() {\n>  \texit_if_skipped_commits \"$bisect_rev\"\n>\n>  \techo \"Bisecting: $bisect_nr revisions left to test after this\"\n> -\tgit branch -f new-bisect \"$bisect_rev\"\n> -\tgit checkout -q new-bisect || exit\n> +\tgit branch -D new-bisect 2> /dev/null\n> +\tgit checkout -q -b new-bisect \"$bisect_rev\" || exit\n>  \tgit branch -M new-bisect bisect\n>  \tgit show-branch \"$bisect_rev\"\n>  }\n> diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\n> index 5e3e544..05f1e15 100755\n> --- a/t/t6030-bisect-porcelain.sh\n> +++ b/t/t6030-bisect-porcelain.sh\n> @@ -284,6 +284,24 @@ test_expect_success 'bisect starting with a detached\n> HEAD' '\n>\n>  '\n>\n> +test_expect_success 'bisect refuses to start if branch bisect exists' '\n> +\tgit bisect reset &&\n> +\tgit branch bisect &&\n> +\ttest_must_fail git bisect start &&\n> +\tgit branch -d bisect &&\n> +\tgit checkout -b bisect &&\n> +\ttest_must_fail git bisect start &&\n> +\tgit checkout master &&\n> +\tgit branch -d bisect\n> +'\n> +\n> +test_expect_success 'bisect refuses to start if branch new-bisect\n> exists' ' +\tgit bisect reset &&\n> +\tgit branch new-bisect &&\n> +\ttest_must_fail git bisect start &&\n> +\tgit branch -d new-bisect\n> +'\n> +\n>  #\n>  #\n>  test_done\n"}]}