Skip to content

Commit

Permalink
Merge branch 'en/merge-dir-rename-corner-case-fix' into jch
Browse files Browse the repository at this point in the history 
The merge code had funny interactions between content based rename
detection and directory rename detection.

* en/merge-dir-rename-corner-case-fix:
  merge-recursive: handle rename-to-self case
  merge-ort: ensure we consult df_conflict and path_conflicts
  t6423: test directory renames causing rename-to-self
  • Loading branch information
@gitster
gitster committed Jul 8, 2021
2 parents adbc70a + 3585d0e commit 9f1136e
Show file tree
Hide file tree
Showing 3 changed files with 193 additions and 7 deletions.
6 changes: 5 additions & 1 deletion merge-ort.c
View file
Expand Up @@ -3270,7 +3270,7 @@ static void process_entry(struct merge_options *opt,
* above.
*/
if (ci->match_mask) {
ci->merged.clean = 1;
ci->merged.clean = !ci->df_conflict && !ci->path_conflict;
if (ci->match_mask == 6) {
/* stages[1] == stages[2] */
ci->merged.result.mode = ci->stages[1].mode;
Expand All @@ -3282,6 +3282,8 @@ static void process_entry(struct merge_options *opt,

ci->merged.result.mode = ci->stages[side].mode;
ci->merged.is_null = !ci->merged.result.mode;
if (ci->merged.is_null)
ci->merged.clean = 1;
oidcpy(&ci->merged.result.oid, &ci->stages[side].oid);

assert(othermask == 2 || othermask == 4);
Expand Down Expand Up @@ -3454,6 +3456,7 @@ static void process_entry(struct merge_options *opt,
path)) {
ci->merged.is_null = 1;
ci->merged.clean = 1;
assert(!ci->df_conflict && !ci->path_conflict);
} else if (ci->path_conflict &&
oideq(&ci->stages[0].oid, &ci->stages[side].oid)) {
/*
Expand All @@ -3480,6 +3483,7 @@ static void process_entry(struct merge_options *opt,
ci->merged.is_null = 1;
ci->merged.result.mode = 0;
oidcpy(&ci->merged.result.oid, null_oid());
assert(!ci->df_conflict);
ci->merged.clean = !ci->path_conflict;
}

Expand Down
19 changes: 13 additions & 6 deletions merge-recursive.c
View file
Expand Up @@ -2804,12 +2804,19 @@ static int process_renames(struct merge_options *opt,
int renamed_stage = a_renames == renames1 ? 2 : 3;
int other_stage = a_renames == renames1 ? 3 : 2;

/*
* Directory renames have a funny corner case...
*/
int renamed_to_self = !strcmp(ren1_src, ren1_dst);

/* BUG: We should only remove ren1_src in the base
* stage and in other_stage (think of rename +
* add-source case).
*/
remove_file(opt, 1, ren1_src,
renamed_stage == 2 || !was_tracked(opt, ren1_src));
if (!renamed_to_self)
remove_file(opt, 1, ren1_src,
renamed_stage == 2 ||
!was_tracked(opt, ren1_src));

oidcpy(&src_other.oid,
&ren1->src_entry->stages[other_stage].oid);
Expand All @@ -2823,6 +2830,9 @@ static int process_renames(struct merge_options *opt,
ren1->dir_rename_original_type == 'A') {
setup_rename_conflict_info(RENAME_VIA_DIR,
opt, ren1, NULL);
} else if (renamed_to_self) {
setup_rename_conflict_info(RENAME_NORMAL,
opt, ren1, NULL);
} else if (oideq(&src_other.oid, null_oid())) {
setup_rename_conflict_info(RENAME_DELETE,
opt, ren1, NULL);
Expand Down Expand Up @@ -3180,7 +3190,6 @@ static int handle_rename_normal(struct merge_options *opt,
struct rename *ren = ci->ren1;
struct merge_file_info mfi;
int clean;
int side = (ren->branch == opt->branch1 ? 2 : 3);

/* Merge the content and write it out */
clean = handle_content_merge(&mfi, opt, path, was_dirty(opt, path),
Expand All @@ -3190,9 +3199,7 @@ static int handle_rename_normal(struct merge_options *opt,
opt->detect_directory_renames == MERGE_DIRECTORY_RENAMES_CONFLICT &&
ren->dir_rename_original_dest) {
if (update_stages(opt, path,
NULL,
side == 2 ? &mfi.blob : NULL,
side == 2 ? NULL : &mfi.blob))
&mfi.blob, &mfi.blob, &mfi.blob))
return -1;
clean = 0; /* not clean, but conflicted */
}
Expand Down
175 changes: 175 additions & 0 deletions t/t6423-merge-rename-directories.sh
View file
Expand Up @@ -5024,6 +5024,181 @@ test_expect_failure '12h: renaming a file within a renamed directory' '
)
'

# Testcase 12i, Directory rename causes rename-to-self
# Commit O: source/{subdir/foo, bar, baz_1}
# Commit A: source/{foo, bar, baz_1}
# Commit B: source/{subdir/{foo, bar}, baz_2}
# Expected: source/{foo, bar, baz_2}, with conflicts on
# source/bar vs. source/subdir/bar

test_setup_12i () {
test_create_repo 12i &&
(
cd 12i &&

mkdir -p source/subdir &&
echo foo >source/subdir/foo &&
echo bar >source/bar &&
echo baz >source/baz &&
git add source &&
git commit -m orig &&

git branch O &&
git branch A &&
git branch B &&

git switch A &&
git mv source/subdir/foo source/foo &&
git commit -m A &&

git switch B &&
git mv source/bar source/subdir/bar &&
echo more baz >>source/baz &&
git commit -m B
)
}

test_expect_success '12i: Directory rename causes rename-to-self' '
test_setup_12i &&
(
cd 12i &&
git checkout A^0 &&
test_must_fail git -c merge.directoryRenames=conflict merge -s recursive B^0 &&
test_path_is_missing source/subdir &&
test_path_is_file source/bar &&
test_path_is_file source/baz &&
git ls-files | uniq >tracked &&
test_line_count = 3 tracked &&
git status --porcelain -uno >actual &&
cat >expect <<-\EOF &&
UU source/bar
M source/baz
EOF
test_cmp expect actual
)
'

# Testcase 12j, Directory rename to root causes rename-to-self
# Commit O: {subdir/foo, bar, baz_1}
# Commit A: {foo, bar, baz_1}
# Commit B: {subdir/{foo, bar}, baz_2}
# Expected: {foo, bar, baz_2}, with conflicts on bar vs. subdir/bar

test_setup_12j () {
test_create_repo 12j &&
(
cd 12j &&

mkdir -p subdir &&
echo foo >subdir/foo &&
echo bar >bar &&
echo baz >baz &&
git add . &&
git commit -m orig &&

git branch O &&
git branch A &&
git branch B &&

git switch A &&
git mv subdir/foo foo &&
git commit -m A &&

git switch B &&
git mv bar subdir/bar &&
echo more baz >>baz &&
git commit -m B
)
}

test_expect_success '12j: Directory rename to root causes rename-to-self' '
test_setup_12j &&
(
cd 12j &&
git checkout A^0 &&
test_must_fail git -c merge.directoryRenames=conflict merge -s recursive B^0 &&
test_path_is_missing subdir &&
test_path_is_file bar &&
test_path_is_file baz &&
git ls-files | uniq >tracked &&
test_line_count = 3 tracked &&
git status --porcelain -uno >actual &&
cat >expect <<-\EOF &&
UU bar
M baz
EOF
test_cmp expect actual
)
'

# Testcase 12k, Directory rename with sibling causes rename-to-self
# Commit O: dirB/foo, dirA/{bar, baz_1}
# Commit A: dirA/{foo, bar, baz_1}
# Commit B: dirB/{foo, bar}, dirA/baz_2
# Expected: dirA/{foo, bar, baz_2}, with conflicts on dirA/bar vs. dirB/bar

test_setup_12k () {
test_create_repo 12k &&
(
cd 12k &&

mkdir dirA dirB &&
echo foo >dirB/foo &&
echo bar >dirA/bar &&
echo baz >dirA/baz &&
git add . &&
git commit -m orig &&

git branch O &&
git branch A &&
git branch B &&

git switch A &&
git mv dirB/* dirA/ &&
git commit -m A &&

git switch B &&
git mv dirA/bar dirB/bar &&
echo more baz >>dirA/baz &&
git commit -m B
)
}

test_expect_success '12k: Directory rename with sibling causes rename-to-self' '
test_setup_12k &&
(
cd 12k &&
git checkout A^0 &&
test_must_fail git -c merge.directoryRenames=conflict merge -s recursive B^0 &&
test_path_is_missing dirB &&
test_path_is_file dirA/bar &&
test_path_is_file dirA/baz &&
git ls-files | uniq >tracked &&
test_line_count = 3 tracked &&
git status --porcelain -uno >actual &&
cat >expect <<-\EOF &&
UU dirA/bar
M dirA/baz
EOF
test_cmp expect actual
)
'

###########################################################################
# SECTION 13: Checking informational and conflict messages
#
Expand Down

0 comments on commit 9f1136e

Please sign in to comment.