launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25201
[Merge] ~cjwatson/launchpad:version-lookup-as-text into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:version-lookup-as-text into launchpad:master with ~cjwatson/launchpad:db-xpr-version-text as a prerequisite.
Commit message:
Match source/binary versions as exact strings
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1881598 in Launchpad itself: "ubuntutools.archive.UbuntuSourcePackage().pull() fails (take 2)"
https://bugs.launchpad.net/launchpad/+bug/1881598
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/389597
There is one case in the primary archive of a package (libapache-authznetldap-perl) with both 0.7-4 and 0.07-4 versions, which compare equal according to Debian version comparison rules and thus the debversion type, but are obviously unequal strings. Adjust version-based lookups to match versions as text strings rather than as versions.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:version-lookup-as-text into launchpad:master.
diff --git a/lib/lp/registry/model/distributionsourcepackage.py b/lib/lp/registry/model/distributionsourcepackage.py
index 9e40f02..da4a021 100644
--- a/lib/lp/registry/model/distributionsourcepackage.py
+++ b/lib/lp/registry/model/distributionsourcepackage.py
@@ -261,7 +261,7 @@ class DistributionSourcePackage(BugTargetBase,
SourcePackageRelease.id AND
SourcePackagePublishingHistory.sourcepackagename = %s AND
SourcePackageRelease.sourcepackagename = %s AND
- SourcePackageRelease.version = %s
+ SourcePackageRelease.version::text = %s
""" % sqlvalues(self.distribution,
self.distribution.all_distro_archive_ids,
self.sourcepackagename,
diff --git a/lib/lp/registry/model/distroseriesdifference.py b/lib/lp/registry/model/distroseriesdifference.py
index 1735ea6..3cdcd16 100644
--- a/lib/lp/registry/model/distroseriesdifference.py
+++ b/lib/lp/registry/model/distroseriesdifference.py
@@ -23,6 +23,7 @@ import six
from sqlobject import StringCol
from storm.expr import (
And,
+ Cast,
Column,
Desc,
Or,
@@ -147,7 +148,7 @@ def most_recent_publications(dsds, in_parent, statuses, match_version=False):
conditions,
SourcePackageRelease.id ==
SourcePackagePublishingHistory.sourcepackagereleaseID,
- SourcePackageRelease.version == version_col,
+ Cast(SourcePackageRelease.version, "text") == version_col,
)
# The sort order is critical so that the DISTINCT ON clause selects the
# most recent publication (i.e. the one with the highest id).
diff --git a/lib/lp/registry/model/sourcepackage.py b/lib/lp/registry/model/sourcepackage.py
index 42da907..9e40417 100644
--- a/lib/lp/registry/model/sourcepackage.py
+++ b/lib/lp/registry/model/sourcepackage.py
@@ -217,12 +217,11 @@ class SourcePackage(BugTargetBase, HasCodeImportsMixin,
return '<%s %r %r %r>' % (self.__class__.__name__,
self.distribution, self.distroseries, self.sourcepackagename)
- def _getPublishingHistory(self, version=None, include_status=None,
- exclude_status=None, order_by=None):
+ def _getPublishingHistory(self, include_status=None, order_by=None):
"""Build a query and return a list of SourcePackagePublishingHistory.
This is mainly a helper function for this class so that code is
- not duplicated. include_status and exclude_status must be a sequence.
+ not duplicated. include_status must be a sequence.
"""
clauses = []
clauses.append(
@@ -235,9 +234,6 @@ class SourcePackage(BugTargetBase, HasCodeImportsMixin,
self.sourcepackagename,
self.distroseries,
self.distribution.all_distro_archive_ids))
- if version:
- clauses.append(
- "SourcePackageRelease.version = %s" % sqlvalues(version))
if include_status:
if not isinstance(include_status, list):
@@ -245,12 +241,6 @@ class SourcePackage(BugTargetBase, HasCodeImportsMixin,
clauses.append("SourcePackagePublishingHistory.status IN %s"
% sqlvalues(include_status))
- if exclude_status:
- if not isinstance(exclude_status, list):
- exclude_status = list(exclude_status)
- clauses.append("SourcePackagePublishingHistory.status NOT IN %s"
- % sqlvalues(exclude_status))
-
query = " AND ".join(clauses)
if not order_by:
@@ -260,12 +250,10 @@ class SourcePackage(BugTargetBase, HasCodeImportsMixin,
query, orderBy=order_by, clauseTables=['SourcePackageRelease'],
prejoinClauseTables=['SourcePackageRelease'])
- def _getFirstPublishingHistory(self, version=None, include_status=None,
- exclude_status=None, order_by=None):
+ def _getFirstPublishingHistory(self, include_status=None, order_by=None):
"""As _getPublishingHistory, but just returns the first item."""
try:
- package = self._getPublishingHistory(
- version, include_status, exclude_status, order_by)[0]
+ package = self._getPublishingHistory(include_status, order_by)[0]
except IndexError:
return None
else:
diff --git a/lib/lp/registry/tests/test_distributionsourcepackage.py b/lib/lp/registry/tests/test_distributionsourcepackage.py
index 15da65e..677d080 100644
--- a/lib/lp/registry/tests/test_distributionsourcepackage.py
+++ b/lib/lp/registry/tests/test_distributionsourcepackage.py
@@ -6,7 +6,10 @@
__metaclass__ = type
from storm.store import Store
-from testtools.matchers import Equals
+from testtools.matchers import (
+ Equals,
+ MatchesStructure,
+ )
import transaction
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
@@ -165,6 +168,21 @@ class TestDistributionSourcePackage(TestCaseWithFactory):
driver = distribution.drivers[0]
self.assertTrue(dsp.personHasDriverRights(driver))
+ def test_getVersion_matches_version_as_text(self):
+ # Versions such as 0.7-4 and 0.07-4 are equal according to the
+ # "debversion" type, but for lookup purposes we compare the text of
+ # the version strings exactly.
+ distribution = self.factory.makeDistribution()
+ dsp = self.factory.makeDistributionSourcePackage(
+ distribution=distribution)
+ spph = self.factory.makeSourcePackagePublishingHistory(
+ archive=distribution.main_archive,
+ sourcepackagename=dsp.sourcepackagename, version="0.7-4")
+ self.assertThat(dsp.getVersion("0.7-4"), MatchesStructure.byEquality(
+ distribution=distribution,
+ sourcepackagerelease=spph.sourcepackagerelease))
+ self.assertIsNone(dsp.getVersion("0.07-4"))
+
class TestDistributionSourcePackageFindRelatedArchives(TestCaseWithFactory):
diff --git a/lib/lp/soyuz/model/archive.py b/lib/lp/soyuz/model/archive.py
index f5f711b..3217e1d 100644
--- a/lib/lp/soyuz/model/archive.py
+++ b/lib/lp/soyuz/model/archive.py
@@ -26,6 +26,7 @@ from sqlobject import (
from storm.base import Storm
from storm.expr import (
And,
+ Cast,
Count,
Desc,
Join,
@@ -656,7 +657,8 @@ class Archive(SQLBase):
raise VersionRequiresName(
"The 'version' parameter can be used only together with"
" the 'name' parameter.")
- clauses.append(SourcePackageRelease.version == version)
+ clauses.append(
+ Cast(SourcePackageRelease.version, "text") == version)
elif not order_by_date:
order_by.insert(1, Desc(SourcePackageRelease.version))
@@ -854,7 +856,8 @@ class Archive(SQLBase):
"The 'version' parameter can be used only together with"
" the 'name' parameter.")
- clauses.append(BinaryPackageRelease.version == version)
+ clauses.append(
+ Cast(BinaryPackageRelease.version, "text") == version)
elif ordered:
order_by.insert(1, Desc(BinaryPackageRelease.version))
@@ -1726,7 +1729,7 @@ class Archive(SQLBase):
SourcePackageRelease.id,
SourcePackageRelease.sourcepackagename == SourcePackageName.id,
SourcePackageName.name == name,
- SourcePackageRelease.version == version,
+ Cast(SourcePackageRelease.version, "text") == version,
SourcePackageRelease.id ==
SourcePackageReleaseFile.sourcepackagereleaseID,
SourcePackageReleaseFile.libraryfileID == LibraryFileAlias.id,
@@ -1750,7 +1753,7 @@ class Archive(SQLBase):
BinaryPackagePublishingHistory.binarypackagename == name,
BinaryPackagePublishingHistory.binarypackagereleaseID ==
BinaryPackageRelease.id,
- BinaryPackageRelease.version == version,
+ Cast(BinaryPackageRelease.version, "text") == version,
BinaryPackageBuild.id == BinaryPackageRelease.buildID,
DistroArchSeries.id == BinaryPackageBuild.distro_arch_series_id,
DistroArchSeries.architecturetag == archtag,
diff --git a/lib/lp/soyuz/model/distroarchseriesbinarypackage.py b/lib/lp/soyuz/model/distroarchseriesbinarypackage.py
index 180a874..a2a92f5 100644
--- a/lib/lp/soyuz/model/distroarchseriesbinarypackage.py
+++ b/lib/lp/soyuz/model/distroarchseriesbinarypackage.py
@@ -9,6 +9,7 @@ __all__ = [
'DistroArchSeriesBinaryPackage',
]
+from storm.expr import Cast
from storm.locals import Desc
from zope.interface import implementer
@@ -120,7 +121,7 @@ class DistroArchSeriesBinaryPackage:
"""See IDistroArchSeriesBinaryPackage."""
bpph = IStore(BinaryPackagePublishingHistory).find(
BinaryPackagePublishingHistory,
- BinaryPackageRelease.version == version,
+ Cast(BinaryPackageRelease.version, "text") == version,
*self._getPublicationJoins()
).order_by(Desc(BinaryPackagePublishingHistory.datecreated)
).first()
diff --git a/lib/lp/soyuz/model/publishing.py b/lib/lp/soyuz/model/publishing.py
index 5e6173a..52ffae4 100644
--- a/lib/lp/soyuz/model/publishing.py
+++ b/lib/lp/soyuz/model/publishing.py
@@ -30,6 +30,7 @@ from sqlobject import (
)
from storm.expr import (
And,
+ Cast,
Desc,
Join,
LeftJoin,
@@ -1061,7 +1062,7 @@ class PublishingSet:
BinaryPackagePublishingHistory.distroarchseriesID == das.id,
BinaryPackagePublishingHistory.binarypackagenameID ==
bpr.binarypackagenameID,
- BinaryPackageRelease.version == bpr.version,
+ Cast(BinaryPackageRelease.version, "text") == bpr.version,
)
candidates = (
diff --git a/lib/lp/soyuz/model/queue.py b/lib/lp/soyuz/model/queue.py
index 82e02a9..6465bd8 100644
--- a/lib/lp/soyuz/model/queue.py
+++ b/lib/lp/soyuz/model/queue.py
@@ -23,6 +23,7 @@ from sqlobject import (
SQLObjectNotFound,
StringCol,
)
+from storm.expr import Cast
from storm.locals import (
And,
Desc,
@@ -1487,7 +1488,7 @@ class PackageUploadSet:
PackageUpload.status.is_in(approved_status),
PackageUpload.archive == archive,
DistroSeries.distribution == distribution,
- SourcePackageRelease.version == version,
+ Cast(SourcePackageRelease.version, "text") == version,
SourcePackageName.name == name)
return conflicts.one()
diff --git a/lib/lp/soyuz/scripts/gina/handlers.py b/lib/lp/soyuz/scripts/gina/handlers.py
index 597e5aa..99c8166 100644
--- a/lib/lp/soyuz/scripts/gina/handlers.py
+++ b/lib/lp/soyuz/scripts/gina/handlers.py
@@ -560,7 +560,7 @@ class SourcePackageHandler:
# the distribution, no matter what status.
query = """
SourcePackageRelease.sourcepackagename = %s AND
- SourcePackageRelease.version = %s AND
+ SourcePackageRelease.version::text = %s AND
SourcePackagePublishingHistory.sourcepackagerelease =
SourcePackageRelease.id AND
SourcePackagePublishingHistory.distroseries =
@@ -762,7 +762,7 @@ class BinaryPackageHandler:
"BinaryPackageRelease.id ="
" BinaryPackagePublishingHistory.binarypackagerelease AND "
"BinaryPackageRelease.binarypackagename=%s AND "
- "BinaryPackageRelease.version=%s AND "
+ "BinaryPackageRelease.version::text = %s AND "
"BinaryPackageRelease.build = BinaryPackageBuild.id AND "
"BinaryPackageBuild.distro_arch_series = DistroArchSeries.id AND "
"DistroArchSeries.distroseries = DistroSeries.id AND "
diff --git a/lib/lp/soyuz/tests/test_archive.py b/lib/lp/soyuz/tests/test_archive.py
index b015953..91acf53 100644
--- a/lib/lp/soyuz/tests/test_archive.py
+++ b/lib/lp/soyuz/tests/test_archive.py
@@ -1565,6 +1565,23 @@ class TestGetBinaryPackageRelease(TestCaseWithFactory):
self.factory.makeArchive().getBinaryPackageRelease(
self.bpns['foo-bin'], '1.2.3-4', 'i386'))
+ def test_matches_version_as_text(self):
+ # Versions such as 1.2.3-4 and 1.02.003-4 are equal according to the
+ # "debversion" type, but for lookup purposes we compare the text of
+ # the version strings exactly.
+ other_i386_pub, other_hppa_pub = self.publisher.getPubBinaries(
+ version='1.02.003-4', archive=self.archive, binaryname='foo-bin',
+ status=PackagePublishingStatus.PUBLISHED,
+ architecturespecific=True)
+ self.assertEqual(
+ self.i386_pub.binarypackagerelease,
+ self.archive.getBinaryPackageRelease(
+ self.bpns['foo-bin'], '1.2.3-4', 'i386'))
+ self.assertEqual(
+ other_i386_pub.binarypackagerelease,
+ self.archive.getBinaryPackageRelease(
+ self.bpns['foo-bin'], '1.02.003-4', 'i386'))
+
class TestGetBinaryPackageReleaseByFileName(TestCaseWithFactory):
"""Ensure that getBinaryPackageReleaseByFileName works as expected."""
@@ -2594,6 +2611,28 @@ class TestGetSourceFileByName(TestCaseWithFactory):
pub2.source_package_name, pub2.source_package_version,
dsc2.filename))
+ def test_matches_version_as_text(self):
+ # Versions such as 0.7-4 and 0.7-04 are equal according to the
+ # "debversion" type, but for lookup purposes we compare the text of
+ # the version strings exactly.
+ pub = self.factory.makeSourcePackagePublishingHistory(
+ archive=self.archive, version='0.7-4')
+ orig = self.factory.makeLibraryFileAlias(
+ filename='foo_0.7.orig.tar.gz')
+ pub.sourcepackagerelease.addFile(orig)
+ pub2 = self.factory.makeSourcePackagePublishingHistory(
+ archive=self.archive, sourcepackagename=pub.sourcepackagename.name,
+ version='0.7-04')
+ orig2 = self.factory.makeLibraryFileAlias(
+ filename='foo_0.7.orig.tar.gz')
+ pub2.sourcepackagerelease.addFile(orig2)
+ self.assertEqual(
+ orig, self.archive.getSourceFileByName(
+ pub.sourcepackagename.name, '0.7-4', orig.filename))
+ self.assertEqual(
+ orig2, self.archive.getSourceFileByName(
+ pub.sourcepackagename.name, '0.7-04', orig2.filename))
+
class TestGetPublishedSources(TestCaseWithFactory):
@@ -2744,6 +2783,22 @@ class TestGetPublishedSources(TestCaseWithFactory):
[pubs[i] for i in (3, 2, 1, 0, 4)],
list(archive.getPublishedSources(order_by_date=True)))
+ def test_matches_version_as_text(self):
+ # Versions such as 0.7-4 and 0.07-4 are equal according to the
+ # "debversion" type, but for lookup purposes we compare the text of
+ # the version strings exactly.
+ archive = self.factory.makeArchive()
+ pub = self.factory.makeSourcePackagePublishingHistory(
+ archive=archive, version='0.7-4')
+ self.assertEqual(
+ [pub], list(archive.getPublishedSources(
+ name=pub.sourcepackagename.name, version='0.7-4',
+ exact_match=True)))
+ self.assertEqual(
+ [], list(archive.getPublishedSources(
+ name=pub.sourcepackagename.name, version='0.07-4',
+ exact_match=True)))
+
class TestGetPublishedSourcesWebService(TestCaseWithFactory):
@@ -3499,6 +3554,24 @@ class TestgetAllPublishedBinaries(TestCaseWithFactory):
[pubs[i] for i in (3, 2, 1, 0, 4)],
list(archive.getAllPublishedBinaries(order_by_date=True)))
+ def test_matches_version_as_text(self):
+ # Versions such as 0.7-4 and 0.07-4 are equal according to the
+ # "debversion" type, but for lookup purposes we compare the text of
+ # the version strings exactly.
+ archive = self.factory.makeArchive()
+ pub = self.factory.makeBinaryPackagePublishingHistory(
+ archive=archive, version='0.7-4')
+ self.assertEqual(
+ [pub],
+ list(archive.getAllPublishedBinaries(
+ name=pub.binarypackagename.name, version='0.7-4',
+ exact_match=True)))
+ self.assertEqual(
+ [],
+ list(archive.getAllPublishedBinaries(
+ name=pub.binarypackagename.name, version='0.07-4',
+ exact_match=True)))
+
class TestRemovingPermissions(TestCaseWithFactory):
