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

Reply via email to