This revision was automatically updated to reflect the committed changes.
Closed by commit rHG69e46c1834ac: wireproto: define and expose types of wire 
command arguments (authored by indygreg, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D3202?vs=7906&id=7981

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

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

CHANGE DETAILS

diff --git a/tests/test-wireproto-command-capabilities.t 
b/tests/test-wireproto-command-capabilities.t
--- a/tests/test-wireproto-command-capabilities.t
+++ b/tests/test-wireproto-command-capabilities.t
@@ -30,11 +30,11 @@
   s>     \r\n
   s>     *\r\n (glob)
   s>     *\x00\x01\x00\x02\x01F (glob)
-  s>     
\xa2Hcommands\xaaEheads\xa2Dargs\x81JpubliconlyKpermissions\x81DpullEknown\xa2Dargs\x81EnodesKpermissions\x81DpullFlookup\xa2Dargs\x81CkeyKpermissions\x81DpullGpushkey\xa2Dargs\x84CkeyInamespaceCnewColdKpermissions\x81DpushHlistkeys\xa2Dargs\x81InamespaceKpermissions\x81DpullHunbundle\xa2Dargs\x81EheadsKpermissions\x81DpushIbranchmap\xa2Dargs\x80Kpermissions\x81DpullIgetbundle\xa2Dargs\x81A*Kpermissions\x81DpullLcapabilities\xa2Dargs\x80Kpermissions\x81DpullLclonebundles\xa2Dargs\x80Kpermissions\x81DpullKcompression\x82\xa1DnameDzstd\xa1DnameDzlib
+  s>     
\xa2Hcommands\xaaEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyFlegacyKpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyFlegacyCnewFlegacyColdFlegacyInamespaceFlegacyKpermissions\x81DpushHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullHunbundle\xa2Dargs\xa1EheadsFlegacyKpermissions\x81DpushIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullIgetbundle\xa2Dargs\xa1A*FlegacyKpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullLclonebundles\xa2Dargs\xa0Kpermissions\x81DpullKcompression\x82\xa1DnameDzstd\xa1DnameDzlib
   s>     \r\n
   received frame(size=*; request=1; stream=2; streamflags=stream-begin; 
type=bytes-response; flags=eos|cbor) (glob)
   s>     0\r\n
   s>     \r\n
-  response: [{b'commands': {b'branchmap': {b'args': [], b'permissions': 
[b'pull']}, b'capabilities': {b'args': [], b'permissions': [b'pull']}, 
b'clonebundles': {b'args': [], b'permissions': [b'pull']}, b'getbundle': 
{b'args': [b'*'], b'permissions': [b'pull']}, b'heads': {b'args': 
[b'publiconly'], b'permissions': [b'pull']}, b'known': {b'args': [b'nodes'], 
b'permissions': [b'pull']}, b'listkeys': {b'args': [b'namespace'], 
b'permissions': [b'pull']}, b'lookup': {b'args': [b'key'], b'permissions': 
[b'pull']}, b'pushkey': {b'args': [b'key', b'namespace', b'new', b'old'], 
b'permissions': [b'push']}, b'unbundle': {b'args': [b'heads'], b'permissions': 
[b'push']}}, b'compression': [{b'name': b'zstd'}, {b'name': b'zlib'}]}]
+  response: [{b'commands': {b'branchmap': {b'args': {}, b'permissions': 
[b'pull']}, b'capabilities': {b'args': {}, b'permissions': [b'pull']}, 
b'clonebundles': {b'args': {}, b'permissions': [b'pull']}, b'getbundle': 
{b'args': {b'*': b'legacy'}, b'permissions': [b'pull']}, b'heads': {b'args': 
{b'publiconly': False}, b'permissions': [b'pull']}, b'known': {b'args': 
{b'nodes': [b'deadbeef']}, b'permissions': [b'pull']}, b'listkeys': {b'args': 
{b'namespace': b'ns'}, b'permissions': [b'pull']}, b'lookup': {b'args': 
{b'key': b'legacy'}, b'permissions': [b'pull']}, b'pushkey': {b'args': {b'key': 
b'legacy', b'namespace': b'legacy', b'new': b'legacy', b'old': b'legacy'}, 
b'permissions': [b'push']}, b'unbundle': {b'args': {b'heads': b'legacy'}, 
b'permissions': [b'push']}}, b'compression': [{b'name': b'zstd'}, {b'name': 
b'zlib'}]}]
 
   $ cat error.log
diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py
--- a/mercurial/wireprotoserver.py
+++ b/mercurial/wireprotoserver.py
@@ -606,10 +606,11 @@
 
     def getargs(self, args):
         data = {}
-        for k in args.split():
+        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]
 
         return data
diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
--- a/mercurial/wireproto.py
+++ b/mercurial/wireproto.py
@@ -713,8 +713,11 @@
 
     ``name`` is the name of the wire protocol command being provided.
 
-    ``args`` is a space-delimited list of named arguments that the command
-    accepts. ``*`` is a special value that says to accept all arguments.
+    ``args`` defines the named arguments accepted by the command. It is
+    ideally a dict mapping argument names to their types. For backwards
+    compatibility, it can be a space-delimited list of argument names. For
+    version 1 transports, ``*`` denotes a special value that says to accept
+    all named arguments.
 
     ``transportpolicy`` is a POLICY_* constant denoting which transports
     this wire protocol command should be exposed to. By default, commands
@@ -752,6 +755,17 @@
                                      'got %s; expected "push" or "pull"' %
                                      permission)
 
+    if 1 in transportversions and not isinstance(args, bytes):
+        raise error.ProgrammingError('arguments for version 1 commands must '
+                                     'be declared as bytes')
+
+    if isinstance(args, bytes):
+        dictargs = {arg: b'legacy' for arg in args.split()}
+    elif isinstance(args, dict):
+        dictargs = args
+    else:
+        raise ValueError('args must be bytes or a dict')
+
     def register(func):
         if 1 in transportversions:
             if name in commands:
@@ -764,7 +778,8 @@
             if name in commandsv2:
                 raise error.ProgrammingError('%s command already registered '
                                              'for version 2' % name)
-            commandsv2[name] = commandentry(func, args=args,
+
+            commandsv2[name] = commandentry(func, args=dictargs,
                                             transports=transports,
                                             permission=permission)
 
@@ -1304,7 +1319,7 @@
 
     for command, entry in commandsv2.items():
         caps['commands'][command] = {
-            'args': sorted(entry.args.split()) if entry.args else [],
+            'args': entry.args,
             'permissions': [entry.permission],
         }
 
@@ -1325,22 +1340,34 @@
 
     return wireprototypes.cborresponse(caps)
 
-@wireprotocommand('heads', args='publiconly', permission='pull',
+@wireprotocommand('heads',
+                  args={
+                      'publiconly': False,
+                  },
+                  permission='pull',
                   transportpolicy=POLICY_V2_ONLY)
 def headsv2(repo, proto, publiconly=False):
     if publiconly:
         repo = repo.filtered('immutable')
 
     return wireprototypes.cborresponse(repo.heads())
 
-@wireprotocommand('known', 'nodes', permission='pull',
+@wireprotocommand('known',
+                  args={
+                      'nodes': [b'deadbeef'],
+                  },
+                  permission='pull',
                   transportpolicy=POLICY_V2_ONLY)
 def knownv2(repo, proto, nodes=None):
     nodes = nodes or []
     result = b''.join(b'1' if n else b'0' for n in repo.known(nodes))
     return wireprototypes.cborresponse(result)
 
-@wireprotocommand('listkeys', 'namespace', permission='pull',
+@wireprotocommand('listkeys',
+                  args={
+                      'namespace': b'ns',
+                  },
+                  permission='pull',
                   transportpolicy=POLICY_V2_ONLY)
 def listkeysv2(repo, proto, namespace=None):
     keys = repo.listkeys(encoding.tolocal(namespace))
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
@@ -1712,7 +1712,12 @@
    are:
 
       args
-         An array of argument names accepted by this command.
+         A map of argument names and their expected types.
+
+         Types are defined as a representative value for the expected type.
+         e.g. an argument expecting a boolean type will have its value
+         set to true. An integer type will have its value set to 42. The
+         actual values are arbitrary and may not have meaning.
       permissions
          An array of permissions required to execute this command.
 



To: indygreg, #hg-reviewers, durin42
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to