Add tests for and fix RPM version sorting

pulp_rpm/app/rpm_version.py

@@ -12,7 +12,6 @@
 
 # flake8: noqa
 
-import re
 from typing import NamedTuple
 from typing import Union
 
@@ -22,7 +21,7 @@ class RpmVersion(NamedTuple):
     Represent an RPM version. It is ordered.
     """
 
-    epoch: int
+    epoch: str
     version: str
     release: str
 
@@ -46,38 +45,36 @@ def from_string(cls, s):
         return cls(e, v, r)
 
     def __lt__(self, other):
-        return compare_rpm_versions(self, other) < 0
+        return _compare_rpm_versions(self, other) < 0
 
     def __gt__(self, other):
-        return compare_rpm_versions(self, other) > 0
+        return _compare_rpm_versions(self, other) > 0
 
     def __eq__(self, other):
-        return compare_rpm_versions(self, other) == 0
+        return _compare_rpm_versions(self, other) == 0
 
     def __le__(self, other):
-        return compare_rpm_versions(self, other) <= 0
+        return _compare_rpm_versions(self, other) <= 0
 
     def __ge__(self, other):
-        return compare_rpm_versions(self, other) >= 0
+        return _compare_rpm_versions(self, other) >= 0
 
 
 def from_evr(s):
     """
     Return an (E, V, R) tuple given a string by splitting
     [e:]version-release into the three possible subcomponents.
-    Default epoch to 0, version and release to empty string if not specified.
+    Default epoch, version and release to empty string if not specified.
 
-    >>> assert from_evr("1:11.13.2.0-1") == (1, "11.13.2.0", "1")
-    >>> assert from_evr("11.13.2.0-1") == (0, "11.13.2.0", "1")
+    >>> assert from_evr("1:11.13.2.0-1") == ("1", "11.13.2.0", "1")
+    >>> assert from_evr("11.13.2.0-1") == ("", "11.13.2.0", "1")
     """
     if ":" in s:
         e, _, vr = s.partition(":")
     else:
-        e = "0"
+        e = ""
         vr = s
 
-    e = int(e)
-
     if "-" in vr:
         v, _, r = vr.partition("-")
     else:
@@ -86,7 +83,7 @@ def from_evr(s):
     return e, v, r
 
 
-def compare_rpm_versions(a: Union[RpmVersion, str], b: Union[RpmVersion, str]) -> int:
+def _compare_rpm_versions(a: Union[RpmVersion, str], b: Union[RpmVersion, str]) -> int:
     """
     Compare two RPM versions ``a`` and ``b`` and return:
     -  1 if the version of a is newer than b
@@ -113,131 +110,178 @@ def compare_rpm_versions(a: Union[RpmVersion, str], b: Union[RpmVersion, str]) -
     if not isinstance(a, RpmVersion) and not isinstance(b, RpmVersion):
         raise TypeError(f"{a!r} and {b!r} must be RpmVersion or strings")
 
+    a_epoch = a.epoch or "0"
+    b_epoch = b.epoch or "0"
+
     # First compare the epoch, if set.  If the epoch's are not the same, then
     # the higher one wins no matter what the rest of the EVR is.
-    if a.epoch != b.epoch:
-        if a.epoch > b.epoch:
-            return 1  # a > b
-        else:
-            return -1  # a < b
+    if a_epoch != b_epoch:
+        epoch_compare = _compare_version_strings(a_epoch, b_epoch)
+        if epoch_compare != 0:
+            return epoch_compare  # a > b
 
     # Epoch is the same, if version + release are the same we have a match
     if (a.version == b.version) and (a.release == b.release):
         return 0  # a == b
 
     # Compare version first, if version is equal then compare release
-    compare_res = vercmp(a.version, b.version)
-    if compare_res != 0:  # a > b || a < b
-        return compare_res
-    else:
-        return vercmp(a.release, b.release)
-
-
-class Vercmp:
-    R_NONALNUMTILDE_CARET = re.compile(rb"^([^a-zA-Z0-9~\^]*)(.*)$")
-    R_NUM = re.compile(rb"^([\d]+)(.*)$")
-    R_ALPHA = re.compile(rb"^([a-zA-Z]+)(.*)$")
+    version_compare = _compare_version_strings(a.version, b.version)
+    if version_compare != 0:  # a > b || a < b
+        return version_compare
+
+    return _compare_version_strings(a.release, b.release)
+
+
+# internal use: each individual component of the EVR is compared using this function
+def _compare_version_strings(first, second):
+    first = first.encode("utf-8")
+    second = second.encode("utf-8")
+
+    if first == second:
+        return 0
+
+    def not_alphanumeric_tilde_or_caret(c):
+        return not (
+            (ord(b"a") <= c <= ord(b"z"))
+            or (ord(b"A") <= c <= ord(b"Z"))
+            or (ord(b"0") <= c <= ord(b"9"))
+            or c == ord(b"~")
+            or c == ord(b"^")
+        )
+
+    def trim_start_matches(data, predicate):
+        """Trim leading bytes that match the predicate"""
+        start = 0
+        while start < len(data) and predicate(data[start]):
+            start += 1
+        return data[start:]
+
+    def strip_prefix(data, prefix):
+        """Strip prefix from data, return (stripped_data, was_stripped)"""
+        if data.startswith(prefix):
+            return data[len(prefix) :], True
+        return data, False
+
+    def matching_contiguous(data, predicate):
+        """Match contiguous characters that satisfy predicate"""
+        if not data:
+            return None, data
+
+        if not predicate(data[0]):
+            return None, data
+
+        end = 0
+        while end < len(data) and predicate(data[end]):
+            end += 1
+
+        return data[:end], data[end:]
+
+    version1_part = first
+    version2_part = second
+
+    while True:
+        # Strip any leading non-alphanumeric, non-tilde, non-caret characters
+        version1_part = trim_start_matches(version1_part, not_alphanumeric_tilde_or_caret)
+        version2_part = trim_start_matches(version2_part, not_alphanumeric_tilde_or_caret)
+
+        # Tilde separator parses as "older" or lesser version
+        version1_stripped, version1_had_tilde = strip_prefix(version1_part, b"~")
+        version2_stripped, version2_had_tilde = strip_prefix(version2_part, b"~")
+
+        if version1_had_tilde and not version2_had_tilde:
+            return -1
+        elif not version1_had_tilde and version2_had_tilde:
+            return 1
+        elif version1_had_tilde and version2_had_tilde:
+            version1_part = version1_stripped
+            version2_part = version2_stripped
+            continue
 
-    @classmethod
-    def compare(cls, first, second):
-        # Rpm versions can only be ascii, anything else is just ignored
-        first = first.encode("ascii", "ignore")
-        second = second.encode("ascii", "ignore")
+        # Caret means the version is less... Unless the other version
+        # has ended, then do the exact opposite.
+        version1_stripped, version1_had_caret = strip_prefix(version1_part, b"^")
+        version2_stripped, version2_had_caret = strip_prefix(version2_part, b"^")
+
+        if version1_had_caret and not version2_had_caret:
+            if not version2_part:  # second has ended
+                return 1  # first > second
+            else:  # second continues
+                return -1  # first < second
+        elif not version1_had_caret and version2_had_caret:
+            if not version1_part:  # first has ended
+                return -1  # first < second
+            else:  # first continues
+                return 1  # first > second
+        elif version1_had_caret and version2_had_caret:
+            version1_part = version1_stripped
+            version2_part = version2_stripped
+            continue
 
-        if first == second:
+        # Check if we've run out of characters
+        if not version1_part and not version2_part:
             return 0
+        elif not version1_part:
+            return -1
+        elif not version2_part:
+            return 1
 
-        while first or second:
-            m1 = cls.R_NONALNUMTILDE_CARET.match(first)
-            m2 = cls.R_NONALNUMTILDE_CARET.match(second)
-            m1_head, first = m1.group(1), m1.group(2)
-            m2_head, second = m2.group(1), m2.group(2)
-            if m1_head or m2_head:
-                # Ignore junk at the beginning
-                continue
-
-            # handle the tilde separator, it sorts before everything else
-            if first.startswith(b"~"):
-                if not second.startswith(b"~"):
-                    return -1
-                first, second = first[1:], second[1:]
-                continue
-            if second.startswith(b"~"):
-                return 1
+        # Parse numeric or alphabetic segments
+        def is_digit(c):
+            return ord(b"0") <= c <= ord(b"9")
 
-            # Now look at the caret, which is like the tilde but pointier.
-            if first.startswith(b"^"):
-                # first has a caret but second has ended
-                if not second:
-                    return 1  # first > second
-
-                # first has a caret but second continues on
-                elif not second.startswith(b"^"):
-                    return -1  # first < second
-
-                # strip the ^ and start again
-                first, second = first[1:], second[1:]
-                continue
-
-            # Caret means the version is less... Unless the other version
-            # has ended, then do the exact opposite.
-            if second.startswith(b"^"):
-                return -1 if not first else 1
-
-            # We've run out of characters to compare.
-            # Note: we have to do this after we compare the ~ and ^ madness
-            # because ~'s and ^'s take precedance.
-            # If we ran to the end of either, we are finished with the loop
-            if not first or not second:
-                break
-
-            # grab first completely alpha or completely numeric segment
-            m1 = cls.R_NUM.match(first)
-            if m1:
-                m2 = cls.R_NUM.match(second)
-                if not m2:
-                    # numeric segments are always newer than alpha segments
-                    return 1
-                isnum = True
-            else:
-                m1 = cls.R_ALPHA.match(first)
-                m2 = cls.R_ALPHA.match(second)
-                if not m2:
-                    return -1
-                isnum = False
+        def is_alpha(c):
+            return (ord(b"a") <= c <= ord(b"z")) or (ord(b"A") <= c <= ord(b"Z"))
+
+        if version1_part and is_digit(version1_part[0]):
+            # First starts with digit - extract numeric segment
+            segment1, version1_part = matching_contiguous(version1_part, is_digit)
 
-            m1_head, first = m1.group(1), m1.group(2)
-            m2_head, second = m2.group(1), m2.group(2)
+            if version2_part and is_digit(version2_part[0]):
+                # Both numeric
+                segment2, version2_part = matching_contiguous(version2_part, is_digit)
 
-            if isnum:
-                # throw away any leading zeros - it's a number, right?
-                m1_head = m1_head.lstrip(b"0")
-                m2_head = m2_head.lstrip(b"0")
+                # Strip leading zeros
+                segment1 = segment1.lstrip(b"0")
+                segment2 = segment2.lstrip(b"0")
 
-                # whichever number has more digits wins
-                m1hlen = len(m1_head)
-                m2hlen = len(m2_head)
-                if m1hlen < m2hlen:
+                # Compare by length first (more digits = larger number)
+                if len(segment1) < len(segment2):
                     return -1
-                if m1hlen > m2hlen:
+                elif len(segment1) > len(segment2):
                     return 1
-
-            # Same number of chars
-            if m1_head < m2_head:
-                return -1
-            if m1_head > m2_head:
+                else:
+                    # Same length, compare lexicographically
+                    if segment1 < segment2:
+                        return -1
+                    elif segment1 > segment2:
+                        return 1
+                    # Equal, continue to next segment
+            else:
+                # First is numeric, second is not - numeric wins
                 return 1
-            # Both segments equal
-            continue
-
-        m1len = len(first)
-        m2len = len(second)
-        if m1len == m2len == 0:
-            return 0
-        if m1len != 0:
-            return 1
-        return -1
+        else:
+            # First starts with alpha or we're at end
+            if version1_part:
+                segment1, version1_part = matching_contiguous(version1_part, is_alpha)
+            else:
+                segment1 = b""
 
+            if version2_part and is_digit(version2_part[0]):
+                # First is alpha, second is numeric - numeric wins
+                return -1
+            else:
+                # Both alpha or at least one is empty
+                if version2_part:
+                    segment2, version2_part = matching_contiguous(version2_part, is_alpha)
+                else:
+                    segment2 = b""
+
+                # Compare alphabetically
+                if segment1 < segment2:
+                    return -1
+                elif segment1 > segment2:
+                    return 1
+                # Equal, continue to next segment
 
-def vercmp(first, second):
-    return Vercmp.compare(first, second)
+    # Should not reach here due to the checks above, but just in case
+    raise RuntimeError("somehow escaped the loop during version comparison")