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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 11, 2008, 11:21 UTC
Message-ID: <alpine.DEB.1.00.0807111316450.3640@eeepc-johanness>
In-Reply-To: <7vabgolxqa.fsf@gitster.siamese.dyndns.org>

Hi,

On Thu, 10 Jul 2008, Junio C Hamano wrote:

Show 18 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > 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?

Maybe I am alone here, but except for that occasion that triggered my "fixed/unfixed" patch, I did have to think, in order to use git-bisect. I said "git bisect start && git bisect bad HEAD && git bisect good HEAD@{1.day.ago}", and then follow the instructions.

Show 19 quoted lines

> > 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".

Okay, that seems like a trivial and good patch.

Show 8 quoted lines

> *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.

That is what I am actually scared off. That in the wake of a nice and trivial patch, things get muddied and complicated like back when "rebase -i -m" was made unusable for the layman.

Ciao, Dscho