Re: [PATCH 2/2] Bisect: implement "bisect dunno" to mark untestable revisions.

Le lundi 8 octobre 2007, Johannes Schindelin a écrit :

Hi,

On Mon, 8 Oct 2007, Christian Couder wrote:

quoted

diff --git a/git-bisect.sh b/git-bisect.sh
index 388887a..c556318 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh

@@ -143,7 +145,7 @@ bisect_write_bad() {

 bisect_good() {
 	bisect_autostart
-        case "$#" in
+	case "$#" in

White space breakage.

The patch tries to fix some white space breakages.

quoted

@@ -153,7 +155,6 @@ bisect_good() {
 		rev=$(git rev-parse --verify "$rev^{commit}") || exit
 		bisect_write_good "$rev"
 		echo "git-bisect good $rev" >>"$GIT_DIR/BISECT_LOG"
-

?

It also removes this unneeded blank line.

quoted

@@ -164,6 +165,28 @@ bisect_write_good() {
 	echo "# good: "$(git show-branch $rev) >>"$GIT_DIR/BISECT_LOG"
 }

+bisect_dunno() {
+	bisect_autostart
+	case "$#" in
+	0)    revs=$(git rev-parse --verify HEAD) || exit ;;
+	*)    revs=$(git rev-parse --revs-only --no-flags "$@") &&
+		test '' != "$revs" || die "Bad rev input: $@" ;;
+	esac
+	for rev in $revs
+	do
+		rev=$(git rev-parse --verify "$rev^{commit}") || exit
+		bisect_write_dunno "$rev"
+		echo "git-bisect dunno $rev" >>"$GIT_DIR/BISECT_LOG"

Should the last line not be put into bisect_write_dunno?  OTOH this is
the only call site of that function, so I strongly doubt that the
function (consisting of 3 lines, where the first is 'rev="$1"') is
necessary at all.

Well, there are "bisect_write_bad" and "bisect_write_good" that already do 
the same thing as "bisect_write_dunno". In fact I thought that it was 
better to just copy "bisect_dunno" from "bisect_good" 
and "bisect_write_dunno" from "bisect_write_good".

If needed I can send another patch to factorise these functions.

quoted

@@ -206,17 +229,104 @@ bisect_auto_next() {
 	bisect_next_check && bisect_next || :
 }

+search_dunno() {
+	_hash="$1"
+	_dunno="$2"
+
+	for _val in $_dunno ; do
+		case $_hash in $_val) return 1 ;; esac
+	done

This would be faster as

	case " $1" in " $2") return 1 ;; esac

I guess.

I will try your suggestion and send an updated patch. Thanks.

But as I said in the other reply, I think this logic belongs into the C
core, instead of generating mostly useless information, passing it down
to the script, and filtering it out again.

Yeah, it's not efficient.

Best regards,
Christian.