Re: [RFC PATCH v5] Implement --first-parent for git rev-list --bisect

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> > git rev-list --first-parent --bisect-all F..E >revs &&
>> > test_line_count = 9 revs &&
>> > for rev in E e1 e2 e3 e4 e5 e6 e7 e8
>> > do
>> >   grep "^$(git rev-parse $rev) " revs ||
>> >   {
>> >     echo "$rev not shown" >&2 &&
>> >     return 1
>> >   }
>> > done &&
>> > sed -e "s/.*(dist=\([0-9]*\)).*/\1/" revs >actual.dists &&
>> > sort -r actual.dists >actual.dists.sorted &&
>> > test_cmp actual.dists.sorted actual.dists
>> 
> From my point of view, this indicates that you want to set those exact
> dist values in stone.

As I already said, I do not think it is absolutely necessary to
declare that the minimum dist is 0 or 1, or how big one step of dist
is.  For those reading from the sidelines, the history we are
testing this new feature over looks like this

#     E		dist=0
#    / \
#   e1  |	dist=1
#   |   |
#   e2  |	dist=2
#   |   |       ...
#   |   |       ...
#   e7  |	dist=2
#   |   |
#   e8  |	dist=1
#    \ /
#     F		dist=0

Current code will say dist=0 for E and F, dist=1 for e1 and e8,
etc., and I am fine if the code suddenly start saying that E and F
(i.e. those at the boundary of the graph) have dist=1 and one hop
weighs 10 so dist=11 for e1 and e8 (i.e. those at one hop from the
boundary).

But I am not fine if E and F get larger dist than e1 and e8, or e1
and e8 get different ones.  I do not think the code quoted upfront
would catch such future breakages.

And I also do not see a reason why somebody wants to make the dist
computation to be 1-based (iow, changing the minimum from 0 to 1) or
one step not to be 1 (iow, giving 11 to e1 and e8), so while I agree
it is not strictly necessary to cast the concrete distance value in
stone, I do not see much harm doing so *if* it helps to make it
simpler the test that is necessary to make sure relative dist values
assigned to these commits are in correct order.