merge-recursive: improve handling for rename/rename(2to1) conflicts
This makes the rename/rename(2to1) conflicts use the new handle_file_collision() function. Since that function was based originally on the rename/rename(2to1) handling code, the main differences here are in what was added. In particular: * Instead of storing files at collide_path~HEAD and collide_path~MERGE, the files are two-way merged and recorded at collide_path. * Instead of recording the version of the renamed file that existed on the renamed side in the index (thus ignoring any changes that were made to the file on the side of history without the rename), we do a three-way content merge on the renamed path, then store that at either stage 2 or stage 3. * Note that since the content merge for each rename may have conflicts, and then we have to merge the two renamed files, we can end up with nested conflict markers. 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
bbafc9c44ae7109f636772350934b3c5d747da44
4 files changed
+89
-148
merge-recursive.c
+14
-90
@@ -696,27 +696,6 @@ static int update_stages(struct merge_options *opt, const char *path,
696
return 0;
697
}
698
699
-static int update_stages_for_stage_data(struct merge_options *opt,
700
- const char *path,
701
- const struct stage_data *stage_data)
702
-{
703
- struct diff_filespec o, a, b;
704
-
705
- o.mode = stage_data->stages[1].mode;
706
- oidcpy(&o.oid, &stage_data->stages[1].oid);
707
-
708
- a.mode = stage_data->stages[2].mode;
709
- oidcpy(&a.oid, &stage_data->stages[2].oid);
710
-
711
- b.mode = stage_data->stages[3].mode;
712
- oidcpy(&b.oid, &stage_data->stages[3].oid);
713
-
714
- return update_stages(opt, path,
715
- is_null_oid(&o.oid) ? NULL : &o,
716
- is_null_oid(&a.oid) ? NULL : &a,
717
- is_null_oid(&b.oid) ? NULL : &b);
718
-}
719
-
699
static void update_entry(struct stage_data *entry,
700
struct diff_filespec *o,
701
struct diff_filespec *a,
@@ -1870,7 +1849,6 @@ static int handle_rename_rename_2to1(struct merge_options *o,
1849
char *path_side_2_desc;
1850
struct merge_file_info mfi_c1;
1851
struct merge_file_info mfi_c2;
1873
- int ret;
1852
1853
output(o, 1, _("CONFLICT (rename/rename): "
1854
"Rename %s->%s in %s. "
@@ -1878,81 +1856,22 @@ static int handle_rename_rename_2to1(struct merge_options *o,
1856
a->path, c1->path, ci->branch1,
1857
b->path, c2->path, ci->branch2);
1858
1881
- remove_file(o, 1, a->path, o->call_depth || would_lose_untracked(a->path));
1882
- remove_file(o, 1, b->path, o->call_depth || would_lose_untracked(b->path));
1883
-
1859
path_side_1_desc = xstrfmt("version of %s from %s", path, a->path);
1860
path_side_2_desc = xstrfmt("version of %s from %s", path, b->path);
1861
if (merge_mode_and_contents(o, a, c1, &ci->ren1_other, path_side_1_desc,
1862
o->branch1, o->branch2,
1888
- o->call_depth * 2, &mfi_c1) ||
1863
+ 1 + o->call_depth * 2, &mfi_c1) ||
1864
merge_mode_and_contents(o, b, &ci->ren2_other, c2, path_side_2_desc,
1865
o->branch1, o->branch2,
1891
- o->call_depth * 2, &mfi_c2))
1866
+ 1 + o->call_depth * 2, &mfi_c2))
1867
return -1;
1868
free(path_side_1_desc);
1869
free(path_side_2_desc);
1870
1896
- if (o->call_depth) {
1897
- /*
1898
- * If mfi_c1.clean && mfi_c2.clean, then it might make
1899
- * sense to do a two-way merge of those results. But, I
1900
- * think in all cases, it makes sense to have the virtual
1901
- * merge base just undo the renames; they can be detected
1902
- * again later for the non-recursive merge.
1903
- */
1904
- remove_file(o, 0, path, 0);
1905
- ret = update_file(o, 0, &mfi_c1.oid, mfi_c1.mode, a->path);
1906
- if (!ret)
1907
- ret = update_file(o, 0, &mfi_c2.oid, mfi_c2.mode,
1908
- b->path);
1909
- } else {
1910
- char *new_path1 = unique_path(o, path, ci->branch1);
1911
- char *new_path2 = unique_path(o, path, ci->branch2);
1912
- output(o, 1, _("Renaming %s to %s and %s to %s instead"),
1913
- a->path, new_path1, b->path, new_path2);
1914
- if (was_dirty(o, path))
1915
- output(o, 1, _("Refusing to lose dirty file at %s"),
1916
- path);
1917
- else if (would_lose_untracked(path))
1918
- /*
1919
- * Only way we get here is if both renames were from
1920
- * a directory rename AND user had an untracked file
1921
- * at the location where both files end up after the
1922
- * two directory renames. See testcase 10d of t6043.
1923
- */
1924
- output(o, 1, _("Refusing to lose untracked file at "
1925
- "%s, even though it's in the way."),
1926
- path);
1927
- else
1928
- remove_file(o, 0, path, 0);
1929
- ret = update_file(o, 0, &mfi_c1.oid, mfi_c1.mode, new_path1);
1930
- if (!ret)
1931
- ret = update_file(o, 0, &mfi_c2.oid, mfi_c2.mode,
1932
- new_path2);
1933
- /*
1934
- * unpack_trees() actually populates the index for us for
1935
- * "normal" rename/rename(2to1) situtations so that the
1936
- * correct entries are at the higher stages, which would
1937
- * make the call below to update_stages_for_stage_data
1938
- * unnecessary. However, if either of the renames came
1939
- * from a directory rename, then unpack_trees() will not
1940
- * have gotten the right data loaded into the index, so we
1941
- * need to do so now. (While it'd be tempting to move this
1942
- * call to update_stages_for_stage_data() to
1943
- * apply_directory_rename_modifications(), that would break
1944
- * our intermediate calls to would_lose_untracked() since
1945
- * those rely on the current in-memory index. See also the
1946
- * big "NOTE" in update_stages()).
1947
- */
1948
- if (update_stages_for_stage_data(o, path, ci->dst_entry1))
1949
- ret = -1;
1950
-
1951
- free(new_path2);
1952
- free(new_path1);
1953
- }
1954
-
1955
- return ret;
1871
+ return handle_file_collision(o, path, a->path, b->path,
1872
+ ci->branch1, ci->branch2,
1873
+ &mfi_c1.oid, mfi_c1.mode,
1874
+ &mfi_c2.oid, mfi_c2.mode);
1875
}
1876
1877
/*
@@ -3361,9 +3280,14 @@ static int process_entry(struct merge_options *o,
3280
clean_merge = -1;
3281
break;
3282
case RENAME_TWO_FILES_TO_ONE:
3364
- clean_merge = 0;
3365
- if (handle_rename_rename_2to1(o, conflict_info))
3366
- clean_merge = -1;
3283
+ /*
3284
+ * Probably unclean merge, but if the two renamed
3285
+ * files merge cleanly and the two resulting files
3286
+ * can then be two-way merged cleanly, I guess it's
3287
+ * a clean merge?
3288
+ */
3289
+ clean_merge = handle_rename_rename_2to1(o,
3290
+ conflict_info);
3291
break;
3292
default:
3293
entry->processed = 0;
t/t6036-recursive-corner-cases.sh
+3
-9
@@ -64,15 +64,12 @@ test_expect_success 'merge simple rename+criss-cross with no modifications' '
64
git ls-files -u >out &&
65
test_line_count = 2 out &&
66
git ls-files -o >out &&
67
- test_line_count = 3 out &&
67
+ test_line_count = 1 out &&
68
69
git rev-parse >expect \
70
- L2:three R2:three \
70
L2:three R2:three &&
71
git rev-parse >actual \
72
:2:three :3:three &&
74
- git hash-object >>actual \
75
- three~HEAD three~R2^0 &&
73
test_cmp expect actual
74
)
75
'
@@ -140,15 +137,12 @@ test_expect_success 'merge criss-cross + rename merges with basic modification'
137
git ls-files -u >out &&
138
test_line_count = 2 out &&
139
git ls-files -o >out &&
143
- test_line_count = 3 out &&
140
+ test_line_count = 1 out &&
141
142
git rev-parse >expect \
146
- L2:three R2:three \
143
L2:three R2:three &&
144
git rev-parse >actual \
145
:2:three :3:three &&
150
- git hash-object >>actual \
151
- three~HEAD three~R2^0 &&
146
test_cmp expect actual
147
)
148
'
@@ -1512,7 +1506,7 @@ test_expect_success 'setup nested conflicts' '
1506
)
1507
'
1508
1515
-test_expect_failure 'check nested conflicts' '
1509
+test_expect_success 'check nested conflicts' '
1510
(
1511
cd nested_conflicts &&
1512
t/t6042-merge-rename-corner-cases.sh
+24
-14
@@ -464,17 +464,28 @@ test_expect_success 'handle rename/rename (2to1) conflict correctly' '
464
git ls-files -u c >out &&
465
test_line_count = 2 out &&
466
git ls-files -o >out &&
467
- test_line_count = 3 out &&
467
+ test_line_count = 1 out &&
468
469
test_path_is_missing a &&
470
test_path_is_missing b &&
471
- test_path_is_file c~HEAD &&
472
- test_path_is_file c~C^0 &&
471
474
- git rev-parse >expect \
475
- C:a B:b &&
476
- git hash-object >actual \
477
- c~HEAD c~C^0 &&
472
+ git rev-parse >expect \
473
+ C:a B:b &&
474
+ git rev-parse >actual \
475
+ :2:c :3:c &&
476
+ test_cmp expect actual &&
477
+
478
+ # Test that the two-way merge in new_a is as expected
479
+ git cat-file -p :2:c >>ours &&
480
+ git cat-file -p :3:c >>theirs &&
481
+ >empty &&
482
+ test_must_fail git merge-file \
483
+ -L "HEAD" \
484
+ -L "" \
485
+ -L "C^0" \
486
+ ours empty theirs &&
487
+ git hash-object c >actual &&
488
+ git hash-object ours >expect &&
489
test_cmp expect actual
490
)
491
'
@@ -940,7 +951,6 @@ test_expect_failure 'mod6-check: chains of rename/rename(1to2) and rename/rename
951
test_conflicts_with_adds_and_renames() {
952
sideL=$1
953
sideR=$2
943
- expect=$3
954
955
# Setup:
956
# L
@@ -1048,7 +1058,7 @@ test_conflicts_with_adds_and_renames() {
1058
)
1059
'
1060
1051
- test_expect_$expect "check simple $sideL/$sideR conflict" '
1061
+ test_expect_success "check simple $sideL/$sideR conflict" '
1062
(
1063
cd simple_${sideL}_${sideR} &&
1064
@@ -1094,10 +1104,10 @@ test_conflicts_with_adds_and_renames() {
1104
'
1105
}
1106
1097
-test_conflicts_with_adds_and_renames rename rename failure
1098
-test_conflicts_with_adds_and_renames rename add success
1099
-test_conflicts_with_adds_and_renames add rename success
1100
-test_conflicts_with_adds_and_renames add add success
1107
+test_conflicts_with_adds_and_renames rename rename
1108
+test_conflicts_with_adds_and_renames rename add
1109
+test_conflicts_with_adds_and_renames add rename
1110
+test_conflicts_with_adds_and_renames add add
1111
1112
# Setup:
1113
# L
@@ -1168,7 +1178,7 @@ test_expect_success 'setup nested conflicts from rename/rename(2to1)' '
1178
)
1179
'
1180
1171
-test_expect_failure 'check nested conflicts from rename/rename(2to1)' '
1181
+test_expect_success 'check nested conflicts from rename/rename(2to1)' '
1182
(
1183
cd nested_conflicts_from_rename_rename &&
1184
t/t6043-merge-rename-directories.sh
+48
-35
@@ -278,7 +278,7 @@ test_expect_success '1d-check: Directory renames cause a rename/rename(2to1) con
278
git ls-files -u >out &&
279
test_line_count = 2 out &&
280
git ls-files -o >out &&
281
- test_line_count = 3 out &&
281
+ test_line_count = 1 out &&
282
283
git rev-parse >actual \
284
:0:x/b :0:x/c :0:x/d :0:x/e :0:x/m :0:x/n &&
@@ -293,15 +293,16 @@ test_expect_success '1d-check: Directory renames cause a rename/rename(2to1) con
293
A:y/wham B:z/wham &&
294
test_cmp expect actual &&
295
296
- test_path_is_missing x/wham &&
297
- test_path_is_file x/wham~HEAD &&
298
- test_path_is_file x/wham~B^0 &&
299
-
300
- git hash-object >actual \
301
- x/wham~HEAD x/wham~B^0 &&
302
- git rev-parse >expect \
303
- A:y/wham B:z/wham &&
304
- test_cmp expect actual
296
+ # Test that the two-way merge in x/wham is as expected
297
+ git cat-file -p :2:x/wham >expect &&
298
+ git cat-file -p :3:x/wham >other &&
299
+ >empty &&
300
+ test_must_fail git merge-file \
301
+ -L "HEAD" \
302
+ -L "" \
303
+ -L "B^0" \
304
+ expect empty other &&
305
+ test_cmp expect x/wham
306
)
307
'
308
@@ -1670,7 +1671,7 @@ test_expect_success '7b-check: rename/rename(2to1), but only due to transitive r
1671
git ls-files -u >out &&
1672
test_line_count = 2 out &&
1673
git ls-files -o >out &&
1673
- test_line_count = 3 out &&
1674
+ test_line_count = 1 out &&
1675
1676
git rev-parse >actual \
1677
:0:y/b :0:y/c :2:y/d :3:y/d &&
@@ -1678,15 +1679,16 @@ test_expect_success '7b-check: rename/rename(2to1), but only due to transitive r
1679
O:z/b O:z/c O:w/d O:x/d &&
1680
test_cmp expect actual &&
1681
1681
- test_path_is_missing y/d &&
1682
- test_path_is_file y/d~HEAD &&
1683
- test_path_is_file y/d~B^0 &&
1684
-
1685
- git hash-object >actual \
1686
- y/d~HEAD y/d~B^0 &&
1687
- git rev-parse >expect \
1688
- O:w/d O:x/d &&
1689
- test_cmp expect actual
1682
+ # Test that the two-way merge in y/d is as expected
1683
+ git cat-file -p :2:y/d >expect &&
1684
+ git cat-file -p :3:y/d >other &&
1685
+ >empty &&
1686
+ test_must_fail git merge-file \
1687
+ -L "HEAD" \
1688
+ -L "" \
1689
+ -L "B^0" \
1690
+ expect empty other &&
1691
+ test_cmp expect y/d
1692
)
1693
'
1694
@@ -3165,7 +3167,7 @@ test_expect_success '10c-check: Overwrite untracked with dir rename/rename(1to2)
3167
# Commit O: z/{a,b,c_1}, x/{d,e,f_2}
3168
# Commit A: y/{a,b}, x/{d,e,f_2,wham_1} + untracked y/wham
3169
# Commit B: z/{a,b,c_1,wham_2}, y/{d,e}
3168
-# Expected: Failed Merge; y/{a,b,d,e} + untracked y/{wham,wham~B^0,wham~HEAD}+
3170
+# Expected: Failed Merge; y/{a,b,d,e} + untracked y/{wham,wham~merged}+
3171
# CONFLICT(rename/rename) z/c_1 vs x/f_2 -> y/wham
3172
# ERROR_MSG(Refusing to lose untracked file at y/wham)
3173
@@ -3219,7 +3221,7 @@ test_expect_success '10d-check: Delete untracked with dir rename/rename(2to1)' '
3221
git ls-files -u >out &&
3222
test_line_count = 2 out &&
3223
git ls-files -o >out &&
3222
- test_line_count = 4 out &&
3224
+ test_line_count = 3 out &&
3225
3226
git rev-parse >actual \
3227
:0:y/a :0:y/b :0:y/d :0:y/e :2:y/wham :3:y/wham &&
@@ -3232,11 +3234,16 @@ test_expect_success '10d-check: Delete untracked with dir rename/rename(2to1)' '
3234
echo important >expect &&
3235
test_cmp expect y/wham &&
3236
3235
- git hash-object >actual \
3236
- y/wham~B^0 y/wham~HEAD &&
3237
- git rev-parse >expect \
3238
- O:x/f O:z/c &&
3239
- test_cmp expect actual
3237
+ # Test that the two-way merge in y/wham~merged is as expected
3238
+ git cat-file -p :2:y/wham >expect &&
3239
+ git cat-file -p :3:y/wham >other &&
3240
+ >empty &&
3241
+ test_must_fail git merge-file \
3242
+ -L "HEAD" \
3243
+ -L "" \
3244
+ -L "B^0" \
3245
+ expect empty other &&
3246
+ test_cmp expect y/wham~merged
3247
)
3248
'
3249
@@ -3689,7 +3696,7 @@ test_expect_success '11e-check: Avoid deleting not-uptodate with dir rename/rena
3696
# Commit O: z/{a,b}, x/{c_1,d_2}
3697
# Commit A: y/{a,b,wham_1}, x/d_2, except y/wham has uncommitted mods
3698
# Commit B: z/{a,b,wham_2}, x/c_1
3692
-# Expected: Failed Merge; y/{a,b} + untracked y/{wham~B^0,wham~B^HEAD} +
3699
+# Expected: Failed Merge; y/{a,b} + untracked y/{wham~merged} +
3700
# y/wham with dirty changes from before merge +
3701
# CONFLICT(rename/rename) x/c vs x/d -> y/wham
3702
# ERROR_MSG(Refusing to lose dirty file at y/wham)
@@ -3741,24 +3748,30 @@ test_expect_success '11f-check: Avoid deleting not-uptodate with dir rename/rena
3748
git ls-files -u >out &&
3749
test_line_count = 2 out &&
3750
git ls-files -o >out &&
3744
- test_line_count = 4 out &&
3751
+ test_line_count = 3 out &&
3752
3753
test_seq 1 10 >expected &&
3754
echo important >>expected &&
3755
test_cmp expected y/wham &&
3756
3757
test_must_fail git rev-parse :1:y/wham &&
3751
- git hash-object >actual \
3752
- y/wham~B^0 y/wham~HEAD &&
3753
- git rev-parse >expect \
3754
- O:x/d O:x/c &&
3755
- test_cmp expect actual &&
3758
3759
git rev-parse >actual \
3760
:0:y/a :0:y/b :2:y/wham :3:y/wham &&
3761
git rev-parse >expect \
3762
O:z/a O:z/b O:x/c O:x/d &&
3761
- test_cmp expect actual
3763
+ test_cmp expect actual &&
3764
+
3765
+ # Test that the two-way merge in y/wham~merged is as expected
3766
+ git cat-file -p :2:y/wham >expect &&
3767
+ git cat-file -p :3:y/wham >other &&
3768
+ >empty &&
3769
+ test_must_fail git merge-file \
3770
+ -L "HEAD" \
3771
+ -L "" \
3772
+ -L "B^0" \
3773
+ expect empty other &&
3774
+ test_cmp expect y/wham~merged
3775
)
3776
'
3777