[PATCH 2 of 2 V2] exchange: refactor bundle specification parsing

Gregory Szorc gregory.szorc at gmail.com
Tue Oct 13 12:15:01 CDT 2015


# HG changeset patch
# User Gregory Szorc <gregory.szorc at gmail.com>
# Date 1444756406 25200
#      Tue Oct 13 10:13:26 2015 -0700
# Node ID 0f386c4467f46f2c863f4dabc3bd6c4ca558a9f7
# Parent  d93dfcd4e505405756891a3418ad26891ffbcfa4
exchange: refactor bundle specification parsing

The old code was tailored to `hg bundle` usage and not appropriate for
use as a general API, which clone bundles will require. The code has
been rewritten to make it more generally suitable.

We introduce dedicated error types to represent invalid and unsupported
bundle specifications. The reason we need dedicated error types (rather
than error.Abort) is because clone bundles will want to catch these
exception as part of filtering entries. We don't want to swallow
error.Abort on principle.

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1241,10 +1241,15 @@ def bundle(ui, repo, fname, dest=None, *
     if 'rev' in opts:
         revs = scmutil.revrange(repo, opts['rev'])
 
     bundletype = opts.get('type', 'bzip2').lower()
-    cgversion, bcompression = exchange.parsebundlespecification(repo,
-                                                                bundletype)
+    try:
+        bcompression, cgversion = exchange.parsebundlespecification(
+                repo, bundletype, strict=False)
+    except error.UnsupportedBundleSpecification as e:
+        raise error.Abort(e.message,
+                          hint=_('see "hg help bundle" for supported '
+                                 'values for --type'))
 
     if opts.get('all'):
         base = ['null']
     else:
diff --git a/mercurial/error.py b/mercurial/error.py
--- a/mercurial/error.py
+++ b/mercurial/error.py
@@ -200,4 +200,13 @@ class CensoredBaseError(RevlogError):
     A delta based on a censored revision must be formed as single patch
     operation which replaces the entire base with new content. This ensures
     the delta may be applied by clones which have not censored the base.
     """
+
+class InvalidBundleSpecification(Exception):
+    """error raised when a bundle specification is invalid.
+
+    This is used for syntax errors as opposed to support errors.
+    """
+
+class UnsupportedBundleSpecification(Exception):
+    """error raised when a bundle specification is not supported."""
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -13,60 +13,86 @@ import discovery, phases, obsolete, book
 import lock as lockmod
 import streamclone
 import tags
 
-_bundlecompspecs = {'none': None,
-                    'bzip2': 'BZ',
-                    'gzip': 'GZ',
-                   }
+# Maps bundle compression human names to internal representation.
+_bundlespeccompressions = {'none': None,
+                           'bzip2': 'BZ',
+                           'gzip': 'GZ',
+                          }
 
-_bundleversionspecs = {'v1': '01',
-                       'v2': '02',
-                       'bundle2': '02', #legacy
-                      }
+# Maps bundle version human names to changegroup versions.
+_bundlespeccgversions = {'v1': '01',
+                         'v2': '02',
+                         'bundle2': '02', #legacy
+                        }
 
-def parsebundlespecification(repo, spec):
-    """return the internal bundle type to use from a user input
+def parsebundlespecification(repo, spec, strict=True):
+    """Parse a bundle string specification into parts.
 
-    This is parsing user specified bundle type as accepted in:
+    Bundle specifications denote a well-defined bundle/exchange format.
+    The content of a given specification should not change over time in
+    order to ensure that bundles produced by a newer version of Mercurial are
+    readable from an older version.
 
-        'hg bundle --type TYPE'.
+    The string currently has the form:
 
-    It accept format in the form [compression][-version]|[version]
+       <compression>-<type>
 
-    Consensus about extensions of the format for various bundle2 feature
-    is to prefix any feature with "+". eg "+treemanifest" or "gzip+phases"
+    Where <compression> is one of the supported compression formats
+    and <type> is (currently) a version string.
+
+    If ``strict`` is True (the default) <compression> is required. Otherwise,
+    it is optional.
+
+    Returns a 2-tuple of (compression, version). Compression will be ``None``
+    if not in strict mode and a compression isn't defined.
+
+    An ``InvalidBundleSpecification`` is raised when the specification is
+    not syntactically well formed.
+
+    An ``UnsupportedBundleSpecification`` is raised when the compression or
+    bundle type/version is not recognized.
+
+    Note: this function will likely eventually return a more complex data
+    structure, including bundle2 part information.
     """
-    comp, version = None, None
+    if strict and '-' not in spec:
+        raise error.InvalidBundleSpecification(
+                _('invalid bundle specification; '
+                  'must be prefixed with compression: %s') % spec)
 
     if '-' in spec:
-        comp, version = spec.split('-', 1)
-    elif spec in _bundlecompspecs:
-        comp = spec
-    elif spec in _bundleversionspecs:
-        version = spec
+        compression, version = spec.split('-', 1)
+
+        if compression not in _bundlespeccompressions:
+            raise error.UnsupportedBundleSpecification(
+                    _('%s compression is not supported') % compression)
+
+        if version not in _bundlespeccgversions:
+            raise error.UnsupportedBundleSpecification(
+                    _('%s is not a recognized bundle version') % version)
     else:
-        raise error.Abort(_('unknown bundle type specified with --type'))
+        # Value could be just the compression or just the version, in which
+        # case some defaults are assumed (but only when not in strict mode).
+        assert not strict
 
-    if comp is None:
-        comp = 'BZ'
-    else:
-        try:
-            comp = _bundlecompspecs[comp]
-        except KeyError:
-            raise error.Abort(_('unknown bundle type specified with --type'))
+        if spec in _bundlespeccompressions:
+            compression = spec
+            version = 'v1'
+            if 'generaldelta' in repo.requirements:
+                version = 'v2'
+        elif spec in _bundlespeccgversions:
+            compression = 'bzip2'
+            version = spec
+        else:
+            raise error.UnsupportedBundleSpecification(
+                    _('%s is not a recognized bundle specification') % spec)
 
-    if version is None:
-        version = '01'
-        if 'generaldelta' in repo.requirements:
-            version = '02'
-    else:
-        try:
-            version = _bundleversionspecs[version]
-        except KeyError:
-            raise error.Abort(_('unknown bundle type specified with --type'))
+    compression = _bundlespeccompressions[compression]
+    version = _bundlespeccgversions[version]
+    return compression, version
 
-    return version, comp
 def readbundle(ui, fh, fname, vfs=None):
     header = changegroup.readexactly(fh, 4)
 
     alg = None
diff --git a/tests/test-bundle-type.t b/tests/test-bundle-type.t
--- a/tests/test-bundle-type.t
+++ b/tests/test-bundle-type.t
@@ -101,7 +101,8 @@ test garbage file
 test invalid bundle type
 
   $ cd t1
   $ hg bundle -a -t garbage ../bgarbage
-  abort: unknown bundle type specified with --type
+  abort: garbage is not a recognized bundle specification
+  (see "hg help bundle" for supported values for --type)
   [255]
   $ cd ..


More information about the Mercurial-devel mailing list