{"thread":{"id":"13070","subject":"[PATCH v2] bisect: fix bad rev checking in \"git bisect good\"","startedAt":"2008-04-12T07:03:35Z","lastAt":"2008-04-12T09:17:36Z","messageCount":2,"participants":["Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"74185","messageId":"20080412090335.e92d3da3.chriscool@tuxfamily.org","threadId":"13070","inReplyTo":null,"subject":"[PATCH v2] bisect: fix bad rev checking in \"git bisect good\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-04-12T07:03:35Z","receivedAt":"2008-04-12T07:03:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"It seems that \"git bisect good\" and \"git bisect skip\" have never\nproperly checked arguments that have been passed to them. As soon\nas one of them can be parsed as a SHA1, no error or warning would\nbe given.\n\nThis is because 'git rev-parse --revs-only --no-flags \"$@\"' always\n\"exit 0\" and outputs all the SHA1 it can found from parsing \"$@\".\n\nThis patch fix this by using, for each \"bisect good\" argument, the\nsame logic as for the \"bisect bad\" argument.\n\nWhile at it, this patch teaches \"bisect bad\" to give a meaningfull\nerror message when it is passed more than one argument.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n git-bisect.sh               |   18 +++++++-----------\n t/t6030-bisect-porcelain.sh |   13 +++++++++++++\n 2 files changed, 20 insertions(+), 11 deletions(-)\n\n\tThis new version should give a better error message\n\tthan the previous one.\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex a1343f6..408775a 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -155,20 +155,16 @@ bisect_state() {\n \t\trev=$(git rev-parse --verify HEAD) ||\n \t\t\tdie \"Bad rev input: HEAD\"\n \t\tbisect_write \"$state\" \"$rev\" ;;\n-\t2,bad)\n-\t\trev=$(git rev-parse --verify \"$2^{commit}\") ||\n-\t\t\tdie \"Bad rev input: $2\"\n-\t\tbisect_write \"$state\" \"$rev\" ;;\n-\t*,good|*,skip)\n+\t2,bad|*,good|*,skip)\n \t\tshift\n-\t\trevs=$(git rev-parse --revs-only --no-flags \"$@\") &&\n-\t\t\ttest '' != \"$revs\" || die \"Bad rev input: $@\"\n-\t\tfor rev in $revs\n+\t\tfor rev in \"$@\"\n \t\tdo\n-\t\t\trev=$(git rev-parse --verify \"$rev^{commit}\") ||\n-\t\t\t\tdie \"Bad rev commit: $rev^{commit}\"\n-\t\t\tbisect_write \"$state\" \"$rev\"\n+\t\t\tsha=$(git rev-parse --verify \"$rev^{commit}\") ||\n+\t\t\t\tdie \"Bad rev input: $rev\"\n+\t\t\tbisect_write \"$state\" \"$sha\"\n \t\tdone ;;\n+\t*,bad)\n+\t\tdie \"'git bisect bad' can take only one argument.\" ;;\n \t*)\n \t\tusage ;;\n \tesac\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex f471c15..32d6118 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -71,6 +71,19 @@ test_expect_success 'bisect start with one bad and good' '\n \tgit bisect next\n '\n \n+test_expect_success 'bisect good and bad fails if not given only revs' '\n+\tgit bisect reset &&\n+\tgit bisect start &&\n+\ttest_must_fail git bisect good foo $HASH1 &&\n+\ttest_must_fail git bisect good $HASH1 bar &&\n+\ttest_must_fail git bisect bad frotz &&\n+\ttest_must_fail git bisect bad $HASH3 $HASH4 &&\n+\ttest_must_fail git bisect skip bar $HASH3 &&\n+\ttest_must_fail git bisect skip $HASH1 foo &&\n+\tgit bisect good $HASH1 &&\n+\tgit bisect bad $HASH4\n+'\n+\n test_expect_success 'bisect reset: back in the master branch' '\n \tgit bisect reset &&\n \techo \"* master\" > branch.expect &&\n-- \n1.5.5.46.gb6f72.dirty\n"},{"id":"74186","messageId":"7vd4ovjutr.fsf@gitster.siamese.dyndns.org","threadId":"13070","inReplyTo":"20080412090335.e92d3da3.chriscool@tuxfamily.org","subject":"Re* [PATCH v2] bisect: fix bad rev checking in \"git bisect good\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-12T09:17:36Z","receivedAt":"2008-04-12T09:17:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I've applied your earlier one to maint and propagated it all the way up,\nand pushed the results out for tonight.  I'll apply the following as a\nfollow-up patch later on top of what I've already pushed out, which is the\ndifference between the one applied and your v2.\n\n-- >8 --\nbisect: report bad rev better\n\nThe previous one overwrote the variable used to report the bad input\nwhen the input is actually bad, and we did not give a useful enough\ninformation.  This corrects it.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n git-bisect.sh |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex c8be9f7..c99ffee 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -155,9 +155,9 @@ bisect_state() {\n \t\tshift\n \t\tfor rev in \"$@\"\n \t\tdo\n-\t\t\trev=$(git rev-parse --verify \"$rev^{commit}\") ||\n+\t\t\tsha=$(git rev-parse --verify \"$rev^{commit}\") ||\n \t\t\t\tdie \"Bad rev input: $rev\"\n-\t\t\tbisect_write \"$state\" \"$rev\"\n+\t\t\tbisect_write \"$state\" \"$sha\"\n \t\tdone ;;\n \t*,bad)\n \t\tdie \"'git bisect bad' can take only one argument.\" ;;\n"}]}