Re: Making it easier to find which change introduced a bug

Re: Making it easier to find which change introduced a bug

[Linus Torvalds]

On Sat, 30 Jul 2005, Alexander Nyberg wrote:
> 
> Linus, do you think we could have something like
> patch-2.6.13-rc4-incremental-broken-out.tar.bz2 that could like Andrew's
> be placed into patches/ in a tree?

Not really. The thing is, since the git patches really _aren't_ serial, 
and merging isn't based on patch-merging at all (unlike quilt, that 
literally merges patches as patches), you can't really linearize a git 
tree without getting some really strange behaviour.

> As it stands today it's easier for us who don't know git to just find
> out in which mainline kernel it works and which -mm it doesn't work in,
> get the broken-out and start push/pop. And I know I'm not the only one
> who has noticed this.

What we can do is try to script the git bisection thing so that it's
really trivial. It's actually very simple to use, and I think somebody had
some example scripts around.

Here's a simple starting point for somebody who wants to try.. It's not 
very well tested, but I've done _some_ testing on it to try to make sure 
it's at least reasonable. It adds four new git commands:

 - "git bisect-start"
	reset bisect state

 - "git bisect-bad"
	mark some version known-bad (if no arguments, then current HEAD)

 - "git bisect-good"
	mark some version known-good (if no arguments, then current HEAD)

 - "git bisect"
	do a bisection between the known bad and the known good heads, and 
	check that version out.

Then, the way you use it is:

	git bisect-start
	git bisect-bad			# Current version is bad
	git bisect-good v2.6.13-rc2	# v2.6.13-rc2 was the last version tested that was good
	git bisect

which will say something like

	Bisecting: 675 revisions left to test after this

and check out the state in the middle. Now, compile that kernel, and boot 
it. Now, let's say that this booted kernel works fine, then just do

	git bisect-good			# this one is good
	git bisect

which will now say

	Bisecting: 337 revisions left to test after this

and you continue along, compiling that one, testing it, and depending on 
whether it is good or bad, you say "git-bisect-good" or "git-bisect-bad", 
and ask for the next bisection.

Until you have no more left, and you'll have been left with the first bad
kernel rev in "refs/bisect/bad".

Oh, and then after you want to reset to the original head, do a

	git checkout master

to get back to the master branch, instead of being in one of the bisection 
branches ("git bisect-start" will do that for you too, actually: it will 
reset the bisection state, and before it does that it checks that you're 
not using some old bisection branch).

Not really any harder than doing series of "quilt push" and "quilt pop", 
now is it?

		Linus

---
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -62,7 +62,9 @@ SCRIPTS=git git-apply-patch-script git-m
 	git-format-patch-script git-sh-setup-script git-push-script \
 	git-branch-script git-parse-remote git-verify-tag-script \
 	git-ls-remote-script git-clone-dumb-http git-rename-script \
-	git-request-pull-script
+	git-request-pull-script git-bisect-bad-script git-bisect-good-script \
+	git-bisect-script git-bisect-start-script
+
 
 PROG=   git-update-cache git-diff-files git-init-db git-write-tree \
 	git-read-tree git-commit-tree git-cat-file git-fsck-cache \
diff --git a/git-bisect-bad-script b/git-bisect-bad-script
new file mode 100755
--- /dev/null
+++ b/git-bisect-bad-script
@@ -0,0 +1,4 @@
+#!/bin/sh
+. git-sh-setup-script || dir "Not a git archive"
+rev=$(git-rev-parse --revs-only --verify --default HEAD "$@") || exit
+echo "$rev" > "$GIT_DIR/refs/bisect/bad"
diff --git a/git-bisect-good-script b/git-bisect-good-script
new file mode 100755
--- /dev/null
+++ b/git-bisect-good-script
@@ -0,0 +1,4 @@
+#!/bin/sh
+. git-sh-setup-script || dir "Not a git archive"
+rev=$(git-rev-parse --revs-only --verify --default HEAD "$@") || exit
+echo "$rev" > "$GIT_DIR/refs/bisect/good-$rev"
diff --git a/git-bisect-script b/git-bisect-script
new file mode 100755
--- /dev/null
+++ b/git-bisect-script
@@ -0,0 +1,15 @@
+#!/bin/sh
+. git-sh-setup-script || dir "Not a git archive"
+bad=$(git-rev-parse --revs-only --verify refs/bisect/bad) || exit
+good=($(git-rev-parse --revs-only --not $(cd "$GIT_DIR" ; ls refs/bisect/good-*))) || exit
+rev=$(git-rev-list --bisect $bad ${good[@]}) || exit
+nr=$(git-rev-list $rev ${good[@]} | wc -l) || exit
+if [ "$nr" = "0" ]; then
+	echo "$bad is first bad commit"
+	git-diff-tree --pretty $bad
+	exit 0
+fi
+echo "Bisecting: $nr revisions left to test after this"
+echo "$rev" > "$GIT_DIR/refs/heads/new-bisect"
+git checkout new-bisect || exit
+cd "$GIT_DIR" && mv refs/heads/new-bisect refs/heads/bisect && ln -sf refs/heads/bisect HEAD
diff --git a/git-bisect-start-script b/git-bisect-start-script
new file mode 100755
--- /dev/null
+++ b/git-bisect-start-script
@@ -0,0 +1,26 @@
+#!/bin/sh
+. git-sh-setup-script || die "Not a git archive"
+
+#
+# Verify HEAD. If we were bisecting before this, reset to the
+# top-of-line master first!
+#
+head=$(readlink $GIT_DIR/HEAD) || die "Bad HEAD - I need a symlink"
+case "$head" in
+refs/heads/bisect*)
+	git checkout master || exit
+	;;
+refs/heads/*)
+	;;
+*)
+	die "Bad HEAD - strange symlink"
+	;;
+esac
+
+#
+# Get rid of any old bisect state
+#
+cd "$GIT_DIR"
+rm -f "refs/heads/bisect"
+rm -rf "refs/bisect/"
+mkdir "refs/bisect"

[PATCH] Making it easier to find which change introduced a bug

[Junio C Hamano]

I have placed this, after slight reworking, in the master
branch.

[jc: This patch is a rework based on what Linus posted to the
     list.  The changes are:

  - The original introduced four separate commands, which was
    three too many, so I merged them into one with subcommands.

  - Since the next thing you would want to do after telling it
    "bad" and "good" is always to bisect, this version does it
    automatically for you.

  - I think the termination condition was wrong.  The original
    version checked if the set of revisions reachable from next
    bisection but not rechable from any of the known good ones
    is empty, but if the current bisection was a bad one, this
    would not terminate, so I changed it to terminate it when
    the set becomes a singleton or empty.

  - Removed the use of shell array variable.
]

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 Makefile          |    2 -
 git-bisect-script |  158 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 159 insertions(+), 1 deletions(-)
 create mode 100755 git-bisect-script

8cc6a083198877fc32224b73c61ec6e6cf8a96f5
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -62,7 +62,7 @@ SCRIPTS=git git-apply-patch-script git-m
 	git-format-patch-script git-sh-setup-script git-push-script \
 	git-branch-script git-parse-remote git-verify-tag-script \
 	git-ls-remote-script git-clone-dumb-http git-rename-script \
-	git-request-pull-script
+	git-request-pull-script git-bisect-script
 
 PROG=   git-update-cache git-diff-files git-init-db git-write-tree \
 	git-read-tree git-commit-tree git-cat-file git-fsck-cache \
diff --git a/git-bisect-script b/git-bisect-script
new file mode 100755
--- /dev/null
+++ b/git-bisect-script
@@ -0,0 +1,158 @@
+#!/bin/sh
+. git-sh-setup-script || dir "Not a git archive"
+
+usage() {
+    echo >&2 'usage: git bisect [start | bad | good | next | reset]
+git bisect start		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.'
+    exit 1
+}
+
+bisect_autostart() {
+	test -d "$GIT_DIR/refs/bisect" || {
+		echo >&2 'You need to start by "git bisect start"'
+		if test -t 0
+		then
+			echo >&2 -n 'Do you want me to do it for you [Y/n]? '
+			read yesno
+			case "$yesno" in
+			[Nn]*)
+				exit ;;
+			esac
+			bisect_start
+		else
+			exit 1
+		fi
+	}
+}
+
+bisect_start() {
+        case "$#" in 0) ;; *) usage ;; esac
+	#
+	# Verify HEAD. If we were bisecting before this, reset to the
+	# top-of-line master first!
+	#
+	head=$(readlink $GIT_DIR/HEAD) || die "Bad HEAD - I need a symlink"
+	case "$head" in
+	refs/heads/bisect*)
+		git checkout master || exit
+		;;
+	refs/heads/*)
+		;;
+	*)
+		die "Bad HEAD - strange symlink"
+		;;
+	esac
+
+	#
+	# Get rid of any old bisect state
+	#
+	rm -f "$GIT_DIR/refs/heads/bisect"
+	rm -rf "$GIT_DIR/refs/bisect/"
+	mkdir "$GIT_DIR/refs/bisect"
+}
+
+bisect_bad() {
+	bisect_autostart
+        case "$#" in 0 | 1) ;; *) usage ;; esac
+	rev=$(git-rev-parse --revs-only --verify --default HEAD "$@") || exit
+	echo "$rev" > "$GIT_DIR/refs/bisect/bad"
+	bisect_auto_next
+}
+
+bisect_good() {
+	bisect_autostart
+        case "$#" in
+	0)    revs=$(git-rev-parse --verify HEAD) || exit ;;
+	*)    revs=$(git-rev-parse --revs-only "$@") || exit ;;
+	esac
+	for rev in $revs
+	do
+	    echo "$rev" >"$GIT_DIR/refs/bisect/good-$rev"
+	done
+	bisect_auto_next
+}
+
+bisect_next_check() {
+	next_ok=no
+        test -f "$GIT_DIR/refs/bisect/bad" &&
+	case "$(cd "$GIT_DIR" && echo refs/bisect/good-*)" in
+	refs/bisect/good-\*) ;;
+	*) next_ok=yes ;;
+	esac
+	case "$next_ok,$1" in
+	no,) false ;;
+	no,fail)
+	    echo >&2 'You need to give me at least one good and one bad revisions.'
+	    exit 1 ;;
+	*)
+	    true ;;
+	esac
+}
+
+bisect_auto_next() {
+	bisect_next_check && bisect_next
+}
+
+bisect_next() {
+        case "$#" in 0) ;; *) usage ;; esac
+	bisect_autostart
+	bisect_next_check fail
+	bad=$(git-rev-parse --verify refs/bisect/bad) &&
+	good=$(git-rev-parse --sq --revs-only --not \
+		$(cd "$GIT_DIR" && ls refs/bisect/good-*)) &&
+	rev=$(eval "git-rev-list --bisect $good $bad") || exit
+	nr=$(eval "git-rev-list $rev $good" | wc -l) || exit
+	if [ "$nr" -le "1" ]; then
+	    echo "$bad is first bad commit"
+	    git-diff-tree --pretty $bad
+	    exit 0
+	fi
+	echo "Bisecting: $nr revisions left to test after this"
+	echo "$rev" > "$GIT_DIR/refs/heads/new-bisect"
+	git checkout new-bisect || exit
+	mv "$GIT_DIR/refs/heads/new-bisect" "$GIT_DIR/refs/heads/bisect" &&
+	ln -sf refs/heads/bisect "$GIT_DIR/HEAD"
+}
+
+bisect_reset() {
+	case "$#" in
+	0) branch=master ;;
+	1) test -f "$GIT_DIR/refs/heads/$1" || {
+	       echo >&2 "$1 does not seem to be a valid branch"
+	       exit 1
+	   }
+	   branch="$1" ;;
+        *)
+	    usage ;;
+	esac
+	git checkout "$branch" &&
+	rm -fr "$GIT_DIR/refs/bisect"
+	rm -f "$GIT_DIR/refs/reads/bisect"
+}
+
+case "$#" in
+0)
+    usage ;;
+*)
+    cmd="$1"
+    shift
+    case "$cmd" in
+    start)
+        bisect_start "$@" ;;
+    bad)
+        bisect_bad "$@" ;;
+    good)
+        bisect_good "$@" ;;
+    next)
+        # Not sure we want "next" at the UI level anymore.
+        bisect_next "$@" ;;
+    reset)
+        bisect_reset "$@" ;;
+    *)
+        usage ;;
+    esac
+esac