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

From
Junio C Hamano <hidden>
Date
2020-06-09 05:18:27
"Chris Torek via GitGitGadget" [off-list ref] writes:
From: Chris Torek <redacted>

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.

quoted hunk
Signed-off-by: Chris Torek <redacted>
---
 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 &&
Thread (1 message) 1 message, 1 author, last reply 2020-06-09
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help