Patchwork D3273: wireproto: convert legacy commands to command executor

login
register
mail settings
Submitter phabricator
Date April 12, 2018, 12:13 a.m.
Message ID <differential-rev-PHID-DREV-6w352tuodjvui6wsgpmm-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/30770/
State Superseded
Headers show

Comments

phabricator - April 12, 2018, 12:13 a.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Calls to the legacy commands "changegroup" and "changegroupsubset" have
  been ported to the new command executor interface.
  
  Because we always pass arguments by name and not position, some
  inconsistent names throughout the code base have been unified.
  
  As part of this change, we no longer had any remaining callers
  of the legacy command methods {between, branches, changegroup,
  changegroupsubset}. So, these interfaces/methods have been dropped
  from peer interfaces. We still have an interface declaring these
  methods. But that interface is implemented on the concrete peer
  types and isn't part of the generic peer interface. (The
  implementations of the command executor continue to call these
  methods.)
  
  The ultimate goal is to remove the per-command methods from the
  generic peer interface: the only interface-conforming way to
  call a command will be with the new executor API. At some point,
  we may want to move the methods outside of the peer classes and
  change the executor implementations to not call methods directly
  on a peer instance.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3273

AFFECTED FILES
  mercurial/bundlerepo.py
  mercurial/exchange.py
  mercurial/localrepo.py
  mercurial/repository.py
  mercurial/wireprotov1peer.py
  tests/test-check-interfaces.py

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/tests/test-check-interfaces.py b/tests/test-check-interfaces.py
--- a/tests/test-check-interfaces.py
+++ b/tests/test-check-interfaces.py
@@ -89,8 +89,7 @@ 
 
     checkzobject(badpeer())
 
-    ziverify.verifyClass(repository.ipeerbaselegacycommands,
-                         httppeer.httppeer)
+    ziverify.verifyClass(repository.ipeerbase, httppeer.httppeer)
     checkzobject(httppeer.httppeer(None, None, None, dummyopener(), None, None))
 
     ziverify.verifyClass(repository.ipeerconnection,
@@ -111,13 +110,11 @@ 
                          wireprotov1peer.peerexecutor)
     checkzobject(wireprotov1peer.peerexecutor(None))
 
-    ziverify.verifyClass(repository.ipeerbaselegacycommands,
-                         sshpeer.sshv1peer)
+    ziverify.verifyClass(repository.ipeerbase, sshpeer.sshv1peer)
     checkzobject(sshpeer.sshv1peer(ui, 'ssh://localhost/foo', None, dummypipe(),
                                    dummypipe(), None, None))
 
-    ziverify.verifyClass(repository.ipeerbaselegacycommands,
-                         sshpeer.sshv2peer)
+    ziverify.verifyClass(repository.ipeerbase, sshpeer.sshv2peer)
     checkzobject(sshpeer.sshv2peer(ui, 'ssh://localhost/foo', None, dummypipe(),
                                    dummypipe(), None, None))
 
diff --git a/mercurial/wireprotov1peer.py b/mercurial/wireprotov1peer.py
--- a/mercurial/wireprotov1peer.py
+++ b/mercurial/wireprotov1peer.py
@@ -255,7 +255,8 @@ 
             else:
                 f.set_result(result)
 
-class wirepeer(repository.legacypeer):
+@zi.implementer(repository.ipeerlegacycommands)
+class wirepeer(repository.peer):
     """Client-side interface for communicating with a peer repository.
 
     Methods commonly call wire protocol commands of the same name.
@@ -454,12 +455,12 @@ 
                 self._abort(error.ResponseError(_("unexpected response:"), d))
         return r
 
-    def changegroup(self, nodes, kind):
+    def changegroup(self, nodes, source):
         n = wireprototypes.encodelist(nodes)
         f = self._callcompressable("changegroup", roots=n)
         return changegroupmod.cg1unpacker(f, 'UN')
 
-    def changegroupsubset(self, bases, heads, kind):
+    def changegroupsubset(self, bases, heads, source):
         self.requirecap('changegroupsubset', _('look up remote changes'))
         bases = wireprototypes.encodelist(bases)
         heads = wireprototypes.encodelist(heads)
diff --git a/mercurial/repository.py b/mercurial/repository.py
--- a/mercurial/repository.py
+++ b/mercurial/repository.py
@@ -190,10 +190,10 @@ 
         Returns an iterable of iterables with the resolved values for each node.
         """
 
-    def changegroup(nodes, kind):
+    def changegroup(nodes, source):
         """Obtain a changegroup with data for descendants of specified nodes."""
 
-    def changegroupsubset(bases, heads, kind):
+    def changegroupsubset(bases, heads, source):
         pass
 
 class ipeercommandexecutor(zi.Interface):
@@ -275,9 +275,6 @@ 
     All peer instances must conform to this interface.
     """
 
-class ipeerbaselegacycommands(ipeerbase, ipeerlegacycommands):
-    """Unified peer interface that supports legacy commands."""
-
 @zi.implementer(ipeerbase)
 class peer(object):
     """Base class for peer repositories."""
@@ -302,10 +299,6 @@ 
             _('cannot %s; remote repository does not support the %r '
               'capability') % (purpose, name))
 
-@zi.implementer(ipeerbaselegacycommands)
-class legacypeer(peer):
-    """peer but with support for legacy wire protocol commands."""
-
 class ifilerevisionssequence(zi.Interface):
     """Contains index data for all revisions of a file.
 
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -321,7 +321,8 @@ 
 
     # End of peer interface.
 
-class locallegacypeer(repository.legacypeer, localpeer):
+@zi.implementer(repository.ipeerlegacycommands)
+class locallegacypeer(localpeer):
     '''peer extension which implements legacy methods too; used for tests with
     restricted capabilities'''
 
@@ -336,8 +337,8 @@ 
     def branches(self, nodes):
         return self._repo.branches(nodes)
 
-    def changegroup(self, basenodes, source):
-        outgoing = discovery.outgoing(self._repo, missingroots=basenodes,
+    def changegroup(self, nodes, source):
+        outgoing = discovery.outgoing(self._repo, missingroots=nodes,
                                       missingheads=self._repo.heads())
         return changegroup.makechangegroup(self._repo, outgoing, '01', source)
 
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -1693,13 +1693,27 @@ 
         cg = pullop.remote.getbundle('pull', common=pullop.common,
                                      heads=pullop.heads or pullop.rheads)
     elif pullop.heads is None:
-        cg = pullop.remote.changegroup(pullop.fetch, 'pull')
+        with pullop.remote.commandexecutor() as e:
+            fcg = e.callcommand('changegroup', {
+                'nodes': pullop.fetch,
+                'source': 'pull',
+            })
+
+        cg = fcg.result()
     elif not pullop.remote.capable('changegroupsubset'):
         raise error.Abort(_("partial pull cannot be done because "
                            "other repository doesn't support "
                            "changegroupsubset."))
     else:
-        cg = pullop.remote.changegroupsubset(pullop.fetch, pullop.heads, 'pull')
+        with pullop.remote.commandexecutor() as e:
+            fcg = e.callcommand('changegroupsubset', {
+                'bases': pullop.fetch,
+                'heads': pullop.heads,
+                'source': 'pull',
+            })
+
+        cg = fcg.result()
+
     bundleop = bundle2.applybundle(pullop.repo, cg, tr, 'pull',
                                    pullop.remote.url())
     pullop.cgresult = bundle2.combinechangegroupresults(bundleop)
diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py
--- a/mercurial/bundlerepo.py
+++ b/mercurial/bundlerepo.py
@@ -553,10 +553,24 @@ 
                 cg = other.getbundle('incoming', common=common, heads=rheads)
             elif onlyheads is None and not other.capable('changegroupsubset'):
                 # compat with older servers when pulling all remote heads
-                cg = other.changegroup(incoming, "incoming")
+
+                with other.commandexecutor() as e:
+                    fcg = e.callcommand('changegroup', {
+                        'nodes': incoming,
+                        'source': 'incoming',
+                    })
+
+                cg = fcg.result()
                 rheads = None
             else:
-                cg = other.changegroupsubset(incoming, rheads, 'incoming')
+                with other.commandexecutor() as e:
+                    fcg = e.callcommand('changegroupsubset', {
+                        'bases': incoming,
+                        'heads': rheads,
+                        'source': 'incoming',
+                    })
+
+                cg = fcg.result()
             if localrepo:
                 bundletype = "HG10BZ"
             else: