All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 6/7] Bisect: factorise "bisect_{bad,good,dunno}" into "bisect_state".
@ 2007-10-14 12:30 Christian Couder
  2007-10-14 16:15 ` Johannes Schindelin
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Couder @ 2007-10-14 12:30 UTC (permalink / raw)
  To: Junio Hamano, Johannes Schindelin; +Cc: git

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 git-bisect.sh |   80 +++++++++++++++++++++-----------------------------------
 1 files changed, 30 insertions(+), 50 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index e12125f..6a5ec5b 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -134,47 +134,33 @@ bisect_write() {
 	test -z "$nolog" && echo "git-bisect $state $rev" >>"$GIT_DIR/BISECT_LOG"
 }
 
-bisect_bad() {
+bisect_state() {
 	bisect_autostart
-	case "$#" in
-	0)
-		rev=$(git rev-parse --verify HEAD) ;;
-	1)
-		rev=$(git rev-parse --verify "$1^{commit}") ;;
+	state=$1
+	case "$#,$state" in
+	0,*)
+		die "Please call 'bisect_state' with at least one argument." ;;
+	1,bad|1,good|1,dunno)
+		rev=$(git rev-parse --verify HEAD) ||
+			die "Bad rev input: HEAD"
+		bisect_write "$state" "$rev" ;;
+	2,bad)
+		rev=$(git rev-parse --verify "$2^{commit}") ||
+			die "Bad rev input: $2"
+		bisect_write "$state" "$rev" ;;
+	*,good|*,dunno)
+		shift
+		revs=$(git rev-parse --revs-only --no-flags "$@") &&
+			test '' != "$revs" || die "Bad rev input: $@"
+		for rev in $revs
+		do
+			rev=$(git rev-parse --verify "$rev^{commit}") ||
+				die "Bad rev commit: $rev^{commit}"
+			bisect_write "$state" "$rev"
+		done ;;
 	*)
 		usage ;;
-	esac || exit
-	bisect_write 'bad' "$rev"
-	bisect_auto_next
-}
-
-bisect_good() {
-	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 'good' "$rev"
-	done
-	bisect_auto_next
-}
-
-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"
-	done
 	bisect_auto_next
 }
 
@@ -404,17 +390,15 @@ bisect_run () {
 	  exit $res
       fi
 
-      # Use "bisect_good" or "bisect_bad"
-      # depending on run success or failure.
+      # Find current state depending on run success or failure.
       if [ $res -gt 0 ]; then
-	  next_bisect='bisect_bad'
+	  state='bad'
       else
-	  next_bisect='bisect_good'
+	  state='good'
       fi
 
-      # We have to use a subshell because bisect_good or
-      # bisect_bad functions can exit.
-      ( $next_bisect > "$GIT_DIR/BISECT_RUN" )
+      # We have to use a subshell because "bisect_state" can exit.
+      ( bisect_state $state > "$GIT_DIR/BISECT_RUN" )
       res=$?
 
       cat "$GIT_DIR/BISECT_RUN"
@@ -443,12 +427,8 @@ case "$#" in
     case "$cmd" in
     start)
         bisect_start "$@" ;;
-    bad)
-        bisect_bad "$@" ;;
-    good)
-        bisect_good "$@" ;;
-    dunno)
-        bisect_dunno "$@" ;;
+    bad|good|dunno)
+        bisect_state "$cmd" "$@" ;;
     next)
         # Not sure we want "next" at the UI level anymore.
         bisect_next "$@" ;;
-- 
1.5.3.4.213.g68ad5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 6/7] Bisect: factorise "bisect_{bad,good,dunno}" into "bisect_state".
  2007-10-14 12:30 [PATCH 6/7] Bisect: factorise "bisect_{bad,good,dunno}" into "bisect_state" Christian Couder
@ 2007-10-14 16:15 ` Johannes Schindelin
  2007-10-15  3:42   ` Christian Couder
  0 siblings, 1 reply; 6+ messages in thread
From: Johannes Schindelin @ 2007-10-14 16:15 UTC (permalink / raw)
  To: Christian Couder; +Cc: Junio Hamano, git

Hi,

On Sun, 14 Oct 2007, Christian Couder wrote:

> -bisect_bad() {
> +bisect_state() {
>  	bisect_autostart
> -	case "$#" in
> -	0)
> -		rev=$(git rev-parse --verify HEAD) ;;
> -	1)
> -		rev=$(git rev-parse --verify "$1^{commit}") ;;
> +	state=$1
> +	case "$#,$state" in
> +	0,*)
> +		die "Please call 'bisect_state' with at least one argument." ;;
> +	1,bad|1,good|1,dunno)
> +		rev=$(git rev-parse --verify HEAD) ||
> +			die "Bad rev input: HEAD"
> +		bisect_write "$state" "$rev" ;;
> +	2,bad)
> +		rev=$(git rev-parse --verify "$2^{commit}") ||
> +			die "Bad rev input: $2"
> +		bisect_write "$state" "$rev" ;;

Really?  As far as I see, "2,bad" is an error in the current bisect.

> @@ -404,17 +390,15 @@ bisect_run () {
>  	  exit $res
>        fi
>  
> -      # Use "bisect_good" or "bisect_bad"
> -      # depending on run success or failure.
> +      # Find current state depending on run success or failure.
>        if [ $res -gt 0 ]; then
> -	  next_bisect='bisect_bad'
> +	  state='bad'
>        else
> -	  next_bisect='bisect_good'
> +	  state='good'
>        fi

Maybe it is time to have a special exit status for "dunno"?  But this is 
not something to fix in your patch, just an idea for a future patch.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 6/7] Bisect: factorise "bisect_{bad,good,dunno}" into "bisect_state".
  2007-10-14 16:15 ` Johannes Schindelin
@ 2007-10-15  3:42   ` Christian Couder
  2007-10-15 12:38     ` Johannes Schindelin
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Couder @ 2007-10-15  3:42 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio Hamano, git

Le dimanche 14 octobre 2007, Johannes Schindelin a écrit :
> Hi,
>
> On Sun, 14 Oct 2007, Christian Couder wrote:
> > -bisect_bad() {
> > +bisect_state() {
> >  	bisect_autostart
> > -	case "$#" in
> > -	0)
> > -		rev=$(git rev-parse --verify HEAD) ;;
> > -	1)
> > -		rev=$(git rev-parse --verify "$1^{commit}") ;;
> > +	state=$1
> > +	case "$#,$state" in
> > +	0,*)
> > +		die "Please call 'bisect_state' with at least one argument." ;;
> > +	1,bad|1,good|1,dunno)
> > +		rev=$(git rev-parse --verify HEAD) ||
> > +			die "Bad rev input: HEAD"
> > +		bisect_write "$state" "$rev" ;;
> > +	2,bad)
> > +		rev=$(git rev-parse --verify "$2^{commit}") ||
> > +			die "Bad rev input: $2"
> > +		bisect_write "$state" "$rev" ;;
>
> Really?  As far as I see, "2,bad" is an error in the current bisect.

But the new "bisect_state" takes one more argument, because the first one 
must be "good" "bad" or "dunno".

So when there is only one argument HEAD is used, and when there are 2 
arguments, $2 is used as the good|bad|dunno rev. 

> > @@ -404,17 +390,15 @@ bisect_run () {
> >  	  exit $res
> >        fi
> >
> > -      # Use "bisect_good" or "bisect_bad"
> > -      # depending on run success or failure.
> > +      # Find current state depending on run success or failure.
> >        if [ $res -gt 0 ]; then
> > -	  next_bisect='bisect_bad'
> > +	  state='bad'
> >        else
> > -	  next_bisect='bisect_good'
> > +	  state='good'
> >        fi
>
> Maybe it is time to have a special exit status for "dunno"?  But this is
> not something to fix in your patch, just an idea for a future patch.

Yes, I will think about it.

Thanks,
Christian.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 6/7] Bisect: factorise "bisect_{bad,good,dunno}" into "bisect_state".
  2007-10-15  3:42   ` Christian Couder
@ 2007-10-15 12:38     ` Johannes Schindelin
  2007-10-16  6:28       ` Christian Couder
  0 siblings, 1 reply; 6+ messages in thread
From: Johannes Schindelin @ 2007-10-15 12:38 UTC (permalink / raw)
  To: Christian Couder; +Cc: Junio Hamano, git

Hi,

On Mon, 15 Oct 2007, Christian Couder wrote:

> Le dimanche 14 octobre 2007, Johannes Schindelin a ?crit :
>
> > On Sun, 14 Oct 2007, Christian Couder wrote:
> > > -bisect_bad() {
> > > +bisect_state() {
> > >  	bisect_autostart
> > > -	case "$#" in
> > > -	0)
> > > -		rev=$(git rev-parse --verify HEAD) ;;
> > > -	1)
> > > -		rev=$(git rev-parse --verify "$1^{commit}") ;;
> > > +	state=$1
> > > +	case "$#,$state" in
> > > +	0,*)
> > > +		die "Please call 'bisect_state' with at least one argument." ;;
> > > +	1,bad|1,good|1,dunno)
> > > +		rev=$(git rev-parse --verify HEAD) ||
> > > +			die "Bad rev input: HEAD"
> > > +		bisect_write "$state" "$rev" ;;
> > > +	2,bad)
> > > +		rev=$(git rev-parse --verify "$2^{commit}") ||
> > > +			die "Bad rev input: $2"
> > > +		bisect_write "$state" "$rev" ;;
> >
> > Really?  As far as I see, "2,bad" is an error in the current bisect.
> 
> But the new "bisect_state" takes one more argument, because the first 
> one must be "good" "bad" or "dunno".
> 
> So when there is only one argument HEAD is used, and when there are 2 
> arguments, $2 is used as the good|bad|dunno rev.

Ah, that explains it!  But do you not need to do "2,bad|2,good|2,dunno" in 
that case?  Or even better: "2,*"?

Thanks,
Dscho

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 6/7] Bisect: factorise "bisect_{bad,good,dunno}" into "bisect_state".
  2007-10-15 12:38     ` Johannes Schindelin
@ 2007-10-16  6:28       ` Christian Couder
  2007-10-16 11:20         ` Johannes Schindelin
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Couder @ 2007-10-16  6:28 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio Hamano, git

Hi Dscho,

Le lundi 15 octobre 2007, Johannes Schindelin a écrit :
> Hi,
>
> On Mon, 15 Oct 2007, Christian Couder wrote:
> >
> > But the new "bisect_state" takes one more argument, because the first
> > one must be "good" "bad" or "dunno".
> >
> > So when there is only one argument HEAD is used, and when there are 2
> > arguments, $2 is used as the good|bad|dunno rev.
>
> Ah, that explains it!  But do you not need to do "2,bad|2,good|2,dunno"
> in that case?  Or even better: "2,*"?

Perhaps it would be an improvement at least in speed for "2,good" 
or "2,dunno".

I wanted to keep exactly the same processing as there was before, in case 
something like "git bisect good v1.5.3.3..v1.5.3.4" was supported. But it 
seems it doesn't work. I don't know if it's a bug or a feature.

Christian.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 6/7] Bisect: factorise "bisect_{bad,good,dunno}" into "bisect_state".
  2007-10-16  6:28       ` Christian Couder
@ 2007-10-16 11:20         ` Johannes Schindelin
  0 siblings, 0 replies; 6+ messages in thread
From: Johannes Schindelin @ 2007-10-16 11:20 UTC (permalink / raw)
  To: Christian Couder; +Cc: Junio Hamano, git

Hi,

[finally, a technical argument on "dunno".  Maybe we really need 
git-bikeshed@vger.kernel.org? ;-)]

On Tue, 16 Oct 2007, Christian Couder wrote:

> Le lundi 15 octobre 2007, Johannes Schindelin a ?crit :
> >
> > On Mon, 15 Oct 2007, Christian Couder wrote:
> > >
> > > But the new "bisect_state" takes one more argument, because the first
> > > one must be "good" "bad" or "dunno".
> > >
> > > So when there is only one argument HEAD is used, and when there are 
> > > 2 arguments, $2 is used as the good|bad|dunno rev.
> >
> > Ah, that explains it!  But do you not need to do 
> > "2,bad|2,good|2,dunno" in that case?  Or even better: "2,*"?
> 
> Perhaps it would be an improvement at least in speed for "2,good" or 
> "2,dunno".

I see: the later case catches them.  Colour me satisfied with your patch.

> I wanted to keep exactly the same processing as there was before, in case 
> something like "git bisect good v1.5.3.3..v1.5.3.4" was supported. But it 
> seems it doesn't work. I don't know if it's a bug or a feature.

I think v1.5.3.3..v1.5.3.4 expands to ^v1.5.3.3 v1.5.3.4, which might 
explain what you are experiencing.

OTOH "git bisect good v1.5.3.3..v1.5.3.4" does not make sense.  bisect 
assumes that all ancestors of a good commit are good, too.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-10-16 11:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-10-14 12:30 [PATCH 6/7] Bisect: factorise "bisect_{bad,good,dunno}" into "bisect_state" Christian Couder
2007-10-14 16:15 ` Johannes Schindelin
2007-10-15  3:42   ` Christian Couder
2007-10-15 12:38     ` Johannes Schindelin
2007-10-16  6:28       ` Christian Couder
2007-10-16 11:20         ` Johannes Schindelin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.