# [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' 26 messages from 2008-06-24 to 2008-06-28. Participants: Johannes Schindelin, Stephan Beyer, Nicolas Pitre, Jeff King, Reini Urban, Daniel Barkalow, SZEDER Gábor, Michael Haggerty, Junio C Hamano, Lea Wiemann, A Large Angry SCM, Karl Hasselström, Christian Couder. Thread: https://gitlist.dev/t/14123 ## Johannes Schindelin, 2008-06-24 14:17 Subject: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0806241515460.9925%40racer ``` 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 --- 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 ``` ## Stephan Beyer, 2008-06-24 14:42 Subject: Re: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <20080624144254.GG5528@leksak.fem-net> URL: https://gitlist.dev/e/20080624144254.GG5528%40leksak.fem-net In-Reply-To: ``` Hi, > So introduce the commands 'git bisect fixed' and 'git bisect unfixed'. Are they intentionally undocumented to not raise confusion? Regards, Stephan -- Stephan Beyer , PGP 0x6EDDD207FCC5040F ``` ## Johannes Schindelin, 2008-06-24 14:55 Subject: Re: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0806241555300.9925%40racer In-Reply-To: <20080624144254.GG5528@leksak.fem-net> ``` 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 ``` ## Nicolas Pitre, 2008-06-24 15:02 Subject: Re: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.10.0806241101160.2979%40xanadu.home In-Reply-To: ``` 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 ``` ## Stephan Beyer, 2008-06-24 15:16 Subject: Re: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <20080624151630.GH5528@leksak.fem-net> URL: https://gitlist.dev/e/20080624151630.GH5528%40leksak.fem-net In-Reply-To: ``` > > > 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 , PGP 0x6EDDD207FCC5040F ``` ## Jeff King, 2008-06-24 16:38 Subject: Re: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <20080624163810.GA4654@sigill.intra.peff.net> URL: https://gitlist.dev/e/20080624163810.GA4654%40sigill.intra.peff.net In-Reply-To: ``` 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 ``` ## Johannes Schindelin, 2008-06-24 16:50 Subject: Re: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0806241750030.9925%40racer In-Reply-To: <20080624163810.GA4654@sigill.intra.peff.net> ``` 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 ``` ## Reini Urban, 2008-06-24 16:54 Subject: Re: [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <486126B5.1090909@x-ray.at> URL: https://gitlist.dev/e/486126B5.1090909%40x-ray.at In-Reply-To: <20080624163810.GA4654@sigill.intra.peff.net> ``` 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". ``` ## Johannes Schindelin, 2008-06-24 17:09 Subject: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0806241808400.9925%40racer In-Reply-To: ``` 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 --- 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 ... + git bisect fixed [] + git bisect unfixed [...] + 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 [ [...]] [--] [...] @@ -24,6 +24,13 @@ git bisect log git bisect run ... use ... to automatically bisect. +When not looking for a regression, but a fix instead, you can use + +git bisect fixed [] + mark as having the fix you are looking for +git bisect unfixed [] + mark 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, 2008-06-24 17:41 Subject: Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <20080624174157.GB9500@sigill.intra.peff.net> URL: https://gitlist.dev/e/20080624174157.GB9500%40sigill.intra.peff.net In-Reply-To: ``` 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, 2008-06-24 19:22 Subject: Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: URL: https://gitlist.dev/e/alpine.LNX.1.00.0806241516440.19665%40iabervon.org In-Reply-To: <20080624174157.GB9500@sigill.intra.peff.net> ``` 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* ``` ## Johannes Schindelin, 2008-06-24 19:26 Subject: Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0806242026441.9925%40racer In-Reply-To: ``` 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 ``` ## SZEDER Gábor, 2008-06-24 19:59 Subject: Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <20080624195907.GI8421@neumann> URL: https://gitlist.dev/e/20080624195907.GI8421%40neumann In-Reply-To: ``` 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 ``` ## Michael Haggerty, 2008-06-24 20:06 Subject: Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <486153DB.3070502@alum.mit.edu> URL: https://gitlist.dev/e/486153DB.3070502%40alum.mit.edu In-Reply-To: ``` 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 ``` ## Johannes Schindelin, 2008-06-24 20:38 Subject: Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0806242137120.9925%40racer In-Reply-To: <486153DB.3070502@alum.mit.edu> ``` 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 ``` ## Junio C Hamano, 2008-06-24 22:30 Subject: Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <7vej6mbh3w.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vej6mbh3w.fsf%40gitster.siamese.dyndns.org In-Reply-To: <20080624174157.GB9500@sigill.intra.peff.net> ``` Jeff King 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! ``` ## Junio C Hamano, 2008-06-24 22:31 Subject: Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <7v8wwubh3j.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v8wwubh3j.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Johannes Schindelin 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. ``` ## Nicolas Pitre, 2008-06-24 22:43 Subject: Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.10.0806241841150.2979%40xanadu.home In-Reply-To: <7v8wwubh3j.fsf@gitster.siamese.dyndns.org> ``` 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 ``` ## Lea Wiemann, 2008-06-24 22:48 Subject: Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <486179C8.2000704@gmail.com> URL: https://gitlist.dev/e/486179C8.2000704%40gmail.com In-Reply-To: <486153DB.3070502@alum.mit.edu> ``` 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, 2008-06-24 23:53 Subject: Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <486188E3.10803@gmail.com> URL: https://gitlist.dev/e/486188E3.10803%40gmail.com In-Reply-To: <486179C8.2000704@gmail.com> ``` 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? ``` ## Karl Hasselström, 2008-06-25 07:27 Subject: Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <20080625072733.GA26605@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20080625072733.GA26605%40diana.vm.bytemark.co.uk In-Reply-To: <486188E3.10803@gmail.com> ``` 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 ``` ## Christian Couder, 2008-06-26 06:03 Subject: Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <200806260803.20731.chriscool@tuxfamily.org> URL: https://gitlist.dev/e/200806260803.20731.chriscool%40tuxfamily.org In-Reply-To: <7v8wwubh3j.fsf@gitster.siamese.dyndns.org> ``` Le mercredi 25 juin 2008, Junio C Hamano a écrit : > Johannes Schindelin 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. ``` ## Johannes Schindelin, 2008-06-27 13:48 Subject: [PATCH, next version] git bisect: introduce 'fixed' and 'unfixed' Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0806271446180.9925%40racer In-Reply-To: <7vej6mbh3w.fsf@gitster.siamese.dyndns.org> ``` 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 --- On Tue, 24 Jun 2008, Junio C Hamano wrote: > Jeff King 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 ... + git bisect fixed [] + git bisect unfixed [...] + 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 [ [...]] [--] [...] @@ -24,6 +24,13 @@ git bisect log git bisect run ... use ... to automatically bisect. +When not looking for a regression, but a fix instead, you can use + +git bisect fixed [] + mark as having the fix you are looking for +git bisect unfixed [] + mark 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, 2008-06-27 23:03 Subject: Re: [PATCH, next version] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <7vprq2o4zb.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vprq2o4zb.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Johannes Schindelin 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, 2008-06-28 13:48 Subject: Re: [PATCH, next version] git bisect: introduce 'fixed' and 'unfixed' Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0806281446260.9925%40racer In-Reply-To: <7vprq2o4zb.fsf@gitster.siamese.dyndns.org> ``` Hi, On Fri, 27 Jun 2008, Junio C Hamano wrote: > Johannes Schindelin 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, 2008-06-28 17:52 Subject: Re: [PATCH, next version] git bisect: introduce 'fixed' and 'unfixed' Message-ID: <7v4p7do3ak.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v4p7do3ak.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Johannes Schindelin writes: >> Johannes Schindelin 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. ```