From 1a3fce9131bb397e2eda906feec37b8cae7a1e29 Mon Sep 17 00:00:00 2001
From: Benjamin Gilbert <bgilbert@backtick.net>
Date: Mon, 22 Jan 2024 23:21:34 -0600
Subject: [PATCH] sanity_checks: don't compare revision against a version
 component

1.3-5 was sorting greater than 1.3.1-1 because the normalized [1, 3, 5]
is greater than [1, 3, 1, 1].

Ensure a revision is always compared with another revision and not with a
version component.  To simplify the implementation, split off the revision
into a separate field of the Version object.  This also allows us to
verify that the revision exists and is numeric.
---
 tools/utils.py | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/tools/utils.py b/tools/utils.py
index a4d9a80d7..d47439f55 100644
--- a/tools/utils.py
+++ b/tools/utils.py
@@ -23,8 +23,15 @@ class Version:
     def __init__(self, s: str) -> None:
         self._s = s
 
-        # split into numeric, alphabetic and non-alphanumeric sequences
-        sequences1 = re.finditer(r'(\d+|[a-zA-Z]+|[^a-zA-Z\d]+)', s)
+        # split off revision and store it separately
+        match = re.match('(.+)-([0-9]+)$', s)
+        if not match:
+            raise ValueError(f'Missing/invalid revision: {s}')
+        v = match[1]
+        self._r = int(match[2])
+
+        # split version into numeric, alphabetic and non-alphanumeric sequences
+        sequences1 = re.finditer(r'(\d+|[a-zA-Z]+|[^a-zA-Z\d]+)', v)
 
         # non-alphanumeric separators are discarded
         sequences2 = [m for m in sequences1 if not re.match(r'[^a-zA-Z\d]+', m.group(1))]
@@ -35,7 +42,7 @@ def __init__(self, s: str) -> None:
         self._v = sequences3
 
     def __str__(self) -> str:
-        return '{} (V={})'.format(self._s, str(self._v))
+        return f'{self._s} (V={str(self._v)}, R={self._r})'
 
     def __repr__(self) -> str:
         return f'<Version: {self._s}>'
@@ -62,12 +69,12 @@ def __ge__(self, other: object) -> bool:
 
     def __eq__(self, other: object) -> bool:
         if isinstance(other, Version):
-            return self._v == other._v
+            return self._v == other._v and self._r == other._r
         return NotImplemented
 
     def __ne__(self, other: object) -> bool:
         if isinstance(other, Version):
-            return self._v != other._v
+            return self._v != other._v or self._r != other._r
         return NotImplemented
 
     def __cmp(self, other: 'Version', comparator: T.Callable[[T.Any, T.Any], bool]) -> bool:
@@ -82,9 +89,12 @@ def __cmp(self, other: 'Version', comparator: T.Callable[[T.Any, T.Any], bool])
             if ours != theirs:
                 return comparator(ours, theirs)
 
-        # if equal length, all components have matched, so equal
-        # otherwise, the version with a suffix remaining is greater
-        return comparator(len(self._v), len(other._v))
+        # if one version has a suffix remaining, that version is greater
+        if len(self._v) != len(other._v):
+            return comparator(len(self._v), len(other._v))
+
+        # versions are equal, so compare revisions
+        return comparator(self._r, other._r)
 
 def is_ci() -> bool:
     return 'CI' in os.environ