bisect: add a --verify flag to `bisect run`

If it's passed, then git bisect run --verify script will first check out the good revision and verify that the script passes there,

then check out the bad revision and verify that it fails there, and only then start the actual git bisect run script.

We use git bisect run heavily for bisecting bugs in LLVM when using clang to build Chromium. We sometimes end up with run scripts that are broken in some way, either by missing the +x bit, or in more subtle ways, and this adds a simple, low conceptual overhead way to smoke check the run script before starting a bisect that could run for a day or two.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-741%2Fnico%2Frunverifyforupstreaming-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-741/nico/runverifyforupstreaming-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/741

 Documentation/git-bisect.txt |  12 +++-
 git-bisect.sh                | 110 ++++++++++++++++++++++++++++++++++-
 t/t6030-bisect-porcelain.sh  |  16 +++++
 3 files changed, 134 insertions(+), 4 deletions(-)

Documentation/git-bisect.txt +9 -3

index fbb39fbdf5..37658d8ed8 100644
@@ -26,7 +26,7 @@ on the subcommand:
  git bisect (visualize|view)
  git bisect replay <logfile>
  git bisect log
- git bisect run <cmd>...
+ git bisect run [--verify] <cmd>...
  git bisect help
 
 This command uses a binary search algorithm to find which commit in

@@ -93,7 +93,6 @@ Eventually there will be no more revisions left to inspect, and the
 command will print out a description of the first bad commit. The
 reference `refs/bisect/bad` will be left pointing at that commit.
 
-
 Bisect reset
 ~~~~~~~~~~~~
 

@@ -317,7 +316,7 @@ If you have a script that can tell if the current source code is good
 or bad, you can bisect by issuing the command:
 
 ------------
-$ git bisect run my_script arguments
+$ git bisect run [--verify] my_script arguments
 ------------
 
 Note that the script (`my_script` in the above example) should exit

@@ -376,6 +375,13 @@ ignored.
 This option is particularly useful in avoiding false positives when a merged
 branch contained broken or non-buildable commits, but the merge itself was OK.
 
+--verify::
++
+Before the actual bisect run, check out the current bad revision and
+verify that the script exits with a code between 1 and 127 (inclusive),
+except 125, then check out the current good revision and verify that
+the script exits with code 0. If not, abort the bisect run.
+
 EXAMPLES
 --------
 

git-bisect.sh +109 -1

index 2f60fefcfa..99ac01fb55 100755
@@ -26,7 +26,7 @@ git bisect replay <logfile>
 	replay bisection log.
 git bisect log
 	show bisect log.
-git bisect run <cmd>...
+git bisect run [--verify | --no-verify] <cmd>...
 	use <cmd>... to automatically bisect.
 
 Please use "git help bisect" to get the full man page.'

@@ -38,6 +38,8 @@ _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
 _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
 TERM_BAD=bad
 TERM_GOOD=good
+CURRENT_BISECT_BAD=
+CURRENT_BISECT_GOOD=
 
 bisect_head()
 {

@@ -215,6 +217,7 @@ bisect_replay () {
 		test "$git $bisect" = "git bisect" || test "$git" = "git-bisect" || continue
 		if test "$git" = "git-bisect"
 		then
+			tail="$rev"
 			rev="$command"
 			command="$bisect"
 		fi

@@ -237,7 +240,112 @@ bisect_replay () {
 	bisect_auto_next
 }
 
+get_current_bisect_bounds () {
+	test -s "$GIT_DIR/BISECT_LOG" || die "$(gettext "We are not bisecting.")"
+	oIFS="$IFS" IFS="$IFS$(printf '\015')"
+	while read git bisect command rev tail
+	do
+		test "$git $bisect" = "git bisect" || test "$git" = "git-bisect" || continue
+		if test "$git" = "git-bisect"
+		then
+			tail="$rev"
+			rev="$command"
+			command="$bisect"
+		fi
+		get_terms
+		git bisect--helper --check-and-set-terms "$command" "$TERM_GOOD" "$TERM_BAD" || exit
+		get_terms
+		case "$command" in
+		skip)
+			;;
+		start)
+			CURRENT_BISECT_BAD="$rev"
+			CURRENT_BISECT_GOOD="$tail"
+			;;
+		"$TERM_GOOD")
+			CURRENT_BISECT_GOOD="$rev" ;;
+		"$TERM_BAD")
+			CURRENT_BISECT_BAD="$rev" ;;
+		*)
+			die "$(gettext "?? what are you talking about?")" ;;
+		esac
+	done <"$GIT_DIR/BISECT_LOG"
+	IFS="$oIFS"
+}
+
 bisect_run () {
+	verify=
+	while test $# -ne 0
+	do
+		case "$1" in
+		--verify)
+			verify=t
+			;;
+		--no-verify)
+			verify=
+			;;
+	--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	if [ -n "$verify" ]; then
+		git rev-parse --verify -q BISECT_HEAD > /dev/null && die "$(gettext "bisect run --verify is incompatible with --no-checkout")"
+
+		get_current_bisect_bounds
+		test -n "$CURRENT_BISECT_BAD" || die "$(gettext "bisect run --verify: no current bad revision")"
+		test -n "$CURRENT_BISECT_GOOD" || die "$(gettext "bisect run --verify: no current good revision")"
+
+		bisected_head=$(bisect_head)
+		rev=$(git rev-parse --verify "$bisected_head") ||
+			die "$(eval_gettext "Bad rev input: \$bisected_head")"
+
+		trap "git checkout -q $rev" 0
+
+		# Check script passes for good rev.
+		command="$@"
+		eval_gettextln "verifying script passes at \$TERM_GOOD rev"
+		eval git checkout -q "$CURRENT_BISECT_GOOD" || die "$(eval_gettext "failed to check out \$TERM_GOOD rev")"
+		"$@"
+		res=$?
+		if [ $res -ne 0 ]
+		then
+			die_with_status $res "$(eval_gettext "aborting: run script fails for \$TERM_GOOD rev")"
+		fi
+
+		# Check script fails orderly for bad rev.
+		command="$@"
+		eval_gettextln "verifying script fails at \$TERM_BAD rev"
+		eval git checkout -q "$CURRENT_BISECT_BAD" || die "$(eval_gettext "failed to check out \$TERM_BAD rev")"
+		"$@"
+		res=$?
+		if [ $res -lt 0 -o $res -ge 128 ]
+		then
+			die "$(eval_gettext "aborting: exit code \$res is < 0 or >= 128")"
+		fi
+		if [ $res -eq 0 ]
+		then
+			die "$(eval_gettext "aborting: run script passes for \$TERM_BAD rev")"
+		fi
+		if [ $res -eq 125 ]
+		then
+			die "$(eval_gettext "aborting: run sript returns 125 (skip) for \$TERM_BAD rev")"
+		fi
+
+		# Check out pre-verify rev again.
+		git checkout -q "$rev"
+		trap '-' 0
+	fi
+
 	git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
 
 	test -n "$*" || die "$(gettext "bisect run failed: no command provided.")"

t/t6030-bisect-porcelain.sh +16 -0

index b886529e59..acb01dcfff 100755
@@ -321,6 +321,22 @@ test_expect_success 'bisect run & skip: find first bad' '
 	grep "$HASH6 is the first bad commit" my_bisect_log.txt
 '
 
+test_expect_success 'bisect run --verify: script fails for good rev' '
+	git bisect reset &&
+	git bisect start $HASH7 $HASH1 &&
+	test_must_fail git bisect run --verify false >my_bisect_log3.txt 2>&1 &&
+	test_i18ngrep "aborting: run script fails for good rev" my_bisect_log3.txt
+'
+
+test_expect_success 'bisect run --verify: script passes for bad rev' '
+	git bisect reset &&
+	git bisect start &&
+	git bisect bad $HASH7 &&
+	git bisect good $HASH1 &&
+	test_must_fail git bisect run --verify true >my_bisect_log5.txt 2>&1 &&
+	test_i18ngrep "aborting: run script passes for bad rev" my_bisect_log5.txt
+'
+
 test_expect_success 'bisect skip only one range' '
 	git bisect reset &&
 	git bisect start $HASH7 $HASH1 &&

base-commit: 9bc233ae1cf19a49e51842c7959d80a675dbd1c0

From: Nico Weber <[email protected]>

If it's passed, then git bisect run --verify script will first check out the good revision and verify that the script passes there, then check out the bad revision and verify that it fails there, and only then start the actual git bisect run script.

We use git bisect run heavily for bisecting bugs in LLVM when using clang to build Chromium. We sometimes end up with run scripts that are broken in some way, either by missing the +x bit, or in more subtle ways, and this adds a simple, low conceptual overhead way to smoke check the run script before starting a bisect that could run for a day or two.

Documentation/git-bisect.txt +1 -1

@@ -317,7 +316,7 @@ If you have a script that can tell if the current source code is good
 or bad, you can bisect by issuing the command:
 
 ------------
-$ git bisect run my_script arguments
+$ git bisect run [--verify] my_script arguments
 ------------
 
 Note that the script (`my_script` in the above example) should exit

git-bisect.sh +4 -0

+			shift
+			break
+			;;
 
 
+		-*)

For this change, it might look like this:

The script given to the git bisect run script may broken in some way. [Explain why it is bad that 'script' is not run on the known bad or good revisions here in the current code. The script is run to test on the midway commit anyway and the lack of +x bit would be noticed just as quickly as you retest a known good or bad revision, so this 'feature' appears as a pure overhead to cause extra checkouts and builds for no great additional benefit, at least to me, and I tried but cannot make a good case for you here].