Skip to content

util/version: fix SemanticVersion comparison precedence#1797

Merged
TheJJ merged 2 commits into
SFTtech:masterfrom
nickk02:fix/semver-comparison-precedence
May 28, 2026
Merged

util/version: fix SemanticVersion comparison precedence#1797
TheJJ merged 2 commits into
SFTtech:masterfrom
nickk02:fix/semver-comparison-precedence

Conversation

@nickk02
Copy link
Copy Markdown
Contributor

@nickk02 nickk02 commented May 28, 2026

Merge Checklist

  • I have read the contribution guide
  • I have added my info to copying.md (only first time contributors)
  • I have run make checkmerge and fixed all mentioned problems (no local build setup; relying on CI)

Description

SemanticVersion.__lt__/__le__/__gt__/__ge__ in openage/util/version.py short-circuit on the first field that differs and return True as soon as any of major/minor/patch satisfies the relation, instead of comparing fields lexicographically. The bug:

>>> from openage.util.version import SemanticVersion as V
>>> V("1.0.0") > V("0.5.0")
False          # wrong, should be True
>>> V("2.0.0") > V("1.99.99")
False          # wrong, should be True
>>> V("1.0.0") < V("0.5.0")
True           # wrong, should be False

The only current caller is openage/convert/service/init/changelog.py:43, which uses > to decide whether a converted modpack is outdated, so this silently flipped some "up-to-date" vs "outdated" decisions.

Fix

Switch the operators to a tuple precedence key and use functools.total_ordering, so the operators agree with each other by construction. Prerelease versions now sort below the same MAJOR.MINOR.PATCH per semver 11.3. Build metadata is ignored for precedence, matching semver 10. Full identifier-by-identifier prerelease comparison (semver 11.4, i.e. alpha < beta < rc) is noted as not yet implemented; the current usage in changelog.py does not need it.

Tests

Added inline doctests covering the regression cases, and registered openage.util.version in openage/testing/testlist.py doctest_modules() so they run as part of the doctest suite.

nickk02 added 2 commits May 28, 2026 03:29
The old __lt__, __le__, __gt__, __ge__ short-circuited on the first
field that differed, returning True as soon as any of major/minor/patch
satisfied the relation. That meant 1.0.0 < 0.5.0 returned True (minor
0 < 5) and 2.0.0 > 1.99.99 returned False, breaking the modpack
"outdated" check in convert/service/init/changelog.py.

Switch to a tuple precedence key and use functools.total_ordering, so
the operators agree with each other by construction. Prerelease versions
now sort below the same MAJOR.MINOR.PATCH per semver 11.3; full
identifier-by-identifier comparison (semver 11.4) is noted as not yet
implemented. Build metadata is ignored for precedence, which matches
semver 10.

Add doctests covering the regressions and register openage.util.version
in testing/testlist.py so they actually run.
Copy link
Copy Markdown
Member

@TheJJ TheJJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants