Comment 6 for bug 1888659

Revision history for this message
Colin Watson (cjwatson) wrote :

The design is intentionally contrary to what you're asking for, although the results happen to often be the same in practice for simple cases. The intent is that a merge proposal's preview diff shows what would be changed in the target branch if the source branch were to be merged into the target branch at the time when the preview diff was generated: or more briefly, "what would happen if I landed this thing?". It is specifically *not* intended that the preview diff be an exact representation of what changes were made in the source branch, and it may differ for various reasons: conflicts are one such case, but for instance it might also be that some of the changes were made on the target branch after the source branch diverged, in which case those changes will be omitted from the preview diff since they aren't part of the projected result of landing the merge proposal.

I will gladly argue and maintain that it's strictly more useful for the preview diff to have its current semantics. As a frequent reviewer of Launchpad merge proposals myself, I generally don't care very much exactly how the changes happened in the source branch: what I mainly care about, and what I argue most reviewers *should* care about, is what would happen to the target branch if I were to approve the merge proposal. (There may be secondary concerns such as clean history, but if a reviewer is concerned with those then they generally need to look directly at individual commits anyway.) It should be largely immaterial to a review whether the conflict markers result from the merge or whether they're literally present in the source branch: the proposer has to fix them either way.

Also, with the present semantics, it's always possible to click through to the source branch and look at individual diffs if you need more detail. By contrast, with the general semantics you're asking for in which the preview diff only represents the changes made to the source branch, there'd be no way to see the projected effect of the merge. (Note that there's no coherent way to represent the effect of a merge as a stream of text without sometimes ending up with some kind of conflict markers, or at least I've never seen any revision control system successfully do so. "See projected effect of merge" implies sometimes having conflict markers. Both "bzr merge" and "git merge" would do exactly the same thing if you tried to merge a branch in this sort of state.)

I don't recall the details for bzr right now, but at least for git I don't believe we have the information you're looking for at the per-line level, only at the per-file level. We ask pygit2 to do most of the work: it gives us a list of conflicted files, and for each of those files we ask it to try harder to produce a merged version (without doing that, all we have are the ancestor version and the version from each side of the merge, rather than anything resembling a diff). pygit2 then gives us a representation of the merged file containing conflict markers: without a considerable amount of extra analysis, we don't know whether any given one of those markers is literally present in the source branch or is an artifact of the trial merge. All we know is the list of files that contain some merge conflicts.

Having said all that: as I said, we already show the list of conflicted files at the top of the MP, in the form of the text "(has conflicts)" after the diffstat followed by a "Conflict in <path>" line for each of the conflicted files. However, that's quite distant from the preview diff, so I can see how you might have missed it. It would certainly be possible to display a clear note closer to the preview diff that the diff contains merge conflicts and so the merge proposal can't be landed until they're resolved; this could go in the sort of place where we might display other status indicators in future (e.g. from CI runs).