From 4709d46936b379605aca110c7dd336d1cff45db6 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 11 Mar 2021 14:51:47 +0100 Subject: Add custom VersionNumber class Qt's API is kind of painful, and we need some custom functionality anyways. Wrap QVersionNumber with our own class instead of piling up workarounds. --- qutebrowser/config/configfiles.py | 13 +++-- qutebrowser/misc/crashdialog.py | 4 +- qutebrowser/utils/qtutils.py | 12 ++--- qutebrowser/utils/utils.py | 86 ++++++++++++++++++++++-------- qutebrowser/utils/version.py | 16 +++--- tests/end2end/test_invocations.py | 2 +- tests/unit/utils/test_utils.py | 109 +++++++++++++++++++++++++++++++++++--- tests/unit/utils/test_version.py | 2 +- 8 files changed, 188 insertions(+), 56 deletions(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 9031c9b96..04aa4ec49 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -128,22 +128,21 @@ class StateConfig(configparser.ConfigParser): # https://github.com/python/typeshed/issues/2093 return # type: ignore[unreachable] - old_version = utils.parse_version(old_qutebrowser_version) - new_version = utils.parse_version(qutebrowser.__version__) - - if old_version.isNull(): + try: + old_version = utils.VersionNumber.parse(old_qutebrowser_version) + except ValueError: log.init.warning(f"Unable to parse old version {old_qutebrowser_version}") return - assert not new_version.isNull(), qutebrowser.__version__ + new_version = utils.VersionNumber.parse(qutebrowser.__version__) if old_version == new_version: self.qutebrowser_version_changed = VersionChange.equal elif new_version < old_version: self.qutebrowser_version_changed = VersionChange.downgrade - elif old_version.segments()[:2] == new_version.segments()[:2]: + elif old_version.segments[:2] == new_version.segments[:2]: self.qutebrowser_version_changed = VersionChange.patch - elif old_version.majorVersion() == new_version.majorVersion(): + elif old_version.major == new_version.major: self.qutebrowser_version_changed = VersionChange.minor else: self.qutebrowser_version_changed = VersionChange.major diff --git a/qutebrowser/misc/crashdialog.py b/qutebrowser/misc/crashdialog.py index ac292dcdb..430553433 100644 --- a/qutebrowser/misc/crashdialog.py +++ b/qutebrowser/misc/crashdialog.py @@ -359,8 +359,8 @@ class _CrashDialog(QDialog): Args: newest: The newest version as a string. """ - new_version = utils.parse_version(newest) - cur_version = utils.parse_version(qutebrowser.__version__) + new_version = utils.VersionNumber.parse(newest) + cur_version = utils.VersionNumber.parse(qutebrowser.__version__) lines = ['The report has been sent successfully. Thanks!'] if new_version > cur_version: lines.append("Note: The newest available version is v{}, " diff --git a/qutebrowser/utils/qtutils.py b/qutebrowser/utils/qtutils.py index f7c5a3ce0..01234a42b 100644 --- a/qutebrowser/utils/qtutils.py +++ b/qutebrowser/utils/qtutils.py @@ -98,15 +98,15 @@ def version_check(version: str, if compiled and exact: raise ValueError("Can't use compiled=True with exact=True!") - parsed = utils.parse_version(version) + parsed = utils.VersionNumber.parse(version) op = operator.eq if exact else operator.ge - result = op(utils.parse_version(qVersion()), parsed) + result = op(utils.VersionNumber.parse(qVersion()), parsed) if compiled and result: # qVersion() ==/>= parsed, now check if QT_VERSION_STR ==/>= parsed. - result = op(utils.parse_version(QT_VERSION_STR), parsed) + result = op(utils.VersionNumber.parse(QT_VERSION_STR), parsed) if compiled and result: # Finally, check PYQT_VERSION_STR as well. - result = op(utils.parse_version(PYQT_VERSION_STR), parsed) + result = op(utils.VersionNumber.parse(PYQT_VERSION_STR), parsed) return result @@ -116,8 +116,8 @@ MAX_WORLD_ID = 256 def is_new_qtwebkit() -> bool: """Check if the given version is a new QtWebKit.""" assert qWebKitVersion is not None - return (utils.parse_version(qWebKitVersion()) > - utils.parse_version('538.1')) + return (utils.VersionNumber.parse(qWebKitVersion()) > + utils.VersionNumber.parse('538.1')) def is_single_process() -> bool: diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py index 698a608ef..af9d5fad7 100644 --- a/qutebrowser/utils/utils.py +++ b/qutebrowser/utils/utils.py @@ -92,26 +92,74 @@ class Comparable(Protocol): ... -if TYPE_CHECKING: - class VersionNumber(Comparable, QVersionNumber): +class VersionNumber: - """WORKAROUND for incorrect PyQt stubs.""" -else: - class VersionNumber(QVersionNumber): + """A representation of a version number.""" - """We can't inherit from Protocol and QVersionNumber at runtime.""" + def __init__(self, *args: int) -> None: + self._ver = QVersionNumber(*args) + if self._ver.isNull(): + raise ValueError("Can't construct a null version") - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - normalized = self.normalized() - if normalized != self: - raise ValueError( - f"Refusing to construct non-normalized version from {args} " - f"(normalized: {tuple(normalized.segments())}).") + normalized = self._ver.normalized() + if normalized != self._ver: + raise ValueError( + f"Refusing to construct non-normalized version from {args} " + f"(normalized: {tuple(normalized.segments())}).") - def __repr__(self): - args = ", ".join(str(s) for s in self.segments()) - return f'VersionNumber({args})' + self.major = self._ver.majorVersion() + self.minor = self._ver.minorVersion() + self.patch = self._ver.microVersion() + self.segments = self._ver.segments() + + assert len(self.segments) <= 3, self.segments + + def __str__(self) -> str: + return ".".join(str(s) for s in self.segments) + + def __repr__(self) -> str: + args = ", ".join(str(s) for s in self.segments) + return f'VersionNumber({args})' + + def strip_patch(self) -> 'VersionNumber': + """Get a new VersionNumber with the patch version removed.""" + return VersionNumber(*self.segments[:2]) + + @classmethod + def parse(cls, s: str) -> 'VersionNumber': + """Parse a version number from a string.""" + ver, _suffix = QVersionNumber.fromString(s) + # FIXME: Should we support a suffix? + + if ver.isNull(): + raise ValueError(f"Failed to parse {s}") + + return cls(*ver.normalized().segments()) + + def __hash__(self) -> int: + return hash(self._ver) + + def __eq__(self, other: object) -> bool: + if not isinstance(other, VersionNumber): + return NotImplemented + return self._ver == other._ver + + def __ne__(self, other: object) -> bool: + if not isinstance(other, VersionNumber): + return NotImplemented + return self._ver != other._ver + + def __ge__(self, other: 'VersionNumber') -> bool: + return self._ver >= other._ver # type: ignore[operator] + + def __gt__(self, other: 'VersionNumber') -> bool: + return self._ver > other._ver # type: ignore[operator] + + def __le__(self, other: 'VersionNumber') -> bool: + return self._ver <= other._ver # type: ignore[operator] + + def __lt__(self, other: 'VersionNumber') -> bool: + return self._ver < other._ver # type: ignore[operator] class Unreachable(Exception): @@ -294,12 +342,6 @@ def read_file_binary(filename: str) -> bytes: return path.read_bytes() -def parse_version(version: str) -> VersionNumber: - """Parse a version string.""" - ver, _suffix = QVersionNumber.fromString(version) - return VersionNumber(ver.normalized()) - - def format_seconds(total_seconds: int) -> str: """Format a count of seconds to get a [H:]M:SS string.""" prefix = '-' if total_seconds < 0 else '' diff --git a/qutebrowser/utils/version.py b/qutebrowser/utils/version.py index a10f50ffc..4a4455c99 100644 --- a/qutebrowser/utils/version.py +++ b/qutebrowser/utils/version.py @@ -160,7 +160,7 @@ def distribution() -> Optional[DistributionInfo]: dist_version: Optional[utils.VersionNumber] = None for version_key in ['VERSION', 'VERSION_ID']: if version_key in info: - dist_version = utils.parse_version(info[version_key]) + dist_version = utils.VersionNumber.parse(info[version_key]) break dist_id = info.get('ID', None) @@ -568,7 +568,7 @@ class WebEngineVersions: } def __str__(self) -> str: - s = f'QtWebEngine {self.webengine.toString()}' + s = f'QtWebEngine {self.webengine}' if self.chromium is not None: s += f', Chromium {self.chromium}' if self.source != 'UA': @@ -585,7 +585,7 @@ class WebEngineVersions: """ assert ua.qt_version is not None, ua return cls( - webengine=utils.parse_version(ua.qt_version), + webengine=utils.VersionNumber.parse(ua.qt_version), chromium=ua.upstream_browser_version, source='UA', ) @@ -602,7 +602,7 @@ class WebEngineVersions: (though hackish) way to get a more accurate result. """ return cls( - webengine=utils.parse_version(versions.webengine), + webengine=utils.VersionNumber.parse(versions.webengine), chromium=versions.chromium, source='ELF', ) @@ -624,11 +624,7 @@ class WebEngineVersions: minor_version = v5_15_3 else: # e.g. 5.14.2 -> 5.14 - segments = pyqt_webengine_version.segments()[:2] - if segments[-1] == 0: - del segments[-1] # pragma: no cover - - minor_version = utils.VersionNumber(*segments) + minor_version = pyqt_webengine_version.strip_patch() return cls._CHROMIUM_VERSIONS.get(minor_version) @@ -651,7 +647,7 @@ class WebEngineVersions: Note that we only can get the PyQtWebEngine version with PyQt 5.13 or newer. With Qt 5.12, we instead rely on qVersion(). """ - parsed = utils.parse_version(pyqt_webengine_version) + parsed = utils.VersionNumber.parse(pyqt_webengine_version) return cls( webengine=parsed, chromium=cls._infer_chromium_version(parsed), diff --git a/tests/end2end/test_invocations.py b/tests/end2end/test_invocations.py index f3d74d1f0..38e40f9b7 100644 --- a/tests/end2end/test_invocations.py +++ b/tests/end2end/test_invocations.py @@ -657,7 +657,7 @@ def test_dark_mode(webengine_versions, quteproc_new, request, quteproc_new.start(args) ver = webengine_versions.webengine - minor_version = f'{ver.majorVersion()}.{ver.minorVersion()}' + minor_version = str(ver.strip_patch()) expected = colors.get(minor_version, colors[None]) quteproc_new.open_path(f'data/darkmode/{filename}.html') diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py index 4cf60943c..a64622dee 100644 --- a/tests/unit/utils/test_utils.py +++ b/tests/unit/utils/test_utils.py @@ -29,6 +29,7 @@ import re import shlex import math import zipfile +import operator from PyQt5.QtCore import QUrl, QRect from PyQt5.QtGui import QClipboard @@ -44,19 +45,113 @@ from qutebrowser.utils import utils, version, usertypes class TestVersionNumber: - @pytest.mark.parametrize('args, expected', [ - ([5, 15, 2], 'VersionNumber(5, 15, 2)'), - ([5, 15], 'VersionNumber(5, 15)'), - ([5], 'VersionNumber(5)'), + @pytest.mark.parametrize('num, expected', [ + (utils.VersionNumber(5, 15, 2), 'VersionNumber(5, 15, 2)'), + (utils.VersionNumber(5, 15), 'VersionNumber(5, 15)'), + (utils.VersionNumber(5), 'VersionNumber(5)'), ]) - def test_repr(self, args, expected): - num = utils.VersionNumber(*args) + def test_repr(self, num, expected): assert repr(num) == expected + @pytest.mark.parametrize('num, expected', [ + (utils.VersionNumber(5, 15, 2), '5.15.2'), + (utils.VersionNumber(5, 15), '5.15'), + (utils.VersionNumber(5), '5'), + ]) + def test_str(self, num, expected): + assert str(num) == expected + def test_not_normalized(self): with pytest.raises(ValueError, match='Refusing to construct'): utils.VersionNumber(5, 15, 0) + @pytest.mark.parametrize('num, expected', [ + (utils.VersionNumber(5, 15, 2), utils.VersionNumber(5, 15)), + (utils.VersionNumber(5, 15), utils.VersionNumber(5, 15)), + (utils.VersionNumber(6), utils.VersionNumber(6)), + ]) + def test_strip_patch(self, num, expected): + assert num.strip_patch() == expected + + @pytest.mark.parametrize('s, expected', [ + ('1x6.2', utils.VersionNumber(1)), + ('6', utils.VersionNumber(6)), + ('5.15', utils.VersionNumber(5, 15)), + ('5.15.3', utils.VersionNumber(5, 15, 3)), + ('5.15.3.dev1234', utils.VersionNumber(5, 15, 3)), + ]) + def test_parse_valid(self, s, expected): + assert utils.VersionNumber.parse(s) == expected + + @pytest.mark.parametrize('s, message', [ + ('foo6', "Failed to parse foo6"), + ('.6', "Failed to parse .6"), + ('0x6.2', "Can't construct a null version"), + ]) + def test_parse_invalid(self, s, message): + with pytest.raises(ValueError, match=message): + utils.VersionNumber.parse(s) + + @pytest.mark.parametrize('lhs, op, rhs, outcome', [ + # == + (utils.VersionNumber(6), operator.eq, utils.VersionNumber(6), True), + (utils.VersionNumber(6), operator.eq, object(), False), + + # != + (utils.VersionNumber(6), operator.ne, utils.VersionNumber(5), True), + (utils.VersionNumber(6), operator.ne, object(), True), + + # >= + (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(5, 13, 5), True), + (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(5, 14, 2), False), + (utils.VersionNumber(5, 14, 3), operator.ge, utils.VersionNumber(5, 14, 2), True), + (utils.VersionNumber(5, 14, 3), operator.ge, utils.VersionNumber(5, 14, 3), True), + (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(5, 13), True), + (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(5, 14), True), + (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(5, 15), False), + (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(4), True), + (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(5), True), + (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(6), False), + + # > + (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(5, 13, 5), True), + (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(5, 14, 2), False), + (utils.VersionNumber(5, 14, 3), operator.gt, utils.VersionNumber(5, 14, 2), True), + (utils.VersionNumber(5, 14, 3), operator.gt, utils.VersionNumber(5, 14, 3), False), + (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(5, 13), True), + (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(5, 14), False), + (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(5, 15), False), + (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(4), True), + (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(5), True), + (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(6), False), + + # <= + (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(5, 13, 5), False), + (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(5, 14, 2), True), + (utils.VersionNumber(5, 14, 3), operator.le, utils.VersionNumber(5, 14, 2), False), + (utils.VersionNumber(5, 14, 3), operator.le, utils.VersionNumber(5, 14, 3), True), + (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(5, 13), False), + (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(5, 14), True), + (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(5, 15), True), + (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(4), False), + (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(5), False), + (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(6), True), + + # < + (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(5, 13, 5), False), + (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(5, 14, 2), True), + (utils.VersionNumber(5, 14, 3), operator.lt, utils.VersionNumber(5, 14, 2), False), + (utils.VersionNumber(5, 14, 3), operator.lt, utils.VersionNumber(5, 14, 3), False), + (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(5, 13), False), + (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(5, 14), False), + (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(5, 15), True), + (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(4), False), + (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(5), False), + (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(6), True), + ]) + def test_comparisons(self, lhs, op, rhs, outcome): + assert op(lhs, rhs) == outcome + ELLIPSIS = '\u2026' @@ -784,7 +879,7 @@ class TestOpenFile: info = version.DistributionInfo( id='org.kde.Platform', parsed=version.Distribution.kde_flatpak, - version=utils.parse_version('5.12'), + version=utils.VersionNumber.parse('5.12'), pretty='Unknown') monkeypatch.setattr(version, 'distribution', lambda: info) diff --git a/tests/unit/utils/test_version.py b/tests/unit/utils/test_version.py index b7da60011..39b898d59 100644 --- a/tests/unit/utils/test_version.py +++ b/tests/unit/utils/test_version.py @@ -962,7 +962,7 @@ class TestWebEngineVersions: ]) def test_from_pyqt(self, qt_version, chromium_version): expected = version.WebEngineVersions( - webengine=utils.parse_version(qt_version), + webengine=utils.VersionNumber.parse(qt_version), chromium=chromium_version, source='PyQt', ) -- cgit v1.2.3-54-g00ecf