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

On Mon, Mar 16, 2015 at 10:05 PM, Junio C Hamano [off-list ref] wrote:

"Philip Oakley" [off-list ref] writes:

quoted

From: "Junio C Hamano" <redacted>

quoted

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

Yes, we keep only one "bad" pointer.

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.

Yes, but the user is supposed to not change the "bad" pointer for no
good reason. For example maybe a mistake was made and the first commit
marked as "bad" was not actually bad.

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.

Yeah, we could do that, but we would have to allow it only if a
special option is passed on the command line, for example:

git bisect bad --alternate <commitish>

and/or we could make "git bisect bad" accept any number of bad commitishs.

That could give additional bonus points to the GSoC student who would
implement it :-)

Thanks,
Christian.