[PATCH 2 of 2 evolve-ext] evolve: raise SplitSuccessorsError when computing dependency for split commits

[Pierre-Yves David]

On 06/22/2015 01:04 PM, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon at fb.com>
> # Date 1435002261 25200
> #      Mon Jun 22 12:44:21 2015 -0700
> # Node ID 2a0d074a400fcb595509453dedc6cbd8fc112f8f
> # Parent  54da8990174e356c3e22b2034072eb74ee9987e9
> evolve: raise SplitSuccessorsError when computing dependency for split commits
>
> We don't know how to handle dependency for split commits yet. When facing a
> splitted commit, instead of silently adding the first successor to the
> dependency list and fail later, we do nothing.

I've multiple feedback on this one. I've applied some of them in 
feffad5e832f, available in my WIP repo.

>
> diff --git a/hgext/evolve.py b/hgext/evolve.py
> --- a/hgext/evolve.py
> +++ b/hgext/evolve.py
> @@ -1316,6 +1316,18 @@ class MultipleSuccessorsError(RuntimeErr
>       def __init__(self, successorssets):
>           self.successorssets = successorssets
>
> +class SplitSuccessorsError(RuntimeError):
> +    """Exception raised by _singlesuccessor when one successor set exit with
> +    multiple successor (split commits)
> +
> +    The object contains the list of successors in its 'successors' attribute
> +    to call to easily recover.
> +    """

I do not things we need a specialised exception yet. We do nothing 
special in this case an we should just reuse the MultipleSuccessorsError 
for now.

> +
> +    def __init__(self, successors):
> +        self.successors = successors
> +
> +

Double empty line


>   def _singlesuccessor(repo, p):
>       """returns p (as rev) if not obsolete or its unique latest successors
>
> @@ -1335,6 +1347,8 @@ def _singlesuccessor(repo, p):
>           newer = obsolete.successorssets(repo, obs.node())
>       if len(newer) > 1:
>           raise MultipleSuccessorsError(newer)
> +    if len(newer[0]) > 1:
> +        raise SplitSuccessorsError(newer)
>
>       return repo[newer[0][0]].rev()
>
> @@ -1358,6 +1372,9 @@ def builddependencies(repo, revs):
>                   succ = _singlesuccessor(repo, p)
>               except MultipleSuccessorsError, exc:
>                   dependencies[r] = exc.successorssets
> +            except SplitSuccessorsError, exc:
> +                pass # The error is handled in the code doing the stabilization
> +

No need for a special treatment yet. let merge this in the previous one.

>               if succ in revs:
>                   dependencies[r].add(succ)
>                   rdependencies[succ].add(r)
> diff --git a/tests/test-evolve.t b/tests/test-evolve.t
> --- a/tests/test-evolve.t
> +++ b/tests/test-evolve.t
> @@ -1182,3 +1182,61 @@ Check that fold respects the allowunstab
>     [255]
>     $ hg fold --exact "19::"
>     2 changesets folded
> +
> +Check that evolve shows error while handling split commits
> +--------------------------------------

1) test-evolve.t is already quite huge, we should start a 
test-evolve-split. instead.

2) make sure you highlight we do not support it -yet- but would like to 
handle it at some point.

3) there is two kind of split situation (all on one branch or spread on 
multiple branch) you might want to introduce a test for each, or at 
least document which on you are testing.
-- 
Pierre-Yves David