[PATCH] util: refactor compression engine capabilities declaration
Gregory Szorc
gregory.szorc at gmail.com
Wed Jan 18 08:48:04 UTC 2017
# HG changeset patch
# User Gregory Szorc <gregory.szorc at gmail.com>
# Date 1484729224 28800
# Wed Jan 18 00:47:04 2017 -0800
# Node ID 016baf3f6299e23b98c205b6506bb6203dba8fd6
# Parent 923336cf8b8afdb41746ecef8a39d773bd5538bf
util: refactor compression engine capabilities declaration
I was asked to refactor the various methods for declaring compression
engine capabilities to be less abusive of methods. I did so by
consolidating everything into a single method returning a dict.
Having written this, I'm not convinced it is objectively better. I'll
let a reviewer decide whether to accept, reject flat out, or push
in a different direction, such as using attributes to declare
capabilities.
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1962,9 +1962,9 @@ def debuginstall(ui, **opts):
wirecompengines = util.compengines.supportedwireengines(util.SERVERROLE)
fm.write('compenginesserver', _('checking available compression engines '
'for wire protocol (%s)\n'),
fm.formatlist([e.name() for e in wirecompengines
- if e.wireprotosupport()],
+ if e.capabilities().get('wireprotoname')],
name='compengine', fmt='%s', sep=', '))
# templates
p = templater.templatepaths()
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -151,9 +151,9 @@ def parsebundlespec(repo, spec, strict=T
', '.join(sorted(missingreqs)))
if not externalnames:
engine = util.compengines.forbundlename(compression)
- compression = engine.bundletype()[1]
+ compression = engine.capabilities()['bundleid']
version = _bundlespeccgversions[version]
return compression, version, params
def readbundle(ui, fh, fname, vfs=None):
@@ -191,9 +191,10 @@ def getbundlespec(ui, fh):
restored.
"""
def speccompression(alg):
try:
- return util.compengines.forbundletype(alg).bundletype()[0]
+ engine = util.compengines.forbundletype(alg)
+ return engine.capabilities()['bundlename']
except KeyError:
return None
b = readbundle(ui, fh, None)
diff --git a/mercurial/hgweb/protocol.py b/mercurial/hgweb/protocol.py
--- a/mercurial/hgweb/protocol.py
+++ b/mercurial/hgweb/protocol.py
@@ -119,9 +119,9 @@ class webproto(wireproto.abstractserverp
# Now find an agreed upon compression format.
for engine in wireproto.supportedcompengines(self.ui, self,
util.SERVERROLE):
- if engine.wireprotosupport().name in compformats:
+ if engine.capabilities()['wireprotoname'] in compformats:
opts = {}
level = self.ui.configint('server',
'%slevel' % engine.name())
if level is not None:
@@ -146,9 +146,9 @@ def call(repo, req, cmd):
def genversion2(gen, compress, engine, engineopts):
# application/mercurial-0.2 always sends a payload header
# identifying the compression engine.
- name = engine.wireprotosupport().name
+ name = engine.capabilities()['wireprotoname']
assert 0 < len(name) < 256
yield struct.pack('B', len(name))
yield name
diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py
--- a/mercurial/httppeer.py
+++ b/mercurial/httppeer.py
@@ -193,9 +193,9 @@ class httppeer(wireproto.wirepeer):
if '0.2tx' in mediatypes and self.capable('compression'):
# We /could/ compare supported compression formats and prune
# non-mutually supported or error if nothing is mutually supported.
# For now, send the full list to the server and have it error.
- comps = [e.wireprotosupport().name for e in
+ comps = [e.capabilities()['wireprotoname'] for e in
util.compengines.supportedwireengines(util.CLIENTROLE)]
protoparams.append('comp=%s' % ','.join(comps))
if protoparams:
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -286,9 +286,9 @@ class localrepository(object):
# Add compression engines.
for name in util.compengines:
engine = util.compengines[name]
- if engine.revlogheader():
+ if engine.capabilities().get('revlogheader'):
self.supported.add('exp-compression-%s' % name)
if not self.vfs.isdir():
if create:
diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -2983,12 +2983,8 @@ class ctxmanager(object):
SERVERROLE = 'server'
CLIENTROLE = 'client'
-compewireprotosupport = collections.namedtuple(u'compenginewireprotosupport',
- (u'name', u'serverpriority',
- u'clientpriority'))
-
class compressormanager(object):
"""Holds registrations of various compression engines.
This class essentially abstracts the differences between compression
@@ -3031,41 +3027,42 @@ class compressormanager(object):
if name in self._engines:
raise error.Abort(_('compression engine %s already registered') %
name)
- bundleinfo = engine.bundletype()
- if bundleinfo:
- bundlename, bundletype = bundleinfo
-
- if bundlename in self._bundlenames:
+ caps = engine.capabilities()
+
+ bundlename = caps.get('bundlename')
+ bundletype = caps.get('bundleid')
+ if bundletype:
+ if bundlename and bundlename in self._bundlenames:
raise error.Abort(_('bundle name %s already registered') %
bundlename)
if bundletype in self._bundletypes:
raise error.Abort(_('bundle type %s already registered by %s') %
(bundletype, self._bundletypes[bundletype]))
- # No external facing name declared.
if bundlename:
self._bundlenames[bundlename] = name
self._bundletypes[bundletype] = name
- wiresupport = engine.wireprotosupport()
- if wiresupport:
- wiretype = wiresupport.name
+ wiretype = caps.get('wireprotoname')
+ if wiretype:
if wiretype in self._wiretypes:
raise error.Abort(_('wire protocol compression %s already '
'registered by %s') %
(wiretype, self._wiretypes[wiretype]))
self._wiretypes[wiretype] = name
- revlogheader = engine.revlogheader()
- if revlogheader and revlogheader in self._revlogheaders:
- raise error.Abort(_('revlog header %s already registered by %s') %
- (revlogheader, self._revlogheaders[revlogheader]))
-
+ revlogheader = caps.get('revlogheader')
if revlogheader:
+ if revlogheader in self._revlogheaders:
+ raise error.Abort(_('revlog header %s already registered by '
+ '%s') % (
+ revlogheader,
+ self._revlogheaders[revlogheader]))
+
self._revlogheaders[revlogheader] = name
self._engines[name] = engine
@@ -3112,9 +3109,10 @@ class compressormanager(object):
loaded.
"""
assert role in (SERVERROLE, CLIENTROLE)
- attr = 'serverpriority' if role == SERVERROLE else 'clientpriority'
+ key = 'serverpriority' if role == SERVERROLE else 'clientpriority'
+ key = 'wireproto%s' % key
engines = [self._engines[e] for e in self._wiretypes.values()]
if onlyavailable:
engines = [e for e in engines if e.available()]
@@ -3122,10 +3120,10 @@ class compressormanager(object):
def getkey(e):
# Sort first by priority, highest first. In case of tie, sort
# alphabetically. This is arbitrary, but ensures output is
# stable.
- w = e.wireprotosupport()
- return -1 * getattr(w, attr), w.name
+ caps = e.capabilities()
+ return -1 * caps[key], caps['wireprotoname']
return list(sorted(engines, key=getkey))
def forwiretype(self, wiretype):
@@ -3166,57 +3164,49 @@ class compressionengine(object):
on C extensions that may not be present).
"""
return True
- def bundletype(self):
- """Describes bundle identifiers for this engine.
-
- If this compression engine isn't supported for bundles, returns None.
-
- If this engine can be used for bundles, returns a 2-tuple of strings of
- the user-facing "bundle spec" compression name and an internal
- identifier used to denote the compression format within bundles. To
- exclude the name from external usage, set the first element to ``None``.
-
- If bundle compression is supported, the class must also implement
- ``compressstream`` and `decompressorreader``.
+ def capabilities(self):
+ """Describe the capabilities and properties of this engine.
+
+ Returns a dict describing the engine in more detail. This dict is
+ consumed at engine registration time to make the engines manager aware
+ of all engines and their abilities.
+
+ The returned dict may have the following keys:
+
+ bundlename
+ String used to identify this engine in the user-facing "bundle
+ specification" string (as used by `hg bundle`). Omit to not register
+ this engine externally.
+
+ bundleid
+ String used to identify this compression format in bundles. Omit
+ to not expose this engine to bundles.
+
+ wireprotoname
+ String that identifies this compression engine in the wire protocol.
+
+ wireprotoserverpriority
+ Integer priority to advertise this engine at on the server.
+ Highest value is most preferred. The priority values are somewhat
+ arbitrary and are only used for default ordering.
+
+ wireprotoclientpriority
+ Integer priority to advertise this engine at on the client.
+
+ revlogheader
+ Byte sequence used to identify chunks in revlogs as compressed with
+ this engine. If this is not defined, the engine does not support
+ revlog compression.
+
+ If ``bundlename`` or ``bundleid`` are set, the class must implement
+ ``compressstream`` and ``decompressorreader``.
+
+ If ``wireprotoname`` is set, the class must implement ``compressstream``
+ and ``decompressorreader``.
"""
- return None
-
- def wireprotosupport(self):
- """Declare support for this compression format on the wire protocol.
-
- If this compression engine isn't supported for compressing wire
- protocol payloads, returns None.
-
- Otherwise, returns ``compenginewireprotosupport`` with the following
- fields:
-
- * String format identifier
- * Integer priority for the server
- * Integer priority for the client
-
- The integer priorities are used to order the advertisement of format
- support by server and client. The highest integer is advertised
- first. Integers with non-positive values aren't advertised.
-
- The priority values are somewhat arbitrary and only used for default
- ordering. The relative order can be changed via config options.
-
- If wire protocol compression is supported, the class must also implement
- ``compressstream`` and ``decompressorreader``.
- """
- return None
-
- def revlogheader(self):
- """Header added to revlog chunks that identifies this engine.
-
- If this engine can be used to compress revlogs, this method should
- return the bytes used to identify chunks compressed with this engine.
- Else, the method should return ``None`` to indicate it does not
- participate in revlog compression.
- """
- return None
+ return {}
def compressstream(self, it, opts=None):
"""Compress an iterator of chunks.
@@ -3261,16 +3251,17 @@ class compressionengine(object):
class _zlibengine(compressionengine):
def name(self):
return 'zlib'
- def bundletype(self):
- return 'gzip', 'GZ'
-
- def wireprotosupport(self):
- return compewireprotosupport('zlib', 20, 20)
-
- def revlogheader(self):
- return 'x'
+ def capabilities(self):
+ return {
+ 'bundlename': 'gzip',
+ 'bundleid': 'GZ',
+ 'wireprotoname': 'zlib',
+ 'wireprotoserverpriority': 20,
+ 'wireprotoclientpriority': 20,
+ 'revlogheader': 'x',
+ }
def compressstream(self, it, opts=None):
opts = opts or {}
@@ -3342,15 +3333,18 @@ compengines.register(_zlibengine())
class _bz2engine(compressionengine):
def name(self):
return 'bz2'
- def bundletype(self):
- return 'bzip2', 'BZ'
-
- # We declare a protocol name but don't advertise by default because
- # it is slow.
- def wireprotosupport(self):
- return compewireprotosupport('bzip2', 0, 0)
+ def capabilities(self):
+ return {
+ 'bundlename': 'bzip2',
+ 'bundleid': 'BZ',
+ # We declare a protocol name but don't advertise by default
+ # because it is slow.
+ 'wireprotoname': 'bzip2',
+ 'wireprotoserverpriority': 0,
+ 'wireprotoclientpriority': 0,
+ }
def compressstream(self, it, opts=None):
opts = opts or {}
z = bz2.BZ2Compressor(opts.get('level', 9))
@@ -3374,10 +3368,12 @@ compengines.register(_bz2engine())
class _truncatedbz2engine(compressionengine):
def name(self):
return 'bz2truncated'
- def bundletype(self):
- return None, '_truncatedBZ'
+ def capabilities(self):
+ return {
+ 'bundleid': '_truncatedBZ',
+ }
# We don't implement compressstream because it is hackily handled elsewhere.
def decompressorreader(self, fh):
@@ -3395,19 +3391,21 @@ compengines.register(_truncatedbz2engine
class _noopengine(compressionengine):
def name(self):
return 'none'
- def bundletype(self):
- return 'none', 'UN'
-
- # Clients always support uncompressed payloads. Servers don't because
- # unless you are on a fast network, uncompressed payloads can easily
- # saturate your network pipe.
- def wireprotosupport(self):
- return compewireprotosupport('none', 0, 10)
-
- # We don't implement revlogheader because it is handled specially
- # in the revlog class.
+ def capabilities(self):
+ return {
+ 'bundlename': 'none',
+ 'bundleid': 'UN',
+ 'wireprotoname': 'none',
+ # Clients always support uncompressed payloads. Servers don't
+ # because unless you are on a fast network, uncompressed
+ # payloads can easily saturate your network pipe.
+ 'wireprotoserverpriority': 0,
+ 'wireprotoclientpriority': 10,
+ # We don't declare the revlog header because it is handled
+ # specially in the revlog class.
+ }
def compressstream(self, it, opts=None):
return it
@@ -3441,16 +3439,17 @@ class _zstdengine(compressionengine):
def available(self):
return bool(self._module)
- def bundletype(self):
- return 'zstd', 'ZS'
-
- def wireprotosupport(self):
- return compewireprotosupport('zstd', 50, 50)
-
- def revlogheader(self):
- return '\x28'
+ def capabilities(self):
+ return {
+ 'bundlename': 'zstd',
+ 'bundleid': 'ZS',
+ 'wireprotoname': 'zstd',
+ 'wireprotoserverpriority': 50,
+ 'wireprotoclientpriority': 50,
+ 'revlogheader': '\x28',
+ }
def compressstream(self, it, opts=None):
opts = opts or {}
# zstd level 3 is almost always significantly faster than zlib
diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
--- a/mercurial/wireproto.py
+++ b/mercurial/wireproto.py
@@ -629,11 +629,12 @@ def supportedcompengines(ui, proto, role
# No explicit config. Filter out the ones that aren't supposed to be
# advertised and return default ordering.
if not configengines:
- attr = 'serverpriority' if role == util.SERVERROLE else 'clientpriority'
+ key = 'serverpriority' if role == util.SERVERROLE else 'clientpriority'
+ key = 'wireproto%s' % key
return [e for e in compengines
- if getattr(e.wireprotosupport(), attr) > 0]
+ if e.capabilities().get(key, 0) > 0]
# If compression engines are listed in the config, assume there is a good
# reason for it (like server operators wanting to achieve specific
# performance characteristics). So fail fast if the config references
@@ -777,9 +778,9 @@ def _capabilities(repo, proto):
caps.append('httpmediatype=0.1rx,0.1tx,0.2tx')
compengines = supportedcompengines(repo.ui, proto, util.SERVERROLE)
if compengines:
- comptypes = ','.join(urlreq.quote(e.wireprotosupport().name)
+ comptypes = ','.join(urlreq.quote(e.capabilities()['wireprotoname'])
for e in compengines)
caps.append('compression=%s' % comptypes)
return caps
More information about the Mercurial-devel
mailing list