diff.c: output correct index lines for a split diff
A patch that changes the filetype (e.g. regular file to symlink) of a path must be split into a deletion event followed by a creation event, which means that we need to have two independent metainfo lines for each. However, the code reused the single set of metainfo lines. As the blob object names recorded on the index lines are usually not used nor validated on the receiving end, this is not an issue with normal use of the resulting patch. However, when accepting a binary patch to delete a blob, git-apply verified that the postimage blob object name on the index line is 0{40}, hence a patch that deletes a regular file blob that records binary contents to create a blob with different filetype (e.g. a symbolic link) failed to apply. "git am -3" also uses the blob object names recorded on the index line, so it would also misbehave when synthesizing a preimage tree. This moves the code to generate metainfo lines around, so that two independent sets of metainfo lines are used for the split halves. Additional tests by Jeff King. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano committed
Jan 26, 2009 at 00:08 UTC
b67b9612e1a90ae093445abeaeff930e9f4cf936
2 files changed
+98
-66
diff.c
+80
-66
@@ -2096,16 +2096,86 @@ static const char *external_diff_attr(const char *name)
2096
return NULL;
2097
}
2098
2099
+static int similarity_index(struct diff_filepair *p)
2100
+{
2101
+ return p->score * 100 / MAX_SCORE;
2102
+}
2103
+
2104
+static void fill_metainfo(struct strbuf *msg,
2105
+ const char *name,
2106
+ const char *other,
2107
+ struct diff_filespec *one,
2108
+ struct diff_filespec *two,
2109
+ struct diff_options *o,
2110
+ struct diff_filepair *p)
2111
+{
2112
+ strbuf_init(msg, PATH_MAX * 2 + 300);
2113
+ switch (p->status) {
2114
+ case DIFF_STATUS_COPIED:
2115
+ strbuf_addf(msg, "similarity index %d%%", similarity_index(p));
2116
+ strbuf_addstr(msg, "\ncopy from ");
2117
+ quote_c_style(name, msg, NULL, 0);
2118
+ strbuf_addstr(msg, "\ncopy to ");
2119
+ quote_c_style(other, msg, NULL, 0);
2120
+ strbuf_addch(msg, '\n');
2121
+ break;
2122
+ case DIFF_STATUS_RENAMED:
2123
+ strbuf_addf(msg, "similarity index %d%%", similarity_index(p));
2124
+ strbuf_addstr(msg, "\nrename from ");
2125
+ quote_c_style(name, msg, NULL, 0);
2126
+ strbuf_addstr(msg, "\nrename to ");
2127
+ quote_c_style(other, msg, NULL, 0);
2128
+ strbuf_addch(msg, '\n');
2129
+ break;
2130
+ case DIFF_STATUS_MODIFIED:
2131
+ if (p->score) {
2132
+ strbuf_addf(msg, "dissimilarity index %d%%\n",
2133
+ similarity_index(p));
2134
+ break;
2135
+ }
2136
+ /* fallthru */
2137
+ default:
2138
+ /* nothing */
2139
+ ;
2140
+ }
2141
+ if (one && two && hashcmp(one->sha1, two->sha1)) {
2142
+ int abbrev = DIFF_OPT_TST(o, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
2143
+
2144
+ if (DIFF_OPT_TST(o, BINARY)) {
2145
+ mmfile_t mf;
2146
+ if ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) ||
2147
+ (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))
2148
+ abbrev = 40;
2149
+ }
2150
+ strbuf_addf(msg, "index %.*s..%.*s",
2151
+ abbrev, sha1_to_hex(one->sha1),
2152
+ abbrev, sha1_to_hex(two->sha1));
2153
+ if (one->mode == two->mode)
2154
+ strbuf_addf(msg, " %06o", one->mode);
2155
+ strbuf_addch(msg, '\n');
2156
+ }
2157
+ if (msg->len)
2158
+ strbuf_setlen(msg, msg->len - 1);
2159
+}
2160
+
2161
static void run_diff_cmd(const char *pgm,
2162
const char *name,
2163
const char *other,
2164
const char *attr_path,
2165
struct diff_filespec *one,
2166
struct diff_filespec *two,
2105
- const char *xfrm_msg,
2167
+ struct strbuf *msg,
2168
struct diff_options *o,
2107
- int complete_rewrite)
2169
+ struct diff_filepair *p)
2170
{
2171
+ const char *xfrm_msg = NULL;
2172
+ int complete_rewrite = (p->status == DIFF_STATUS_MODIFIED) && p->score;
2173
+
2174
+ if (msg) {
2175
+ fill_metainfo(msg, name, other, one, two, o, p);
2176
+ xfrm_msg = msg->len ? msg->buf : NULL;
2177
+ }
2178
+
2179
if (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))
2180
pgm = NULL;
2181
else {
@@ -2145,11 +2215,6 @@ static void diff_fill_sha1_info(struct diff_filespec *one)
2215
hashclr(one->sha1);
2216
}
2217
2148
-static int similarity_index(struct diff_filepair *p)
2149
-{
2150
- return p->score * 100 / MAX_SCORE;
2151
-}
2152
-
2218
static void strip_prefix(int prefix_length, const char **namep, const char **otherp)
2219
{
2220
/* Strip the prefix but do not molest /dev/null and absolute paths */
@@ -2163,13 +2228,11 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
2228
{
2229
const char *pgm = external_diff();
2230
struct strbuf msg;
2166
- char *xfrm_msg;
2231
struct diff_filespec *one = p->one;
2232
struct diff_filespec *two = p->two;
2233
const char *name;
2234
const char *other;
2235
const char *attr_path;
2172
- int complete_rewrite = 0;
2236
2237
name = p->one->path;
2238
other = (strcmp(name, p->two->path) ? p->two->path : NULL);
@@ -2179,83 +2242,34 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
2242
2243
if (DIFF_PAIR_UNMERGED(p)) {
2244
run_diff_cmd(pgm, name, NULL, attr_path,
2182
- NULL, NULL, NULL, o, 0);
2245
+ NULL, NULL, NULL, o, p);
2246
return;
2247
}
2248
2249
diff_fill_sha1_info(one);
2250
diff_fill_sha1_info(two);
2251
2189
- strbuf_init(&msg, PATH_MAX * 2 + 300);
2190
- switch (p->status) {
2191
- case DIFF_STATUS_COPIED:
2192
- strbuf_addf(&msg, "similarity index %d%%", similarity_index(p));
2193
- strbuf_addstr(&msg, "\ncopy from ");
2194
- quote_c_style(name, &msg, NULL, 0);
2195
- strbuf_addstr(&msg, "\ncopy to ");
2196
- quote_c_style(other, &msg, NULL, 0);
2197
- strbuf_addch(&msg, '\n');
2198
- break;
2199
- case DIFF_STATUS_RENAMED:
2200
- strbuf_addf(&msg, "similarity index %d%%", similarity_index(p));
2201
- strbuf_addstr(&msg, "\nrename from ");
2202
- quote_c_style(name, &msg, NULL, 0);
2203
- strbuf_addstr(&msg, "\nrename to ");
2204
- quote_c_style(other, &msg, NULL, 0);
2205
- strbuf_addch(&msg, '\n');
2206
- break;
2207
- case DIFF_STATUS_MODIFIED:
2208
- if (p->score) {
2209
- strbuf_addf(&msg, "dissimilarity index %d%%\n",
2210
- similarity_index(p));
2211
- complete_rewrite = 1;
2212
- break;
2213
- }
2214
- /* fallthru */
2215
- default:
2216
- /* nothing */
2217
- ;
2218
- }
2219
-
2220
- if (hashcmp(one->sha1, two->sha1)) {
2221
- int abbrev = DIFF_OPT_TST(o, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
2222
-
2223
- if (DIFF_OPT_TST(o, BINARY)) {
2224
- mmfile_t mf;
2225
- if ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) ||
2226
- (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))
2227
- abbrev = 40;
2228
- }
2229
- strbuf_addf(&msg, "index %.*s..%.*s",
2230
- abbrev, sha1_to_hex(one->sha1),
2231
- abbrev, sha1_to_hex(two->sha1));
2232
- if (one->mode == two->mode)
2233
- strbuf_addf(&msg, " %06o", one->mode);
2234
- strbuf_addch(&msg, '\n');
2235
- }
2236
-
2237
- if (msg.len)
2238
- strbuf_setlen(&msg, msg.len - 1);
2239
- xfrm_msg = msg.len ? msg.buf : NULL;
2240
-
2252
if (!pgm &&
2253
DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two) &&
2254
(S_IFMT & one->mode) != (S_IFMT & two->mode)) {
2244
- /* a filepair that changes between file and symlink
2255
+ /*
2256
+ * a filepair that changes between file and symlink
2257
* needs to be split into deletion and creation.
2258
*/
2259
struct diff_filespec *null = alloc_filespec(two->path);
2260
run_diff_cmd(NULL, name, other, attr_path,
2249
- one, null, xfrm_msg, o, 0);
2261
+ one, null, &msg, o, p);
2262
free(null);
2263
+ strbuf_release(&msg);
2264
+
2265
null = alloc_filespec(one->path);
2266
run_diff_cmd(NULL, name, other, attr_path,
2253
- null, two, xfrm_msg, o, 0);
2267
+ null, two, &msg, o, p);
2268
free(null);
2269
}
2270
else
2271
run_diff_cmd(pgm, name, other, attr_path,
2258
- one, two, xfrm_msg, o, complete_rewrite);
2272
+ one, two, &msg, o, p);
2273
2274
strbuf_release(&msg);
2275
}
t/t4114-apply-typechange.sh
+18
@@ -25,6 +25,10 @@ test_expect_success 'setup repository and commits' '
25
git update-index foo &&
26
git commit -m "foo back to file" &&
27
git branch foo-back-to-file &&
28
+ printf "\0" > foo &&
29
+ git update-index foo &&
30
+ git commit -m "foo becomes binary" &&
31
+ git branch foo-becomes-binary &&
32
rm -f foo &&
33
git update-index --remove foo &&
34
mkdir foo &&
@@ -85,6 +89,20 @@ test_expect_success 'symlink becomes file' '
89
'
90
test_debug 'cat patch'
91
92
+test_expect_success 'binary file becomes symlink' '
93
+ git checkout -f foo-becomes-binary &&
94
+ git diff-tree -p --binary HEAD foo-symlinked-to-bar > patch &&
95
+ git apply --index < patch
96
+ '
97
+test_debug 'cat patch'
98
+
99
+test_expect_success 'symlink becomes binary file' '
100
+ git checkout -f foo-symlinked-to-bar &&
101
+ git diff-tree -p --binary HEAD foo-becomes-binary > patch &&
102
+ git apply --index < patch
103
+ '
104
+test_debug 'cat patch'
105
+
106
107
test_expect_success 'symlink becomes directory' '
108
git checkout -f foo-symlinked-to-bar &&