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

[Gregory Szorc]

# HG changeset patch
# User Gregory Szorc <gregory.szorc at gmail.com>
# Date 1444691621 25200
#      Mon Oct 12 16:13:41 2015 -0700
# Node ID 7ae92885f2b6c0757efb134aa5e80b72e5b1ed70
# Parent  883b69ea57cf6ac049f91059a5af7e1e0a60eb6b
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 a dedicated error type to represent unsupported bundle
specifications. We could bikeshed on the name here. "Unknown" is also
appropriate since the specification /could/ be valid (but only to a new
client). However, "unsupported" isn't false, so I think it's fine.

The reason we need a dedicated error type (rather than error.Abort) is
because clone bundles will want to catch this 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,7 @@ 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 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,82 @@ 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 ``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.Abort(_('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 ..