diff --git a/merge-ort.c b/merge-ort.c index 00923ce3cd749b..544be9e466c9b5 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -2953,32 +2953,6 @@ static int process_renames(struct merge_options *opt, if (!oldinfo || oldinfo->merged.clean) continue; - /* - * Rename caching from a previous commit might give us an - * irrelevant rename for the current commit. - * - * Imagine: - * foo/A -> bar/A - * was a cached rename for the upstream side from the - * previous commit (without the directories being renamed), - * but the next commit being replayed - * * does NOT add or delete files - * * does NOT have directory renames - * * does NOT modify any files under bar/ - * * does NOT modify foo/A - * * DOES modify other files under foo/ (otherwise the - * !oldinfo check above would have already exited for - * us) - * In such a case, our trivial directory resolution will - * have already merged bar/, and our attempt to process - * the cached - * foo/A -> bar/A - * would be counterproductive, and lack the necessary - * information anyway. Skip such renames. - */ - if (!newinfo) - continue; - /* * diff_filepairs have copies of pathnames, thus we have to * use standard 'strcmp()' (negated) instead of '=='. @@ -3329,6 +3303,28 @@ static void use_cached_pairs(struct merge_options *opt, if (!new_name) new_name = old_name; + /* + * If this is a rename and the target path is either + * absent from opt->priv->paths (because a parent + * directory was trivially resolved) or already cleanly + * resolved (e.g. all three sides agree on its content), + * the cached rename is irrelevant for this commit. + * Skip it here rather than in process_renames() to + * preserve VERIFY_CI(newinfo)'s ability to catch bugs + * for non-cached renames (see 979ee83e8a90 (merge-ort: + * fix corner case recursive submodule/directory conflict + * handling, 2025-12-29) for an example of a bug that + * assertion caught). The rename remains in cached_pairs + * for use in subsequent commits. + */ + if (entry->value) { + struct merged_info *mi; + + mi = strmap_get(&opt->priv->paths, new_name); + if (!mi || mi->clean) + continue; + } + /* * cached_pairs has *copies* of old_name and new_name, * because it has to persist across merges. Since diff --git a/t/t6429-merge-sequence-rename-caching.sh b/t/t6429-merge-sequence-rename-caching.sh index 15dd2d94b75f0a..56ee9689898276 100755 --- a/t/t6429-merge-sequence-rename-caching.sh +++ b/t/t6429-merge-sequence-rename-caching.sh @@ -846,4 +846,64 @@ test_expect_success 'rename a file, use it on first pick, but irrelevant on seco ) ' +# +# In the following testcase: +# Base: subdir/file_1 +# Upstream: file_1 (renamed from subdir/file) +# Topic_1: subdir/file_2 (modified subdir/file) +# Topic_2: subdir/file_2, file_2 (added another "file" with same contents) +# Topic_3: file_2 (deleted subdir/file) +# +# +# This testcase presents no problems for git traditionally, but the fact that +# subdir/file -> file +# gets cached after the first pick presents a problem for the third commit +# to be replayed, because file has contents file_2 on all three sides and +# is thus trivially resolved early. The point of renames is to allow us to +# three-way merge contents across multiple filenames, but if the target is +# already resolved, we risk throwing an assertion. Verify that the code +# correctly drops the irrelevant rename in order to avoid hitting that +# assertion. +# +test_expect_success 'cached rename does not assert on trivially clean target' ' + git init cached-rename-trivially-clean-target && + ( + cd cached-rename-trivially-clean-target && + + mkdir subdir && + printf "%s\n" 1 2 3 >subdir/file && + git add subdir/file && + git commit -m orig && + + git branch upstream && + git branch topic && + + git switch upstream && + git mv subdir/file file && + git commit -m "rename subdir/file to file" && + + git switch topic && + + echo 4 >>subdir/file && + git add subdir/file && + git commit -m "modify subdir/file" && + + cp subdir/file file && + git add file && + git commit -m "copy subdir/file to file" && + + git rm subdir/file && + git commit -m "delete subdir/file" && + + git switch upstream && + git replay --onto HEAD upstream..topic && + git checkout topic && + + git ls-files >tracked-files && + test_line_count = 1 tracked-files && + printf "%s\n" 1 2 3 4 >expect && + test_cmp expect file + ) +' + test_done