[PATCH evolve-ext] evolve: handle merge commit with single obsolete parent (issue4389)

[Andrew Halberstadt]

On 07/12/15 01:17 PM, Pierre-Yves David wrote:
>
>
> On 12/04/2015 05:33 PM, Andrew Halberstadt wrote:
>> # HG changeset patch
>> # User Andrew Halberstadt <ahalberstadt at mozilla.com>
>> # Date 1448588311 18000
>> #      Thu Nov 26 20:38:31 2015 -0500
>> # Node ID 51adab56f47c124702d2b7f2df1030a16c49678d
>> # Parent  333e056b3034aa328bc7a260b0cdcf641fe3d323
>> evolve: handle merge commit with single obsolete parent (issue4389)
>
> That patch is close to be ready. I just have a small feedback on the way
> we handle pctx=None
>
> (btw, is it even possible to have pctx to None now? should we forbid it?)

There is another call to relocate in _solvebumped that doesn't pass in 
pctx. I'm not 100% sure what a bumped changeset is, but if that's 
something that should be solved as part of issue4389, I can look into it.

Thanks. I'm about to head out to a work week and going on vacation 
after, so may be a couple weeks until I have time for a new patch.

-Andrew

>> This handles evolving merge commits with a single obsolete parent. Merge
>> commits with two obsolete parents are still unsupported. Note this
>> depends
>> on a change to merge.graft in core. Older versions of mercurial will not
>> have this functionality. Also, test-unstable.t will fail with older
>> versions.
>>
>> diff --git a/hgext/evolve.py b/hgext/evolve.py
>> --- a/hgext/evolve.py
>> +++ b/hgext/evolve.py
>> @@ -890,29 +890,26 @@ def rewrite(repo, old, updates, head, ne
>>           tr.close()
>>           return newid, created
>>       finally:
>>           lockmod.release(lock, wlock, tr)
>>
>>   class MergeFailure(util.Abort):
>>       pass
>>
>> -def relocate(repo, orig, dest, keepbranch=False):
>> +def relocate(repo, orig, dest, pctx=None, keepbranch=False):
>>       """rewrite <rev> on dest"""
>>       if orig.rev() == dest.rev():
>>           raise util.Abort(_('tried to relocate a node on top of
>> itself'),
>>                            hint=_("This shouldn't happen. If you still "
>>                                   "need to move changesets, please do
>> so "
>>                                   "manually with nothing to rebase -
>> working "
>>                                   "directory parent is also
>> destination"))
>>
>> -    if not orig.p2().rev() == node.nullrev:
>> -        raise util.Abort(
>> -            'no support for evolving merge changesets yet',
>> -            hint="Redo the merge and use `hg prune <old> --succ
>> <new>` to obsolete the old one")
>> +    pctx = pctx or orig.p1()
>
> We should still detect case where pctx is None and orig has two parents
> and bail out. This would catch bad API usage.
>
> Also, if you are testing, please explicitly test for None
>
> The mercurial blessed patter for this assignement is:
>
> if pctx is None:
>      pctx = orig.p1()
 >
>>       destbookmarks = repo.nodebookmarks(dest.node())
>>       nodesrc = orig.node()
>>       destphase = repo[nodesrc].phase()
>>       commitmsg = orig.description()
>>
>>       cache = {}
>>       sha1s = re.findall(sha1re, commitmsg)
>>       unfi = repo.unfiltered()
>> @@ -942,17 +939,28 @@ def relocate(repo, orig, dest, keepbranc
>>           try:
>>               if repo['.'].rev() != dest.rev():
>>                   merge.update(repo, dest, False, True, False)
>>               if bmactive(repo):
>>                   repo.ui.status(_("(leaving bookmark %s)\n") %
>> bmactive(repo))
>>               bmdeactivate(repo)
>>               if keepbranch:
>>                   repo.dirstate.setbranch(orig.branch())
>> -            r = merge.graft(repo, orig, orig.p1(), ['local', 'graft'])
>> +
>> +            try:
>> +                r = merge.graft(repo, orig, pctx, ['local', 'graft'],
>> True)
>> +            except TypeError:
>> +                # not using recent enough mercurial
>> +                if len(orig.parents()) == 2:
>> +                    raise util.Abort(
>> +                        'no support for evolving merge changesets yet',
>> +                        hint="Redo the merge and use `hg prune <old>
>> --succ <new>` to obsolete the old one")
>> +
>> +                r = merge.graft(repo, orig, pctx, ['local', 'graft'])
>> +
>>               if r[-1]:  #some conflict
>>                   raise util.Abort(
>>                           'unresolved merge conflicts (see hg help
>> resolve)')
>>               if commitmsg is None:
>>                   commitmsg = orig.description()
>>               extra = dict(orig.extra())
>>               if 'branch' in extra:
>>                   del extra['branch']
>> @@ -1743,23 +1751,29 @@ def _aspiringdescendant(repo, revs):
>>               if unstable not in result:
>>                   tovisit.append(unstable)
>>                   result.add(unstable)
>>       return sorted(result - target)
>>
>>   def _solveunstable(ui, repo, orig, dryrun=False, confirm=False,
>>                      progresscb=None):
>>       """Stabilize a unstable changeset"""
>> -    obs = orig.parents()[0]
>> -    if not obs.obsolete() and len(orig.parents()) == 2:
>> -        obs = orig.parents()[1] # second parent is obsolete ?
>> -
>> -    if not obs.obsolete():
>> +    pctx = orig.p1()
>> +    if len(orig.parents()) == 2:
>> +        if not pctx.obsolete():
>> +            pctx = orig.p2()  # second parent is obsolete ?
>> +        elif orig.p2().obsolete():
>> +            raise util.Abort(
>> +                'no support for evolving merge changesets with two
>> obsolete parents yet',
>> +                hint="Redo the merge and use `hg prune <old> --succ
>> <new>` to obsolete the old one")
>> +
>
> Fun fact: the next step to be able to handle merge is to evolve unstable
> merge twice, one time for each parents. That will be further
> modification in:
> 1) this very block
> 2) the logic scheduling resolution to schedule these two steps.
>
> (not that this would be the most simple way to be able to evolve such
> merge, a nicer way probably involves a consensus merge like approach).
>