From b2669e84a9010021fabfa137c4c535376730132e Mon Sep 17 00:00:00 2001 From: "Nathan J. Mehl" Date: Fri, 20 Nov 2020 17:56:47 -0500 Subject: [PATCH] Fix tilde comparisons This PR fixes https://github.com/memory/python-dpkg/issues/3, which correctly notes that comparisons of debian versions that share a common root but where the first additional character of the longer version string is a tilde were being handled incorrectly. Also, while we're here: - format with Black - test under python 3.9 - update pep8 test to agree with Black's default line length - add a TODO to refactor the compare_versions function, which now fails pylint's too-many-branches test --- .travis.yml | 5 +++-- README.md | 4 ++-- pydpkg/__init__.py | 16 +++++++++++++--- setup.py | 3 ++- tests/test_dpkg.py | 24 ++++++++++++++++++++++-- 5 files changed, 42 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index d03213a..e91a7f8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,11 +6,12 @@ python: - "3.6" - "3.7" - "3.8" + - "3.9" before_install: - "pip install -U pip" install: - "pip install -e .[test]" script: - "py.test tests/" - - "pylint pydpkg/" - - "pep8 pydpkg/" + - "pylint -d R0912 pydpkg/" + - "pep8 --max-line-length=90 --ignore=E203 pydpkg/" diff --git a/README.md b/README.md index 959b763..e3d7c8d 100644 --- a/README.md +++ b/README.md @@ -8,8 +8,8 @@ This library can be used to: on platforms that generally lack a native implementation of dpkg 2. compare dpkg version strings, using a pure Python implementation of - the algorithm described at - https://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Version + the algorithm described in section 5.6.12 of the debian-policy manual: + https://www.debian.org/doc/debian-policy/ch-controlfields.html#version 3. Parse debian source description (dsc) files, inspect their contents and verify that their source files are present and checksums are diff --git a/pydpkg/__init__.py b/pydpkg/__init__.py index 015ca3a..cd1fc76 100644 --- a/pydpkg/__init__.py +++ b/pydpkg/__init__.py @@ -421,8 +421,8 @@ def listify(revision_str): """Split a revision string into a list of alternating between strings and numbers, padded on either end to always be "str, int, str, int..." and always be of even length. This allows us to trivially implement the - comparison algorithm described at - http://debian.org/doc/debian-policy/ch-controlfields.html#s-f-Version + comparison algorithm described at section 5.6.12 in: + https://www.debian.org/doc/debian-policy/ch-controlfields.html#version """ result = [] while revision_str: @@ -479,8 +479,9 @@ def dstringcmp(a, b): @staticmethod def compare_revision_strings(rev1, rev2): """Compare two debian revision strings as described at - https://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Version + https://www.debian.org/doc/debian-policy/ch-controlfields.html#version """ + # FIXME(memory): this function now fails pylint R0912 too-many-branches if rev1 == rev2: return 0 # listify pads results so that we will always be comparing ints to ints @@ -491,6 +492,9 @@ def compare_revision_strings(rev1, rev2): return 0 try: for i, item in enumerate(list1): + # explicitly raise IndexError if we've fallen off the edge of list2 + if i >= len(list2): + raise IndexError # just in case if not isinstance(item, list2[i].__class__): raise DpkgVersionError( @@ -511,8 +515,14 @@ def compare_revision_strings(rev1, rev2): return Dpkg.dstringcmp(item, list2[i]) except IndexError: # rev1 is longer than rev2 but otherwise equal, hence greater + # ...except for goddamn tildes + if list1[len(list2)][0][0] == "~": + return -1 return 1 # rev1 is shorter than rev2 but otherwise equal, hence lesser + # ...except for goddamn tildes + if list2[len(list1)][0][0] == "~": + return 1 return -1 @staticmethod diff --git a/setup.py b/setup.py index 14c70b3..457b946 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import setup -__VERSION__ = "1.4.4" +__VERSION__ = "1.5.0" setup( name="pydpkg", @@ -33,6 +33,7 @@ "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", + "Programming Language :: Python :: 3.9", "Programming Language :: Python :: Implementation :: CPython", "Topic :: System :: Archiving :: Packaging", ], diff --git a/tests/test_dpkg.py b/tests/test_dpkg.py index 8f5bc46..38cfb90 100644 --- a/tests/test_dpkg.py +++ b/tests/test_dpkg.py @@ -123,8 +123,8 @@ def test_dstringcmp(self): self.assertEqual(Dpkg.dstringcmp("0", "0"), 0) self.assertEqual(Dpkg.dstringcmp("a", "a"), 0) - # taken from - # http://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Version + # taken from section 5.6.12 at: + # https://www.debian.org/doc/debian-policy/ch-controlfields.html#version self.assertEqual( sorted(["a", "", "~", "~~a", "~~"], key=Dpkg.dstringcmp_key), ["~~", "~~a", "~", "", "a"], @@ -192,6 +192,16 @@ def test_compare_versions(self): self.assertEqual(Dpkg.compare_versions("0.0.9", "0.0.10"), -1) self.assertEqual(Dpkg.compare_versions("0.9.0", "0.10.0"), -1) self.assertEqual(Dpkg.compare_versions("9.0.0", "10.0.0"), -1) + self.assertEqual(Dpkg.compare_versions("1.2.3-1~deb7u1", "1.2.3-1"), -1) + self.assertEqual( + Dpkg.compare_versions( + "2.7.4+reloaded2-13ubuntu1", "2.7.4+reloaded2-13+deb9u1" + ), + -1, + ) + self.assertEqual( + Dpkg.compare_versions("2.7.4+reloaded2-13", "2.7.4+reloaded2-13+deb9u1"), -1 + ) # greater than self.assertEqual(Dpkg.compare_versions("0.0.1-0", "0:0.0.0"), 1) @@ -200,6 +210,16 @@ def test_compare_versions(self): self.assertEqual(Dpkg.compare_versions("0.0.9", "0.0.1"), 1) self.assertEqual(Dpkg.compare_versions("0.9.0", "0.1.0"), 1) self.assertEqual(Dpkg.compare_versions("9.0.0", "1.0.0"), 1) + self.assertEqual(Dpkg.compare_versions("1.2.3-1", "1.2.3-1~deb7u1"), 1) + self.assertEqual( + Dpkg.compare_versions( + "2.7.4+reloaded2-13+deb9u1", "2.7.4+reloaded2-13ubuntu1" + ), + 1, + ) + self.assertEqual( + Dpkg.compare_versions("2.7.4+reloaded2-13+deb9u1", "2.7.4+reloaded2-13"), 1 + ) # unicode me harder self.assertEqual(Dpkg.compare_versions(u"2:0.0.44-1", u"2:0.0.44-nobin"), -1)