From 98a1a00d53018c7e664644d886466a820aa5e6d7 Mon Sep 17 00:00:00 2001 From: Dmitry Goncharov Date: Thu, 6 Mar 2025 15:30:26 +0000 Subject: [PATCH 1/2] t6423: add a testcase causing a failed assertion in process_renames If one side of history renames a directory A/ -> B/, and the other side of history adds new files to A/, then directory rename detection notices and moves or suggests moving those new files to B/. A similar thing is done for paths renamed into A/, causing them to be transitively renamed into B/. But, if the file originally came from B/, then this can end up causing a file to be renamed back to itself. merge-ort crashes under this special case, due to a slightly overzealous assertion: git: merge-ort.c:3051: process_renames: Assertion `source_deleted || oldinfo->filemask & old_sidemask' failed. Aborted (core dumped) Add a testcase demonstrating this. Signed-off-by: Dmitry Goncharov [en: Instead of adding a new testsuite, place it near similar tests in t6423, adjusting to match the style of those tests. Tweak the commit message to not repeat the entire testcase, but just describe the bug. Also update the line number in the error message.] Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t6423-merge-rename-directories.sh | 41 +++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 94080c65d1..7b8d79f56c 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -5363,6 +5363,47 @@ test_expect_merge_algorithm failure success '12m: Change parent of renamed-dir t ) ' +test_setup_12n () { + git init 12n && + ( + cd 12n && + + mkdir tools && + echo hello >tools/hello && + git add tools/hello && + git commit -m "O" && + + git branch O && + git branch A && + git branch B && + + git switch A && + echo world >world && + git add world && + git commit -q world -m 'Add world' && + + git mv world tools/world && + git commit -m "Move world into tools/" && + + git switch B && + git mv tools/hello hello && + git commit -m "Move hello from tools/ to toplevel" + ) +} + +test_expect_failure '12n: Directory rename transitively makes rename back to self' ' + test_setup_12n && + ( + cd 12n && + + git checkout -q B^0 && + + test_must_fail git cherry-pick A^0 >out && + grep "CONFLICT (file location).*should perhaps be moved" out + ) +' + + ########################################################################### # SECTION 13: Checking informational and conflict messages # From 3adba40858036a5a44f550aaab5287ad135f5f87 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 6 Mar 2025 15:30:27 +0000 Subject: [PATCH 2/2] merge-ort: fix slightly overzealous assertion for rename-to-self merge-ort has a number of sanity checks on the file it is processing in process_renames(). One of these sanity checks was slightly overzealous because it indirectly assumed that a renamed file always ended up at a different path than where it started. That is normally an entirely fair assumption, but directory rename detection can make things interesting. As a quick refresher, if one side of history renames directory A/ -> B/, and the other side of history adds new files to A/, then directory rename detection notices and suggests moving those new files to B/. A similar thing is done for paths renamed into A/, causing them to be transitively renamed into B/. But, if the file originally came from B/, then this can end up causing a file to be renamed back to itself. It turns out the rest of the code following this assertion handled the case fine; the assertion was just an extra sanity check, not a rigid precondition. Therefore, simply adjust the assertion to pass under this special case as well. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-ort.c | 3 ++- t/t6423-merge-rename-directories.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/merge-ort.c b/merge-ort.c index 46e78c3ffa..b0ff2236af 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -3048,7 +3048,8 @@ static int process_renames(struct merge_options *opt, } } - assert(source_deleted || oldinfo->filemask & old_sidemask); + assert(source_deleted || oldinfo->filemask & old_sidemask || + !strcmp(pair->one->path, pair->two->path)); /* Need to check for special types of rename conflicts... */ if (collision && !source_deleted) { diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 7b8d79f56c..79d889b94c 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -5391,7 +5391,7 @@ test_setup_12n () { ) } -test_expect_failure '12n: Directory rename transitively makes rename back to self' ' +test_expect_success '12n: Directory rename transitively makes rename back to self' ' test_setup_12n && ( cd 12n &&