Re: [PATCH 1/3] t/t3430: avoid undocumented git diff behavior

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

"Chris Torek via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Chris Torek <chris.torek@gmail.com>
>
> According to the documentation, "git diff" takes at most two commit-ish,
> or an A..B style range, or an A...B style symmetric difference range.
> The autosquash-and-exec test relied on "git diff HEAD^!", which works
> fine for ordinary commits as the revision parse produces two commit-ish,
> namely ^HEAD^ and HEAD.
>
> For merge commits, however, this test makes use of an undocumented
> feature:

s/undocumented feature/undefined behaviour/;

The show.sh scripts wants to compute the diff against first parent,
and it uses a range notation HEAD^! which happens to mean
HEAD^..HEAD for a single parent commit, but it forgets that the
commit it may get fed could be a merge.  What the code happens to do
when given "git diff ^HEAD^2 HEAD^..HEAD" is undefined behaviour and
does not even ...

> the resulting revision parse has all the parents as UNINTERESTING
> followed by the HEAD commit.  This looks identical to a symmetric
> diff parse, which lists the merge bases as UNINTERESTING, followed by
> the A (UNINTERESTING) and B revs.  So the diff winds up treating it
> as one, using the first oid (i.e., HEAD^) and the last (i.e., HEAD).
> The documentation, however, says nothing about this usage.

...deserve to be explained in a paragraph like this, I would think.

> Since diff actually just uses HEAD^ and HEAD, call for these directly
> here.  That makes it possible to improve the diff code's handling of
> symmetric difference arguments.

Yes, the resulting code expresses the intent much better.


>
> Signed-off-by: Chris Torek <chris.torek@gmail.com>
> ---
>  t/t3430-rebase-merges.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
> index a1bc3e20016..b454f400ebd 100755
> --- a/t/t3430-rebase-merges.sh
> +++ b/t/t3430-rebase-merges.sh
> @@ -420,7 +420,7 @@ test_expect_success 'with --autosquash and --exec' '
>  	git commit --fixup B B.t &&
>  	write_script show.sh <<-\EOF &&
>  	subject="$(git show -s --format=%s HEAD)"
> -	content="$(git diff HEAD^! | tail -n 1)"
> +	content="$(git diff HEAD^ HEAD | tail -n 1)"
>  	echo "$subject: $content"
>  	EOF
>  	test_tick &&