On Tue, Jan 5, 2021 at 10:12 AM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > > On Tue, Jan 5, 2021 at 4:34 AM Michel Dänzer <michel@xxxxxxxxxxx> wrote: > > On 2020-12-14 3:37 p.m., Michel Dänzer wrote: > > > (Originally reported as a GitLab issue: > > > https://gitlab.com/gitlab-org/gitlab/-/issues/292754) > > > > Does the lack of response mean this is considered not a bug? > > Perhaps the original slipped under the radar of the area experts. > Adding Elijah to the Cc: list... I'm not actually an expert with three-way content merging. That's the stuff handled by the xdiff library; I work on the stuff above that layer (covering things such as directory/file conflicts, rename detection, recursive merge base consolidation, not enough or too many files for three-way content merging, filetypes that can't be three-way content merged such as symlinks or submodules or binary files, etc.) It looks like Phillip already answered here too, but you had more questions. Maybe I could try to elaborate on the answer a tiny bit. I did try out the testcase and noted that while "git apply" fails (unless you pass the --3way flag), a "git rebase --apply" succeeds due to usage of the --3way flag which falls back to a three-way merge instead of doing a patch application. "git rebase --merge" succeeds since it jumps straight to the three-way merge. git-apply/git-am need three context lines to succeed, yes, but xdiff has a full copy of all three files it is merging. It does NOT apply patches. And if you line up the relevant sections of the three files, to me it looks like the relevant change made by gitlab-issue-292754/pre-rebase is a subset of the change made by gitlab-issue-292754/base. Yes, there's just one common unchanged line separating the two hunks in the second commit, which seems to be your point of contention, but it doesn't seem surprising to me at least. I think it's kind of similar to clean-textual-merge-but-semantic-conflict cases -- for example, one side adds a new caller of some function, and the other side changes the signature of that function. That will merge cleanly, but fail to even compile. The "context" is different, but the only way to correctly merge that is have a human (perhaps with the aid of an automated testsuite) take a look at the result. Such is always the case with merges. However, that's just my gut take and as I said, I'm by no means an expert with three-way content merges. Maybe an xdiff expert could comment more. Elijah
