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

From: Philip Oakley <philipoakley@iee.org>
Date: Mar 19, 2015, 23:03 UTC
Message-ID: <3FAFDE160E204496A38A4B6C53FB9B32@PhilipOakley>
In-Reply-To: <xmqqtwxjo4nf.fsf@gitster.dls.corp.google.com>

From: "Junio C Hamano" <gitster@pobox.com>
Sent: Tuesday, March 17, 2015 6:33 PM

Show 34 quoted lines

> Christian Couder <christian.couder@gmail.com> writes:
>
>> On Mon, Mar 16, 2015 at 10:05 PM, Junio C Hamano <gitster@pobox.com> 
>> wrote:
>>
>>> 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.
>
> That is irrelevant, no?  Nobody is questioning that the user is
> supposed to judge if a commit is "good" or "bad" correctly.

[...]

Show 8 quoted lines

>> and/or we could make "git bisect bad" accept any number of bad
>> commitishs.
>
> Yes, that is exactly what I meant.
>
> The way I understand the Philip's point is that the user may have
> a-priori knowledge that a breakage from the same cause appears in
> both tips of these branches.

Just to clarify; my initial query followed on from the way Junio had described it with having two tips which were known bad. I hadn't been aware of how the bisect worked on a DAG, so I wanted to fully understand Junio's comment regarding the expectation of a clean jump to commit 4 (i.e. shouldn't we test commit 4 before assuming it's actually bad). I was quite happy with a bisect of a linear list, but was unsure about how Git dissected DAGs.

I can easily see cases in more complicated product branching where users report intermittent operation for various product variants (especially if modular) and one wants to seek out those commits that introduced the behavious (which is typically some racy condition - otherwise it would be deterministic).

Given Junio's explantion with the two bad commits (on different legs) I'd sort of assumed it could be both user given, or algorithmically determined as part of the bisect.

Show 7 quoted lines

> In such a case, we can start bisection
> after marking the merge-base of two 'bad' commits, e.g. 4 in the
> illustration in the message you are responding to, instead of
> including 5, 6, and 8 in the suspect set.
>
> You need to be careful, though.  An obvious pitfall is what you
> should do when there is a criss-cross merge.

You end up with possibly two (or more) merges being marked as the source of the bad behaviour, especially when racy ;-)

>
> Thanks.
> --

Hope that helps. Philip