Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/rb-521110 into lp:launchpad/devel.
Requested reviews: Launchpad code reviewers (launchpad-reviewers): code This reverts the fix for bug 521110, as it requires a recent python-debian which is not available on the buildbot. -- https://code.launchpad.net/~jelmer/launchpad/rb-521110/+merge/31704 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/rb-521110 into lp:launchpad/devel.
=== modified file 'lib/lp/archivepublisher/debversion.py' --- lib/lp/archivepublisher/debversion.py 2010-07-30 14:42:37 +0000 +++ lib/lp/archivepublisher/debversion.py 2010-08-03 22:13:49 +0000 @@ -10,37 +10,27 @@ __metaclass__ = type -# This code came from sourcerer but has been heavily modified since. - -from debian import changelog +# This code came from sourcerer. 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+.~]+$') -VersionError = changelog.VersionError - - -class BadInputError(VersionError): - pass - - -class BadEpochError(BadInputError): - pass - - -class BadUpstreamError(BadInputError): - pass - - -class BadRevisionError(BadInputError): - pass - - -class Version(changelog.Version): +# Character comparison table for upstream and revision components +cmp_table = "~ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz+-.:" + + +class VersionError(Exception): pass +class BadInputError(VersionError): pass +class BadEpochError(BadInputError): pass +class BadUpstreamError(BadInputError): pass +class BadRevisionError(BadInputError): pass + +class Version(object): """Debian version number. This class is designed to be reasonably transparent and allow you @@ -54,34 +44,136 @@ Properties: epoch Epoch upstream Upstream version - debian_version Debian/local revision + revision 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") - - try: - changelog.Version.__init__(self, ver) - except ValueError, e: - raise VersionError(e) - - if self.epoch is not None: + raise BadInputError, "Input cannot be empty" + + # Epoch is component before first colon + idx = ver.find(":") + if idx != -1: + self.epoch = ver[:idx] if not len(self.epoch): - raise BadEpochError("Epoch cannot be empty") - if not valid_epoch.match(self.epoch): - raise BadEpochError("Bad epoch format") - - 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_version): + raise BadEpochError, "Epoch cannot be empty" + if not valid_epoch.search(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): + raise BadUpstreamError, "Upstream version cannot be empty" + if not valid_upstream.search(self.upstream): raise BadUpstreamError( - "Bad upstream version format %s" % self.upstream_version) + "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 === modified file 'lib/lp/archivepublisher/tests/test_debversion.py' --- lib/lp/archivepublisher/tests/test_debversion.py 2010-07-29 11:30:55 +0000 +++ lib/lp/archivepublisher/tests/test_debversion.py 2010-08-03 22:13:49 +0000 @@ -9,11 +9,8 @@ import unittest -from lp.archivepublisher.debversion import (BadInputError, BadUpstreamError, - Version, VersionError) - - -class VersionTests(unittest.TestCase): + +class Version(unittest.TestCase): # Known values that should work VALUES = ( "1", @@ -54,81 +51,323 @@ 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.""" - self.assertRaises(VersionError, Version, ":1") + from lp.archivepublisher.debversion import Version, BadEpochError + self.assertRaises(BadEpochError, Version, ":1") def testEpochNonNumeric(self): """Version should fail with non-numeric epoch.""" - self.assertRaises(VersionError, Version, "a:1") + from lp.archivepublisher.debversion import Version, BadEpochError + self.assertRaises(BadEpochError, Version, "a:1") def testEpochNonInteger(self): """Version should fail with non-integral epoch.""" - v = Version("1.0:1") - self.assertEquals("1.0:1", v.upstream_version) + from lp.archivepublisher.debversion import Version, BadEpochError + self.assertRaises(BadEpochError, Version, "1.0:1") def testEpochNonNegative(self): """Version should fail with a negative epoch.""" - self.assertRaises(VersionError, Version, "-1:1") + from lp.archivepublisher.debversion import Version, BadEpochError + self.assertRaises(BadEpochError, 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.""" - self.assertRaises(VersionError, Version, "1!0") + from lp.archivepublisher.debversion import Version, BadUpstreamError + self.assertRaises(BadUpstreamError, Version, "1!0") def testRevisionNotEmpty(self): - """Version should not allow an empty revision.""" - v = Version("1-") - self.assertEquals("1-", v.upstream_version) - self.assertEquals(None, v.debian_version) + """Version should fail with empty revision.""" + from lp.archivepublisher.debversion import Version, BadRevisionError + self.assertRaises(BadRevisionError, Version, "1-") def testRevisionInvalid(self): """Version should fail when revision contains a bad character.""" - self.assertRaises(VersionError, Version, "1-!") + from lp.archivepublisher.debversion import Version, BadRevisionError + self.assertRaises(BadRevisionError, 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.""" - self.assertEquals(Version("1.0"), Version("0:1.0")) - - def notestNullRevisionIsZero(self): - """Version should treat an omitted revision as being equal to zero. + 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("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-08-03 14:52:38 +0000 +++ lib/lp/registry/browser/tests/distroseries-views.txt 2010-08-03 22:13:49 +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': Could not parse version... + 'Hardy-6.06-LTS': Bad upstream version format The distroseries version is unique to a distribution. Version '2009.06' cannot be reused by another Ubuntu series. === modified file 'lib/lp/registry/interfaces/distroseries.py' --- lib/lp/registry/interfaces/distroseries.py 2010-08-03 14:59:22 +0000 +++ lib/lp/registry/interfaces/distroseries.py 2010-08-03 22:13:49 +0000 @@ -127,7 +127,7 @@ Version(version) except VersionError, error: raise LaunchpadValidationError( - "'%s': %s" % (version, error)) + "'%s': %s" % (version, error[0])) class IDistroSeriesEditRestricted(Interface): === modified file 'lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt' --- lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt 2010-06-30 14:09:06 +0000 +++ lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt 2010-08-03 22:13:49 +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': Could not parse version: Bad upstream version format foobar + ERROR: Queue item ignored: bad version found in '.../dist-upgrader_20070219.1234_all.tar.gz': ('Bad upstream version format', 'foobar') Check if the queue item remained in ACCEPTED and not cruft was inserted in the archive:
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp