Re: [RFC PATCH v5] Implement --first-parent for git rev-list --bisect
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jul 4, 2018, 10:26 UTC
- Message-ID
- <nycvar.QRO.7.76.6.1807041224460.75@tvgsbejvaqbjf.bet>
- In-Reply-To
- <xmqq36x0ndza.fsf@gitster-ct.c.googlers.com>
Hi Junio,
On Tue, 3 Jul 2018, Junio C Hamano wrote:
Show 32 quoted lines
> Tiago Botelho <tiagonbotelho@gmail.com> 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
>
> The distance in the current graph might be all lower single-digits,
> but you need "sort -n -r" to be future-proof, as dist=10 would sort
> next to dist=1, perhaps in between dist=1 and dist=2.
>
> Another thing you missed in my message is that the above does not
> say what distance value each commit should be assigned in the
> history. Even though the grep loop makes sure that each of E, e1,
> ... e8 appears at least once, and line-count before that ensures
> that the output has 9 entries (and taken together, it guarantees
> that each of these appears not just "at least once", but also
> exactly once), nothing guarantees if they are getting relative
> distance correctly, as the sed strips a bit too much (and that
> relates to my earlier point why starting from a concrete expected
> output and explicitly discard the aspect of the output we do not
> care about before comparison---that way, we can easily tell when the
> code is _designed to_ discard too much).From my point of view, this indicates that you want to set those exact dist values in stone. I am not sure I follow that reasoning, as this particular test case's purpose is to ensure that the `--first-parent` option works with `--bisect`, not that the distance values match a fixed expectation.
In short: I disagree that the *exact* values (beyond testing the order) should be tested *here*.
Ciao, Dscho