merge-recursive: improve rename/rename(1to2)/add[/add] handling
When we have a rename/rename(1to2) conflict, each of the renames can collide with a file addition. Each of these rename/add conflicts suffered from the same kinds of problems that normal rename/add suffered from. Make the code use handle_file_conflicts() as well so that we get all the same fixes and consistent behavior between the different conflict types. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren committed
Nov 7, 2018 at 20:40 UTC
48c9cb9d6d058bcf18e931a1ed0d88792bb506c9
3 files changed
+113
-94
merge-recursive.c
+77
-77
@@ -1709,80 +1709,17 @@ static int handle_rename_add(struct merge_options *o,
1709
ci->dst_entry1->stages[other_stage].mode);
1710
}
1711
1712
-static int handle_file(struct merge_options *o,
1713
- struct diff_filespec *rename,
1714
- int stage,
1715
- struct rename_conflict_info *ci)
1716
-{
1717
- char *dst_name = rename->path;
1718
- struct stage_data *dst_entry;
1719
- const char *cur_branch, *other_branch;
1720
- struct diff_filespec other;
1721
- struct diff_filespec *add;
1722
- int ret;
1723
-
1724
- if (stage == 2) {
1725
- dst_entry = ci->dst_entry1;
1726
- cur_branch = ci->branch1;
1727
- other_branch = ci->branch2;
1728
- } else {
1729
- dst_entry = ci->dst_entry2;
1730
- cur_branch = ci->branch2;
1731
- other_branch = ci->branch1;
1732
- }
1733
-
1734
- add = filespec_from_entry(&other, dst_entry, stage ^ 1);
1735
- if (add) {
1736
- int ren_src_was_dirty = was_dirty(o, rename->path);
1737
- char *add_name = unique_path(o, rename->path, other_branch);
1738
- if (update_file(o, 0, &add->oid, add->mode, add_name))
1739
- return -1;
1740
-
1741
- if (ren_src_was_dirty) {
1742
- output(o, 1, _("Refusing to lose dirty file at %s"),
1743
- rename->path);
1744
- }
1745
- /*
1746
- * Because the double negatives somehow keep confusing me...
1747
- * 1) update_wd iff !ren_src_was_dirty.
1748
- * 2) no_wd iff !update_wd
1749
- * 3) so, no_wd == !!ren_src_was_dirty == ren_src_was_dirty
1750
- */
1751
- remove_file(o, 0, rename->path, ren_src_was_dirty);
1752
- dst_name = unique_path(o, rename->path, cur_branch);
1753
- } else {
1754
- if (dir_in_way(rename->path, !o->call_depth, 0)) {
1755
- dst_name = unique_path(o, rename->path, cur_branch);
1756
- output(o, 1, _("%s is a directory in %s adding as %s instead"),
1757
- rename->path, other_branch, dst_name);
1758
- } else if (!o->call_depth &&
1759
- would_lose_untracked(rename->path)) {
1760
- dst_name = unique_path(o, rename->path, cur_branch);
1761
- output(o, 1, _("Refusing to lose untracked file at %s; "
1762
- "adding as %s instead"),
1763
- rename->path, dst_name);
1764
- }
1765
- }
1766
- if ((ret = update_file(o, 0, &rename->oid, rename->mode, dst_name)))
1767
- ; /* fall through, do allow dst_name to be released */
1768
- else if (stage == 2)
1769
- ret = update_stages(o, rename->path, NULL, rename, add);
1770
- else
1771
- ret = update_stages(o, rename->path, NULL, add, rename);
1772
-
1773
- if (dst_name != rename->path)
1774
- free(dst_name);
1775
-
1776
- return ret;
1777
-}
1778
-
1712
static int handle_rename_rename_1to2(struct merge_options *o,
1713
struct rename_conflict_info *ci)
1714
{
1715
/* One file was renamed in both branches, but to different names. */
1716
+ struct merge_file_info mfi;
1717
+ struct diff_filespec other;
1718
+ struct diff_filespec *add;
1719
struct diff_filespec *one = ci->pair1->one;
1720
struct diff_filespec *a = ci->pair1->two;
1721
struct diff_filespec *b = ci->pair2->two;
1722
+ char *path_desc;
1723
1724
output(o, 1, _("CONFLICT (rename/rename): "
1725
"Rename \"%s\"->\"%s\" in branch \"%s\" "
@@ -1790,15 +1727,16 @@ static int handle_rename_rename_1to2(struct merge_options *o,
1727
one->path, a->path, ci->branch1,
1728
one->path, b->path, ci->branch2,
1729
o->call_depth ? _(" (left unresolved)") : "");
1793
- if (o->call_depth) {
1794
- struct merge_file_info mfi;
1795
- struct diff_filespec other;
1796
- struct diff_filespec *add;
1797
- if (merge_mode_and_contents(o, one, a, b, one->path,
1798
- ci->branch1, ci->branch2,
1799
- o->call_depth * 2, &mfi))
1800
- return -1;
1730
1731
+ path_desc = xstrfmt("%s and %s, both renamed from %s",
1732
+ a->path, b->path, one->path);
1733
+ if (merge_mode_and_contents(o, one, a, b, path_desc,
1734
+ ci->branch1, ci->branch2,
1735
+ o->call_depth * 2, &mfi))
1736
+ return -1;
1737
+ free(path_desc);
1738
+
1739
+ if (o->call_depth) {
1740
/*
1741
* FIXME: For rename/add-source conflicts (if we could detect
1742
* such), this is wrong. We should instead find a unique
@@ -1830,8 +1768,70 @@ static int handle_rename_rename_1to2(struct merge_options *o,
1768
}
1769
else
1770
remove_file_from_cache(b->path);
1833
- } else if (handle_file(o, a, 2, ci) || handle_file(o, b, 3, ci))
1834
- return -1;
1771
+ } else {
1772
+ /*
1773
+ * For each destination path, we need to see if there is a
1774
+ * rename/add collision. If not, we can write the file out
1775
+ * to the specified location.
1776
+ */
1777
+ add = filespec_from_entry(&other, ci->dst_entry1, 2 ^ 1);
1778
+ if (add) {
1779
+ if (handle_file_collision(o, a->path,
1780
+ NULL, NULL,
1781
+ ci->branch1, ci->branch2,
1782
+ &mfi.oid, mfi.mode,
1783
+ &add->oid, add->mode) < 0)
1784
+ return -1;
1785
+ } else {
1786
+ char *new_path = NULL;
1787
+ if (dir_in_way(a->path, !o->call_depth, 0)) {
1788
+ new_path = unique_path(o, a->path, ci->branch1);
1789
+ output(o, 1, _("%s is a directory in %s adding "
1790
+ "as %s instead"),
1791
+ a->path, ci->branch2, new_path);
1792
+ } else if (would_lose_untracked(a->path)) {
1793
+ new_path = unique_path(o, a->path, ci->branch1);
1794
+ output(o, 1, _("Refusing to lose untracked file"
1795
+ " at %s; adding as %s instead"),
1796
+ a->path, new_path);
1797
+ }
1798
+
1799
+ if (update_file(o, 0, &mfi.oid, mfi.mode, new_path ? new_path : a->path))
1800
+ return -1;
1801
+ free(new_path);
1802
+ if (update_stages(o, a->path, NULL, a, NULL))
1803
+ return -1;
1804
+ }
1805
+
1806
+ add = filespec_from_entry(&other, ci->dst_entry2, 3 ^ 1);
1807
+ if (add) {
1808
+ if (handle_file_collision(o, b->path,
1809
+ NULL, NULL,
1810
+ ci->branch1, ci->branch2,
1811
+ &add->oid, add->mode,
1812
+ &mfi.oid, mfi.mode) < 0)
1813
+ return -1;
1814
+ } else {
1815
+ char *new_path = NULL;
1816
+ if (dir_in_way(b->path, !o->call_depth, 0)) {
1817
+ new_path = unique_path(o, b->path, ci->branch2);
1818
+ output(o, 1, _("%s is a directory in %s adding "
1819
+ "as %s instead"),
1820
+ b->path, ci->branch1, new_path);
1821
+ } else if (would_lose_untracked(b->path)) {
1822
+ new_path = unique_path(o, b->path, ci->branch2);
1823
+ output(o, 1, _("Refusing to lose untracked file"
1824
+ " at %s; adding as %s instead"),
1825
+ b->path, new_path);
1826
+ }
1827
+
1828
+ if (update_file(o, 0, &mfi.oid, mfi.mode, new_path ? new_path : b->path))
1829
+ return -1;
1830
+ free(new_path);
1831
+ if (update_stages(o, b->path, NULL, NULL, b))
1832
+ return -1;
1833
+ }
1834
+ }
1835
1836
return 0;
1837
}
t/t6042-merge-rename-corner-cases.sh
+21
-8
@@ -684,7 +684,7 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting
684
git ls-files -u c >out &&
685
test_line_count = 2 out &&
686
git ls-files -o >out &&
687
- test_line_count = 5 out &&
687
+ test_line_count = 1 out &&
688
689
git rev-parse >expect \
690
A:a C:b B:b C:c B:c &&
@@ -692,14 +692,27 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting
692
:1:a :2:b :3:b :2:c :3:c &&
693
test_cmp expect actual &&
694
695
- git rev-parse >expect \
696
- C:c B:c C:b B:b &&
697
- git hash-object >actual \
698
- c~HEAD c~B\^0 b~HEAD b~B\^0 &&
699
- test_cmp expect actual &&
695
+ # Record some contents for re-doing merges
696
+ git cat-file -p A:a >stuff &&
697
+ git cat-file -p C:b >important_info &&
698
+ git cat-file -p B:c >precious_data &&
699
+ >empty &&
700
701
- test_path_is_missing b &&
702
- test_path_is_missing c
701
+ # Test the merge in b
702
+ test_must_fail git merge-file \
703
+ -L "HEAD" \
704
+ -L "" \
705
+ -L "B^0" \
706
+ important_info empty stuff &&
707
+ test_cmp important_info b &&
708
+
709
+ # Test the merge in c
710
+ test_must_fail git merge-file \
711
+ -L "HEAD" \
712
+ -L "" \
713
+ -L "B^0" \
714
+ stuff empty precious_data &&
715
+ test_cmp stuff c
716
)
717
'
718
t/t6043-merge-rename-directories.sh
+15
-9
@@ -1078,7 +1078,7 @@ test_expect_success '5c-check: Transitive rename would cause rename/rename/renam
1078
git ls-files -u >out &&
1079
test_line_count = 6 out &&
1080
git ls-files -o >out &&
1081
- test_line_count = 3 out &&
1081
+ test_line_count = 1 out &&
1082
1083
git rev-parse >actual \
1084
:0:y/b :0:y/c :0:y/e &&
@@ -1094,9 +1094,9 @@ test_expect_success '5c-check: Transitive rename would cause rename/rename/renam
1094
test_cmp expect actual &&
1095
1096
git hash-object >actual \
1097
- w/d~HEAD w/d~B^0 z/d &&
1097
+ z/d &&
1098
git rev-parse >expect \
1099
- O:x/d B:w/d O:x/d &&
1099
+ O:x/d &&
1100
test_cmp expect actual &&
1101
test_path_is_missing x/d &&
1102
test_path_is_file y/d &&
@@ -3672,7 +3672,7 @@ test_expect_success '11e-check: Avoid deleting not-uptodate with dir rename/rena
3672
git ls-files -u >out &&
3673
test_line_count = 4 out &&
3674
git ls-files -o >out &&
3675
- test_line_count = 4 out &&
3675
+ test_line_count = 3 out &&
3676
3677
echo different >expected &&
3678
echo mods >>expected &&
@@ -3684,11 +3684,17 @@ test_expect_success '11e-check: Avoid deleting not-uptodate with dir rename/rena
3684
O:z/a O:z/b O:x/d O:x/c O:x/c A:y/c O:x/c &&
3685
test_cmp expect actual &&
3686
3687
- git hash-object >actual \
3688
- y/c~B^0 y/c~HEAD &&
3689
- git rev-parse >expect \
3690
- O:x/c A:y/c &&
3691
- test_cmp expect actual
3687
+ # See if y/c~merged has expected contents; requires manually
3688
+ # doing the expected file merge
3689
+ git cat-file -p A:y/c >c1 &&
3690
+ git cat-file -p B:z/c >c2 &&
3691
+ >empty &&
3692
+ test_must_fail git merge-file \
3693
+ -L "HEAD" \
3694
+ -L "" \
3695
+ -L "B^0" \
3696
+ c1 empty c2 &&
3697
+ test_cmp c1 y/c~merged
3698
)
3699
'
3700