launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/launchpad/no-disabled-arch-publications-bug-648715 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #648715 Binary publications should not be created for disabled architectures
  https://bugs.launchpad.net/bugs/648715


= Summary = 
Prevent arch-all packages from being published in disabled architectures

= Description =
A fix was done a while ago that stopped disabled distroarchseries from 
publishing.  This was the wrong approach - what needs to happen is to prevent 
new publications in the first place.

This only happens for arch-all (architecture independent packages) because 
directly targeted builds for disabled architectures are already prevented.

= Implementation =
A few stages:
 * Remove the old code that prevented publication.  We want to do this now so 
that old publications can be removed from the repo.
 * Modify the test so that it checks that arch-all binaries don't get 
published in disabled distroarchseries.
 * Change the publication creating code to ignore disabled architectures when 
calculating the list of distroarchseries for arch-all packages.
-- 
https://code.launchpad.net/~julian-edwards/launchpad/no-disabled-arch-publications-bug-648715/+merge/36838
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/no-disabled-arch-publications-bug-648715 into lp:launchpad.

=== modified file 'lib/lp/soyuz/model/distroarchseries.py'
--- lib/lp/soyuz/model/distroarchseries.py	2010-09-10 15:28:42 +0000
+++ lib/lp/soyuz/model/distroarchseries.py	2010-09-28 10:51:01 +0000
@@ -338,11 +338,6 @@
 
     def publish(self, diskpool, log, archive, pocket, is_careful=False):
         """See `ICanPublishPackages`."""
-        if not self.enabled:
-            log.debug(
-                "Skipping disabled architecture %s" % self.architecturetag)
-            return set()
-
         log.debug("Attempting to publish pending binaries for %s"
               % self.architecturetag)
 

=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py	2010-09-03 15:02:39 +0000
+++ lib/lp/soyuz/model/queue.py	2010-09-28 10:51:01 +0000
@@ -1442,20 +1442,24 @@
         build_archtag = self.build.distro_arch_series.architecturetag
         # Determine the target arch series.
         # This will raise NotFoundError if anything odd happens.
-        target_dar = self.packageupload.distroseries[build_archtag]
+        target_das = self.packageupload.distroseries[build_archtag]
         debug(logger, "Publishing build to %s/%s/%s" % (
-            target_dar.distroseries.distribution.name,
-            target_dar.distroseries.name,
+            target_das.distroseries.distribution.name,
+            target_das.distroseries.name,
             build_archtag))
-        # And get the other distroarchseriess
-        other_dars = set(self.packageupload.distroseries.architectures)
-        other_dars = other_dars - set([target_dar])
+        # Get the other enabled distroarchseries for this
+        # distroseries.  If the binary is architecture independent then
+        # we need to publish it in all of those too.
+        other_das = set(
+            arch for arch in self.packageupload.distroseries.architectures
+            if arch.enabled)
+        other_das = other_das - set([target_das])
         # First up, publish everything in this build into that dar.
         published_binaries = []
         for binary in self.build.binarypackages:
-            target_dars = set([target_dar])
+            target_dars = set([target_das])
             if not binary.architecturespecific:
-                target_dars = target_dars.union(other_dars)
+                target_dars = target_dars.union(other_das)
                 debug(logger, "... %s/%s (Arch Independent)" % (
                     binary.binarypackagename.name,
                     binary.version))

=== modified file 'lib/lp/soyuz/tests/test_publishing_top_level_api.py'
--- lib/lp/soyuz/tests/test_publishing_top_level_api.py	2010-09-10 15:24:47 +0000
+++ lib/lp/soyuz/tests/test_publishing_top_level_api.py	2010-09-28 10:51:01 +0000
@@ -5,7 +5,10 @@
 
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
-from lp.soyuz.enums import PackagePublishingStatus
+from lp.soyuz.enums import (
+    PackagePublishingStatus,
+    PackageUploadStatus,
+    )
 from lp.soyuz.tests.test_publishing import TestNativePublishingBase
 
 
@@ -421,14 +424,37 @@
             expected_result=[pub_published_release, pub_pending_release])
 
     def test_publishing_disabled_distroarchseries(self):
-        # Disabled DASes will be skipped even if there are pending
-        # publications for them.
-        binaries = self.getPubBinaries(architecturespecific=True)
-        # Just use the first binary.
-        binary = binaries[0]
-        self.assertEqual(PackagePublishingStatus.PENDING, binary.status)
-
-        binary.distroarchseries.enabled = False
-        self._publish(pocket=binary.pocket)
-
-        self.assertEqual(PackagePublishingStatus.PENDING, binary.status)
+        # Disabled DASes will not receive new publications at all.
+
+        # Make an arch-all source and some builds for it.
+        archive = self.factory.makeArchive(
+            distribution=self.ubuntutest, virtualized=False)
+        source = self.getPubSource(
+            archive=archive, architecturehintlist='all')
+        [build_i386] = source.createMissingBuilds()
+        bin_i386 = self.uploadBinaryForBuild(build_i386, 'bin-i386')
+
+        # Now make sure they have a packageupload (but no publishing
+        # records).
+        changes_file_name = '%s_%s_%s.changes' % (
+            bin_i386.name, bin_i386.version, build_i386.arch_tag)
+        pu_i386 = self.addPackageUpload(
+            build_i386.archive, build_i386.distro_arch_series.distroseries,
+            build_i386.pocket, changes_file_content='anything',
+            changes_file_name=changes_file_name,
+            upload_status=PackageUploadStatus.ACCEPTED)
+        pu_i386.addBuild(build_i386)
+
+        # Now we make hppa a disabled architecture, and then call the
+        # publish method on the packageupload.  The arch-all binary
+        # should be published only in the i386 arch, not the hppa one.
+        hppa = pu_i386.distroseries.getDistroArchSeries('hppa')
+        hppa.enabled = False
+        for pu_build in pu_i386.builds:
+            pu_build.publish()
+
+        publications = archive.getAllPublishedBinaries(name="bin-i386")
+
+        self.assertEqual(1, publications.count())
+        self.assertEqual(
+            'i386', publications[0].distroarchseries.architecturetag)