D3202: wireproto: define and expose types of wire command arguments
indygreg (Gregory Szorc)
phabricator at mercurial-scm.org
Wed Apr 11 12:35:04 EDT 2018
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)
- at wireprotocommand('heads', args='publiconly', permission='pull',
+ at 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())
- at wireprotocommand('known', 'nodes', permission='pull',
+ at 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)
- at wireprotocommand('listkeys', 'namespace', permission='pull',
+ at 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
More information about the Mercurial-devel
mailing list