[TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Johannes Schindelin]

When you look for a fix instead of a regression, it can be quite hard
to twist your brain into choosing the correct bisect command between
'git bisect bad' and 'git bisect good'.

So introduce the commands 'git bisect fixed' and 'git bisect unfixed'.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	When Randal talked about this on IRC, I laughed.  But I just had 
	the case where it took me _three_ attempts at a bisection, only
	to give up and write this patchlet.

	May it help someone else, too.

 git-bisect.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 8b11107..d833e21 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -501,6 +501,8 @@ case "$#" in
 *)
     cmd="$1"
     shift
+    test $cmd = fixed && cmd=bad
+    test $cmd = unfixed && cmd=good
     case "$cmd" in
     help)
         git bisect -h ;;
-- 
1.5.6.127.g3fb9f

Re: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Stephan Beyer]

Hi,

> So introduce the commands 'git bisect fixed' and 'git bisect unfixed'.

Are they intentionally undocumented to not raise confusion?

Regards,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

Re: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Johannes Schindelin]

Hi,

On Tue, 24 Jun 2008, Stephan Beyer wrote:

> > So introduce the commands 'git bisect fixed' and 'git bisect unfixed'.
> 
> Are they intentionally undocumented to not raise confusion?

Umm.  Which part of "TOY" is unclear?

Ciao,
Dscho

Re: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Nicolas Pitre]

On Tue, 24 Jun 2008, Johannes Schindelin wrote:

> 
> When you look for a fix instead of a regression, it can be quite hard
> to twist your brain into choosing the correct bisect command between
> 'git bisect bad' and 'git bisect good'.
> 
> So introduce the commands 'git bisect fixed' and 'git bisect unfixed'.

I really like it.  And yes, I know what you mean.


Nicolas

Re: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Stephan Beyer]

> > > So introduce the commands 'git bisect fixed' and 'git bisect unfixed'.
> > 
> > Are they intentionally undocumented to not raise confusion?
> 
> Umm.  Which part of "TOY" is unclear?

The T, O and Y.
No; after searching for "TOY PATCH" on gmane: none :)

Regards.

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

Re: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Jeff King]

On Tue, Jun 24, 2008 at 03:17:18PM +0100, Johannes Schindelin wrote:

> When you look for a fix instead of a regression, it can be quite hard
> to twist your brain into choosing the correct bisect command between
> 'git bisect bad' and 'git bisect good'.
> 
> So introduce the commands 'git bisect fixed' and 'git bisect unfixed'.

Thanks. This just bit me the other day, and I thought of the same
solution. I think it might be worth a "non-toy" patch.

-Peff

Re: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Johannes Schindelin]

Hi,

On Tue, 24 Jun 2008, Jeff King wrote:

> On Tue, Jun 24, 2008 at 03:17:18PM +0100, Johannes Schindelin wrote:
> 
> > When you look for a fix instead of a regression, it can be quite hard
> > to twist your brain into choosing the correct bisect command between
> > 'git bisect bad' and 'git bisect good'.
> > 
> > So introduce the commands 'git bisect fixed' and 'git bisect unfixed'.
> 
> Thanks. This just bit me the other day, and I thought of the same
> solution. I think it might be worth a "non-toy" patch.

Okay, that's 3 people who I take the courage from to turn this into a 
proper patch.

Ciao,
Dscho

Re: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Reini Urban]

Jeff King schrieb:
> On Tue, Jun 24, 2008 at 03:17:18PM +0100, Johannes Schindelin wrote:
> 
>> When you look for a fix instead of a regression, it can be quite hard
>> to twist your brain into choosing the correct bisect command between
>> 'git bisect bad' and 'git bisect good'.
>>
>> So introduce the commands 'git bisect fixed' and 'git bisect unfixed'.
> 
> Thanks. This just bit me the other day, and I thought of the same
> solution. I think it might be worth a "non-toy" patch.

Maybe "notfixed" is a better wording than "unfixed".

[NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Johannes Schindelin]

When you look for a fix instead of a regression, it can be quite hard
to twist your brain into choosing the correct bisect command between
'git bisect bad' and 'git bisect good'.

So introduce the commands 'git bisect fixed' and 'git bisect unfixed'.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Tue, 24 Jun 2008, Johannes Schindelin wrote:

	> Okay, that's 3 people who I take the courage from to turn this 
	> into a proper patch.

	And this is my first attempt at a proper patch for it.

	Now with documentation, and hopefully all places where the
	user is being told about a "bad" commit.

 Documentation/git-bisect.txt |   16 ++++++++++++++++
 git-bisect.sh                |   25 ++++++++++++++++++-------
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 3ca0d33..3fb3b11 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -26,6 +26,9 @@ on the subcommand:
  git bisect log
  git bisect run <cmd>...
 
+ git bisect fixed [<rev>]
+ git bisect unfixed [<rev>...]
+
 This command uses 'git-rev-list --bisect' option to help drive the
 binary search process to find which change introduced a bug, given an
 old "good" commit object name and a later "bad" commit object name.
@@ -76,6 +79,19 @@ 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".
 
+Searching for fixes instead of regressions
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Sometimes you need to find a fix, not a regression.  The bisection
+machinery is really the same for this, but it might be tricky to remember
+to mark a commit "bad" when it contains the fix.
+
+So synonyms for "bad" and "good" are available, "fixed" and "unfixed"
+respectively.
+
+To mark a commit that contains the fix, call "git bisect fixed", and
+"git bisect unfixed" if it does not contain the fix.
+
 Bisect reset
 ~~~~~~~~~~~~
 
diff --git a/git-bisect.sh b/git-bisect.sh
index 8b11107..6e71e1a 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
+USAGE='[help|start|bad|good|fixed|unfixed|skip|next|reset|visualize|replay|log|run]'
 LONG_USAGE='git bisect help
         print this long help message.
 git bisect start [<bad> [<good>...]] [--] [<pathspec>...]
@@ -24,6 +24,13 @@ git bisect log
 git bisect run <cmd>...
         use <cmd>... to automatically bisect.
 
+When not looking for a regression, but a fix instead, you can use
+
+git bisect fixed [<rev>]
+	mark <rev> as having the fix you are looking for
+git bisect unfixed [<rev>]
+	mark <rev> as not having the fix you are looking for
+
 Please use "git help bisect" to get the full man page.'
 
 OPTIONS_SPEC=
@@ -216,7 +223,7 @@ bisect_next_check() {
 	t,,good)
 		# have bad but not good.  we could bisect although
 		# this is less optimum.
-		echo >&2 'Warning: bisecting only with a bad commit.'
+		echo >&2 'Warning: bisecting only with a bad (or fixed) commit.'
 		if test -t 0
 		then
 			printf >&2 'Are you sure [Y/n]? '
@@ -231,7 +238,7 @@ bisect_next_check() {
 			THEN='then '
 		}
 		echo >&2 'You '$THEN'need to give me at least one good' \
-			'and one bad revisions.'
+			'and one bad (or fixed) revision.'
 		echo >&2 '(You can use "git bisect bad" and' \
 			'"git bisect good" for that.)'
 		exit 1 ;;
@@ -324,7 +331,7 @@ exit_if_skipped_commits () {
 	_tried=$1
 	if expr "$_tried" : ".*[|].*" > /dev/null ; then
 		echo "There are only 'skip'ped commit left to test."
-		echo "The first bad commit could be any of:"
+		echo "The first bad (or fixed) commit could be any of:"
 		echo "$_tried" | tr '[|]' '[\012]'
 		echo "We cannot bisect more!"
 		exit 2
@@ -356,7 +363,7 @@ bisect_next() {
 	fi
 	if [ "$bisect_rev" = "$bad" ]; then
 		exit_if_skipped_commits "$bisect_tried"
-		echo "$bisect_rev is first bad commit"
+		echo "$bisect_rev is first bad (or fixed) commit"
 		git diff-tree --pretty $bisect_rev
 		exit 0
 	fi
@@ -474,7 +481,8 @@ bisect_run () {
 
       cat "$GIT_DIR/BISECT_RUN"
 
-      if grep "first bad commit could be any of" "$GIT_DIR/BISECT_RUN" \
+      if grep "first bad (or fixed) commit could be any of" \
+			"$GIT_DIR/BISECT_RUN" \
 		> /dev/null; then
 	  echo >&2 "bisect run cannot continue any more"
 	  exit $res
@@ -486,7 +494,8 @@ bisect_run () {
 	  exit $res
       fi
 
-      if grep "is first bad commit" "$GIT_DIR/BISECT_RUN" > /dev/null; then
+      if grep "is first bad (or fixed) commit" \
+		"$GIT_DIR/BISECT_RUN" > /dev/null; then
 	  echo "bisect run success"
 	  exit 0;
       fi
@@ -501,6 +510,8 @@ case "$#" in
 *)
     cmd="$1"
     shift
+    test $cmd = fixed && cmd=bad
+    test $cmd = unfixed && cmd=good
     case "$cmd" in
     help)
         git bisect -h ;;
-- 
1.5.6.173.gde14c

Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Jeff King]

On Tue, Jun 24, 2008 at 06:09:28PM +0100, Johannes Schindelin wrote:

> 	And this is my first attempt at a proper patch for it.
> 
> 	Now with documentation, and hopefully all places where the
> 	user is being told about a "bad" commit.

This looks reasonably sane to me. The only thing I can think of that
we're missing is that "git bisect visualize" will still show the refs as
"bisect/bad" and "bisect/good".

To fix that, you'd have to ask people to start the bisect by saying "I
am bisecting to find a fix, not a breakage." And then you could change
the refnames and all of the messages as appropriate.

-Peff

Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Daniel Barkalow]

On Tue, 24 Jun 2008, Jeff King wrote:

> On Tue, Jun 24, 2008 at 06:09:28PM +0100, Johannes Schindelin wrote:
> 
> > 	And this is my first attempt at a proper patch for it.
> > 
> > 	Now with documentation, and hopefully all places where the
> > 	user is being told about a "bad" commit.
> 
> This looks reasonably sane to me. The only thing I can think of that
> we're missing is that "git bisect visualize" will still show the refs as
> "bisect/bad" and "bisect/good".
> 
> To fix that, you'd have to ask people to start the bisect by saying "I
> am bisecting to find a fix, not a breakage." And then you could change
> the refnames and all of the messages as appropriate.

That would also be a good way of taking care of the problem where someone 
gets distracted while running a slow test, forgets what they're looking 
for, and marks the result as "bad" instead of "unfixed".

	-Daniel
*This .sig left intentionally blank*

Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Johannes Schindelin]

Hi,

On Tue, 24 Jun 2008, Daniel Barkalow wrote:

> On Tue, 24 Jun 2008, Jeff King wrote:
> 
> > On Tue, Jun 24, 2008 at 06:09:28PM +0100, Johannes Schindelin wrote:
> > 
> > > 	And this is my first attempt at a proper patch for it.
> > > 
> > > 	Now with documentation, and hopefully all places where the
> > > 	user is being told about a "bad" commit.
> > 
> > This looks reasonably sane to me. The only thing I can think of that
> > we're missing is that "git bisect visualize" will still show the refs as
> > "bisect/bad" and "bisect/good".
> > 
> > To fix that, you'd have to ask people to start the bisect by saying "I
> > am bisecting to find a fix, not a breakage." And then you could change
> > the refnames and all of the messages as appropriate.
> 
> That would also be a good way of taking care of the problem where someone 
> gets distracted while running a slow test, forgets what they're looking 
> for, and marks the result as "bad" instead of "unfixed".

Feel free to rework my patch.

Ciao,
Dscho

Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[SZEDER Gábor]

On Tue, Jun 24, 2008 at 06:09:28PM +0100, Johannes Schindelin wrote:
> So introduce the commands 'git bisect fixed' and 'git bisect unfixed'.
And maybe this one squashed on it, to add completion support for the
new subcommands.


---
 contrib/completion/git-completion.bash |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ebf7cde..014adab 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -511,7 +511,9 @@ _git_add ()
 
 _git_bisect ()
 {
-	local subcommands="start bad good reset visualize replay log"
+	local subcommands="
+		start bad good reset visualize replay log fixed unfixed
+		"
 	local subcommand="$(__git_find_subcommand "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		__gitcomp "$subcommands"
-- 
1.5.6.64.g7dc1df

Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Michael Haggerty]

Johannes Schindelin wrote:
> When you look for a fix instead of a regression, it can be quite hard
> to twist your brain into choosing the correct bisect command between
> 'git bisect bad' and 'git bisect good'.
> 
> So introduce the commands 'git bisect fixed' and 'git bisect unfixed'.

It seems to me that your problem is that git-bisect requires the "good"
revision to be older than the "bad" one.  If this requirement were
removed, would there still be a need for "fixed" vs. "unfixed"?

A bisection search doesn't care what labels are applied to the two
endpoints, as it only looks for transitions between the labels.
Therefore it should be easy to teach git-bisect to locate either kind of
transition, "bad" -> "good" or "good" -> "bad", depending only on where
the user places the original "good" and "bad" tags.

Michael

Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Johannes Schindelin]

Hi,

On Tue, 24 Jun 2008, Michael Haggerty wrote:

> Johannes Schindelin wrote:
> > When you look for a fix instead of a regression, it can be quite hard
> > to twist your brain into choosing the correct bisect command between
> > 'git bisect bad' and 'git bisect good'.
> > 
> > So introduce the commands 'git bisect fixed' and 'git bisect unfixed'.
> 
> It seems to me that your problem is that git-bisect requires the "good" 
> revision to be older than the "bad" one.  If this requirement were 
> removed, would there still be a need for "fixed" vs. "unfixed"?

Nope.

The thing that makes "fixed" and "bad" special is that _one_ commit 
introduced that.

Ciao,
Dscho

Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Jun 24, 2008 at 06:09:28PM +0100, Johannes Schindelin wrote:
>
>> 	And this is my first attempt at a proper patch for it.
>> 
>> 	Now with documentation, and hopefully all places where the
>> 	user is being told about a "bad" commit.
>
> This looks reasonably sane to me. The only thing I can think of that
> we're missing is that "git bisect visualize" will still show the refs as
> "bisect/bad" and "bisect/good".
>
> To fix that, you'd have to ask people to start the bisect by saying "I
> am bisecting to find a fix, not a breakage." And then you could change
> the refnames and all of the messages as appropriate.

It probably is not just a good idea, but is a necessary fix, to remove
confusion like this example that appears everywhere:

>  		echo >&2 'You '$THEN'need to give me at least one good' \
> -			'and one bad revisions.'
> +			'and one bad (or fixed) revision.'
>  		echo >&2 '(You can use "git bisect bad" and' \
>  			'"git bisect good" for that.)'

People who are reading the change Dscho did in the "patch" form may not
notice it, but imagine how the above looks to the end user who was told
that "new bisect can now look for fixes", who does not need to nor even
want to know that the new feature is implemented by making bad and fixed
synonyms.

They need to mentally reword "good" into "unfixed" and "bisect bad" into
"bisect fixed" while reading the output from the above pieces, but the
point of this new "look for fixes" feature is they do not have to do the
rewording anymore!

Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Tue, 24 Jun 2008, Michael Haggerty wrote:
> ...
>> It seems to me that your problem is that git-bisect requires the "good" 
>> revision to be older than the "bad" one.  If this requirement were 
>> removed, would there still be a need for "fixed" vs. "unfixed"?
>
> Nope.
>
> The thing that makes "fixed" and "bad" special is that _one_ commit 
> introduced that.

That was my initial reaction, and I actually was about to phrase it more
bluntly: you do not understand what "bisect" is.

But that was a reaction without thinking things through.  It may not be
what "git bisect" currently is, but the suggestion does not go against
what the underlying "git rev-list --bisect" is at all.  I think what
Michael is speculating is different, and it makes sense in its own way.

Instead of having a set of bisect/good-* refs and a single bisect-bad ref,
your "fixed and unfixed" mode could work quite differently.  By noticing
that the topology the user specified with initial good and bad have
ancient bad and recent good --- that is, "it used to be bad but now it is
good" --- you could instead use a set of bisect/bad-* refs and a single
bisect-good ref, and feed good and bad swapped to "rev-list --bisect" in
bisect_next().  That way, the labels given by visualize will match what
the user is doing automatically.

I said "it makes sense in its own way", because it is _quite_ different
from how git-bisect currently assumes, and restructuring git-bisect to
operate naturally in a way Michael describes would be a much larger
surgery with costs (including risks of bugs) associated with it, which
needs to be weighed in when judging that approach would actually make
sense.

Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Nicolas Pitre]

On Tue, 24 Jun 2008, Junio C Hamano wrote:

> Instead of having a set of bisect/good-* refs and a single bisect-bad ref,
> your "fixed and unfixed" mode could work quite differently.  By noticing
> that the topology the user specified with initial good and bad have
> ancient bad and recent good --- that is, "it used to be bad but now it is
> good" --- you could instead use a set of bisect/bad-* refs and a single
> bisect-good ref, and feed good and bad swapped to "rev-list --bisect" in
> bisect_next().  That way, the labels given by visualize will match what
> the user is doing automatically.

... and the final answer would be "the first good commit is ...".

That would be awesome, much nicer than yet more keywords.

> I said "it makes sense in its own way", because it is _quite_ different
> from how git-bisect currently assumes, and restructuring git-bisect to
> operate naturally in a way Michael describes would be a much larger
> surgery with costs (including risks of bugs) associated with it, which
> needs to be weighed in when judging that approach would actually make
> sense.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


Nicolas

Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Lea Wiemann]

Michael Haggerty wrote:
> Therefore it should be easy to teach git-bisect to locate either kind of
> transition, "bad" -> "good" or "good" -> "bad", depending only on where
> the user places the original "good" and "bad" tags.

I think this is a good suggestion (though I haven't thought things 
through).  Another idea is to add "old" and "new" (or something like 
that) as aliases to "good" and "bad", since that's the only semantics 
that the bisect labels actually seem to have.

-- Lea

Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[A Large Angry SCM]

Lea Wiemann wrote:
> Michael Haggerty wrote:
>> Therefore it should be easy to teach git-bisect to locate either kind of
>> transition, "bad" -> "good" or "good" -> "bad", depending only on where
>> the user places the original "good" and "bad" tags.
> 
> I think this is a good suggestion (though I haven't thought things 
> through).  Another idea is to add "old" and "new" (or something like 
> that) as aliases to "good" and "bad", since that's the only semantics 
> that the bisect labels actually seem to have.

"Before" and "After" the "Change" maybe?

Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Karl Hasselström]

On 2008-06-24 19:53:07 -0400, A Large Angry SCM wrote:

> Lea Wiemann wrote:
>
> > I think this is a good suggestion (though I haven't thought things
> > through). Another idea is to add "old" and "new" (or something
> > like that) as aliases to "good" and "bad", since that's the only
> > semantics that the bisect labels actually seem to have.
>
> "Before" and "After" the "Change" maybe?

Ha. It would not be hard to make it accept any two tags the user
happens to use. fast/slow, works/broken, fina-fisken/totalkvaddad, ...
it even comes with built-in internationalization!

/me ducks.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'

[Christian Couder]

Le mercredi 25 juin 2008, Junio C Hamano a écrit :
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > On Tue, 24 Jun 2008, Michael Haggerty wrote:
> > ...
> >
> >> It seems to me that your problem is that git-bisect requires the
> >> "good" revision to be older than the "bad" one.  

Yes, "git bisect" works if the good revisions are ancestors of the bad 
revision.

Currently if you mistake good and bad revs (and if one of the rev is
an ancestor of the other) you get something like:

$ git bisect start HEAD~3 HEAD
'git rev-list --bisect-vars' failed:
maybe you mistake good and bad revs?

I also noticed that if the good and bad are siblings for example like:

A-B-C-D
   \E-F

and you say:

$ git bisect start D F

(that means D is bad and F is good)

then it will kind of "work" but only C and D will be considered as possible 
first bad commits. This is arguably a bug because for example E could have 
fixed a bug that always existed, and then the first bad commit is B or A 
depending how we define it.

> >> If this requirement 
> >> were removed, would there still be a need for "fixed" vs. "unfixed"?

Well this requirement can be "removed" in different ways.

1) We could just allow anything to be called "bad" and "good" as long as 
there is either:

- only one bad revision and all good revisions are its ancestor, or
- only one good revision and all bad revisions are its ancestor

2) Another way to remove the requirement is to make it work in the siblings 
case above.

> > Nope.
> >
> > The thing that makes "fixed" and "bad" special is that _one_ commit
> > introduced that.
>
> That was my initial reaction, and I actually was about to phrase it more
> bluntly: you do not understand what "bisect" is.
>
> But that was a reaction without thinking things through.  It may not be
> what "git bisect" currently is, but the suggestion does not go against
> what the underlying "git rev-list --bisect" is at all.

If we want to make the siblings case (case 2) work, then "git 
rev-list --bisect" needs work though.

> I think what 
> Michael is speculating is different, and it makes sense in its own way.
>
> Instead of having a set of bisect/good-* refs and a single bisect-bad
> ref, your "fixed and unfixed" mode could work quite differently.  By
> noticing that the topology the user specified with initial good and bad
> have ancient bad and recent good --- that is, "it used to be bad but now
> it is good" --- you could instead use a set of bisect/bad-* refs and a
> single bisect-good ref, and feed good and bad swapped to "rev-list
> --bisect" in bisect_next().  That way, the labels given by visualize will
> match what the user is doing automatically.

Yes, that is the case 1 above.

> I said "it makes sense in its own way", because it is _quite_ different
> from how git-bisect currently assumes, and restructuring git-bisect to
> operate naturally in a way Michael describes would be a much larger
> surgery with costs (including risks of bugs) associated with it, which
> needs to be weighed in when judging that approach would actually make
> sense.

Yes it needs work in git-bisect.sh and I don't think the current situation 
with the "maybe you mistake good and bad revs?" error message is too bad.

Regards,
Christian.

[PATCH, next version] git bisect: introduce 'fixed' and 'unfixed'

[Johannes Schindelin]

When you look for a fix instead of a regression, it can be quite hard
to twist your brain into choosing the correct bisect command between
'git bisect bad' and 'git bisect good'.

So introduce the commands 'git bisect fixed' and 'git bisect unfixed'.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Tue, 24 Jun 2008, Junio C Hamano wrote:

	> Jeff King <peff@peff.net> writes:
	> 
	> > On Tue, Jun 24, 2008 at 06:09:28PM +0100, Johannes Schindelin 
	> > wrote:
	> >
	> >> 	And this is my first attempt at a proper patch for it.
	> >> 
	> >> 	Now with documentation, and hopefully all places where the
	> >> 	user is being told about a "bad" commit.
	> >
	> > This looks reasonably sane to me. The only thing I can think 
	> > of that we're missing is that "git bisect visualize" will still
	> > show the refs as "bisect/bad" and "bisect/good".
	> >
	> > To fix that, you'd have to ask people to start the bisect by 
	> > saying "I am bisecting to find a fix, not a breakage." And then
	> > you could change the refnames and all of the messages as
	> > appropriate.
	> 
	> It probably is not just a good idea, but is a necessary fix, to 
	> remove confusion like this example that appears everywhere:
	> 
	> >  		echo >&2 'You '$THEN'need to give me at least one good' \
	> > -			'and one bad revisions.'
	> > +			'and one bad (or fixed) revision.'
	> >  		echo >&2 '(You can use "git bisect bad" and' \
	> >  			'"git bisect good" for that.)'
	> 
	> People who are reading the change Dscho did in the "patch" form 
	> may not notice it, but imagine how the above looks to the end user
	> who was told that "new bisect can now look for fixes", who does
	> not need to nor even want to know that the new feature is
	> implemented by making bad and fixed synonyms.
	> 
	> They need to mentally reword "good" into "unfixed" and "bisect 
	> bad" into "bisect fixed" while reading the output from the above 
	> pieces, but the point of this new "look for fixes" feature is they do 
	> not have to do the rewording anymore!

	How about autodetecting from the user's last input what she meant?

 Documentation/git-bisect.txt |   16 ++++++++++++++++
 git-bisect.sh                |   42 ++++++++++++++++++++++++++++++------------
 2 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 3ca0d33..3fb3b11 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -26,6 +26,9 @@ on the subcommand:
  git bisect log
  git bisect run <cmd>...
 
+ git bisect fixed [<rev>]
+ git bisect unfixed [<rev>...]
+
 This command uses 'git-rev-list --bisect' option to help drive the
 binary search process to find which change introduced a bug, given an
 old "good" commit object name and a later "bad" commit object name.
@@ -76,6 +79,19 @@ 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".
 
+Searching for fixes instead of regressions
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Sometimes you need to find a fix, not a regression.  The bisection
+machinery is really the same for this, but it might be tricky to remember
+to mark a commit "bad" when it contains the fix.
+
+So synonyms for "bad" and "good" are available, "fixed" and "unfixed"
+respectively.
+
+To mark a commit that contains the fix, call "git bisect fixed", and
+"git bisect unfixed" if it does not contain the fix.
+
 Bisect reset
 ~~~~~~~~~~~~
 
diff --git a/git-bisect.sh b/git-bisect.sh
index 8b11107..197489b 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
+USAGE='[help|start|bad|good|fixed|unfixed|skip|next|reset|visualize|replay|log|run]'
 LONG_USAGE='git bisect help
         print this long help message.
 git bisect start [<bad> [<good>...]] [--] [<pathspec>...]
@@ -24,6 +24,13 @@ git bisect log
 git bisect run <cmd>...
         use <cmd>... to automatically bisect.
 
+When not looking for a regression, but a fix instead, you can use
+
+git bisect fixed [<rev>]
+	mark <rev> as having the fix you are looking for
+git bisect unfixed [<rev>]
+	mark <rev> as not having the fix you are looking for
+
 Please use "git help bisect" to get the full man page.'
 
 OPTIONS_SPEC=
@@ -32,6 +39,8 @@ require_work_tree
 
 _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"
+GOOD="good"
+BAD="bad"
 
 sq() {
 	@@PERL@@ -e '
@@ -216,7 +225,7 @@ bisect_next_check() {
 	t,,good)
 		# have bad but not good.  we could bisect although
 		# this is less optimum.
-		echo >&2 'Warning: bisecting only with a bad commit.'
+		echo >&2 'Warning: bisecting only with a $BAD commit.'
 		if test -t 0
 		then
 			printf >&2 'Are you sure [Y/n]? '
@@ -230,10 +239,10 @@ bisect_next_check() {
 			echo >&2 'You need to start by "git bisect start".'
 			THEN='then '
 		}
-		echo >&2 'You '$THEN'need to give me at least one good' \
-			'and one bad revisions.'
-		echo >&2 '(You can use "git bisect bad" and' \
-			'"git bisect good" for that.)'
+		echo >&2 'You '$THEN'need to give me at least one $GOOD' \
+			'and one $BAD revision.'
+		echo >&2 '(You can use "git bisect $BAD" and' \
+			'"git bisect $GOOD" for that.)'
 		exit 1 ;;
 	esac
 }
@@ -250,7 +259,7 @@ eval_rev_list() {
 
 	if [ $res -ne 0 ]; then
 		echo >&2 "'git rev-list --bisect-vars' failed:"
-		echo >&2 "maybe you mistake good and bad revs?"
+		echo >&2 "maybe you mistake $GOOD and $BAD revs?"
 		exit $res
 	fi
 
@@ -324,7 +333,7 @@ exit_if_skipped_commits () {
 	_tried=$1
 	if expr "$_tried" : ".*[|].*" > /dev/null ; then
 		echo "There are only 'skip'ped commit left to test."
-		echo "The first bad commit could be any of:"
+		echo "The first $BAD commit could be any of:"
 		echo "$_tried" | tr '[|]' '[\012]'
 		echo "We cannot bisect more!"
 		exit 2
@@ -351,12 +360,12 @@ bisect_next() {
 	eval "$eval" || exit
 
 	if [ -z "$bisect_rev" ]; then
-		echo "$bad was both good and bad"
+		echo "$bad was both $GOOD and $BAD"
 		exit 1
 	fi
 	if [ "$bisect_rev" = "$bad" ]; then
 		exit_if_skipped_commits "$bisect_tried"
-		echo "$bisect_rev is first bad commit"
+		echo "$bisect_rev is first $BAD commit"
 		git diff-tree --pretty $bisect_rev
 		exit 0
 	fi
@@ -474,7 +483,8 @@ bisect_run () {
 
       cat "$GIT_DIR/BISECT_RUN"
 
-      if grep "first bad commit could be any of" "$GIT_DIR/BISECT_RUN" \
+      if grep "first $BAD commit could be any of" \
+			"$GIT_DIR/BISECT_RUN" \
 		> /dev/null; then
 	  echo >&2 "bisect run cannot continue any more"
 	  exit $res
@@ -486,7 +496,8 @@ bisect_run () {
 	  exit $res
       fi
 
-      if grep "is first bad commit" "$GIT_DIR/BISECT_RUN" > /dev/null; then
+      if grep "is first $BAD commit" \
+		"$GIT_DIR/BISECT_RUN" > /dev/null; then
 	  echo "bisect run success"
 	  exit 0;
       fi
@@ -502,6 +513,13 @@ case "$#" in
     cmd="$1"
     shift
     case "$cmd" in
+    fixed|unfixed)
+	BAD="fixed"
+	GOOD="unfixed"
+	test "$cmd" = fixed && cmd=bad
+	test "$cmd" = unfixed && cmd=good
+    esac
+    case "$cmd" in
     help)
         git bisect -h ;;
     start)
-- 
1.5.6.173.gde14c

Re: [PATCH, next version] git bisect: introduce 'fixed' and 'unfixed'

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> When you look for a fix instead of a regression, it can be quite hard
> to twist your brain into choosing the correct bisect command between
> 'git bisect bad' and 'git bisect good'.

Hmm, I do not currently see any differene between master and next version
of bisect.  In what way is this 'next' version?

Aside from the 'visualize' issue this does not attempt to address, I
wonder if it may be a good idea to detect and warn mixed usage as well
(e.g. "You earlier said 'bad' but now you are saying 'fixed' -- are you
sure?"), and if so if it can be implemented easily.

Re: [PATCH, next version] git bisect: introduce 'fixed' and 'unfixed'

[Johannes Schindelin]

Hi,

On Fri, 27 Jun 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > When you look for a fix instead of a regression, it can be quite hard 
> > to twist your brain into choosing the correct bisect command between 
> > 'git bisect bad' and 'git bisect good'.
> 
> Hmm, I do not currently see any differene between master and next version
> of bisect.  In what way is this 'next' version?

It has a "BAD" and a "GOOD" variable that are reset to "fixed" and 
"unfixed" if the user said "fixed" or "unfixed".

> Aside from the 'visualize' issue this does not attempt to address,

Yes, I forgot about that issue, mainly because I do not use it myself...

> I wonder if it may be a good idea to detect and warn mixed usage as well 
> (e.g. "You earlier said 'bad' but now you are saying 'fixed' -- are you 
> sure?"), and if so if it can be implemented easily.

Hmm.  I tried to avoid that, as it would mean a larger patch.  But I guess 
you could write .git/BISECT_TERMS or some such.

But that, together with the visualize part, would take more time than I am 
willing to spend on this issue.

Well, I guess I'll leave it then,
Dscho

Re: [PATCH, next version] git bisect: introduce 'fixed' and 'unfixed'

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> > When you look for a fix instead of a regression, it can be quite hard 
>> > to twist your brain into choosing the correct bisect command between 
>> > 'git bisect bad' and 'git bisect good'.
>> 
>> Hmm, I do not currently see any differene between master and next version
>> of bisect.  In what way is this 'next' version?
>
> It has a "BAD" and a "GOOD" variable that are reset to "fixed" and 
> "unfixed" if the user said "fixed" or "unfixed".

Ah, Ok, you did not mean "this is meant to applied to 'next' branch", but
meant "[PATCH v$N]" for some N > 1.

> But that, together with the visualize part, would take more time than I am 
> willing to spend on this issue.

Other people would find itch (or they may not).  Either way is fine.