launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/copies-respect-new into lp:launchpad.

Commit message:
Send copies to NEW if they contain binaries without overrides.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #993120 in Launchpad itself: "Copy from PPA with binaries evades NEW and puts new packages into universe"
  https://bugs.launchpad.net/launchpad/+bug/993120

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/copies-respect-new/+merge/221529

Make binaryful copies respect NEW.  This is not ideal because it results in entries in the NEW queue which don't really explain why they're there; however, it's a first step towards a more reasonable representation of copies.  This is particularly important to make sure that there's an opportunity for packages staged in PPAs to receive proper review when copied into the Ubuntu archive.
-- 
https://code.launchpad.net/~cjwatson/launchpad/copies-respect-new/+merge/221529
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/copies-respect-new into lp:launchpad.

=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml	2014-02-18 11:40:52 +0000
+++ lib/lp/soyuz/configure.zcml	2014-05-30 12:14:46 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2014 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -199,7 +199,6 @@
         <require
             permission="launchpad.Edit"
             attributes="
-                setNew
                 setUnapproved
                 setRejected
                 setAccepted

=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py	2014-05-15 08:49:44 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py	2014-05-30 12:14:46 +0000
@@ -515,8 +515,7 @@
             raise CannotCopy("Package %r %r not found." % (name, version))
         return source_package
 
-    def _checkPolicies(self, source_name, source_component=None,
-                       auto_approve=False):
+    def _checkPolicies(self, source_name, source_package, auto_approve=False):
         # This helper will only return if it's safe to carry on with the
         # copy, otherwise it raises SuspendJobException to tell the job
         # runner to suspend the job.
@@ -532,7 +531,8 @@
             # metadata.
             defaults = UnknownOverridePolicy().calculateSourceOverrides(
                 self.target_archive, self.target_distroseries,
-                self.target_pocket, [source_name], source_component)
+                self.target_pocket, [source_name],
+                source_package.sourcepackagerelease.component)
             self.addSourceOverride(defaults[0])
             if auto_approve:
                 auto_approve = self.target_archive.canAdministerQueue(
@@ -556,6 +556,29 @@
                     self.requester, self.getSourceOverride().component,
                     self.target_pocket, self.target_distroseries)
 
+        # Go through the same routine for binary overrides if necessary.  We
+        # assume that the above permission check on the source component is
+        # sufficient.
+        if self.include_binaries:
+            binaries = source_package.getBuiltBinaries()
+            bpn_archtag = [(
+                bpph.binarypackagerelease.binarypackagename,
+                bpph.distroarchseries.architecturetag) for bpph in binaries]
+            ancestry = override_policy.calculateBinaryOverrides(
+                self.target_archive, self.target_distroseries,
+                self.target_pocket, bpn_archtag)
+            if len(ancestry) != len(binaries):
+                # XXX cjwatson bug=1079577: Binary overrides for Ubuntu
+                # should be calculated using the Ubuntu override policy, and
+                # they should be saved in the metadata with an API for
+                # overriding them.
+                approve_new = auto_approve or copy_policy.autoApproveNew(
+                    self.target_archive, self.target_distroseries,
+                    self.target_pocket)
+                if not approve_new:
+                    self._createPackageUpload()
+                    raise SuspendJobException
+
         # The package is not new (it has ancestry) so check the copy
         # policy for existing packages.
         approve_existing = auto_approve or copy_policy.autoApprove(
@@ -643,9 +666,7 @@
             [self.context.id]).any()
         if pu is None:
             source_name = getUtility(ISourcePackageNameSet)[self.package_name]
-            self._checkPolicies(
-                source_name, source_package.sourcepackagerelease.component,
-                self.auto_approve)
+            self._checkPolicies(source_name, source_package, self.auto_approve)
 
         # The package is free to go right in, so just copy it now.
         ancestry = self.target_archive.getPublishedSources(

=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py	2014-05-15 09:06:24 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py	2014-05-30 12:14:46 +0000
@@ -18,6 +18,7 @@
 from zope.security.proxy import removeSecurityProxy
 
 from lp.bugs.interfaces.bugtask import BugTaskStatus
+from lp.buildmaster.enums import BuildStatus
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.model.distroseriesdifferencecomment import (
@@ -1003,6 +1004,40 @@
             [removeSecurityProxy(job).context.id]).one()
         self.assertEqual(PackageUploadStatus.NEW, pu.status)
 
+    def test_copying_new_binaries(self):
+        # A copy involving source and new binaries requires NEW queue approval.
+        target_archive = self.factory.makeArchive(
+            self.distroseries.distribution, purpose=ArchivePurpose.PRIMARY)
+        source_archive = self.factory.makeArchive()
+        old_spph = self.publisher.getPubSource(
+            distroseries=self.distroseries, sourcename="copyme", version="1.0",
+            status=PackagePublishingStatus.PUBLISHED, archive=target_archive)
+        self.publisher.getPubBinaries(
+            binaryname="copyme", pub_source=old_spph,
+            distroseries=self.distroseries,
+            status=PackagePublishingStatus.PUBLISHED)
+        new_spph = self.publisher.getPubSource(
+            distroseries=self.distroseries, sourcename="copyme", version="1.1",
+            status=PackagePublishingStatus.PUBLISHED, archive=source_archive)
+        for build in new_spph.createMissingBuilds():
+            build.updateStatus(BuildStatus.FULLYBUILT)
+            for binaryname in ("copyme", "libcopyme-dev"):
+                bpr = self.publisher.uploadBinaryForBuild(build, binaryname)
+                self.publisher.publishBinaryInArchive(
+                    bpr, new_spph.archive,
+                    status=PackagePublishingStatus.PUBLISHED)
+            pu = self.publisher.addPackageUpload(
+                new_spph.archive, self.distroseries,
+                changes_file_name="copyme_1.1_%s.changes" % build.arch_tag)
+            pu.addBuild(build)
+        job = self.createCopyJobForSPPH(
+            new_spph, source_archive, target_archive, include_binaries=True)
+        self.runJob(job)
+        self.assertEqual(JobStatus.SUSPENDED, job.status)
+        pu = getUtility(IPackageUploadSet).getByPackageCopyJobIDs(
+            [removeSecurityProxy(job).context.id]).one()
+        self.assertEqual(PackageUploadStatus.NEW, pu.status)
+
     def test_copying_to_main_archive_unapproved(self):
         # Uploading to a series that is in a state that precludes auto
         # approval will cause the job to suspend and a packageupload
@@ -1302,6 +1337,10 @@
             distroseries=self.distroseries, sourcename="copyme",
             version="2.8-0", status=PackagePublishingStatus.PUBLISHED,
             component='multiverse', section='web', archive=target_archive)
+        self.publisher.getPubBinaries(
+            binaryname="copyme", pub_source=old_spph,
+            distroseries=self.distroseries,
+            status=PackagePublishingStatus.PUBLISHED)
         old_spr = old_spph.sourcepackagerelease
         diff_file = self.publisher.addMockFile("diff_file", restricted=True)
         package_diff = old_spr.requestDiffTo(target_archive.owner, spr)