launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/nu-overrideArchive-early into lp:launchpad.

Commit message:
Override uploads from the primary archive to partner based solely on the .changes component, not the component after calculating package overrides. This simplifies things considerably, as the policy archive is set correctly before anything that cares runs.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #545962 in Launchpad itself: "Incorrect override when uploading a package to partner"
  https://bugs.launchpad.net/launchpad/+bug/545962

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/nu-overrideArchive-early/+merge/227865

archiveuploader overrides an upload from primary archive to the partner archive if package override lookups place it in the partner component. This isn't used in practice, as the .changes always specifies that the files are targeted to partner.

This branch moves the partner archive override much earlier in upload processing, based on just the pre-override components listed in .changes, not the post-package-override-lookup components. A number of partner-specific hacks can then disappear from NascentUpload and DSCFile, as the upload archive is always correct and we need not hunt for ancestry in both primary and partner.
-- 
https://code.launchpad.net/~wgrant/launchpad/nu-overrideArchive-early/+merge/227865
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/nu-overrideArchive-early into lp:launchpad.

=== modified file 'lib/lp/archiveuploader/dscfile.py'
--- lib/lp/archiveuploader/dscfile.py	2013-06-26 07:53:23 +0000
+++ lib/lp/archiveuploader/dscfile.py	2014-07-23 06:15:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 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).
 
 """ DSCFile and related.
@@ -71,7 +71,6 @@
     ArchivePurpose,
     SourcePackageFormat,
     )
-from lp.soyuz.interfaces.archive import IArchiveSet
 
 
 def unpack_source(dsc_filepath):
@@ -443,17 +442,7 @@
 
         :raise: `NotFoundError` when the wanted file could not be found.
         """
-        # We cannot check the archive purpose for partner archives here,
-        # because the archive override rules have not been applied yet.
-        # Uploads destined for the Ubuntu main archive and the 'partner'
-        # component will eventually end up in the partner archive though.
-        if (self.policy.archive.purpose == ArchivePurpose.PRIMARY and
-            self.component_name == 'partner'):
-            archives = [
-                getUtility(IArchiveSet).getByDistroPurpose(
-                distribution=self.policy.distro,
-                purpose=ArchivePurpose.PARTNER)]
-        elif (self.policy.archive.purpose == ArchivePurpose.PPA and
+        if (self.policy.archive.purpose == ArchivePurpose.PPA and
             determine_source_file_type(filename) in (
                 SourcePackageFileType.ORIG_TARBALL,
                 SourcePackageFileType.COMPONENT_ORIG_TARBALL)):

=== modified file 'lib/lp/archiveuploader/nascentupload.py'
--- lib/lp/archiveuploader/nascentupload.py	2014-07-22 11:31:48 +0000
+++ lib/lp/archiveuploader/nascentupload.py	2014-07-23 06:15:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 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).
 
 """The processing of nascent uploads.
@@ -161,6 +161,12 @@
             self.run_and_check_error(uploaded_file.checkNameIsTaintFree)
             self.run_and_check_error(uploaded_file.checkSizeAndCheckSum)
 
+        # Override archive location if necessary to cope with partner
+        # uploads to a primary path. We consider an upload to be
+        # targeted to partner if the .changes lists any files in the
+        # partner component.
+        self.overrideArchive()
+
         self._check_overall_consistency()
         if self.sourceful:
             self._check_sourceful_consistency()
@@ -185,11 +191,6 @@
         self.find_and_apply_overrides()
         self._overrideDDEBSs()
 
-        # Override archive location if necessary to cope with partner
-        # uploads to a primary path. Yes, we actually potentially change
-        # the target archive based on the override component.
-        self.overrideArchive()
-
         # Check upload rights for the signer of the upload.
         self.verify_acl(build)
 
@@ -450,11 +451,6 @@
         """Return a set of components present in the uploaded files."""
         return set(file.component_name for file in self.changes.files)
 
-    @property
-    def is_partner(self):
-        """Return true if this is an upload to the partner archive."""
-        return PARTNER_COMPONENT_NAME in self.getComponents()
-
     def reject(self, msg):
         """Add the provided message to the rejection message."""
         self.rejections.append(msg)
@@ -677,13 +673,8 @@
         use_default_component = True
         autoaccept_new = False
         override_at_all = True
-        if self.policy.archive.is_primary:
-            # overrideArchive can switch to the partner archive if there
-            # is ancestry there, so try partner after primary.
-            partner = self.policy.archive.distribution.getArchiveByComponent(
-                PARTNER_COMPONENT_NAME)
-            if partner is not None:
-                archives.append(partner)
+        if self.policy.archive.is_partner:
+            use_default_component = False
         elif self.policy.archive.is_copy:
             # Copy archives always inherit their overrides from the
             # primary archive. We don't want to perform the version
@@ -700,12 +691,6 @@
             autoaccept_new = True
             override_at_all = False
 
-        # NascentUpload.is_partner additionally checks if any of the
-        # components are partner.
-        if (self.policy.archive.is_partner or
-                (self.policy.archive.is_primary and self.is_partner)):
-            use_default_component = False
-
         for uploaded_file in self.changes.files:
             if isinstance(uploaded_file, DSCFile):
                 self.logger.debug(