Re: [RFC/PATCH] Bisect: teach "bisect start" to optionally use one bad and many good revs.

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [RFC/PATCH] Bisect: teach "bisect start" to optionally use one bad and many good revs.

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:02

Christian Couder [off-list ref] writes:
In fact this patch implements:

    git bisect start [<bad> [<good>...]] [--] [<pathspec>...]

I think this is more backward compatible because older script
probably didn't used -- before <pathspec>...

On the other hand, there may be some confusion between revs
(<bad> and <good>...) and <pathspec>... if -- is not used
and if an invalid rev or a pathspec that looks like a rev is
given.
I think the intent of the check you have for '--' to cover the
above issue is good.
quoted hunk
Signed-off-by: Christian Couder <redacted>
---
 git-bisect.sh         |  105 ++++++++++++++++++++++++++++++++++++++-----------
 t/t6030-bisect-run.sh |   20 ++++++++-
 2 files changed, 99 insertions(+), 26 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index 11313a7..d5a13ee 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,15 +1,24 @@
 #!/bin/sh
 
 USAGE='[start|bad|good|next|reset|visualize|replay|log|run]'
-LONG_USAGE='git bisect start [<pathspec>]	reset bisect state and start bisection.
-git bisect bad [<rev>]		mark <rev> a known-bad revision.
-git bisect good [<rev>...]	mark <rev>... known-good revisions.
-git bisect next			find next bisection to test and check it out.
-git bisect reset [<branch>]	finish bisection search and go back to branch.
-git bisect visualize            show bisect status in gitk.
-git bisect replay <logfile>	replay bisection log.
-git bisect log			show bisect log.
-git bisect run <cmd>... 	use <cmd>... to automatically bisect.'
+LONG_USAGE='git bisect start [<bad> [<good>...]] [--] [<pathspec>...]
+        reset bisect state and start bisection.
+git bisect bad [<rev>]
+        mark <rev> a known-bad revision.
+git bisect good [<rev>...]
+        mark <rev>... known-good revisions.
+git bisect next
+        find next bisection to test and check it out.
+git bisect reset [<branch>]
+        finish bisection search and go back to branch.
+git bisect visualize
+        show bisect status in gitk.
+git bisect replay <logfile>
+        replay bisection log.
+git bisect log
+        show bisect log.
+git bisect run <cmd>...
+        use <cmd>... to automatically bisect.'
Much easier to read ;-).
 	mkdir "$GIT_DIR/refs/bisect"
+
+	#
+	# Check for one bad and then some good revisions.
+	#
+	has_double_dash=0
+	for arg in "$@"; do
+	    case "$arg" in --) has_double_dash=1; break ;; esac
+	done
Style.  'in "$@"' is superfluous.

	for arg
        do
        	...
	done
+	orig_args="$@"
Doesn't this defeat the whole point of later running 'sq' on it?
The reason we do sq is to protect whitespaces in pathspecs and
make the strings correctly split when evaled/sourced.

A simple test program:

        #!/bin/sh
        sq () {
                echo "** fake sq $* **"
                i=0
                for j
                do
                        echo "$i: <$j>"
                        i=$(( $i + 1 ))
                done
                echo
        }

        set x a 'b c' d
        shift
        # Now, $# = 3, $1 = 'a', $2 = 'b c' and $3 = 'd'
        orig_args="$@"
        sq "$@"
        sq "$orig_args"
...
-	    sq "$@"
-	} >"$GIT_DIR/BISECT_LOG"
-	sq "$@" >"$GIT_DIR/BISECT_NAMES"
+	    sq "$orig_args"
+	} >>"$GIT_DIR/BISECT_LOG"
+	bisect_auto_next
 }

[PATCH] Bisect: teach "bisect start" to optionally use one bad and many good revs.

From: Christian Couder <hidden>
Date: 2016-06-15 22:43:02

As Junio said:

"One bad commit is fundamentally needed for bisect to run,
and if we beforehand know more good commits, we can narrow
the bisect space down without doing the whole tree checkout
every time we give good commits.  I think it may be a good
idea to have:

    git bisect start [$bad [$good1 $good2...]] [-- <paths>...]

as a short-hand for this command sequence:

    git bisect start
    git bisect bad $bad
    git bisect good $good1 $good2...

That would be a good script-shorterner, without limiting it to
any specific use scenario."

In fact this patch implements:

    git bisect start [<bad> [<good>...]] [--] [<pathspec>...]

I think this is more backward compatible because older script
probably didn't used -- before <pathspec>...

On the other hand, there may be some confusion between revs
(<bad> and <good>...) and <pathspec>... if -- is not used
and if an invalid rev or a pathspec that looks like a rev is
given.

Signed-off-by: Christian Couder <redacted>
---

Junio wrote:
quoted
+	for arg in "$@"; do
+	    case "$arg" in --) has_double_dash=1; break ;; esac
+	done
Style.  'in "$@"' is superfluous.
Fixed in this patch.
quoted
+	orig_args="$@"
Doesn't this defeat the whole point of later running 'sq' on it?
The reason we do sq is to protect whitespaces in pathspecs and
make the strings correctly split when evaled/sourced.
This should also be fixed.

Thanks,
Christian.


 git-bisect.sh         |  105 ++++++++++++++++++++++++++++++++++++++-----------
 t/t6030-bisect-run.sh |   20 ++++++++-
 2 files changed, 99 insertions(+), 26 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index 11313a7..2e68e3d 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,15 +1,24 @@
 #!/bin/sh
 
 USAGE='[start|bad|good|next|reset|visualize|replay|log|run]'
-LONG_USAGE='git bisect start [<pathspec>]	reset bisect state and start bisection.
-git bisect bad [<rev>]		mark <rev> a known-bad revision.
-git bisect good [<rev>...]	mark <rev>... known-good revisions.
-git bisect next			find next bisection to test and check it out.
-git bisect reset [<branch>]	finish bisection search and go back to branch.
-git bisect visualize            show bisect status in gitk.
-git bisect replay <logfile>	replay bisection log.
-git bisect log			show bisect log.
-git bisect run <cmd>... 	use <cmd>... to automatically bisect.'
+LONG_USAGE='git bisect start [<bad> [<good>...]] [--] [<pathspec>...]
+        reset bisect state and start bisection.
+git bisect bad [<rev>]
+        mark <rev> a known-bad revision.
+git bisect good [<rev>...]
+        mark <rev>... known-good revisions.
+git bisect next
+        find next bisection to test and check it out.
+git bisect reset [<branch>]
+        finish bisection search and go back to branch.
+git bisect visualize
+        show bisect status in gitk.
+git bisect replay <logfile>
+        replay bisection log.
+git bisect log
+        show bisect log.
+git bisect run <cmd>...
+        use <cmd>... to automatically bisect.'
 
 . git-sh-setup
 require_work_tree
@@ -70,14 +79,48 @@ bisect_start() {
 	#
 	# Get rid of any old bisect state
 	#
-	rm -f "$GIT_DIR/refs/heads/bisect"
-	rm -rf "$GIT_DIR/refs/bisect/"
+	bisect_clean_state
 	mkdir "$GIT_DIR/refs/bisect"
+
+	#
+	# 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=$(sq "$@")
+	bad_seen=0
+	while [ $# -gt 0 ]; do
+	    arg="$1"
+	    case "$arg" in
+	    --)
+	        shift
+		break
+		;;
+	    *)
+	        rev=$(git-rev-parse --verify "$arg^{commit}" 2>/dev/null) || {
+		    test $has_double_dash -eq 1 &&
+		        die "'$arg' does not appear to be a valid revision"
+		    break
+		}
+		if [ $bad_seen -eq 0 ]; then
+		    bad_seen=1
+		    bisect_write_bad "$rev"
+		else
+		    bisect_write_good "$rev"
+		fi
+	        shift
+		;;
+	    esac
+        done
+
+	sq "$@" >"$GIT_DIR/BISECT_NAMES"
 	{
 	    printf "git-bisect start"
-	    sq "$@"
-	} >"$GIT_DIR/BISECT_LOG"
-	sq "$@" >"$GIT_DIR/BISECT_NAMES"
+	    echo "$orig_args"
+	} >>"$GIT_DIR/BISECT_LOG"
+	bisect_auto_next
 }
 
 bisect_bad() {
@@ -90,12 +133,17 @@ bisect_bad() {
 	*)
 		usage ;;
 	esac || exit
-	echo "$rev" >"$GIT_DIR/refs/bisect/bad"
-	echo "# bad: "$(git-show-branch $rev) >>"$GIT_DIR/BISECT_LOG"
+	bisect_write_bad "$rev"
 	echo "git-bisect bad $rev" >>"$GIT_DIR/BISECT_LOG"
 	bisect_auto_next
 }
 
+bisect_write_bad() {
+	rev="$1"
+	echo "$rev" >"$GIT_DIR/refs/bisect/bad"
+	echo "# bad: "$(git-show-branch $rev) >>"$GIT_DIR/BISECT_LOG"
+}
+
 bisect_good() {
 	bisect_autostart
         case "$#" in
@@ -106,13 +154,19 @@ bisect_good() {
 	for rev in $revs
 	do
 		rev=$(git-rev-parse --verify "$rev^{commit}") || exit
-		echo "$rev" >"$GIT_DIR/refs/bisect/good-$rev"
-		echo "# good: "$(git-show-branch $rev) >>"$GIT_DIR/BISECT_LOG"
+		bisect_write_good "$rev"
 		echo "git-bisect good $rev" >>"$GIT_DIR/BISECT_LOG"
+
 	done
 	bisect_auto_next
 }
 
+bisect_write_good() {
+	rev="$1"
+	echo "$rev" >"$GIT_DIR/refs/bisect/good-$rev"
+	echo "# good: "$(git-show-branch $rev) >>"$GIT_DIR/BISECT_LOG"
+}
+
 bisect_next_check() {
 	next_ok=no
         test -f "$GIT_DIR/refs/bisect/bad" &&
@@ -190,14 +244,19 @@ bisect_reset() {
 	    usage ;;
 	esac
 	if git checkout "$branch"; then
-		rm -fr "$GIT_DIR/refs/bisect"
-		rm -f "$GIT_DIR/refs/heads/bisect" "$GIT_DIR/head-name"
-		rm -f "$GIT_DIR/BISECT_LOG"
-		rm -f "$GIT_DIR/BISECT_NAMES"
-		rm -f "$GIT_DIR/BISECT_RUN"
+		rm -f "$GIT_DIR/head-name"
+		bisect_clean_state
 	fi
 }
 
+bisect_clean_state() {
+	rm -fr "$GIT_DIR/refs/bisect"
+	rm -f "$GIT_DIR/refs/heads/bisect"
+	rm -f "$GIT_DIR/BISECT_LOG"
+	rm -f "$GIT_DIR/BISECT_NAMES"
+	rm -f "$GIT_DIR/BISECT_RUN"
+}
+
 bisect_replay () {
 	test -r "$1" || {
 		echo >&2 "cannot read $1 for replaying"
diff --git a/t/t6030-bisect-run.sh b/t/t6030-bisect-run.sh
index 39c7228..455dc60 100755
--- a/t/t6030-bisect-run.sh
+++ b/t/t6030-bisect-run.sh
@@ -40,8 +40,8 @@ test_expect_success \
 # We want to automatically find the commit that
 # introduced "Another" into hello.
 test_expect_success \
-    'git bisect run simple case' \
-    'echo "#!/bin/sh" > test_script.sh &&
+    '"git bisect run" simple case' \
+    'echo "#"\!"/bin/sh" > test_script.sh &&
      echo "grep Another hello > /dev/null" >> test_script.sh &&
      echo "test \$? -ne 0" >> test_script.sh &&
      chmod +x test_script.sh &&
@@ -49,7 +49,21 @@ test_expect_success \
      git bisect good $HASH1 &&
      git bisect bad $HASH4 &&
      git bisect run ./test_script.sh > my_bisect_log.txt &&
-     grep "$HASH3 is first bad commit" my_bisect_log.txt'
+     grep "$HASH3 is first bad commit" my_bisect_log.txt &&
+     git bisect reset'
+
+# We want to automatically find the commit that
+# introduced "Ciao" into hello.
+test_expect_success \
+    '"git bisect run" with more complex "git bisect start"' \
+    'echo "#"\!"/bin/sh" > test_script.sh &&
+     echo "grep Ciao hello > /dev/null" >> test_script.sh &&
+     echo "test \$? -ne 0" >> test_script.sh &&
+     chmod +x test_script.sh &&
+     git bisect start $HASH4 $HASH1 &&
+     git bisect run ./test_script.sh > my_bisect_log.txt &&
+     grep "$HASH4 is first bad commit" my_bisect_log.txt &&
+     git bisect reset'
 
 #
 #
-- 
1.5.1.rc3.21.g02918

[PATCH] Documentation: bisect: "start" accepts one bad and many good commits

From: Christian Couder <hidden>
Date: 2016-06-15 22:43:03

Signed-off-by: Christian Couder <redacted>
---
 Documentation/git-bisect.txt |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index b2bc58d..5f68ee1 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -15,7 +15,7 @@ DESCRIPTION
 The command takes various subcommands, and different options depending
 on the subcommand:
 
- git bisect start [<paths>...]
+ git bisect start [<bad> [<good>...]] [--] [<paths>...]
  git bisect bad <rev>
  git bisect good <rev>
  git bisect reset [<branch>]
@@ -134,15 +134,26 @@ $ git reset --hard HEAD~3		# try 3 revs before what
 Then compile and test the one you chose to try. After that, tell
 bisect what the result was as usual.
 
-Cutting down bisection by giving path parameter to bisect start
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Cutting down bisection by giving more parameters to bisect start
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 You can further cut down the number of trials if you know what part of
 the tree is involved in the problem you are tracking down, by giving
 paths parameters when you say `bisect start`, like this:
 
 ------------
-$ git bisect start arch/i386 include/asm-i386
+$ git bisect start -- arch/i386 include/asm-i386
+------------
+
+If you know beforehand more than one good commits, you can narrow the
+bisect space down without doing the whole tree checkout every time you
+give good commits. You give the bad revision immediately after `start`
+and then you give all the good revisions you have:
+
+------------
+$ git bisect start v2.6.20-rc6 v2.6.20-rc4 v2.6.20-rc1 --
+                   # v2.6.20-rc6 is bad
+                   # v2.6.20-rc4 and v2.6.20-rc1 are good
 ------------
 
 Bisect run
-- 
1.5.1.rc3.21.g02918
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help