[PATCH 2 of 2 V2] revset: new predicates to find key merge revisions

[Pierre-Yves David]

On 03/10/2016 02:35 PM, Simon Farnsworth wrote:
> On 10/03/2016, 14:32, "Pierre-Yves David" <pierre-yves.david at ens-lyon.org> wrote:
>
>
>> On 03/10/2016 02:26 PM, Simon Farnsworth wrote:
>>> On 10/03/2016, 13:51, "Pierre-Yves David" <pierre-yves.david at ens-lyon.org> wrote:
>>>
>>>
>>>> On 03/10/2016 10:12 AM, Simon Farnsworth wrote:
>>>>> On 10/03/2016, 00:17, "Pierre-Yves David" <pierre-yves.david at ens-lyon.org> wrote:
>>>>>
>>>>>
>>>>>> On 03/07/2016 06:27 PM, Simon Farnsworth wrote:
>>>>>>> # HG changeset patch
>>>>>>> # User Simon Farnsworth <simonfar at fb.com>
>>>>>>> # Date 1457374657 0
>>>>>>> #      Mon Mar 07 18:17:37 2016 +0000
>>>>>>> # Node ID faf1fd7f7079801368317a681f55dbc46e878de0
>>>>>>> # Parent  d92485c84727d67bcbf462b26dcfc1b7c746d07d
>>>>>>> revset: new predicates to find key merge revisions
>>>>>>>
>>>>>>> When you have tricky merge conflicts, it's useful to look at the history of
>>>>>>> the conflict. In a fast moving repository, you're drowned in revisions that
>>>>>>> are in the merge but that can't impact on a conflict.
>>>>>>>
>>>>>>> Provide a new revset predicate to help out; conflict(type, pat) allows you
>>>>>>> to find the "base", "other" or "local" revision types for the merge
>>>>>>> conflicts identified by pat. This, in turn, enables you to write revsets to
>>>>>>> find all revisions that are of interest in a conflict situation.
>>>>>>
>>>>>> I'm a bit confused about this.
>>>>>>
>>>>>> Is there situation where "local" is not p1() and "other" is not p2()
>>>>>> (except for "update" merge were "local" does not exist)? If not, I'm not
>>>>>> sure why we need this predicate for them.
>>>>>
>>>>> These two predicates point you to the introrev of the conflict, not the merge points. I can clarify the commit message in a V3 submission. The goal is that between base, local, other, p1() and p2(), you have all the points you need to write revsets that find you interesting commits from the point of view of a human resolving the commit; for example, 'conflict("base", "file"):conflict("other", "file")' is all commits from the three way merge base to the latest change on the other side that could have caused the conflict.
>>>>
>>>> As said in another thread, I'm not super eager to promote "introrev" as
>>>> the one relevant for the "conflict" too much. especially because it is
>>>> already accessible using last(::p1() and file(FILENAME)).
>>>
>>> The main reason for looking at the introrev is that it's the last revision that's likely to be relevant, especially in a monolithic repository - commits after this one that did not touch this file are probably to other areas of the project, and we've already got p1() and p2() for the merge parents (albeit under horrible names).
>>>
>>>>
>>>> If having access introrev is really useful (for speed?) we could
>>>> probably add a keyword argument to the 'file()' revset to restrict the
>>>> search to a specific file revision.
>>>
>>> I'm not sure how the keyword argument is meant to help - could you explain this a bit further?
>>
>> Something like file(README, from=7916edd30816) would give you the
>> introrev of README as in 7916edd30816
>
> So I'd get conflict("local", "README") with file("README", from=p1())?

Yep

>
>>
>>> The introrev is cheaply available at this point, because I have a filectx in hand, hence going for it.
>>>
>>>>
>>>>>> What does "base" return exactly, I could not really infer it from the
>>>>>> documentation.
>>>>>
>>>>> It's the common ancestor of the two commits being merged - we call it "base" elsewhere in merge documentation, so I've kept that name rather than ancestor.
>>>>
>>>> I know what's the general meaning of "base" in a merge context, but what
>>>> are you returning here? the changeset common ancestor of the introrev of
>>>> that version? Also, there might be more than one common ancestors so It
>>>> is unclear to me how you behave in this case.
>>>
>>> In current code, there is only one common ancestor used as the base of the three-way merge for a file; that's the commit I'm returning. In this case, going back to the introrev seems unnecessary - we should have chosen an older node if that was a better merge base, so that's the node I'm returning.
>>
>> So we return the one initially picked by the merge code. What if there
>> is multiple file match by pattern.
>
> I return the set of base revisions - if there are multiple files, I can return multiple revisions.

Okay, If you think of a way to improve the documentation to make this 
clear that would probably help.


-- 
Pierre-Yves David