Skip to content

Commit b67b961

Browse files
gitstergitster
committed
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>
1 parent b938f62 commit b67b961

File tree

2 files changed

+98
-66
lines changed

2 files changed

+98
-66
lines changed

‎diff.c‎

Lines changed: 80 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -2096,16 +2096,86 @@ static const char *external_diff_attr(const char *name)
20962096
return NULL;
20972097
}
20982098

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+
20992161
static void run_diff_cmd(const char *pgm,
21002162
const char *name,
21012163
const char *other,
21022164
const char *attr_path,
21032165
struct diff_filespec *one,
21042166
struct diff_filespec *two,
2105-
const char *xfrm_msg,
2167+
struct strbuf *msg,
21062168
struct diff_options *o,
2107-
int complete_rewrite)
2169+
struct diff_filepair *p)
21082170
{
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+
21092179
if (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))
21102180
pgm = NULL;
21112181
else {
@@ -2145,11 +2215,6 @@ static void diff_fill_sha1_info(struct diff_filespec *one)
21452215
hashclr(one->sha1);
21462216
}
21472217

2148-
static int similarity_index(struct diff_filepair *p)
2149-
{
2150-
return p->score * 100 / MAX_SCORE;
2151-
}
2152-
21532218
static void strip_prefix(int prefix_length, const char **namep, const char **otherp)
21542219
{
21552220
/* 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)
21632228
{
21642229
const char *pgm = external_diff();
21652230
struct strbuf msg;
2166-
char *xfrm_msg;
21672231
struct diff_filespec *one = p->one;
21682232
struct diff_filespec *two = p->two;
21692233
const char *name;
21702234
const char *other;
21712235
const char *attr_path;
2172-
int complete_rewrite = 0;
21732236

21742237
name = p->one->path;
21752238
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)
21792242

21802243
if (DIFF_PAIR_UNMERGED(p)) {
21812244
run_diff_cmd(pgm, name, NULL, attr_path,
2182-
NULL, NULL, NULL, o, 0);
2245+
NULL, NULL, NULL, o, p);
21832246
return;
21842247
}
21852248

21862249
diff_fill_sha1_info(one);
21872250
diff_fill_sha1_info(two);
21882251

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-
22412252
if (!pgm &&
22422253
DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two) &&
22432254
(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
22452257
* needs to be split into deletion and creation.
22462258
*/
22472259
struct diff_filespec *null = alloc_filespec(two->path);
22482260
run_diff_cmd(NULL, name, other, attr_path,
2249-
one, null, xfrm_msg, o, 0);
2261+
one, null, &msg, o, p);
22502262
free(null);
2263+
strbuf_release(&msg);
2264+
22512265
null = alloc_filespec(one->path);
22522266
run_diff_cmd(NULL, name, other, attr_path,
2253-
null, two, xfrm_msg, o, 0);
2267+
null, two, &msg, o, p);
22542268
free(null);
22552269
}
22562270
else
22572271
run_diff_cmd(pgm, name, other, attr_path,
2258-
one, two, xfrm_msg, o, complete_rewrite);
2272+
one, two, &msg, o, p);
22592273

22602274
strbuf_release(&msg);
22612275
}

‎t/t4114-apply-typechange.sh‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ test_expect_success 'setup repository and commits' '
2525
git update-index foo &&
2626
git commit -m "foo back to file" &&
2727
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 &&
2832
rm -f foo &&
2933
git update-index --remove foo &&
3034
mkdir foo &&
@@ -85,6 +89,20 @@ test_expect_success 'symlink becomes file' '
8589
'
8690
test_debug 'cat patch'
8791

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+
88106

89107
test_expect_success 'symlink becomes directory' '
90108
git checkout -f foo-symlinked-to-bar &&

0 commit comments

Comments
 (0)