merge-recursive: make "CONFLICT (rename/delete)" message show both paths
The current message printed by "git merge-recursive" for a rename/delete conflict is like this: CONFLICT (rename/delete): new-path deleted in HEAD and renamed in other-branch. Version other-branch of new-path left in tree. To be more helpful, the message should show both paths of the rename and state that the deletion occurred at the old path, not the new path. So change the message to the following format: CONFLICT (rename/delete): old-path deleted in HEAD and renamed to new-path in other-branch. Version other-branch of new-path left in tree. Since this doubles the number of cases in handle_change_delete (modify vs. rename), refactor the code to halve the number of cases again by merging the cases where o->branch1 has the change and o->branch2 has the delete with the cases that are the other way around. Also add a simple test of the new conflict message. Signed-off-by: Matt McCutchen <matt@mattmccutchen.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Matt McCutchen committed
Jan 28, 2017 at 15:37 UTC
b26d87f2da22a5970b15bebe588bcd6ccac98e9b
2 files changed
+86
-54
merge-recursive.c
+63
-54
@@ -1061,16 +1061,20 @@ static int merge_file_one(struct merge_options *o,
1061
}
1062
1063
static int handle_change_delete(struct merge_options *o,
1064
- const char *path,
1064
+ const char *path, const char *old_path,
1065
const struct object_id *o_oid, int o_mode,
1066
- const struct object_id *a_oid, int a_mode,
1067
- const struct object_id *b_oid, int b_mode,
1066
+ const struct object_id *changed_oid,
1067
+ int changed_mode,
1068
+ const char *change_branch,
1069
+ const char *delete_branch,
1070
const char *change, const char *change_past)
1071
{
1070
- char *renamed = NULL;
1072
+ char *alt_path = NULL;
1073
+ const char *update_path = path;
1074
int ret = 0;
1075
+
1076
if (dir_in_way(path, !o->call_depth, 0)) {
1073
- renamed = unique_path(o, path, a_oid ? o->branch1 : o->branch2);
1077
+ update_path = alt_path = unique_path(o, path, change_branch);
1078
}
1079
1080
if (o->call_depth) {
@@ -1081,43 +1085,43 @@ static int handle_change_delete(struct merge_options *o,
1085
*/
1086
ret = remove_file_from_cache(path);
1087
if (!ret)
1084
- ret = update_file(o, 0, o_oid, o_mode,
1085
- renamed ? renamed : path);
1086
- } else if (!a_oid) {
1087
- if (!renamed) {
1088
- output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
1089
- "and %s in %s. Version %s of %s left in tree."),
1090
- change, path, o->branch1, change_past,
1091
- o->branch2, o->branch2, path);
1092
- ret = update_file(o, 0, b_oid, b_mode, path);
1093
- } else {
1094
- output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
1095
- "and %s in %s. Version %s of %s left in tree at %s."),
1096
- change, path, o->branch1, change_past,
1097
- o->branch2, o->branch2, path, renamed);
1098
- ret = update_file(o, 0, b_oid, b_mode, renamed);
1099
- }
1088
+ ret = update_file(o, 0, o_oid, o_mode, update_path);
1089
} else {
1101
- if (!renamed) {
1102
- output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
1103
- "and %s in %s. Version %s of %s left in tree."),
1104
- change, path, o->branch2, change_past,
1105
- o->branch1, o->branch1, path);
1090
+ if (!alt_path) {
1091
+ if (!old_path) {
1092
+ output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
1093
+ "and %s in %s. Version %s of %s left in tree."),
1094
+ change, path, delete_branch, change_past,
1095
+ change_branch, change_branch, path);
1096
+ } else {
1097
+ output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
1098
+ "and %s to %s in %s. Version %s of %s left in tree."),
1099
+ change, old_path, delete_branch, change_past, path,
1100
+ change_branch, change_branch, path);
1101
+ }
1102
} else {
1107
- output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
1108
- "and %s in %s. Version %s of %s left in tree at %s."),
1109
- change, path, o->branch2, change_past,
1110
- o->branch1, o->branch1, path, renamed);
1111
- ret = update_file(o, 0, a_oid, a_mode, renamed);
1103
+ if (!old_path) {
1104
+ output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
1105
+ "and %s in %s. Version %s of %s left in tree at %s."),
1106
+ change, path, delete_branch, change_past,
1107
+ change_branch, change_branch, path, alt_path);
1108
+ } else {
1109
+ output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
1110
+ "and %s to %s in %s. Version %s of %s left in tree at %s."),
1111
+ change, old_path, delete_branch, change_past, path,
1112
+ change_branch, change_branch, path, alt_path);
1113
+ }
1114
}
1115
/*
1114
- * No need to call update_file() on path when !renamed, since
1115
- * that would needlessly touch path. We could call
1116
- * update_file_flags() with update_cache=0 and update_wd=0,
1117
- * but that's a no-op.
1116
+ * No need to call update_file() on path when change_branch ==
1117
+ * o->branch1 && !alt_path, since that would needlessly touch
1118
+ * path. We could call update_file_flags() with update_cache=0
1119
+ * and update_wd=0, but that's a no-op.
1120
*/
1121
+ if (change_branch != o->branch1 || alt_path)
1122
+ ret = update_file(o, 0, changed_oid, changed_mode, update_path);
1123
}
1120
- free(renamed);
1124
+ free(alt_path);
1125
1126
return ret;
1127
}
@@ -1125,28 +1129,17 @@ static int handle_change_delete(struct merge_options *o,
1129
static int conflict_rename_delete(struct merge_options *o,
1130
struct diff_filepair *pair,
1131
const char *rename_branch,
1128
- const char *other_branch)
1132
+ const char *delete_branch)
1133
{
1134
const struct diff_filespec *orig = pair->one;
1135
const struct diff_filespec *dest = pair->two;
1132
- const struct object_id *a_oid = NULL;
1133
- const struct object_id *b_oid = NULL;
1134
- int a_mode = 0;
1135
- int b_mode = 0;
1136
-
1137
- if (rename_branch == o->branch1) {
1138
- a_oid = &dest->oid;
1139
- a_mode = dest->mode;
1140
- } else {
1141
- b_oid = &dest->oid;
1142
- b_mode = dest->mode;
1143
- }
1136
1137
if (handle_change_delete(o,
1138
o->call_depth ? orig->path : dest->path,
1139
+ o->call_depth ? NULL : orig->path,
1140
&orig->oid, orig->mode,
1148
- a_oid, a_mode,
1149
- b_oid, b_mode,
1141
+ &dest->oid, dest->mode,
1142
+ rename_branch, delete_branch,
1143
_("rename"), _("renamed")))
1144
return -1;
1145
@@ -1665,11 +1658,27 @@ static int handle_modify_delete(struct merge_options *o,
1658
struct object_id *a_oid, int a_mode,
1659
struct object_id *b_oid, int b_mode)
1660
{
1661
+ const char *modify_branch, *delete_branch;
1662
+ struct object_id *changed_oid;
1663
+ int changed_mode;
1664
+
1665
+ if (a_oid) {
1666
+ modify_branch = o->branch1;
1667
+ delete_branch = o->branch2;
1668
+ changed_oid = a_oid;
1669
+ changed_mode = a_mode;
1670
+ } else {
1671
+ modify_branch = o->branch2;
1672
+ delete_branch = o->branch1;
1673
+ changed_oid = b_oid;
1674
+ changed_mode = b_mode;
1675
+ }
1676
+
1677
return handle_change_delete(o,
1669
- path,
1678
+ path, NULL,
1679
o_oid, o_mode,
1671
- a_oid, a_mode,
1672
- b_oid, b_mode,
1680
+ changed_oid, changed_mode,
1681
+ modify_branch, delete_branch,
1682
_("modify"), _("modified"));
1683
}
1684
t/t6045-merge-rename-delete.sh
new
+23
@@ -0,0 +1,23 @@
1
+#!/bin/sh
2
+
3
+test_description='Merge-recursive rename/delete conflict message'
4
+. ./test-lib.sh
5
+
6
+test_expect_success 'rename/delete' '
7
+ echo foo >A &&
8
+ git add A &&
9
+ git commit -m "initial" &&
10
+
11
+ git checkout -b rename &&
12
+ git mv A B &&
13
+ git commit -m "rename" &&
14
+
15
+ git checkout master &&
16
+ git rm A &&
17
+ git commit -m "delete" &&
18
+
19
+ test_must_fail git merge --strategy=recursive rename >output &&
20
+ test_i18ngrep "CONFLICT (rename/delete): A deleted in HEAD and renamed to B in rename. Version rename of B left in tree." output
21
+'
22
+
23
+test_done