merge-ort: fix a crash in process_renames for a file transitively renamed to itself #1873
Conversation
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 <[email protected]>
[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 <[email protected]>
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 <[email protected]>
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
This patch series was integrated into seen via git@ae2c445. |
|
This patch series was integrated into seen via git@84edbe7. |
|
This patch series was integrated into next via git@8f38331. |
|
This branch is now known as |
|
This patch series was integrated into seen via git@565cb8f. |
|
This patch series was integrated into seen via git@70ef27a. |
|
This patch series was integrated into seen via git@1de4da1. |
|
This patch series was integrated into seen via git@69f1f4c. |
|
This patch series was integrated into seen via git@9b4c4f4. |
|
This patch series was integrated into seen via git@cc6d95b. |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
|
User |
|
User |
|
This patch series was integrated into seen via git@298521e. |
|
There was a status update in the "Cooking" section about the branch The merge-recursive and merge-ort machinery crashed in corner cases when certain renames are involved. Will cook in 'next'. source: <[email protected]> |
|
This patch series was integrated into seen via git@7b46eaa. |
|
This patch series was integrated into seen via git@40e3eec. |
|
There was a status update in the "Cooking" section about the branch The merge-recursive and merge-ort machinery crashed in corner cases when certain renames are involved. Will cook in 'next'. source: <[email protected]> |
|
This patch series was integrated into seen via git@9a99c05. |
|
This patch series was integrated into seen via git@bf81c71. |
|
This patch series was integrated into seen via git@145e84b. |
|
This patch series was integrated into seen via git@d0e6cc7. |
|
There was a status update in the "Cooking" section about the branch The merge-recursive and merge-ort machinery crashed in corner cases when certain renames are involved. Will merge to 'master'. source: <[email protected]> |
|
This patch series was integrated into seen via git@d333aa1. |
|
There was a status update in the "Cooking" section about the branch The merge-recursive and merge-ort machinery crashed in corner cases when certain renames are involved. Will merge to 'master'. source: <[email protected]> |
|
This patch series was integrated into seen via git@7cbf66a. |
|
This patch series was integrated into seen via git@9209cc8. |
|
There was a status update in the "Graduated to 'master'" section about the branch The merge-recursive and merge-ort machinery crashed in corner cases when certain renames are involved. source: <[email protected]> |
|
This patch series was integrated into seen via git@52241c9. |
|
This patch series was integrated into master via git@52241c9. |
|
This patch series was integrated into next via git@52241c9. |
|
Closed via 52241c9. |
3 participants
Maintainer note: this is not a recent regression; it need not be included in v2.49.0.
This is a follow-up to Dmitry's report of a case where both merge-ort and merge-recursive fail to properly handle a very specific type of conflict. See https://lore.kernel.org/git/[email protected]/ for the earlier thread; the first patch is an adaptation of his demonstrating the issue, and the second patch is my fix for it.
cc: Dmitry Goncharov [email protected]
cc: Taylor Blau [email protected]
cc: Elijah Newren [email protected]