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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 11, 2008, 06:51 UTC
Message-ID: <7vabgolxqa.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.DEB.1.00.0807110155040.3279@eeepc-johanness>

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

Show 13 quoted lines

> On Thu, 10 Jul 2008, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
> Of course it can be that the user commits a pilot error and says "but that 
> unrelated version was good", while the fork point(s) between good and bad 
> was bad (and this might be even the intention of the user, to find _one_ 
> commit that introduced the bug).
>
> Speaking of plural, what if some of the merge bases are good, some are 
> bad?
>
> Without carefully thinking it through, you might even _break_ the tool.

And you think it is better to make all of your _users_ think it through every time? Isn't it more error prone?

> All I was proposing is keeping the current semantics, keeping the 
> mechanism simple, and therefore reliable.

What I suggested to Christian (sorry, I've been busy and I still haven't checked if that is what was implemented in the patch -- that is why I suggested you to read the original thread) was:

	- check good and bad to see if they are forked

        - iff they are,

          - have the user check merge bases and make sure they are all
            good.  otherwise, the initial good/bad pair is unsuitable for
            bisection, so explain the situation and quit [*1*];

	  - otherwise, keep these good markers.

	- do the usual bisection --- from this point on it is "simple and
          reliable as it has always been".

And I do not think adding the "pre-check" stage before going into the main part of the processing that we have always done is against "keeping the mechanism simple and reliable".

[Footnote]

*1* We _could_ make things more complex by offering to swap good and bad at this point and then continue bisecting to find a commit to cherry-pick to forward port the fix. Arguably, that step would be a new code and could start out to be buggy --- it _could_ be called destabilizing what has been reliable, but even then, it would be a separate codepath and a new bug will be something that triggers only when the user accepts that offer. I do not see what the big deal is that you seem to be worried about.