@matheusbsbs / matheus-bagliano / commits / ec61d14963

merge-recursive: Fix modify/delete resolution in the recursive case

When o->call_depth>0 and we have conflicts, we try to find "middle ground" when creating the virtual merge base. In the case of content conflicts, this can be done by doing a three-way content merge and using the result. In all parts where the three-way content merge is clean, it is the correct middle ground, and in parts where it conflicts there is no middle ground but the conflict markers provide a good compromise since they are unlikely to accidentally match any further changes. In the case of a modify/delete conflict, we cannot do the same thing. Accepting either endpoint as the resolution for the virtual merge base runs the risk that when handling the non-recursive case we will silently accept one person's resolution over another without flagging a conflict. In this case, the closest "middle ground" we have is actually the merge base of the candidate merge bases. (We could alternatively attempt a three way content merge using an empty file in place of the deleted file, but that seems to be more work than necessary.) Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Aug 11, 2011 at 23:20 UTC ec61d14963470b02db0d6e3825e2b5bbb1815cb6

2 files changed +26 -16

merge-recursive.c

+24 -14

@@ -1322,27 +1322,42 @@ error_return:

1322

1323 static void handle_delete_modify(struct merge_options *o,

1324 const char *path,

1325 - const char *new_path,

1325 + unsigned char *o_sha, int o_mode,

1326 unsigned char *a_sha, int a_mode,

1327 unsigned char *b_sha, int b_mode)

1328 {

1329 - if (!a_sha) {

1329 + char *renamed = NULL;

1330 + if (dir_in_way(path, !o->call_depth)) {

1331 + renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2);

1332 + }

1333 +

1334 + if (o->call_depth) {

1335 + /*

1336 + * We cannot arbitrarily accept either a_sha or b_sha as

1337 + * correct; since there is no true "middle point" between

1338 + * them, simply reuse the base version for virtual merge base.

1339 + */

1340 + remove_file_from_cache(path);

1341 + update_file(o, 0, o_sha, o_mode, renamed ? renamed : path);

1342 + } else if (!a_sha) {

1343 output(o, 1, "CONFLICT (delete/modify): %s deleted in %s "

1344 "and modified in %s. Version %s of %s left in tree%s%s.",

1345 path, o->branch1,

1346 o->branch2, o->branch2, path,

1334 - NULL == new_path ? "" : " at ",

1335 - NULL == new_path ? "" : new_path);

1336 - update_file(o, 0, b_sha, b_mode, new_path ? new_path : path);

1347 + NULL == renamed ? "" : " at ",

1348 + NULL == renamed ? "" : renamed);

1349 + update_file(o, 0, b_sha, b_mode, renamed ? renamed : path);

1350 } else {

1351 output(o, 1, "CONFLICT (delete/modify): %s deleted in %s "

1352 "and modified in %s. Version %s of %s left in tree%s%s.",

1353 path, o->branch2,

1354 o->branch1, o->branch1, path,

1342 - NULL == new_path ? "" : " at ",

1343 - NULL == new_path ? "" : new_path);

1344 - update_file(o, 0, a_sha, a_mode, new_path ? new_path : path);

1355 + NULL == renamed ? "" : " at ",

1356 + NULL == renamed ? "" : renamed);

1357 + update_file(o, 0, a_sha, a_mode, renamed ? renamed : path);

1358 }

1359 + free(renamed);

1360 +

1361 }

1362

1363 static int merge_content(struct merge_options *o,

@@ -1512,14 +1527,9 @@ static int process_entry(struct merge_options *o,

1527 remove_file(o, 1, path, !a_sha);

1528 } else {

1529 /* Modify/delete; deleted side may have put a directory in the way */

1515 - char *renamed = NULL;

1530 clean_merge = 0;

1517 - if (dir_in_way(path, !o->call_depth)) {

1518 - renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2);

1519 - }

1520 - handle_delete_modify(o, path, renamed,

1531 + handle_delete_modify(o, path, o_sha, o_mode,

1532 a_sha, a_mode, b_sha, b_mode);

1522 - free(renamed);

1533 }

1534 } else if ((!o_sha && a_sha && !b_sha) ||

1535 (!o_sha && !a_sha && b_sha)) {

t/t6036-recursive-corner-cases.sh

+2 -2

@@ -296,7 +296,7 @@ test_expect_success 'setup criss-cross + modify/delete resolved differently' '

296 git tag E

297 '

298

299 -test_expect_failure 'git detects conflict merging criss-cross+modify/delete' '

299 +test_expect_success 'git detects conflict merging criss-cross+modify/delete' '

300 git checkout D^0 &&

301

302 test_must_fail git merge -s recursive E^0 &&

@@ -308,7 +308,7 @@ test_expect_failure 'git detects conflict merging criss-cross+modify/delete' '

308 test $(git rev-parse :2:file) = $(git rev-parse B:file)

309 '

310

311 -test_expect_failure 'git detects conflict merging criss-cross+modify/delete, reverse direction' '

311 +test_expect_success 'git detects conflict merging criss-cross+modify/delete, reverse direction' '

312 git reset --hard &&

313 git checkout E^0 &&

314