Re: Bisect marking new commits incorrectly

[Adam Dinwoodie <adam@dinwoodie.org>]

On Wednesday 22 November 2017 at 05:21 pm +0100, Christian Couder wrote:
> On Wed, Nov 22, 2017 at 3:39 PM, Adam Dinwoodie <adam@dinwoodie.org> wrote:
> > In trying to do a bisect on the Git repository, I seem to have come
> > across surprising behavior where the order in which `git bisect` appears
> > to forget that previous commits were marked as new.
> 
> Yeah, the algorithm uses many "old" commits and only one "new" commit.
> 
> [...]
> 
> > As you can see, in both cases, only the most recent "new" command
> > appears to have any effect.  I'd have expected that both commits would
> > have been marked as "new", and the bisect run would use both facts to
> > work out which commit is the target of the bisection.
> 
> I didn't look at your test case, but the algorithm to find another
> commit to test is described here:
> 
> https://git-scm.com/docs/git-bisect-lk2009.html#_bisection_algorithm
> 
> and you can see that the first rule of the algorithm is to keep only
> the commits that are ancestors of the "new" commit (including the
> "new" commit itself).
> 
> The reason for this rule is that other commits cannot have introduced
> the behavior we are looking for. The result of this rule is that git
> bisect will always ask you to test a commit that is an ancestor of the
> "new" commit.
> 
> So if you test a commit that git bisect asks you to test, and it
> appears that this commit is "new", then you can just discard the
> previous "new" commit because it will give you less information than
> the new "new" one.
> (The old "new" will not let you discard any commits that the new "new"
> commit allows you to discard, because it is a descendant of the new
> "new" commit.)
> 
> If you don't test the commit that git bisect asks you to test, then
> git bisect assumes that you know better and discards the old "new".

Ah, that makes sense, thank you for the explanation.

In my case, I knew two commits were "new", but didn't know which would
provide more information to the bisect algorithm; I'd assumed if I
provided both, Git would work out the appropriate thing to do with the
combined information without me needing to work it out.

> > This is using v2.15.0.  It's possibly relevant that 95a731ce is a
> > direct parent of 14c63a9d.
> >
> > Is this a bug, or intentional behaviour?  Am I missing something that
> > means it's actually sensible to have Git silently discard some bisect
> > commands in this sort of circumstance?
> 
> Well, instead of silently discarding a the old "new" commit when the
> new "new" commit is not an ancestor of it, git bisect could perhaps
> warn or ask you to confirm that you know what you are doing.

Warning the user seems a sensible suggestion to me.  I'll add it to my
list of things to spend some time on at some point in the future :)