Patchwork discovery: move code to create outgoing from roots and heads

login
register
mail settings
Submitter Gregory Szorc
Date Aug. 4, 2016, 5:09 a.m.
Message ID <298e109645948c545650.1470287354@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/16074/
State Accepted
Headers show

Comments

Gregory Szorc - Aug. 4, 2016, 5:09 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1470287272 25200
#      Wed Aug 03 22:07:52 2016 -0700
# Node ID 298e109645948c54565037e7c8859f8de7b72f63
# Parent  73ff159923c1f05899c27238409ca398342d9ae0
discovery: move code to create outgoing from roots and heads

changegroup.changegroupsubset() contained somewhat low-level code for
constructing an "outgoing" instance from a list of roots and heads
nodes. It feels like discovery.py is a more appropriate location for
this code.

This code can definitely be optimized, as outgoing.missing will
recompute the set of changesets we've already discovered from
cl.between(). But code shouldn't be refactored during a move, so
I've simply inserted a TODO calling attention to that.
Pierre-Yves David - Aug. 4, 2016, 2:49 p.m.
On 08/04/2016 07:09 AM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1470287272 25200
> #      Wed Aug 03 22:07:52 2016 -0700
> # Node ID 298e109645948c54565037e7c8859f8de7b72f63
> # Parent  73ff159923c1f05899c27238409ca398342d9ae0
> discovery: move code to create outgoing from roots and heads
>
> changegroup.changegroupsubset() contained somewhat low-level code for
> constructing an "outgoing" instance from a list of roots and heads
> nodes. It feels like discovery.py is a more appropriate location for
> this code.
>
> This code can definitely be optimized, as outgoing.missing will
> recompute the set of changesets we've already discovered from
> cl.between(). But code shouldn't be refactored during a move, so
> I've simply inserted a TODO calling attention to that.

Thanks. I've pushed this (with a minor tweak)

> +def outgoingbetween(repo, roots, heads):
> +    """Compute an ``outgoing`` consisting of nodes between roots and heads.

I changed, "compute" to "create" to make it clearer that this is a 
constructor function.

Patch

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -941,27 +941,17 @@  def changegroupsubset(repo, roots, heads
 
     It is fairly complex as determining which filenodes and which
     manifest nodes need to be included for the changeset to be complete
     is non-trivial.
 
     Another wrinkle is doing the reverse, figuring out which changeset in
     the changegroup a particular filenode or manifestnode belongs to.
     """
-    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.
-    csets, roots, heads = cl.nodesbetween(roots, heads)
-    included = set(csets)
-    discbases = [n for n in discbases if n not in included]
-    outgoing = discovery.outgoing(cl, discbases, heads)
+    outgoing = discovery.outgoingbetween(repo, roots, heads)
     bundler = getbundler(version, repo)
     return getsubset(repo, outgoing, bundler, source)
 
 def getlocalchangegroupraw(repo, source, outgoing, bundlecaps=None,
                            version='01'):
     """Like getbundle, but taking a discovery.outgoing as an argument.
 
     This is only implemented for local repos and reuses potentially
diff --git a/mercurial/discovery.py b/mercurial/discovery.py
--- a/mercurial/discovery.py
+++ b/mercurial/discovery.py
@@ -96,16 +96,37 @@  class outgoing(object):
         return self._common
 
     @util.propertycache
     def missing(self):
         if self._missing is None:
             self._computecommonmissing()
         return self._missing
 
+def outgoingbetween(repo, roots, heads):
+    """Compute 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(cl, 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
     not in other.
 
     If onlyheads is given, only nodes ancestral to nodes in onlyheads
     (inclusive) are included. If you already know the local repo's heads,
     passing them in onlyheads is faster than letting them be recomputed here.