threads / patch / 14123

patchgit bisect: introduce 'fixed' and 'unfixed'

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

## tl;dr

26 messages between Jun 24, 2008 and Jun 28, 2008. Diffs are folded; open one to read it.

replies: 25people: 13as markdown or json

Johannes Schindelin· Jun 24, 2008, 14:17 UTC · lore

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(-)
Show changes to git-bisect.sh +2 −0
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
Stephan Beyer· Jun 24, 2008, 14:42 UTC · re: Johannes Schindelin · lore

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

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
Johannes Schindelin· Jun 24, 2008, 14:55 UTC · re: Stephan Beyer · lore

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

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

Stephan Beyer· Jun 24, 2008, 15:16 UTC · re: Johannes Schindelin · lore

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

Show 5 quoted lines
> > > 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
Nicolas Pitre· Jun 24, 2008, 15:02 UTC · re: Johannes Schindelin · lore

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

On Tue, 24 Jun 2008, Johannes Schindelin wrote:
Show 6 quoted lines
> 
> 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
Jeff King· Jun 24, 2008, 16:38 UTC · re: Johannes Schindelin · lore

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

On Tue, Jun 24, 2008 at 03:17:18PM +0100, Johannes Schindelin wrote:
Show 5 quoted lines
> 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
Johannes Schindelin· Jun 24, 2008, 16:50 UTC · re: Jeff King · lore

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

Hi,
On Tue, 24 Jun 2008, Jeff King wrote:
Show 10 quoted lines
> 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

Johannes Schindelin· Jun 24, 2008, 17:09 UTC · re: Johannes Schindelin · lore

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

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(-)
Show changes to 2 files +34 −7

Documentation/git-bisect.txt, git-bisect.sh

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
Jeff King· Jun 24, 2008, 17:41 UTC · re: Johannes Schindelin · lore

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

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
Daniel Barkalow· Jun 24, 2008, 19:22 UTC · re: Jeff King · lore

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

On Tue, 24 Jun 2008, Jeff King wrote:
Show 14 quoted lines
> 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*
Johannes Schindelin· Jun 24, 2008, 19:26 UTC · re: Daniel Barkalow · lore

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

Hi,
On Tue, 24 Jun 2008, Daniel Barkalow wrote:
Show 20 quoted lines
> 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

Junio C Hamano· Jun 24, 2008, 22:30 UTC · re: Jeff King · lore

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

Jeff King <peff@peff.net> writes:
Show 14 quoted lines
> 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:

Show 5 quoted lines
>  		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!

Johannes Schindelin· Jun 27, 2008, 13:48 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to 2 files +46 −12

Documentation/git-bisect.txt, git-bisect.sh

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
Junio C Hamano· Jun 27, 2008, 23:03 UTC · re: Johannes Schindelin · lore

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

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.

Johannes Schindelin· Jun 28, 2008, 13:48 UTC · re: Junio C Hamano · lore

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

Hi,
On Fri, 27 Jun 2008, Junio C Hamano wrote:
Show 8 quoted lines
> 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

Junio C Hamano· Jun 28, 2008, 17:52 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 11 quoted lines
>> 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.
SZEDER Gábor· Jun 24, 2008, 19:59 UTC · re: Johannes Schindelin · lore

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

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(-)
Show changes to contrib/completion/git-completion.bash +3 −1
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
Michael Haggerty· Jun 24, 2008, 20:06 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin wrote:
Show 5 quoted lines
> 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
Johannes Schindelin· Jun 24, 2008, 20:38 UTC · re: Michael Haggerty · lore

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

Hi,
On Tue, 24 Jun 2008, Michael Haggerty wrote:
Show 10 quoted lines
> 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

Junio C Hamano· Jun 24, 2008, 22:31 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 10 quoted lines
> 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.

Nicolas Pitre· Jun 24, 2008, 22:43 UTC · re: Junio C Hamano · lore

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

On Tue, 24 Jun 2008, Junio C Hamano wrote:
Show 8 quoted lines
> 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.
Show 11 quoted lines
> 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
Christian Couder· Jun 26, 2008, 06:03 UTC · re: Junio C Hamano · lore

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

Le mercredi 25 juin 2008, Junio C Hamano a écrit :
Show 6 quoted lines
> 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.
Show 11 quoted lines
> > 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.

Show 11 quoted lines
> 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.
Show 6 quoted lines
> 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.

Lea Wiemann· Jun 24, 2008, 22:48 UTC · re: Michael Haggerty · lore

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

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
A Large Angry SCM· Jun 24, 2008, 23:53 UTC · re: Lea Wiemann · lore

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

Lea Wiemann wrote:
Show 9 quoted lines
> 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?
Karl Hasselström· Jun 25, 2008, 07:27 UTC · re: A Large Angry SCM · lore

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

On 2008-06-24 19:53:07 -0400, A Large Angry SCM wrote:
Show 8 quoted lines
> 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
Reini Urban· Jun 24, 2008, 16:54 UTC · re: Jeff King · lore

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

Jeff King schrieb:
Show 10 quoted lines
> 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".

← back to recent threads