[PATCH 2 of 2 evolve-ext] evolve: (issue4386) cleanup, split, fold and bijection in `hg prune`

[Pierre-Yves David]

On 06/29/2015 04:18 PM, Matt Harbison wrote:
> On Mon, 29 Jun 2015 15:33:34 -0400, Pierre-Yves David
> <pierre-yves.david at ens-lyon.org> wrote:
>
>>
>>
>> On 06/26/2015 10:56 AM, Laurent Charignon wrote:
>>>
>>>> On Jun 26, 2015, at 1:19 AM, Pierre-Yves David
>>>> <pierre-yves.david at ens-lyon.org> wrote:
>>>>
>>>>
>>>>
>>>> On 06/25/2015 05:17 PM, Laurent Charignon wrote:
>>>>> # HG changeset patch
>>>>> # User Laurent Charignon <lcharignon at fb.com>
>>>>> # Date 1435264430 25200
>>>>> #      Thu Jun 25 13:33:50 2015 -0700
>>>>> # Node ID d2da33829da4b22bd7afd354f4780e8ab3d670a3
>>>>> # Parent  0a1bf08ebdd93bfb199c18db4ea4e23c071e73cc
>>>>> evolve: (issue4386) cleanup, split, fold and bijection in `hg prune`
>>>>>
>>>>> Before this patch, the prune command was splitting and folding
>>>>> implicitely
>>>>> based on the number of successors and precursors. This patch makes the
>>>>> two behavior explicit by requesting a flag to perform a split or a
>>>>> fold.
>>>>>
>>>>> diff --git a/hgext/evolve.py b/hgext/evolve.py
>>>>> --- a/hgext/evolve.py
>>>>> +++ b/hgext/evolve.py
>>>>> @@ -2092,6 +2092,8 @@ def _getmetadata(**opts):
>>>>>        ('s', 'succ', [], _("successor changeset")),
>>>>>        ('r', 'rev', [], _("revisions to prune")),
>>>>>        ('k', 'keep', None, _("does not modify working copy during
>>>>> prune")),
>>>>> +     ('', 'fold', False, _("perform a fold")),
>>>>> +     ('', 'split', False, _("perform a split")),
>>>>
>>>> s/perform/register/ ?
>>>
>>> I like perform better than register. Do you want to use "do" instead
>>> or one of "execute", "achieve", "complete" ?
>>> You like register because you register the operation by adding markers?
>>
>> We are performing neither a split of a fold here. Changesets already
>> exists and no new content is created. so "perform" fit badly here.
>>
>> We are just creating markers, making something obsolete. when we add a
>> successors flag, we do not create the successors either. We merely
>> register it as a successors.
>>
>>>> help is maybe a bit too dry, but could be okay.
>>>>
>>>>>        ('B', 'bookmark', '', _("remove revs only reachable from given"
>>>>>                                " bookmark"))] + metadataopts,
>>>>>       _('[OPTION] [-r] REV...'))
>>>>> @@ -2110,8 +2112,10 @@ def cmdprune(ui, repo, *revs, **opts):
>>>>>       You can use the ``--succ`` option to inform mercurial that a
>>>>> newer version
>>>>>       of the pruned changeset exists.
>>>>>
>>>>> -    If you precise one revision to prune and multiple successor,
>>>>> it is a split.
>>>>> -    If you precise several commits and a single successor, it is a
>>>>> fold.
>>>>> +    If you precise one revision to prune and multiple successor,
>>>>> it is a split
>>>>> +    and you have to add --split.
>>>>> +    If you precise several commits and a single successor, it is a
>>>>> fold and you
>>>>> +    have to add --fold.
>>>>>       If you precise more than one revision to prune and more than
>>>>> one successor
>>>>>       there has to be one successor per revision to prune and a 1-1
>>>>> bijection
>>>>>       is done.
>>>>> @@ -2120,6 +2124,8 @@ def cmdprune(ui, repo, *revs, **opts):
>>>>>       succs = opts['new'] + opts['succ']
>>>>>       bookmark = opts.get('bookmark')
>>>>>       metadata = _getmetadata(**opts)
>>>>> +    fold = opts.get('fold')
>>>>> +    split = opts.get('split')
>>
>> We probably want to bail out if multiple flag are specified.
>>
>>>>>       if bookmark:
>>>>>           marks,revs = _reachablefrombookmark(repo, revs, bookmark)
>>>>> @@ -2165,6 +2171,13 @@ def cmdprune(ui, repo, *revs, **opts):
>>>>>                   raise util.Abort(msg)
>>>>>               relations = [(p, (s,)) for p, s in zip(precs, sucs)]
>>>>>           else:
>>>>> +            if len(precs) == 1 and len(sucs) > 1 and not split:
>>>>> +                msg = "please add --split if you want to do a split"
>>>>
>>>> 1) We want to be more precise about why we abort
>>>> 2) Our canonical wording is "do you want X?"
>>>>
>>>> "1 precursor for %d successors, do you want --split?"
>>>
>>> Good point, I will update that
>>>>
>>>>> +                raise util.Abort(msg)
>>>>> +            if len(sucs) == 1 and len(precs) > 1 and not fold:
>>>>> +                msg = "please add --fold if you want to do a fold"
>>>>
>>>> ditto
>>>>
>>>>> +                raise util.Abort(msg)
>>>>> +
>>>>>               relations = [(p, sucs) for p in precs]
>>>>>
>>>>>           wdp = repo['.']
>>>>> diff --git a/tests/test-corrupt.t b/tests/test-corrupt.t
>>>>> --- a/tests/test-corrupt.t
>>>>> +++ b/tests/test-corrupt.t
>>>>> @@ -101,7 +101,7 @@
>>>>>        summary:     add A
>>>>>
>>>>>
>>>>> -  $ hg kill -n -1 -- -2 -3
>>>>> +  $ hg kill --fold -n -1 -- -2 -3
>>>>
>>>>
>>>> As we are looking at it, these `hg kill` want to be replaced by `hg
>>>> prune`
>>>
>>> Should we also remove the alias in another series?
>>
>> Sure, this belong to another series.
>
> Any chance of keeping it (maybe hidden)?  I've typed it so many times,
> it will probably take quite a bit of time to retrain.

We are talking about updating the test. Not the removing the alias.

-- 
Pierre-Yves David