bisect: fix quoting TRIED revs when "bad" commit is also "skip"ped

When the "bad" commit was also "skip"ped and when more than one
commit was skipped, the "filter_skipped" function would have
printed something like:

    bisect_rev=<hash1>|<hash2>

(where <hash1> and <hash2> are hexadecimal sha1 hashes)

and this would have been evaled later as piping "bisect_rev=<hash1>"
into "<hash2>", which would have failed.

So this patch makes the "filter_skipped" function properly quote
what it outputs, so that it will print something like:

bisect_rev='<hash1>|<hash2>'

which will be properly evaled later.  The caller was not stopping
properly because the scriptlet this function returned to be evaled
was not strung together with && and because of this, an error in
an earlier part of the output was simply ignored.

A test case is added to the test suite.

And while at it, we also initialize the VARS, FOUND and TRIED
variables, so that we protect ourselves from environment variables
the user may have with these names.

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

1 files changed, 41 insertions, 35 deletions

diff --git a/git-bisect.sh b/git-bisect.sh
index 97ac60087..f9a5c0bdf 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -269,56 +269,62 @@ filter_skipped() {
 
 	# Let's parse the output of:
 	# "git rev-list --bisect-vars --bisect-all ..."
-	eval_rev_list "$_eval" | while read hash line
-	do
-		case "$VARS,$FOUND,$TRIED,$hash" in
-			# We display some vars.
-			1,*,*,*) echo "$hash $line" ;;
-
-			# Split line.
-			,*,*,---*) ;;
-
-			# We had nothing to search.
+	eval_rev_list "$_eval" | {
+		VARS= FOUND= TRIED=
+		while read hash line
+		do
+			case "$VARS,$FOUND,$TRIED,$hash" in
+			1,*,*,*)
+				# "bisect_foo=bar" read from rev-list output.
+				echo "$hash &&"
+				;;
+			,*,*,---*)
+				# Separator
+				;;
 			,,,bisect_rev*)
-				echo "bisect_rev="
+				# We had nothing to search.
+				echo "bisect_rev= &&"
 				VARS=1
 				;;
-
-			# We did not find a good bisect rev.
-			# This should happen only if the "bad"
-			# commit is also a "skip" commit.
 			,,*,bisect_rev*)
-				echo "bisect_rev=$TRIED"
+				# We did not find a good bisect rev.
+				# This should happen only if the "bad"
+				# commit is also a "skip" commit.
+				echo "bisect_rev='$TRIED' &&"
 				VARS=1
 				;;
-
-			# We are searching.
 			,,*,*)
+				# We are searching.
 				TRIED="${TRIED:+$TRIED|}$hash"
 				case "$_skip" in
 				*$hash*) ;;
 				*)
-					echo "bisect_rev=$hash"
-					echo "bisect_tried=\"$TRIED\""
+					echo "bisect_rev=$hash &&"
+					echo "bisect_tried='$TRIED' &&"
 					FOUND=1
 					;;
 				esac
 				;;
-
-			# We have already found a rev to be tested.
-			,1,*,bisect_rev*) VARS=1 ;;
-			,1,*,*) ;;
-
-			# ???
-			*) die "filter_skipped error " \
-			    "VARS: '$VARS' " \
-			    "FOUND: '$FOUND' " \
-			    "TRIED: '$TRIED' " \
-			    "hash: '$hash' " \
-			    "line: '$line'"
-			;;
-		esac
-	done
+			,1,*,bisect_rev*)
+				# We have already found a rev to be tested.
+				VARS=1
+				;;
+			,1,*,*)
+				;;
+			*)
+				# Unexpected input
+				echo "die 'filter_skipped error'"
+				die "filter_skipped error " \
+				    "VARS: '$VARS' " \
+				    "FOUND: '$FOUND' " \
+				    "TRIED: '$TRIED' " \
+				    "hash: '$hash' " \
+				    "line: '$line'"
+				;;
+			esac
+		done
+		echo ':'
+	}
 }
 
 exit_if_skipped_commits () {