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

From: Christian Couder <chriscool@tuxfamily.org>
Date: Jun 26, 2008, 06:03 UTC
Message-ID: <200806260803.20731.chriscool@tuxfamily.org>
In-Reply-To: <7v8wwubh3j.fsf@gitster.siamese.dyndns.org>

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.