[PATCH 3 of 3] merge: add file ancestor linknode to mergestate

[Martin von Zweigbergk]

On Wed, Jan 13, 2016 at 11:42 AM, Durham Goode <durham at fb.com> wrote:
>
>
>
>
>
> On 1/13/16, 10:47 AM, "Martin von Zweigbergk" <martinvonz at google.com> wrote:
>
>>On Wed, Jan 6, 2016 at 5:40 PM, Durham Goode <durham at fb.com> wrote:
>>> # HG changeset patch
>>> # User Durham Goode <durham at fb.com>
>>> # Date 1452127911 28800
>>> #      Wed Jan 06 16:51:51 2016 -0800
>>> # Node ID a8adf78352f494c7cf88221bd346ecdda2a9cdfa
>>> # Parent  7afd0e010fd8d7fea0e6339f5f280fa192c1e176
>>> merge: add file ancestor linknode to mergestate
>>>
>>> During a merge, each file has a current filenode, an other filenode, and an
>>> ancestor filenode. The ancestor filenode's linknode is not stored though, and we
>>> rely on the ability for the filectx() to look up the linknode by using the
>>> revlog's linkrev. In alternative backends (like remotefilelog), linkrev's may
>>> not be present or may have restriction that prevent arbitrary linkrev look up
>>> given a filenode.
>>>
>>> This patch accounts for that by storing the ancestor filenode's linknode in the
>>> merge state so that it is available later at resolution time.
>>>
>>> This results in some minor test changes due to the extra state, and due to us no
>>> longer always using the earliest commit (i.e. the linkrev target) for that
>>> filenode (which is ok, since we're still using an ancestor of current and other
>>> that uses the correct filenode).
>>
>>Why is the linknode looked up at merge time not always the same as
>>that looked up at resolution time? I'm guessing I misunderstood the
>>description.
>
> Because at merge time it's using the commit node from the common commit ancestor (not the actual linkrev target of the file node).
>
> For instance, if you had a repo with a DAG like:
>
> C->B->A
> D->B->A
> Where A creates file foo and C and D edit file foo (but B does not).
>
> If we merge C and D, the merge time commit node is 'B' since it is the commit common ancestor, but the linkrev for foo-v1 points at A, since that's what introduced foo-v1.  But using B is ok, since it also contains foo-v1.

Thanks for the explanation. I think I see what's going on now. It
seems like your patch doesn't seem to match the commit message. You
say "This patch accounts for that by storing the ancestor filenode's
linknode in the merge state so that it is available later at
resolution time." (i.e. commit A in your example), but what you
actually store is the merge's common ancestor (commit B in your
example). The first paragraph in the commit message could also use
some rephrasing, specifically the "The ancestor filenode's linknode is
not stored though", which to me implies that that's what you're about
to change.