[PATCH 3 of 5] outgoing: add a 'missingroots' argument

[Gregory Szorc]

On Thu, Aug 11, 2016 at 1:06 PM, Pierre-Yves David <
pierre-yves.david at ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> # Date 1470774698 -7200
> #      Tue Aug 09 22:31:38 2016 +0200
> # Node ID 8c4fcb1244bdf79caadadc73fc1e489a160a07ec
> # Parent  9ff7059253fd00094799f592462590cd837fb457
> # EXP-Topic outgoing
> outgoing: add a 'missingroots' argument
>
> This argument can be used instead of 'commonheads' to determine the
> 'outgoing'
> set. We remove the outgoingbetween function as its role can now be handled
> by
> 'outgoing' itself.
>
> I've thought of using an external function instead of making the
> constructor
> more complicated. However, there is low hanging fruit to improve the
> current
> code flow by storing some side products of the processing of
> 'missingroots'. So
> in my opinion it make senses to add all this to the class.
>
> diff -r 9ff7059253fd -r 8c4fcb1244bd mercurial/changegroup.py
> --- a/mercurial/changegroup.py  Tue Aug 09 15:55:44 2016 +0200
> +++ b/mercurial/changegroup.py  Tue Aug 09 22:31:38 2016 +0200
> @@ -943,7 +943,7 @@ def changegroupsubset(repo, roots, heads
>      Another wrinkle is doing the reverse, figuring out which changeset in
>      the changegroup a particular filenode or manifestnode belongs to.
>      """
> -    outgoing = discovery.outgoingbetween(repo, roots, heads)
> +    outgoing = discovery.outgoing(repo, missingroots=roots,
> missingheads=heads)
>      bundler = getbundler(version, repo)
>      return getsubset(repo, outgoing, bundler, source)
>
> diff -r 9ff7059253fd -r 8c4fcb1244bd mercurial/discovery.py
> --- a/mercurial/discovery.py    Tue Aug 09 15:55:44 2016 +0200
> +++ b/mercurial/discovery.py    Tue Aug 09 22:31:38 2016 +0200
> @@ -76,11 +76,25 @@ class outgoing(object):
>      The sets are computed on demand from the heads, unless provided
> upfront
>      by discovery.'''
>
> -    def __init__(self, repo, commonheads=None, missingheads=None):
> +    def __init__(self, repo, commonheads=None, missingheads=None,
> +                 missingroots=None):
> +        # at least one of them must not be set
> +        assert None in (commonheads, missingroots)
>          cl = repo.changelog
>          if not missingheads:
>              missingheads = cl.heads()
> -        if not commonheads:
> +        if missingroots:
> +            discbases = []
> +            for n in missingroots:
> +                discbases.extend([p for p in cl.parents(n) if p !=
> nullid])
> +            # TODO remove call to nodesbetween.
> +            # TODO populate attributes on outgoing instance instead of
> setting
> +            # discbases.
> +            csets, roots, heads = cl.nodesbetween(missingroots,
> missingheads)
> +            included = set(csets)
> +            missingheads = heads
> +            commonheads = [n for n in discbases if n not in included]
> +        elif not commonheads:
>              commonheads = [nullid]
>          self.commonheads = commonheads
>          self.missingheads = missingheads
> @@ -106,27 +120,6 @@ class outgoing(object):
>              self._computecommonmissing()
>          return self._missing
>
> -def outgoingbetween(repo, roots, heads):
> -    """create an ``outgoing`` consisting of nodes between roots and heads
> -
> -    The ``missing`` nodes will be descendants of any of the ``roots`` and
> -    ancestors of any of the ``heads``, both are which are defined as a
> list
> -    of binary nodes.
> -    """
> -    cl = repo.changelog
> -    if not roots:
> -        roots = [nullid]
> -    discbases = []
> -    for n in roots:
> -        discbases.extend([p for p in cl.parents(n) if p != nullid])
> -    # TODO remove call to nodesbetween.
> -    # TODO populate attributes on outgoing instance instead of setting
> -    # discbases.
> -    csets, roots, heads = cl.nodesbetween(roots, heads)
> -    included = set(csets)
> -    discbases = [n for n in discbases if n not in included]
> -    return outgoing(repo, discbases, heads)
> -
>  def findcommonoutgoing(repo, other, onlyheads=None, force=False,
>                         commoninc=None, portable=False):
>      '''Return an outgoing instance to identify the nodes present in repo
> but
>

I'm not thrilled about the complexity of outgoing.__init__. Personally, I'd
rather have a "dumb" container object that is populated by N specialized
functions (like "outgoingbetween"). But that's just my style.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20160814/876a0fa4/attachment.html>