From 86273e5764ab7faaa84c3de1a428ed24a6cfdf1f Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 8 Sep 2010 00:40:41 -0600 Subject: merge-recursive: D/F conflicts where was_a_dir/file -> was_a_dir In merge-recursive.c, whenever there was a rename where a file name on one side of the rename matches a directory name on the other side of the merge, then the very first check that string_list_has_string(&o->current_directory_set, ren1_dst) would trigger forcing it into marking it as a rename/directory conflict. However, if the path is only renamed on one side and a simple three-way merge between the separate files resolves cleanly, then we don't need to mark it as a rename/directory conflict. So, we can simply move the check for rename/directory conflicts after we've verified that there isn't a rename/rename conflict and that a threeway content merge doesn't work. This changes the particular error message one gets in the case where the directory name that a file on one side of the rename matches is not also part of the rename pair. For example, with commits containing the files: COMMON -> (HEAD, MERGE ) --------- --------------- ------- sub/file1 -> (sub/file1, newsub) -> (newsub/newfile, ) then previously when one tried to merge MERGE into HEAD, one would get CONFLICT (rename/directory): Rename sub/file1->newsub in HEAD directory newsub added in merge Renaming sub/file1 to newsub~HEAD instead Adding newsub/newfile Automatic merge failed; fix conflicts and then commit the result. After this patch, the error message will instead become: Removing newsub Adding newsub/newfile CONFLICT (file/directory): There is a directory with name newsub in merge. Adding newsub as newsub~HEAD Automatic merge failed; fix conflicts and then commit the result. That makes more sense to me, because git can't know that there's a conflict until after it's tried resolving paths involving newsub/newfile to see if they are still in the way at the end (and if newsub/newfile is not in the way at the end, there should be no conflict at all, which did not hold with git previously). Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 20e1779428..5ea6d11507 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -935,14 +935,7 @@ static int process_renames(struct merge_options *o, try_merge = 0; - if (string_list_has_string(&o->current_directory_set, ren1_dst)) { - clean_merge = 0; - output(o, 1, "CONFLICT (rename/directory): Rename %s->%s in %s " - " directory %s added in %s", - ren1_src, ren1_dst, branch1, - ren1_dst, branch2); - conflict_rename_dir(o, ren1, branch1); - } else if (sha_eq(src_other.sha1, null_sha1)) { + if (sha_eq(src_other.sha1, null_sha1)) { clean_merge = 0; output(o, 1, "CONFLICT (rename/delete): Rename %s->%s in %s " "and deleted in %s", @@ -1034,6 +1027,13 @@ static int process_renames(struct merge_options *o, if (!ren1->dst_entry->stages[2].mode != !ren1->dst_entry->stages[3].mode) ren1->dst_entry->processed = 0; + } else if (string_list_has_string(&o->current_directory_set, ren1_dst)) { + clean_merge = 0; + output(o, 1, "CONFLICT (rename/directory): Rename %s->%s in %s " + " directory %s added in %s", + ren1_src, ren1_dst, branch1, + ren1_dst, branch2); + conflict_rename_dir(o, ren1, branch1); } else { if (mfi.merge || !mfi.clean) output(o, 1, "Renaming %s => %s", ren1_src, ren1_dst); -- cgit v1.2.3 From 7edba4c45c33ba1e17d88907c1601dc660a44522 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 20 Sep 2010 02:28:35 -0600 Subject: merge-recursive: Restructure showing how to chain more process_* functions In 3734893 (merge-recursive: Fix D/F conflicts 2010-07-09), process_df_entry() was added to process_renames() and process_entry() but in a somewhat restrictive manner. Modify the code slightly to make it clearer how we could chain more such functions if necessary, and alter process_df_entry() to handle such chaining. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 51c0536d4e..7db1538f12 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1259,9 +1259,8 @@ static int process_df_entry(struct merge_options *o, const char *conf; struct stat st; - /* We currently only handle D->F cases */ - assert((!o_sha && a_sha && !b_sha) || - (!o_sha && !a_sha && b_sha)); + if (!((!o_sha && a_sha && !b_sha) || (!o_sha && !a_sha && b_sha))) + return 1; /* we don't handle non D-F cases */ entry->processed = 1; @@ -1350,6 +1349,12 @@ int merge_trees(struct merge_options *o, && !process_df_entry(o, path, e)) clean = 0; } + for (i = 0; i < entries->nr; i++) { + struct stage_data *e = entries->items[i].util; + if (!e->processed) + die("Unprocessed path??? %s", + entries->items[i].string); + } string_list_clear(re_merge, 0); string_list_clear(re_head, 0); -- cgit v1.2.3 From 41d70bd6a94d1bd1ec09d2a3d6ebd419abb25bca Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:28:47 -0600 Subject: merge-recursive: Small code clarification -- variable name and comments process_renames() had a variable named "stage" and derived variables src_other and dst_other whose purpose was not immediately obvious; also, I want to extend the scope of this variable and use it later, so it should have a more descriptive name. Do so, and add a brief comment explaining how it is used and what it relates to. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 7db1538f12..f9531c8069 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -924,15 +924,23 @@ static int process_renames(struct merge_options *o, struct string_list_item *item; /* we only use sha1 and mode of these */ struct diff_filespec src_other, dst_other; - int try_merge, stage = a_renames == renames1 ? 3: 2; + int try_merge; - remove_file(o, 1, ren1_src, o->call_depth || stage == 3); + /* + * unpack_trees loads entries from common-commit + * into stage 1, from head-commit into stage 2, and + * from merge-commit into stage 3. We keep track + * of which side corresponds to the rename. + */ + int renamed_stage = a_renames == renames1 ? 2 : 3; + int other_stage = a_renames == renames1 ? 3 : 2; - hashcpy(src_other.sha1, ren1->src_entry->stages[stage].sha); - src_other.mode = ren1->src_entry->stages[stage].mode; - hashcpy(dst_other.sha1, ren1->dst_entry->stages[stage].sha); - dst_other.mode = ren1->dst_entry->stages[stage].mode; + remove_file(o, 1, ren1_src, o->call_depth || renamed_stage == 2); + hashcpy(src_other.sha1, ren1->src_entry->stages[other_stage].sha); + src_other.mode = ren1->src_entry->stages[other_stage].mode; + hashcpy(dst_other.sha1, ren1->dst_entry->stages[other_stage].sha); + dst_other.mode = ren1->dst_entry->stages[other_stage].mode; try_merge = 0; if (sha_eq(src_other.sha1, null_sha1)) { -- cgit v1.2.3 From 09c01f856f65ffdde9a61708a5d74a372d68807a Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:28:48 -0600 Subject: merge-recursive: Rename conflict_rename_rename*() for clarity The names conflict_rename_rename and conflict_rename_rename_2 did not make it clear what they were handling. Since the first of these handles one file being renamed in both branches to different files, while the latter handles two different files being renamed to the same thing, add a little '1to2' and '2to1' suffix on these and an explanatory comment to make their intent clearer. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index f9531c8069..771564f7f4 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -730,12 +730,13 @@ static struct merge_file_info merge_file(struct merge_options *o, return result; } -static void conflict_rename_rename(struct merge_options *o, - struct rename *ren1, - const char *branch1, - struct rename *ren2, - const char *branch2) +static void conflict_rename_rename_1to2(struct merge_options *o, + struct rename *ren1, + const char *branch1, + struct rename *ren2, + const char *branch2) { + /* One file was renamed in both branches, but to different names. */ char *del[2]; int delp = 0; const char *ren1_dst = ren1->pair->two->path; @@ -782,12 +783,13 @@ static void conflict_rename_dir(struct merge_options *o, free(new_path); } -static void conflict_rename_rename_2(struct merge_options *o, - struct rename *ren1, - const char *branch1, - struct rename *ren2, - const char *branch2) +static void conflict_rename_rename_2to1(struct merge_options *o, + struct rename *ren1, + const char *branch1, + struct rename *ren2, + const char *branch2) { + /* Two files were renamed to the same thing. */ char *new_path1 = unique_path(o, ren1->pair->two->path, branch1); char *new_path2 = unique_path(o, ren2->pair->two->path, branch2); output(o, 1, "Renaming %s to %s and %s to %s instead", @@ -889,7 +891,7 @@ static int process_renames(struct merge_options *o, update_file(o, 0, ren1->pair->one->sha1, ren1->pair->one->mode, src); } - conflict_rename_rename(o, ren1, branch1, ren2, branch2); + conflict_rename_rename_1to2(o, ren1, branch1, ren2, branch2); } else { struct merge_file_info mfi; remove_file(o, 1, ren1_src, 1); @@ -1004,7 +1006,7 @@ static int process_renames(struct merge_options *o, "Rename %s->%s in %s", ren1_src, ren1_dst, branch1, ren2->pair->one->path, ren2->pair->two->path, branch2); - conflict_rename_rename_2(o, ren1, branch1, ren2, branch2); + conflict_rename_rename_2to1(o, ren1, branch1, ren2, branch2); } else try_merge = 1; -- cgit v1.2.3 From 605c1bcfc46b60f76dfb4f89f76029b7bb13d232 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:28:49 -0600 Subject: merge-recursive: Nuke rename/directory conflict detection Since we want to resolve merges in-core and then detect at the end whether D/F conflicts remain in the way, we should just apply renames in-core and let logic elsewhere check for D/F conflicts. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 18 ------------------ 1 file changed, 18 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 771564f7f4..9fde5a6f75 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -772,17 +772,6 @@ static void conflict_rename_rename_1to2(struct merge_options *o, free(del[delp]); } -static void conflict_rename_dir(struct merge_options *o, - struct rename *ren1, - const char *branch1) -{ - char *new_path = unique_path(o, ren1->pair->two->path, branch1); - output(o, 1, "Renaming %s to %s instead", ren1->pair->one->path, new_path); - remove_file(o, 0, ren1->pair->two->path, 0); - update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path); - free(new_path); -} - static void conflict_rename_rename_2to1(struct merge_options *o, struct rename *ren1, const char *branch1, @@ -1043,13 +1032,6 @@ static int process_renames(struct merge_options *o, if (!ren1->dst_entry->stages[2].mode != !ren1->dst_entry->stages[3].mode) ren1->dst_entry->processed = 0; - } else if (string_list_has_string(&o->current_directory_set, ren1_dst)) { - clean_merge = 0; - output(o, 1, "CONFLICT (rename/directory): Rename %s->%s in %s " - " directory %s added in %s", - ren1_src, ren1_dst, branch1, - ren1_dst, branch2); - conflict_rename_dir(o, ren1, branch1); } else { if (mfi.merge || !mfi.clean) output(o, 1, "Renaming %s => %s", ren1_src, ren1_dst); -- cgit v1.2.3 From 6ef2cb008f132e4649ef925a8e1950d3138054d5 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:28:50 -0600 Subject: merge-recursive: Move rename/delete handling into dedicated function This move is in preparation for the function growing and being called from multiple places in order to handle D/F conflicts. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 9fde5a6f75..c8ac6aebcb 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -730,6 +730,25 @@ static struct merge_file_info merge_file(struct merge_options *o, return result; } +static void conflict_rename_delete(struct merge_options *o, + struct diff_filepair *pair, + const char *rename_branch, + const char *other_branch) +{ + char *dest_name = pair->two->path; + + output(o, 1, "CONFLICT (rename/delete): Rename %s->%s in %s " + "and deleted in %s", + pair->one->path, pair->two->path, rename_branch, + other_branch); + if (!o->call_depth) + update_stages(dest_name, NULL, + rename_branch == o->branch1 ? pair->two : NULL, + rename_branch == o->branch1 ? NULL : pair->two, + 1); + update_file(o, 0, pair->two->sha1, pair->two->mode, dest_name); +} + static void conflict_rename_rename_1to2(struct merge_options *o, struct rename *ren1, const char *branch1, @@ -936,17 +955,7 @@ static int process_renames(struct merge_options *o, if (sha_eq(src_other.sha1, null_sha1)) { clean_merge = 0; - output(o, 1, "CONFLICT (rename/delete): Rename %s->%s in %s " - "and deleted in %s", - ren1_src, ren1_dst, branch1, - branch2); - update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst); - if (!o->call_depth) - update_stages(ren1_dst, NULL, - branch1 == o->branch1 ? - ren1->pair->two : NULL, - branch1 == o->branch1 ? - NULL : ren1->pair->two, 1); + conflict_rename_delete(o, ren1->pair, branch1, branch2); } else if ((dst_other.mode == ren1->pair->two->mode) && sha_eq(dst_other.sha1, ren1->pair->two->sha1)) { /* Added file on the other side -- cgit v1.2.3 From 5e3ce663b03bf14443ef8b4c60b407664f97040b Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:28:51 -0600 Subject: merge-recursive: Move delete/modify handling into dedicated function This move is in preparation for the function being called from multiple places in order to handle D/F conflicts. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index c8ac6aebcb..a8f68cf679 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1118,6 +1118,26 @@ error_return: return ret; } +static void handle_delete_modify(struct merge_options *o, + const char *path, + unsigned char *a_sha, int a_mode, + unsigned char *b_sha, int b_mode) +{ + if (!a_sha) { + output(o, 1, "CONFLICT (delete/modify): %s deleted in %s " + "and modified in %s. Version %s of %s left in tree.", + path, o->branch1, + o->branch2, o->branch2, path); + update_file(o, 0, b_sha, b_mode, path); + } else { + output(o, 1, "CONFLICT (delete/modify): %s deleted in %s " + "and modified in %s. Version %s of %s left in tree.", + path, o->branch2, + o->branch1, o->branch1, path); + update_file(o, 0, a_sha, a_mode, path); + } +} + /* Per entry merge function */ static int process_entry(struct merge_options *o, const char *path, struct stage_data *entry) @@ -1150,19 +1170,8 @@ static int process_entry(struct merge_options *o, } else { /* Deleted in one and changed in the other */ clean_merge = 0; - if (!a_sha) { - output(o, 1, "CONFLICT (delete/modify): %s deleted in %s " - "and modified in %s. Version %s of %s left in tree.", - path, o->branch1, - o->branch2, o->branch2, path); - update_file(o, 0, b_sha, b_mode, path); - } else { - output(o, 1, "CONFLICT (delete/modify): %s deleted in %s " - "and modified in %s. Version %s of %s left in tree.", - path, o->branch2, - o->branch1, o->branch1, path); - update_file(o, 0, a_sha, a_mode, path); - } + handle_delete_modify(o, path, + a_sha, a_mode, b_sha, b_mode); } } else if ((!o_sha && a_sha && !b_sha) || -- cgit v1.2.3 From 0c4918d1c1722c1faaa909f3f8486d5a0d816538 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:28:52 -0600 Subject: merge-recursive: Move process_entry's content merging into a function This move is in preparation for merge_content growing and being called from multiple places in order to handle D/F conflicts. I also snuck in a small change to the output in the case that the merged content for the file matches the current file contents, to make it better match (and thus more able to take over) how other merge_file() calls in process_renames() are handled. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 71 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 28 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index a8f68cf679..9c415c8e5a 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1138,6 +1138,47 @@ static void handle_delete_modify(struct merge_options *o, } } + +static int merge_content(struct merge_options *o, + const char *path, + unsigned char *o_sha, int o_mode, + unsigned char *a_sha, int a_mode, + unsigned char *b_sha, int b_mode) +{ + const char *reason = "content"; + struct merge_file_info mfi; + struct diff_filespec one, a, b; + + if (!o_sha) { + reason = "add/add"; + o_sha = (unsigned char *)null_sha1; + } + one.path = a.path = b.path = (char *)path; + hashcpy(one.sha1, o_sha); + one.mode = o_mode; + hashcpy(a.sha1, a_sha); + a.mode = a_mode; + hashcpy(b.sha1, b_sha); + b.mode = b_mode; + + mfi = merge_file(o, &one, &a, &b, o->branch1, o->branch2); + if (mfi.clean && sha_eq(mfi.sha, a_sha) && mfi.mode == a.mode) + output(o, 3, "Skipped %s (merged same as existing)", path); + else + output(o, 2, "Auto-merging %s", path); + + if (!mfi.clean) { + if (S_ISGITLINK(mfi.mode)) + reason = "submodule"; + output(o, 1, "CONFLICT (%s): Merge conflict in %s", + reason, path); + } + + update_file(o, mfi.clean, mfi.sha, mfi.mode, path); + return mfi.clean; + +} + /* Per entry merge function */ static int process_entry(struct merge_options *o, const char *path, struct stage_data *entry) @@ -1206,34 +1247,8 @@ static int process_entry(struct merge_options *o, } else if (a_sha && b_sha) { /* Case C: Added in both (check for same permissions) and */ /* case D: Modified in both, but differently. */ - const char *reason = "content"; - struct merge_file_info mfi; - struct diff_filespec one, a, b; - - if (!o_sha) { - reason = "add/add"; - o_sha = (unsigned char *)null_sha1; - } - output(o, 2, "Auto-merging %s", path); - one.path = a.path = b.path = (char *)path; - hashcpy(one.sha1, o_sha); - one.mode = o_mode; - hashcpy(a.sha1, a_sha); - a.mode = a_mode; - hashcpy(b.sha1, b_sha); - b.mode = b_mode; - - mfi = merge_file(o, &one, &a, &b, - o->branch1, o->branch2); - - clean_merge = mfi.clean; - if (!mfi.clean) { - if (S_ISGITLINK(mfi.mode)) - reason = "submodule"; - output(o, 1, "CONFLICT (%s): Merge conflict in %s", - reason, path); - } - update_file(o, mfi.clean, mfi.sha, mfi.mode, path); + clean_merge = merge_content(o, path, + o_sha, o_mode, a_sha, a_mode, b_sha, b_mode); } else if (!o_sha && !a_sha && !b_sha) { /* * this entry was deleted altogether. a_mode == 0 means -- cgit v1.2.3 From 25c3936349ba052f0c7e9ba3ce138b89abcae6ee Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:28:53 -0600 Subject: merge-recursive: New data structures for deferring of D/F conflicts Since we need to resolve paths (including renames) in-core first and defer checking of D/F conflicts (namely waiting to see if directories are still in the way after all paths are resolved) before updating files involved in D/F conflicts, we will need to first process_renames, then record some information about the rename needed at D/F resolution time, and then make use of that information when resolving D/F conflicts at the end. This commit adds some relevant data structures for storing the necessary information. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 9c415c8e5a..6ea4e3d2db 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -63,6 +63,22 @@ static int sha_eq(const unsigned char *a, const unsigned char *b) return a && b && hashcmp(a, b) == 0; } +enum rename_type { + RENAME_NORMAL = 0, + RENAME_DELETE, + RENAME_ONE_FILE_TO_TWO +}; + +struct rename_df_conflict_info { + enum rename_type rename_type; + struct diff_filepair *pair1; + struct diff_filepair *pair2; + const char *branch1; + const char *branch2; + struct stage_data *dst_entry1; + struct stage_data *dst_entry2; +}; + /* * Since we want to write the index eventually, we cannot reuse the index * for these (temporary) data. @@ -74,9 +90,37 @@ struct stage_data unsigned mode; unsigned char sha[20]; } stages[4]; + struct rename_df_conflict_info *rename_df_conflict_info; unsigned processed:1; }; +static inline void setup_rename_df_conflict_info(enum rename_type rename_type, + struct diff_filepair *pair1, + struct diff_filepair *pair2, + const char *branch1, + const char *branch2, + struct stage_data *dst_entry1, + struct stage_data *dst_entry2) +{ + struct rename_df_conflict_info *ci = xcalloc(1, sizeof(struct rename_df_conflict_info)); + ci->rename_type = rename_type; + ci->pair1 = pair1; + ci->branch1 = branch1; + ci->branch2 = branch2; + + ci->dst_entry1 = dst_entry1; + dst_entry1->rename_df_conflict_info = ci; + dst_entry1->processed = 0; + + assert(!pair2 == !dst_entry2); + if (dst_entry2) { + ci->dst_entry2 = dst_entry2; + ci->pair2 = pair2; + dst_entry2->rename_df_conflict_info = ci; + dst_entry2->processed = 0; + } +} + static int show(struct merge_options *o, int v) { return (!o->call_depth && o->verbosity >= v) || o->verbosity >= 5; -- cgit v1.2.3 From 2ff739f9d2fd2f34e7a7bfb9b5f2d1d2b2ec5326 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:28:54 -0600 Subject: merge-recursive: New function to assist resolving renames in-core only process_renames() and process_entry() have nearly identical code for doing three-way file merging to resolve content changes. Since we are already deferring some of the current rename handling in order to better handle D/F conflicts, it seems to make sense to defer content merging as well and remove the (nearly) duplicated code sections for handling this merging. To facilitate this process, add a new update_stages_and_entry() function which will map the higher stage index entries from two files involved in a rename into the resulting rename destination's index entries, and update the associated stage_data structure. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 6ea4e3d2db..2ba05a5b59 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -417,11 +417,10 @@ static struct string_list *get_renames(struct merge_options *o, return renames; } -static int update_stages(const char *path, struct diff_filespec *o, +static int update_stages_options(const char *path, struct diff_filespec *o, struct diff_filespec *a, struct diff_filespec *b, - int clear) + int clear, int options) { - int options = ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE; if (clear) if (remove_file_from_cache(path)) return -1; @@ -437,6 +436,34 @@ static int update_stages(const char *path, struct diff_filespec *o, return 0; } +static int update_stages(const char *path, struct diff_filespec *o, + struct diff_filespec *a, struct diff_filespec *b, + int clear) +{ + int options = ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE; + return update_stages_options(path, o, a, b, clear, options); +} + +static int update_stages_and_entry(const char *path, + struct stage_data *entry, + struct diff_filespec *o, + struct diff_filespec *a, + struct diff_filespec *b, + int clear) +{ + int options; + + entry->processed = 0; + entry->stages[1].mode = o->mode; + entry->stages[2].mode = a->mode; + entry->stages[3].mode = b->mode; + hashcpy(entry->stages[1].sha, o->sha1); + hashcpy(entry->stages[2].sha, a->sha1); + hashcpy(entry->stages[3].sha, b->sha1); + options = ADD_CACHE_OK_TO_ADD | ADD_CACHE_SKIP_DFCHECK; + return update_stages_options(path, o, a, b, clear, options); +} + static int remove_file(struct merge_options *o, int clean, const char *path, int no_wd) { -- cgit v1.2.3 From 384c166807e6eefbefa3e9f253216b2aee6d912a Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:28:55 -0600 Subject: merge-recursive: Have process_entry() skip D/F or rename entries If an entry has an associated rename_df_conflict_info, skip it and allow it to be processed by process_df_entry(). Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 2ba05a5b59..319780458e 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1267,6 +1267,9 @@ static int process_entry(struct merge_options *o, unsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode); unsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode); + if (entry->rename_df_conflict_info) + return 1; /* Such cases are handled elsewhere. */ + entry->processed = 1; if (o_sha && (!a_sha || !b_sha)) { /* Case A: Deleted in one */ -- cgit v1.2.3 From 9c0bbb50f0dd916fe1684991d10d93f3c67311ba Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:28:56 -0600 Subject: merge-recursive: Structure process_df_entry() to handle more cases Modify process_df_entry() (mostly just indentation level changes) to get it ready for handling more D/F conflict type cases. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 83 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 47 insertions(+), 36 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 319780458e..aa496cd360 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1336,13 +1336,19 @@ static int process_entry(struct merge_options *o, } /* - * Per entry merge function for D/F conflicts, to be called only after - * all files below dir have been processed. We do this because in the - * cases we can cleanly resolve D/F conflicts, process_entry() can clean - * out all the files below the directory for us. + * Per entry merge function for D/F (and/or rename) conflicts. In the + * cases we can cleanly resolve D/F conflicts, process_entry() can + * clean out all the files below the directory for us. All D/F + * conflict cases must be handled here at the end to make sure any + * directories that can be cleaned out, are. + * + * Some rename conflicts may also be handled here that don't necessarily + * involve D/F conflicts, since the code to handle them is generic enough + * to handle those rename conflicts with or without D/F conflicts also + * being involved. */ static int process_df_entry(struct merge_options *o, - const char *path, struct stage_data *entry) + const char *path, struct stage_data *entry) { int clean_merge = 1; unsigned o_mode = entry->stages[1].mode; @@ -1351,42 +1357,47 @@ static int process_df_entry(struct merge_options *o, unsigned char *o_sha = stage_sha(entry->stages[1].sha, o_mode); unsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode); unsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode); - const char *add_branch; - const char *other_branch; - unsigned mode; - const unsigned char *sha; - const char *conf; struct stat st; - if (!((!o_sha && a_sha && !b_sha) || (!o_sha && !a_sha && b_sha))) - return 1; /* we don't handle non D-F cases */ - entry->processed = 1; + if (entry->rename_df_conflict_info) { + die("Not yet implemented."); + } else if (!o_sha && !!a_sha != !!b_sha) { + /* directory -> (directory, file) */ + const char *add_branch; + const char *other_branch; + unsigned mode; + const unsigned char *sha; + const char *conf; - if (a_sha) { - add_branch = o->branch1; - other_branch = o->branch2; - mode = a_mode; - sha = a_sha; - conf = "file/directory"; - } else { - add_branch = o->branch2; - other_branch = o->branch1; - mode = b_mode; - sha = b_sha; - conf = "directory/file"; - } - if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) { - const char *new_path = unique_path(o, path, add_branch); - clean_merge = 0; - output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. " - "Adding %s as %s", - conf, path, other_branch, path, new_path); - remove_file(o, 0, path, 0); - update_file(o, 0, sha, mode, new_path); + if (a_sha) { + add_branch = o->branch1; + other_branch = o->branch2; + mode = a_mode; + sha = a_sha; + conf = "file/directory"; + } else { + add_branch = o->branch2; + other_branch = o->branch1; + mode = b_mode; + sha = b_sha; + conf = "directory/file"; + } + if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) { + const char *new_path = unique_path(o, path, add_branch); + clean_merge = 0; + output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. " + "Adding %s as %s", + conf, path, other_branch, path, new_path); + remove_file(o, 0, path, 0); + update_file(o, 0, sha, mode, new_path); + } else { + output(o, 2, "Adding %s", path); + update_file(o, 1, sha, mode, path); + } } else { - output(o, 2, "Adding %s", path); - update_file(o, 1, sha, mode, path); + entry->processed = 0; + return 1; /* not handled; assume clean until processed */ } return clean_merge; -- cgit v1.2.3 From 36de17048aaa3c3011c8b4ee1f89ee65b7ad2f99 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:28:57 -0600 Subject: merge-recursive: Update conflict_rename_rename_1to2() call signature To facilitate having this function called later using information stored in a rename_df_conflict_info struct, accept a diff_filepair instead of a rename. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index aa496cd360..691c97c333 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -821,16 +821,16 @@ static void conflict_rename_delete(struct merge_options *o, } static void conflict_rename_rename_1to2(struct merge_options *o, - struct rename *ren1, + struct diff_filepair *pair1, const char *branch1, - struct rename *ren2, + struct diff_filepair *pair2, const char *branch2) { /* One file was renamed in both branches, but to different names. */ char *del[2]; int delp = 0; - const char *ren1_dst = ren1->pair->two->path; - const char *ren2_dst = ren2->pair->two->path; + const char *ren1_dst = pair1->two->path; + const char *ren2_dst = pair2->two->path; const char *dst_name1 = ren1_dst; const char *dst_name2 = ren2_dst; if (string_list_has_string(&o->current_directory_set, ren1_dst)) { @@ -851,12 +851,12 @@ static void conflict_rename_rename_1to2(struct merge_options *o, /* * Uncomment to leave the conflicting names in the resulting tree * - * update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, dst_name1); - * update_file(o, 0, ren2->pair->two->sha1, ren2->pair->two->mode, dst_name2); + * update_file(o, 0, pair1->two->sha1, pair1->two->mode, dst_name1); + * update_file(o, 0, pair2->two->sha1, pair2->two->mode, dst_name2); */ } else { - update_stages(dst_name1, NULL, ren1->pair->two, NULL, 1); - update_stages(dst_name2, NULL, NULL, ren2->pair->two, 1); + update_stages(dst_name1, NULL, pair1->two, NULL, 1); + update_stages(dst_name2, NULL, NULL, pair2->two, 1); } while (delp--) free(del[delp]); @@ -970,7 +970,7 @@ static int process_renames(struct merge_options *o, update_file(o, 0, ren1->pair->one->sha1, ren1->pair->one->mode, src); } - conflict_rename_rename_1to2(o, ren1, branch1, ren2, branch2); + conflict_rename_rename_1to2(o, ren1->pair, branch1, ren2->pair, branch2); } else { struct merge_file_info mfi; remove_file(o, 1, ren1_src, 1); -- cgit v1.2.3 From 61b8bcae2d8f71c566150b0a12e35a9fdc76b82b Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:28:58 -0600 Subject: merge-recursive: Update merge_content() call signature Enable calling merge_content() and providing more information about renames and D/F conflicts (which we will want to do from process_df_entry()). Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 691c97c333..b9bc95a659 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1214,7 +1214,8 @@ static int merge_content(struct merge_options *o, const char *path, unsigned char *o_sha, int o_mode, unsigned char *a_sha, int a_mode, - unsigned char *b_sha, int b_mode) + unsigned char *b_sha, int b_mode, + const char *df_rename_conflict_branch) { const char *reason = "content"; struct merge_file_info mfi; @@ -1322,7 +1323,8 @@ static int process_entry(struct merge_options *o, /* Case C: Added in both (check for same permissions) and */ /* case D: Modified in both, but differently. */ clean_merge = merge_content(o, path, - o_sha, o_mode, a_sha, a_mode, b_sha, b_mode); + o_sha, o_mode, a_sha, a_mode, b_sha, b_mode, + NULL); } else if (!o_sha && !a_sha && !b_sha) { /* * this entry was deleted altogether. a_mode == 0 means -- cgit v1.2.3 From 2a669c341a857f53d946dec762e6f22a54b12dd2 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:28:59 -0600 Subject: merge-recursive: Avoid doubly merging rename/add conflict contents When a commit moves A to B while another commit created B (or moved C to B), and these two different commits serve as different merge-bases for a later merge, c94736a (merge-recursive: don't segfault while handling rename clashes 2009-07-30) added some special code to avoid segfaults. Since that commit, the two versions of B are merged in place (which could be potentially conflicting) and the intermediate result is used as the virtual ancestor. However, right before this special merge, try_merge was turned on, meaning that process_renames() would try an alternative merge that ignores the 'add' part of the conflict, and, if the merge is clean, store that as the new virtual ancestor. This could cause incorrect merging of criss-cross merges; it would typically result in just recording a slightly confusing merge base, but in some cases it could cause silent acceptance of one side of a merge as the final resolution when a conflict should have been flagged. When we do a special merge for such a rename/add conflict between merge-bases, turn try_merge off to avoid an inappropriate second merge. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 1 + 1 file changed, 1 insertion(+) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index b9bc95a659..6ffc67ade4 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1061,6 +1061,7 @@ static int process_renames(struct merge_options *o, mfi.sha, mfi.mode, ren1_dst); + try_merge = 0; } else { new_path = unique_path(o, ren1_dst, branch2); output(o, 1, "Adding as %s instead", new_path); -- cgit v1.2.3 From 07413c5a3115df424bee1ebe627676df0734f787 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:29:00 -0600 Subject: merge-recursive: Move handling of double rename of one file to two Move the handling of rename/rename conflicts where one file is renamed to two different files, from process_renames() to process_df_entry(). Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 57 +++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 16 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 6ffc67ade4..d0c68ce2b4 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -855,8 +855,11 @@ static void conflict_rename_rename_1to2(struct merge_options *o, * update_file(o, 0, pair2->two->sha1, pair2->two->mode, dst_name2); */ } else { - update_stages(dst_name1, NULL, pair1->two, NULL, 1); - update_stages(dst_name2, NULL, NULL, pair2->two, 1); + update_stages(ren1_dst, NULL, pair1->two, NULL, 1); + update_stages(ren2_dst, NULL, NULL, pair2->two, 1); + + update_file(o, 0, pair1->two->sha1, pair1->two->mode, dst_name1); + update_file(o, 0, pair2->two->sha1, pair2->two->mode, dst_name2); } while (delp--) free(del[delp]); @@ -958,19 +961,15 @@ static int process_renames(struct merge_options *o, ren2->dst_entry->processed = 1; ren2->processed = 1; if (strcmp(ren1_dst, ren2_dst) != 0) { - clean_merge = 0; - output(o, 1, "CONFLICT (rename/rename): " - "Rename \"%s\"->\"%s\" in branch \"%s\" " - "rename \"%s\"->\"%s\" in \"%s\"%s", - src, ren1_dst, branch1, - src, ren2_dst, branch2, - o->call_depth ? " (left unresolved)": ""); - if (o->call_depth) { - remove_file_from_cache(src); - update_file(o, 0, ren1->pair->one->sha1, - ren1->pair->one->mode, src); - } - conflict_rename_rename_1to2(o, ren1->pair, branch1, ren2->pair, branch2); + setup_rename_df_conflict_info(RENAME_ONE_FILE_TO_TWO, + ren1->pair, + ren2->pair, + branch1, + branch2, + ren1->dst_entry, + ren2->dst_entry); + remove_file(o, 0, ren1_dst, 0); + /* ren2_dst not in head, so no need to delete */ } else { struct merge_file_info mfi; remove_file(o, 1, ren1_src, 1); @@ -1364,7 +1363,33 @@ static int process_df_entry(struct merge_options *o, entry->processed = 1; if (entry->rename_df_conflict_info) { - die("Not yet implemented."); + struct rename_df_conflict_info *conflict_info = entry->rename_df_conflict_info; + char *src; + switch (conflict_info->rename_type) { + case RENAME_ONE_FILE_TO_TWO: + src = conflict_info->pair1->one->path; + clean_merge = 0; + output(o, 1, "CONFLICT (rename/rename): " + "Rename \"%s\"->\"%s\" in branch \"%s\" " + "rename \"%s\"->\"%s\" in \"%s\"%s", + src, conflict_info->pair1->two->path, conflict_info->branch1, + src, conflict_info->pair2->two->path, conflict_info->branch2, + o->call_depth ? " (left unresolved)" : ""); + if (o->call_depth) { + remove_file_from_cache(src); + update_file(o, 0, conflict_info->pair1->one->sha1, + conflict_info->pair1->one->mode, src); + } + conflict_rename_rename_1to2(o, conflict_info->pair1, + conflict_info->branch1, + conflict_info->pair2, + conflict_info->branch2); + conflict_info->dst_entry2->processed = 1; + break; + default: + entry->processed = 0; + break; + } } else if (!o_sha && !!a_sha != !!b_sha) { /* directory -> (directory, file) */ const char *add_branch; -- cgit v1.2.3 From 161cf7f9490285700432f23a953f8e238f3469f5 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:29:01 -0600 Subject: merge-recursive: Move handling of double rename of one file to other file Move the handling of rename/rename conflicts where one file is renamed on both sides to the same file, from process_renames() to process_entry(). Here we avoid the three way merge logic by just using update_stages_and_entry() to move the higher stage entries in the index from the rename source to the rename destination, and then allow process_entry() to do its magic. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 32 ++++++-------------------------- 1 file changed, 6 insertions(+), 26 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index d0c68ce2b4..05d06f720e 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -971,33 +971,13 @@ static int process_renames(struct merge_options *o, remove_file(o, 0, ren1_dst, 0); /* ren2_dst not in head, so no need to delete */ } else { - struct merge_file_info mfi; remove_file(o, 1, ren1_src, 1); - mfi = merge_file(o, - ren1->pair->one, - ren1->pair->two, - ren2->pair->two, - branch1, - branch2); - if (mfi.merge || !mfi.clean) - output(o, 1, "Renaming %s->%s", src, ren1_dst); - - if (mfi.merge) - output(o, 2, "Auto-merging %s", ren1_dst); - - if (!mfi.clean) { - output(o, 1, "CONFLICT (content): merge conflict in %s", - ren1_dst); - clean_merge = 0; - - if (!o->call_depth) - update_stages(ren1_dst, - ren1->pair->one, - ren1->pair->two, - ren2->pair->two, - 1 /* clear */); - } - update_file(o, mfi.clean, mfi.sha, mfi.mode, ren1_dst); + update_stages_and_entry(ren1_dst, + ren1->dst_entry, + ren1->pair->one, + ren1->pair->two, + ren2->pair->two, + 1 /* clear */); } } else { /* Renamed in 1, maybe changed in 2 */ -- cgit v1.2.3 From 3b130adf9c8b0b37acb0959b84a3222bc22ebcff Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:29:02 -0600 Subject: merge-recursive: Delay handling of rename/delete conflicts Move the handling of rename/delete conflicts from process_renames() to process_df_entry(). Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 05d06f720e..92b2b849fa 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1004,8 +1004,20 @@ static int process_renames(struct merge_options *o, try_merge = 0; if (sha_eq(src_other.sha1, null_sha1)) { - clean_merge = 0; - conflict_rename_delete(o, ren1->pair, branch1, branch2); + if (string_list_has_string(&o->current_directory_set, ren1_dst)) { + ren1->dst_entry->processed = 0; + setup_rename_df_conflict_info(RENAME_DELETE, + ren1->pair, + NULL, + branch1, + branch2, + ren1->dst_entry, + NULL); + remove_file(o, 0, ren1_dst, 0); + } else { + clean_merge = 0; + conflict_rename_delete(o, ren1->pair, branch1, branch2); + } } else if ((dst_other.mode == ren1->pair->two->mode) && sha_eq(dst_other.sha1, ren1->pair->two->sha1)) { /* Added file on the other side @@ -1346,6 +1358,12 @@ static int process_df_entry(struct merge_options *o, struct rename_df_conflict_info *conflict_info = entry->rename_df_conflict_info; char *src; switch (conflict_info->rename_type) { + case RENAME_DELETE: + clean_merge = 0; + conflict_rename_delete(o, conflict_info->pair1, + conflict_info->branch1, + conflict_info->branch2); + break; case RENAME_ONE_FILE_TO_TWO: src = conflict_info->pair1->one->path; clean_merge = 0; -- cgit v1.2.3 From 882fd11aff6f0e8add77e75924678cce875a0eaf Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:29:03 -0600 Subject: merge-recursive: Delay content merging for renames Move the handling of content merging for renames from process_renames() to process_df_entry(). Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 51 +++++++++++++++------------------------------------ 1 file changed, 15 insertions(+), 36 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 92b2b849fa..0ca54bd882 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1073,7 +1073,6 @@ static int process_renames(struct merge_options *o, if (try_merge) { struct diff_filespec *one, *a, *b; - struct merge_file_info mfi; src_other.path = (char *)ren1_src; one = ren1->pair->one; @@ -1084,41 +1083,16 @@ static int process_renames(struct merge_options *o, b = ren1->pair->two; a = &src_other; } - mfi = merge_file(o, one, a, b, - o->branch1, o->branch2); - - if (mfi.clean && - sha_eq(mfi.sha, ren1->pair->two->sha1) && - mfi.mode == ren1->pair->two->mode) { - /* - * This message is part of - * t6022 test. If you change - * it update the test too. - */ - output(o, 3, "Skipped %s (merged same as existing)", ren1_dst); - - /* There may be higher stage entries left - * in the index (e.g. due to a D/F - * conflict) that need to be resolved. - */ - if (!ren1->dst_entry->stages[2].mode != - !ren1->dst_entry->stages[3].mode) - ren1->dst_entry->processed = 0; - } else { - if (mfi.merge || !mfi.clean) - output(o, 1, "Renaming %s => %s", ren1_src, ren1_dst); - if (mfi.merge) - output(o, 2, "Auto-merging %s", ren1_dst); - if (!mfi.clean) { - output(o, 1, "CONFLICT (rename/modify): Merge conflict in %s", - ren1_dst); - clean_merge = 0; - - if (!o->call_depth) - update_stages(ren1_dst, - one, a, b, 1); - } - update_file(o, mfi.clean, mfi.sha, mfi.mode, ren1_dst); + update_stages_and_entry(ren1_dst, ren1->dst_entry, one, a, b, 1); + if (string_list_has_string(&o->current_directory_set, ren1_dst)) { + setup_rename_df_conflict_info(RENAME_NORMAL, + ren1->pair, + NULL, + branch1, + NULL, + ren1->dst_entry, + NULL); + remove_file(o, 0, ren1_dst, 0); } } } @@ -1358,6 +1332,11 @@ static int process_df_entry(struct merge_options *o, struct rename_df_conflict_info *conflict_info = entry->rename_df_conflict_info; char *src; switch (conflict_info->rename_type) { + case RENAME_NORMAL: + clean_merge = merge_content(o, path, + o_sha, o_mode, a_sha, a_mode, b_sha, b_mode, + conflict_info->branch1); + break; case RENAME_DELETE: clean_merge = 0; conflict_rename_delete(o, conflict_info->pair1, -- cgit v1.2.3 From 71f7ffcc02a2f38436c59146b86897389130a538 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:29:04 -0600 Subject: merge-recursive: Delay modify/delete conflicts if D/F conflict present When handling merges with modify/delete conflicts, if the modified path is involved in a D/F conflict, handle the issue in process_df_entry() rather than process_entry(). Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 0ca54bd882..ffcecc7f49 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1249,6 +1249,10 @@ static int process_entry(struct merge_options *o, output(o, 2, "Removing %s", path); /* do not touch working file if it did not exist */ remove_file(o, 1, path, !a_sha); + } else if (string_list_has_string(&o->current_directory_set, + path)) { + entry->processed = 0; + return 1; /* Assume clean till processed */ } else { /* Deleted in one and changed in the other */ clean_merge = 0; @@ -1367,6 +1371,11 @@ static int process_df_entry(struct merge_options *o, entry->processed = 0; break; } + } else if (o_sha && (!a_sha || !b_sha)) { + /* Modify/delete; deleted side may have put a directory in the way */ + clean_merge = 0; + handle_delete_modify(o, path, + a_sha, a_mode, b_sha, b_mode); } else if (!o_sha && !!a_sha != !!b_sha) { /* directory -> (directory, file) */ const char *add_branch; -- cgit v1.2.3 From a0de2f6bd3b79f7ab61ea90f795b964a7f7f3d6d Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:29:05 -0600 Subject: conflict_rename_delete(): Check whether D/F conflicts are still present If all the paths below some directory involved in a D/F conflict were not removed during the rest of the merge, then the contents of the file whose path conflicted needs to be recorded in file with an alternative filename. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index ffcecc7f49..09ce327c17 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -807,6 +807,8 @@ static void conflict_rename_delete(struct merge_options *o, const char *other_branch) { char *dest_name = pair->two->path; + int df_conflict = 0; + struct stat st; output(o, 1, "CONFLICT (rename/delete): Rename %s->%s in %s " "and deleted in %s", @@ -817,7 +819,13 @@ static void conflict_rename_delete(struct merge_options *o, rename_branch == o->branch1 ? pair->two : NULL, rename_branch == o->branch1 ? NULL : pair->two, 1); + if (lstat(dest_name, &st) == 0 && S_ISDIR(st.st_mode)) { + dest_name = unique_path(o, dest_name, rename_branch); + df_conflict = 1; + } update_file(o, 0, pair->two->sha1, pair->two->mode, dest_name); + if (df_conflict) + free(dest_name); } static void conflict_rename_rename_1to2(struct merge_options *o, -- cgit v1.2.3 From 2adc7dcc111636ed16601dc7516ced1c5cfda088 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:29:06 -0600 Subject: conflict_rename_rename_1to2(): Fix checks for presence of D/F conflicts This function is called from process_df_entry(), near the end of the merge. Rather than just checking whether one of the sides of the merge had a directory at the same path as one of our files, check whether that directory is still present by this point of our merge. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 09ce327c17..85d69eb567 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -841,17 +841,16 @@ static void conflict_rename_rename_1to2(struct merge_options *o, const char *ren2_dst = pair2->two->path; const char *dst_name1 = ren1_dst; const char *dst_name2 = ren2_dst; - if (string_list_has_string(&o->current_directory_set, ren1_dst)) { + struct stat st; + if (lstat(ren1_dst, &st) == 0 && S_ISDIR(st.st_mode)) { dst_name1 = del[delp++] = unique_path(o, ren1_dst, branch1); output(o, 1, "%s is a directory in %s adding as %s instead", ren1_dst, branch2, dst_name1); - remove_file(o, 0, ren1_dst, 0); } - if (string_list_has_string(&o->current_directory_set, ren2_dst)) { + if (lstat(ren2_dst, &st) == 0 && S_ISDIR(st.st_mode)) { dst_name2 = del[delp++] = unique_path(o, ren2_dst, branch2); output(o, 1, "%s is a directory in %s adding as %s instead", ren2_dst, branch1, dst_name2); - remove_file(o, 0, ren2_dst, 0); } if (o->call_depth) { remove_file_from_cache(dst_name1); -- cgit v1.2.3 From 4ab9a157d06956ce5a1060a28404cbade3039fa2 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:29:07 -0600 Subject: merge_content(): Check whether D/F conflicts are still present If all the paths below some directory involved in a D/F conflict were not removed during the rest of the merge, then the contents of the file whose path conflicted needs to be recorded in file with an alternative filename. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 85d69eb567..a3f986d874 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1193,6 +1193,8 @@ static int merge_content(struct merge_options *o, const char *reason = "content"; struct merge_file_info mfi; struct diff_filespec one, a, b; + struct stat st; + unsigned df_conflict_remains = 0; if (!o_sha) { reason = "add/add"; @@ -1207,7 +1209,13 @@ static int merge_content(struct merge_options *o, b.mode = b_mode; mfi = merge_file(o, &one, &a, &b, o->branch1, o->branch2); - if (mfi.clean && sha_eq(mfi.sha, a_sha) && mfi.mode == a.mode) + if (df_rename_conflict_branch && + lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) { + df_conflict_remains = 1; + } + + if (mfi.clean && !df_conflict_remains && + sha_eq(mfi.sha, a_sha) && mfi.mode == a.mode) output(o, 3, "Skipped %s (merged same as existing)", path); else output(o, 2, "Auto-merging %s", path); @@ -1219,7 +1227,17 @@ static int merge_content(struct merge_options *o, reason, path); } - update_file(o, mfi.clean, mfi.sha, mfi.mode, path); + if (df_conflict_remains) { + const char *new_path; + update_file_flags(o, mfi.sha, mfi.mode, path, + o->call_depth || mfi.clean, 0); + new_path = unique_path(o, path, df_rename_conflict_branch); + mfi.clean = 0; + output(o, 1, "Adding as %s instead", new_path); + update_file_flags(o, mfi.sha, mfi.mode, new_path, 0, 1); + } else { + update_file(o, mfi.clean, mfi.sha, mfi.mode, path); + } return mfi.clean; } -- cgit v1.2.3 From 84a08a47b9559e76df96645c536845f31ba4dc7b Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:29:08 -0600 Subject: handle_delete_modify(): Check whether D/F conflicts are still present If all the paths below some directory involved in a D/F conflict were not removed during the rest of the merge, then the contents of the file whose path conflicted needs to be recorded in file with an alternative filename. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index a3f986d874..0271d16c66 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1164,25 +1164,29 @@ error_return: static void handle_delete_modify(struct merge_options *o, const char *path, + const char *new_path, unsigned char *a_sha, int a_mode, unsigned char *b_sha, int b_mode) { if (!a_sha) { output(o, 1, "CONFLICT (delete/modify): %s deleted in %s " - "and modified in %s. Version %s of %s left in tree.", + "and modified in %s. Version %s of %s left in tree%s%s.", path, o->branch1, - o->branch2, o->branch2, path); - update_file(o, 0, b_sha, b_mode, path); + o->branch2, o->branch2, path, + path == new_path ? "" : " at ", + path == new_path ? "" : new_path); + update_file(o, 0, b_sha, b_mode, new_path); } else { output(o, 1, "CONFLICT (delete/modify): %s deleted in %s " - "and modified in %s. Version %s of %s left in tree.", + "and modified in %s. Version %s of %s left in tree%s%s.", path, o->branch2, - o->branch1, o->branch1, path); - update_file(o, 0, a_sha, a_mode, path); + o->branch1, o->branch1, path, + path == new_path ? "" : " at ", + path == new_path ? "" : new_path); + update_file(o, 0, a_sha, a_mode, new_path); } } - static int merge_content(struct merge_options *o, const char *path, unsigned char *o_sha, int o_mode, @@ -1281,7 +1285,7 @@ static int process_entry(struct merge_options *o, } else { /* Deleted in one and changed in the other */ clean_merge = 0; - handle_delete_modify(o, path, + handle_delete_modify(o, path, path, a_sha, a_mode, b_sha, b_mode); } @@ -1398,8 +1402,11 @@ static int process_df_entry(struct merge_options *o, } } else if (o_sha && (!a_sha || !b_sha)) { /* Modify/delete; deleted side may have put a directory in the way */ + const char *new_path = path; + if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) + new_path = unique_path(o, path, a_sha ? o->branch1 : o->branch2); clean_merge = 0; - handle_delete_modify(o, path, + handle_delete_modify(o, path, new_path, a_sha, a_mode, b_sha, b_mode); } else if (!o_sha && !!a_sha != !!b_sha) { /* directory -> (directory, file) */ -- cgit v1.2.3 From ef02b317212443036be6007bba3a1a5946460dc9 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:29:09 -0600 Subject: merge-recursive: Make room for directories in D/F conflicts When there are unmerged entries present, make sure to check for D/F conflicts first and remove any files present in HEAD that would be in the way of creating files below the correspondingly named directory. Such files will be processed again at the end of the merge in process_df_entry(); at that time we will be able to tell if we need to and can reinstate the file, whether we need to place its contents in a different file due to the directory still being present, etc. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 0271d16c66..38514629bd 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -346,6 +346,63 @@ static struct string_list *get_unmerged(void) return unmerged; } +static void make_room_for_directories_of_df_conflicts(struct merge_options *o, + struct string_list *entries) +{ + /* If there are D/F conflicts, and the paths currently exist + * in the working copy as a file, we want to remove them to + * make room for the corresponding directory. Such paths will + * later be processed in process_df_entry() at the end. If + * the corresponding directory ends up being removed by the + * merge, then the file will be reinstated at that time; + * otherwise, if the file is not supposed to be removed by the + * merge, the contents of the file will be placed in another + * unique filename. + * + * NOTE: This function relies on the fact that entries for a + * D/F conflict will appear adjacent in the index, with the + * entries for the file appearing before entries for paths + * below the corresponding directory. + */ + const char *last_file = NULL; + int last_len; + struct stage_data *last_e; + int i; + + for (i = 0; i < entries->nr; i++) { + const char *path = entries->items[i].string; + int len = strlen(path); + struct stage_data *e = entries->items[i].util; + + /* + * Check if last_file & path correspond to a D/F conflict; + * i.e. whether path is last_file+'/'+. + * If so, remove last_file to make room for path and friends. + */ + if (last_file && + len > last_len && + memcmp(path, last_file, last_len) == 0 && + path[last_len] == '/') { + output(o, 3, "Removing %s to make room for subdirectory; may re-add later.", last_file); + unlink(last_file); + } + + /* + * Determine whether path could exist as a file in the + * working directory as a possible D/F conflict. This + * will only occur when it exists in stage 2 as a + * file. + */ + if (S_ISREG(e->stages[2].mode) || S_ISLNK(e->stages[2].mode)) { + last_file = path; + last_len = len; + last_e = e; + } else { + last_file = NULL; + } + } +} + struct rename { struct diff_filepair *pair; @@ -1488,6 +1545,7 @@ int merge_trees(struct merge_options *o, get_files_dirs(o, merge); entries = get_unmerged(); + make_room_for_directories_of_df_conflicts(o, entries); re_head = get_renames(o, head, common, head, merge, entries); re_merge = get_renames(o, merge, common, head, merge, entries); clean = process_renames(o, re_head, re_merge); -- cgit v1.2.3 From 4c0c1810c98e20ce05b2f026d6f9d62cfef7aefc Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 20 Sep 2010 02:29:10 -0600 Subject: merge-recursive: Remove redundant path clearing for D/F conflicts The code had several places where individual checks were done to remove files that could be in the way of directories in D/F conflicts. Not all D/F conflicts could have a path cleared for them in such a manner, however, leading to the need to create make_room_for_directories_of_df_conflicts() as done in the previous patch. That new function could not have been incorporated into the code sooner, since not all relevant code paths had been deferred to process_df_entry() yet, leading to the creation of even more of these now-redundant path removals. Clean out all of these extra D/F path clearing cases. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 38514629bd..bae98845ce 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1032,8 +1032,6 @@ static int process_renames(struct merge_options *o, branch2, ren1->dst_entry, ren2->dst_entry); - remove_file(o, 0, ren1_dst, 0); - /* ren2_dst not in head, so no need to delete */ } else { remove_file(o, 1, ren1_src, 1); update_stages_and_entry(ren1_dst, @@ -1077,7 +1075,6 @@ static int process_renames(struct merge_options *o, branch2, ren1->dst_entry, NULL); - remove_file(o, 0, ren1_dst, 0); } else { clean_merge = 0; conflict_rename_delete(o, ren1->pair, branch1, branch2); @@ -1156,7 +1153,6 @@ static int process_renames(struct merge_options *o, NULL, ren1->dst_entry, NULL); - remove_file(o, 0, ren1_dst, 0); } } } @@ -1338,7 +1334,7 @@ static int process_entry(struct merge_options *o, } else if (string_list_has_string(&o->current_directory_set, path)) { entry->processed = 0; - return 1; /* Assume clean till processed */ + return 1; /* Assume clean until processed */ } else { /* Deleted in one and changed in the other */ clean_merge = 0; @@ -1362,15 +1358,7 @@ static int process_entry(struct merge_options *o, if (string_list_has_string(&o->current_directory_set, path)) { /* Handle D->F conflicts after all subfiles */ entry->processed = 0; - /* But get any file out of the way now, so conflicted - * entries below the directory of the same name can - * be put in the working directory. - */ - if (a_sha) - output(o, 2, "Removing %s", path); - /* do not touch working file if it did not exist */ - remove_file(o, 0, path, !a_sha); - return 1; /* Assume clean till processed */ + return 1; /* Assume clean until processed */ } else { output(o, 2, "Adding %s", path); update_file(o, 1, sha, mode, path); @@ -1492,7 +1480,6 @@ static int process_df_entry(struct merge_options *o, output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. " "Adding %s as %s", conf, path, other_branch, path, new_path); - remove_file(o, 0, path, 0); update_file(o, 0, sha, mode, new_path); } else { output(o, 2, "Adding %s", path); -- cgit v1.2.3 From c8516500b1ee6025466a207cd86dc30421c3b6e6 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 21 Oct 2010 07:34:33 -0700 Subject: merge-recursive:make_room_for_directories - work around dumb compilers Some vintage of gcc does not seem to notice last_len is only used when last_file is already set to non-NULL at which point last_len is also set. Noticed on FreeBSD 8 Signed-off-by: Junio C Hamano --- merge-recursive.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index bae98845ce..231e5cbd7f 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -365,7 +365,7 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o, * below the corresponding directory. */ const char *last_file = NULL; - int last_len; + int last_len = 0; struct stage_data *last_e; int i; -- cgit v1.2.3