diff options
| -rw-r--r-- | merge-ort.c | 74 | ||||
| -rwxr-xr-x | t/t6423-merge-rename-directories.sh | 105 |
2 files changed, 153 insertions, 26 deletions
diff --git a/merge-ort.c b/merge-ort.c index 01f150ef3b..8b7de0fbd8 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -2398,6 +2398,27 @@ static void compute_collisions(struct strmap *collisions, } } +static void free_collisions(struct strmap *collisions) +{ + struct hashmap_iter iter; + struct strmap_entry *entry; + + /* Free each value in the collisions map */ + strmap_for_each_entry(collisions, &iter, entry) { + struct collision_info *info = entry->value; + string_list_clear(&info->source_files, 0); + } + /* + * In compute_collisions(), we set collisions.strdup_strings to 0 + * so that we wouldn't have to make another copy of the new_path + * allocated by apply_dir_rename(). But now that we've used them + * and have no other references to these strings, it is time to + * deallocate them. + */ + free_strmap_strings(collisions); + strmap_clear(collisions, 1); +} + static char *check_for_directory_rename(struct merge_options *opt, const char *path, unsigned side_index, @@ -2406,18 +2427,23 @@ static char *check_for_directory_rename(struct merge_options *opt, struct strmap *collisions, int *clean_merge) { - char *new_path = NULL; + char *new_path; struct strmap_entry *rename_info; - struct strmap_entry *otherinfo = NULL; + struct strmap_entry *otherinfo; const char *new_dir; + int other_side = 3 - side_index; + /* + * Cases where we don't have or don't want a directory rename for + * this path. + */ if (strmap_empty(dir_renames)) - return new_path; + return NULL; + if (strmap_get(&collisions[other_side], path)) + return NULL; rename_info = check_dir_renamed(path, dir_renames); if (!rename_info) - return new_path; - /* old_dir = rename_info->key; */ - new_dir = rename_info->value; + return NULL; /* * This next part is a little weird. We do not want to do an @@ -2443,6 +2469,7 @@ static char *check_for_directory_rename(struct merge_options *opt, * As it turns out, this also prevents N-way transient rename * confusion; See testcases 9c and 9d of t6043. */ + new_dir = rename_info->value; /* old_dir = rename_info->key; */ otherinfo = strmap_get_entry(dir_rename_exclusions, new_dir); if (otherinfo) { path_msg(opt, INFO_DIR_RENAME_SKIPPED_DUE_TO_RERENAME, 1, @@ -2454,7 +2481,8 @@ static char *check_for_directory_rename(struct merge_options *opt, } new_path = handle_path_level_conflicts(opt, path, side_index, - rename_info, collisions); + rename_info, + &collisions[side_index]); *clean_merge &= (new_path != NULL); return new_path; @@ -3171,18 +3199,15 @@ static int detect_regular_renames(struct merge_options *opt, static int collect_renames(struct merge_options *opt, struct diff_queue_struct *result, unsigned side_index, + struct strmap *collisions, struct strmap *dir_renames_for_side, struct strmap *rename_exclusions) { int i, clean = 1; - struct strmap collisions; struct diff_queue_struct *side_pairs; - struct hashmap_iter iter; - struct strmap_entry *entry; struct rename_info *renames = &opt->priv->renames; side_pairs = &renames->pairs[side_index]; - compute_collisions(&collisions, dir_renames_for_side, side_pairs); for (i = 0; i < side_pairs->nr; ++i) { struct diff_filepair *p = side_pairs->queue[i]; @@ -3198,7 +3223,7 @@ static int collect_renames(struct merge_options *opt, side_index, dir_renames_for_side, rename_exclusions, - &collisions, + collisions, &clean); possibly_cache_new_pair(renames, p, side_index, new_path); @@ -3224,20 +3249,6 @@ static int collect_renames(struct merge_options *opt, result->queue[result->nr++] = p; } - /* Free each value in the collisions map */ - strmap_for_each_entry(&collisions, &iter, entry) { - struct collision_info *info = entry->value; - string_list_clear(&info->source_files, 0); - } - /* - * In compute_collisions(), we set collisions.strdup_strings to 0 - * so that we wouldn't have to make another copy of the new_path - * allocated by apply_dir_rename(). But now that we've used them - * and have no other references to these strings, it is time to - * deallocate them. - */ - free_strmap_strings(&collisions); - strmap_clear(&collisions, 1); return clean; } @@ -3248,6 +3259,7 @@ static int detect_and_process_renames(struct merge_options *opt, { struct diff_queue_struct combined = { 0 }; struct rename_info *renames = &opt->priv->renames; + struct strmap collisions[3]; int need_dir_renames, s, i, clean = 1; unsigned detection_run = 0; @@ -3297,12 +3309,22 @@ static int detect_and_process_renames(struct merge_options *opt, ALLOC_GROW(combined.queue, renames->pairs[1].nr + renames->pairs[2].nr, combined.alloc); + for (i = MERGE_SIDE1; i <= MERGE_SIDE2; i++) { + int other_side = 3 - i; + compute_collisions(&collisions[i], + &renames->dir_renames[other_side], + &renames->pairs[i]); + } clean &= collect_renames(opt, &combined, MERGE_SIDE1, + collisions, &renames->dir_renames[2], &renames->dir_renames[1]); clean &= collect_renames(opt, &combined, MERGE_SIDE2, + collisions, &renames->dir_renames[1], &renames->dir_renames[2]); + for (i = MERGE_SIDE1; i <= MERGE_SIDE2; i++) + free_collisions(&collisions[i]); STABLE_QSORT(combined.queue, combined.nr, compare_pairs); trace2_region_leave("merge", "directory renames", opt->repo); diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 479db32cd6..99baf77cbf 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -5199,6 +5199,111 @@ test_expect_success '12k: Directory rename with sibling causes rename-to-self' ' ) ' +# Testcase 12l, Both sides rename a directory into the other side, both add +# a file which after directory renames are the same filename +# Commit O: sub1/file, sub2/other +# Commit A: sub3/file, sub2/{other, new_add_add_file_1} +# Commit B: sub1/{file, newfile}, sub1/sub2/{other, new_add_add_file_2} +# +# In words: +# A: sub1/ -> sub3/, add sub2/new_add_add_file_1 +# B: sub2/ -> sub1/sub2, add sub1/newfile, add sub1/sub2/new_add_add_file_2 +# +# Expected: sub3/{file, newfile, sub2/other} +# CONFLICT (add/add): sub1/sub2/new_add_add_file +# +# Note that sub1/newfile is not extraneous. Directory renames are only +# detected if they are needed, and they are only needed if the old directory +# had a new file added on the opposite side of history. So sub1/newfile +# is needed for there to be a sub1/ -> sub3/ rename. + +test_setup_12l () { + test_create_repo 12l_$1 && + ( + cd 12l_$1 && + + mkdir sub1 sub2 + echo file >sub1/file && + echo other >sub2/other && + git add sub1 sub2 && + git commit -m "O" && + + git branch O && + git branch A && + git branch B && + + git checkout A && + git mv sub1 sub3 && + echo conflicting >sub2/new_add_add_file && + git add sub2 && + test_tick && + git add -u && + git commit -m "A" && + + git checkout B && + echo dissimilar >sub2/new_add_add_file && + echo brand >sub1/newfile && + git add sub1 sub2 && + git mv sub2 sub1 && + test_tick && + git commit -m "B" + ) +} + +test_expect_merge_algorithm failure success '12l (B into A): Rename into each other + add/add conflict' ' + test_setup_12l BintoA && + ( + cd 12l_BintoA && + + git checkout -q A^0 && + + test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 && + + test_stdout_line_count = 5 git ls-files -s && + + git rev-parse >actual \ + :0:sub3/file :0:sub3/newfile :0:sub3/sub2/other \ + :2:sub1/sub2/new_add_add_file \ + :3:sub1/sub2/new_add_add_file && + git rev-parse >expect \ + O:sub1/file B:sub1/newfile O:sub2/other \ + A:sub2/new_add_add_file \ + B:sub1/sub2/new_add_add_file && + test_cmp expect actual && + + git ls-files -o >actual && + test_write_lines actual expect >expect && + test_cmp expect actual + ) +' + +test_expect_merge_algorithm failure success '12l (A into B): Rename into each other + add/add conflict' ' + test_setup_12l AintoB && + ( + cd 12l_AintoB && + + git checkout -q B^0 && + + test_must_fail git -c merge.directoryRenames=true merge -s recursive A^0 && + + test_stdout_line_count = 5 git ls-files -s && + + git rev-parse >actual \ + :0:sub3/file :0:sub3/newfile :0:sub3/sub2/other \ + :2:sub1/sub2/new_add_add_file \ + :3:sub1/sub2/new_add_add_file && + git rev-parse >expect \ + O:sub1/file B:sub1/newfile O:sub2/other \ + B:sub1/sub2/new_add_add_file \ + A:sub2/new_add_add_file && + test_cmp expect actual && + + git ls-files -o >actual && + test_write_lines actual expect >expect && + test_cmp expect actual + ) +' + ########################################################################### # SECTION 13: Checking informational and conflict messages # |