bisect: fix bad rev checking in "git bisect good"

It seems that "git bisect good" and "git bisect skip" have never
properly checked arguments that have been passed to them. As soon
as one of them can be parsed as a SHA1, no error or warning would
be given.

This is because 'git rev-parse --revs-only --no-flags "$@"' always
"exit 0" and outputs all the SHA1 it can found from parsing "$@".

This patch fix this by using, for each "bisect good" argument, the
same logic as for the "bisect bad" argument.

While at it, this patch teaches "bisect bad" to give a meaningfull
error message when it is passed more than one argument.

Note that if "git bisect good" or "git bisect skip" is given some
proper revs and then something that is not a proper rev, then the
first proper revs will still have been marked as "good" or "skip".

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/git-bisect.sh b/git-bisect.sh
index f8857747c4cc5cb3b958e925192e0040157ea911..c8be9f7e8a33db81d9dbbfcd943eaa341aee7595 100755 (executable)
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -151,20 +151,16 @@ bisect_state() {
                rev=$(git rev-parse --verify HEAD) ||
                        die "Bad rev input: HEAD"
                bisect_write "$state" "$rev" ;;
-       2,bad)
-               rev=$(git rev-parse --verify "$2^{commit}") ||
-                       die "Bad rev input: $2"
-               bisect_write "$state" "$rev" ;;
-       *,good|*,skip)
+       2,bad|*,good|*,skip)
                shift
-               revs=$(git rev-parse --revs-only --no-flags "$@") &&
-                       test '' != "$revs" || die "Bad rev input: $@"
-               for rev in $revs
+               for rev in "$@"
                do
                        rev=$(git rev-parse --verify "$rev^{commit}") ||
-                               die "Bad rev commit: $rev^{commit}"
+                               die "Bad rev input: $rev"
                        bisect_write "$state" "$rev"
                done ;;
+       *,bad)
+               die "'git bisect bad' can take only one argument." ;;
        *)
                usage ;;
        esac

diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 4908e878fee48069fbdc339f17ad6e865111ae00..dc87f13558f0225f85c94337b771378d20351604 100755 (executable)
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -71,6 +71,19 @@ test_expect_success 'bisect start with one bad and good' '
        git bisect next
 '
 
+test_expect_success 'bisect good and bad fails if not given only revs' '
+       git bisect reset &&
+       git bisect start &&
+       test_must_fail git bisect good foo $HASH1 &&
+       test_must_fail git bisect good $HASH1 bar &&
+       test_must_fail git bisect bad frotz &&
+       test_must_fail git bisect bad $HASH3 $HASH4 &&
+       test_must_fail git bisect skip bar $HASH3 &&
+       test_must_fail git bisect skip $HASH1 foo &&
+       git bisect good $HASH1 &&
+       git bisect bad $HASH4
+'
+
 test_expect_success 'bisect reset: back in the master branch' '
        git bisect reset &&
        echo "* master" > branch.expect &&