Skip to content

Commit

Permalink
sanity_checks: don't compare revision against a version component
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bgilbert committed Jan 23, 2024
1 parent 67b267c commit 1a3fce9
Showing 1 changed file with 18 additions and 8 deletions.
26 changes: 18 additions & 8 deletions tools/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand All @@ -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}>'
Expand All @@ -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:
Expand All @@ -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
Expand Down

0 comments on commit 1a3fce9

Please sign in to comment.