We generally think of a rename as removing one path entirely and placing similar content at a new path. In other words, after a rename, the original path is now empty. But that is not necessarily the case with rewrite detection (which is not currently possible to do for merge-recursive). The current merge code blindly removes paths that are used as rename sources; however, we should check to see if there is useful content at that path. There are basically two interesting cases: 1. One side renames a path, but also puts new content (or a symlink) at the same path. We want to detect the rename, and have changes from the other side applied to the rename destination. The new content at the original path should be left untouched. The current code just calls remove_file, but that ignores the concept that the renaming side may put something else useful there. We should detect this case and either remove (if no new content), or put the new content in place at the original path. 2. Both sides renamed and installed new content at the original path. If they didn't rename to the same destination, it is a conflict, and we already mark it as such. But if it's the same destination, then it's not a conflict; the renamed content will be merged at the new destination. For the new content at the original path, we have to do a 3-way merge. The base must be the null sha1, because this "slot" for content didn't exist before (it was taken up by the content which got renamed away). So if only one side installed new content, that content automatically wins. If both sides did, and it is the same content, then that content is OK. But if the content is different, then we have a conflict and should do the usual conflict-markers thing. This patch implements the semantics described above, which lays the groundwork for turning on rewrite detection. Signed-off-by: Jeff King <peff@xxxxxxxx> --- I split this one out for easier review. The semantics I am proposing are a superset of the current "remove" behavior (it's just that without break detection on, you can't trigger the other cases). But by putting this in before enabling break detection, you can test easily that the new code isn't breaking any existing behavior. merge-recursive.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 63 insertions(+), 2 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 8e82a8b..16263b0 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -706,6 +706,64 @@ static void update_file(struct merge_options *o, update_file_flags(o, sha, mode, path, o->call_depth || clean, !o->call_depth); } +static int update_or_remove(struct merge_options *o, + const unsigned char *sha1, unsigned mode, + const char *path, int update_wd) +{ + if (is_null_sha1(sha1)) + return remove_file(o, 1, path, !update_wd); + + update_file_flags(o, sha1, mode, path, 1, update_wd); + return 0; +} + +static void merge_rename_source(struct merge_options *o, + const char *path, + struct stage_data *d) +{ + if (is_null_sha1(d->stages[2].sha)) { + /* + * If both were real renames (not from a broken pair), we can + * stop caring about the path. We don't touch the working + * directory, though. The path must be gone in HEAD, so there + * is no point (and anything we did delete would be an + * untracked file). + */ + if (is_null_sha1(d->stages[3].sha)) { + remove_file(o, 1, path, 1); + return; + } + + /* + * If "ours" was a real rename, but the other side came + * from a broken pair, then their version is the right + * resolution (because we have no content, ours having been + * renamed away, and they have new content). + */ + update_file_flags(o, d->stages[3].sha, d->stages[3].mode, + path, 1, 1); + return; + } + + /* + * Now we have the opposite. "theirs" is a real rename, but ours + * is from a broken pair. We resolve in favor of us, but we don't + * need to touch the working directory. + */ + if (is_null_sha1(d->stages[3].sha)) { + update_file_flags(o, d->stages[2].sha, d->stages[2].mode, + path, 1, 0); + return; + } + + /* + * Otherwise, both came from broken pairs. We need to do an actual + * merge on the entries. We can just mark it as unprocessed and + * the regular code will handle it. + */ + d->processed = 0; +} + /* Low level file merging, update and removal */ struct merge_file_info { @@ -1025,7 +1083,7 @@ static int process_renames(struct merge_options *o, ren1->dst_entry, ren2->dst_entry); } else { - remove_file(o, 1, ren1_src, 1); + merge_rename_source(o, ren1_src, ren1->src_entry); update_stages_and_entry(ren1_dst, ren1->dst_entry, ren1->pair->one, @@ -1049,7 +1107,10 @@ static int process_renames(struct merge_options *o, int renamed_stage = a_renames == renames1 ? 2 : 3; int other_stage = a_renames == renames1 ? 3 : 2; - remove_file(o, 1, ren1_src, o->call_depth || renamed_stage == 2); + update_or_remove(o, + ren1->src_entry->stages[renamed_stage].sha, + ren1->src_entry->stages[renamed_stage].mode, + ren1_src, renamed_stage == 3); hashcpy(src_other.sha1, ren1->src_entry->stages[other_stage].sha); src_other.mode = ren1->src_entry->stages[other_stage].mode; -- 1.7.4.41.g423da.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html