Skip to content

Commit 5c72261

Browse files
newrengitster
authored andcommitted
t4058: add more tests and documentation for duplicate tree entry handling
Commit 4d6be03 ("diffcore-rename: avoid processing duplicate destinations", 2015-02-26) added t4058 to demonstrate that a workaround it added to avoid double frees (namely to just turn off rename detection when trees had duplicate entries) would indeed avoid segfaults. The tests, though, give the impression that the expected diffs are "correct" when in reality they are just "don't segfault, and do something semi-reasonable under the circumstances". Add some notes to make this clearer. Also, commit 25d5ea4 ("[PATCH] Redo rename/copy detection logic.", 2005-05-24) added a similar workaround to avoid segfaults, but for rename_src rather than rename_dst. I do not see any tests in the testsuite to cover the collision detection of entries limited to the source side, so add a couple. Signed-off-by: Elijah Newren <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 81c4bf0 commit 5c72261

File tree

1 file changed

+45
-2
lines changed

1 file changed

+45
-2
lines changed

t/t4058-diff-duplicates.sh

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
#!/bin/sh
22

3+
# NOTICE:
4+
# This testsuite does a number of diffs and checks that the output match.
5+
# However, it is a "garbage in, garbage out" situation; the trees have
6+
# duplicate entries for individual paths, and it results in diffs that do
7+
# not make much sense. As such, it is not clear that the diffs are
8+
# "correct". The primary purpose of these tests was to verify that
9+
# diff-tree does not segfault, but there is perhaps some value in ensuring
10+
# that the diff output isn't wildly unreasonable.
11+
312
test_description='test tree diff when trees have duplicate entries'
413
. ./test-lib.sh
514

@@ -57,7 +66,16 @@ test_expect_success 'create trees with duplicate entries' '
5766
git tag two $outer_two
5867
'
5968

60-
test_expect_success 'diff-tree between trees' '
69+
test_expect_success 'create tree without duplicate entries' '
70+
blob_one=$(echo one | git hash-object -w --stdin) &&
71+
outer_three=$(make_tree \
72+
100644 renamed $blob_one
73+
) &&
74+
git tag three $outer_three
75+
'
76+
77+
test_expect_success 'diff-tree between duplicate trees' '
78+
# See NOTICE at top of file
6179
{
6280
printf ":000000 100644 $ZERO_OID $blob_two A\touter/inner\n" &&
6381
printf ":000000 100644 $ZERO_OID $blob_two A\touter/inner\n" &&
@@ -71,9 +89,34 @@ test_expect_success 'diff-tree between trees' '
7189
'
7290

7391
test_expect_success 'diff-tree with renames' '
74-
# same expectation as above, since we disable rename detection
92+
# See NOTICE at top of file.
7593
git diff-tree -M -r --no-abbrev one two >actual &&
7694
test_cmp expect actual
7795
'
7896

97+
test_expect_success 'diff-tree FROM duplicate tree' '
98+
# See NOTICE at top of file.
99+
{
100+
printf ":100644 000000 $blob_one $ZERO_OID D\touter/inner\n" &&
101+
printf ":100644 000000 $blob_two $ZERO_OID D\touter/inner\n" &&
102+
printf ":100644 000000 $blob_two $ZERO_OID D\touter/inner\n" &&
103+
printf ":100644 000000 $blob_two $ZERO_OID D\touter/inner\n" &&
104+
printf ":000000 100644 $ZERO_OID $blob_one A\trenamed\n"
105+
} >expect &&
106+
git diff-tree -r --no-abbrev one three >actual &&
107+
test_cmp expect actual
108+
'
109+
110+
test_expect_success 'diff-tree FROM duplicate tree, with renames' '
111+
# See NOTICE at top of file.
112+
{
113+
printf ":100644 000000 $blob_two $ZERO_OID D\touter/inner\n" &&
114+
printf ":100644 000000 $blob_two $ZERO_OID D\touter/inner\n" &&
115+
printf ":100644 000000 $blob_two $ZERO_OID D\touter/inner\n" &&
116+
printf ":100644 100644 $blob_one $blob_one R100\touter/inner\trenamed\n"
117+
} >expect &&
118+
git diff-tree -M -r --no-abbrev one three >actual &&
119+
test_cmp expect actual
120+
'
121+
79122
test_done

0 commit comments

Comments
 (0)