git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Couder <chriscool@tuxfamily.org>
To: Junio Hamano <junkio@cox.net>, Ingo Molnar <mingo@elte.hu>
Cc: git@vger.kernel.org
Subject: [PATCH v2] bisect: fix bad rev checking in "git bisect good"
Date: Sat, 12 Apr 2008 09:03:35 +0200	[thread overview]
Message-ID: <20080412090335.e92d3da3.chriscool@tuxfamily.org> (raw)

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.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 git-bisect.sh               |   18 +++++++-----------
 t/t6030-bisect-porcelain.sh |   13 +++++++++++++
 2 files changed, 20 insertions(+), 11 deletions(-)

	This new version should give a better error message
	than the previous one.

diff --git a/git-bisect.sh b/git-bisect.sh
index a1343f6..408775a 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -155,20 +155,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}"
-			bisect_write "$state" "$rev"
+			sha=$(git rev-parse --verify "$rev^{commit}") ||
+				die "Bad rev input: $rev"
+			bisect_write "$state" "$sha"
 		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 f471c15..32d6118 100755
--- 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 &&
-- 
1.5.5.46.gb6f72.dirty

             reply	other threads:[~2008-04-12  6:58 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-12  7:03 Christian Couder [this message]
2008-04-12  9:17 ` Re* [PATCH v2] bisect: fix bad rev checking in "git bisect good" Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080412090335.e92d3da3.chriscool@tuxfamily.org \
    --to=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    --cc=mingo@elte.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).