D3179: wireproto: port heads command to wire protocol v2

indygreg (Gregory Szorc) phabricator at mercurial-scm.org
Sat Apr 7 05:53:44 UTC 2018


indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  After much thought and consideration, wire protocol version 2's
  commands will be defined in different functions from the existing
  commands. This will make it easier to implement these commands
  because it won't require shoehorning things like response formatting
  and argument declaration into the same APIs.
  
  For example, wire protocol version 1 requires that commands declare
  a fixed and ordered list of argument names. It isn't really possible
  to insert new arguments or have optional arguments without
  breaking backwards compatibility. Wire protocol version 2, however,
  uses CBOR maps for passing arguments. So arguments a) can be
  optional b) can be added without BC c) can be strongly typed.
  
  This commit starts our trek towards reimplementing the wire protocol
  for version 2 with the heads command. It is pretty similar to the
  existing heads command. One added feature is it can be told to
  operate on only public phase changesets. This is useful for
  making discovery faster when a repo has tens of thousands of
  draft phase heads (such as Mozilla's "try" repository).
  
  The HTTPv2 server-side protocol has had its `getargs()` implementation
  updated to reflect that arguments are a map and not a list.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/help/internals/wireprotocol.txt
  mercurial/wireproto.py
  mercurial/wireprotoframing.py
  mercurial/wireprotoserver.py
  mercurial/wireprototypes.py
  tests/test-wireproto-command-heads.t

CHANGE DETAILS

diff --git a/tests/test-wireproto-command-heads.t b/tests/test-wireproto-command-heads.t
new file mode 100644
--- /dev/null
+++ b/tests/test-wireproto-command-heads.t
@@ -0,0 +1,96 @@
+  $ . $TESTDIR/wireprotohelpers.sh
+
+  $ hg init server
+  $ enablehttpv2 server
+  $ cd server
+  $ hg debugdrawdag << EOF
+  > H I J
+  > | | |
+  > E F G
+  > | |/
+  > C D
+  > |/
+  > B
+  > |
+  > A
+  > EOF
+
+  $ hg phase --force --secret J
+  $ hg phase --public E
+
+  $ hg log -r 'E + H + I + G + J' -T '{rev}:{node} {desc} {phase}\n'
+  4:78d2dca436b2f5b188ac267e29b81e07266d38fc E public
+  7:ae492e36b0c8339ffaf328d00b85b4525de1165e H draft
+  8:1d6f6b91d44aaba6d5e580bc30a9948530dbe00b I draft
+  6:29446d2dc5419c5f97447a8bc062e4cc328bf241 G draft
+  9:dec04b246d7cbb670c6689806c05ad17c835284e J secret
+
+  $ hg serve -p $HGPORT -d --pid-file hg.pid -E error.log
+  $ cat hg.pid > $DAEMON_PIDS
+
+All non-secret heads returned by default
+
+  $ sendhttpv2peer << EOF
+  > command heads
+  > EOF
+  creating http peer for wire protocol version 2
+  sending heads command
+  s>     POST /api/exp-http-v2-0001/ro/heads HTTP/1.1\r\n
+  s>     Accept-Encoding: identity\r\n
+  s>     accept: application/mercurial-exp-framing-0003\r\n
+  s>     content-type: application/mercurial-exp-framing-0003\r\n
+  s>     content-length: 20\r\n
+  s>     host: $LOCALIP:$HGPORT\r\n (glob)
+  s>     user-agent: Mercurial debugwireproto\r\n
+  s>     \r\n
+  s>     \x0c\x00\x00\x01\x00\x01\x01\x11\xa1DnameEheads
+  s> makefile('rb', None)
+  s>     HTTP/1.1 200 OK\r\n
+  s>     Server: testing stub value\r\n
+  s>     Date: $HTTP_DATE$\r\n
+  s>     Content-Type: application/mercurial-exp-framing-0003\r\n
+  s>     Transfer-Encoding: chunked\r\n
+  s>     \r\n
+  s>     48\r\n
+  s>     @\x00\x00\x01\x00\x02\x01F
+  s>     \x83T\x1dok\x91\xd4J\xab\xa6\xd5\xe5\x80\xbc0\xa9\x94\x850\xdb\xe0\x0bT\xaeI.6\xb0\xc83\x9f\xfa\xf3(\xd0\x0b\x85\xb4R]\xe1\x16^T)Dm-\xc5A\x9c_\x97Dz\x8b\xc0b\xe4\xcc2\x8b\xf2A
+  s>     \r\n
+  received frame(size=64; request=1; stream=2; streamflags=stream-begin; type=bytes-response; flags=eos|cbor)
+  s>     0\r\n
+  s>     \r\n
+  response: [[b'\x1dok\x91\xd4J\xab\xa6\xd5\xe5\x80\xbc0\xa9\x94\x850\xdb\xe0\x0b', b'\xaeI.6\xb0\xc83\x9f\xfa\xf3(\xd0\x0b\x85\xb4R]\xe1\x16^', b')Dm-\xc5A\x9c_\x97Dz\x8b\xc0b\xe4\xcc2\x8b\xf2A']]
+
+Requesting just the public heads works
+
+  $ sendhttpv2peer << EOF
+  > command heads
+  >     publiconly 1
+  > EOF
+  creating http peer for wire protocol version 2
+  sending heads command
+  s>     POST /api/exp-http-v2-0001/ro/heads HTTP/1.1\r\n
+  s>     Accept-Encoding: identity\r\n
+  s>     accept: application/mercurial-exp-framing-0003\r\n
+  s>     content-type: application/mercurial-exp-framing-0003\r\n
+  s>     content-length: 39\r\n
+  s>     host: $LOCALIP:$HGPORT\r\n (glob)
+  s>     user-agent: Mercurial debugwireproto\r\n
+  s>     \r\n
+  s>     \x1f\x00\x00\x01\x00\x01\x01\x11\xa2Dargs\xa1JpubliconlyA1DnameEheads
+  s> makefile('rb', None)
+  s>     HTTP/1.1 200 OK\r\n
+  s>     Server: testing stub value\r\n
+  s>     Date: $HTTP_DATE$\r\n
+  s>     Content-Type: application/mercurial-exp-framing-0003\r\n
+  s>     Transfer-Encoding: chunked\r\n
+  s>     \r\n
+  s>     1e\r\n
+  s>     \x16\x00\x00\x01\x00\x02\x01F
+  s>     \x81Tx\xd2\xdc\xa46\xb2\xf5\xb1\x88\xac&~)\xb8\x1e\x07&m8\xfc
+  s>     \r\n
+  received frame(size=22; request=1; stream=2; streamflags=stream-begin; type=bytes-response; flags=eos|cbor)
+  s>     0\r\n
+  s>     \r\n
+  response: [[b'x\xd2\xdc\xa46\xb2\xf5\xb1\x88\xac&~)\xb8\x1e\x07&m8\xfc']]
+
+  $ cat error.log
diff --git a/mercurial/wireprototypes.py b/mercurial/wireprototypes.py
--- a/mercurial/wireprototypes.py
+++ b/mercurial/wireprototypes.py
@@ -97,6 +97,11 @@
     def __init__(self, gen=None):
         self.gen = gen
 
+class cborresponse(object):
+    """Encode the response value as CBOR."""
+    def __init__(self, v):
+        self.value = v
+
 class baseprotocolhandler(zi.Interface):
     """Abstract base class for wire protocol handlers.
 
@@ -115,7 +120,10 @@
     def getargs(args):
         """return the value for arguments in <args>
 
-        returns a list of values (same order as <args>)"""
+        For version 1 transports, returns a list of values in the same
+        order they appear in ``args``. For version 2 transports, returns
+        a dict mapping argument name to value.
+        """
 
     def getprotocaps():
         """Returns the list of protocol-level capabilities of client
diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py
--- a/mercurial/wireprotoserver.py
+++ b/mercurial/wireprotoserver.py
@@ -12,6 +12,9 @@
 import threading
 
 from .i18n import _
+from .thirdparty import (
+    cbor,
+)
 from .thirdparty.zope import (
     interface as zi,
 )
@@ -563,6 +566,12 @@
         action, meta = reactor.onbytesresponseready(outstream,
                                                     command['requestid'],
                                                     rsp.data)
+    elif isinstance(rsp, wireprototypes.cborresponse):
+        encoded = cbor.dumps(rsp.value, canonical=True)
+        action, meta = reactor.onbytesresponseready(outstream,
+                                                    command['requestid'],
+                                                    encoded,
+                                                    iscbor=True)
     else:
         action, meta = reactor.onapplicationerror(
             _('unhandled response type from wire proto command'))
@@ -600,10 +609,10 @@
         for k in args.split():
             if k == '*':
                 raise NotImplementedError('do not support * args')
-            else:
+            elif k in self._args:
                 data[k] = self._args[k]
 
-        return [data[k] for k in args.split()]
+        return data
 
     def getprotocaps(self):
         # Protocol capabilities are currently not implemented for HTTP V2.
diff --git a/mercurial/wireprotoframing.py b/mercurial/wireprotoframing.py
--- a/mercurial/wireprotoframing.py
+++ b/mercurial/wireprotoframing.py
@@ -346,18 +346,22 @@
             if done:
                 break
 
-def createbytesresponseframesfrombytes(stream, requestid, data,
+def createbytesresponseframesfrombytes(stream, requestid, data, iscbor=False,
                                        maxframesize=DEFAULT_MAX_FRAME_SIZE):
     """Create a raw frame to send a bytes response from static bytes input.
 
     Returns a generator of bytearrays.
     """
 
     # Simple case of a single frame.
     if len(data) <= maxframesize:
+        flags = FLAG_BYTES_RESPONSE_EOS
+        if iscbor:
+            flags |= FLAG_BYTES_RESPONSE_CBOR
+
         yield stream.makeframe(requestid=requestid,
                                typeid=FRAME_TYPE_BYTES_RESPONSE,
-                               flags=FLAG_BYTES_RESPONSE_EOS,
+                               flags=flags,
                                payload=data)
         return
 
@@ -372,6 +376,9 @@
         else:
             flags = FLAG_BYTES_RESPONSE_CONTINUATION
 
+        if iscbor:
+            flags |= FLAG_BYTES_RESPONSE_CBOR
+
         yield stream.makeframe(requestid=requestid,
                                typeid=FRAME_TYPE_BYTES_RESPONSE,
                                flags=flags,
@@ -605,16 +612,17 @@
 
         return meth(frame)
 
-    def onbytesresponseready(self, stream, requestid, data):
+    def onbytesresponseready(self, stream, requestid, data, iscbor=False):
         """Signal that a bytes response is ready to be sent to the client.
 
         The raw bytes response is passed as an argument.
         """
         ensureserverstream(stream)
 
         def sendframes():
             for frame in createbytesresponseframesfrombytes(stream, requestid,
-                                                            data):
+                                                            data,
+                                                            iscbor=iscbor):
                 yield frame
 
             self._activecommands.remove(requestid)
diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
--- a/mercurial/wireproto.py
+++ b/mercurial/wireproto.py
@@ -518,7 +518,15 @@
     func, spec = commandtable[command]
 
     args = proto.getargs(spec)
-    return func(repo, proto, *args)
+
+    # Version 1 protocols define arguments as a list. Version 2 uses a dict.
+    if isinstance(args, list):
+        return func(repo, proto, *args)
+    elif isinstance(args, dict):
+        return func(repo, proto, **args)
+    else:
+        raise error.ProgrammingError('unexpected type returned from '
+                                     'proto.getargs(): %s' % type(args))
 
 def options(cmd, keys, others):
     opts = {}
@@ -996,7 +1004,7 @@
     return wireprototypes.streamres(
         gen=chunks, prefer_uncompressed=not prefercompressed)
 
- at wireprotocommand('heads', permission='pull')
+ at wireprotocommand('heads', permission='pull', transportpolicy=POLICY_V1_ONLY)
 def heads(repo, proto):
     h = repo.heads()
     return wireprototypes.bytesresponse(encodelist(h) + '\n')
@@ -1197,3 +1205,13 @@
                 bundler.newpart('error:pushraced',
                                 [('message', stringutil.forcebytestr(exc))])
             return wireprototypes.streamreslegacy(gen=bundler.getchunks())
+
+# Wire protocol version 2 commands only past this point.
+
+ at wireprotocommand('heads', args='publiconly', permission='pull',
+                  transportpolicy=POLICY_V2_ONLY)
+def headsv2(repo, proto, publiconly=False):
+    if publiconly:
+        repo = repo.filtered('immutable')
+
+    return wireprototypes.cborresponse(repo.heads())
diff --git a/mercurial/help/internals/wireprotocol.txt b/mercurial/help/internals/wireprotocol.txt
--- a/mercurial/help/internals/wireprotocol.txt
+++ b/mercurial/help/internals/wireprotocol.txt
@@ -1649,3 +1649,40 @@
 
 The server may also respond with a generic error type, which contains a string
 indicating the failure.
+
+Frame-Based Protocol Commands
+=============================
+
+**Experimental and under active development**
+
+This section documents the wire protocol commands exposed to transports
+using the frame-based protocol. The set of commands exposed through
+these transports is distinct from the set of commands exposed to legacy
+transports.
+
+The frame-based protocol uses CBOR to encode command execution requests.
+All command arguments must be mapped to a specific or set of CBOR data
+types.
+
+The response to many commands is also CBOR. There is no common response
+format: each command defines its own response format.
+
+TODO require node type be specified, as N bytes of binary node value
+could be ambiguous once SHA-1 is replaced.
+
+heads
+-----
+
+Obtain DAG heads in the repository.
+
+The command accepts the following arguments:
+
+publiconly (optional)
+   (boolean) If set, operate on the DAG for public phase changesets only.
+   Non-public (i.e. draft) phase DAG heads will not be returned.
+
+The response is a CBOR array of bytestrings defining changeset nodes
+of DAG heads. The array can be empty if the repository is empty or no
+changesets satisfied the request.
+
+TODO consider exposing phase of heads in response



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


More information about the Mercurial-devel mailing list