Skip to content

Commit 52396e1

Browse files
newrengitster
authored andcommitted
merge-recursive: set paths correctly when three-way merging content
merge_3way() has code to mark different sides of the conflict with info about where the content comes from. If the names of the files involved match, it simply uses the branch name. If the names of the files do not match, it uses branchname:filename. Unfortunately, merge_content() previously always called it with one.path = a.path = b.path. Granted, it didn't have other path information available to it for years, but that was corrected by passing rename_conflict_info in commit 3c217c0 ("merge-recursive: Provide more info in conflict markers with file renames", 2011-08-11). In that commit, instead of just fixing the bug with the pathnames, it created fake branch names incorporating both the branch name and file name. This "fake branch" workaround was extended further when I pulled that logic out into a special function in commit dac4741 ("merge-recursive: Create function for merging with branchname:file markers", 2011-08-11), and a number of other sites outside of merge_content() have been added which call into that. However, this Rube-Goldberg-esque setup is not merely duplicate code and unnecessary work, it also risked having other callsites invoke it in a way that would result in markers of the form branchname:filename:filename (i.e. with the filename repeated). Fix this whole mess by: - setting one.path, a.path, and b.path appropriately - calling merge_file_1() directly - deleting the merge_file_special_markers() workaround wrapper Signed-off-by: Elijah Newren <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 2d3b1c5 commit 52396e1

File tree

1 file changed

+9
-40
lines changed

1 file changed

+9
-40
lines changed

merge-recursive.c

Lines changed: 9 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,35 +1366,6 @@ static int merge_file_1(struct merge_options *o,
13661366
return 0;
13671367
}
13681368

1369-
static int merge_file_special_markers(struct merge_options *o,
1370-
const struct diff_filespec *one,
1371-
const struct diff_filespec *a,
1372-
const struct diff_filespec *b,
1373-
const char *target_filename,
1374-
const char *branch1,
1375-
const char *filename1,
1376-
const char *branch2,
1377-
const char *filename2,
1378-
struct merge_file_info *mfi)
1379-
{
1380-
char *side1 = NULL;
1381-
char *side2 = NULL;
1382-
int ret;
1383-
1384-
if (filename1)
1385-
side1 = xstrfmt("%s:%s", branch1, filename1);
1386-
if (filename2)
1387-
side2 = xstrfmt("%s:%s", branch2, filename2);
1388-
1389-
ret = merge_file_1(o, one, a, b, target_filename,
1390-
side1 ? side1 : branch1,
1391-
side2 ? side2 : branch2, mfi);
1392-
1393-
free(side1);
1394-
free(side2);
1395-
return ret;
1396-
}
1397-
13981369
static int merge_file_one(struct merge_options *o,
13991370
const char *path,
14001371
const struct object_id *o_oid, int o_mode,
@@ -1729,14 +1700,10 @@ static int handle_rename_rename_2to1(struct merge_options *o,
17291700

17301701
path_side_1_desc = xstrfmt("%s (was %s)", path, a->path);
17311702
path_side_2_desc = xstrfmt("%s (was %s)", path, b->path);
1732-
if (merge_file_special_markers(o, a, c1, &ci->ren1_other,
1733-
path_side_1_desc,
1734-
o->branch1, c1->path,
1735-
o->branch2, ci->ren1_other.path, &mfi_c1) ||
1736-
merge_file_special_markers(o, b, &ci->ren2_other, c2,
1737-
path_side_2_desc,
1738-
o->branch1, ci->ren2_other.path,
1739-
o->branch2, c2->path, &mfi_c2))
1703+
if (merge_file_1(o, a, c1, &ci->ren1_other, path_side_1_desc,
1704+
o->branch1, o->branch2, &mfi_c1) ||
1705+
merge_file_1(o, b, &ci->ren2_other, c2, path_side_2_desc,
1706+
o->branch1, o->branch2, &mfi_c2))
17401707
return -1;
17411708
free(path_side_1_desc);
17421709
free(path_side_2_desc);
@@ -3059,14 +3026,16 @@ static int merge_content(struct merge_options *o,
30593026
path2 = (rename_conflict_info->pair2 ||
30603027
o->branch2 == rename_conflict_info->branch1) ?
30613028
pair1->two->path : pair1->one->path;
3029+
one.path = pair1->one->path;
3030+
a.path = (char *)path1;
3031+
b.path = (char *)path2;
30623032

30633033
if (dir_in_way(path, !o->call_depth,
30643034
S_ISGITLINK(pair1->two->mode)))
30653035
df_conflict_remains = 1;
30663036
}
3067-
if (merge_file_special_markers(o, &one, &a, &b, path,
3068-
o->branch1, path1,
3069-
o->branch2, path2, &mfi))
3037+
if (merge_file_1(o, &one, &a, &b, path,
3038+
o->branch1, o->branch2, &mfi))
30703039
return -1;
30713040

30723041
/*

0 commit comments

Comments
 (0)