Merge lp:~julian-edwards/launchpad/no-disabled-arch-publications-bug-648715 into lp:launchpad/db-devel

[Julian Edwards]

Prevent arch-all packages from being published in disabled architectures.

= 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.

[William Grant (wgrant)]

PublishingSet.copyBinariesTo needs to be fixed as well. Ideally this would be abstracted.

review: Needs Fixing (code)

[Graham Binns (gmb)]

review: Approve (code)

[Julian Edwards (julian-edwards)]

I've added a new revision that also does the same check when copying packages
betweeb distroseries. Would you mind blessing that too please? Thanks.
(partial diff attached)

[Graham Binns (gmb)]

> I've added a new revision that also does the same check when copying packages
> betweeb distroseries. Would you mind blessing that too please? Thanks.
> (partial diff attached)

r=me on those changes.

review: Approve (code)

[William Grant (wgrant)]

review: Approve (code)

=== 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 16:07:50 +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/publishing.py'
--- lib/lp/soyuz/model/publishing.py	2010-08-30 15:00:23 +0000
+++ lib/lp/soyuz/model/publishing.py	2010-09-28 16:07:50 +0000
@@ -1234,6 +1234,10 @@
             binarypackagerelease = binary.binarypackagerelease
             target_component = override_component or binary.component
 
+            # XXX 2010-09-28 Julian bug=649859
+            # This piece of code duplicates the logic in
+            # PackageUploadBuild.publish(), it needs to be refactored.
+
             if binarypackagerelease.architecturespecific:
                 # If the binary is architecture specific and the target
                 # distroseries does not include the architecture then we
@@ -1250,7 +1254,9 @@
                     continue
                 destination_architectures = [target_architecture]
             else:
-                destination_architectures = distroseries.architectures
+                destination_architectures = [
+                    arch for arch in distroseries.architectures
+                    if arch.enabled]
 
             for distroarchseries in destination_architectures:
 

=== 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 16:07:50 +0000
@@ -1442,20 +1442,30 @@
         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.
+
+        # XXX Julian 2010-09-28 bug=649859
+        # This logic is duplicated in
+        # PackagePublishingSet.copyBinariesTo() and should be
+        # refactored.
+        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/scripts/tests/test_copypackage.py'
--- lib/lp/soyuz/scripts/tests/test_copypackage.py	2010-08-27 11:19:54 +0000
+++ lib/lp/soyuz/scripts/tests/test_copypackage.py	2010-09-28 16:07:50 +0000
@@ -977,6 +977,44 @@
              ],
             [copy.displayname for copy in copies])
 
+    def test_copying_arch_indep_binaries_with_disabled_arches(self):
+        # When copying an arch-indep binary to a new series, we must not
+        # copy it into architectures that are disabled.
+
+        # Make a new arch-all source and binary in breezy-autotest:
+        archive = self.factory.makeArchive(
+            distribution=self.test_publisher.ubuntutest, virtualized=False)
+        source = self.test_publisher.getPubSource(
+            archive=archive, architecturehintlist='all')
+        [bin_i386, bin_hppa] = self.test_publisher.getPubBinaries(
+            pub_source=source)
+
+        # Now make a new distroseries with two architectures, one of
+        # which is disabled.
+        nobby = self.factory.makeDistroSeries(
+            distribution=self.test_publisher.ubuntutest, name='nobby')
+        i386_pf = self.factory.makeProcessorFamily(name='my_i386')
+        nobby_i386 = self.factory.makeDistroArchSeries(
+            distroseries=nobby, architecturetag='i386',
+            processorfamily=i386_pf)
+        hppa_pf = self.factory.makeProcessorFamily(name='my_hppa')
+        nobby_hppa = self.factory.makeDistroArchSeries(
+            distroseries=nobby, architecturetag='hppa',
+            processorfamily=hppa_pf)
+        nobby_hppa.enabled = False
+        nobby.nominatedarchindep = nobby_i386
+        self.test_publisher.addFakeChroots(nobby)
+
+        # Now we can copy the package with binaries.
+        copies = _do_direct_copy(
+            source, source.archive, nobby, source.pocket, True)
+
+        # The binary should not be published for hppa.
+        self.assertEquals(
+            [u'foo 666 in nobby',
+             u'foo-bin 666 in nobby i386',],
+            [copy.displayname for copy in copies])
+
 
 class DoDelayedCopyTestCase(TestCaseWithFactory):
 

=== 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 16:07:50 +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)