Re: [PATCH v4] rev-list: refuse --first-parent combined with --bisect

[Junio C Hamano <gitster@pobox.com>]

"Philip Oakley" <philipoakley@iee.org> writes:

> From: "Junio C Hamano" <gitster@pobox.com>
>
>> Hence, if you have a history that looks like this:
>>
>>
>>   G...1---2---3---4---6---8---B
>>                    \
>>                     5---7---B
>>
>> it follows that 4 must also be "bad".  It used to be good long time
>> ago somewhere before 1, and somewhere along way on the history,
>> there was a single breakage event that we are hunting for.  That
>> single event cannot be 5, 6, 7 or 8 because breakage at say 5 would
>> not explain why the tip of the upper branch is broken---its breakage
>> has no way to propagate there.  The breakage must have happened at 4
>> or before that commit.
>
> Is it not worth at least confirming the assertion that 4 is bad before
> proceding, or at least an option to confirm that in complex scenarios
> where the fault may be devious.

That raises a somewhat interesting tangent.

Christian seems to be forever interested in bisect, so I'll add him
to the Cc list ;-)

There is no way to give multiple "bad" from the command line.  You
can say "git bisect start rev rev rev..." but that gives only one
bad and everything else is good.  And once you specify one of the
above two bad ones (say, the child of 8), then we will not even
offer the other one (i.e. the child of 7) as a candidate to be
tested.  So in that sense, "confirm that 4 is bad before proceeding"
is a moot point.

However, you can say "git bisect bad <rev>" (and "git bisect good
<rev>" for that matter) on a rev that is unrelated to what the
current bisection state is.  E.g. after you mark the child of 8 as
"bad", the bisected graph would become

   G...1---2---3---4---6---8---B

and you would be offered to test somewhere in the middle, say, 4.
But it is perfectly OK for you to respond with "git bisect bad 7",
if you know 7 is bad.

I _think_ the current code blindly overwrites the "bad" pointer,
making the bisection state into this graph if you do so.

   G...1---2---3---4
                    \
                     5---B

This is very suboptimal.  The side branch 4-to-7 could be much
longer than the original trunk 4-to-the-tip, in which case we would
have made the suspect space _larger_, not smaller.

We certainly should be able to take advantage of the fact that the
current "bad" commit (i.e. the child of 8) and the newly given "bad"
commit (i.e. 7) are both known to be bad and mark 4 as "bad" instead
when that happens, instead of doing the suboptimal thing the code
currently does.