[PATCH] merge: improve conflict markers by pointing to introrev

[Pierre-Yves David]

On 03/08/2016 01:24 PM, Simon Farnsworth wrote:
> On 08/03/2016, 12:41, "Pierre-Yves David" <pierre-yves.david at ens-lyon.org> wrote:
>
>
>> On 03/07/2016 05:12 PM, Simon Farnsworth wrote:
>>> On 07/03/2016, 14:56, "Yuya Nishihara" <youjah at gmail.com on behalf of yuya at tcha.org> wrote:
>>>
>>>
>>>> On Mon, 7 Mar 2016 13:58:29 +0000, Simon Farnsworth wrote:
>>>>> On 07/03/2016, 13:22, "Yuya Nishihara" <youjah at gmail.com on behalf of yuya at tcha.org> wrote:
>>>>>
>>>>>
>>>>>
>>>>>> On Fri, 4 Mar 2016 08:31:40 -0800, Simon Farnsworth wrote:
>>>>>>> # HG changeset patch
>>>>>>> # User Simon Farnsworth <simonfar at fb.com>
>>>>>>> # Date 1457034054 0
>>>>>>> #      Thu Mar 03 19:40:54 2016 +0000
>>>>>>> # Node ID f9c8bf605e5b96530f6a4636c78f9c444767c445
>>>>>>> # Parent  9d6c19c8413a039aff8399d6b1db573cb6610fab
>>>>>>> merge: improve conflict markers by pointing to introrev
>>>>>>>
>>>>>>> If you're working in a fast moving repository, the nodes chosen for conflict
>>>>>>> markers are apparently nonsensical - they're the revisions you're actually
>>>>>>> merging, even if the file in conflict did not change in those revisions.
>>>>>>>
>>>>>>> Change the conflict markers to find the introrev for local and other, and
>>>>>>> use that as the node instead. This means that the conflict markers now point
>>>>>>> to the commits in which the conflicting file was last changed.
>>>>>>
>>>>>> The default conflict marker includes {tags} and {bookmarks}. I don't think
>>>>>> they should be changed to the introrev. Perhaps we'll need new template keyword
>>>>>> for the introrev.
>>>>>
>>>>> Why do you think that {tags} and {bookmarks} should be on the nodes being merged, not on the introrevs?
>>>>>
>>>>> My reasoning for changing the node itself (hence also changing {tags} and {bookmarks}) is that when you're dealing with a merge conflict, you're not interested in the merge heads themselves - in the common cases, you either know exactly which nodes are being merged (because you're merging two trees of your own work together), or one is familiar (your own work), and the other is random (because you don't know the state of the remote tree at the point you did "hg pull"). Having the tags and bookmarks in the conflict marker come from the introrev means that I get told if the conflict happened at an interesting point in the tree. For slow-moving trees, this is uninteresting information - the introrev of a conflict is the same as the merge revision, because I simply don't get many conflicts in the first place; it's once you get into fast moving trees that conflicts get interesting, and in those cases, the merge nodes are pseudo-random anyway.
>>>>
>>>> If you're merging two bookmarked branches, {bookmarks} can provide which side
>>>> is which branch. I think that's why the conflict marker shows {bookmarks}.
>>>
>>> My assumption is that you already know which side is which bookmark - and I'm enhancing merge state to have descriptions of "local" and "other" that will help with that.
>>
>> It is not clear to me that "you already know which side is which
>> bookmark", people get confused easily especially on large merge.
>> Furthermore when they use command overlay that hide them the underlying
>> Mercurial invocation (like your employer do).
>
> I'm reworking this so that conflict markers can access both the merge heads and the introrev to get you tags and bookmarks from both the merge side and the introrev. The issue this patch tries to address is that if you've got a large, fast moving, monolithic repository, the conflict markers are confusing (and we've had complaints that Mercurial doesn't get them right) - you're working on the backend C++ codebase, but (for some reason that's beyond you), you've got a conflict that references Android work only.
>
> By pulling back to the introrev for the commit, you get a clearer conflict marker - you're now seeing a commit that actually touched the file in conflict, instead of an apparently random commit from later in the tree.
>
>>
>>>> Does the bookmark of the introrev provide something interesting?
>>>
>>> Depends on your workflow; it does if you're in the habit of creating bookmarks at interesting intermediate states, as then you'll get told that this conflict is with the interesting intermediate state, not with the latest state.
>>>
>>> I can see the point of giving the conflict markers both nodes, though (introrev and merge point), and defaulting to tags and bookmarks from the merge node, not the introrev node. I'll look into this for a V2.
>>
>> My general opinion on this "using introrev" business is that it is a red
>> herring. Revision touching a file are likely to be numberous so print
>> the latest one is going to marginaly help in some case and confuse user
>> in all the other. I would rather see provided an easy way to see the
>> "what revision touched that file on each side" information than trying
>> to try to fit very complexe information into the limited conflict marker
>> entry.
>
> That's the goal of the extra revset predicates (separate patch series); by providing you with an easy route to find the three interesting points of a three way merge on a file (introrev of the local and other parts of the merge, and the base node), you have the tools to write a revset alias that gets you all the commits that touched unresolved files (for example):

My point is that there is no such things as "the three interesting 
points of a three ways merge". The last changeset touching a file have 
no reason to be much more interesting than the second last one, etc…

-- 
Pierre-Yves David