undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge branch 'en/merge-dual-dir-renames-fix'

Browse Source 
Fixes a long-standing corner case bug around directory renames in
the merge-ort strategy.

* en/merge-dual-dir-renames-fix:
  merge-ort: fix issue with dual rename and add/add conflict
  merge-ort: shuffle the computation and cleanup of potential collisions
  merge-ort: make a separate function for freeing struct collisions
  merge-ort: small cleanups of check_for_directory_rename
  t6423: add tests of dual directory rename plus add/add conflict
This commit is contained in:
Junio C Hamano 2022-07-18 13:31:56 -07:00
commit e3349f2888
2 changed files with 153 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
merge-ort.c
View File

@ -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);

105
t/t6423-merge-rename-directories.sh
View File

@ -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
#