Skip to content

merge-ort: handle cached rename & trivial resolution interaction better#2274

Open
newren wants to merge 1 commit intogit:masterfrom
newren:better-cache-rename-trivial-resolution-fix
Open

merge-ort: handle cached rename & trivial resolution interaction better#2274
newren wants to merge 1 commit intogit:masterfrom
newren:better-cache-rename-trivial-resolution-fix

Conversation

@newren
Copy link
Copy Markdown
Contributor

@newren newren commented Apr 19, 2026

Long-standing bug discovered somewhat recently.

Back in commit a562d90 (merge-ort: fix failing merges in special
corner case, 2025-11-03), we hit a rename assertion due to a trivial
directory resolution affecting the parent of a cached rename.  Since
the path didn't need to be considered, we side-stepped it with

   if (!newinfo)
     continue;

in process_renames().  We have since run into a case in production
where a trivial resolution of a file affects the direct target of a
cached rename rather than a parent directory of it.  Add a testcase
demonstrating this additional case.

Now, if we were to follow the lead of commit a562d90, we could
resolve this alternate case with an extra condition on the above if:

   if (!newinfo || newinfo->merged.clean)
     continue;

However, if we had done that earlier, we would have made 979ee83
(merge-ort: fix corner case recursive submodule/directory conflict
handling, 2025-12-29) harder to find and fix, and this particular
position for this condition isn't actually at the root of the issue
but downstream from it.

Instead, let's rip out this if-check from a562d90 and put in an
alternative that more directly addresses trivially resolved paths that
happen to be cached renames or parent directories thereof, which is a
better fix for the original testcase and which also solves the newly
added testcase as well.

Signed-off-by: Elijah Newren <newren@gmail.com>
@newren newren force-pushed the better-cache-rename-trivial-resolution-fix branch from e23f2e5 to bce3857 Compare April 19, 2026 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant