Skip to content

Commit ddafa7e

Browse files
Junio C HamanoLinus Torvalds
Junio C Hamano
authored and
Linus Torvalds
committed
[PATCH] diff-helper: Fix R/C score parsing under -z flag.
The score number that follow R/C status were parsed but the parse pointer was not updated, causing the entire line to become unrecognized. This patch fixes this problem. There was a test missing to catch this breakage, which this commit adds as t4009-diff-rename-4.sh. The diff-raw tests used in related t4005-diff-rename-2.sh (the same test without -z) and t4007-rename-3.sh were stricter than necessarily, despite that the comment for the tests said otherwise. This patch also corrects them. The documentation is updated to say that the status can optionally be followed by a number called "score"; it does not have to stay similarity index forever and there is no reason to limit it only to C and R. Signed-off-by: Junio C Hamano <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent cad88fd commit ddafa7e

File tree

5 files changed

+219
-18
lines changed

5 files changed

+219
-18
lines changed

‎Documentation/diff-format.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ That is, from the left to the right:
3636
(6) sha1 for "src"; 0{40} if creation or unmerged.
3737
(7) a space.
3838
(8) sha1 for "dst"; 0{40} if creation, unmerged or "look at work tree".
39-
(9) status, followed by similarlity index number only for C and R.
39+
(9) status, followed by optional "score" number.
4040
(10) a tab or a NUL when '-z' option is used.
4141
(11) path for "src"
4242
(12) a tab or a NUL when '-z' option is used; only exists for C or R.

‎diff-helper.c

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -80,17 +80,16 @@ int main(int ac, const char **av) {
8080
if (!strchr("MCRNDU", status))
8181
break;
8282
two_paths = score = 0;
83-
if (status == 'R' || status == 'C') {
83+
if (status == 'R' || status == 'C')
8484
two_paths = 1;
85-
sscanf(cp, "%d", &score);
86-
if (line_termination) {
87-
cp = strchr(cp,
88-
inter_name_termination);
89-
if (!cp)
90-
break;
91-
}
92-
}
9385

86+
/* pick up score if exists */
87+
if (sscanf(cp, "%d", &score) != 1)
88+
score = 0;
89+
cp = strchr(cp,
90+
inter_name_termination);
91+
if (!cp)
92+
break;
9493
if (*cp++ != inter_name_termination)
9594
break;
9695

‎t/t4005-diff-rename-2.sh

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,17 @@ test_description='Same rename detection as t4003 but testing diff-raw.
88
'
99
. ./test-lib.sh
1010

11+
_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
12+
_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
13+
sanitize_diff_raw='s/ '"$_x40"' '"$_x40"' \([A-Z]\)[0-9]* / X X \1# /'
1114
compare_diff_raw () {
1215
# When heuristics are improved, the score numbers would change.
1316
# Ignore them while comparing.
14-
sed -e 's/ \([CR]\)[0-9]* /\1#/' <"$1" >.tmp-1
15-
sed -e 's/ \([CR]\)[0-9]* /\1#/' <"$2" >.tmp-2
17+
# Also we do not check SHA1 hash generation in this test, which
18+
# is a job for t0000-basic.sh
19+
20+
sed -e "$sanitize_diff_raw" <"$1" >.tmp-1
21+
sed -e "$sanitize_diff_raw" <"$2" >.tmp-2
1622
diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2
1723
}
1824

@@ -109,11 +115,6 @@ test_expect_success \
109115
'validate output from rename/copy detection (#2)' \
110116
'compare_diff_raw current expected'
111117

112-
test_expect_success \
113-
'prepare work tree once again' \
114-
'cat ../../COPYING >COPYING &&
115-
git-update-cache --add --remove COPYING COPYING.1'
116-
117118
# make sure diff-helper can grok it.
118119
mv expected diff-raw
119120
GIT_DIFF_OPTS=--unified=0 git-diff-helper <diff-raw >current
@@ -151,6 +152,11 @@ test_expect_success \
151152
# anything about rezrov nor COPYING, since the revised again diff-raw
152153
# nows how to say Copy.
153154

155+
test_expect_success \
156+
'prepare work tree once again' \
157+
'cat ../../COPYING >COPYING &&
158+
git-update-cache --add --remove COPYING COPYING.1'
159+
154160
git-diff-cache -C $tree >current
155161
cat >expected <<\EOF
156162
:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234 COPYING COPYING.1

‎t/t4007-rename-3.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ test_description='Rename interaction with pathspec.
1010

1111
_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
1212
_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
13-
sanitize_diff_raw='s/ \('"$_x40"'\) \1 \([CR]\)[0-9]* / \1 \1 \2# /'
13+
sanitize_diff_raw='s/ '"$_x40"' '"$_x40"' \([A-Z]\)[0-9]* / X X \1# /'
1414
compare_diff_raw () {
1515
# When heuristics are improved, the score numbers would change.
1616
# Ignore them while comparing.

‎t/t4009-diff-rename-4.sh

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
#!/bin/sh
2+
#
3+
# Copyright (c) 2005 Junio C Hamano
4+
#
5+
6+
test_description='Same rename detection as t4003 but testing diff-raw -z.
7+
8+
'
9+
. ./test-lib.sh
10+
11+
_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
12+
_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
13+
sanitize_diff_raw='/^:/s/ '"$_x40"' '"$_x40"' \([A-Z]\)[0-9]*$/ X X \1#/'
14+
compare_diff_raw () {
15+
# When heuristics are improved, the score numbers would change.
16+
# Ignore them while comparing.
17+
# Also we do not check SHA1 hash generation in this test, which
18+
# is a job for t0000-basic.sh
19+
20+
tr '\0' '\012' <"$1" | sed -e "$sanitize_diff_raw" >.tmp-1
21+
tr '\0' '\012' <"$2" | sed -e "$sanitize_diff_raw" >.tmp-2
22+
diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2
23+
}
24+
25+
compare_diff_patch () {
26+
# When heuristics are improved, the score numbers would change.
27+
# Ignore them while comparing.
28+
sed -e '/^similarity index [0-9]*%$/d' <"$1" >.tmp-1
29+
sed -e '/^similarity index [0-9]*%$/d' <"$2" >.tmp-2
30+
diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2
31+
}
32+
33+
test_expect_success \
34+
'prepare reference tree' \
35+
'cat ../../COPYING >COPYING &&
36+
echo frotz >rezrov &&
37+
git-update-cache --add COPYING rezrov &&
38+
tree=$(git-write-tree) &&
39+
echo $tree'
40+
41+
test_expect_success \
42+
'prepare work tree' \
43+
'sed -e 's/HOWEVER/However/' <COPYING >COPYING.1 &&
44+
sed -e 's/GPL/G.P.L/g' <COPYING >COPYING.2 &&
45+
rm -f COPYING &&
46+
git-update-cache --add --remove COPYING COPYING.?'
47+
48+
# tree has COPYING and rezrov. work tree has COPYING.1 and COPYING.2,
49+
# both are slightly edited, and unchanged rezrov. We say COPYING.1
50+
# and COPYING.2 are based on COPYING, and do not say anything about
51+
# rezrov.
52+
53+
git-diff-cache -z -M $tree >current
54+
55+
cat >expected <<\EOF
56+
:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234
57+
COPYING
58+
COPYING.1
59+
:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 06c67961bbaed34a127f76d261f4c0bf73eda471 R1234
60+
COPYING
61+
COPYING.2
62+
EOF
63+
64+
test_expect_success \
65+
'validate output from rename/copy detection (#1)' \
66+
'compare_diff_raw current expected'
67+
68+
# make sure diff-helper can grok it.
69+
mv current diff-raw
70+
GIT_DIFF_OPTS=--unified=0 git-diff-helper -z <diff-raw >current
71+
cat >expected <<\EOF
72+
diff --git a/COPYING b/COPYING.1
73+
copy from COPYING
74+
copy to COPYING.1
75+
--- a/COPYING
76+
+++ b/COPYING.1
77+
@@ -6 +6 @@
78+
- HOWEVER, in order to allow a migration to GPLv3 if that seems like
79+
+ However, in order to allow a migration to GPLv3 if that seems like
80+
diff --git a/COPYING b/COPYING.2
81+
rename old COPYING
82+
rename new COPYING.2
83+
--- a/COPYING
84+
+++ b/COPYING.2
85+
@@ -2 +2 @@
86+
- Note that the only valid version of the GPL as far as this project
87+
+ Note that the only valid version of the G.P.L as far as this project
88+
@@ -6 +6 @@
89+
- HOWEVER, in order to allow a migration to GPLv3 if that seems like
90+
+ HOWEVER, in order to allow a migration to G.P.Lv3 if that seems like
91+
@@ -12 +12 @@
92+
- This file is licensed under the GPL v2, or a later version
93+
+ This file is licensed under the G.P.L v2, or a later version
94+
EOF
95+
96+
test_expect_success \
97+
'validate output from diff-helper (#1)' \
98+
'compare_diff_patch current expected'
99+
100+
################################################################
101+
102+
test_expect_success \
103+
'prepare work tree again' \
104+
'mv COPYING.2 COPYING &&
105+
git-update-cache --add --remove COPYING COPYING.1 COPYING.2'
106+
107+
# tree has COPYING and rezrov. work tree has COPYING and COPYING.1,
108+
# both are slightly edited, and unchanged rezrov. We say COPYING.1
109+
# is based on COPYING and COPYING is still there, and do not say anything
110+
# about rezrov.
111+
112+
git-diff-cache -z -C $tree >current
113+
cat >expected <<\EOF
114+
:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 06c67961bbaed34a127f76d261f4c0bf73eda471 M
115+
COPYING
116+
:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234
117+
COPYING
118+
COPYING.1
119+
EOF
120+
121+
test_expect_success \
122+
'validate output from rename/copy detection (#2)' \
123+
'compare_diff_raw current expected'
124+
125+
# make sure diff-helper can grok it.
126+
mv current diff-raw
127+
GIT_DIFF_OPTS=--unified=0 git-diff-helper -z <diff-raw >current
128+
cat >expected <<\EOF
129+
diff --git a/COPYING b/COPYING
130+
--- a/COPYING
131+
+++ b/COPYING
132+
@@ -2 +2 @@
133+
- Note that the only valid version of the GPL as far as this project
134+
+ Note that the only valid version of the G.P.L as far as this project
135+
@@ -6 +6 @@
136+
- HOWEVER, in order to allow a migration to GPLv3 if that seems like
137+
+ HOWEVER, in order to allow a migration to G.P.Lv3 if that seems like
138+
@@ -12 +12 @@
139+
- This file is licensed under the GPL v2, or a later version
140+
+ This file is licensed under the G.P.L v2, or a later version
141+
diff --git a/COPYING b/COPYING.1
142+
copy from COPYING
143+
copy to COPYING.1
144+
--- a/COPYING
145+
+++ b/COPYING.1
146+
@@ -6 +6 @@
147+
- HOWEVER, in order to allow a migration to GPLv3 if that seems like
148+
+ However, in order to allow a migration to GPLv3 if that seems like
149+
EOF
150+
151+
test_expect_success \
152+
'validate output from diff-helper (#2)' \
153+
'compare_diff_patch current expected'
154+
155+
################################################################
156+
157+
# tree has COPYING and rezrov. work tree has the same COPYING and
158+
# copy-edited COPYING.1, and unchanged rezrov. We should not say
159+
# anything about rezrov nor COPYING, since the revised again diff-raw
160+
# nows how to say Copy.
161+
162+
test_expect_success \
163+
'prepare work tree once again' \
164+
'cat ../../COPYING >COPYING &&
165+
git-update-cache --add --remove COPYING COPYING.1'
166+
167+
git-diff-cache -z -C $tree >current
168+
cat >expected <<\EOF
169+
:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234
170+
COPYING
171+
COPYING.1
172+
EOF
173+
174+
test_expect_success \
175+
'validate output from rename/copy detection (#3)' \
176+
'compare_diff_raw current expected'
177+
178+
# make sure diff-helper can grok it.
179+
mv current diff-raw
180+
GIT_DIFF_OPTS=--unified=0 git-diff-helper -z <diff-raw >current
181+
cat >expected <<\EOF
182+
diff --git a/COPYING b/COPYING.1
183+
copy from COPYING
184+
copy to COPYING.1
185+
--- a/COPYING
186+
+++ b/COPYING.1
187+
@@ -6 +6 @@
188+
- HOWEVER, in order to allow a migration to GPLv3 if that seems like
189+
+ However, in order to allow a migration to GPLv3 if that seems like
190+
EOF
191+
192+
test_expect_success \
193+
'validate output from diff-helper (#3)' \
194+
'compare_diff_patch current expected'
195+
196+
test_done

0 commit comments

Comments
 (0)