Merge lp:~cjwatson/launchpad/redirect-release-uploads into lp:launchpad

[Colin Watson]

If Distribution.redirect_release_uploads is set, redirect RELEASE pocket uploads to PROPOSED, and refuse some copies.

== Summary ==

''Summarise the problem that you're solving.''

Bug 1068071: To make raring more continuously usable, UE wants to land all uploads in raring-proposed and promote them automatically to raring following tests somewhat akin to those used by Debian's testing suite, in what amounts to a continuous integration system. We'll operate the automation, but its usefulness will be significantly enhanced by redirecting uploads automatically from raring to raring-proposed.

== Proposed fix ==

A new Distribution.redirect_release_uploads column is making its way through buildbot/QA as I type this. If it's set, then:

 * Automatically rewrite source uploads from RELEASE to PROPOSED in archiveuploader, adding a note to acceptance mails that this has happened.
 * Forbid copies into the RELEASE pocket, except for queue admins (if nothing else, such copies are used by the automation itself).

== Pre-implementation notes ==

Extensive discussions on #ubuntu-release over the last week or so. I was going to automatically rewrite some copies as well, but William convinced me that this was excessively magical and complicated the API in ways that would be troublesome in future. We'll probably end up SRUing syncpackage instead.

== Implementation details ==

The insecure upload policy seemed a sensible place to do redirects, but it does have the slight problem that setDistroSeriesAndPocket is sometimes called before we have a NascentUpload to add warnings to. I ended up stashing the warning temporarily in policy.redirect_warning, which is a little bit of a kludge.

This causes acceptance mails to have some extra bits (exemplified by the changes to nascentupload-announcements.txt) due to the normal contents of %(SUMMARY)s. I don't think this is a problem.

The copy restriction seemed most naturally done in checkUploadToPocket, although that needs to be guarded by a check_redirect=False parameter because that method is also called for build uploads which must not be redirected. A couple of places were doing their own checks for copies into the RELEASE pocket of PPAs; I couldn't see any reason why these wouldn't also want all the other checks in checkUploadToPocket - indeed, copies into the pockets forbidden there will generally cause publisher badness - other than making sure that they raised the correct exception type, so I rewrote these in terms of that method.

== LOC Rationale ==

+166. This seems cheap at the price for what should be a major improvement in the ongoing stability of Ubuntu. I have about 5600 lines of credit to help absorb this, and still have some more refactoring work to come.

== Tests ==

Probably wants the whole test suite, but in particular:

  bin/test -vvct archiveuploader -t soyuz

== Demo and Q/A ==

Flip the switch on dogfood, upload source to the RELEASE pocket, whatever its current development series is, check that it goes to PROPOSED, check that a non-queue-admin can't copy it into RELEASE, and check that a queue admin can.

== Lint ==

Pre-existing / false positives:

./lib/lp/archiveuploader/tests/nascentupload-announcements.txt
     186: want exceeds 78 characters.
     187: want exceeds 78 characters.
     660: want exceeds 78 characters.
     727: want exceeds 78 characters.
     729: want exceeds 78 characters.
     780: want exceeds 78 characters.
     782: want exceeds 78 characters.
./lib/lp/soyuz/emailtemplates/upload-accepted.txt
      14: Line has trailing whitespace.
./lib/lp/soyuz/scripts/packagecopier.py
      50: E302 expected 2 blank lines, found 1

[William Grant (wgrant)]

Thanks for the fixes.

review: Approve (code)

[William Grant (wgrant)]

Actually, the archiveuploader changes look like they will override PPA and partner uploads too, which is sort of bad.

review: Needs Fixing (code)

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/archiveuploader/nascentupload.py'
--- lib/lp/archiveuploader/nascentupload.py	2012-07-03 08:04:35 +0000
+++ lib/lp/archiveuploader/nascentupload.py	2012-10-25 09:05:26 +0000
@@ -138,6 +138,9 @@
             self.reject(
                 "Unable to find distroseries: %s" % self.changes.suite_name)
 
+        if policy.redirect_warning is not None:
+            self.warn(policy.redirect_warning)
+
         # Make sure the changes file name is well-formed.
         self.run_and_reject_on_error(self.changes.checkFileName)
 

=== modified file 'lib/lp/archiveuploader/tests/nascentupload-announcements.txt'
--- lib/lp/archiveuploader/tests/nascentupload-announcements.txt	2012-03-29 07:56:15 +0000
+++ lib/lp/archiveuploader/tests/nascentupload-announcements.txt	2012-10-25 09:05:26 +0000
@@ -513,6 +513,11 @@
     DEBUG
     DEBUG ==
     DEBUG
+    DEBUG  OK: bar_1.0.orig.tar.gz
+    DEBUG  OK: bar_1.0-4.diff.gz
+    DEBUG  OK: bar_1.0-4.dsc
+    DEBUG      -> Component: universe Section: devel
+    DEBUG
     DEBUG Announcing to [e-mail]
     DEBUG
     DEBUG Thank you for your contribution to Ubuntu Linux.
@@ -728,6 +733,11 @@
     <BLANKLINE>
     =3D=3D
     <BLANKLINE>
+     OK: bar_1.0.orig.tar.gz
+     OK: bar_1.0-10.diff.gz
+     OK: bar_1.0-10.dsc
+         -> Component: universe Section: devel
+    <BLANKLINE>
     Announcing to [e-mail]
     <BLANKLINE>
     Thank you for your contribution to Ubuntu Linux.

=== modified file 'lib/lp/archiveuploader/tests/test_sync_notification.py'
--- lib/lp/archiveuploader/tests/test_sync_notification.py	2012-01-06 06:07:34 +0000
+++ lib/lp/archiveuploader/tests/test_sync_notification.py	2012-10-25 09:05:26 +0000
@@ -38,6 +38,7 @@
         self.distroseries = spph.distroseries
         self.archive = spph.distroseries.main_archive
         self.pocket = spph.pocket
+        self.redirect_warning = None
 
     setDistroSeriesAndPocket = FakeMethod()
     validateUploadType = FakeMethod()

=== modified file 'lib/lp/archiveuploader/tests/test_uploadpolicy.py'
--- lib/lp/archiveuploader/tests/test_uploadpolicy.py	2012-09-13 05:14:05 +0000
+++ lib/lp/archiveuploader/tests/test_uploadpolicy.py	2012-10-25 09:05:26 +0000
@@ -16,6 +16,7 @@
     InsecureUploadPolicy,
     )
 from lp.registry.interfaces.distribution import IDistributionSet
+from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.database.sqlbase import flush_database_updates
 from lp.testing import (
@@ -193,6 +194,34 @@
             NotFoundError, policy.setDistroSeriesAndPocket,
             'nonexistent_security')
 
+    def test_redirect_release_uploads_primary(self):
+        # With the insecure policy, the
+        # Distribution.redirect_release_uploads flag causes uploads to the
+        # RELEASE pocket to be automatically redirected to PROPOSED.
+        ubuntu = getUtility(IDistributionSet)["ubuntu"]
+        with celebrity_logged_in("admin"):
+            ubuntu.redirect_release_uploads = True
+        flush_database_updates()
+        insecure_policy = findPolicyByName("insecure")
+        insecure_policy.setOptions(FakeOptions(distroseries="hoary"))
+        self.assertEqual("hoary", insecure_policy.distroseries.name)
+        self.assertEqual(
+            PackagePublishingPocket.PROPOSED, insecure_policy.pocket)
+
+    def test_redirect_release_uploads_ppa(self):
+        # The Distribution.redirect_release_uploads flag does not affect PPA
+        # uploads.
+        ubuntu = getUtility(IDistributionSet)["ubuntu"]
+        with celebrity_logged_in("admin"):
+            ubuntu.redirect_release_uploads = True
+        flush_database_updates()
+        insecure_policy = findPolicyByName("insecure")
+        insecure_policy.archive = self.factory.makeArchive()
+        insecure_policy.setOptions(FakeOptions(distroseries="hoary"))
+        self.assertEqual("hoary", insecure_policy.distroseries.name)
+        self.assertEqual(
+            PackagePublishingPocket.RELEASE, insecure_policy.pocket)
+
     def setHoaryStatus(self, status):
         ubuntu = getUtility(IDistributionSet)["ubuntu"]
         with celebrity_logged_in("admin"):

=== modified file 'lib/lp/archiveuploader/tests/test_uploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_uploadprocessor.py	2012-09-27 02:53:00 +0000
+++ lib/lp/archiveuploader/tests/test_uploadprocessor.py	2012-10-25 09:05:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Functional tests for uploadprocessor.py."""
@@ -1963,6 +1963,38 @@
         self.assertEqual("optional", ddeb.priority_name)
         self.assertEqual(deb.priority_name, ddeb.priority_name)
 
+    def test_redirect_release_uploads(self):
+        # Setting the Distribution.redirect_release_uploads flag causes
+        # release pocket uploads to be redirected to proposed.
+        uploadprocessor = self.setupBreezyAndGetUploadProcessor(
+            policy="insecure")
+        self.switchToAdmin()
+        self.ubuntu.redirect_release_uploads = True
+        # Don't bother with announcements.
+        self.breezy.changeslist = None
+        self.switchToUploader()
+        upload_dir = self.queueUpload("bar_1.0-1")
+        self.processUpload(uploadprocessor, upload_dir)
+        self.assertEmail(
+            contents=["Redirecting ubuntu breezy to ubuntu breezy-proposed."],
+            recipients=[])
+        [queue_item] = self.breezy.getPackageUploads(
+            status=PackageUploadStatus.NEW, name=u"bar",
+            version=u"1.0-1", exact_match=True)
+        self.assertEqual(PackagePublishingPocket.PROPOSED, queue_item.pocket)
+
+        queue_item.acceptFromQueue()
+        pop_notifications()
+        upload_dir = self.queueUpload("bar_1.0-2")
+        self.processUpload(uploadprocessor, upload_dir)
+        self.assertEmail(
+            contents=["Redirecting ubuntu breezy to ubuntu breezy-proposed."],
+            recipients=[])
+        [queue_item] = self.breezy.getPackageUploads(
+            status=PackageUploadStatus.DONE, name=u"bar",
+            version=u"1.0-2", exact_match=True)
+        self.assertEqual(PackagePublishingPocket.PROPOSED, queue_item.pocket)
+
 
 class TestUploadHandler(TestUploadProcessorBase):
 

=== modified file 'lib/lp/archiveuploader/uploadpolicy.py'
--- lib/lp/archiveuploader/uploadpolicy.py	2012-09-17 05:25:38 +0000
+++ lib/lp/archiveuploader/uploadpolicy.py	2012-10-25 09:05:26 +0000
@@ -32,6 +32,7 @@
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
+from lp.soyuz.enums import ArchivePurpose
 
 # Number of seconds in an hour (used later)
 HOURS = 3600
@@ -76,6 +77,7 @@
     name = 'abstract'
     options = None
     accepted_type = None  # Must be defined in subclasses.
+    redirect_warning = None
 
     def __init__(self):
         """Prepare a policy..."""
@@ -201,6 +203,20 @@
     name = 'insecure'
     accepted_type = ArchiveUploadType.SOURCE_ONLY
 
+    def setDistroSeriesAndPocket(self, dr_name):
+        """Set the distroseries and pocket from the provided name.
+
+        The insecure policy redirects uploads to a different pocket if
+        Distribution.redirect_release_uploads is set.
+        """
+        super(InsecureUploadPolicy, self).setDistroSeriesAndPocket(dr_name)
+        if (self.archive.purpose == ArchivePurpose.PRIMARY and
+            self.distro.redirect_release_uploads and
+            self.pocket == PackagePublishingPocket.RELEASE):
+            self.pocket = PackagePublishingPocket.PROPOSED
+            self.redirect_warning = "Redirecting %s to %s-proposed." % (
+                self.distroseries, self.distroseries)
+
     def rejectPPAUploads(self, upload):
         """Insecure policy allows PPA upload."""
         return False

=== modified file 'lib/lp/archiveuploader/uploadprocessor.py'
--- lib/lp/archiveuploader/uploadprocessor.py	2012-06-29 08:40:05 +0000
+++ lib/lp/archiveuploader/uploadprocessor.py	2012-10-25 09:05:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Code for 'processing' 'uploads'. Also see nascentupload.py.

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2012-10-22 10:09:48 +0000
+++ lib/lp/registry/configure.zcml	2012-10-25 09:05:26 +0000
@@ -1623,6 +1623,7 @@
                 official_malone
                 owner
                 package_derivatives_email
+                redirect_release_uploads
                 summary
                 title"/>
         <require

=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py	2012-10-05 05:34:13 +0000
+++ lib/lp/registry/interfaces/distribution.py	2012-10-25 09:05:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interfaces including and related to IDistribution."""
@@ -361,6 +361,11 @@
         description=_("True if this distribution has sources published."),
         readonly=True, required=False)
 
+    redirect_release_uploads = exported(Bool(
+        title=_("Redirect release pocket uploads"),
+        description=_("Redirect release pocket uploads to proposed pocket"),
+        readonly=False, required=True))
+
     def getArchiveIDList(archive=None):
         """Return a list of archive IDs suitable for sqlvalues() or quote().
 

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2012-09-27 18:04:35 +0000
+++ lib/lp/registry/model/distribution.py	2012-10-25 09:05:26 +0000
@@ -257,6 +257,7 @@
         schema=TranslationPermission, default=TranslationPermission.OPEN)
     active = True
     package_derivatives_email = StringCol(notNull=False, default=None)
+    redirect_release_uploads = BoolCol(notNull=True, default=False)
 
     def __repr__(self):
         displayname = self.displayname.encode('ASCII', 'backslashreplace')

=== modified file 'lib/lp/soyuz/doc/archive.txt'
--- lib/lp/soyuz/doc/archive.txt	2012-09-27 02:53:00 +0000
+++ lib/lp/soyuz/doc/archive.txt	2012-10-25 09:05:26 +0000
@@ -2175,7 +2175,7 @@
     ...     "package3", "1.2", cprov.archive, "updates", person=mark)
     Traceback (most recent call last):
     ...
-    CannotCopy: Destination pocket must be 'release' for a PPA.
+    CannotCopy: PPA uploads must be for the RELEASE pocket.
 
 syncSource() will always use only the latest publication of the
 specific source, ignoring the previous ones. Multiple publications can

=== modified file 'lib/lp/soyuz/doc/soyuz-set-of-uploads.txt'
--- lib/lp/soyuz/doc/soyuz-set-of-uploads.txt	2012-10-03 08:20:24 +0000
+++ lib/lp/soyuz/doc/soyuz-set-of-uploads.txt	2012-10-25 09:05:26 +0000
@@ -74,7 +74,6 @@
 Having set up that infrastructure we need to prepare a breezy distroseries
 for the ubuntutest distribution.
 
-    >>> from lp.registry.interfaces.pocket import PackagePublishingPocket
     >>> from lp.registry.model.distribution import Distribution
     >>> from lp.soyuz.enums import PackageUploadStatus
     >>> from lp.soyuz.scripts.initialize_distroseries import (
@@ -101,7 +100,9 @@
 
 Add disk content for file inherited from ubuntu/breezy-autotest:
 
-    >>> from lp.services.librarianserver.testing.server import fillLibrarianFile
+    >>> from lp.services.librarianserver.testing.server import (
+    ...     fillLibrarianFile,
+    ...     )
     >>> fillLibrarianFile(54)
 
 Now that the infrastructure is ready, we prepare a set of useful methods.
@@ -345,6 +346,11 @@
     <BLANKLINE>
     =3D=3D
     <BLANKLINE>
+     OK: bar_1.0.orig.tar.gz
+     OK: bar_1.0-4.diff.gz
+     OK: bar_1.0-4.dsc
+         -> Component: universe Section: devel
+    <BLANKLINE>
     Announcing to [e-mail]
     <BLANKLINE>
     Thank you for your contribution to Ubuntu Test.

=== modified file 'lib/lp/soyuz/emailtemplates/upload-accepted.txt'
--- lib/lp/soyuz/emailtemplates/upload-accepted.txt	2011-12-18 23:30:56 +0000
+++ lib/lp/soyuz/emailtemplates/upload-accepted.txt	2012-10-25 09:05:26 +0000
@@ -5,6 +5,8 @@
 
 ==
 
+%(SUMMARY)s
+
 %(ANNOUNCE)s
 
 Thank you for your contribution to %(DISTRO)s.

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2012-10-23 12:36:50 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2012-10-25 09:05:26 +0000
@@ -42,6 +42,7 @@
     'NoTokensForTeams',
     'PocketNotFound',
     'PriorityNotFound',
+    'RedirectedPocket',
     'SectionNotFound',
     'VersionRequiresName',
     'default_name_by_purpose',
@@ -206,6 +207,19 @@
             "'%s' state." % (pocket.name, distroseries.status.name))
 
 
+@error_status(httplib.FORBIDDEN)
+class RedirectedPocket(Exception):
+    """Returned for a pocket that would normally be redirected to another.
+
+    This is used in contexts (e.g. copies) where actually doing the
+    redirection would be Too Much Magic."""
+
+    def __init__(self, distroseries, pocket, preferred):
+        Exception.__init__(self,
+            "Not permitted to upload directly to %s; try %s instead." %
+            (distroseries.getSuite(pocket), distroseries.getSuite(preferred)))
+
+
 class CannotUploadToPPA(CannotUploadToArchive):
     """Raised when a person cannot upload to a PPA."""
 
@@ -598,11 +612,13 @@
         Return True if the upload is allowed and False if denied.
         """
 
-    def checkUploadToPocket(distroseries, pocket):
+    def checkUploadToPocket(distroseries, pocket, person=None):
         """Check if an upload to a particular archive and pocket is possible.
 
         :param distroseries: A `IDistroSeries`
         :param pocket: A `PackagePublishingPocket`
+        :param person: Check for redirected pockets if this person is not a
+            queue admin.
         :return: Reason why uploading is not possible or None
         """
 

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2012-10-24 09:25:16 +0000
+++ lib/lp/soyuz/model/archive.py	2012-10-25 09:05:26 +0000
@@ -148,6 +148,7 @@
     NoSuchPPA,
     NoTokensForTeams,
     PocketNotFound,
+    RedirectedPocket,
     validate_external_dependencies,
     VersionRequiresName,
     )
@@ -1266,7 +1267,7 @@
         # Allow anything else.
         return True
 
-    def checkUploadToPocket(self, distroseries, pocket):
+    def checkUploadToPocket(self, distroseries, pocket, person=None):
         """See `IArchive`."""
         if self.is_partner:
             if pocket not in (
@@ -1282,6 +1283,14 @@
             # existing builds after a series is released.
             return
         else:
+            if (self.purpose == ArchivePurpose.PRIMARY and
+                person is not None and
+                not self.canAdministerQueue(
+                    person, pocket=pocket, distroseries=distroseries) and
+                pocket == PackagePublishingPocket.RELEASE and
+                self.distribution.redirect_release_uploads):
+                return RedirectedPocket(
+                    distroseries, pocket, PackagePublishingPocket.PROPOSED)
             # Uploads to the partner archive are allowed in any distroseries
             # state.
             # XXX julian 2005-05-29 bug=117557:
@@ -1799,14 +1808,11 @@
         from lp.soyuz.scripts.packagecopier import do_copy
 
         pocket = self._text_to_pocket(to_pocket)
-        # Fail immediately if the destination pocket is not Release and
-        # this archive is a PPA.
-        if self.is_ppa and pocket != PackagePublishingPocket.RELEASE:
-            raise CannotCopy(
-                "Destination pocket must be 'release' for a PPA.")
-
-        # Now convert the to_series string to a real distroseries.
         series = self._text_to_series(to_series)
+        reason = self.checkUploadToPocket(series, pocket, person=person)
+        if reason:
+            # Wrap any forbidden-pocket error in CannotCopy.
+            raise CannotCopy(unicode(reason))
 
         # Perform the copy, may raise CannotCopy. Don't do any further
         # permission checking: this method is protected by

=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py	2012-10-23 12:36:50 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py	2012-10-25 09:05:26 +0000
@@ -611,10 +611,12 @@
         :raise CannotCopy: If the copy fails for a reason that the user
             can deal with.
         """
-        if self.target_archive.is_ppa:
-            if self.target_pocket != PackagePublishingPocket.RELEASE:
-                raise CannotCopy(
-                    "Destination pocket must be 'release' for a PPA.")
+        reason = self.target_archive.checkUploadToPocket(
+            self.target_distroseries, self.target_pocket,
+            person=self.requester)
+        if reason:
+            # Wrap any forbidden-pocket error in CannotCopy.
+            raise CannotCopy(unicode(reason))
 
         source_package = self.findSourcePublication()
 

=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py	2012-09-28 06:15:58 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py	2012-10-25 09:05:26 +0000
@@ -205,7 +205,8 @@
             destination_component = None
 
         # Is the destination pocket open at all?
-        reason = archive.checkUploadToPocket(dest_series, pocket)
+        reason = archive.checkUploadToPocket(
+            dest_series, pocket, person=person)
         if reason is not None:
             raise CannotCopy(reason)
 

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2012-10-24 09:25:16 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2012-10-25 09:05:26 +0000
@@ -66,6 +66,7 @@
     NoRightsForArchive,
     NoRightsForComponent,
     NoSuchPPA,
+    RedirectedPocket,
     VersionRequiresName,
     )
 from lp.soyuz.interfaces.archivearch import IArchiveArchSet
@@ -713,6 +714,29 @@
             archive.checkUploadToPocket(
                 distroseries, PackagePublishingPocket.RELEASE))
 
+    def test_checkUploadToPocket_handles_redirects(self):
+        # Uploading to the release pocket is disallowed if
+        # Distribution.redirect_release_uploads is set.
+        archive, distroseries = self.makeArchiveAndActiveDistroSeries(
+            purpose=ArchivePurpose.PRIMARY)
+        with person_logged_in(archive.distribution.owner):
+            archive.distribution.redirect_release_uploads = True
+        person = self.factory.makePerson()
+        self.assertIsInstance(
+            archive.checkUploadToPocket(
+                distroseries, PackagePublishingPocket.RELEASE, person=person),
+            RedirectedPocket)
+        # The proposed pocket is unaffected.
+        self.assertIsNone(
+            archive.checkUploadToPocket(
+                distroseries, PackagePublishingPocket.PROPOSED, person=person))
+        # Queue admins bypass this check.
+        with person_logged_in(archive.distribution.owner):
+            archive.newQueueAdmin(person, "main")
+        self.assertIsNone(
+            archive.checkUploadToPocket(
+                distroseries, PackagePublishingPocket.RELEASE, person=person))
+
     def test_checkUpload_package_permission(self):
         archive, distroseries = self.makeArchiveAndActiveDistroSeries(
             purpose=ArchivePurpose.PRIMARY)

=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py	2012-10-23 12:36:50 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py	2012-10-25 09:05:26 +0000
@@ -416,8 +416,7 @@
         self.assertEqual(JobStatus.FAILED, job.status)
 
         self.assertEqual(
-            "Destination pocket must be 'release' for a PPA.",
-            job.error_message)
+            "PPA uploads must be for the RELEASE pocket.", job.error_message)
 
     def assertOopsRecorded(self, job):
         self.assertEqual(JobStatus.FAILED, job.status)
@@ -451,6 +450,35 @@
         transaction.abort()
         self.assertOopsRecorded(job)
 
+    def test_target_primary_redirects(self):
+        # For primary archives with redirect_release_uploads set, ordinary
+        # uploaders may not copy directly into the release pocket.
+        job = create_proper_job(self.factory)
+        job.target_archive.distribution.redirect_release_uploads = True
+        # CannotCopy exceptions when copying into a primary archive are
+        # swallowed, but reportFailure is still called.
+        naked_job = removeSecurityProxy(job)
+        naked_job.reportFailure = FakeMethod()
+        transaction.commit()
+        self.runJob(job)
+        self.assertEqual(1, naked_job.reportFailure.call_count)
+
+    def test_target_primary_queue_admin_bypasses_redirect(self):
+        # For primary archives with redirect_release_uploads set, queue
+        # admins may copy directly into the release pocket anyway.
+        job = create_proper_job(self.factory)
+        job.target_archive.distribution.redirect_release_uploads = True
+        with person_logged_in(job.target_archive.owner):
+            job.target_archive.newPocketQueueAdmin(
+                job.requester, PackagePublishingPocket.RELEASE)
+        # CannotCopy exceptions when copying into a primary archive are
+        # swallowed, but reportFailure is still called.
+        naked_job = removeSecurityProxy(job)
+        naked_job.reportFailure = FakeMethod()
+        transaction.commit()
+        self.runJob(job)
+        self.assertEqual(0, naked_job.reportFailure.call_count)
+
     def test_run(self):
         # A proper test run synchronizes packages.