Merge lp:~stevenk/launchpad/attemptcopy-no-source into lp:launchpad

[Steve Kowalik]

When a PCJ is created to copy just binaries from one pocket to another, the PCJ machinery attempts to update package diffs for the copied source publication. Except there isn't one and so it OOPSes.

I have corrected the lie that was the variable 'copied_sources' and now iterate all copied publications looking for sources and only perform package diff things if we found one.

I will claim LoC credit against https://code.launchpad.net/~stevenk/launchpad/destroy-simplified-branch-ff/+merge/127630 .

[William Grant (wgrant)]

You can probably drop the explicit sourcepackagename and version from the test, but otherwise looks good.

review: Approve (code)

=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py	2012-08-20 18:20:36 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py	2012-10-04 07:04:23 +0000
@@ -76,6 +76,7 @@
     PackageCopyJobType,
     )
 from lp.soyuz.interfaces.packagediff import PackageDiffAlreadyRequested
+from lp.soyuz.interfaces.publishing import ISourcePackagePublishingHistory
 from lp.soyuz.interfaces.queue import IPackageUploadSet
 from lp.soyuz.interfaces.section import ISectionSet
 from lp.soyuz.model.archive import Archive
@@ -602,7 +603,7 @@
         override = self.getSourceOverride()
         copy_policy = self.getPolicyImplementation()
         send_email = copy_policy.send_email(self.target_archive)
-        copied_sources = do_copy(
+        copied_publications = do_copy(
             sources=[source_package], archive=self.target_archive,
             series=self.target_distroseries, pocket=self.target_pocket,
             include_binaries=self.include_binaries, check_permissions=True,
@@ -612,16 +613,21 @@
             unembargo=self.unembargo)
 
         # Add a PackageDiff for this new upload if it has ancestry.
-        if copied_sources and not ancestry.is_empty():
-            from_spr = copied_sources[0].sourcepackagerelease
-            for ancestor in ancestry:
-                to_spr = ancestor.sourcepackagerelease
-                if from_spr != to_spr:
-                    try:
-                        to_spr.requestDiffTo(self.requester, from_spr)
-                    except PackageDiffAlreadyRequested:
-                        pass
+        if copied_publications and not ancestry.is_empty():
+            from_spr = None
+            for publication in copied_publications:
+                if ISourcePackagePublishingHistory.providedBy(publication):
+                    from_spr = publication.sourcepackagerelease
                     break
+            if from_spr:
+                for ancestor in ancestry:
+                    to_spr = ancestor.sourcepackagerelease
+                    if from_spr != to_spr:
+                        try:
+                            to_spr.requestDiffTo(self.requester, from_spr)
+                        except PackageDiffAlreadyRequested:
+                            pass
+                        break
 
         if pu is not None:
             # A PackageUpload will only exist if the copy job had to be

=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py	2012-09-27 02:53:00 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py	2012-10-04 07:04:23 +0000
@@ -1494,6 +1494,44 @@
         # The job should have set the PU status to REJECTED.
         self.assertEqual(PackageUploadStatus.REJECTED, pu.status)
 
+    def test_diffs_are_not_created_when_only_copying_binaries(self):
+        # The job will not fail because a packagediff from a source that wasn't
+        # copied could not be created.
+        archive = self.distroseries.distribution.main_archive
+        source = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=self.distroseries, sourcepackagename="copyme",
+            version="2.8-1", status=PackagePublishingStatus.PUBLISHED,
+            pocket=PackagePublishingPocket.RELEASE, archive=archive,
+            component='multiverse')
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            status=PackagePublishingStatus.PUBLISHED,
+            pocket=PackagePublishingPocket.UPDATES, archive=archive,
+            distroseries=self.distroseries,
+            sourcepackagerelease=source.sourcepackagerelease)
+        das = self.factory.makeDistroArchSeries(distroseries=self.distroseries)
+        self.factory.makeBinaryPackagePublishingHistory(
+            status=PackagePublishingStatus.PUBLISHED, distroarchseries=das,
+            pocket=PackagePublishingPocket.UPDATES, archive=archive,
+            source_package_release=spph.sourcepackagerelease)
+        requester = self.factory.makePerson()
+        with person_logged_in(archive.owner):
+            archive.newComponentUploader(requester, 'multiverse')
+        source = getUtility(IPlainPackageCopyJobSource)
+        job = source.create(
+            package_name="copyme", package_version="2.8-1",
+            source_archive=archive, target_archive=archive,
+            target_distroseries=self.distroseries,
+            target_pocket=PackagePublishingPocket.RELEASE,
+            include_binaries=True, requester=requester)
+        self.runJob(job)
+        self.assertEqual(JobStatus.COMPLETED, job.status)
+        self.assertContentEqual(
+            [], archive.getPublishedSources(
+                status=PackagePublishingStatus.PENDING))
+        self.assertEqual(
+            1, archive.getPublishedOnDiskBinaries(
+                status=PackagePublishingStatus.PENDING).count())
+
 
 class TestViaCelery(TestCaseWithFactory):
     """PackageCopyJob runs under Celery."""