From c43ba42e8d3b3b85e322c36d35053f650835ce0a Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:19:49 -0600 Subject: merge-recursive: Make BUG message more legible by adding a newline Hopefully no one ever hits this error except when making large changes to merge-recursive.c and debugging... Signed-off-by: Elijah Newren 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 ae6ade4ecb..d6f238dab0 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -230,7 +230,7 @@ struct tree *write_tree_from_memory(struct merge_options *o) for (i = 0; i < active_nr; i++) { struct cache_entry *ce = active_cache[i]; if (ce_stage(ce)) - fprintf(stderr, "BUG: %d %.*s", ce_stage(ce), + fprintf(stderr, "BUG: %d %.*s\n", ce_stage(ce), (int)ce_namelen(ce), ce->name); } die("Bug in merge-recursive.c"); -- cgit v1.2.1 From abafc88e76dc731e340b8ec0674b38b05d43b4f7 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:19:50 -0600 Subject: merge-recursive: Correct a comment Signed-off-by: Elijah Newren 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 d6f238dab0..a7ba9b0aa4 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1437,7 +1437,7 @@ static int process_df_entry(struct merge_options *o, 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) */ + /* directory -> (directory, file) or -> (directory, file) */ const char *add_branch; const char *other_branch; unsigned mode; -- cgit v1.2.1 From 0c05942087a14be4560d09299fb57117308a69d5 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:19:51 -0600 Subject: merge-recursive: Mark some diff_filespec struct arguments const Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index a7ba9b0aa4..8ad4c7ef38 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -459,9 +459,10 @@ static struct string_list *get_renames(struct merge_options *o, return renames; } -static int update_stages_options(const char *path, struct diff_filespec *o, - struct diff_filespec *a, struct diff_filespec *b, - int clear, int options) +static int update_stages_options(const char *path, const struct diff_filespec *o, + const struct diff_filespec *a, + const struct diff_filespec *b, + int clear, int options) { if (clear) if (remove_file_from_cache(path)) @@ -710,9 +711,9 @@ struct merge_file_info { static int merge_3way(struct merge_options *o, mmbuffer_t *result_buf, - struct diff_filespec *one, - struct diff_filespec *a, - struct diff_filespec *b, + const struct diff_filespec *one, + const struct diff_filespec *a, + const struct diff_filespec *b, const char *branch1, const char *branch2) { @@ -770,9 +771,9 @@ static int merge_3way(struct merge_options *o, } static struct merge_file_info merge_file(struct merge_options *o, - struct diff_filespec *one, - struct diff_filespec *a, - struct diff_filespec *b, + const struct diff_filespec *one, + const struct diff_filespec *a, + const struct diff_filespec *b, const char *branch1, const char *branch2) { -- cgit v1.2.1 From 650467cf89e61815cfa1c942544a3659eda88aeb Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:19:52 -0600 Subject: merge-recursive: Consolidate different update_stages functions We are only calling update_stages_options() one way really, so we can consolidate the slightly different variants into one and remove some parameters whose values are always the same. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 8ad4c7ef38..368a498a51 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -459,11 +459,12 @@ static struct string_list *get_renames(struct merge_options *o, return renames; } -static int update_stages_options(const char *path, const struct diff_filespec *o, - const struct diff_filespec *a, - const struct diff_filespec *b, - int clear, int options) +static int update_stages(const char *path, const struct diff_filespec *o, + const struct diff_filespec *a, + const struct diff_filespec *b) { + int clear = 1; + int options = ADD_CACHE_OK_TO_ADD | ADD_CACHE_SKIP_DFCHECK; if (clear) if (remove_file_from_cache(path)) return -1; @@ -479,14 +480,6 @@ static int update_stages_options(const char *path, const 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, @@ -503,8 +496,7 @@ static int update_stages_and_entry(const char *path, 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); + return update_stages(path, o, a, b); } static int remove_file(struct merge_options *o, int clean, @@ -860,8 +852,7 @@ static void conflict_rename_delete(struct merge_options *o, if (!o->call_depth) update_stages(dest_name, NULL, rename_branch == o->branch1 ? pair->two : NULL, - rename_branch == o->branch1 ? NULL : pair->two, - 1); + rename_branch == o->branch1 ? NULL : pair->two); if (lstat(dest_name, &st) == 0 && S_ISDIR(st.st_mode)) { dest_name = unique_path(o, dest_name, rename_branch); df_conflict = 1; @@ -905,8 +896,8 @@ 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(ren1_dst, NULL, pair1->two, NULL, 1); - update_stages(ren2_dst, NULL, NULL, pair2->two, 1); + update_stages(ren1_dst, NULL, pair1->two, NULL); + update_stages(ren2_dst, NULL, NULL, pair2->two); update_file(o, 0, pair1->two->sha1, pair1->two->mode, dst_name1); update_file(o, 0, pair2->two->sha1, pair2->two->mode, dst_name2); -- cgit v1.2.1 From 3d6b8e884c45e65e0abda431690cd4b3bcaf04f0 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:19:53 -0600 Subject: merge-recursive: Remember to free generated unique path names Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 368a498a51..f172df1695 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1061,7 +1061,6 @@ static int process_renames(struct merge_options *o, renamed: clean merge */ update_file(o, 1, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst); } else if (!sha_eq(dst_other.sha1, null_sha1)) { - const char *new_path; clean_merge = 0; try_merge = 1; output(o, 1, "CONFLICT (rename/add): Rename %s->%s in %s. " @@ -1090,9 +1089,10 @@ static int process_renames(struct merge_options *o, ren1_dst); try_merge = 0; } else { - new_path = unique_path(o, ren1_dst, branch2); + char *new_path = unique_path(o, ren1_dst, branch2); output(o, 1, "Adding as %s instead", new_path); update_file(o, 0, dst_other.sha1, dst_other.mode, new_path); + free(new_path); } } else if ((item = string_list_lookup(renames2Dst, ren1_dst))) { ren2 = item->util; @@ -1260,13 +1260,14 @@ static int merge_content(struct merge_options *o, } if (df_conflict_remains) { - const char *new_path; + 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); + free(new_path); } else { update_file(o, mfi.clean, mfi.sha, mfi.mode, path); } @@ -1422,12 +1423,14 @@ 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); + char *renamed = NULL; + if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) { + renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2); + } clean_merge = 0; - handle_delete_modify(o, path, new_path, + handle_delete_modify(o, path, renamed ? renamed : path, a_sha, a_mode, b_sha, b_mode); + free(renamed); } else if (!o_sha && !!a_sha != !!b_sha) { /* directory -> (directory, file) or -> (directory, file) */ const char *add_branch; @@ -1450,12 +1453,13 @@ static int process_df_entry(struct merge_options *o, conf = "directory/file"; } if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) { - const char *new_path = unique_path(o, path, add_branch); + 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); update_file(o, 0, sha, mode, new_path); + free(new_path); } else { output(o, 2, "Adding %s", path); update_file(o, 1, sha, mode, path); -- cgit v1.2.1 From 0b30e8125130a78bc7c0e13e7f45ba105bd206b8 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:19:54 -0600 Subject: merge-recursive: Avoid working directory changes during recursive case make_room_for_directories_of_df_conflicts() is about making sure necessary working directory changes can succeed. When o->call_depth > 0 (i.e. the recursive case), we do not want to make any working directory changes so this function should be skipped. Note that make_room_for_directories_of_df_conflicts() is broken as has been pointed out by Junio; it should NOT be unlinking files. What it should do is keep track of files that could be unlinked if a directory later needs to be written in their place. However, that work also is only relevant in the non-recursive case, so this change is helpful either way. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index f172df1695..e6a6a81ec2 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -353,6 +353,13 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o, int last_len = 0; int i; + /* + * If we're merging merge-bases, we don't want to bother with + * any working directory changes. + */ + if (o->call_depth) + return; + for (i = 0; i < entries->nr; i++) { const char *path = entries->items[i].string; int len = strlen(path); -- cgit v1.2.1 From 7b1c610f84a46fa237627b3307707afb520e555c Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:19:55 -0600 Subject: merge-recursive: Fix recursive case with D/F conflict via add/add conflict When a D/F conflict is introduced via an add/add conflict, when o->call_depth > 0 we need to ensure that the higher stage entry from the base stage is removed. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index e6a6a81ec2..418246376b 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1466,6 +1466,8 @@ static int process_df_entry(struct merge_options *o, "Adding %s as %s", conf, path, other_branch, path, new_path); update_file(o, 0, sha, mode, new_path); + if (o->call_depth) + remove_file_from_cache(path); free(new_path); } else { output(o, 2, "Adding %s", path); -- cgit v1.2.1 From f0fd4d05e8a17fe5ccdd4d3edd686bc6702b8144 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:19:56 -0600 Subject: merge-recursive: Fix sorting order and directory change assumptions We cannot assume that directory/file conflicts will appear in sorted order; for example, 'letters.txt' comes between 'letters' and 'letters/file'. Thanks to Johannes for a pointer about qsort stability issues with Windows and suggested code change. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- merge-recursive.c | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 418246376b..7757e5f163 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -331,6 +331,37 @@ static struct string_list *get_unmerged(void) return unmerged; } +static int string_list_df_name_compare(const void *a, const void *b) +{ + const struct string_list_item *one = a; + const struct string_list_item *two = b; + int onelen = strlen(one->string); + int twolen = strlen(two->string); + /* + * Here we only care that entries for D/F conflicts are + * adjacent, in particular with the file of the D/F conflict + * appearing before files below the corresponding directory. + * The order of the rest of the list is irrelevant for us. + * + * To achieve this, we sort with df_name_compare and provide + * the mode S_IFDIR so that D/F conflicts will sort correctly. + * We use the mode S_IFDIR for everything else for simplicity, + * since in other cases any changes in their order due to + * sorting cause no problems for us. + */ + int cmp = df_name_compare(one->string, onelen, S_IFDIR, + two->string, twolen, S_IFDIR); + /* + * Now that 'foo' and 'foo/bar' compare equal, we have to make sure + * that 'foo' comes before 'foo/bar'. + */ + if (cmp) + return cmp; + return onelen - twolen; +} + + + static void make_room_for_directories_of_df_conflicts(struct merge_options *o, struct string_list *entries) { @@ -343,11 +374,6 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o, * 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 = 0; @@ -360,6 +386,10 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o, if (o->call_depth) return; + /* Ensure D/F conflicts are adjacent in the entries list. */ + qsort(entries->items, entries->nr, sizeof(*entries->items), + string_list_df_name_compare); + for (i = 0; i < entries->nr; i++) { const char *path = entries->items[i].string; int len = strlen(path); -- cgit v1.2.1 From f2507b4e0ef0b7fc8c1e75004e8a86a0430dc512 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:19:57 -0600 Subject: merge-recursive: Fix code checking for D/F conflicts still being present Previously, we were using lstat() to determine if a directory was still present after a merge (and thus in the way of adding a file). We should have been using lstat() only to determine if untracked directories were in the way (and then only when necessary to check for untracked directories); we should instead using the index to determine if there is a tracked directory in the way. Create a new function to do this and use it to replace the existing checks for directories being in the way. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 49 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 15 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 7757e5f163..3515ece55a 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -591,6 +591,30 @@ static void flush_buffer(int fd, const char *buf, unsigned long size) } } +static int dir_in_way(const char *path, int check_working_copy) +{ + int pos, pathlen = strlen(path); + char *dirpath = xmalloc(pathlen + 2); + struct stat st; + + strcpy(dirpath, path); + dirpath[pathlen] = '/'; + dirpath[pathlen+1] = '\0'; + + pos = cache_name_pos(dirpath, pathlen+1); + + if (pos < 0) + pos = -1 - pos; + if (pos < active_nr && + !strncmp(dirpath, active_cache[pos]->name, pathlen+1)) { + free(dirpath); + return 1; + } + + free(dirpath); + return check_working_copy && !lstat(path, &st) && S_ISDIR(st.st_mode); +} + static int would_lose_untracked(const char *path) { int pos = cache_name_pos(path, strlen(path)); @@ -880,7 +904,6 @@ static void conflict_rename_delete(struct merge_options *o, { 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", @@ -890,7 +913,7 @@ static void conflict_rename_delete(struct merge_options *o, update_stages(dest_name, NULL, rename_branch == o->branch1 ? pair->two : NULL, rename_branch == o->branch1 ? NULL : pair->two); - if (lstat(dest_name, &st) == 0 && S_ISDIR(st.st_mode)) { + if (dir_in_way(dest_name, !o->call_depth)) { dest_name = unique_path(o, dest_name, rename_branch); df_conflict = 1; } @@ -912,13 +935,12 @@ 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; - struct stat st; - if (lstat(ren1_dst, &st) == 0 && S_ISDIR(st.st_mode)) { + if (dir_in_way(ren1_dst, !o->call_depth)) { 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); } - if (lstat(ren2_dst, &st) == 0 && S_ISDIR(st.st_mode)) { + if (dir_in_way(ren2_dst, !o->call_depth)) { 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); @@ -1078,7 +1100,7 @@ static int process_renames(struct merge_options *o, try_merge = 0; if (sha_eq(src_other.sha1, null_sha1)) { - if (string_list_has_string(&o->current_directory_set, ren1_dst)) { + if (dir_in_way(ren1_dst, 0 /*check_wc*/)) { ren1->dst_entry->processed = 0; setup_rename_df_conflict_info(RENAME_DELETE, ren1->pair, @@ -1157,7 +1179,7 @@ static int process_renames(struct merge_options *o, a = &src_other; } update_stages_and_entry(ren1_dst, ren1->dst_entry, one, a, b, 1); - if (string_list_has_string(&o->current_directory_set, ren1_dst)) { + if (dir_in_way(ren1_dst, 0 /*check_wc*/)) { setup_rename_df_conflict_info(RENAME_NORMAL, ren1->pair, NULL, @@ -1262,7 +1284,6 @@ 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) { @@ -1279,7 +1300,7 @@ static int merge_content(struct merge_options *o, mfi = merge_file(o, &one, &a, &b, o->branch1, o->branch2); if (df_rename_conflict_branch && - lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) { + dir_in_way(path, !o->call_depth)) { df_conflict_remains = 1; } @@ -1344,8 +1365,7 @@ 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)) { + } else if (dir_in_way(path, 0 /*check_wc*/)) { entry->processed = 0; return 1; /* Assume clean until processed */ } else { @@ -1368,7 +1388,7 @@ static int process_entry(struct merge_options *o, mode = b_mode; sha = b_sha; } - if (string_list_has_string(&o->current_directory_set, path)) { + if (dir_in_way(path, 0 /*check_wc*/)) { /* Handle D->F conflicts after all subfiles */ entry->processed = 0; return 1; /* Assume clean until processed */ @@ -1416,7 +1436,6 @@ 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); - struct stat st; entry->processed = 1; if (entry->rename_df_conflict_info) { @@ -1461,7 +1480,7 @@ 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 */ char *renamed = NULL; - if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) { + if (dir_in_way(path, !o->call_depth)) { renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2); } clean_merge = 0; @@ -1489,7 +1508,7 @@ static int process_df_entry(struct merge_options *o, sha = b_sha; conf = "directory/file"; } - if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) { + if (dir_in_way(path, !o->call_depth)) { 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. " -- cgit v1.2.1 From 70cc3d36eba58f8bf177c91d82781fb727a9a4fa Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:19:58 -0600 Subject: merge-recursive: Save D/F conflict filenames instead of unlinking them Rename make_room_for_directories_of_df_conflicts() to record_df_conflict_files() to reflect the change in functionality. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 3515ece55a..99c38d5514 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -360,20 +360,24 @@ static int string_list_df_name_compare(const void *a, const void *b) return onelen - twolen; } - - -static void make_room_for_directories_of_df_conflicts(struct merge_options *o, - struct string_list *entries) +static void record_df_conflict_files(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. + /* If there is a D/F conflict and the file for such a conflict + * currently exist in the working copy, we want to allow it to + * be removed to make room for the corresponding directory if + * needed. The files underneath the directories of such D/F + * conflicts will be handled in process_entry(), while the + * files of such D/F conflicts will be processed later in + * process_df_entry(). If the corresponding directory ends up + * being removed by the merge, then no additional work needs + * to be done by process_df_entry() for the conflicting file. + * If the directory needs to be written to the working copy, + * then the conflicting file will simply be removed (e.g. in + * make_room_for_path). If the directory is written to the + * working copy but the file also has a conflict that needs to + * be resolved, then process_df_entry() will reinstate the + * file with a new unique name. */ const char *last_file = NULL; int last_len = 0; @@ -390,6 +394,7 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o, qsort(entries->items, entries->nr, sizeof(*entries->items), string_list_df_name_compare); + string_list_clear(&o->df_conflict_file_set, 1); for (i = 0; i < entries->nr; i++) { const char *path = entries->items[i].string; int len = strlen(path); @@ -398,14 +403,15 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o, /* * 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 so, record that it's okay to remove last_file to make + * room for path and friends if needed. */ 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); + string_list_insert(&o->df_conflict_file_set, last_file); } /* @@ -1569,7 +1575,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); + record_df_conflict_files(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); @@ -1795,6 +1801,8 @@ void init_merge_options(struct merge_options *o) o->current_file_set.strdup_strings = 1; memset(&o->current_directory_set, 0, sizeof(struct string_list)); o->current_directory_set.strdup_strings = 1; + memset(&o->df_conflict_file_set, 0, sizeof(struct string_list)); + o->df_conflict_file_set.strdup_strings = 1; } int parse_merge_opt(struct merge_options *o, const char *s) -- cgit v1.2.1 From aacb82de3ff8ae7b0a9e4cfec16c1807b6c315ef Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:19:59 -0600 Subject: merge-recursive: Split was_tracked() out of would_lose_untracked() Checking whether a filename was part of stage 0 or stage 2 is code that we would like to be able to call from a few other places without also lstat()-ing the file to see if it exists in the working copy. 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 99c38d5514..a30e5a4449 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -621,7 +621,7 @@ static int dir_in_way(const char *path, int check_working_copy) return check_working_copy && !lstat(path, &st) && S_ISDIR(st.st_mode); } -static int would_lose_untracked(const char *path) +static int was_tracked(const char *path) { int pos = cache_name_pos(path, strlen(path)); @@ -638,11 +638,16 @@ static int would_lose_untracked(const char *path) switch (ce_stage(active_cache[pos])) { case 0: case 2: - return 0; + return 1; } pos++; } - return file_exists(path); + return 0; +} + +static int would_lose_untracked(const char *path) +{ + return !was_tracked(path) && file_exists(path); } static int make_room_for_path(const char *path) -- cgit v1.2.1 From ed0148a520ec2ec88f0574c2e107aba0a46936e1 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:01 -0600 Subject: merge-recursive: Allow make_room_for_path() to remove D/F entries If there were several files conflicting below a directory corresponding to a D/F conflict, and the file of that D/F conflict is in the way, we want it to be removed. Since files of D/F conflicts are handled last, they can be reinstated later and possibly with a new unique name. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index a30e5a4449..5d6fc0d047 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -410,7 +410,6 @@ static void record_df_conflict_files(struct merge_options *o, 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); string_list_insert(&o->df_conflict_file_set, last_file); } @@ -650,11 +649,30 @@ static int would_lose_untracked(const char *path) return !was_tracked(path) && file_exists(path); } -static int make_room_for_path(const char *path) +static int make_room_for_path(struct merge_options *o, const char *path) { - int status; + int status, i; const char *msg = "failed to create path '%s'%s"; + /* Unlink any D/F conflict files that are in the way */ + for (i = 0; i < o->df_conflict_file_set.nr; i++) { + const char *df_path = o->df_conflict_file_set.items[i].string; + size_t pathlen = strlen(path); + size_t df_pathlen = strlen(df_path); + if (df_pathlen < pathlen && + path[df_pathlen] == '/' && + strncmp(path, df_path, df_pathlen) == 0) { + output(o, 3, + "Removing %s to make room for subdirectory\n", + df_path); + unlink(df_path); + unsorted_string_list_delete_item(&o->df_conflict_file_set, + i, 0); + break; + } + } + + /* Make sure leading directories are created */ status = safe_create_leading_directories_const(path); if (status) { if (status == -3) { @@ -722,7 +740,7 @@ static void update_file_flags(struct merge_options *o, } } - if (make_room_for_path(path) < 0) { + if (make_room_for_path(o, path) < 0) { update_wd = 0; free(buf); goto update_index; -- cgit v1.2.1 From b8ddf16424f2d4f29a73b89dabbb764763744abe Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:02 -0600 Subject: merge-recursive: Split update_stages_and_entry; only update stages at end Instead of having the process_renames logic update the stages in the index for the rename destination, have the index updated after process_entry or process_df_entry. This will also allow us to have process_entry determine whether a file was tracked and existed in the working copy before the merge started. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 5d6fc0d047..ce57ea530b 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -90,6 +90,7 @@ struct stage_data { } stages[4]; struct rename_df_conflict_info *rename_df_conflict_info; unsigned processed:1; + unsigned involved_in_rename:1; }; static inline void setup_rename_df_conflict_info(enum rename_type rename_type, @@ -522,15 +523,11 @@ static int update_stages(const char *path, const struct diff_filespec *o, return 0; } -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) +static void update_entry(struct stage_data *entry, + struct diff_filespec *o, + struct diff_filespec *a, + struct diff_filespec *b) { - int options; - entry->processed = 0; entry->stages[1].mode = o->mode; entry->stages[2].mode = a->mode; @@ -538,7 +535,6 @@ static int update_stages_and_entry(const char *path, hashcpy(entry->stages[1].sha, o->sha1); hashcpy(entry->stages[2].sha, a->sha1); hashcpy(entry->stages[3].sha, b->sha1); - return update_stages(path, o, a, b); } static int remove_file(struct merge_options *o, int clean, @@ -1097,12 +1093,11 @@ static int process_renames(struct merge_options *o, ren2->dst_entry); } else { remove_file(o, 1, ren1_src, 1); - update_stages_and_entry(ren1_dst, - ren1->dst_entry, - ren1->pair->one, - ren1->pair->two, - ren2->pair->two, - 1 /* clear */); + update_entry(ren1->dst_entry, + ren1->pair->one, + ren1->pair->two, + ren2->pair->two); + ren1->dst_entry->involved_in_rename = 1; } } else { /* Renamed in 1, maybe changed in 2 */ @@ -1207,7 +1202,8 @@ static int process_renames(struct merge_options *o, b = ren1->pair->two; a = &src_other; } - update_stages_and_entry(ren1_dst, ren1->dst_entry, one, a, b, 1); + update_entry(ren1->dst_entry, one, a, b); + ren1->dst_entry->involved_in_rename = 1; if (dir_in_way(ren1_dst, 0 /*check_wc*/)) { setup_rename_df_conflict_info(RENAME_NORMAL, ren1->pair, @@ -1304,6 +1300,7 @@ static void handle_delete_modify(struct merge_options *o, } static int merge_content(struct merge_options *o, + unsigned involved_in_rename, const char *path, unsigned char *o_sha, int o_mode, unsigned char *a_sha, int a_mode, @@ -1344,6 +1341,8 @@ static int merge_content(struct merge_options *o, reason = "submodule"; output(o, 1, "CONFLICT (%s): Merge conflict in %s", reason, path); + if (involved_in_rename) + update_stages(path, &one, &a, &b); } if (df_conflict_remains) { @@ -1428,7 +1427,7 @@ 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. */ - clean_merge = merge_content(o, path, + clean_merge = merge_content(o, entry->involved_in_rename, path, o_sha, o_mode, a_sha, a_mode, b_sha, b_mode, NULL); } else if (!o_sha && !a_sha && !b_sha) { @@ -1472,7 +1471,7 @@ static int process_df_entry(struct merge_options *o, char *src; switch (conflict_info->rename_type) { case RENAME_NORMAL: - clean_merge = merge_content(o, path, + clean_merge = merge_content(o, entry->involved_in_rename, path, o_sha, o_mode, a_sha, a_mode, b_sha, b_mode, conflict_info->branch1); break; -- cgit v1.2.1 From 531357a4cc2b93ec68099890b835f50e462ceab7 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:03 -0600 Subject: merge-recursive: Fix deletion of untracked file in rename/delete conflicts In the recursive case (o->call_depth > 0), we do not modify the working directory. However, when o->call_depth==0, file renames can mean we need to delete the old filename from the working copy. Since there have been lots of changes and mistakes here, let's go through the details. Let's start with a simple explanation of what we are trying to achieve: Original goal: If a file is renamed on the side of history being merged into head, the filename serving as the source of that rename needs to be removed from the working directory. The path to getting the above statement implemented in merge-recursive took several steps. The relevant bits of code may be instructive to keep in mind for the explanation, especially since an English-only description involves double negatives that are hard to follow. These bits of code are: int remove_file(..., const char *path, int no_wd) { ... int update_working_directory = !o->call_depth && !no_wd; and remove_file(o, 1, ren1_src, ); Where the choice for has morphed over time: 65ac6e9 (merge-recursive: adjust to loosened "working file clobbered" check 2006-10-27), introduced the "no_wd" parameter to remove_file() and used "1" for . This meant ren1_src was never deleted, leaving it around in the working copy. In 8371234 (Remove uncontested renamed files during merge. 2006-12-13), was changed to "index_only" (where index_only == !!o->call_depth; see b7fa51da). This was equivalent to using "0" for (due to the early logic in remove_file), and is orthogonal to the condition we actually want to check at this point; it resulted in the source file being removed except when index_only was false. This was problematic because the file could have been renamed on the side of history including head, in which case ren1_src could correspond to an untracked file that should not be deleted. In 183d797 (Keep untracked files not involved in a merge. 2007-02-04), was changed to "index_only || stage == 3". While this gives correct behavior, the "index_only ||" portion of is unnecessary and makes the code slightly harder to follow. There were also two further changes to this expression, though without any change in behavior. First in b7fa51d (merge-recursive: get rid of the index_only global variable 2008-09-02), it was changed to "o->call_depth || stage == 3". (index_only == !!o->call_depth). Later, in 41d70bd6 (merge-recursive: Small code clarification -- variable name and comments), this was changed to "o->call_depth || renamed_stage == 2" (where stage was renamed to other_stage and renamed_stage == other_stage ^ 1). So we ended with being "o->call_depth || renamed_stage == 2". But the "o->call_depth ||" piece was unnecessary. We can remove it, leaving us with being "renamed_stage == 2". This doesn't change behavior at all, but it makes the code clearer. Which is good, because it's about to get uglier. Corrected goal: If a file is renamed on the side of history being merged into head, the filename serving as the source of that rename needs to be removed from the working directory *IF* that file is tracked in head AND the file tracked in head is related to the original file. Note that the only difference between the original goal and the corrected goal is the two extra conditions added at the end. The first condition is relevant in a rename/delete conflict. If the file was deleted on the HEAD side of the merge and an untracked file of the same name was added to the working copy, then without that extra condition the untracked file will be erroneously deleted. This changes to "renamed_stage == 2 || !was_tracked(ren1_src)". The second additional condition is relevant in two cases. The first case the second condition can occur is when a file is deleted and a completely different file is added with the same name. To my knowledge, merge-recursive has no mechanism for detecting deleted-and- replaced-by-different-file cases, so I am simply punting on this possibility. The second case for the second condition to occur is when there is a rename/rename/add-source conflict. That is, when the original file was renamed on both sides of history AND the original filename is being re-used by some unrelated (but tracked) content. This case also presents some additional difficulties for us since we cannot currently detect these rename/rename/add-source conflicts; as long as the rename detection logic "optimizes" by ignoring filenames that are present at both ends of the diff, these conflicts will go unnoticed. However, rename/rename conflicts are handled by an entirely separate codepath not being discussed here, so this case is not relevant for the line of code under consideration. In summary: Change from "o->call_depth || renamed_stage == 2" to "renamed_stage == 2 || !was_tracked(ren1_src)", in order to remove unnecessary code and avoid deleting untracked files. 96 lines of explanation in the changelog to describe a one-line fix... Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index ce57ea530b..089cfe88c1 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1115,7 +1115,8 @@ 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); + remove_file(o, 1, ren1_src, + renamed_stage == 2 || !was_tracked(ren1_src)); hashcpy(src_other.sha1, ren1->src_entry->stages[other_stage].sha); src_other.mode = ren1->src_entry->stages[other_stage].mode; -- cgit v1.2.1 From 0a6b87126e6a4e2649f6da23db67eeaad11d102e Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:04 -0600 Subject: merge-recursive: Make dead code for rename/rename(2to1) conflicts undead The code for rename_rename_2to1 conflicts (two files both being renamed to the same filename) was dead since the rename/add path was always being independently triggered for each of the renames instead. Further, reviving the dead code showed that it was inherently buggy and would always segfault -- among a few other bugs. Move the else-if branch for the rename/rename block before the rename/add block to make sure it is checked first, and fix up the rename/rename(2to1) code segments to make it handle most cases. Work is still needed to handle higher dimensional corner cases such as rename/rename/modify/modify issues. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 70 ++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 48 insertions(+), 22 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 089cfe88c1..47b32f79d4 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -996,17 +996,36 @@ static void conflict_rename_rename_2to1(struct merge_options *o, struct rename *ren2, const char *branch2) { + char *path = ren1->pair->two->path; /* same as ren2->pair->two->path */ /* 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", - ren1->pair->one->path, new_path1, - ren2->pair->one->path, new_path2); - remove_file(o, 0, ren1->pair->two->path, 0); - update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path1); - update_file(o, 0, ren2->pair->two->sha1, ren2->pair->two->mode, new_path2); - free(new_path2); - free(new_path1); + if (o->call_depth) { + struct merge_file_info mfi; + struct diff_filespec one, a, b; + + one.path = a.path = b.path = path; + hashcpy(one.sha1, null_sha1); + one.mode = 0; + hashcpy(a.sha1, ren1->pair->two->sha1); + a.mode = ren1->pair->two->mode; + hashcpy(b.sha1, ren2->pair->two->sha1); + b.mode = ren2->pair->two->mode; + mfi = merge_file(o, &one, &a, &b, branch1, branch2); + output(o, 1, "Adding merged %s", path); + update_file(o, 0, mfi.sha, mfi.mode, path); + } else { + char *new_path1 = unique_path(o, path, branch1); + char *new_path2 = unique_path(o, path, branch2); + output(o, 1, "Renaming %s to %s and %s to %s instead", + ren1->pair->one->path, new_path1, + ren2->pair->one->path, new_path2); + remove_file(o, 0, path, 0); + update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, + new_path1); + update_file(o, 0, ren2->pair->two->sha1, ren2->pair->two->mode, + new_path2); + free(new_path2); + free(new_path1); + } } static int process_renames(struct merge_options *o, @@ -1021,12 +1040,12 @@ static int process_renames(struct merge_options *o, for (i = 0; i < a_renames->nr; i++) { sre = a_renames->items[i].util; string_list_insert(&a_by_dst, sre->pair->two->path)->util - = sre->dst_entry; + = (void *)sre; } for (i = 0; i < b_renames->nr; i++) { sre = b_renames->items[i].util; string_list_insert(&b_by_dst, sre->pair->two->path)->util - = sre->dst_entry; + = (void *)sre; } for (i = 0, j = 0; i < a_renames->nr || j < b_renames->nr;) { @@ -1138,6 +1157,23 @@ static int process_renames(struct merge_options *o, clean_merge = 0; conflict_rename_delete(o, ren1->pair, branch1, branch2); } + } else if ((item = string_list_lookup(renames2Dst, ren1_dst))) { + char *ren2_src, *ren2_dst; + ren2 = item->util; + ren2_src = ren2->pair->one->path; + ren2_dst = ren2->pair->two->path; + + clean_merge = 0; + ren2->processed = 1; + remove_file(o, 1, ren2_src, + renamed_stage == 3 || would_lose_untracked(ren1_src)); + + output(o, 1, "CONFLICT (rename/rename): " + "Rename %s->%s in %s. " + "Rename %s->%s in %s", + ren1_src, ren1_dst, branch1, + ren2_src, ren2_dst, branch2); + conflict_rename_rename_2to1(o, ren1, branch1, ren2, 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 @@ -1178,16 +1214,6 @@ static int process_renames(struct merge_options *o, update_file(o, 0, dst_other.sha1, dst_other.mode, new_path); free(new_path); } - } else if ((item = string_list_lookup(renames2Dst, ren1_dst))) { - ren2 = item->util; - clean_merge = 0; - ren2->processed = 1; - output(o, 1, "CONFLICT (rename/rename): " - "Rename %s->%s in %s. " - "Rename %s->%s in %s", - ren1_src, ren1_dst, branch1, - ren2->pair->one->path, ren2->pair->two->path, branch2); - conflict_rename_rename_2to1(o, ren1, branch1, ren2, branch2); } else try_merge = 1; -- cgit v1.2.1 From 7769a75e96f998b2f1ef51e8a2e88058f56fa519 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:05 -0600 Subject: merge-recursive: Add comments about handling rename/add-source cases There are a couple of places where changes are needed to for situations involving rename/add-source issues. Add comments about the needed changes (and existing bugs) until git has been enabled to detect such cases. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 47b32f79d4..7f169ade1e 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1085,6 +1085,9 @@ static int process_renames(struct merge_options *o, } ren1->dst_entry->processed = 1; + /* BUG: We should only mark src_entry as processed if we + * are not dealing with a rename + add-source case. + */ ren1->src_entry->processed = 1; if (ren1->processed) @@ -1111,6 +1114,10 @@ static int process_renames(struct merge_options *o, ren1->dst_entry, ren2->dst_entry); } else { + /* BUG: We should only remove ren1_src in + * the base stage (think of rename + + * add-source cases). + */ remove_file(o, 1, ren1_src, 1); update_entry(ren1->dst_entry, ren1->pair->one, @@ -1134,6 +1141,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; + /* BUG: We should only remove ren1_src in the base + * stage and in other_stage (think of rename + + * add-source case). + */ remove_file(o, 1, ren1_src, renamed_stage == 2 || !was_tracked(ren1_src)); -- cgit v1.2.1 From 51931bf08e7de1f597d36bc2fd38b5310b6da7dd Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:06 -0600 Subject: merge-recursive: Improve handling of rename target vs. directory addition When dealing with file merging and renames and D/F conflicts and possible criss-cross merges (how's that for a corner case?), we did not do a thorough job ensuring the index and working directory had the correct contents. Fix the logic in merge_content() to handle this. Also, correct some erroneous tests in t6022 that were expecting the wrong number of unmerged index entries. These changes fix one of the tests in t6042 (and almost fix another one from t6042 as well). Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 7f169ade1e..c6f177896f 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1379,19 +1379,34 @@ static int merge_content(struct merge_options *o, reason = "submodule"; output(o, 1, "CONFLICT (%s): Merge conflict in %s", reason, path); - if (involved_in_rename) + if (involved_in_rename && !df_conflict_remains) update_stages(path, &one, &a, &b); } if (df_conflict_remains) { char *new_path; - update_file_flags(o, mfi.sha, mfi.mode, path, - o->call_depth || mfi.clean, 0); + if (o->call_depth) { + remove_file_from_cache(path); + } else { + if (!mfi.clean) + update_stages(path, &one, &a, &b); + else { + int file_from_stage2 = was_tracked(path); + struct diff_filespec merged; + hashcpy(merged.sha1, mfi.sha); + merged.mode = mfi.mode; + + update_stages(path, NULL, + file_from_stage2 ? &merged : NULL, + file_from_stage2 ? NULL : &merged); + } + + } 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); + update_file(o, 0, mfi.sha, mfi.mode, new_path); free(new_path); + mfi.clean = 0; } else { update_file(o, mfi.clean, mfi.sha, mfi.mode, path); } @@ -1580,6 +1595,8 @@ 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); + if (o->call_depth) + remove_file_from_cache(path); update_file(o, 0, sha, mode, new_path); if (o->call_depth) remove_file_from_cache(path); -- cgit v1.2.1 From edd2faf52eda344eca2c01bc0b1d9f7f2f002a42 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:07 -0600 Subject: merge-recursive: Consolidate process_entry() and process_df_entry() The whole point of adding process_df_entry() was to ensure that files of D/F conflicts were processed after paths under the corresponding directory. However, given that the entries are in sorted order, all we need to do is iterate through them in reverse order to achieve the same effect. That lets us remove some duplicated code, and lets us keep track of one less thing as we read the code ("do we need to make sure this is processed before process_df_entry() or do we need to defer it until then?"). Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 188 +++++++++++++++++------------------------------------- 1 file changed, 57 insertions(+), 131 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index c6f177896f..29a5390b93 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -116,7 +116,6 @@ static inline void setup_rename_df_conflict_info(enum rename_type rename_type, ci->dst_entry2 = dst_entry2; ci->pair2 = pair2; dst_entry2->rename_df_conflict_info = ci; - dst_entry2->processed = 0; } } @@ -365,20 +364,17 @@ static void record_df_conflict_files(struct merge_options *o, struct string_list *entries) { /* If there is a D/F conflict and the file for such a conflict - * currently exist in the working copy, we want to allow it to - * be removed to make room for the corresponding directory if - * needed. The files underneath the directories of such D/F - * conflicts will be handled in process_entry(), while the - * files of such D/F conflicts will be processed later in - * process_df_entry(). If the corresponding directory ends up - * being removed by the merge, then no additional work needs - * to be done by process_df_entry() for the conflicting file. - * If the directory needs to be written to the working copy, - * then the conflicting file will simply be removed (e.g. in - * make_room_for_path). If the directory is written to the - * working copy but the file also has a conflict that needs to - * be resolved, then process_df_entry() will reinstate the - * file with a new unique name. + * currently exist in the working copy, we want to allow it to be + * removed to make room for the corresponding directory if needed. + * The files underneath the directories of such D/F conflicts will + * be processed before the corresponding file involved in the D/F + * conflict. If the D/F directory ends up being removed by the + * merge, then we won't have to touch the D/F file. If the D/F + * directory needs to be written to the working copy, then the D/F + * file will simply be removed (in make_room_for_path()) to make + * room for the necessary paths. Note that if both the directory + * and the file need to be present, then the D/F file will be + * reinstated with a new unique name at the time it is processed. */ const char *last_file = NULL; int last_len = 0; @@ -1323,17 +1319,17 @@ static void handle_delete_modify(struct merge_options *o, "and modified in %s. Version %s of %s left in tree%s%s.", path, o->branch1, o->branch2, o->branch2, path, - path == new_path ? "" : " at ", - path == new_path ? "" : new_path); - update_file(o, 0, b_sha, b_mode, new_path); + NULL == new_path ? "" : " at ", + NULL == new_path ? "" : new_path); + update_file(o, 0, b_sha, b_mode, new_path ? new_path : path); } else { output(o, 1, "CONFLICT (delete/modify): %s deleted in %s " "and modified in %s. Version %s of %s left in tree%s%s.", path, o->branch2, o->branch1, o->branch1, path, - path == new_path ? "" : " at ", - path == new_path ? "" : new_path); - update_file(o, 0, a_sha, a_mode, new_path); + NULL == new_path ? "" : " at ", + NULL == new_path ? "" : new_path); + update_file(o, 0, a_sha, a_mode, new_path ? new_path : path); } } @@ -1431,93 +1427,6 @@ 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 */ - if ((!a_sha && !b_sha) || - (!b_sha && blob_unchanged(o_sha, a_sha, normalize, path)) || - (!a_sha && blob_unchanged(o_sha, b_sha, normalize, path))) { - /* Deleted in both or deleted in one and - * unchanged in the other */ - if (a_sha) - 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 (dir_in_way(path, 0 /*check_wc*/)) { - entry->processed = 0; - return 1; /* Assume clean until processed */ - } else { - /* Deleted in one and changed in the other */ - clean_merge = 0; - handle_delete_modify(o, path, path, - a_sha, a_mode, b_sha, b_mode); - } - - } else if ((!o_sha && a_sha && !b_sha) || - (!o_sha && !a_sha && b_sha)) { - /* Case B: Added in one. */ - unsigned mode; - const unsigned char *sha; - - if (a_sha) { - mode = a_mode; - sha = a_sha; - } else { - mode = b_mode; - sha = b_sha; - } - if (dir_in_way(path, 0 /*check_wc*/)) { - /* Handle D->F conflicts after all subfiles */ - entry->processed = 0; - return 1; /* Assume clean until processed */ - } else { - output(o, 2, "Adding %s", path); - update_file(o, 1, sha, mode, path); - } - } else if (a_sha && b_sha) { - /* Case C: Added in both (check for same permissions) and */ - /* case D: Modified in both, but differently. */ - clean_merge = merge_content(o, entry->involved_in_rename, path, - 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 - * we had that path and want to actively remove it. - */ - remove_file(o, 1, path, !a_mode); - } else - die("Fatal merge failure, shouldn't happen."); - - return clean_merge; -} - -/* - * 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) -{ - int clean_merge = 1; - unsigned o_mode = entry->stages[1].mode; - unsigned a_mode = entry->stages[2].mode; - unsigned b_mode = entry->stages[3].mode; - 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); - entry->processed = 1; if (entry->rename_df_conflict_info) { struct rename_df_conflict_info *conflict_info = entry->rename_df_conflict_info; @@ -1552,24 +1461,38 @@ static int process_df_entry(struct merge_options *o, 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)) { - /* Modify/delete; deleted side may have put a directory in the way */ - char *renamed = NULL; - if (dir_in_way(path, !o->call_depth)) { - renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2); + /* Case A: Deleted in one */ + if ((!a_sha && !b_sha) || + (!b_sha && blob_unchanged(o_sha, a_sha, normalize, path)) || + (!a_sha && blob_unchanged(o_sha, b_sha, normalize, path))) { + /* Deleted in both or deleted in one and + * unchanged in the other */ + if (a_sha) + output(o, 2, "Removing %s", path); + /* do not touch working file if it did not exist */ + remove_file(o, 1, path, !a_sha); + } else { + /* Modify/delete; deleted side may have put a directory in the way */ + char *renamed = NULL; + clean_merge = 0; + if (dir_in_way(path, !o->call_depth)) { + renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2); + } + handle_delete_modify(o, path, renamed, + a_sha, a_mode, b_sha, b_mode); + free(renamed); } - clean_merge = 0; - handle_delete_modify(o, path, renamed ? renamed : path, - a_sha, a_mode, b_sha, b_mode); - free(renamed); - } else if (!o_sha && !!a_sha != !!b_sha) { - /* directory -> (directory, file) or -> (directory, file) */ + } else if ((!o_sha && a_sha && !b_sha) || + (!o_sha && !a_sha && b_sha)) { + /* Case B: Added in one. */ + /* [nothing|directory] -> ([nothing|directory], file) */ + const char *add_branch; const char *other_branch; unsigned mode; @@ -1605,10 +1528,20 @@ static int process_df_entry(struct merge_options *o, output(o, 2, "Adding %s", path); update_file(o, 1, sha, mode, path); } - } else { - entry->processed = 0; - return 1; /* not handled; assume clean until processed */ - } + } else if (a_sha && b_sha) { + /* Case C: Added in both (check for same permissions) and */ + /* case D: Modified in both, but differently. */ + clean_merge = merge_content(o, entry->involved_in_rename, path, + 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 + * we had that path and want to actively remove it. + */ + remove_file(o, 1, path, !a_mode); + } else + die("Fatal merge failure, shouldn't happen."); return clean_merge; } @@ -1656,20 +1589,13 @@ int merge_trees(struct merge_options *o, 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); - for (i = 0; i < entries->nr; i++) { + for (i = entries->nr-1; 0 <= i; i--) { const char *path = entries->items[i].string; struct stage_data *e = entries->items[i].util; if (!e->processed && !process_entry(o, path, e)) clean = 0; } - for (i = 0; i < entries->nr; i++) { - const char *path = entries->items[i].string; - struct stage_data *e = entries->items[i].util; - if (!e->processed - && !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) -- cgit v1.2.1 From 4f66dade81c2ebc6e1143b6c116960bfcd837067 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:08 -0600 Subject: merge-recursive: Cleanup and consolidation of rename_conflict_info The consolidation of process_entry() and process_df_entry() allows us to consolidate more code paths concerning rename conflicts, and to do a few additional related cleanups. It also means we are using rename_df_conflict_info in some cases where there is no D/F conflict; rename it to rename_conflict_info. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 134 +++++++++++++++++++++++++++--------------------------- 1 file changed, 66 insertions(+), 68 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 29a5390b93..e68396bcb0 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -66,10 +66,11 @@ static int sha_eq(const unsigned char *a, const unsigned char *b) enum rename_type { RENAME_NORMAL = 0, RENAME_DELETE, + RENAME_ONE_FILE_TO_ONE, RENAME_ONE_FILE_TO_TWO }; -struct rename_df_conflict_info { +struct rename_conflict_info { enum rename_type rename_type; struct diff_filepair *pair1; struct diff_filepair *pair2; @@ -88,34 +89,33 @@ struct stage_data { unsigned mode; unsigned char sha[20]; } stages[4]; - struct rename_df_conflict_info *rename_df_conflict_info; + struct rename_conflict_info *rename_conflict_info; unsigned processed:1; - unsigned involved_in_rename: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) +static inline void setup_rename_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)); + struct rename_conflict_info *ci = xcalloc(1, sizeof(struct rename_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->rename_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->rename_conflict_info = ci; } } @@ -952,10 +952,29 @@ static void conflict_rename_rename_1to2(struct merge_options *o, /* One file was renamed in both branches, but to different names. */ char *del[2]; int delp = 0; + const char *src = pair1->one->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; + + output(o, 1, "CONFLICT (rename/rename): " + "Rename \"%s\"->\"%s\" in branch \"%s\" " + "rename \"%s\"->\"%s\" in \"%s\"%s", + src, pair1->two->path, branch1, + src, pair2->two->path, branch2, + o->call_depth ? " (left unresolved)" : ""); + if (o->call_depth) { + /* + * FIXME: Why remove file from cache, and then + * immediately readd it? Why not just overwrite using + * update_file only? Also...this is buggy for + * rename/add-source situations... + */ + remove_file_from_cache(src); + update_file(o, 0, pair1->one->sha1, pair1->one->mode, src); + } + if (dir_in_way(ren1_dst, !o->call_depth)) { dst_name1 = del[delp++] = unique_path(o, ren1_dst, branch1); output(o, 1, "%s is a directory in %s adding as %s instead", @@ -1096,20 +1115,16 @@ static int process_renames(struct merge_options *o, if (ren2) { const char *ren2_src = ren2->pair->one->path; const char *ren2_dst = ren2->pair->two->path; + enum rename_type rename_type; /* Renamed in 1 and renamed in 2 */ if (strcmp(ren1_src, ren2_src) != 0) die("ren1.src != ren2.src"); ren2->dst_entry->processed = 1; ren2->processed = 1; if (strcmp(ren1_dst, ren2_dst) != 0) { - setup_rename_df_conflict_info(RENAME_ONE_FILE_TO_TWO, - ren1->pair, - ren2->pair, - branch1, - branch2, - ren1->dst_entry, - ren2->dst_entry); + rename_type = RENAME_ONE_FILE_TO_TWO; } else { + rename_type = RENAME_ONE_FILE_TO_ONE; /* BUG: We should only remove ren1_src in * the base stage (think of rename + * add-source cases). @@ -1119,8 +1134,14 @@ static int process_renames(struct merge_options *o, ren1->pair->one, ren1->pair->two, ren2->pair->two); - ren1->dst_entry->involved_in_rename = 1; } + setup_rename_conflict_info(rename_type, + ren1->pair, + ren2->pair, + branch1, + branch2, + ren1->dst_entry, + ren2->dst_entry); } else { /* Renamed in 1, maybe changed in 2 */ struct string_list_item *item; @@ -1151,19 +1172,13 @@ static int process_renames(struct merge_options *o, try_merge = 0; if (sha_eq(src_other.sha1, null_sha1)) { - if (dir_in_way(ren1_dst, 0 /*check_wc*/)) { - ren1->dst_entry->processed = 0; - setup_rename_df_conflict_info(RENAME_DELETE, - ren1->pair, - NULL, - branch1, - branch2, - ren1->dst_entry, - NULL); - } else { - clean_merge = 0; - conflict_rename_delete(o, ren1->pair, branch1, branch2); - } + setup_rename_conflict_info(RENAME_DELETE, + ren1->pair, + NULL, + branch1, + branch2, + ren1->dst_entry, + NULL); } else if ((item = string_list_lookup(renames2Dst, ren1_dst))) { char *ren2_src, *ren2_dst; ren2 = item->util; @@ -1237,16 +1252,13 @@ static int process_renames(struct merge_options *o, a = &src_other; } update_entry(ren1->dst_entry, one, a, b); - ren1->dst_entry->involved_in_rename = 1; - if (dir_in_way(ren1_dst, 0 /*check_wc*/)) { - setup_rename_df_conflict_info(RENAME_NORMAL, - ren1->pair, - NULL, - branch1, - NULL, - ren1->dst_entry, - NULL); - } + setup_rename_conflict_info(RENAME_NORMAL, + ren1->pair, + NULL, + branch1, + NULL, + ren1->dst_entry, + NULL); } } } @@ -1334,12 +1346,11 @@ static void handle_delete_modify(struct merge_options *o, } static int merge_content(struct merge_options *o, - unsigned involved_in_rename, 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 *df_rename_conflict_branch) + struct rename_conflict_info *rename_conflict_info) { const char *reason = "content"; struct merge_file_info mfi; @@ -1359,8 +1370,7 @@ static int merge_content(struct merge_options *o, b.mode = b_mode; mfi = merge_file(o, &one, &a, &b, o->branch1, o->branch2); - if (df_rename_conflict_branch && - dir_in_way(path, !o->call_depth)) { + if (rename_conflict_info && dir_in_way(path, !o->call_depth)) { df_conflict_remains = 1; } @@ -1375,7 +1385,7 @@ static int merge_content(struct merge_options *o, reason = "submodule"; output(o, 1, "CONFLICT (%s): Merge conflict in %s", reason, path); - if (involved_in_rename && !df_conflict_remains) + if (rename_conflict_info && !df_conflict_remains) update_stages(path, &one, &a, &b); } @@ -1398,7 +1408,7 @@ static int merge_content(struct merge_options *o, } } - new_path = unique_path(o, path, df_rename_conflict_branch); + new_path = unique_path(o, path, rename_conflict_info->branch1); output(o, 1, "Adding as %s instead", new_path); update_file(o, 0, mfi.sha, mfi.mode, new_path); free(new_path); @@ -1428,14 +1438,14 @@ static int process_entry(struct merge_options *o, unsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode); entry->processed = 1; - if (entry->rename_df_conflict_info) { - struct rename_df_conflict_info *conflict_info = entry->rename_df_conflict_info; - char *src; + if (entry->rename_conflict_info) { + struct rename_conflict_info *conflict_info = entry->rename_conflict_info; switch (conflict_info->rename_type) { case RENAME_NORMAL: - clean_merge = merge_content(o, entry->involved_in_rename, path, + case RENAME_ONE_FILE_TO_ONE: + clean_merge = merge_content(o, path, o_sha, o_mode, a_sha, a_mode, b_sha, b_mode, - conflict_info->branch1); + conflict_info); break; case RENAME_DELETE: clean_merge = 0; @@ -1444,19 +1454,7 @@ static int process_entry(struct merge_options *o, conflict_info->branch2); break; 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, @@ -1531,7 +1529,7 @@ 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. */ - clean_merge = merge_content(o, entry->involved_in_rename, path, + clean_merge = merge_content(o, path, o_sha, o_mode, a_sha, a_mode, b_sha, b_mode, NULL); } else if (!o_sha && !a_sha && !b_sha) { -- cgit v1.2.1 From 3c217c077a86d9ae06ed26bb0baa4475d0e28a0e Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:09 -0600 Subject: merge-recursive: Provide more info in conflict markers with file renames Whenever there are merge conflicts in file contents, we would mark the different sides of the conflict with the two branches being merged. However, when there is a rename involved as well, the branchname is not sufficient to specify where the conflicting content came from. In such cases, mark the two sides of the conflict with branchname:filename rather than just branchname. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index e68396bcb0..a3bbca8d58 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1353,6 +1353,7 @@ static int merge_content(struct merge_options *o, struct rename_conflict_info *rename_conflict_info) { const char *reason = "content"; + char *side1 = NULL, *side2 = NULL; struct merge_file_info mfi; struct diff_filespec one, a, b; unsigned df_conflict_remains = 0; @@ -1369,10 +1370,31 @@ static int merge_content(struct merge_options *o, hashcpy(b.sha1, b_sha); b.mode = b_mode; - mfi = merge_file(o, &one, &a, &b, o->branch1, o->branch2); - if (rename_conflict_info && dir_in_way(path, !o->call_depth)) { - df_conflict_remains = 1; + if (rename_conflict_info) { + const char *path1, *path2; + struct diff_filepair *pair1 = rename_conflict_info->pair1; + + path1 = (o->branch1 == rename_conflict_info->branch1) ? + pair1->two->path : pair1->one->path; + /* If rename_conflict_info->pair2 != NULL, we are in + * RENAME_ONE_FILE_TO_ONE case. Otherwise, we have a + * normal rename. + */ + path2 = (rename_conflict_info->pair2 || + o->branch2 == rename_conflict_info->branch1) ? + pair1->two->path : pair1->one->path; + side1 = xmalloc(strlen(o->branch1) + strlen(path1) + 2); + side2 = xmalloc(strlen(o->branch2) + strlen(path2) + 2); + sprintf(side1, "%s:%s", o->branch1, path1); + sprintf(side2, "%s:%s", o->branch2, path2); + + if (dir_in_way(path, !o->call_depth)) + df_conflict_remains = 1; } + mfi = merge_file(o, &one, &a, &b, + side1 ? side1 : o->branch1, side2 ? side2 : o->branch2); + free(side1); + free(side2); if (mfi.clean && !df_conflict_remains && sha_eq(mfi.sha, a_sha) && mfi.mode == a.mode) -- cgit v1.2.1 From 5b448b8530308b1f5a7a721cb1bf0ba557b5c78d Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:10 -0600 Subject: merge-recursive: When we detect we can skip an update, actually skip it In 882fd11 (merge-recursive: Delay content merging for renames 2010-09-20), there was code that checked for whether we could skip updating a file in the working directory, based on whether the merged version matched the current working copy. Due to the desire to handle directory/file conflicts that were resolvable, that commit deferred content merging by first updating the index with the unmerged entries and then moving the actual merging (along with the skip-the-content-update check) to another function that ran later in the merge process. As part moving the content merging code, a bug was introduced such that although the message about skipping the update would be printed (whenever GIT_MERGE_VERBOSITY was sufficiently high), the file would be unconditionally updated in the working copy anyway. When we detect that the file does not need to be updated in the working copy, update the index appropriately and then return early before updating the working copy. Note that there was a similar change in b2c8c0a (merge-recursive: When we detect we can skip an update, actually skip it 2011-02-28), but it was reverted by 6db4105 (Revert "Merge branch 'en/merge-recursive'" 2011-05-19) since it did not fix both of the relevant types of unnecessary update breakages and, worse, it made use of some band-aids that caused other problems. The reason this change works is due to the changes earlier in this series to (a) record_df_conflict_files instead of just unlinking them early, (b) allowing make_room_for_path() to remove D/F entries, (c) the splitting of update_stages_and_entry() to have its functionality called at different points, and (d) making the pathnames of the files involved in the merge available to merge_content(). Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index a3bbca8d58..8b88d6266c 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1354,6 +1354,7 @@ static int merge_content(struct merge_options *o, { const char *reason = "content"; char *side1 = NULL, *side2 = NULL; + const char *path1 = NULL, *path2 = NULL; struct merge_file_info mfi; struct diff_filespec one, a, b; unsigned df_conflict_remains = 0; @@ -1371,7 +1372,6 @@ static int merge_content(struct merge_options *o, b.mode = b_mode; if (rename_conflict_info) { - const char *path1, *path2; struct diff_filepair *pair1 = rename_conflict_info->pair1; path1 = (o->branch1 == rename_conflict_info->branch1) ? @@ -1397,9 +1397,22 @@ static int merge_content(struct merge_options *o, free(side2); if (mfi.clean && !df_conflict_remains && - sha_eq(mfi.sha, a_sha) && mfi.mode == a.mode) + sha_eq(mfi.sha, a_sha) && mfi.mode == a_mode) { + int path_renamed_outside_HEAD; output(o, 3, "Skipped %s (merged same as existing)", path); - else + /* + * The content merge resulted in the same file contents we + * already had. We can return early if those file contents + * are recorded at the correct path (which may not be true + * if the merge involves a rename). + */ + path_renamed_outside_HEAD = !path2 || !strcmp(path, path2); + if (!path_renamed_outside_HEAD) { + add_cacheinfo(mfi.mode, mfi.sha, path, + 0 /*stage*/, 1 /*refresh*/, 0 /*options*/); + return mfi.clean; + } + } else output(o, 2, "Auto-merging %s", path); if (!mfi.clean) { -- cgit v1.2.1 From ec61d14963470b02db0d6e3825e2b5bbb1815cb6 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:11 -0600 Subject: 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 Signed-off-by: Junio C Hamano --- merge-recursive.c | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 8b88d6266c..897cada6cf 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1322,27 +1322,42 @@ error_return: static void handle_delete_modify(struct merge_options *o, const char *path, - const char *new_path, + unsigned char *o_sha, int o_mode, unsigned char *a_sha, int a_mode, unsigned char *b_sha, int b_mode) { - if (!a_sha) { + char *renamed = NULL; + if (dir_in_way(path, !o->call_depth)) { + renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2); + } + + if (o->call_depth) { + /* + * We cannot arbitrarily accept either a_sha or b_sha as + * correct; since there is no true "middle point" between + * them, simply reuse the base version for virtual merge base. + */ + remove_file_from_cache(path); + update_file(o, 0, o_sha, o_mode, renamed ? renamed : path); + } else if (!a_sha) { output(o, 1, "CONFLICT (delete/modify): %s deleted in %s " "and modified in %s. Version %s of %s left in tree%s%s.", path, o->branch1, o->branch2, o->branch2, path, - NULL == new_path ? "" : " at ", - NULL == new_path ? "" : new_path); - update_file(o, 0, b_sha, b_mode, new_path ? new_path : path); + NULL == renamed ? "" : " at ", + NULL == renamed ? "" : renamed); + update_file(o, 0, b_sha, b_mode, renamed ? renamed : path); } else { output(o, 1, "CONFLICT (delete/modify): %s deleted in %s " "and modified in %s. Version %s of %s left in tree%s%s.", path, o->branch2, o->branch1, o->branch1, path, - NULL == new_path ? "" : " at ", - NULL == new_path ? "" : new_path); - update_file(o, 0, a_sha, a_mode, new_path ? new_path : path); + NULL == renamed ? "" : " at ", + NULL == renamed ? "" : renamed); + update_file(o, 0, a_sha, a_mode, renamed ? renamed : path); } + free(renamed); + } static int merge_content(struct merge_options *o, @@ -1512,14 +1527,9 @@ static int process_entry(struct merge_options *o, remove_file(o, 1, path, !a_sha); } else { /* Modify/delete; deleted side may have put a directory in the way */ - char *renamed = NULL; clean_merge = 0; - if (dir_in_way(path, !o->call_depth)) { - renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2); - } - handle_delete_modify(o, path, renamed, + handle_delete_modify(o, path, o_sha, o_mode, a_sha, a_mode, b_sha, b_mode); - free(renamed); } } else if ((!o_sha && a_sha && !b_sha) || (!o_sha && !a_sha && b_sha)) { -- cgit v1.2.1 From 6bdaead1e58ab2aa3ea81a998117d1d5afb41a3b Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:12 -0600 Subject: merge-recursive: Introduce a merge_file convenience function merge_file previously required diff_filespec arguments, but all callers only had sha1s and modes. Rename merge_file to merge_file_1 and introduce a new merge_file convenience function which takes the sha1s and modes and creates the temporary diff_filespec variables needed to call merge_file_1. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 72 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 37 insertions(+), 35 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 897cada6cf..dcfb7228a5 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -844,12 +844,12 @@ static int merge_3way(struct merge_options *o, return merge_status; } -static struct merge_file_info merge_file(struct merge_options *o, - const struct diff_filespec *one, - const struct diff_filespec *a, - const struct diff_filespec *b, - const char *branch1, - const char *branch2) +static struct merge_file_info merge_file_1(struct merge_options *o, + const struct diff_filespec *one, + const struct diff_filespec *a, + const struct diff_filespec *b, + const char *branch1, + const char *branch2) { struct merge_file_info result; result.merge = 0; @@ -918,6 +918,26 @@ static struct merge_file_info merge_file(struct merge_options *o, return result; } +static struct merge_file_info merge_file(struct merge_options *o, + const char *path, + const unsigned char *o_sha, int o_mode, + const unsigned char *a_sha, int a_mode, + const unsigned char *b_sha, int b_mode, + const char *branch1, + const char *branch2) +{ + struct diff_filespec one, a, b; + + 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; + return merge_file_1(o, &one, &a, &b, branch1, branch2); +} + static void conflict_rename_delete(struct merge_options *o, struct diff_filepair *pair, const char *rename_branch, @@ -1015,16 +1035,10 @@ static void conflict_rename_rename_2to1(struct merge_options *o, /* Two files were renamed to the same thing. */ if (o->call_depth) { struct merge_file_info mfi; - struct diff_filespec one, a, b; - - one.path = a.path = b.path = path; - hashcpy(one.sha1, null_sha1); - one.mode = 0; - hashcpy(a.sha1, ren1->pair->two->sha1); - a.mode = ren1->pair->two->mode; - hashcpy(b.sha1, ren2->pair->two->sha1); - b.mode = ren2->pair->two->mode; - mfi = merge_file(o, &one, &a, &b, branch1, branch2); + mfi = merge_file(o, path, null_sha1, 0, + ren1->pair->two->sha1, ren1->pair->two->mode, + ren2->pair->two->sha1, ren2->pair->two->mode, + branch1, branch2); output(o, 1, "Adding merged %s", path); update_file(o, 0, mfi.sha, mfi.mode, path); } else { @@ -1211,24 +1225,12 @@ static int process_renames(struct merge_options *o, ren1_dst, branch2); if (o->call_depth) { struct merge_file_info mfi; - struct diff_filespec one, a, b; - - one.path = a.path = b.path = - (char *)ren1_dst; - hashcpy(one.sha1, null_sha1); - one.mode = 0; - hashcpy(a.sha1, ren1->pair->two->sha1); - a.mode = ren1->pair->two->mode; - hashcpy(b.sha1, dst_other.sha1); - b.mode = dst_other.mode; - mfi = merge_file(o, &one, &a, &b, - branch1, - branch2); + mfi = merge_file(o, ren1_dst, null_sha1, 0, + ren1->pair->two->sha1, ren1->pair->two->mode, + dst_other.sha1, dst_other.mode, + branch1, branch2); output(o, 1, "Adding merged %s", ren1_dst); - update_file(o, 0, - mfi.sha, - mfi.mode, - ren1_dst); + update_file(o, 0, mfi.sha, mfi.mode, ren1_dst); try_merge = 0; } else { char *new_path = unique_path(o, ren1_dst, branch2); @@ -1406,8 +1408,8 @@ static int merge_content(struct merge_options *o, if (dir_in_way(path, !o->call_depth)) df_conflict_remains = 1; } - mfi = merge_file(o, &one, &a, &b, - side1 ? side1 : o->branch1, side2 ? side2 : o->branch2); + mfi = merge_file_1(o, &one, &a, &b, + side1 ? side1 : o->branch1, side2 ? side2 : o->branch2); free(side1); free(side2); -- cgit v1.2.1 From c52ff85d97c698c902870305a010e2303e297b87 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:13 -0600 Subject: merge-recursive: Fix rename/rename(1to2) resolution for virtual merge base When renaming one file to two files, we really should be doing a content merge. Also, in the recursive case, undoing the renames and recording the merged file in the index with the source of the rename (while deleting both destinations) allows the renames to be re-detected in the non-recursive merge and will result in fewer spurious conflicts. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index dcfb7228a5..f64cfc582a 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -984,17 +984,6 @@ static void conflict_rename_rename_1to2(struct merge_options *o, src, pair1->two->path, branch1, src, pair2->two->path, branch2, o->call_depth ? " (left unresolved)" : ""); - if (o->call_depth) { - /* - * FIXME: Why remove file from cache, and then - * immediately readd it? Why not just overwrite using - * update_file only? Also...this is buggy for - * rename/add-source situations... - */ - remove_file_from_cache(src); - update_file(o, 0, pair1->one->sha1, pair1->one->mode, src); - } - if (dir_in_way(ren1_dst, !o->call_depth)) { dst_name1 = del[delp++] = unique_path(o, ren1_dst, branch1); output(o, 1, "%s is a directory in %s adding as %s instead", @@ -1006,14 +995,21 @@ static void conflict_rename_rename_1to2(struct merge_options *o, ren2_dst, branch1, dst_name2); } if (o->call_depth) { - remove_file_from_cache(dst_name1); - remove_file_from_cache(dst_name2); + struct merge_file_info mfi; + mfi = merge_file(o, src, + pair1->one->sha1, pair1->one->mode, + pair1->two->sha1, pair1->two->mode, + pair2->two->sha1, pair2->two->mode, + branch1, branch2); /* - * Uncomment to leave the conflicting names in the resulting tree - * - * update_file(o, 0, pair1->two->sha1, pair1->two->mode, dst_name1); - * update_file(o, 0, pair2->two->sha1, pair2->two->mode, dst_name2); + * FIXME: For rename/add-source conflicts (if we could detect + * such), this is wrong. We should instead find a unique + * pathname and then either rename the add-source file to that + * unique path, or use that unique path instead of src here. */ + update_file(o, 0, mfi.sha, mfi.mode, src); + remove_file_from_cache(ren1_dst); + remove_file_from_cache(ren2_dst); } else { update_stages(ren1_dst, NULL, pair1->two, NULL); update_stages(ren2_dst, NULL, NULL, pair2->two); -- cgit v1.2.1 From a99b7f2256ebd2d1db59c578f3da003e36f259c4 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:14 -0600 Subject: merge-recursive: Small cleanups for conflict_rename_rename_1to2 Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 60 +++++++++++++++++++++++++------------------------------ 1 file changed, 27 insertions(+), 33 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index f64cfc582a..8020d4b513 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -964,58 +964,55 @@ static void conflict_rename_delete(struct merge_options *o, } static void conflict_rename_rename_1to2(struct merge_options *o, - struct diff_filepair *pair1, - const char *branch1, - struct diff_filepair *pair2, - const char *branch2) + struct rename_conflict_info *ci) { /* One file was renamed in both branches, but to different names. */ + struct diff_filespec *one = ci->pair1->one; + struct diff_filespec *a = ci->pair1->two; + struct diff_filespec *b = ci->pair2->two; + const char *dst_name_a = a->path; + const char *dst_name_b = b->path; char *del[2]; int delp = 0; - const char *src = pair1->one->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; output(o, 1, "CONFLICT (rename/rename): " "Rename \"%s\"->\"%s\" in branch \"%s\" " "rename \"%s\"->\"%s\" in \"%s\"%s", - src, pair1->two->path, branch1, - src, pair2->two->path, branch2, + one->path, a->path, ci->branch1, + one->path, b->path, ci->branch2, o->call_depth ? " (left unresolved)" : ""); - if (dir_in_way(ren1_dst, !o->call_depth)) { - dst_name1 = del[delp++] = unique_path(o, ren1_dst, branch1); + if (dir_in_way(a->path, !o->call_depth)) { + dst_name_a = del[delp++] = unique_path(o, a->path, ci->branch1); output(o, 1, "%s is a directory in %s adding as %s instead", - ren1_dst, branch2, dst_name1); + a->path, ci->branch2, dst_name_a); } - if (dir_in_way(ren2_dst, !o->call_depth)) { - dst_name2 = del[delp++] = unique_path(o, ren2_dst, branch2); + if (dir_in_way(b->path, !o->call_depth)) { + dst_name_b = del[delp++] = unique_path(o, b->path, ci->branch2); output(o, 1, "%s is a directory in %s adding as %s instead", - ren2_dst, branch1, dst_name2); + b->path, ci->branch1, dst_name_b); } if (o->call_depth) { struct merge_file_info mfi; - mfi = merge_file(o, src, - pair1->one->sha1, pair1->one->mode, - pair1->two->sha1, pair1->two->mode, - pair2->two->sha1, pair2->two->mode, - branch1, branch2); + mfi = merge_file(o, one->path, + one->sha1, one->mode, + a->sha1, a->mode, + b->sha1, b->mode, + ci->branch1, ci->branch2); /* * FIXME: For rename/add-source conflicts (if we could detect * such), this is wrong. We should instead find a unique * pathname and then either rename the add-source file to that * unique path, or use that unique path instead of src here. */ - update_file(o, 0, mfi.sha, mfi.mode, src); - remove_file_from_cache(ren1_dst); - remove_file_from_cache(ren2_dst); + update_file(o, 0, mfi.sha, mfi.mode, one->path); + remove_file_from_cache(a->path); + remove_file_from_cache(b->path); } else { - update_stages(ren1_dst, NULL, pair1->two, NULL); - update_stages(ren2_dst, NULL, NULL, pair2->two); + update_stages(a->path, NULL, a, NULL); + update_stages(b->path, NULL, NULL, b); - update_file(o, 0, pair1->two->sha1, pair1->two->mode, dst_name1); - update_file(o, 0, pair2->two->sha1, pair2->two->mode, dst_name2); + update_file(o, 0, a->sha1, a->mode, dst_name_a); + update_file(o, 0, b->sha1, b->mode, dst_name_b); } while (delp--) free(del[delp]); @@ -1503,10 +1500,7 @@ static int process_entry(struct merge_options *o, break; case RENAME_ONE_FILE_TO_TWO: clean_merge = 0; - conflict_rename_rename_1to2(o, conflict_info->pair1, - conflict_info->branch1, - conflict_info->pair2, - conflict_info->branch2); + conflict_rename_rename_1to2(o, conflict_info); break; default: entry->processed = 0; -- cgit v1.2.1 From 461f5041178c02f3360920f42b542df7add0033f Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:15 -0600 Subject: merge-recursive: Defer rename/rename(2to1) handling until process_entry This puts the code for the different types of double rename conflicts closer together (fewer lines of other code separating the two paths) and increases similarity between how they are handled. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 104 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 62 insertions(+), 42 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 8020d4b513..ccb9343563 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -67,7 +67,8 @@ enum rename_type { RENAME_NORMAL = 0, RENAME_DELETE, RENAME_ONE_FILE_TO_ONE, - RENAME_ONE_FILE_TO_TWO + RENAME_ONE_FILE_TO_TWO, + RENAME_TWO_FILES_TO_ONE }; struct rename_conflict_info { @@ -1019,32 +1020,40 @@ static void conflict_rename_rename_1to2(struct merge_options *o, } static void conflict_rename_rename_2to1(struct merge_options *o, - struct rename *ren1, - const char *branch1, - struct rename *ren2, - const char *branch2) + struct rename_conflict_info *ci) { - char *path = ren1->pair->two->path; /* same as ren2->pair->two->path */ - /* Two files were renamed to the same thing. */ + /* Two files, a & b, were renamed to the same thing, c. */ + struct diff_filespec *a = ci->pair1->one; + struct diff_filespec *b = ci->pair2->one; + struct diff_filespec *c1 = ci->pair1->two; + struct diff_filespec *c2 = ci->pair2->two; + char *path = c1->path; /* == c2->path */ + + output(o, 1, "CONFLICT (rename/rename): " + "Rename %s->%s in %s. " + "Rename %s->%s in %s", + a->path, c1->path, ci->branch1, + b->path, c2->path, ci->branch2); + + remove_file(o, 1, a->path, would_lose_untracked(a->path)); + remove_file(o, 1, b->path, would_lose_untracked(b->path)); + if (o->call_depth) { struct merge_file_info mfi; mfi = merge_file(o, path, null_sha1, 0, - ren1->pair->two->sha1, ren1->pair->two->mode, - ren2->pair->two->sha1, ren2->pair->two->mode, - branch1, branch2); + c1->sha1, c1->mode, + c2->sha1, c2->mode, + ci->branch1, ci->branch2); output(o, 1, "Adding merged %s", path); update_file(o, 0, mfi.sha, mfi.mode, path); } else { - char *new_path1 = unique_path(o, path, branch1); - char *new_path2 = unique_path(o, path, branch2); + char *new_path1 = unique_path(o, path, ci->branch1); + char *new_path2 = unique_path(o, path, ci->branch2); output(o, 1, "Renaming %s to %s and %s to %s instead", - ren1->pair->one->path, new_path1, - ren2->pair->one->path, new_path2); + a->path, new_path1, b->path, new_path2); remove_file(o, 0, path, 0); - update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, - new_path1); - update_file(o, 0, ren2->pair->two->sha1, ren2->pair->two->mode, - new_path2); + update_file(o, 0, c1->sha1, c1->mode, new_path1); + update_file(o, 0, c2->sha1, c2->mode, new_path2); free(new_path2); free(new_path1); } @@ -1075,6 +1084,7 @@ static int process_renames(struct merge_options *o, struct rename *ren1 = NULL, *ren2 = NULL; const char *branch1, *branch2; const char *ren1_src, *ren1_dst; + struct string_list_item *lookup; if (i >= a_renames->nr) { ren2 = b_renames->items[j++].util; @@ -1106,30 +1116,30 @@ static int process_renames(struct merge_options *o, ren1 = tmp; } + if (ren1->processed) + continue; + ren1->processed = 1; ren1->dst_entry->processed = 1; /* BUG: We should only mark src_entry as processed if we * are not dealing with a rename + add-source case. */ ren1->src_entry->processed = 1; - if (ren1->processed) - continue; - ren1->processed = 1; - ren1_src = ren1->pair->one->path; ren1_dst = ren1->pair->two->path; if (ren2) { + /* One file renamed on both sides */ const char *ren2_src = ren2->pair->one->path; const char *ren2_dst = ren2->pair->two->path; enum rename_type rename_type; - /* Renamed in 1 and renamed in 2 */ if (strcmp(ren1_src, ren2_src) != 0) - die("ren1.src != ren2.src"); + die("ren1_src != ren2_src"); ren2->dst_entry->processed = 1; ren2->processed = 1; if (strcmp(ren1_dst, ren2_dst) != 0) { rename_type = RENAME_ONE_FILE_TO_TWO; + clean_merge = 0; } else { rename_type = RENAME_ONE_FILE_TO_ONE; /* BUG: We should only remove ren1_src in @@ -1149,9 +1159,32 @@ static int process_renames(struct merge_options *o, branch2, ren1->dst_entry, ren2->dst_entry); + } else if ((lookup = string_list_lookup(renames2Dst, ren1_dst))) { + /* Two different files renamed to the same thing */ + char *ren2_dst; + ren2 = lookup->util; + ren2_dst = ren2->pair->two->path; + if (strcmp(ren1_dst, ren2_dst) != 0) + die("ren1_dst != ren2_dst"); + + clean_merge = 0; + ren2->processed = 1; + /* + * BUG: We should only mark src_entry as processed + * if we are not dealing with a rename + add-source + * case. + */ + ren2->src_entry->processed = 1; + + setup_rename_conflict_info(RENAME_TWO_FILES_TO_ONE, + ren1->pair, + ren2->pair, + branch1, + branch2, + ren1->dst_entry, + ren2->dst_entry); } else { /* Renamed in 1, maybe changed in 2 */ - struct string_list_item *item; /* we only use sha1 and mode of these */ struct diff_filespec src_other, dst_other; int try_merge; @@ -1186,23 +1219,6 @@ static int process_renames(struct merge_options *o, branch2, ren1->dst_entry, NULL); - } else if ((item = string_list_lookup(renames2Dst, ren1_dst))) { - char *ren2_src, *ren2_dst; - ren2 = item->util; - ren2_src = ren2->pair->one->path; - ren2_dst = ren2->pair->two->path; - - clean_merge = 0; - ren2->processed = 1; - remove_file(o, 1, ren2_src, - renamed_stage == 3 || would_lose_untracked(ren1_src)); - - output(o, 1, "CONFLICT (rename/rename): " - "Rename %s->%s in %s. " - "Rename %s->%s in %s", - ren1_src, ren1_dst, branch1, - ren2_src, ren2_dst, branch2); - conflict_rename_rename_2to1(o, ren1, branch1, ren2, 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 @@ -1502,6 +1518,10 @@ static int process_entry(struct merge_options *o, clean_merge = 0; conflict_rename_rename_1to2(o, conflict_info); break; + case RENAME_TWO_FILES_TO_ONE: + clean_merge = 0; + conflict_rename_rename_2to1(o, conflict_info); + break; default: entry->processed = 0; break; -- cgit v1.2.1 From 232c635f7e22acb2ddf526753fdcd3d3718afe99 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:16 -0600 Subject: merge-recursive: Record more data needed for merging with dual renames When two different files are renamed to one, we need to be able to do three-way merges for both of those files. To do that, we need to record the sha1sum of the (possibly modified) file on the unrenamed side. Modify setup_rename_conflict_info() to take this extra information and record it when the rename_type is RENAME_TWO_FILES_TO_ONE. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 42 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index ccb9343563..27714c5bd7 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -79,6 +79,8 @@ struct rename_conflict_info { const char *branch2; struct stage_data *dst_entry1; struct stage_data *dst_entry2; + struct diff_filespec ren1_other; + struct diff_filespec ren2_other; }; /* @@ -100,7 +102,10 @@ static inline void setup_rename_conflict_info(enum rename_type rename_type, const char *branch1, const char *branch2, struct stage_data *dst_entry1, - struct stage_data *dst_entry2) + struct stage_data *dst_entry2, + struct merge_options *o, + struct stage_data *src_entry1, + struct stage_data *src_entry2) { struct rename_conflict_info *ci = xcalloc(1, sizeof(struct rename_conflict_info)); ci->rename_type = rename_type; @@ -118,6 +123,24 @@ static inline void setup_rename_conflict_info(enum rename_type rename_type, ci->pair2 = pair2; dst_entry2->rename_conflict_info = ci; } + + if (rename_type == RENAME_TWO_FILES_TO_ONE) { + /* + * For each rename, there could have been + * modifications on the side of history where that + * file was not renamed. + */ + int ostage1 = o->branch1 == branch1 ? 3 : 2; + int ostage2 = ostage1 ^ 1; + + ci->ren1_other.path = pair1->one->path; + hashcpy(ci->ren1_other.sha1, src_entry1->stages[ostage1].sha); + ci->ren1_other.mode = src_entry1->stages[ostage1].mode; + + ci->ren2_other.path = pair2->one->path; + hashcpy(ci->ren2_other.sha1, src_entry2->stages[ostage2].sha); + ci->ren2_other.mode = src_entry2->stages[ostage2].mode; + } } static int show(struct merge_options *o, int v) @@ -1158,7 +1181,10 @@ static int process_renames(struct merge_options *o, branch1, branch2, ren1->dst_entry, - ren2->dst_entry); + ren2->dst_entry, + o, + NULL, + NULL); } else if ((lookup = string_list_lookup(renames2Dst, ren1_dst))) { /* Two different files renamed to the same thing */ char *ren2_dst; @@ -1182,7 +1208,11 @@ static int process_renames(struct merge_options *o, branch1, branch2, ren1->dst_entry, - ren2->dst_entry); + ren2->dst_entry, + o, + ren1->src_entry, + ren2->src_entry); + } else { /* Renamed in 1, maybe changed in 2 */ /* we only use sha1 and mode of these */ @@ -1218,6 +1248,9 @@ static int process_renames(struct merge_options *o, branch1, branch2, ren1->dst_entry, + NULL, + o, + NULL, NULL); } else if ((dst_other.mode == ren1->pair->two->mode) && sha_eq(dst_other.sha1, ren1->pair->two->sha1)) { @@ -1269,6 +1302,9 @@ static int process_renames(struct merge_options *o, branch1, NULL, ren1->dst_entry, + NULL, + o, + NULL, NULL); } } -- cgit v1.2.1 From dac4741554e7672e69d1bc6b4f912cd16ad83b38 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:17 -0600 Subject: merge-recursive: Create function for merging with branchname:file markers We want to be able to reuse the code to do a three-way file content merge and have the conflict markers use both branchname and filename. Split it out into a separate function. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 42 +++++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 27714c5bd7..4ceb6aac88 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -942,6 +942,36 @@ static struct merge_file_info merge_file_1(struct merge_options *o, return result; } +static struct merge_file_info +merge_file_special_markers(struct merge_options *o, + const struct diff_filespec *one, + const struct diff_filespec *a, + const struct diff_filespec *b, + const char *branch1, + const char *filename1, + const char *branch2, + const char *filename2) +{ + char *side1 = NULL; + char *side2 = NULL; + struct merge_file_info mfi; + + if (filename1) { + side1 = xmalloc(strlen(branch1) + strlen(filename1) + 2); + sprintf(side1, "%s:%s", branch1, filename1); + } + if (filename2) { + side2 = xmalloc(strlen(branch2) + strlen(filename2) + 2); + sprintf(side2, "%s:%s", branch2, filename2); + } + + mfi = merge_file_1(o, one, a, b, + side1 ? side1 : branch1, side2 ? side2 : branch2); + free(side1); + free(side2); + return mfi; +} + static struct merge_file_info merge_file(struct merge_options *o, const char *path, const unsigned char *o_sha, int o_mode, @@ -1415,7 +1445,6 @@ static int merge_content(struct merge_options *o, struct rename_conflict_info *rename_conflict_info) { const char *reason = "content"; - char *side1 = NULL, *side2 = NULL; const char *path1 = NULL, *path2 = NULL; struct merge_file_info mfi; struct diff_filespec one, a, b; @@ -1445,18 +1474,13 @@ static int merge_content(struct merge_options *o, path2 = (rename_conflict_info->pair2 || o->branch2 == rename_conflict_info->branch1) ? pair1->two->path : pair1->one->path; - side1 = xmalloc(strlen(o->branch1) + strlen(path1) + 2); - side2 = xmalloc(strlen(o->branch2) + strlen(path2) + 2); - sprintf(side1, "%s:%s", o->branch1, path1); - sprintf(side2, "%s:%s", o->branch2, path2); if (dir_in_way(path, !o->call_depth)) df_conflict_remains = 1; } - mfi = merge_file_1(o, &one, &a, &b, - side1 ? side1 : o->branch1, side2 ? side2 : o->branch2); - free(side1); - free(side2); + mfi = merge_file_special_markers(o, &one, &a, &b, + o->branch1, path1, + o->branch2, path2); if (mfi.clean && !df_conflict_remains && sha_eq(mfi.sha, a_sha) && mfi.mode == a_mode) { -- cgit v1.2.1 From 434b8525e7a68893106f6360ff0a261f03c37512 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:18 -0600 Subject: merge-recursive: Consider modifications in rename/rename(2to1) conflicts Our previous conflict resolution for renaming two different files to the same name ignored the fact that each of those files may have modifications from both sides of history to consider. We need to do a three-way merge for each of those files, and then handle the conflict of both sets of merged contents trying to be recorded with the same name. It is important to note that this changes our strategy in the recursive case. After doing a three-way content merge of each of the files involved, we still are faced with the fact that we are trying to put both of the results (including conflict markers) into the same path. We could do another two-way merge, but I think that becomes confusing. Also, taking a hint from the modify/delete and rename/delete cases we handled earlier, a more useful "common ground" would be to keep the three-way content merge but record it with the original filename. The renames can still be detected, we just allow it to be done in the o->call_depth=0 case. This seems to result in simpler & easier to understand merge conflicts as well, as evidenced by some of the changes needed in our testsuite in t6036. (However, it should be noted that this change will cause problems those renames also occur along with a file being added whose name matches the source of the rename. Since git currently cannot detect rename/add-source situations, though, this codepath is not currently used for those cases anyway. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 4ceb6aac88..3124144011 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1081,6 +1081,8 @@ static void conflict_rename_rename_2to1(struct merge_options *o, struct diff_filespec *c1 = ci->pair1->two; struct diff_filespec *c2 = ci->pair2->two; char *path = c1->path; /* == c2->path */ + struct merge_file_info mfi_c1; + struct merge_file_info mfi_c2; output(o, 1, "CONFLICT (rename/rename): " "Rename %s->%s in %s. " @@ -1091,22 +1093,32 @@ static void conflict_rename_rename_2to1(struct merge_options *o, remove_file(o, 1, a->path, would_lose_untracked(a->path)); remove_file(o, 1, b->path, would_lose_untracked(b->path)); + mfi_c1 = merge_file_special_markers(o, a, c1, &ci->ren1_other, + o->branch1, c1->path, + o->branch2, ci->ren1_other.path); + mfi_c2 = merge_file_special_markers(o, b, &ci->ren2_other, c2, + o->branch1, ci->ren2_other.path, + o->branch2, c2->path); + if (o->call_depth) { - struct merge_file_info mfi; - mfi = merge_file(o, path, null_sha1, 0, - c1->sha1, c1->mode, - c2->sha1, c2->mode, - ci->branch1, ci->branch2); - output(o, 1, "Adding merged %s", path); - update_file(o, 0, mfi.sha, mfi.mode, path); + /* + * If mfi_c1.clean && mfi_c2.clean, then it might make + * sense to do a two-way merge of those results. But, I + * think in all cases, it makes sense to have the virtual + * merge base just undo the renames; they can be detected + * again later for the non-recursive merge. + */ + remove_file(o, 0, path, 0); + update_file(o, 0, mfi_c1.sha, mfi_c1.mode, a->path); + update_file(o, 0, mfi_c2.sha, mfi_c2.mode, b->path); } else { char *new_path1 = unique_path(o, path, ci->branch1); char *new_path2 = unique_path(o, path, ci->branch2); output(o, 1, "Renaming %s to %s and %s to %s instead", a->path, new_path1, b->path, new_path2); remove_file(o, 0, path, 0); - update_file(o, 0, c1->sha1, c1->mode, new_path1); - update_file(o, 0, c2->sha1, c2->mode, new_path2); + update_file(o, 0, mfi_c1.sha, mfi_c1.mode, new_path1); + update_file(o, 0, mfi_c2.sha, mfi_c2.mode, new_path2); free(new_path2); free(new_path1); } -- cgit v1.2.1 From b70332520d2ba0f32b5006feff86d71242061011 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:19 -0600 Subject: merge-recursive: Make modify/delete handling code reusable modify/delete and rename/delete share a lot of similarities; we'd like all the criss-cross and D/F conflict handling specializations to be shared between the two. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 82 ++++++++++++++++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 34 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 3124144011..500efffb34 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -992,6 +992,46 @@ static struct merge_file_info merge_file(struct merge_options *o, return merge_file_1(o, &one, &a, &b, branch1, branch2); } +static void handle_change_delete(struct merge_options *o, + const char *path, + const unsigned char *o_sha, int o_mode, + const unsigned char *a_sha, int a_mode, + const unsigned char *b_sha, int b_mode, + const char *change, const char *change_past) +{ + char *renamed = NULL; + if (dir_in_way(path, !o->call_depth)) { + renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2); + } + + if (o->call_depth) { + /* + * We cannot arbitrarily accept either a_sha or b_sha as + * correct; since there is no true "middle point" between + * them, simply reuse the base version for virtual merge base. + */ + remove_file_from_cache(path); + update_file(o, 0, o_sha, o_mode, renamed ? renamed : path); + } else if (!a_sha) { + output(o, 1, "CONFLICT (%s/delete): %s deleted in %s " + "and %s in %s. Version %s of %s left in tree%s%s.", + change, path, o->branch1, + change_past, o->branch2, o->branch2, path, + NULL == renamed ? "" : " at ", + NULL == renamed ? "" : renamed); + update_file(o, 0, b_sha, b_mode, renamed ? renamed : path); + } else { + output(o, 1, "CONFLICT (%s/delete): %s deleted in %s " + "and %s in %s. Version %s of %s left in tree%s%s.", + change, path, o->branch2, + change_past, o->branch1, o->branch1, path, + NULL == renamed ? "" : " at ", + NULL == renamed ? "" : renamed); + update_file(o, 0, a_sha, a_mode, renamed ? renamed : path); + } + free(renamed); +} + static void conflict_rename_delete(struct merge_options *o, struct diff_filepair *pair, const char *rename_branch, @@ -1409,44 +1449,18 @@ error_return: return ret; } -static void handle_delete_modify(struct merge_options *o, +static void handle_modify_delete(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) { - char *renamed = NULL; - if (dir_in_way(path, !o->call_depth)) { - renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2); - } - - if (o->call_depth) { - /* - * We cannot arbitrarily accept either a_sha or b_sha as - * correct; since there is no true "middle point" between - * them, simply reuse the base version for virtual merge base. - */ - remove_file_from_cache(path); - update_file(o, 0, o_sha, o_mode, renamed ? renamed : path); - } else if (!a_sha) { - output(o, 1, "CONFLICT (delete/modify): %s deleted in %s " - "and modified in %s. Version %s of %s left in tree%s%s.", - path, o->branch1, - o->branch2, o->branch2, path, - NULL == renamed ? "" : " at ", - NULL == renamed ? "" : renamed); - update_file(o, 0, b_sha, b_mode, renamed ? renamed : path); - } else { - output(o, 1, "CONFLICT (delete/modify): %s deleted in %s " - "and modified in %s. Version %s of %s left in tree%s%s.", - path, o->branch2, - o->branch1, o->branch1, path, - NULL == renamed ? "" : " at ", - NULL == renamed ? "" : renamed); - update_file(o, 0, a_sha, a_mode, renamed ? renamed : path); - } - free(renamed); - + handle_change_delete(o, + path, + o_sha, o_mode, + a_sha, a_mode, + b_sha, b_mode, + "modify", "modified"); } static int merge_content(struct merge_options *o, @@ -1612,7 +1626,7 @@ static int process_entry(struct merge_options *o, } else { /* Modify/delete; deleted side may have put a directory in the way */ clean_merge = 0; - handle_delete_modify(o, path, o_sha, o_mode, + handle_modify_delete(o, path, o_sha, o_mode, a_sha, a_mode, b_sha, b_mode); } } else if ((!o_sha && a_sha && !b_sha) || -- cgit v1.2.1 From e03acb8bc1f03827210235dd72578b8b26f19c51 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:20 -0600 Subject: merge-recursive: Have conflict_rename_delete reuse modify/delete code Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 500efffb34..f29aaf7fc6 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1037,24 +1037,38 @@ static void conflict_rename_delete(struct merge_options *o, const char *rename_branch, const char *other_branch) { - char *dest_name = pair->two->path; - int df_conflict = 0; + const struct diff_filespec *orig = pair->one; + const struct diff_filespec *dest = pair->two; + const char *path; + const unsigned char *a_sha = NULL; + const unsigned char *b_sha = NULL; + int a_mode = 0; + int b_mode = 0; + + if (rename_branch == o->branch1) { + a_sha = dest->sha1; + a_mode = dest->mode; + } else { + b_sha = dest->sha1; + b_mode = dest->mode; + } - 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); - if (dir_in_way(dest_name, !o->call_depth)) { - dest_name = unique_path(o, dest_name, rename_branch); - df_conflict = 1; + if (o->call_depth) { + remove_file_from_cache(dest->path); + path = orig->path; + } else { + path = dest->path; + update_stages(dest->path, NULL, + rename_branch == o->branch1 ? dest : NULL, + rename_branch == o->branch1 ? NULL : dest); } - update_file(o, 0, pair->two->sha1, pair->two->mode, dest_name); - if (df_conflict) - free(dest_name); + + handle_change_delete(o, + path, + orig->sha1, orig->mode, + a_sha, a_mode, + b_sha, b_mode, + "rename", "renamed"); } static void conflict_rename_rename_1to2(struct merge_options *o, -- cgit v1.2.1 From 1ac91b32b5f153a91c10fb236a3769541be60ae9 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:21 -0600 Subject: merge-recursive: add handling for rename/rename/add-dest/add-dest Each side of the rename in rename/rename(1to2) could potentially also be involved in a rename/add conflict. Ensure stages for such conflicts are also recorded. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index f29aaf7fc6..2059f25620 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1071,6 +1071,19 @@ static void conflict_rename_delete(struct merge_options *o, "rename", "renamed"); } +static struct diff_filespec *filespec_from_entry(struct diff_filespec *target, + struct stage_data *entry, + int stage) +{ + unsigned char *sha = entry->stages[stage].sha; + unsigned mode = entry->stages[stage].mode; + if (mode == 0 || is_null_sha1(sha)) + return NULL; + hashcpy(target->sha1, sha); + target->mode = mode; + return target; +} + static void conflict_rename_rename_1to2(struct merge_options *o, struct rename_conflict_info *ci) { @@ -1116,8 +1129,12 @@ static void conflict_rename_rename_1to2(struct merge_options *o, remove_file_from_cache(a->path); remove_file_from_cache(b->path); } else { - update_stages(a->path, NULL, a, NULL); - update_stages(b->path, NULL, NULL, b); + struct diff_filespec other; + update_stages(a->path, NULL, + a, filespec_from_entry(&other, ci->dst_entry1, 3)); + + update_stages(b->path, NULL, + filespec_from_entry(&other, ci->dst_entry2, 2), b); update_file(o, 0, a->sha1, a->mode, dst_name_a); update_file(o, 0, b->sha1, b->mode, dst_name_b); -- cgit v1.2.1 From 3672c971483020ba5255576aec0be670c76d019b Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:22 -0600 Subject: merge-recursive: Fix working copy handling for rename/rename/add/add If either side of a rename/rename(1to2) conflict is itself also involved in a rename/add-dest conflict, then we need to make sure both the rename and the added file appear in the working copy. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 73 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 25 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 2059f25620..c7d5a4591b 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1084,6 +1084,52 @@ static struct diff_filespec *filespec_from_entry(struct diff_filespec *target, return target; } +static void handle_file(struct merge_options *o, + struct diff_filespec *rename, + int stage, + struct rename_conflict_info *ci) +{ + char *dst_name = rename->path; + struct stage_data *dst_entry; + const char *cur_branch, *other_branch; + struct diff_filespec other; + struct diff_filespec *add; + + if (stage == 2) { + dst_entry = ci->dst_entry1; + cur_branch = ci->branch1; + other_branch = ci->branch2; + } else { + dst_entry = ci->dst_entry2; + cur_branch = ci->branch2; + other_branch = ci->branch1; + } + + add = filespec_from_entry(&other, dst_entry, stage ^ 1); + if (stage == 2) + update_stages(rename->path, NULL, rename, add); + else + update_stages(rename->path, NULL, add, rename); + + if (add) { + char *add_name = unique_path(o, rename->path, other_branch); + update_file(o, 0, add->sha1, add->mode, add_name); + + remove_file(o, 0, rename->path, 0); + dst_name = unique_path(o, rename->path, cur_branch); + } else { + if (dir_in_way(rename->path, !o->call_depth)) { + dst_name = unique_path(o, rename->path, cur_branch); + output(o, 1, "%s is a directory in %s adding as %s instead", + rename->path, other_branch, dst_name); + } + } + update_file(o, 0, rename->sha1, rename->mode, dst_name); + + if (dst_name != rename->path) + free(dst_name); +} + static void conflict_rename_rename_1to2(struct merge_options *o, struct rename_conflict_info *ci) { @@ -1091,10 +1137,6 @@ static void conflict_rename_rename_1to2(struct merge_options *o, struct diff_filespec *one = ci->pair1->one; struct diff_filespec *a = ci->pair1->two; struct diff_filespec *b = ci->pair2->two; - const char *dst_name_a = a->path; - const char *dst_name_b = b->path; - char *del[2]; - int delp = 0; output(o, 1, "CONFLICT (rename/rename): " "Rename \"%s\"->\"%s\" in branch \"%s\" " @@ -1102,16 +1144,6 @@ static void conflict_rename_rename_1to2(struct merge_options *o, one->path, a->path, ci->branch1, one->path, b->path, ci->branch2, o->call_depth ? " (left unresolved)" : ""); - if (dir_in_way(a->path, !o->call_depth)) { - dst_name_a = del[delp++] = unique_path(o, a->path, ci->branch1); - output(o, 1, "%s is a directory in %s adding as %s instead", - a->path, ci->branch2, dst_name_a); - } - if (dir_in_way(b->path, !o->call_depth)) { - dst_name_b = del[delp++] = unique_path(o, b->path, ci->branch2); - output(o, 1, "%s is a directory in %s adding as %s instead", - b->path, ci->branch1, dst_name_b); - } if (o->call_depth) { struct merge_file_info mfi; mfi = merge_file(o, one->path, @@ -1129,18 +1161,9 @@ static void conflict_rename_rename_1to2(struct merge_options *o, remove_file_from_cache(a->path); remove_file_from_cache(b->path); } else { - struct diff_filespec other; - update_stages(a->path, NULL, - a, filespec_from_entry(&other, ci->dst_entry1, 3)); - - update_stages(b->path, NULL, - filespec_from_entry(&other, ci->dst_entry2, 2), b); - - update_file(o, 0, a->sha1, a->mode, dst_name_a); - update_file(o, 0, b->sha1, b->mode, dst_name_b); + handle_file(o, a, 2, ci); + handle_file(o, b, 3, ci); } - while (delp--) - free(del[delp]); } static void conflict_rename_rename_2to1(struct merge_options *o, -- cgit v1.2.1 From f53d39778cf41cc93aa140bb41bb7d3a6590e179 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:25 -0600 Subject: merge-recursive: Fix spurious 'refusing to lose untracked file...' messages Calling update_stages() before update_file() can sometimes result in git thinking the file being updated is untracked (whenever update_stages moves it to stage 3). Reverse the call order, and add a big comment to update_stages to hopefully prevent others from making the same mistake. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index c7d5a4591b..2bebc9721c 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -526,6 +526,15 @@ static int update_stages(const char *path, const struct diff_filespec *o, const struct diff_filespec *a, const struct diff_filespec *b) { + + /* + * NOTE: It is usually a bad idea to call update_stages on a path + * before calling update_file on that same path, since it can + * sometimes lead to spurious "refusing to lose untracked file..." + * messages from update_file (via make_room_for path via + * would_lose_untracked). Instead, reverse the order of the calls + * (executing update_file first and then update_stages). + */ int clear = 1; int options = ADD_CACHE_OK_TO_ADD | ADD_CACHE_SKIP_DFCHECK; if (clear) @@ -1039,7 +1048,6 @@ static void conflict_rename_delete(struct merge_options *o, { const struct diff_filespec *orig = pair->one; const struct diff_filespec *dest = pair->two; - const char *path; const unsigned char *a_sha = NULL; const unsigned char *b_sha = NULL; int a_mode = 0; @@ -1053,22 +1061,21 @@ static void conflict_rename_delete(struct merge_options *o, b_mode = dest->mode; } + handle_change_delete(o, + o->call_depth ? orig->path : dest->path, + orig->sha1, orig->mode, + a_sha, a_mode, + b_sha, b_mode, + "rename", "renamed"); + if (o->call_depth) { remove_file_from_cache(dest->path); - path = orig->path; } else { - path = dest->path; update_stages(dest->path, NULL, rename_branch == o->branch1 ? dest : NULL, rename_branch == o->branch1 ? NULL : dest); } - handle_change_delete(o, - path, - orig->sha1, orig->mode, - a_sha, a_mode, - b_sha, b_mode, - "rename", "renamed"); } static struct diff_filespec *filespec_from_entry(struct diff_filespec *target, @@ -1106,11 +1113,6 @@ static void handle_file(struct merge_options *o, } add = filespec_from_entry(&other, dst_entry, stage ^ 1); - if (stage == 2) - update_stages(rename->path, NULL, rename, add); - else - update_stages(rename->path, NULL, add, rename); - if (add) { char *add_name = unique_path(o, rename->path, other_branch); update_file(o, 0, add->sha1, add->mode, add_name); @@ -1125,6 +1127,10 @@ static void handle_file(struct merge_options *o, } } update_file(o, 0, rename->sha1, rename->mode, dst_name); + if (stage == 2) + update_stages(rename->path, NULL, rename, add); + else + update_stages(rename->path, NULL, add, rename); if (dst_name != rename->path) free(dst_name); -- cgit v1.2.1 From 35a74abff32c32c455a74974130ad2af7d81dfd9 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:27 -0600 Subject: merge-recursive: Avoid unnecessary file rewrites Often times, a potential conflict at a path is resolved by merge-recursive by using the content that was already present at that location. In such cases, we do not want to overwrite the content that is already present, as that could trigger unnecessary recompilations. One of the patches earlier in this series ("merge-recursive: When we detect we can skip an update, actually skip it") fixed the cases that involved content merges, but there were a few other cases as well. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 2bebc9721c..71febe94a3 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1036,7 +1036,14 @@ static void handle_change_delete(struct merge_options *o, change_past, o->branch1, o->branch1, path, NULL == renamed ? "" : " at ", NULL == renamed ? "" : renamed); - update_file(o, 0, a_sha, a_mode, renamed ? renamed : path); + if (renamed) + update_file(o, 0, a_sha, a_mode, renamed); + /* + * No need to call update_file() on path when !renamed, since + * that would needlessly touch path. We could call + * update_file_flags() with update_cache=0 and update_wd=0, + * but that's a no-op. + */ } free(renamed); } @@ -1396,10 +1403,20 @@ static int process_renames(struct merge_options *o, NULL); } else if ((dst_other.mode == ren1->pair->two->mode) && sha_eq(dst_other.sha1, ren1->pair->two->sha1)) { - /* Added file on the other side - identical to the file being - renamed: clean merge */ - update_file(o, 1, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst); + /* + * Added file on the other side identical to + * the file being renamed: clean merge. + * Also, there is no need to overwrite the + * file already in the working copy, so call + * update_file_flags() instead of + * update_file(). + */ + update_file_flags(o, + ren1->pair->two->sha1, + ren1->pair->two->mode, + ren1_dst, + 1, /* update_cache */ + 0 /* update_wd */); } else if (!sha_eq(dst_other.sha1, null_sha1)) { clean_merge = 0; try_merge = 1; @@ -1727,7 +1744,8 @@ static int process_entry(struct merge_options *o, free(new_path); } else { output(o, 2, "Adding %s", path); - update_file(o, 1, sha, mode, path); + /* do not overwrite file if already present */ + update_file_flags(o, sha, mode, path, 1, !a_sha); } } else if (a_sha && b_sha) { /* Case C: Added in both (check for same permissions) and */ -- cgit v1.2.1 From 6d63070cacf88cd03b4bc73fd64fea046accd450 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 11 Aug 2011 23:20:29 -0600 Subject: merge-recursive: Fix virtual merge base for rename/rename(1to2)/add-dest Earlier in this series, the patch "merge-recursive: add handling for rename/rename/add-dest/add-dest" added code to handle the rename on each side of history also being involved in a rename/add conflict, but only did so in the non-recursive case. Add code for the recursive case, ensuring that the "added" files are not simply deleted. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 71febe94a3..78555b6a39 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1159,6 +1159,8 @@ static void conflict_rename_rename_1to2(struct merge_options *o, o->call_depth ? " (left unresolved)" : ""); if (o->call_depth) { struct merge_file_info mfi; + struct diff_filespec other; + struct diff_filespec *add; mfi = merge_file(o, one->path, one->sha1, one->mode, a->sha1, a->mode, @@ -1171,8 +1173,25 @@ static void conflict_rename_rename_1to2(struct merge_options *o, * unique path, or use that unique path instead of src here. */ update_file(o, 0, mfi.sha, mfi.mode, one->path); - remove_file_from_cache(a->path); - remove_file_from_cache(b->path); + + /* + * Above, we put the merged content at the merge-base's + * path. Now we usually need to delete both a->path and + * b->path. However, the rename on each side of the merge + * could also be involved in a rename/add conflict. In + * such cases, we should keep the added file around, + * resolving the conflict at that path in its favor. + */ + add = filespec_from_entry(&other, ci->dst_entry1, 2 ^ 1); + if (add) + update_file(o, 0, add->sha1, add->mode, a->path); + else + remove_file_from_cache(a->path); + add = filespec_from_entry(&other, ci->dst_entry2, 3 ^ 1); + if (add) + update_file(o, 0, add->sha1, add->mode, b->path); + else + remove_file_from_cache(b->path); } else { handle_file(o, a, 2, ci); handle_file(o, b, 3, ci); -- cgit v1.2.1 From f701aae0774f4517b46dd866812c269c78f1e198 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Fri, 12 Aug 2011 20:23:51 -0600 Subject: merge-recursive: Don't re-sort a list whose order we depend upon In record_df_conflict_files() we would resort the entries list using df_name_compare to get a convenient ordering. Unfortunately, this broke assumptions of the get_renames() code (via string_list_lookup() calls) which needed the list to be in the standard ordering. When those lookups would fail, duplicate stage_data entries could be inserted, causing the process_renames and process_entry code to fail (in particular, a path that that process_renames had marked as processed would still be processed anyway in process_entry due to the duplicate entry). Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 78555b6a39..d60fd7a0cd 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -400,6 +400,7 @@ static void record_df_conflict_files(struct merge_options *o, * and the file need to be present, then the D/F file will be * reinstated with a new unique name at the time it is processed. */ + struct string_list df_sorted_entries; const char *last_file = NULL; int last_len = 0; int i; @@ -412,14 +413,20 @@ static void record_df_conflict_files(struct merge_options *o, return; /* Ensure D/F conflicts are adjacent in the entries list. */ - qsort(entries->items, entries->nr, sizeof(*entries->items), + memset(&df_sorted_entries, 0, sizeof(struct string_list)); + for (i = 0; i < entries->nr; i++) { + struct string_list_item *next = &entries->items[i]; + string_list_append(&df_sorted_entries, next->string)->util = + next->util; + } + qsort(df_sorted_entries.items, entries->nr, sizeof(*entries->items), string_list_df_name_compare); string_list_clear(&o->df_conflict_file_set, 1); - for (i = 0; i < entries->nr; i++) { - const char *path = entries->items[i].string; + for (i = 0; i < df_sorted_entries.nr; i++) { + const char *path = df_sorted_entries.items[i].string; int len = strlen(path); - struct stage_data *e = entries->items[i].util; + struct stage_data *e = df_sorted_entries.items[i].util; /* * Check if last_file & path correspond to a D/F conflict; @@ -447,6 +454,7 @@ static void record_df_conflict_files(struct merge_options *o, last_file = NULL; } } + string_list_clear(&df_sorted_entries, 0); } struct rename { -- cgit v1.2.1