changeset 39810:0b61d21f05cc

wireprotov2: declare command arguments richly Previously, we declared command arguments with an example of their value. After this commit, we declare command arguments as a dict of metadata. This allows us to define the value type, whether the argument is required, and provide default values. This in turn allows us to have nice things, such as less boilerplate code in individual commands for validating input and assigning default values. It should also make command behavior more consistent as a result. Test output changed slightly because I realized that the "fields" argument wasn't being consistently defined as a set. Oops! Other test output changed because of slight differences in code performing type validation. Differential Revision: https://phab.mercurial-scm.org/D4615
author Gregory Szorc <gregory.szorc@gmail.com>
date Thu, 30 Aug 2018 17:43:47 -0700
parents ddca38941b2b
children ae20f52437e9
files mercurial/wireprotov2server.py tests/test-http-protocol.t tests/test-wireproto-command-capabilities.t tests/test-wireproto-command-filedata.t tests/test-wireproto-command-manifestdata.t
diffstat 5 files changed, 260 insertions(+), 123 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial/wireprotov2server.py	Sat Sep 15 17:26:23 2018 +0900
+++ b/mercurial/wireprotov2server.py	Thu Aug 30 17:43:47 2018 -0700
@@ -311,6 +311,10 @@
         action, meta = reactor.oncommandresponsereadyobjects(
             outstream, command['requestid'], objs)
 
+    except error.WireprotoCommandError as e:
+        action, meta = reactor.oncommanderror(
+            outstream, command['requestid'], e.message, e.messageargs)
+
     except Exception as e:
         action, meta = reactor.onservererror(
             outstream, command['requestid'],
@@ -348,13 +352,39 @@
         return HTTP_WIREPROTO_V2
 
     def getargs(self, args):
+        # First look for args that were passed but aren't registered on this
+        # command.
+        extra = set(self._args) - set(args)
+        if extra:
+            raise error.WireprotoCommandError(
+                'unsupported argument to command: %s' %
+                ', '.join(sorted(extra)))
+
+        # And look for required arguments that are missing.
+        missing = {a for a in args if args[a]['required']} - set(self._args)
+
+        if missing:
+            raise error.WireprotoCommandError(
+                'missing required arguments: %s' % ', '.join(sorted(missing)))
+
+        # Now derive the arguments to pass to the command, taking into
+        # account the arguments specified by the client.
         data = {}
-        for k, typ in args.items():
-            if k == '*':
-                raise NotImplementedError('do not support * args')
-            elif k in self._args:
-                # TODO consider validating value types.
-                data[k] = self._args[k]
+        for k, meta in sorted(args.items()):
+            # This argument wasn't passed by the client.
+            if k not in self._args:
+                data[k] = meta['default']()
+                continue
+
+            v = self._args[k]
+
+            # Sets may be expressed as lists. Silently normalize.
+            if meta['type'] == 'set' and isinstance(v, list):
+                v = set(v)
+
+            # TODO consider more/stronger type validation.
+
+            data[k] = v
 
         return data
 
@@ -404,8 +434,10 @@
     # TODO expose available changesetdata fields.
 
     for command, entry in COMMANDS.items():
+        args = {arg: meta['example'] for arg, meta in entry.args.items()}
+
         caps['commands'][command] = {
-            'args': entry.args,
+            'args': args,
             'permissions': [entry.permission],
         }
 
@@ -500,7 +532,23 @@
 
     ``name`` is the name of the wire protocol command being provided.
 
-    ``args`` is a dict of argument names to example values.
+    ``args`` is a dict defining arguments accepted by the command. Keys are
+    the argument name. Values are dicts with the following keys:
+
+       ``type``
+          The argument data type. Must be one of the following string
+          literals: ``bytes``, ``int``, ``list``, ``dict``, ``set``,
+          or ``bool``.
+
+       ``default``
+          A callable returning the default value for this argument. If not
+          specified, ``None`` will be the default value.
+
+       ``required``
+          Bool indicating whether the argument is required.
+
+       ``example``
+          An example value for this argument.
 
     ``permission`` defines the permission type needed to run this command.
     Can be ``push`` or ``pull``. These roughly map to read-write and read-only,
@@ -529,6 +577,36 @@
         raise error.ProgrammingError('arguments for version 2 commands '
                                      'must be declared as dicts')
 
+    for arg, meta in args.items():
+        if arg == '*':
+            raise error.ProgrammingError('* argument name not allowed on '
+                                         'version 2 commands')
+
+        if not isinstance(meta, dict):
+            raise error.ProgrammingError('arguments for version 2 commands '
+                                         'must declare metadata as a dict')
+
+        if 'type' not in meta:
+            raise error.ProgrammingError('%s argument for command %s does not '
+                                         'declare type field' % (arg, name))
+
+        if meta['type'] not in ('bytes', 'int', 'list', 'dict', 'set', 'bool'):
+            raise error.ProgrammingError('%s argument for command %s has '
+                                         'illegal type: %s' % (arg, name,
+                                                               meta['type']))
+
+        if 'example' not in meta:
+            raise error.ProgrammingError('%s argument for command %s does not '
+                                         'declare example field' % (arg, name))
+
+        if 'default' in meta and meta.get('required'):
+            raise error.ProgrammingError('%s argument for command %s is marked '
+                                         'as required but has a default value' %
+                                         (arg, name))
+
+        meta.setdefault('default', lambda: None)
+        meta.setdefault('required', False)
+
     def register(func):
         if name in COMMANDS:
             raise error.ProgrammingError('%s command already registered '
@@ -550,16 +628,25 @@
 def capabilitiesv2(repo, proto):
     yield _capabilitiesv2(repo, proto)
 
-@wireprotocommand('changesetdata',
-                  args={
-                      'noderange': [[b'0123456...'], [b'abcdef...']],
-                      'nodes': [b'0123456...'],
-                      'fields': {b'parents', b'revision'},
-                  },
-                  permission='pull')
-def changesetdata(repo, proto, noderange=None, nodes=None, fields=None):
-    fields = fields or set()
-
+@wireprotocommand(
+    'changesetdata',
+    args={
+        'noderange': {
+            'type': 'list',
+            'example': [[b'0123456...'], [b'abcdef...']],
+        },
+        'nodes': {
+            'type': 'list',
+            'example': [b'0123456...'],
+        },
+        'fields': {
+            'type': 'set',
+            'default': set,
+            'example': {b'parents', b'revision'},
+        },
+    },
+    permission='pull')
+def changesetdata(repo, proto, noderange, nodes, fields):
     # TODO look for unknown fields and abort when they can't be serviced.
 
     if noderange is None and nodes is None:
@@ -691,24 +778,32 @@
 
     return fl
 
-@wireprotocommand('filedata',
-                  args={
-                      'haveparents': True,
-                      'nodes': [b'0123456...'],
-                      'fields': [b'parents', b'revision'],
-                      'path': b'foo.txt',
-                  },
-                  permission='pull')
-def filedata(repo, proto, haveparents=False, nodes=None, fields=None,
-             path=None):
-    fields = fields or set()
-
-    if nodes is None:
-        raise error.WireprotoCommandError('nodes argument must be defined')
-
-    if path is None:
-        raise error.WireprotoCommandError('path argument must be defined')
-
+@wireprotocommand(
+    'filedata',
+    args={
+        'haveparents': {
+            'type': 'bool',
+            'default': lambda: False,
+            'example': True,
+        },
+        'nodes': {
+            'type': 'list',
+            'required': True,
+            'example': [b'0123456...'],
+        },
+        'fields': {
+            'type': 'set',
+            'default': set,
+            'example': {b'parents', b'revision'},
+        },
+        'path': {
+            'type': 'bytes',
+            'required': True,
+            'example': b'foo.txt',
+        }
+    },
+    permission='pull')
+def filedata(repo, proto, haveparents, nodes, fields, path):
     try:
         # Extensions may wish to access the protocol handler.
         store = getfilestore(repo, proto, path)
@@ -776,44 +871,63 @@
         except GeneratorExit:
             pass
 
-@wireprotocommand('heads',
-                  args={
-                      'publiconly': False,
-                  },
-                  permission='pull')
-def headsv2(repo, proto, publiconly=False):
+@wireprotocommand(
+    'heads',
+    args={
+        'publiconly': {
+            'type': 'bool',
+            'default': lambda: False,
+            'example': False,
+        },
+    },
+    permission='pull')
+def headsv2(repo, proto, publiconly):
     if publiconly:
         repo = repo.filtered('immutable')
 
     yield repo.heads()
 
-@wireprotocommand('known',
-                  args={
-                      'nodes': [b'deadbeef'],
-                  },
-                  permission='pull')
-def knownv2(repo, proto, nodes=None):
-    nodes = nodes or []
+@wireprotocommand(
+    'known',
+    args={
+        'nodes': {
+            'type': 'list',
+            'default': list,
+            'example': [b'deadbeef'],
+        },
+    },
+    permission='pull')
+def knownv2(repo, proto, nodes):
     result = b''.join(b'1' if n else b'0' for n in repo.known(nodes))
     yield result
 
-@wireprotocommand('listkeys',
-                  args={
-                      'namespace': b'ns',
-                  },
-                  permission='pull')
-def listkeysv2(repo, proto, namespace=None):
+@wireprotocommand(
+    'listkeys',
+    args={
+        'namespace': {
+            'type': 'bytes',
+            'required': True,
+            'example': b'ns',
+        },
+    },
+    permission='pull')
+def listkeysv2(repo, proto, namespace):
     keys = repo.listkeys(encoding.tolocal(namespace))
     keys = {encoding.fromlocal(k): encoding.fromlocal(v)
             for k, v in keys.iteritems()}
 
     yield keys
 
-@wireprotocommand('lookup',
-                  args={
-                      'key': b'foo',
-                  },
-                  permission='pull')
+@wireprotocommand(
+    'lookup',
+    args={
+        'key': {
+            'type': 'bytes',
+            'required': True,
+            'example': b'foo',
+        },
+    },
+    permission='pull')
 def lookupv2(repo, proto, key):
     key = encoding.tolocal(key)
 
@@ -822,26 +936,32 @@
 
     yield node
 
-@wireprotocommand('manifestdata',
-                  args={
-                      'nodes': [b'0123456...'],
-                      'haveparents': True,
-                      'fields': [b'parents', b'revision'],
-                      'tree': b'',
-                  },
-                  permission='pull')
-def manifestdata(repo, proto, haveparents=False, nodes=None, fields=None,
-                 tree=None):
-    fields = fields or set()
-
-    if nodes is None:
-        raise error.WireprotoCommandError(
-            'nodes argument must be defined')
-
-    if tree is None:
-        raise error.WireprotoCommandError(
-            'tree argument must be defined')
-
+@wireprotocommand(
+    'manifestdata',
+    args={
+        'nodes': {
+            'type': 'list',
+            'required': True,
+            'example': [b'0123456...'],
+        },
+        'haveparents': {
+            'type': 'bool',
+            'default': lambda: False,
+            'example': True,
+        },
+        'fields': {
+            'type': 'set',
+            'default': set,
+            'example': {b'parents', b'revision'},
+        },
+        'tree': {
+            'type': 'bytes',
+            'required': True,
+            'example': b'',
+        },
+    },
+    permission='pull')
+def manifestdata(repo, proto, haveparents, nodes, fields, tree):
     store = repo.manifestlog.getstorage(tree)
 
     # Validate the node is known and abort on unknown revisions.
@@ -905,14 +1025,31 @@
         except GeneratorExit:
             pass
 
-@wireprotocommand('pushkey',
-                  args={
-                      'namespace': b'ns',
-                      'key': b'key',
-                      'old': b'old',
-                      'new': b'new',
-                  },
-                  permission='push')
+@wireprotocommand(
+    'pushkey',
+    args={
+        'namespace': {
+            'type': 'bytes',
+            'required': True,
+            'example': b'ns',
+        },
+        'key': {
+            'type': 'bytes',
+            'required': True,
+            'example': b'key',
+        },
+        'old': {
+            'type': 'bytes',
+            'required': True,
+            'example': b'old',
+        },
+        'new': {
+            'type': 'bytes',
+            'required': True,
+            'example': 'new',
+        },
+    },
+    permission='push')
 def pushkeyv2(repo, proto, namespace, key, old, new):
     # TODO handle ui output redirection
     yield repo.pushkey(encoding.tolocal(namespace),
--- a/tests/test-http-protocol.t	Sat Sep 15 17:26:23 2018 +0900
+++ b/tests/test-http-protocol.t	Thu Aug 30 17:43:47 2018 -0700
@@ -313,7 +313,7 @@
   s>     Content-Type: application/mercurial-cbor\r\n
   s>     Content-Length: *\r\n (glob)
   s>     \r\n
-  s>     \xa3GapibaseDapi/Dapis\xa1Pexp-http-v2-0001\xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree@Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1Nv1capabilitiesY\x01\xd3batch branchmap $USUAL_BUNDLE2_CAPS$ changegroupsubset compression=$BUNDLE2_COMPRESSIONS$ getbundle httpheader=1024 httpmediatype=0.1rx,0.1tx,0.2tx known lookup pushkey streamreqs=generaldelta,revlogv1 unbundle=HG10GZ,HG10BZ,HG10UN unbundlehash
+  s>     \xa3GapibaseDapi/Dapis\xa1Pexp-http-v2-0001\xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree@Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1Nv1capabilitiesY\x01\xd3batch branchmap $USUAL_BUNDLE2_CAPS$ changegroupsubset compression=$BUNDLE2_COMPRESSIONS$ getbundle httpheader=1024 httpmediatype=0.1rx,0.1tx,0.2tx known lookup pushkey streamreqs=generaldelta,revlogv1 unbundle=HG10GZ,HG10BZ,HG10UN unbundlehash
   sending heads command
   s>     POST /api/exp-http-v2-0001/ro/heads HTTP/1.1\r\n
   s>     Accept-Encoding: identity\r\n
--- a/tests/test-wireproto-command-capabilities.t	Sat Sep 15 17:26:23 2018 +0900
+++ b/tests/test-wireproto-command-capabilities.t	Thu Aug 30 17:43:47 2018 -0700
@@ -212,7 +212,7 @@
   s>     Content-Type: application/mercurial-cbor\r\n
   s>     Content-Length: *\r\n (glob)
   s>     \r\n
-  s>     \xa3GapibaseDapi/Dapis\xa1Pexp-http-v2-0001\xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree@Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1Nv1capabilitiesY\x01\xd3batch branchmap $USUAL_BUNDLE2_CAPS$ changegroupsubset compression=$BUNDLE2_COMPRESSIONS$ getbundle httpheader=1024 httpmediatype=0.1rx,0.1tx,0.2tx known lookup pushkey streamreqs=generaldelta,revlogv1 unbundle=HG10GZ,HG10BZ,HG10UN unbundlehash
+  s>     \xa3GapibaseDapi/Dapis\xa1Pexp-http-v2-0001\xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree@Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1Nv1capabilitiesY\x01\xd3batch branchmap $USUAL_BUNDLE2_CAPS$ changegroupsubset compression=$BUNDLE2_COMPRESSIONS$ getbundle httpheader=1024 httpmediatype=0.1rx,0.1tx,0.2tx known lookup pushkey streamreqs=generaldelta,revlogv1 unbundle=HG10GZ,HG10BZ,HG10UN unbundlehash
   cbor> {
     b'apibase': b'api/',
     b'apis': {
@@ -254,10 +254,10 @@
           },
           b'filedata': {
             b'args': {
-              b'fields': [
+              b'fields': set([
                 b'parents',
                 b'revision'
-              ],
+              ]),
               b'haveparents': True,
               b'nodes': [
                 b'0123456...'
@@ -304,10 +304,10 @@
           },
           b'manifestdata': {
             b'args': {
-              b'fields': [
+              b'fields': set([
                 b'parents',
                 b'revision'
-              ],
+              ]),
               b'haveparents': True,
               b'nodes': [
                 b'0123456...'
@@ -369,7 +369,7 @@
   s>     Content-Type: application/mercurial-cbor\r\n
   s>     Content-Length: *\r\n (glob)
   s>     \r\n
-  s>     \xa3GapibaseDapi/Dapis\xa1Pexp-http-v2-0001\xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree@Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1Nv1capabilitiesY\x01\xd3batch branchmap $USUAL_BUNDLE2_CAPS$ changegroupsubset compression=$BUNDLE2_COMPRESSIONS$ getbundle httpheader=1024 httpmediatype=0.1rx,0.1tx,0.2tx known lookup pushkey streamreqs=generaldelta,revlogv1 unbundle=HG10GZ,HG10BZ,HG10UN unbundlehash
+  s>     \xa3GapibaseDapi/Dapis\xa1Pexp-http-v2-0001\xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree@Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1Nv1capabilitiesY\x01\xd3batch branchmap $USUAL_BUNDLE2_CAPS$ changegroupsubset compression=$BUNDLE2_COMPRESSIONS$ getbundle httpheader=1024 httpmediatype=0.1rx,0.1tx,0.2tx known lookup pushkey streamreqs=generaldelta,revlogv1 unbundle=HG10GZ,HG10BZ,HG10UN unbundlehash
   sending capabilities command
   s>     POST /api/exp-http-v2-0001/ro/capabilities HTTP/1.1\r\n
   s>     Accept-Encoding: identity\r\n
@@ -392,11 +392,11 @@
   s>     \xa1FstatusBok
   s>     \r\n
   received frame(size=11; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=continuation)
-  s>     30e\r\n
-  s>     \x06\x03\x00\x01\x00\x02\x001
-  s>     \xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree@Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1
+  s>     314\r\n
+  s>     \x0c\x03\x00\x01\x00\x02\x001
+  s>     \xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree@Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1
   s>     \r\n
-  received frame(size=774; request=1; stream=2; streamflags=; type=command-response; flags=continuation)
+  received frame(size=780; request=1; stream=2; streamflags=; type=command-response; flags=continuation)
   s>     8\r\n
   s>     \x00\x00\x00\x01\x00\x02\x002
   s>     \r\n
@@ -442,10 +442,10 @@
         },
         b'filedata': {
           b'args': {
-            b'fields': [
+            b'fields': set([
               b'parents',
               b'revision'
-            ],
+            ]),
             b'haveparents': True,
             b'nodes': [
               b'0123456...'
@@ -492,10 +492,10 @@
         },
         b'manifestdata': {
           b'args': {
-            b'fields': [
+            b'fields': set([
               b'parents',
               b'revision'
-            ],
+            ]),
             b'haveparents': True,
             b'nodes': [
               b'0123456...'
--- a/tests/test-wireproto-command-filedata.t	Sat Sep 15 17:26:23 2018 +0900
+++ b/tests/test-wireproto-command-filedata.t	Thu Aug 30 17:43:47 2018 -0700
@@ -69,14 +69,14 @@
   s>     Content-Type: application/mercurial-exp-framing-0005\r\n
   s>     Transfer-Encoding: chunked\r\n
   s>     \r\n
-  s>     45\r\n
-  s>     =\x00\x00\x01\x00\x02\x012
-  s>     \xa2Eerror\xa1GmessageX\x1enodes argument must be definedFstatusEerror
+  s>     4e\r\n
+  s>     F\x00\x00\x01\x00\x02\x012
+  s>     \xa2Eerror\xa1GmessageX\'missing required arguments: nodes, pathFstatusEerror
   s>     \r\n
-  received frame(size=61; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
+  received frame(size=70; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
   s>     0\r\n
   s>     \r\n
-  abort: nodes argument must be defined!
+  abort: missing required arguments: nodes, path!
   [255]
 
   $ sendhttpv2peer << EOF
@@ -101,14 +101,14 @@
   s>     Content-Type: application/mercurial-exp-framing-0005\r\n
   s>     Transfer-Encoding: chunked\r\n
   s>     \r\n
-  s>     44\r\n
-  s>     <\x00\x00\x01\x00\x02\x012
-  s>     \xa2Eerror\xa1GmessageX\x1dpath argument must be definedFstatusEerror
+  s>     47\r\n
+  s>     ?\x00\x00\x01\x00\x02\x012
+  s>     \xa2Eerror\xa1GmessageX missing required arguments: pathFstatusEerror
   s>     \r\n
-  received frame(size=60; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
+  received frame(size=63; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
   s>     0\r\n
   s>     \r\n
-  abort: path argument must be defined!
+  abort: missing required arguments: path!
   [255]
 
 Unknown node is an error
--- a/tests/test-wireproto-command-manifestdata.t	Sat Sep 15 17:26:23 2018 +0900
+++ b/tests/test-wireproto-command-manifestdata.t	Thu Aug 30 17:43:47 2018 -0700
@@ -65,14 +65,14 @@
   s>     Content-Type: application/mercurial-exp-framing-0005\r\n
   s>     Transfer-Encoding: chunked\r\n
   s>     \r\n
-  s>     45\r\n
-  s>     =\x00\x00\x01\x00\x02\x012
-  s>     \xa2Eerror\xa1GmessageX\x1enodes argument must be definedFstatusEerror
+  s>     4e\r\n
+  s>     F\x00\x00\x01\x00\x02\x012
+  s>     \xa2Eerror\xa1GmessageX\'missing required arguments: nodes, treeFstatusEerror
   s>     \r\n
-  received frame(size=61; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
+  received frame(size=70; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
   s>     0\r\n
   s>     \r\n
-  abort: nodes argument must be defined!
+  abort: missing required arguments: nodes, tree!
   [255]
 
   $ sendhttpv2peer << EOF
@@ -97,14 +97,14 @@
   s>     Content-Type: application/mercurial-exp-framing-0005\r\n
   s>     Transfer-Encoding: chunked\r\n
   s>     \r\n
-  s>     44\r\n
-  s>     <\x00\x00\x01\x00\x02\x012
-  s>     \xa2Eerror\xa1GmessageX\x1dtree argument must be definedFstatusEerror
+  s>     47\r\n
+  s>     ?\x00\x00\x01\x00\x02\x012
+  s>     \xa2Eerror\xa1GmessageX missing required arguments: treeFstatusEerror
   s>     \r\n
-  received frame(size=60; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
+  received frame(size=63; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
   s>     0\r\n
   s>     \r\n
-  abort: tree argument must be defined!
+  abort: missing required arguments: tree!
   [255]
 
 Unknown node is an error