Re: [PATCH] describe: count the distance correctly when commit dates are skewed

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

Kristofer Karlsson <krka@spotify.com> writes:

> I have a series locally that builds on top of this topic
> (lt/describe-commit-date-skew), and while working on it I
> noticed that the topic makes an expected failure pass in the
> commit-graph test mode, which fails the test script:
>
>   $ cd t && GIT_TEST_COMMIT_GRAPH=1 ./t6120-describe.sh
>   [...]
>   ok 102 - describe --candidates=2 HEAD # TODO known breakage vanished
>   [...]
>   # 1 known breakage(s) vanished; please update test(s)

Yes, that is exactly the one I pointed out passing in CI in a few
message upthread.

> That test now passes when a commit-graph has been written for
> it, which GIT_TEST_COMMIT_GRAPH does, but still fails without
> one, so it cannot just be flipped.  I think Linus's patch is
> good, but it needs something like this squashed in.  My series
> would remove the unset again once the test passes in both
> modes.

Perhaps.

We'd want these tests to pass whether we have commit-graph or not
(in other words, the presence of commit-graph should not affect
correctness but only performance in an ideal world), but we live in
an imperfect world, so as a stop-gap measure I think it is OK to only
test in one single setting, even though I would really prefer to
test both with and without the feature, and have all tests pass.

Short of that, it would be better to test only the "default"
case.  Since mid-2019, core.commitGraph defaults to true, so it may
make sense to use the same trick as your patch below but freeze the
setting to always test this feature under GIT_TEST_COMMIT_GRAPH
instead.  It would require flipping one expect_failure to
expect_success in the case involved, though.

> -- >8 --
> Subject: t6120: keep disjoint history out of the commit-graph
>
> With commits walked in generation number order, the
> "--candidates=2" test for disjoint histories passes when a
> commit-graph exists, which GIT_TEST_COMMIT_GRAPH=1 writes, but
> still fails without one.  Keep that history out of the
> commit-graph so the expected failure holds in both cases.
>
> Signed-off-by: Kristofer Karlsson <krka@spotify.com>
> ---
>  t/t6120-describe.sh | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> index a75f9227d9..969f878946 100755
> --- a/t/t6120-describe.sh
> +++ b/t/t6120-describe.sh
> @@ -704,6 +704,10 @@ test_expect_success 'setup: describe commits with
> disjoint bases' '
>   git init disjoint1 &&
>   (
>   cd disjoint1 &&
> + # describe walks this history differently with a
> + # commit-graph, so always test it without one,
> + # whatever GIT_TEST_COMMIT_GRAPH says
> + sane_unset GIT_TEST_COMMIT_GRAPH &&
>
>   echo o >> file && git add file && git commit -m o &&
>   git tag H -a -m H &&