← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jelmer/launchpad/135610-duplicated-ancestry into lp:launchpad/devel

 

Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/135610-duplicated-ancestry into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
  #55774 Soyuz tests explore "Building from Accepted" when the production doesn't
  https://bugs.launchpad.net/bugs/55774
  #135610 rejected upload, for binary upload + promotion in the same cycle
  https://bugs.launchpad.net/bugs/135610


This patch fixes findSourcePackageRelease() to deal with multiple source package publishing entries for a package/version tuple.

Previously we only supported a single spph for a particular package/version - if there were more we would raise an AssertionError. We now only raise an AssertionError if there is more than one PUBLISHED spph record. Multiple PENDING records are fine. 

This branch also cleans up some tech debt by removing a code path in findSourcePackageRelease that is not used (or planned to be used) in production. Source packages have to be published now before findSourcePackageRelease can find them; the only situation where this wouldn't be the case is in some of the nascent upload tests, which I have fixed.
-- 
https://code.launchpad.net/~jelmer/launchpad/135610-duplicated-ancestry/+merge/38444
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/135610-duplicated-ancestry into lp:launchpad/devel.
=== modified file 'lib/lp/archiveuploader/nascentuploadfile.py'
--- lib/lp/archiveuploader/nascentuploadfile.py	2010-10-02 11:41:43 +0000
+++ lib/lp/archiveuploader/nascentuploadfile.py	2010-10-14 18:06:51 +0000
@@ -50,8 +50,8 @@
 from lp.soyuz.enums import (
     BinaryPackageFormat,
     PackagePublishingPriority,
+    PackagePublishingStatus,
     PackageUploadCustomFormat,
-    PackageUploadStatus,
     )
 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
 from lp.soyuz.interfaces.component import IComponentSet
@@ -781,11 +781,9 @@
     #   Database relationship methods
     #
     def findSourcePackageRelease(self):
-        """Return the respective ISourcePackagRelease for this binary upload.
+        """Return the respective ISourcePackageRelease for this binary upload.
 
-        It inspect publication in the targeted DistroSeries and also the
-        ACCEPTED queue for sources matching stored
-        (source_name, source_version).
+        It inspect publication in the targeted DistroSeries.
 
         It raises UploadError if the source was not found.
 
@@ -793,43 +791,24 @@
         mixed_uploads (source + binary) we do not have the source stored
         in DB yet (see verifySourcepackagerelease).
         """
+        assert self.source_name is not None
+        assert self.source_version is not None
         distroseries = self.policy.distroseries
         spphs = distroseries.getPublishedSources(
             self.source_name, version=self.source_version,
             include_pending=True, archive=self.policy.archive)
 
-        sourcepackagerelease = None
-        if spphs:
-            # We know there's only going to be one release because
-            # version is unique.
-            assert spphs.count() == 1, "Duplicated ancestry"
-            sourcepackagerelease = spphs[0].sourcepackagerelease
-        else:
-            # XXX cprov 2006-08-09 bug=55774: Building from ACCEPTED is
-            # special condition, not really used in production. We should
-            # remove the support for this use case.
-            self.logger.debug(
-                "No source published, checking the ACCEPTED queue")
-
-            queue_candidates = distroseries.getQueueItems(
-                status=PackageUploadStatus.ACCEPTED,
-                name=self.source_name, version=self.source_version,
-                archive=self.policy.archive, exact_match=True)
-
-            for queue_item in queue_candidates:
-                if queue_item.sources.count():
-                    sourcepackagerelease = queue_item.sourcepackagerelease
-
-        if sourcepackagerelease is None:
-            # At this point, we can't really do much more to try
-            # building this package. If we look in the NEW queue it is
-            # possible that multiple versions of the package exist there
-            # and we know how bad that can be. Time to give up!
+        if spphs.count() == 0:
             raise UploadError(
                 "Unable to find source package %s/%s in %s" % (
                 self.source_name, self.source_version, distroseries.name))
 
-        return sourcepackagerelease
+        # There can only be one matching published source package.
+        published_spphs = [
+            spph for spph in spphs
+            if spph.status == PackagePublishingStatus.PUBLISHED]
+        assert len(published_spphs) <= 1, "Duplicated ancestry"
+        return spphs[0].sourcepackagerelease
 
     def verifySourcePackageRelease(self, sourcepackagerelease):
         """Check if the given ISourcePackageRelease matches the context."""

=== modified file 'lib/lp/archiveuploader/tests/nascentupload-epoch-handling.txt'
--- lib/lp/archiveuploader/tests/nascentupload-epoch-handling.txt	2009-04-17 10:32:16 +0000
+++ lib/lp/archiveuploader/tests/nascentupload-epoch-handling.txt	2010-10-14 18:06:51 +0000
@@ -208,6 +208,14 @@
     >>> bar_spr.version
     u'1:1.0-9'
 
+    >>> from lp.registry.interfaces.pocket import PackagePublishingPocket
+    >>> from lp.soyuz.interfaces.publishing import IPublishingSet
+    >>> getUtility(IPublishingSet).newSourcePublication(
+    ...     bar_src_upload.policy.distro.main_archive, bar_spr,
+    ...     bar_src_upload.policy.distroseries, bar_spr.component,
+    ...     bar_spr.section, PackagePublishingPocket.RELEASE)
+    <SourcePackagePublishingHistory at ...>
+
 Let's accept the source and claim 'build from accepted' to process the
 respective binary:
 
@@ -217,9 +225,6 @@
     >>> bar_src_queue.status.name
     'ACCEPTED'
 
-    >>> from canonical.launchpad.ftests import syncUpdate
-    >>> syncUpdate(bar_src_queue)
-
 For a binary upload we expect the same, a BinaryPackageRelease
 'version' that includes 'epoch':
 

=== modified file 'lib/lp/archiveuploader/tests/nascentupload.txt'
--- lib/lp/archiveuploader/tests/nascentupload.txt	2010-10-06 11:46:51 +0000
+++ lib/lp/archiveuploader/tests/nascentupload.txt	2010-10-14 18:06:51 +0000
@@ -484,86 +484,6 @@
   1
 
 
-=== Building From ACCEPTED queue ===
-
-XXX cprov 20060728: Building from ACCEPTED is special condition, not
-really used in production. We should remove the support for this use
-case, see further info in bug #55774.
-
-Next we send in the binaries, since the source should be in ACCEPTED the
-binary should go straight there too. Note that we are not specifying
-any build, so it should be created appropriately, it is what happens
-with staged security uploads:
-
-XXX cprov 20070404: we are using a modified sync policy because we
-need unsigned changes and binaries uploads (same as security, but also
-accepts non-security uploads)
-
-  >>> from lp.archiveuploader.uploadpolicy import ArchiveUploadType
-  >>> modified_sync_policy = getPolicy(
-  ...     name='sync', distro='ubuntu', distroseries='hoary')
-  >>> modified_sync_policy.accepted_type = ArchiveUploadType.BINARY_ONLY
-
-  >>> ed_bin = NascentUpload.from_changesfile_path(
-  ...      datadir('split-upload-test/ed_0.2-20_i386.changes'),
-  ...      modified_sync_policy, mock_logger_quiet)
-
-  >>> ed_bin.process()
-  >>> ed_bin.is_rejected
-  False
-
-  >>> success = ed_bin.do_accept()
-
-  >>> ed_bin.queue_root.status.name
-  'NEW'
-
-A build was created to represent the relationship between ed_src
-(waiting in ACCEPTED queue) and the just uploaded ed_bin:
-
-  >>> ed_build = ed_bin.queue_root.builds[0].build
-
-  >>> ed_spr.id  == ed_build.source_package_release.id
-  True
-
-  >>> ed_build.title
-  u'i386 build of ed 0.2-20 in ubuntu hoary RELEASE'
-
-  >>> ed_build.status.name
-  'FULLYBUILT'
-
-Binary control file attributes are stored as text in the
-BinaryPackageRelease record.
-
-  >>> ed_bpr = ed_build.binarypackages[0]
-
-  >>> ed_bpr.depends
-  u'libc6 (>= 2.6-1)'
-
-  >>> ed_bpr.suggests
-  u'emacs'
-
-  >>> ed_bpr.conflicts
-  u'vi'
-
-  >>> ed_bpr.replaces
-  u'vim'
-
-  >>> ed_bpr.provides
-  u'ed'
-
-  >>> ed_bpr.essential
-  False
-
-  >>> ed_bpr.pre_depends
-  u'dpkg'
-
-  >>> ed_bpr.enhances
-  u'bash'
-
-  >>> ed_bpr.breaks
-  u'emacs'
-
-
 === Staged Source and Binary upload with multiple binaries ===
 
 As we could see both, sources and binaries, get into Launchpad via
@@ -613,12 +533,18 @@
   >>> multibar_src_queue.status.name
   'ACCEPTED'
 
-We can just assume the source was published by step 3 and 4 for
-simplicity and claim 'build from ACCEPTED' feature.
+Then the source gets accepted and published, step 3 and 4:
+
+  >>> from lp.registry.interfaces.pocket import PackagePublishingPocket
+  >>> from lp.soyuz.interfaces.publishing import IPublishingSet
+  >>> getUtility(IPublishingSet).newSourcePublication(
+  ...     multibar_src_queue.archive, multibar_spr,
+  ...     sync_policy.distroseries, multibar_spr.component,
+  ...     multibar_spr.section, PackagePublishingPocket.RELEASE)
+  <SourcePackagePublishingHistory at ...>
 
 Build creation is done based on the SourcePackageRelease object, step 5:
 
-  >>> from lp.registry.interfaces.pocket import PackagePublishingPocket
   >>> multibar_build = multibar_spr.createBuild(
   ...     hoary['i386'], PackagePublishingPocket.RELEASE,
   ...     multibar_src_queue.archive)
@@ -636,8 +562,8 @@
 and the collection of DEB files produced.
 
 At this point slave-scanner moves the upload to the appropriate path
-(/srv/launchpad.net/builddmaster) and invokes process-upload.py with
-the 'buildd' upload policy and the build record id.
+(/srv/launchpad.net/builddmaster). A cron job invokes process-upload.py
+with the 'buildd' upload policy and processes all files in that directory.
 
   >>> buildd_policy = getPolicy(
   ...     name='buildd', distro='ubuntu', distroseries='hoary')

=== modified file 'lib/lp/archiveuploader/tests/test_nascentuploadfile.py'
--- lib/lp/archiveuploader/tests/test_nascentuploadfile.py	2010-10-06 11:46:51 +0000
+++ lib/lp/archiveuploader/tests/test_nascentuploadfile.py	2010-10-14 18:06:51 +0000
@@ -25,7 +25,10 @@
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.archiveuploader.tests import AbsolutelyAnythingGoesUploadPolicy
 from lp.buildmaster.enums import BuildStatus
-from lp.soyuz.enums import PackageUploadCustomFormat
+from lp.soyuz.enums import (
+    PackageUploadCustomFormat,
+    PackagePublishingStatus,
+    )
 from lp.testing import TestCaseWithFactory
 
 
@@ -387,3 +390,97 @@
             "foo_0.42_i386.deb", "main/python", "unknown", "mypkg", "0.42",
             None)
         self.assertRaises(UploadError, uploadfile.checkBuild, build)
+
+    def test_findSourcePackageRelease(self):
+        # findSourcePackageRelease finds the matching SourcePackageRelease.
+        das = self.factory.makeDistroArchSeries(
+            distroseries=self.policy.distroseries, architecturetag="i386")
+        build = self.factory.makeBinaryPackageBuild(
+            distroarchseries=das,
+            archive=self.policy.archive)
+        uploadfile = self.createDebBinaryUploadFile(
+            "foo_0.42_i386.deb", "main/python", "unknown", "mypkg", "0.42",
+            None)
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagename=self.factory.makeSourcePackageName("foo"),
+            distroseries=self.policy.distroseries,
+            version="0.42", archive=self.policy.archive)
+        control = self.getBaseControl()
+        control["Source"] = "foo"
+        uploadfile.parseControl(control)
+        self.assertEquals(
+            spph.sourcepackagerelease, uploadfile.findSourcePackageRelease())
+
+    def test_findSourcePackageRelease_no_spph(self):
+        # findSourcePackageRelease raises UploadError if there is no
+        # SourcePackageRelease.
+        das = self.factory.makeDistroArchSeries(
+            distroseries=self.policy.distroseries, architecturetag="i386")
+        build = self.factory.makeBinaryPackageBuild(
+            distroarchseries=das,
+            archive=self.policy.archive)
+        uploadfile = self.createDebBinaryUploadFile(
+            "foo_0.42_i386.deb", "main/python", "unknown", "mypkg", "0.42",
+            None)
+        control = self.getBaseControl()
+        control["Source"] = "foo"
+        uploadfile.parseControl(control)
+        self.assertRaises(UploadError, uploadfile.findSourcePackageRelease)
+
+    def test_findSourcePackageRelease_multiple_sprs(self):
+        # findSourcePackageRelease finds the last uploaded
+        # SourcePackageRelease and can deal with multiple pending source
+        # package releases.
+        das = self.factory.makeDistroArchSeries(
+            distroseries=self.policy.distroseries, architecturetag="i386")
+        build = self.factory.makeBinaryPackageBuild(
+            distroarchseries=das,
+            archive=self.policy.archive)
+        uploadfile = self.createDebBinaryUploadFile(
+            "foo_0.42_i386.deb", "main/python", "unknown", "mypkg", "0.42",
+            None)
+        spn = self.factory.makeSourcePackageName("foo")
+        spph1 = self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagename=spn,
+            distroseries=self.policy.distroseries,
+            version="0.42", archive=self.policy.archive,
+            status=PackagePublishingStatus.PUBLISHED)
+        spph2 = self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagename=spn,
+            distroseries=self.policy.distroseries,
+            version="0.42", archive=self.policy.archive,
+            status=PackagePublishingStatus.PENDING)
+        control = self.getBaseControl()
+        control["Source"] = "foo"
+        uploadfile.parseControl(control)
+        self.assertEquals(
+            spph2.sourcepackagerelease, uploadfile.findSourcePackageRelease())
+
+    def test_findSourcePackageRelease_duplicated_ancestry(self):
+        # findSourcePackageRelease errors out if there are multiple
+        # published SourcePackageReleases, as this should never
+        # be possible.
+        das = self.factory.makeDistroArchSeries(
+            distroseries=self.policy.distroseries, architecturetag="i386")
+        build = self.factory.makeBinaryPackageBuild(
+            distroarchseries=das,
+            archive=self.policy.archive)
+        uploadfile = self.createDebBinaryUploadFile(
+            "foo_0.42_i386.deb", "main/python", "unknown", "mypkg", "0.42",
+            None)
+        spn = self.factory.makeSourcePackageName("foo")
+        spph1 = self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagename=spn,
+            distroseries=self.policy.distroseries,
+            version="0.42", archive=self.policy.archive,
+            status=PackagePublishingStatus.PUBLISHED)
+        spph2 = self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagename=spn,
+            distroseries=self.policy.distroseries,
+            version="0.42", archive=self.policy.archive,
+            status=PackagePublishingStatus.PUBLISHED)
+        control = self.getBaseControl()
+        control["Source"] = "foo"
+        uploadfile.parseControl(control)
+        self.assertRaises(
+            AssertionError, uploadfile.findSourcePackageRelease)


Follow ups