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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 6, 2018, 18:14 UTC
Message-ID: <xmqqwou8kz9b.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <nycvar.QRO.7.76.6.1807041224460.75@tvgsbejvaqbjf.bet>

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

Show 16 quoted lines

>> > 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.