fix parse_version, add tests, fixes #2556

src/borg/testsuite/version.py

@@ -0,0 +1,38 @@
+import pytest
+
+from ..version import parse_version
+
+
+@pytest.mark.parametrize("version_str, version_tuple", [
+    # setuptools < 8.0 uses "-"
+    ('1.0.0a1.dev204-g8866961.d20170606', (1, 0, 0, -4, 1)),
+    ('1.0.0a1.dev204-g8866961', (1, 0, 0, -4, 1)),
+    ('1.0.0-d20170606', (1, 0, 0, -1)),
+    # setuptools >= 8.0 uses "+"
+    ('1.0.0a1.dev204+g8866961.d20170606', (1, 0, 0, -4, 1)),
+    ('1.0.0a1.dev204+g8866961', (1, 0, 0, -4, 1)),
+    ('1.0.0+d20170606', (1, 0, 0, -1)),
+    # pre-release versions:
+    ('1.0.0a1', (1, 0, 0, -4, 1)),
+    ('1.0.0a2', (1, 0, 0, -4, 2)),
+    ('1.0.0b3', (1, 0, 0, -3, 3)),
+    ('1.0.0rc4', (1, 0, 0, -2, 4)),
+    # release versions:
+    ('0.0.0', (0, 0, 0, -1)),
+    ('0.0.11', (0, 0, 11, -1)),
+    ('0.11.0', (0, 11, 0, -1)),
+    ('11.0.0', (11, 0, 0, -1)),
+])
+def test_parse_version(version_str, version_tuple):
+    assert parse_version(version_str) == version_tuple
+
+
+def test_parse_version_invalid():
+    with pytest.raises(ValueError):
+        assert parse_version('')  # we require x.y.z versions
+    with pytest.raises(ValueError):
+        assert parse_version('1')  # we require x.y.z versions
+    with pytest.raises(ValueError):
+        assert parse_version('1.2')  # we require x.y.z versions
+    with pytest.raises(ValueError):
+        assert parse_version('crap')

src/borg/version.py

@@ -3,33 +3,33 @@ import re
 
 def parse_version(version):
     """
-    simplistic parser for setuptools_scm versions
+    Simplistic parser for setuptools_scm versions.
 
-    supports final versions and alpha ('a'), beta ('b') and rc versions. It just discards commits since last tag
-    and git revision hash.
+    Supports final versions and alpha ('a'), beta ('b') and release candidate ('rc') versions.
+    It does not try to parse anything else than that, even if there is more in the version string.
 
     Output is a version tuple containing integers. It ends with one or two elements that ensure that relational
-    operators yield correct relations for alpha, beta and rc versions too. For final versions the last element
-    is a -1, for prerelease versions the last two elements are a smaller negative number and the number of e.g.
-    the beta.
-
-    Note, this sorts version 1.0 before 1.0.0.
+    operators yield correct relations for alpha, beta and rc versions, too.
+    For final versions the last element is a -1.
+    For prerelease versions the last two elements are a smaller negative number and the number of e.g. the beta.
 
     This version format is part of the remote protocol, don‘t change in breaking ways.
     """
-
-    parts = version.split('+')[0].split('.')
-    if parts[-1].startswith('dev'):
-        del parts[-1]
-    version = [int(segment) for segment in parts[:-1]]
-
-    prerelease = re.fullmatch('([0-9]+)(a|b|rc)([0-9]+)', parts[-1])
-    if prerelease:
-        version_type = {'a': -4, 'b': -3, 'rc': -2}[prerelease.group(2)]
-        version += [int(prerelease.group(1)), version_type, int(prerelease.group(3))]
+    version_re = r"""
+        (?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>\d+)   # version, e.g. 1.2.33
+        (?P<prerelease>(?P<ptype>a|b|rc)(?P<pnum>\d+))?  # optional prerelease, e.g. a1 or b2 or rc33
+    """
+    m = re.match(version_re, version, re.VERBOSE)
+    if m is None:
+        raise ValueError('Invalid version string %s' % version)
+    gd = m.groupdict()
+    version = [int(gd['major']), int(gd['minor']), int(gd['patch'])]
+    if m.lastgroup == 'prerelease':
+        p_type = {'a': -4, 'b': -3, 'rc': -2}[gd['ptype']]
+        p_num = int(gd['pnum'])
+        version += [p_type, p_num]
     else:
-        version += [int(parts[-1]), -1]
-
+        version += [-1]
     return tuple(version)