Skip to content

Commit d1075ad

Browse files
gitstergitster
committed
Merge branch 'en/merge-path-collision'
Handling of conflicting renames in merge-recursive have further been made consistent with how existing codepaths try to mimic what is done to add/add conflicts. * en/merge-path-collision: merge-recursive: apply collision handling unification to recursive case
2 parents a4fd114 + 8020504 commit d1075ad

File tree

2 files changed

+77
-114
lines changed

2 files changed

+77
-114
lines changed

‎merge-recursive.c

Lines changed: 47 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -1560,35 +1560,6 @@ static int handle_file_collision(struct merge_options *opt,
15601560
b, a);
15611561
}
15621562

1563-
/*
1564-
* In the recursive case, we just opt to undo renames
1565-
*/
1566-
if (opt->priv->call_depth && (prev_path1 || prev_path2)) {
1567-
/* Put first file (a->oid, a->mode) in its original spot */
1568-
if (prev_path1) {
1569-
if (update_file(opt, 1, a, prev_path1))
1570-
return -1;
1571-
} else {
1572-
if (update_file(opt, 1, a, collide_path))
1573-
return -1;
1574-
}
1575-
1576-
/* Put second file (b->oid, b->mode) in its original spot */
1577-
if (prev_path2) {
1578-
if (update_file(opt, 1, b, prev_path2))
1579-
return -1;
1580-
} else {
1581-
if (update_file(opt, 1, b, collide_path))
1582-
return -1;
1583-
}
1584-
1585-
/* Don't leave something at collision path if unrenaming both */
1586-
if (prev_path1 && prev_path2)
1587-
remove_file(opt, 1, collide_path, 0);
1588-
1589-
return 0;
1590-
}
1591-
15921563
/* Remove rename sources if rename/add or rename/rename(2to1) */
15931564
if (prev_path1)
15941565
remove_file(opt, 1, prev_path1,
@@ -1749,85 +1720,56 @@ static int handle_rename_rename_1to2(struct merge_options *opt,
17491720
return -1;
17501721
free(path_desc);
17511722

1752-
if (opt->priv->call_depth) {
1753-
/*
1754-
* FIXME: For rename/add-source conflicts (if we could detect
1755-
* such), this is wrong. We should instead find a unique
1756-
* pathname and then either rename the add-source file to that
1757-
* unique path, or use that unique path instead of src here.
1758-
*/
1759-
if (update_file(opt, 0, &mfi.blob, o->path))
1760-
return -1;
1723+
if (opt->priv->call_depth)
1724+
remove_file_from_index(opt->repo->index, o->path);
17611725

1762-
/*
1763-
* Above, we put the merged content at the merge-base's
1764-
* path. Now we usually need to delete both a->path and
1765-
* b->path. However, the rename on each side of the merge
1766-
* could also be involved in a rename/add conflict. In
1767-
* such cases, we should keep the added file around,
1768-
* resolving the conflict at that path in its favor.
1769-
*/
1770-
add = &ci->ren1->dst_entry->stages[flip_stage(2)];
1771-
if (is_valid(add)) {
1772-
if (update_file(opt, 0, add, a->path))
1773-
return -1;
1774-
}
1775-
else
1776-
remove_file_from_index(opt->repo->index, a->path);
1777-
add = &ci->ren2->dst_entry->stages[flip_stage(3)];
1778-
if (is_valid(add)) {
1779-
if (update_file(opt, 0, add, b->path))
1780-
return -1;
1781-
}
1782-
else
1783-
remove_file_from_index(opt->repo->index, b->path);
1726+
/*
1727+
* For each destination path, we need to see if there is a
1728+
* rename/add collision. If not, we can write the file out
1729+
* to the specified location.
1730+
*/
1731+
add = &ci->ren1->dst_entry->stages[flip_stage(2)];
1732+
if (is_valid(add)) {
1733+
add->path = mfi.blob.path = a->path;
1734+
if (handle_file_collision(opt, a->path,
1735+
NULL, NULL,
1736+
ci->ren1->branch,
1737+
ci->ren2->branch,
1738+
&mfi.blob, add) < 0)
1739+
return -1;
17841740
} else {
1785-
/*
1786-
* For each destination path, we need to see if there is a
1787-
* rename/add collision. If not, we can write the file out
1788-
* to the specified location.
1789-
*/
1790-
add = &ci->ren1->dst_entry->stages[flip_stage(2)];
1791-
if (is_valid(add)) {
1792-
add->path = mfi.blob.path = a->path;
1793-
if (handle_file_collision(opt, a->path,
1794-
NULL, NULL,
1795-
ci->ren1->branch,
1796-
ci->ren2->branch,
1797-
&mfi.blob, add) < 0)
1798-
return -1;
1799-
} else {
1800-
char *new_path = find_path_for_conflict(opt, a->path,
1801-
ci->ren1->branch,
1802-
ci->ren2->branch);
1803-
if (update_file(opt, 0, &mfi.blob,
1804-
new_path ? new_path : a->path))
1805-
return -1;
1806-
free(new_path);
1807-
if (update_stages(opt, a->path, NULL, a, NULL))
1808-
return -1;
1809-
}
1741+
char *new_path = find_path_for_conflict(opt, a->path,
1742+
ci->ren1->branch,
1743+
ci->ren2->branch);
1744+
if (update_file(opt, 0, &mfi.blob,
1745+
new_path ? new_path : a->path))
1746+
return -1;
1747+
free(new_path);
1748+
if (!opt->priv->call_depth &&
1749+
update_stages(opt, a->path, NULL, a, NULL))
1750+
return -1;
1751+
}
18101752

1811-
add = &ci->ren2->dst_entry->stages[flip_stage(3)];
1812-
if (is_valid(add)) {
1813-
add->path = mfi.blob.path = b->path;
1814-
if (handle_file_collision(opt, b->path,
1815-
NULL, NULL,
1816-
ci->ren1->branch,
1817-
ci->ren2->branch,
1818-
add, &mfi.blob) < 0)
1819-
return -1;
1820-
} else {
1821-
char *new_path = find_path_for_conflict(opt, b->path,
1822-
ci->ren2->branch,
1823-
ci->ren1->branch);
1824-
if (update_file(opt, 0, &mfi.blob,
1825-
new_path ? new_path : b->path))
1826-
return -1;
1827-
free(new_path);
1828-
if (update_stages(opt, b->path, NULL, NULL, b))
1829-
return -1;
1830-
}
1753+
add = &ci->ren2->dst_entry->stages[flip_stage(3)];
1754+
if (is_valid(add)) {
1755+
add->path = mfi.blob.path = b->path;
1756+
if (handle_file_collision(opt, b->path,
1757+
NULL, NULL,
1758+
ci->ren1->branch,
1759+
ci->ren2->branch,
1760+
add, &mfi.blob) < 0)
1761+
return -1;
1762+
} else {
1763+
char *new_path = find_path_for_conflict(opt, b->path,
1764+
ci->ren2->branch,
1765+
ci->ren1->branch);
1766+
if (update_file(opt, 0, &mfi.blob,
1767+
new_path ? new_path : b->path))
1768+
return -1;
1769+
free(new_path);
1770+
if (!opt->priv->call_depth &&
1771+
update_stages(opt, b->path, NULL, NULL, b))
1772+
return -1;
18311773
}
18321774

18331775
return 0;

‎t/t6036-recursive-corner-cases.sh

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ test_expect_success 'merge simple rename+criss-cross with no modifications' '
6060
test_must_fail git merge -s recursive R2^0 &&
6161
6262
git ls-files -s >out &&
63-
test_line_count = 2 out &&
63+
test_line_count = 5 out &&
6464
git ls-files -u >out &&
65-
test_line_count = 2 out &&
65+
test_line_count = 3 out &&
6666
git ls-files -o >out &&
6767
test_line_count = 1 out &&
6868
@@ -133,9 +133,9 @@ test_expect_success 'merge criss-cross + rename merges with basic modification'
133133
test_must_fail git merge -s recursive R2^0 &&
134134
135135
git ls-files -s >out &&
136-
test_line_count = 2 out &&
136+
test_line_count = 5 out &&
137137
git ls-files -u >out &&
138-
test_line_count = 2 out &&
138+
test_line_count = 3 out &&
139139
git ls-files -o >out &&
140140
test_line_count = 1 out &&
141141
@@ -218,8 +218,18 @@ test_expect_success 'git detects differently handled merges conflict' '
218218
git ls-files -o >out &&
219219
test_line_count = 1 out &&
220220
221-
git rev-parse >expect \
222-
C:new_a D:new_a E:new_a &&
221+
git cat-file -p C:new_a >ours &&
222+
git cat-file -p C:a >theirs &&
223+
>empty &&
224+
test_must_fail git merge-file \
225+
-L "Temporary merge branch 1" \
226+
-L "" \
227+
-L "Temporary merge branch 2" \
228+
ours empty theirs &&
229+
sed -e "s/^\([<=>]\)/\1\1\1/" ours >ours-tweaked &&
230+
git hash-object ours-tweaked >expect &&
231+
git rev-parse >>expect \
232+
D:new_a E:new_a &&
223233
git rev-parse >actual \
224234
:1:new_a :2:new_a :3:new_a &&
225235
test_cmp expect actual &&
@@ -257,7 +267,8 @@ test_expect_success 'git detects differently handled merges conflict, swapped' '
257267
ctime=$(git log --no-walk --date=raw --format=%cd C | awk "{print \$1}") &&
258268
newctime=$(($btime+1)) &&
259269
git fast-export --no-data --all | sed -e s/$ctime/$newctime/ | git fast-import --force --quiet &&
260-
# End of differences; rest is copy-paste of last test
270+
# End of most differences; rest is copy-paste of last test,
271+
# other than swapping C:a and C:new_a due to order switch
261272
262273
git checkout D^0 &&
263274
test_must_fail git merge -s recursive E^0 &&
@@ -269,8 +280,18 @@ test_expect_success 'git detects differently handled merges conflict, swapped' '
269280
git ls-files -o >out &&
270281
test_line_count = 1 out &&
271282
272-
git rev-parse >expect \
273-
C:new_a D:new_a E:new_a &&
283+
git cat-file -p C:a >ours &&
284+
git cat-file -p C:new_a >theirs &&
285+
>empty &&
286+
test_must_fail git merge-file \
287+
-L "Temporary merge branch 1" \
288+
-L "" \
289+
-L "Temporary merge branch 2" \
290+
ours empty theirs &&
291+
sed -e "s/^\([<=>]\)/\1\1\1/" ours >ours-tweaked &&
292+
git hash-object ours-tweaked >expect &&
293+
git rev-parse >>expect \
294+
D:new_a E:new_a &&
274295
git rev-parse >actual \
275296
:1:new_a :2:new_a :3:new_a &&
276297
test_cmp expect actual &&

0 commit comments

Comments
 (0)