git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v8 1/7] bisect: move argument parsing before state modification.

From
Jon Seymour <jon.seymour@gmail.com>
Date
Jul 31, 2011, 11:55 UTC
Message-ID
<1312113321-28760-2-git-send-email-jon.seymour@gmail.com>
In-Reply-To
<1312113321-28760-1-git-send-email-jon.seymour@gmail.com>

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>
---
 git-bisect.sh               |   66 +++++++++++++++++++++---------------------
 t/t6030-bisect-porcelain.sh |   14 +++++++--
 2 files changed, 44 insertions(+), 36 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index b2186a8..20f6dd5 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -60,6 +60,39 @@ bisect_autostart() {
 
 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.
 	#
 	head=$(GIT_DIR="$GIT_DIR" git symbolic-ref -q HEAD) ||
@@ -98,39 +131,6 @@ bisect_start() {
 	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,
 	# "bisect_auto_next" below may exit or misbehave.
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index b5063b6..b3d1b14 100755
--- 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
 '
 
-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_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 &&
-- 
1.7.6.391.g168d0.dirty
Previous: Jon SeymourNext: Jon Seymour
Message 2 of 17 in “bisect: Add support for --no-checkout and --update-ref=<ref> options.”
  1. 0/7 bisect: Add support for --no-checkout and --update-ref=<ref> options.Jon Seymour, Jul 31, 2011
  2. 1/7 bisect: move argument parsing before state modification.Jon Seymour, Jul 31, 2011
  3. 2/7 bisect: add tests to document expected behaviour in presence of broken trees.Jon Seymour, Jul 31, 2011
  4. 3/7 bisect: introduce support for --update-ref=<ref> option.Jon Seymour, Jul 31, 2011
  5. 4/7 bisect: introduce --no-checkout, --update-ref=<ref> support into porcelain.Jon Seymour, Jul 31, 2011
  6. Christian CouderJul 31, 2011
  7. Jon SeymourJul 31, 2011
  8. Christian CouderJul 31, 2011
  9. Christian CouderJul 31, 2011
  10. Jon SeymourJul 31, 2011
  11. Christian CouderJul 31, 2011
  12. Christian CouderJul 31, 2011
  13. Jon SeymourJul 31, 2011
  14. 5/7 bisect: add tests for the --no-checkout and --update-ref options.Jon Seymour, Jul 31, 2011
  15. 6/7 bisect: add documentation for --no-checkout and --update-ref=<ref> options.Jon Seymour, Jul 31, 2011
  16. 7/7 bisect: support --update-ref <ref>Jon Seymour, Jul 31, 2011
  17. Jon SeymourJul 31, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.