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

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

Tiago Botelho <tiagonbotelho@gmail.com> writes:

> This will enable users to implement bisecting on first parents
> which can be useful for when the commits from a feature branch
> that we want to merge are not always tested.
>
> Signed-off-by: Tiago Botelho <tiagonbotelho@hotmail.com>
> ---
>
> This patch adds all Junio's suggestions, namely do_find_bisection() being
> broken when assigning q's weight to p if in first parent mode and q being
> not UNINTERESTING and its weight still not being known.
>
> The graph displayed in the unit tests was also changed from being top-bottom
> to be left-right in order to keep it consistent with graphs in other tests.
>
>  bisect.c                   | 45 ++++++++++++++++++++++++++++++---------------
>  bisect.h                   |  3 ++-
>  builtin/rev-list.c         |  3 +++
>  revision.c                 |  3 ---
>  t/t6002-rev-list-bisect.sh | 37 +++++++++++++++++++++++++++++++++++++
>  5 files changed, 72 insertions(+), 19 deletions(-)

> diff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh
> index a66140803..774d9a4fd 100755
> --- a/t/t6002-rev-list-bisect.sh
> +++ b/t/t6002-rev-list-bisect.sh
> @@ -263,4 +263,41 @@ test_expect_success 'rev-parse --bisect can default to good/bad refs' '
> ...
> +test_output_expect_success "--bisect-all --first-parent" 'git rev-list --bisect-all --first-parent FX ^A' <<EOF
> +$(git rev-parse EX) (dist=1)
> +$(git rev-parse D) (dist=1)
> +$(git rev-parse FX) (dist=0)
> +EOF
> +
>  test_done

Running this test number of times gives me spurious errors.  Is the
order of these output lines unstable?  How do we "sort" these
bisect-all results?  If we are not sorting and the output depends on
happenstance, then probably we would need to compare the expected
and actual output after sorting.  Or if the output depends on
something under our control (e.g. they are related to topology and
relative commit timestamp), we probably should try to control that
"something" tighter so that we can rely on the order of the lines in
the "expect" file.

It also appears that we have "--bisect and --first-parent do not
work well together" in t6000, which also needs to be updated.  I
needed the following squashed into this patch to make "make test"
pass.

diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
index 969e4e9e52..981198ae6e 100755
--- a/t/t6000-rev-list-misc.sh
+++ b/t/t6000-rev-list-misc.sh
@@ -96,8 +96,8 @@ test_expect_success 'rev-list can show index objects' '
 	test_cmp expect actual
 '
 
-test_expect_success '--bisect and --first-parent can not be combined' '
-	test_must_fail git rev-list --bisect --first-parent HEAD
+test_expect_success '--bisect and --first-parent can now be combined' '
+	git rev-list --bisect --first-parent HEAD
 '
 
 test_expect_success '--header shows a NUL after each commit' '