[PATCH 1 of 2] graft: allow creating sibling grafts

[Durham Goode]

On 4/6/15 1:29 PM, Matt Mackall wrote:
> On Sun, 2015-04-05 at 19:24 -0700, Durham Goode wrote:
>> # HG changeset patch
>> # User Durham Goode <durham at fb.com>
>> # Date 1428260138 25200
>> #      Sun Apr 05 11:55:38 2015 -0700
>> # Node ID 409103499e22860c64bd334e671753dfcf0072a6
>> # Parent  4a4018831d2ebc3c9cae9c6613e6a2497b4f0993
>> graft: allow creating sibling grafts
>>
>> Previously it was impossible to graft a commit onto it's own parent (i.e. create
>> a copy of the commit). This is useful when wanting to create a backup of the
>> commit before continuing to amend it. This patch enables that behavior.
> I'm about +0 on these two patches from functionality level, but I'm
> finding the patch itself confusing.
>
>> diff --git a/mercurial/merge.py b/mercurial/merge.py
>> --- a/mercurial/merge.py
>> +++ b/mercurial/merge.py
>> @@ -1184,12 +1184,14 @@ def graft(repo, ctx, pctx, labels):
>>       labels - merge labels eg ['local', 'graft']
>>   
>>       """
>> +    wpctx = repo['.']
>> +    mergeancestor = repo.changelog.isancestor(wpctx.node(), ctx.node())
>> +    stats = update(repo, ctx.node(), True, True, False, pctx.node(),
>> +                   mergeancestor=mergeancestor, labels=labels)
> So.. we should allow merging with an ancestor whenever the target is an
> ancestor? If that's actually the case, then this patch should be a
> single line adding mergeancestor=True. But the only case we've described
> so far is just when the target is p1(source).
mergeancestor isn't just about merging with the ancestor, it's about 
telling the merge logic to always accept the incoming change (in the 
case of a prompt, like 'remote changed (c) but local deleted (d)') 
because we know they are more recent.  That's not the case for all 
grafts, so I didn't want to set it to True all the time.
>
> (I think we should in fact be able to graft patches onto ancestors. For
> instance, grafting a bug fix back on to the not-yet-diverged release
> point, but am less motivated by the particular abuse^Wuse case you have
> in mind. Similarly, I think there's value in tracking repeated grafts.)
We already can graft on to ancestors (using graft -f), just not the 
immediate parent (which is what this fixes).
>
>>       repo.dirstate.beginparentchange()
>> -    repo.setparents(repo['.'].node(), nullid)
>> +    repo.setparents(wpctx.node(), nullid)
> Why is this changing? Is it just a clean-up because we've made wpctx a
> variable above? If so, it actually obscures the intent a bit. We just
> want to drop the second parent _of the current state_, and it's not
> immediately obvious to someone reading the code that wpctx is still
> relevant after an update().
This was just cleanup since we already had the value, and update in this 
case doesn't change the working copy (i think?).  I see how it could be 
confusing though. I can change it back.