launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00417
[Merge] lp:~jelmer/launchpad/rb-521110 into lp:launchpad/devel
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:
Follow ups
