Add LooseVersion class with risky comparison, deprecate parse_loose_version (#9646)

* Replace parse_loose_version with LooseVersion

* Fix LooseVersion docstring

* Strengthen LooseVersion comparison

* Update changelog

AUTHORS.md

@@ -154,6 +154,7 @@ Authors
 * [LeCoyote](https://github.com/LeCoyote)
 * [Lee Watson](https://github.com/TheReverend403)
 * [Leo Famulari](https://github.com/lfam)
+* [Leon G](https://github.com/LeonGr)
 * [lf](https://github.com/lf-)
 * [Liam Marshall](https://github.com/liamim)
 * [Lior Sabag](https://github.com/liorsbg)

certbot/CHANGELOG.md

@@ -6,7 +6,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Added
 
-*
+* Add `certbot.util.LooseVersion` class. See [GH #9489](https://github.com/certbot/certbot/issues/9489).
 
 ### Changed
 

certbot/certbot/_internal/tests/util_test.py

@@ -625,6 +625,53 @@ class AtexitRegisterTest(unittest.TestCase):
             atexit_func(*args[1:], **kwargs)
 
 
+class LooseVersionTest(unittest.TestCase):
+    """Test for certbot.util.LooseVersion.
+
+    These tests are based on the original tests for
+    distutils.version.LooseVersion at
+    https://github.com/python/cpython/blob/v3.10.0/Lib/distutils/tests/test_version.py#L58-L81.
+
+    """
+
+    @classmethod
+    def _call(cls, *args, **kwargs):
+        from certbot.util import LooseVersion
+        return LooseVersion(*args, **kwargs)
+
+    def test_less_than(self):
+        comparisons = (('1.5.1', '1.5.2b2'),
+            ('3.4j', '1996.07.12'),
+            ('2g6', '11g'),
+            ('0.960923', '2.2beta29'),
+            ('1.13++', '5.5.kw'),
+            ('2.0', '2.0.1'),
+            ('a', 'b'))
+        for v1, v2 in comparisons:
+            assert self._call(v1).try_risky_comparison(self._call(v2)) == -1
+
+    def test_equal(self):
+        comparisons = (('8.02', '8.02'),
+            ('1a', '1a'),
+            ('2', '2.0.0'),
+            ('2.0', '2.0.0'))
+        for v1, v2 in comparisons:
+            assert self._call(v1).try_risky_comparison(self._call(v2)) == 0
+
+    def test_greater_than(self):
+        comparisons = (('161', '3.10a'),
+            ('3.2.pl0', '3.1.1.6'))
+        for v1, v2 in comparisons:
+            assert self._call(v1).try_risky_comparison(self._call(v2)) == 1
+
+    def test_incomparible(self):
+        comparisons = (('bookworm/sid', '9'),
+            ('1a', '1.0'))
+        for v1, v2 in comparisons:
+            with pytest.raises(ValueError):
+                assert self._call(v1).try_risky_comparison(self._call(v2))
+
+
 class ParseLooseVersionTest(unittest.TestCase):
     """Test for certbot.util.parse_loose_version.
 

certbot/certbot/util.py

@@ -2,6 +2,7 @@
 import argparse
 import atexit
 import errno
+import itertools
 import logging
 import platform
 import re
@@ -48,6 +49,79 @@ class CSR(NamedTuple):
     form: str
 
 
+class LooseVersion:
+    """A version with loose rules, i.e. any given string is a valid version number.
+
+    but regular comparison is not supported. Instead, the `try_risky_comparison` method is
+    provided, which may return an error if two LooseVersions are 'incomparible'.
+    For example when integer and string version components are present in the same position.
+
+    Differences with old distutils.version.LooseVersion:
+    (https://github.com/python/cpython/blob/v3.10.0/Lib/distutils/version.py#L269)
+    Most version comparisons should give the same result. However, if a version has multiple
+    trailing zeroes, not all of them are used in the comparison. This ensure that, for example,
+    "2.0" and "2.0.0" are equal.
+    """
+
+    def __init__(self, version_string: str) -> None:
+        """Parses a version string into its components.
+
+        :param str version_string: version string
+        """
+        components: List[Union[int, str]]
+        components = [x for x in _VERSION_COMPONENT_RE.split(version_string)
+                              if x and x != '.']
+        for i, obj in enumerate(components):
+            try:
+                components[i] = int(obj)
+            except ValueError:
+                pass
+
+        self.version_components = components
+
+    def try_risky_comparison(self, other: 'LooseVersion') -> int:
+        """Compares the LooseVersion to another value.
+
+        If the other value is another LooseVersion, the version components are compared. Otherwise,
+        an exception is raised.
+
+        Comparison is performed element-wise. If the version components being compared are of
+        different types, the two versions are considered incomparible. Otherwise, if either of the
+        components is not equal to the other, less or greater is returned based on the comparison's
+        result. In case the two versions are of different lengths, some elements in the longer
+        version have not yet been compared. If these are all equal to zero, the two versions are
+        equal. Otherwise, the longer version is greater.
+
+        If the two versions are incomparible, an exception is raised. Otherwise, the returned
+        integer indicates the result of the comparison. If self == other, 0 is returned.
+        If self > other, 1 is returned. If self < other -1 is returned.
+
+        Examples:
+        Equality:
+        - LooseVersion('1.0').try_risky_comparison(LooseVersion('1.0')) -> 0
+        - LooseVersion('2.0.0a').try_risky_comparison(LooseVersion('2.0.0a')) -> 0
+        Inequality:
+        - LooseVersion('2.0.0').try_risky_comparison(LooseVersion('1.0')) -> 1
+        - LooseVersion('1.0.1').try_risky_comparison(LooseVersion('2.0a')) -> -1
+        Incomparability:
+        - LooseVersion('1a').try_risky_comparison(LooseVersion('1.0')) -> ValueError
+        """
+        try:
+            for self_vc, other_vc in itertools.zip_longest(self.version_components,
+                                                           other.version_components,
+                                                           fillvalue=0):
+                # ensure mypy ignores types here and catch any TypeErrors
+                if self_vc < other_vc:  # type: ignore
+                    return -1
+                elif self_vc > other_vc:  # type: ignore
+                    return 1
+            return 0
+        except TypeError:
+            raise ValueError("Cannot meaningfully compare LooseVersion {} with LooseVersion {} "
+                             "due to comparison of version components with different types."
+                             .format(self.version_components, other.version_components))
+
+
 # ANSI SGR escape codes
 # Formats text as bold or with increased intensity
 ANSI_SGR_BOLD = '\033[1m'
@@ -640,28 +714,17 @@ def atexit_register(func: Callable, *args: Any, **kwargs: Any) -> None:
 
 def parse_loose_version(version_string: str) -> List[Union[int, str]]:
     """Parses a version string into its components.
-
     This code and the returned tuple is based on the now deprecated
     distutils.version.LooseVersion class from the Python standard library.
     Two LooseVersion classes and two lists as returned by this function should
     compare in the same way. See
     https://github.com/python/cpython/blob/v3.10.0/Lib/distutils/version.py#L205-L347.
-
     :param str version_string: version string
-
     :returns: list of parsed version string components
     :rtype: list
-
     """
-    components: List[Union[int, str]]
-    components = [x for x in _VERSION_COMPONENT_RE.split(version_string)
-                          if x and x != '.']
-    for i, obj in enumerate(components):
-        try:
-            components[i] = int(obj)
-        except ValueError:
-            pass
-    return components
+    loose_version = LooseVersion(version_string)
+    return loose_version.version_components
 
 
 def _atexit_call(func: Callable, *args: Any, **kwargs: Any) -> None: