Merge lp:~salgado/launchpad/vostok-upload-policy into lp:launchpad

[Guilherme Salgado]

This branch moves the code related to validating the upload type from nascentupload to the upload policy, as a separate method. This makes it easy to unit test that code as well as adding more flexibility for subclasses that might want to validate differently.

It also replaces the three can_upload_* variables with a single enum variable.

[Jeroen T. Vermeulen (jtv)]

Beautiful. A few notes, of course:

Lines 199 and 319 of the diff have multiple imports that don't conform to the new import formatting rules:

from foobar import (
    Bar,
    Foo,
    )

In line 379 of the diff, the "else" and the assertion throw the symmetry of the code slightly out of whack. Just an idea, but maybe you can move it out into a separate "else" clause?

    elif upload.binaryful:
        # [...]
    else:
        raise AssertionError("...")

The list of hand-wrapped text lines at line 382 of the diff puzzles me. Why not a single dedent("""triple-quoted multi-line string""")?

Finally, I think I see either an indentation problem or something involving tabs around line 390. Did you run "make lint" over the current form of this branch?

But that's all small fry. The branch is definitely an improvement.

review: Approve

[Guilherme Salgado (salgado)]

On Wed, 2010-08-25 at 05:27 +0000, Jeroen T. Vermeulen wrote:
> Review: Approve
> Beautiful. A few notes, of course:
>
> Lines 199 and 319 of the diff have multiple imports that don't conform
> to the new import formatting rules:
>
> from foobar import (
> Bar,
> Foo,
> )

Oops, fixed.

>
> In line 379 of the diff, the "else" and the assertion throw the
> symmetry of the code slightly out of whack. Just an idea, but maybe
> you can move it out into a separate "else" clause?
>
> elif upload.binaryful:
> # [...]
> else:
> raise AssertionError("...")

That wfm.

>
> The list of hand-wrapped text lines at line 382 of the diff puzzles
> me. Why not a single dedent("""triple-quoted multi-line string""")?

That's code I just moved around, but I've changed it as you suggest.

>
> Finally, I think I see either an indentation problem or something

Broken indentation indeed; didn't notice when moving the code around.

> involving tabs around line 390. Did you run "make lint" over the
> current form of this branch?

I never did because 'bzr send' would do that for me, but now that I'm
using bzr lp-submit...

>
> But that's all small fry. The branch is definitely an improvement.

Thanks for the review!

If I can make a suggestion as well, it'd be much better if you could
include chunks of the diff in your review[1], for context, instead of
using the line numbers of the diff. That way we don't have to go back
and forth between your comments and the diff (in my case between my mail
client and the merge-proposal's page), but more importantly because the
diff is a moving target, so there's a big chance that the line numbers
have changed when I get to reply to your review. :)

[1] I do that by replying to the m-p email on my mail client, so the
diff is quoted and I just insert my comments at the appropriate places.

[Michael Nelson (michael.nelson)]

r=me for -r11430..11432.

=== modified file 'lib/lp/archiveuploader/nascentupload.py'
--- lib/lp/archiveuploader/nascentupload.py	2010-08-24 08:43:51 +0000
+++ lib/lp/archiveuploader/nascentupload.py	2010-08-25 17:14:48 +0000
@@ -146,10 +146,11 @@
         UploadError will be raised and sent up to the caller. If this happens
         the caller should call the reject method and process a rejection.
         """
+        policy = self.policy
         self.logger.debug("Beginning processing.")
 
         try:
-            self.policy.setDistroSeriesAndPocket(self.changes.suite_name)
+            policy.setDistroSeriesAndPocket(self.changes.suite_name)
         except NotFoundError:
             self.reject(
                 "Unable to find distroseries: %s" % self.changes.suite_name)
@@ -185,36 +186,12 @@
             isinstance(self.changes.files[0], CustomUploadFile)):
             self.logger.debug("Single Custom Upload detected.")
         else:
-            if self.sourceful and not self.policy.can_upload_source:
-                self.reject("Upload is sourceful, but policy refuses "
-                            "sourceful uploads.")
-
-            elif self.binaryful and not self.policy.can_upload_binaries:
-                messages = [
-                    "Upload rejected because it contains binary packages.",
-                    "Ensure you are using `debuild -S`, or an equivalent",
-                    "command, to generate only the source package before",
-                    "re-uploading.",
-                    ]
-                if self.is_ppa:
-                    messages.append(
-                    "See https://help.launchpad.net/Packaging/PPA for more "
-                    "information.")
-                self.reject(" ".join(messages))
-
-            elif (self.sourceful and self.binaryful and
-                  not self.policy.can_upload_mixed):
-                self.reject("Upload is source/binary but policy refuses "
-                            "mixed uploads.")
-
-            elif self.sourceful and not self.changes.dsc:
+            policy.validateUploadType(self)
+
+            if self.sourceful and not self.changes.dsc:
                 self.reject(
                     "Unable to find the DSC file in the source upload.")
 
-            else:
-                # Upload content are consistent with the current policy.
-                pass
-
             # Apply the overrides from the database. This needs to be done
             # before doing component verifications because the component
             # actually comes from overrides for packages that are not NEW.
@@ -227,7 +204,7 @@
         self.verify_acl()
 
         # Perform policy checks.
-        self.policy.checkUpload(self)
+        policy.checkUpload(self)
 
         # That's all folks.
         self.logger.debug("Finished checking upload.")
@@ -996,8 +973,6 @@
                     # so late in the game is that in the
                     # mixed-upload case we only have a
                     # sourcepackagerelease to verify here!
-                    assert self.policy.can_upload_mixed, (
-                        "Current policy does not allow mixed uploads.")
                     assert sourcepackagerelease, (
                         "No sourcepackagerelease was found.")
                     binary_package_file.verifySourcePackageRelease(

=== modified file 'lib/lp/archiveuploader/tests/__init__.py'
--- lib/lp/archiveuploader/tests/__init__.py	2010-08-20 20:31:18 +0000
+++ lib/lp/archiveuploader/tests/__init__.py	2010-08-25 17:14:48 +0000
@@ -117,6 +117,10 @@
         # We require the changes to be signed but not the dsc
         self.unsigned_dsc_ok = True
 
+    def validateUploadType(self, upload):
+        """We accept uploads of any type."""
+        pass
+
     def policySpecificChecks(self, upload):
         """Nothing, let it go."""
         pass

=== modified file 'lib/lp/archiveuploader/tests/nascentupload-announcements.txt'
--- lib/lp/archiveuploader/tests/nascentupload-announcements.txt	2010-08-25 17:14:46 +0000
+++ lib/lp/archiveuploader/tests/nascentupload-announcements.txt	2010-08-25 17:14:48 +0000
@@ -297,9 +297,10 @@
 NEW binary upload to RELEASE pocket via 'sync' policy (we need to
 override sync policy to allow binary uploads):
 
+  >>> from lp.archiveuploader.uploadpolicy import ArchiveUploadType
   >>> modified_sync_policy = getPolicy(
   ...     name='sync', distro='ubuntu', distroseries='hoary')
-  >>> modified_sync_policy.can_upload_binaries = True
+  >>> modified_sync_policy.accepted_type = ArchiveUploadType.BINARY_ONLY
 
   >>> bar_src = NascentUpload.from_changesfile_path(
   ...     datadir('suite/bar_1.0-1_binary/bar_1.0-1_i386.changes'),

=== modified file 'lib/lp/archiveuploader/tests/nascentupload.txt'
--- lib/lp/archiveuploader/tests/nascentupload.txt	2010-08-20 12:11:30 +0000
+++ lib/lp/archiveuploader/tests/nascentupload.txt	2010-08-25 17:14:48 +0000
@@ -499,9 +499,10 @@
 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.can_upload_binaries = True
+  >>> 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 file 'lib/lp/archiveuploader/tests/nascentuploadfile.txt'
--- lib/lp/archiveuploader/tests/nascentuploadfile.txt	2010-08-20 12:11:30 +0000
+++ lib/lp/archiveuploader/tests/nascentuploadfile.txt	2010-08-25 17:14:48 +0000
@@ -531,9 +531,10 @@
 The timestamp of the files in the .deb are tested against the policy for being 
 too new:
 
+   >>> from lp.archiveuploader.uploadpolicy import ArchiveUploadType
    >>> old_only_policy = getPolicy(
    ...     name='insecure', distro='ubuntu', distroseries='hoary')
-   >>> old_only_policy.can_upload_binaries = True
+   >>> old_only_policy.accepted_type = ArchiveUploadType.BINARY_ONLY
    >>> old_only_policy.future_time_grace = -5 * 365 * 24 * 60 * 60
 
    >>> ed_binary_deb = DebBinaryUploadFile(ed_deb_path,
@@ -549,7 +550,7 @@
 
    >>> new_only_policy = getPolicy(
    ...     name='insecure', distro='ubuntu', distroseries='hoary')
-   >>> new_only_policy.can_upload_binaries = True
+   >>> new_only_policy.accepted_type = ArchiveUploadType.BINARY_ONLY
    >>> new_only_policy.earliest_year = 2010
    >>> ed_binary_deb = DebBinaryUploadFile(ed_deb_path,
    ...                     'e31eeb0b6b3b87e1ea79378df864ffff',

=== modified file 'lib/lp/archiveuploader/tests/test_buildduploads.py'
--- lib/lp/archiveuploader/tests/test_buildduploads.py	2010-08-20 20:31:18 +0000
+++ lib/lp/archiveuploader/tests/test_buildduploads.py	2010-08-25 17:14:48 +0000
@@ -5,13 +5,139 @@
 
 __metaclass__ = type
 
+import os
+
+from zope.component import getUtility
+
 from canonical.database.constants import UTC_NOW
 from canonical.launchpad.ftests import import_public_test_keys
-from canonical.launchpad.interfaces import PackagePublishingStatus
-from lp.archiveuploader.tests.test_securityuploads import (
-    TestStagedBinaryUploadBase,
-    )
+from canonical.launchpad.interfaces import (
+    PackagePublishingStatus,
+    PackageUploadStatus,
+    )
+
+from lp.archiveuploader.tests.test_uploadprocessor import (
+    TestUploadProcessorBase,
+    )
+from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
+
+
+class TestStagedBinaryUploadBase(TestUploadProcessorBase):
+    name = 'baz'
+    version = '1.0-1'
+    distribution_name = None
+    distroseries_name = None
+    pocket = None
+    policy = 'buildd'
+    no_mails = True
+
+    @property
+    def distribution(self):
+        return getUtility(IDistributionSet)[self.distribution_name]
+
+    @property
+    def distroseries(self):
+        return self.distribution[self.distroseries_name]
+
+    @property
+    def package_name(self):
+        return "%s_%s" % (self.name, self.version)
+
+    @property
+    def source_dir(self):
+        return self.package_name
+
+    @property
+    def source_changesfile(self):
+        return "%s_source.changes" % self.package_name
+
+    @property
+    def binary_dir(self):
+        return "%s_binary" % self.package_name
+
+    def getBinaryChangesfileFor(self, archtag):
+        return "%s_%s.changes" % (self.package_name, archtag)
+
+    def setUp(self):
+        """Setup environment for staged binaries upload via security policy.
+
+        1. Setup queue directory and other basic attributes
+        2. Override policy options to get security policy and not send emails
+        3. Setup a common UploadProcessor with the overridden options
+        4. Store number of build present before issuing any upload
+        5. Upload the source package via security policy
+        6. Clean log messages.
+        7. Commit transaction, so the upload source can be seen.
+        """
+        super(TestStagedBinaryUploadBase, self).setUp()
+        self.options.context = self.policy
+        self.options.nomails = self.no_mails
+        # Set up the uploadprocessor with appropriate options and logger
+        self.uploadprocessor = self.getUploadProcessor(self.layer.txn)
+        self.builds_before_upload = BinaryPackageBuild.select().count()
+        self.source_queue = None
+        self._uploadSource()
+        self.log.lines = []
+        self.layer.txn.commit()
+
+    def assertBuildsCreated(self, amount):
+        """Assert that a given 'amount' of build records was created."""
+        builds_count = BinaryPackageBuild.select().count()
+        self.assertEqual(
+            self.builds_before_upload + amount, builds_count)
+
+    def _prepareUpload(self, upload_dir):
+        """Place a copy of the upload directory into incoming queue."""
+        os.system("cp -a %s %s" %
+            (os.path.join(self.test_files_dir, upload_dir),
+             os.path.join(self.queue_folder, "incoming")))
+
+    def _uploadSource(self):
+        """Upload and Accept (if necessary) the base source."""
+        self._prepareUpload(self.source_dir)
+        self.uploadprocessor.processChangesFile(
+            os.path.join(self.queue_folder, "incoming", self.source_dir),
+            self.source_changesfile)
+        queue_item = self.uploadprocessor.last_processed_upload.queue_root
+        self.assertTrue(
+            queue_item is not None,
+            "Source Upload Failed\nGot: %s" % "\n".join(self.log.lines))
+        acceptable_statuses = [
+            PackageUploadStatus.NEW,
+            PackageUploadStatus.UNAPPROVED,
+            ]
+        if queue_item.status in acceptable_statuses:
+            queue_item.setAccepted()
+        # Store source queue item for future use.
+        self.source_queue = queue_item
+
+    def _uploadBinary(self, archtag):
+        """Upload the base binary.
+
+        Ensure it got processed and has a respective queue record.
+        Return the IBuild attached to upload.
+        """
+        self._prepareUpload(self.binary_dir)
+        self.uploadprocessor.processChangesFile(
+            os.path.join(self.queue_folder, "incoming", self.binary_dir),
+            self.getBinaryChangesfileFor(archtag))
+        queue_item = self.uploadprocessor.last_processed_upload.queue_root
+        self.assertTrue(
+            queue_item is not None,
+            "Binary Upload Failed\nGot: %s" % "\n".join(self.log.lines))
+        self.assertEqual(queue_item.builds.count(), 1)
+        return queue_item.builds[0].build
+
+    def _createBuild(self, archtag):
+        """Create a build record attached to the base source."""
+        spr = self.source_queue.sources[0].sourcepackagerelease
+        build = spr.createBuild(
+            distro_arch_series=self.distroseries[archtag],
+            pocket=self.pocket, archive=self.distroseries.main_archive)
+        self.layer.txn.commit()
+        return build
 
 
 class TestBuilddUploads(TestStagedBinaryUploadBase):

=== modified file 'lib/lp/archiveuploader/tests/test_nascentupload_documentation.py'
--- lib/lp/archiveuploader/tests/test_nascentupload_documentation.py	2010-08-23 13:39:29 +0000
+++ lib/lp/archiveuploader/tests/test_nascentupload_documentation.py	2010-08-25 17:14:48 +0000
@@ -30,6 +30,7 @@
     getPolicy,
     mock_logger_quiet,
     )
+from lp.archiveuploader.uploadpolicy import ArchiveUploadType
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.soyuz.interfaces.component import IComponentSet
 
@@ -52,7 +53,7 @@
 def getUploadForBinary(upload_path):
     """Return a NascentUpload object for binaries."""
     policy = getPolicy(name='sync', distro='ubuntu', distroseries='hoary')
-    policy.can_upload_binaries = True
+    policy.accepted_type = ArchiveUploadType.BINARY_ONLY
     return NascentUpload.from_changesfile_path(
         datadir(upload_path), policy, mock_logger_quiet)
 

=== modified file 'lib/lp/archiveuploader/tests/test_ppauploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_ppauploadprocessor.py	2010-08-20 20:31:18 +0000
+++ lib/lp/archiveuploader/tests/test_ppauploadprocessor.py	2010-08-25 17:14:48 +0000
@@ -681,12 +681,9 @@
             "bar_1.0-1-mixed", "~name16/ubuntu")
         self.processUpload(self.uploadprocessor, upload_dir)
 
-        self.assertEqual(
-            self.uploadprocessor.last_processed_upload.rejection_message,
-            'Upload rejected because it contains binary packages. Ensure '
-            'you are using `debuild -S`, or an equivalent command, to '
-            'generate only the source package before re-uploading. See '
-            'https://help.launchpad.net/Packaging/PPA for more information.')
+        self.assertIn(
+            'Source/binary (i.e. mixed) uploads are not allowed.',
+            self.uploadprocessor.last_processed_upload.rejection_message)
 
     def testPGPSignatureNotPreserved(self):
         """PGP signatures should be removed from PPA .changes files.

=== added file 'lib/lp/archiveuploader/tests/test_uploadpolicy.py'
--- lib/lp/archiveuploader/tests/test_uploadpolicy.py	1970-01-01 00:00:00 +0000
+++ lib/lp/archiveuploader/tests/test_uploadpolicy.py	2010-08-25 17:14:48 +0000
@@ -0,0 +1,127 @@
+#!/usr/bin/python
+#
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+
+from lp.app.errors import NotFoundError
+from lp.archiveuploader.uploadpolicy import (
+    AbstractUploadPolicy,
+    ArchiveUploadType,
+    )
+from lp.testing import TestCase, TestCaseWithFactory
+
+
+class TestUploadPolicy_validateUploadType(TestCase):
+    """Test what kind (sourceful/binaryful/mixed) of uploads are accepted."""
+
+    def test_sourceful_accepted(self):
+        policy = make_policy(accepted_type=ArchiveUploadType.SOURCE_ONLY)
+        upload = make_fake_upload(sourceful=True)
+
+        policy.validateUploadType(upload)
+
+        self.assertEquals([], upload.rejections)
+
+    def test_binaryful_accepted(self):
+        policy = make_policy(accepted_type=ArchiveUploadType.BINARY_ONLY)
+        upload = make_fake_upload(binaryful=True)
+
+        policy.validateUploadType(upload)
+
+        self.assertEquals([], upload.rejections)
+
+    def test_mixed_accepted(self):
+        policy = make_policy(accepted_type=ArchiveUploadType.MIXED_ONLY)
+        upload = make_fake_upload(sourceful=True, binaryful=True)
+
+        policy.validateUploadType(upload)
+
+        self.assertEquals([], upload.rejections)
+
+    def test_sourceful_not_accepted(self):
+        policy = make_policy(accepted_type=ArchiveUploadType.BINARY_ONLY)
+        upload = make_fake_upload(sourceful=True)
+
+        policy.validateUploadType(upload)
+
+        self.assertIn(
+            'Sourceful uploads are not accepted by this policy.',
+            upload.rejections)
+
+    def test_binaryful_not_accepted(self):
+        policy = make_policy(accepted_type=ArchiveUploadType.SOURCE_ONLY)
+        upload = make_fake_upload(binaryful=True)
+
+        policy.validateUploadType(upload)
+
+        self.assertTrue(len(upload.rejections) > 0)
+        self.assertIn(
+            'Upload rejected because it contains binary packages.',
+            upload.rejections[0])
+
+    def test_mixed_not_accepted(self):
+        policy = make_policy(accepted_type=ArchiveUploadType.SOURCE_ONLY)
+        upload = make_fake_upload(sourceful=True, binaryful=True)
+
+        policy.validateUploadType(upload)
+
+        self.assertIn(
+            'Source/binary (i.e. mixed) uploads are not allowed.',
+            upload.rejections)
+
+    def test_sourceful_when_only_mixed_accepted(self):
+        policy = make_policy(accepted_type=ArchiveUploadType.MIXED_ONLY)
+        upload = make_fake_upload(sourceful=True, binaryful=False)
+
+        policy.validateUploadType(upload)
+
+        self.assertIn(
+            'Sourceful uploads are not accepted by this policy.',
+            upload.rejections)
+
+    def test_binaryful_when_only_mixed_accepted(self):
+        policy = make_policy(accepted_type=ArchiveUploadType.MIXED_ONLY)
+        upload = make_fake_upload(sourceful=False, binaryful=True)
+
+        policy.validateUploadType(upload)
+
+        self.assertTrue(len(upload.rejections) > 0)
+        self.assertIn(
+            'Upload rejected because it contains binary packages.',
+            upload.rejections[0])
+
+
+class TestUploadPolicy(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_setDistroSeriesAndPocket_distro_not_found(self):
+        policy = AbstractUploadPolicy()
+        policy.distro = self.factory.makeDistribution()
+        self.assertRaises(
+            NotFoundError, policy.setDistroSeriesAndPocket,
+            'nonexistent_security')
+
+
+class FakeNascentUpload:
+
+    def __init__(self, sourceful, binaryful):
+        self.sourceful = sourceful
+        self.binaryful = binaryful
+        self.is_ppa = False
+        self.rejections = []
+
+    def reject(self, msg):
+        self.rejections.append(msg)
+
+
+def make_fake_upload(sourceful=False, binaryful=False):
+    return FakeNascentUpload(sourceful, binaryful)
+
+
+def make_policy(accepted_type):
+    policy = AbstractUploadPolicy()
+    policy.accepted_type = accepted_type
+    return policy

=== modified file 'lib/lp/archiveuploader/uploadpolicy.py'
--- lib/lp/archiveuploader/uploadpolicy.py	2010-08-25 17:14:46 +0000
+++ lib/lp/archiveuploader/uploadpolicy.py	2010-08-25 17:14:48 +0000
@@ -7,6 +7,7 @@
 
 __all__ = [
     "AbstractUploadPolicy",
+    "ArchiveUploadType",
     "BuildDaemonUploadPolicy",
     "findPolicyByName",
     "IArchiveUploadPolicy",
@@ -14,6 +15,8 @@
     "UploadPolicyError",
     ]
 
+from textwrap import dedent
+
 from zope.component import (
     getGlobalSiteManager,
     getUtility,
@@ -28,6 +31,9 @@
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
 
+from lazr.enum import EnumeratedType, Item
+
+
 # Defined here so that uploadpolicy.py doesn't depend on lp.code.
 SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME = 'recipe'
 # Number of seconds in an hour (used later)
@@ -52,6 +58,13 @@
     """
 
 
+class ArchiveUploadType(EnumeratedType):
+
+    SOURCE_ONLY = Item("Source only")
+    BINARY_ONLY = Item("Binary only")
+    MIXED_ONLY = Item("Mixed only")
+
+
 class AbstractUploadPolicy:
     """Encapsulate the policy of an upload to a launchpad archive.
 
@@ -63,8 +76,9 @@
     """
     implements(IArchiveUploadPolicy)
 
+    name = 'abstract'
     options = None
-    name = 'abstract'
+    accepted_type = None # Must be defined in subclasses.
 
     def __init__(self):
         """Prepare a policy..."""
@@ -75,14 +89,45 @@
         self.unsigned_changes_ok = False
         self.unsigned_dsc_ok = False
         self.create_people = True
-        self.can_upload_source = True
-        self.can_upload_binaries = True
-        self.can_upload_mixed = True
         # future_time_grace is in seconds. 28800 is 8 hours
         self.future_time_grace = 8 * HOURS
         # The earliest year we accept in a deb's file's mtime
         self.earliest_year = 1984
 
+    def validateUploadType(self, upload):
+        """Check that the type of the given upload is accepted by this policy.
+
+        When the type (e.g. sourceful, binaryful or mixed) is not accepted,
+        the upload is rejected.
+        """
+        if upload.sourceful and upload.binaryful:
+            if self.accepted_type != ArchiveUploadType.MIXED_ONLY:
+                upload.reject(
+                    "Source/binary (i.e. mixed) uploads are not allowed.")
+
+        elif upload.sourceful:
+            if self.accepted_type != ArchiveUploadType.SOURCE_ONLY:
+                upload.reject(
+                    "Sourceful uploads are not accepted by this policy.")
+
+        elif upload.binaryful:
+            if self.accepted_type != ArchiveUploadType.BINARY_ONLY:
+                message = dedent("""
+                    Upload rejected because it contains binary packages.
+                    Ensure you are using `debuild -S`, or an equivalent
+                    command, to generate only the source package before
+                    re-uploading.""")
+
+                if upload.is_ppa:
+                    message += dedent("""
+                        See https://help.launchpad.net/Packaging/PPA for
+                        more information.""")
+                upload.reject(message)
+
+        else:
+            raise AssertionError(
+                "Upload is not sourceful, binaryful or mixed.")
+
     def getUploader(self, changes):
         """Get the person who is doing the uploading."""
         return changes.signer
@@ -171,11 +216,7 @@
     """The insecure upload policy is used by the poppy interface."""
 
     name = 'insecure'
-
-    def __init__(self):
-        AbstractUploadPolicy.__init__(self)
-        self.can_upload_binaries = False
-        self.can_upload_mixed = False
+    accepted_type = ArchiveUploadType.SOURCE_ONLY
 
     def rejectPPAUploads(self, upload):
         """Insecure policy allows PPA upload."""
@@ -283,14 +324,13 @@
     """The build daemon upload policy is invoked by the slave scanner."""
 
     name = 'buildd'
+    accepted_type = ArchiveUploadType.BINARY_ONLY
 
     def __init__(self):
         super(BuildDaemonUploadPolicy, self).__init__()
         # We permit unsigned uploads because we trust our build daemons
         self.unsigned_changes_ok = True
         self.unsigned_dsc_ok = True
-        self.can_upload_source = False
-        self.can_upload_mixed = False
 
     def setOptions(self, options):
         AbstractUploadPolicy.setOptions(self, options)
@@ -314,15 +354,13 @@
     """This policy is invoked when processing sync uploads."""
 
     name = 'sync'
+    accepted_type = ArchiveUploadType.SOURCE_ONLY
 
     def __init__(self):
         AbstractUploadPolicy.__init__(self)
         # We don't require changes or dsc to be signed for syncs
         self.unsigned_changes_ok = True
         self.unsigned_dsc_ok = True
-        # We don't want binaries in a sync
-        self.can_upload_mixed = False
-        self.can_upload_binaries = False
 
     def policySpecificChecks(self, upload):
         """Perform sync specific checks."""

=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py	2010-08-24 09:51:26 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py	2010-08-25 17:14:48 +0000
@@ -40,6 +40,7 @@
 from canonical.launchpad.webapp import errorlog
 from lp.app.errors import NotFoundError
 from lp.archiveuploader.uploadpolicy import (
+    ArchiveUploadType,
     BuildDaemonUploadPolicy,
     IArchiveUploadPolicy,
     SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME,
@@ -78,11 +79,7 @@
     """Policy for uploading the results of a source package recipe build."""
 
     name = SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME
-
-    def __init__(self):
-        super(SourcePackageRecipeUploadPolicy, self).__init__()
-        self.can_upload_source = True
-        self.can_upload_binaries = False
+    accepted_type = ArchiveUploadType.SOURCE_ONLY
 
     def getUploader(self, changes):
         """Return the person doing the upload."""

=== modified file 'lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt'
--- lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt	2010-08-20 12:11:30 +0000
+++ lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt	2010-08-25 17:14:48 +0000
@@ -253,6 +253,7 @@
     ...     sourcename="foo", distroseries=hoary, version="1.0-2",
     ...     status=PackagePublishingStatus.PUBLISHED)
 
+    >>> from lp.archiveuploader.uploadpolicy import ArchiveUploadType
     >>> from canonical.launchpad.ftests import import_public_test_keys
     >>> from canonical.launchpad.interfaces import IComponentSet
     >>> from lp.soyuz.model.component import ComponentSelection
@@ -264,7 +265,7 @@
     >>> trash = ComponentSelection(distroseries=hoary, component=universe)
     >>> sync_policy = getPolicy(name='sync', distro='ubuntu',
     ...    distroseries='hoary')
-    >>> sync_policy.can_upload_binaries = True
+    >>> sync_policy.accepted_type = ArchiveUploadType.BINARY_ONLY
     >>> foo_upload = NascentUpload.from_changesfile_path(
     ...    datadir('suite/foo_1.0-2_multi_binary/foo_1.0-2_i386.changes'),
     ...    sync_policy, mock_logger_quiet)

=== modified file 'lib/lp/soyuz/doc/soyuz-set-of-uploads.txt'
--- lib/lp/soyuz/doc/soyuz-set-of-uploads.txt	2010-08-06 14:29:36 +0000
+++ lib/lp/soyuz/doc/soyuz-set-of-uploads.txt	2010-08-25 17:14:48 +0000
@@ -319,22 +319,6 @@
   Subject: bar_1.0-3_source.changes rejected
   ...
 
-Check the rejection (instead of failure) of an upload to a series or
-pocket which does not exist. We use the security upload policy for easier
-testing, as unsigned uploads are allowed.
-
-  >>> simulate_upload(
-  ...     'unstable_1.0-1', upload_policy="security", loglevel=logging.ERROR)
-  Rejected uploads: ['unstable_1.0-1']
-
-  >>> read_email()
-  To: Celso Providelo <[e-mail]>
-  Subject: unstable_1.0-1_source.changes rejected
-  ...
-  Rejected:
-  Unable to find distroseries: unstable
-  ...
-
 Force weird behavior with rfc2047 sentences containing '.' on
 bar_1.0-4, which caused bug # 41102.
 
@@ -605,6 +589,9 @@
 
 == Staged Security Uploads ==
 
+XXX: Salgado, 2010-08-25, bug=624078: This entire section should be removed
+but if you do so publish-distro.py (called further down) will hang.
+
 Perform a staged (source-first) security upload, simulating a run of
 the buildd-queuebuilder inbetween, to verify fix for bug 53437. To be
 allowed a security upload, we need to use a released distroseries,
@@ -617,7 +604,7 @@
   >>> ss = SectionSelection(distroseries=warty, section=devel)
 
   >>> simulate_upload(
-  ...     'baz_1.0-1', is_new=True, upload_policy="security",
+  ...     'baz_1.0-1', is_new=True, upload_policy="anything",
   ...     series="warty-security", distro="ubuntu")
 
 Check there's a SourcePackageRelease with no build.
@@ -650,7 +637,7 @@
 Upload the i386 binary:
 
   >>> simulate_upload(
-  ...     'baz_1.0-1_single_binary', is_new=True, upload_policy="security",
+  ...     'baz_1.0-1_single_binary', is_new=True, upload_policy="anything",
   ...     distro="ubuntu", series="warty-security")
 
 Should still just have one build, and it should now be FULLYBUILT.