bisect: move argument parsing before state modification.

Currently 'git bisect start' modifies some state prior to checking
that its arguments are valid.

This change moves argument validation before state modification
with the effect that state modification does not occur
unless argument validations succeeds.

An existing test is changed to check that new bisect state
is not created if arguments are invalid.

A new test is added to check that existing bisect state
is not modified if arguments are invalid.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/git-bisect.sh b/git-bisect.sh
index b2186a86279e8919214bb205400a05f32a316c3b..20f6dd5625160f65cde186bb17138c6ffbc1ff12 100755 (executable)
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -59,6 +59,39 @@ bisect_autostart() {
 }
 
 bisect_start() {
 }
 
 bisect_start() {

+       #
+       # Check for one bad and then some good revisions.
+       #
+       has_double_dash=0
+       for arg; do
+           case "$arg" in --) has_double_dash=1; break ;; esac
+       done
+       orig_args=$(git rev-parse --sq-quote "$@")
+       bad_seen=0
+       eval=''
+       while [ $# -gt 0 ]; do
+           arg="$1"
+           case "$arg" in
+           --)
+               shift
+               break
+               ;;
+           *)
+               rev=$(git rev-parse -q --verify "$arg^{commit}") || {
+                   test $has_double_dash -eq 1 &&
+                       die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
+                   break
+               }
+               case $bad_seen in
+               0) state='bad' ; bad_seen=1 ;;
+               *) state='good' ;;
+               esac
+               eval="$eval bisect_write '$state' '$rev' 'nolog'; "
+               shift
+               ;;
+           esac
+       done
+

        #
        # Verify HEAD.
        #
        #
        # Verify HEAD.
        #


@@ -97,39 +130,6 @@ bisect_start() {
        #
        bisect_clean_state || exit
 
        #
        bisect_clean_state || exit
 

-       #
-       # Check for one bad and then some good revisions.
-       #
-       has_double_dash=0
-       for arg; do
-           case "$arg" in --) has_double_dash=1; break ;; esac
-       done
-       orig_args=$(git rev-parse --sq-quote "$@")
-       bad_seen=0
-       eval=''
-       while [ $# -gt 0 ]; do
-           arg="$1"
-           case "$arg" in
-           --)
-               shift
-               break
-               ;;
-           *)
-               rev=$(git rev-parse -q --verify "$arg^{commit}") || {
-                   test $has_double_dash -eq 1 &&
-                       die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
-                   break
-               }
-               case $bad_seen in
-               0) state='bad' ; bad_seen=1 ;;
-               *) state='good' ;;
-               esac
-               eval="$eval bisect_write '$state' '$rev' 'nolog'; "
-               shift
-               ;;
-           esac
-       done
-

        #
        # Change state.
        # In case of mistaken revs or checkout error, or signals received,
        #
        # Change state.
        # In case of mistaken revs or checkout error, or signals received,

diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index b5063b6fe6c37f4b41a89b71c6d52ac0b5c07127..b3d1b1430bad60ab0dae1896cc646a5c4f2fdfbd 100755 (executable)
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -138,15 +138,23 @@ test_expect_success 'bisect start: back in good branch' '
        grep "* other" branch.output > /dev/null
 '
 
        grep "* other" branch.output > /dev/null
 '
 

-test_expect_success 'bisect start: no ".git/BISECT_START" if junk rev' '
-       git bisect start $HASH4 $HASH1 -- &&
-       git bisect good &&
+test_expect_success 'bisect start: no ".git/BISECT_START" created if junk rev' '
+       git bisect reset &&

        test_must_fail git bisect start $HASH4 foo -- &&
        git branch > branch.output &&
        grep "* other" branch.output > /dev/null &&
        test_must_fail test -e .git/BISECT_START
 '
 
        test_must_fail git bisect start $HASH4 foo -- &&
        git branch > branch.output &&
        grep "* other" branch.output > /dev/null &&
        test_must_fail test -e .git/BISECT_START
 '
 

+test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if junk rev' '
+       git bisect start $HASH4 $HASH1 -- &&
+       git bisect good &&
+       cp .git/BISECT_START saved &&
+       test_must_fail git bisect start $HASH4 foo -- &&
+       git branch > branch.output &&
+       grep "* (no branch)" branch.output > /dev/null &&
+       test_cmp saved .git/BISECT_START
+'

 test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
        git bisect start $HASH4 $HASH1 -- &&
        git bisect good &&
 test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
        git bisect start $HASH4 $HASH1 -- &&
        git bisect good &&