Re: [PATCH] bisect: test merge base if good rev is not an ancestor of bad rev

From: Christian Couder <chriscool@tuxfamily.org>
Date: Jul 10, 2008, 22:36 UTC
Message-ID: <200807110036.17504.chriscool@tuxfamily.org>
In-Reply-To: <7vd4llpkxq.fsf@gitster.siamese.dyndns.org>

Le jeudi 10 juillet 2008, Junio C Hamano a écrit :

Show 8 quoted lines

> Christian Couder <chriscool@tuxfamily.org> writes:
> > Yeah, in that case...
> >
> >> The whole idea of "bisect" relies on that idea, that any ancestor of a
> >> good commit is good.  Otherwise you'd have to check the commits one by
> >> one, not in a bisecting manner.
>
> Didn't we already discuss this at length?

Yes, the thread is there:

http://thread.gmane.org/gmane.comp.version-control.git/86951

Show 12 quoted lines

> > No, you just need to check that the merge bases between the bad rev on
> > one side and each good rev on the other side are good too. And if that
> > is the case, then you can be sure that bisection will point to a first
> > bad commit.
> >
> > So the choice is between a simple and fast but not fully reliable
> > bisect, or a more complex and slower but fully reliable bisect.
>
> I have not looked at your implementation, but I do think:
>
>  - The current one is not "fully reliable"; the user needs to know what
>    he is doing.  You might call it "prone to user errors".

I agree.

>  - "Test this merge-base before going forward, please" will add typically
>    only one round of check (if you have more merge bases between good and
>    bad, you need to test all of them are good to be sure), so it is not
>    "slower nor more complex".

By "slower" I meant that it would need more rounds of check on average. By "more complex" I meant that more code is needed.

And I think you are right, all the merge bases need to be tested so I will send a patch on top of the patch discussed here.

Another idea to fix the problem, might be to bisect as usual and at the end before saying "X is first bad commit" to check if some of X parents are merge bases between the bad rev and a good rev. If that is the case, then we could ask the user to check that these parents are all good. On average this would probably reduce the number of revs the user must check.

Regards, Christian.