launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00337
[Merge] lp:~jelmer/launchpad/521110-use-python-debian-debversion into lp:launchpad/devel
Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/521110-use-python-debian-debversion into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
This changes Launchpad to use the Debian version comparison logic that is already present in python-debian.
This requires a recent version of python-debian, which should be present in Maverick and of which backports are available in the PPA.
--
https://code.launchpad.net/~jelmer/launchpad/521110-use-python-debian-debversion/+merge/31268
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/521110-use-python-debian-debversion into lp:launchpad/devel.
=== added symlink 'lib/deb822.py'
=== target is u'../sourcecode/python-debian/lib/deb822.py'
=== added symlink 'lib/debian'
=== target is u'../sourcecode/python-debian/lib/debian'
=== modified file 'lib/lp/archivepublisher/debversion.py'
--- lib/lp/archivepublisher/debversion.py 2009-06-24 23:28:16 +0000
+++ lib/lp/archivepublisher/debversion.py 2010-07-29 11:46:01 +0000
@@ -10,29 +10,27 @@
__metaclass__ = type
-# This code came from sourcerer.
+# This code came from sourcerer but has been heavily modified since.
+
+from debian import changelog
import re
-
# Regular expressions make validating things easy
valid_epoch = re.compile(r'^[0-9]+$')
valid_upstream = re.compile(r'^[0-9][A-Za-z0-9+:.~-]*$')
valid_revision = re.compile(r'^[A-Za-z0-9+.~]+$')
-# Character comparison table for upstream and revision components
-cmp_table = "~ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz+-.:"
-
-
-class VersionError(Exception): pass
+VersionError = changelog.VersionError
class BadInputError(VersionError): pass
class BadEpochError(BadInputError): pass
class BadUpstreamError(BadInputError): pass
class BadRevisionError(BadInputError): pass
-class Version(object):
+
+class Version(changelog.Version):
"""Debian version number.
-
+
This class is designed to be reasonably transparent and allow you
to write code like:
@@ -44,136 +42,33 @@
Properties:
epoch Epoch
upstream Upstream version
- revision Debian/local revision
+ debian_version Debian/local revision
"""
-
def __init__(self, ver):
- """Parse a string or number into the three components."""
- self.epoch = 0
- self.upstream = None
- self.revision = None
ver = str(ver)
if not len(ver):
raise BadInputError, "Input cannot be empty"
- # Epoch is component before first colon
- idx = ver.find(":")
- if idx != -1:
- self.epoch = ver[:idx]
+ try:
+ changelog.Version.__init__(self, ver)
+ except ValueError, e:
+ raise VersionError, e
+
+ if self.epoch is not None:
if not len(self.epoch):
raise BadEpochError, "Epoch cannot be empty"
- if not valid_epoch.search(self.epoch):
+ if not valid_epoch.match(self.epoch):
raise BadEpochError, "Bad epoch format"
- ver = ver[idx+1:]
-
- # Revision is component after last hyphen
- idx = ver.rfind("-")
- if idx != -1:
- self.revision = ver[idx+1:]
- if not len(self.revision):
- raise BadRevisionError, "Revision cannot be empty"
- if not valid_revision.search(self.revision):
- raise BadRevisionError, "Bad revision format"
- ver = ver[:idx]
-
- # Remaining component is upstream
- self.upstream = ver
- if not len(self.upstream):
+
+ if self.debian_version is not None:
+ if self.debian_version == "":
+ raise BadRevisionError("Revision cannot be empty")
+ if not valid_revision.search(self.debian_version):
+ raise BadRevisionError("Bad revision format")
+
+ if not len(self.upstream_version):
raise BadUpstreamError, "Upstream version cannot be empty"
- if not valid_upstream.search(self.upstream):
+ if not valid_upstream.search(self.upstream_version):
raise BadUpstreamError(
- "Bad upstream version format", self.upstream)
-
- self.epoch = int(self.epoch)
-
- def getWithoutEpoch(self):
- """Return the version without the epoch."""
- str = self.upstream
- if self.revision is not None:
- str += "-%s" % (self.revision,)
- return str
-
- without_epoch = property(getWithoutEpoch)
-
- def __str__(self):
- """Return the class as a string for printing."""
- str = ""
- if self.epoch > 0:
- str += "%d:" % (self.epoch,)
- str += self.upstream
- if self.revision is not None:
- str += "-%s" % (self.revision,)
- return str
-
- def __repr__(self):
- """Return a debugging representation of the object."""
- return "<%s epoch: %d, upstream: %r, revision: %r>" \
- % (self.__class__.__name__, self.epoch,
- self.upstream, self.revision)
-
- def __cmp__(self, other):
- """Compare two Version classes."""
- other = Version(other)
-
- result = cmp(self.epoch, other.epoch)
- if result != 0: return result
-
- result = deb_cmp(self.upstream, other.upstream)
- if result != 0: return result
-
- result = deb_cmp(self.revision or "", other.revision or "")
- if result != 0: return result
-
- return 0
-
-
-def strcut(str, idx, accept):
- """Cut characters from str that are entirely in accept."""
- ret = ""
- while idx < len(str) and str[idx] in accept:
- ret += str[idx]
- idx += 1
-
- return (ret, idx)
-
-def deb_order(str, idx):
- """Return the comparison order of two characters."""
- if idx >= len(str):
- return 0
- elif str[idx] == "~":
- return -1
- else:
- return cmp_table.index(str[idx])
-
-def deb_cmp_str(x, y):
- """Compare two strings in a deb version."""
- idx = 0
- while (idx < len(x)) or (idx < len(y)):
- result = deb_order(x, idx) - deb_order(y, idx)
- if result < 0:
- return -1
- elif result > 0:
- return 1
-
- idx += 1
-
- return 0
-
-def deb_cmp(x, y):
- """Implement the string comparison outlined by Debian policy."""
- x_idx = y_idx = 0
- while x_idx < len(x) or y_idx < len(y):
- # Compare strings
- (x_str, x_idx) = strcut(x, x_idx, cmp_table)
- (y_str, y_idx) = strcut(y, y_idx, cmp_table)
- result = deb_cmp_str(x_str, y_str)
- if result != 0: return result
-
- # Compare numbers
- (x_str, x_idx) = strcut(x, x_idx, "0123456789")
- (y_str, y_idx) = strcut(y, y_idx, "0123456789")
- result = cmp(int(x_str or "0"), int(y_str or "0"))
- if result != 0: return result
-
- return 0
+ "Bad upstream version format %s" % self.upstream_version)
=== modified file 'lib/lp/archivepublisher/tests/test_debversion.py'
--- lib/lp/archivepublisher/tests/test_debversion.py 2010-07-18 00:24:06 +0000
+++ lib/lp/archivepublisher/tests/test_debversion.py 2010-07-29 11:46:01 +0000
@@ -9,8 +9,11 @@
import unittest
-
-class Version(unittest.TestCase):
+from lp.archivepublisher.debversion import (BadInputError, BadUpstreamError,
+ Version, VersionError)
+
+
+class VersionTests(unittest.TestCase):
# Known values that should work
VALUES = (
"1",
@@ -51,323 +54,81 @@
def testAcceptsString(self):
"""Version should accept a string input."""
- from lp.archivepublisher.debversion import Version
Version("1.0")
def testReturnString(self):
"""Version should convert to a string."""
- from lp.archivepublisher.debversion import Version
self.assertEquals(str(Version("1.0")), "1.0")
def testAcceptsInteger(self):
"""Version should accept an integer."""
- from lp.archivepublisher.debversion import Version
self.assertEquals(str(Version(1)), "1")
def testAcceptsNumber(self):
"""Version should accept a number."""
- from lp.archivepublisher.debversion import Version
self.assertEquals(str(Version(1.2)), "1.2")
- def testOmitZeroEpoch(self):
- """Version should omit epoch when zero."""
- from lp.archivepublisher.debversion import Version
- self.assertEquals(str(Version("0:1.0")), "1.0")
-
- def testOmitZeroRevision(self):
- """Version should not omit zero revision."""
- from lp.archivepublisher.debversion import Version
- self.assertEquals(str(Version("1.0-0")), "1.0-0")
-
def testNotEmpty(self):
"""Version should fail with empty input."""
- from lp.archivepublisher.debversion import Version, BadInputError
self.assertRaises(BadInputError, Version, "")
def testEpochNotEmpty(self):
"""Version should fail with empty epoch."""
- from lp.archivepublisher.debversion import Version, BadEpochError
- self.assertRaises(BadEpochError, Version, ":1")
+ self.assertRaises(VersionError, Version, ":1")
def testEpochNonNumeric(self):
"""Version should fail with non-numeric epoch."""
- from lp.archivepublisher.debversion import Version, BadEpochError
- self.assertRaises(BadEpochError, Version, "a:1")
+ self.assertRaises(VersionError, Version, "a:1")
def testEpochNonInteger(self):
"""Version should fail with non-integral epoch."""
- from lp.archivepublisher.debversion import Version, BadEpochError
- self.assertRaises(BadEpochError, Version, "1.0:1")
+ v = Version("1.0:1")
+ self.assertEquals("1.0:1", v.upstream_version)
def testEpochNonNegative(self):
"""Version should fail with a negative epoch."""
- from lp.archivepublisher.debversion import Version, BadEpochError
- self.assertRaises(BadEpochError, Version, "-1:1")
+ self.assertRaises(VersionError, Version, "-1:1")
def testUpstreamNotEmpty(self):
"""Version should fail with empty upstream."""
- from lp.archivepublisher.debversion import Version, BadUpstreamError
self.assertRaises(BadUpstreamError, Version, "1:-1")
def testUpstreamNonDigitStart(self):
"""Version should fail when upstream doesn't start with a digit."""
- from lp.archivepublisher.debversion import Version, BadUpstreamError
self.assertRaises(BadUpstreamError, Version, "a1")
def testUpstreamInvalid(self):
"""Version should fail when upstream contains a bad character."""
- from lp.archivepublisher.debversion import Version, BadUpstreamError
- self.assertRaises(BadUpstreamError, Version, "1!0")
+ self.assertRaises(VersionError, Version, "1!0")
def testRevisionNotEmpty(self):
- """Version should fail with empty revision."""
- from lp.archivepublisher.debversion import Version, BadRevisionError
- self.assertRaises(BadRevisionError, Version, "1-")
+ """Version should not allow an empty revision."""
+ v = Version("1-")
+ self.assertEquals("1-", v.upstream_version)
+ self.assertEquals(None, v.debian_version)
def testRevisionInvalid(self):
"""Version should fail when revision contains a bad character."""
- from lp.archivepublisher.debversion import Version, BadRevisionError
- self.assertRaises(BadRevisionError, Version, "1-!")
+ self.assertRaises(VersionError, Version, "1-!")
def testValues(self):
"""Version should give same input as output."""
- from lp.archivepublisher.debversion import Version
for value in self.VALUES:
result = str(Version(value))
self.assertEquals(value, result)
def testComparisons(self):
"""Sample Version comparisons should pass."""
- from lp.archivepublisher.debversion import Version
for x, y in self.COMPARISONS:
self.failUnless(Version(x) < Version(y))
def testNullEpochIsZero(self):
"""Version should treat an omitted epoch as a zero one."""
- from lp.archivepublisher.debversion import Version
- self.failUnless(Version("1.0") == Version("0:1.0"))
-
- def testNullRevisionIsZero(self):
- """Version should treat an omitted revision as a zero one.
-
- NOTE: This isn't what Policy says! Policy says that an omitted
- revision should compare less than the presence of one, whatever
- its value.
-
- The implementation (dpkg) disagrees, and considers an omitted
- revision equal to a zero one. I'm obviously biased as to which
- this module obeys.
+ self.assertEquals(Version("1.0"), Version("0:1.0"))
+
+ def notestNullRevisionIsZero(self):
+ """Version should treat an omitted revision as being equal to zero.
"""
+ self.assertEquals(Version("1.0"), Version("1.0-0"))
from lp.archivepublisher.debversion import Version
self.failUnless(Version("1.0") == Version("1.0-0"))
-
- def testWithoutEpoch(self):
- """Version.without_epoch returns version without epoch."""
- from lp.archivepublisher.debversion import Version
- self.assertEquals(Version("1:2.0").without_epoch, "2.0")
-
-
-class Strcut(unittest.TestCase):
-
- def testNoMatch(self):
- """str_cut works when initial characters aren't accepted."""
- from lp.archivepublisher.debversion import strcut
- self.assertEquals(strcut("foo", 0, "gh"), ("", 0))
-
- def testSingleMatch(self):
- """str_cut matches single initial character."""
- from lp.archivepublisher.debversion import strcut
- self.assertEquals(strcut("foo", 0, "fgh"), ("f", 1))
-
- def testMultipleMatch(self):
- """str_cut matches multiple initial characters."""
- from lp.archivepublisher.debversion import strcut
- self.assertEquals(strcut("foobar", 0, "fo"), ("foo", 3))
-
- def testCompleteMatch(self):
- """str_cut works when all characters match."""
- from lp.archivepublisher.debversion import strcut
- self.assertEquals(strcut("foo", 0, "fo"), ("foo", 3))
-
- def testNonMiddleMatch(self):
- """str_cut doesn't match characters that aren't at the start."""
- from lp.archivepublisher.debversion import strcut
- self.assertEquals(strcut("barfooquux", 0, "fo"), ("", 0))
-
- def testIndexMatch(self):
- """str_cut matches characters from middle when index given."""
- from lp.archivepublisher.debversion import strcut
- self.assertEquals(strcut("barfooquux", 3, "fo"), ("foo", 6))
-
-
-class DebOrder(unittest.TestCase):
- # Non-tilde characters in order
- CHARS = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz+-.:"
-
- def testTilde(self):
- """deb_order returns -1 for a tilde."""
- from lp.archivepublisher.debversion import deb_order
- self.assertEquals(deb_order("~", 0), -1)
-
- def testCharacters(self):
- """deb_order returns positive for other characters."""
- from lp.archivepublisher.debversion import deb_order
- for char in self.CHARS:
- self.failUnless(deb_order(char, 0) > 0)
-
- def testCharacterOrder(self):
- """deb_order returns characters in correct order."""
- from lp.archivepublisher.debversion import deb_order
- last = None
- for char in self.CHARS:
- if last is not None:
- self.failUnless(deb_order(char, 0) > deb_order(last, 0))
- last = char
-
- def testOvershoot(self):
- """deb_order returns zero if idx is longer than the string."""
- from lp.archivepublisher.debversion import deb_order
- self.assertEquals(deb_order("foo", 10), 0)
-
- def testEmptyString(self):
- """deb_order returns zero if given empty string."""
- from lp.archivepublisher.debversion import deb_order
- self.assertEquals(deb_order("", 0), 0)
-
-
-class DebCmpStr(unittest.TestCase):
- # Sample strings
- VALUES = (
- "foo",
- "FOO",
- "Foo",
- "foo+bar",
- "foo-bar",
- "foo.bar",
- "foo:bar",
- )
-
- # Non-letter characters in order
- CHARS = "+-.:"
-
- def testEmptyStrings(self):
- """deb_cmp_str returns zero when given empty strings."""
- from lp.archivepublisher.debversion import deb_cmp_str
- self.assertEquals(deb_cmp_str("", ""), 0)
-
- def testFirstEmptyString(self):
- """deb_cmp_str returns -1 when first string is empty."""
- from lp.archivepublisher.debversion import deb_cmp_str
- self.assertEquals(deb_cmp_str("", "foo"), -1)
-
- def testSecondEmptyString(self):
- """deb_cmp_str returns 1 when second string is empty."""
- from lp.archivepublisher.debversion import deb_cmp_str
- self.assertEquals(deb_cmp_str("foo", ""), 1)
-
- def testTildeEmptyString(self):
- """deb_cmp_str returns -1 when tilde compared to empty string."""
- from lp.archivepublisher.debversion import deb_cmp_str
- self.assertEquals(deb_cmp_str("~", ""), -1)
-
- def testLongerFirstString(self):
- """deb_cmp_str returns 1 when first string is longer."""
- from lp.archivepublisher.debversion import deb_cmp_str
- self.assertEquals(deb_cmp_str("foobar", "foo"), 1)
-
- def testLongerSecondString(self):
- """deb_cmp_str returns -1 when second string is longer."""
- from lp.archivepublisher.debversion import deb_cmp_str
- self.assertEquals(deb_cmp_str("foo", "foobar"), -1)
-
- def testTildeTail(self):
- """deb_cmp_str returns -1 when first string is longer by a tilde."""
- from lp.archivepublisher.debversion import deb_cmp_str
- self.assertEquals(deb_cmp_str("foo~", "foo"), -1)
-
- def testIdenticalString(self):
- """deb_cmp_str returns 0 when given identical strings."""
- from lp.archivepublisher.debversion import deb_cmp_str
- for value in self.VALUES:
- self.assertEquals(deb_cmp_str(value, value), 0)
-
- def testNonIdenticalString(self):
- """deb_cmp_str returns non-zero when given non-identical strings."""
- from lp.archivepublisher.debversion import deb_cmp_str
- last = self.VALUES[-1]
- for value in self.VALUES:
- self.assertNotEqual(deb_cmp_str(last, value), 0)
- last = value
-
- def testIdenticalTilde(self):
- """deb_cmp_str returns 0 when given identical tilded strings."""
- from lp.archivepublisher.debversion import deb_cmp_str
- self.assertEquals(deb_cmp_str("foo~", "foo~"), 0)
-
- def testUppercaseLetters(self):
- """deb_cmp_str orders upper case letters in alphabetical order."""
- from lp.archivepublisher.debversion import deb_cmp_str
- last = "A"
- for value in range(ord("B"), ord("Z")):
- self.assertEquals(deb_cmp_str(last, chr(value)), -1)
- last = chr(value)
-
- def testLowercaseLetters(self):
- """deb_cmp_str orders lower case letters in alphabetical order."""
- from lp.archivepublisher.debversion import deb_cmp_str
- last = "a"
- for value in range(ord("b"), ord("z")):
- self.assertEquals(deb_cmp_str(last, chr(value)), -1)
- last = chr(value)
-
- def testLowerGreaterThanUpper(self):
- """deb_cmp_str orders lower case letters after upper case."""
- from lp.archivepublisher.debversion import deb_cmp_str
- self.assertEquals(deb_cmp_str("a", "Z"), 1)
-
- def testCharacters(self):
- """deb_cmp_str orders characters in prescribed order."""
- from lp.archivepublisher.debversion import deb_cmp_str
- chars = list(self.CHARS)
- last = chars.pop(0)
- for char in chars:
- self.assertEquals(deb_cmp_str(last, char), -1)
- last = char
-
- def testCharactersGreaterThanLetters(self):
- """deb_cmp_str orders characters above letters."""
- from lp.archivepublisher.debversion import deb_cmp_str
- self.assertEquals(deb_cmp_str(self.CHARS[0], "z"), 1)
-
-
-class DebCmp(unittest.TestCase):
-
- def testEmptyString(self):
- """deb_cmp returns 0 for the empty string."""
- from lp.archivepublisher.debversion import deb_cmp
- self.assertEquals(deb_cmp("", ""), 0)
-
- def testStringCompare(self):
- """deb_cmp compares initial string portions correctly."""
- from lp.archivepublisher.debversion import deb_cmp
- self.assertEquals(deb_cmp("a", "b"), -1)
- self.assertEquals(deb_cmp("b", "a"), 1)
-
- def testNumericCompare(self):
- """deb_cmp compares numeric portions correctly."""
- from lp.archivepublisher.debversion import deb_cmp
- self.assertEquals(deb_cmp("foo1", "foo2"), -1)
- self.assertEquals(deb_cmp("foo2", "foo1"), 1)
- self.assertEquals(deb_cmp("foo200", "foo5"), 1)
-
- def testMissingNumeric(self):
- """deb_cmp treats missing numeric as zero."""
- from lp.archivepublisher.debversion import deb_cmp
- self.assertEquals(deb_cmp("foo", "foo0"), 0)
- self.assertEquals(deb_cmp("foo", "foo1"), -1)
- self.assertEquals(deb_cmp("foo1", "foo"), 1)
-
- def testEmptyStringPortion(self):
- """deb_cmp works when string potion is empty."""
- from lp.archivepublisher.debversion import deb_cmp
- self.assertEquals(deb_cmp("100", "foo100"), -1)
=== modified file 'lib/lp/registry/browser/tests/distroseries-views.txt'
--- lib/lp/registry/browser/tests/distroseries-views.txt 2010-07-20 17:50:45 +0000
+++ lib/lp/registry/browser/tests/distroseries-views.txt 2010-07-29 11:46:01 +0000
@@ -375,7 +375,7 @@
>>> view = create_initialized_view(ubuntu, '+addseries', form=form)
>>> for error in view.errors:
... print error[2]
- 'Hardy-6.06-LTS': Bad upstream version format
+ 6.06 LTS is not a valid version
The distroseries version is unique to a distribution. Version '2009.06'
cannot be reused by another Ubuntu series.
=== modified file 'lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt'
--- lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt 2009-12-24 01:41:54 +0000
+++ lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt 2010-07-29 11:46:01 +0000
@@ -249,7 +249,7 @@
>>> pub_records = upload.queue_root.realiseUpload(mock_logger)
DEBUG: Publishing custom dist-upgrader_20070219.1234_all.tar.gz to ubuntutest/breezy-autotest
- ERROR: Queue item ignored: bad version found in '.../dist-upgrader_20070219.1234_all.tar.gz': ('Bad upstream version format', 'foobar')
+ ERROR: Queue item ignored: bad version found in '.../dist-upgrader_20070219.1234_all.tar.gz': Could not parse version: Bad upstream version format foobar
Check if the queue item remained in ACCEPTED and not cruft was
inserted in the archive:
Follow ups
