summaryrefslogtreecommitdiff
path: root/t/unit-tests/clar/example/main.c
diff options
context:
space:
mode:
authorElijah Newren <newren@gmail.com>2025-11-03 18:01:48 +0000
committerJunio C Hamano <gitster@pobox.com>2025-11-17 14:08:09 -0800
commita562d90a350dcbddc8794809aef9608467022e34 (patch)
tree640f47e62f53ae2923fd8a2e38c71a2a632f673a /t/unit-tests/clar/example/main.c
parentd5663a4b05640a44aa52a0cc32ba6a601d7c9149 (diff)
merge-ort: fix failing merges in special corner case
At GitHub, we had a repository that was triggering git: merge-ort.c:3032: process_renames: Assertion `newinfo && !newinfo->merged.clean` failed. during git replay. This sounds similar to the somewhat recent f6ecb603ff8a (merge-ort: fix directory rename on top of source of other rename/delete, 2025-08-06), but the cause is different. Unlike that case, there are no rename-to-self situations arising in this case, and new to this case it can only be triggered during a replay operation on the 2nd or later commit being replayed, never on the first merge in the sequence. To trigger, the repository needs: * an upstream which: * renames a file to a different directory, e.g. old/file -> new/file * leaves other files remaining in the original directory (so that e.g. "old/" still exists upstream even though file has been removed from it and placed elsewhere) * a topic branch being rebased where: * a commit in the sequence: * modifies old/file * a subsequent commit in the sequence being replayed: * does NOT touch *anything* under new/ * does NOT touch old/file * DOES modify other paths under old/ * does NOT have any relevant renames that we need to detect _anywhere_ elsewhere in the tree (meaning this interacts interestingly with both directory renames and cached renames) In such a case, the assertion will trigger. The fix turns out to be surprisingly simple. I have a very vague recollection that I actually considered whether to add such an if-check years ago when I added the very similar one for oldinfo in 1b6b902d95a5 (merge-ort: process_renames() now needs more defensiveness, 2021-01-19), but I think I couldn't figure out a possible way to trigger it and was worried at the time that if I didn't know how to trigger it then I wasn't so sure that simply skipping it was correct. Waiting did give me a chance to put more thorough tests and checks into place for the rename-to-self cases a few months back, which I might not have found as easily otherwise. Anyway, put the check in place now and add a test that demonstrates the fix. Note that this bug, as demonstrated by the conditions listed above, runs at the intersection of relevant renames, trivial directory resolutions, and cached renames. All three of those optimizations are ones that unfortunately make the code (and testcases!) a bit more complex, and threading all three makes it a bit more so. However, the testcase isn't crazy enough that I'd expect no one to ever hit it in practice, and was confused why we didn't see it before. After some digging, I discovered that merge.directoryRenames=false is a workaround to this bug, and GitHub used that setting until recently (it was a "temporary" match-what-libgit2-does piece of code that lasted years longer than intended). Since the conditions I gave above for triggering this bug rule out the possibility of there being directory renames, one might assume that it shouldn't matter whether you try to detect such renames if there aren't any. However, due to commit a16e8efe5c2b (merge-ort: fix merge.directoryRenames=false, 2025-03-13), the heavy hammer used there means that merge.directoryRenames=false ALSO turns off rename caching, which is critical to triggering the bug. This becomes a bit more than an aside since... Re-reading that old commit, a16e8efe5c2b (merge-ort: fix merge.directoryRenames=false, 2025-03-13), it appears that the solution to this latest bug might have been at least a partial alternative solution to that old commit. And it may have been an improved alternative (or at least help implement one), since it may be able to avoid the heavy-handed disabling of rename cache. That might be an interesting future thing to investigate, but is not critical for the current fix. However, since I spent time digging it all up, at least leave a small comment tweak breadcrumb to help some future reader (myself or others) who wants to dig further to connect the dots a little quicker. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 't/unit-tests/clar/example/main.c')
0 files changed, 0 insertions, 0 deletions