← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/launchpad/bug-788078 into lp:launchpad

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-788078 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #788078 in Launchpad itself: "Allow the Soyuz publisher to optionally publish only non-Ubuntu distros"
  https://bugs.launchpad.net/launchpad/+bug/788078

For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-788078/+merge/68680

= Summary =

We want to be able to publish all Ubuntu-derived distributions with a single script run, without having to name them on the command line.  This will take admin involvement out of the process for deriving a new distribution.


== Proposed fix ==

A new option for publish-distro, --all-derived, makes the script iterate over all derived distributions.

(I chose the option name so that it could use -a as a short form; a useful mnemonic and besides, -d and -D were both taken.)


== Pre-implementation notes ==

There's no clear line between "all Ubuntu-derived distributions" and "all derived distributions, not counting Ubuntu despite it being technically a derivative of Debian."  We're not expecting to host any distributions that would reveal the difference.  If we did, the current behaviour would probably be the right one until it got too big to publish on the same server.


== Implementation details ==

There were no loop-carried dependencies in the script; its main method is stateless.  Piece of cake to make it iterate over all matching distributions.  We could easily extend command-line distribution selection for more fine-grained load-balancing in the future, e.g. "only distributions "a" thru "f" on this server."

A subtlety is that I made the --distribution option default to None (but then default to "ubuntu" while looking up the distro) so that validateOptions can check for mistakes like "--distribution=ubuntu --all-derived" (which might not do what one might expect).


== Tests ==

{{{
./bin/test -vvc lp.soyuz.scripts.tests.test_publishdistro
./bin/test -vvc lp.archivepublisher
./bin/test -vvc lp.soyuz -t publish
}}}


== Demo and Q/A ==

Enable the "derived distributions" feature flags.  Create a derived distribution.  Run publish-distro.py --all-derived.  Archive directories for the new distro will appear in /srv/launchpad.net/ (or /var/tmp/archive on a dev system) but the ubuntu ones will not be updated.  A regular "publish-distro.py --distribution=ubuntu" will will publish Ubuntu only.


= Launchpad lint =

This includes some files whose changes I split out to a separate branch:

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/soyuz/scripts/publishdistro.py
  lib/lp/soyuz/scripts/tests/test_publishdistro.py
  lib/lp/soyuz/scripts/processaccepted.py
  lib/lp/soyuz/tests/test_processaccepted.py
  lib/lp/registry/interfaces/distribution.py
  lib/lp/registry/tests/test_distribution.py
  lib/lp/registry/model/distribution.py
-- 
https://code.launchpad.net/~jtv/launchpad/bug-788078/+merge/68680
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-788078 into lp:launchpad.
=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py	2011-07-13 18:18:42 +0000
+++ lib/lp/registry/interfaces/distribution.py	2011-07-21 13:04:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0211,E0213
@@ -750,6 +750,13 @@
         :return: A dict as per `IDistribution.getCurrentSourceReleases`
         """
 
+    def getDerivedDistributions():
+        """Find derived distributions.
+
+        :return: An iterable of all derived distributions (not including
+            Ubuntu, even if it is technically derived from Debian).
+        """
+
 
 class NoSuchDistribution(NameLookupFailed):
     """Raised when we try to find a distribution that doesn't exist."""

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2011-07-14 14:08:04 +0000
+++ lib/lp/registry/model/distribution.py	2011-07-21 13:04:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0611,W0212
@@ -157,6 +157,7 @@
     DistributionSourcePackage,
     )
 from lp.registry.model.distroseries import DistroSeries
+from lp.registry.model.distroseriesparent import DistroSeriesParent
 from lp.registry.model.hasdrivers import HasDriversMixin
 from lp.registry.model.karma import KarmaContextMixin
 from lp.registry.model.milestone import (
@@ -1993,3 +1994,12 @@
             result[sourcepackage] = DistributionSourcePackageRelease(
                 distro, sp_release)
         return result
+
+    def getDerivedDistributions(self):
+        """See `IDistributionSet`."""
+        ubuntu_id = getUtility(ILaunchpadCelebrities).ubuntu.id
+        return IStore(DistroSeries).find(
+            Distribution,
+            Distribution.id == DistroSeries.distributionID,
+            DistroSeries.id == DistroSeriesParent.derived_series_id,
+            DistroSeries.distributionID != ubuntu_id).config(distinct=True)

=== modified file 'lib/lp/registry/tests/test_distribution.py'
--- lib/lp/registry/tests/test_distribution.py	2011-06-17 19:47:36 +0000
+++ lib/lp/registry/tests/test_distribution.py	2011-07-21 13:04:29 +0000
@@ -20,10 +20,15 @@
 from canonical.testing.layers import (
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
+    ZopelessDatabaseLayer,
     )
 from lp.app.errors import NotFoundError
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.registry.errors import NoSuchDistroSeries
-from lp.registry.interfaces.distribution import IDistribution
+from lp.registry.interfaces.distribution import (
+    IDistribution,
+    IDistributionSet,
+    )
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.tests.test_distroseries import (
@@ -37,6 +42,7 @@
     login_person,
     TestCaseWithFactory,
     )
+from lp.testing.matchers import Provides
 from lp.testing.views import create_initialized_view
 
 
@@ -421,3 +427,43 @@
         self.assertNotEqual(distribution.owner, distribution.registrant)
         self.assertEqual(distribution.owner, self.owner)
         self.assertEqual(distribution.registrant, self.registrant)
+
+
+class DistributionSet(TestCaseWithFactory):
+    """Test case for `IDistributionSet`."""
+
+    layer = ZopelessDatabaseLayer
+
+    def test_implements_interface(self):
+        self.assertThat(
+            getUtility(IDistributionSet), Provides(IDistributionSet))
+
+    def test_getDerivedDistributions_finds_derived_distro(self):
+        dsp = self.factory.makeDistroSeriesParent()
+        derived_distro = dsp.derived_series.distribution
+        distroset = getUtility(IDistributionSet)
+        self.assertIn(derived_distro, distroset.getDerivedDistributions())
+
+    def test_getDerivedDistributions_ignores_nonderived_distros(self):
+        distroset = getUtility(IDistributionSet)
+        nonderived_distro = self.factory.makeDistribution()
+        self.assertNotIn(
+            nonderived_distro, distroset.getDerivedDistributions())
+
+    def test_getDerivedDistributions_ignores_ubuntu_even_if_derived(self):
+        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+        self.factory.makeDistroSeriesParent(
+            derived_series=ubuntu.currentseries)
+        distroset = getUtility(IDistributionSet)
+        self.assertNotIn(ubuntu, distroset.getDerivedDistributions())
+
+    def test_getDerivedDistribution_finds_each_distro_just_once(self):
+        # Derived distros are not duplicated in the output of
+        # getDerivedDistributions, even if they have multiple parents and
+        # multiple derived series.
+        dsp = self.factory.makeDistroSeriesParent()
+        distro = dsp.derived_series.distribution
+        other_series = self.factory.makeDistroSeries(distribution=distro)
+        self.factory.makeDistroSeriesParent(derived_series=other_series)
+        distroset = getUtility(IDistributionSet)
+        self.assertEqual(1, len(list(distroset.getDerivedDistributions())))

=== modified file 'lib/lp/soyuz/scripts/processaccepted.py'
--- lib/lp/soyuz/scripts/processaccepted.py	2011-07-14 16:51:59 +0000
+++ lib/lp/soyuz/scripts/processaccepted.py	2011-07-21 13:04:29 +0000
@@ -19,7 +19,6 @@
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
-from canonical.launchpad.interfaces.lpstorm import IStore
 from canonical.launchpad.webapp.errorlog import (
     ErrorReportingUtility,
     ScriptRequest,
@@ -31,7 +30,6 @@
 from lp.bugs.interfaces.bugtask import BugTaskStatus
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.registry.model.distroseriesparent import DistroSeriesParent
 from lp.services.scripts.base import (
     LaunchpadCronScript,
     LaunchpadScriptFailure,
@@ -287,19 +285,6 @@
         else:
             self.txn.commit()
 
-    def findDerivedDistros(self):
-        """Find Ubuntu-derived distributions."""
-        # Avoid circular imports.
-        from lp.registry.model.distribution import Distribution
-        from lp.registry.model.distroseries import DistroSeries
-
-        ubuntu_id = getUtility(ILaunchpadCelebrities).ubuntu.id
-        return IStore(DistroSeries).find(
-            Distribution,
-            Distribution.id == DistroSeries.distributionID,
-            DistroSeries.id == DistroSeriesParent.derived_series_id,
-            DistroSeries.distributionID != ubuntu_id).config(distinct=True)
-
     def findNamedDistro(self, distro_name):
         """Find the `Distribution` called `distro_name`."""
         self.logger.debug("Finding distribution %s.", distro_name)
@@ -312,7 +297,7 @@
     def findTargetDistros(self):
         """Find the distribution(s) to process, based on arguments."""
         if self.options.derived:
-            return self.findDerivedDistros()
+            return getUtility(IDistributionSet).getDerivedDistributions()
         else:
             return [self.findNamedDistro(self.args[0])]
 

=== modified file 'lib/lp/soyuz/scripts/publishdistro.py'
--- lib/lp/soyuz/scripts/publishdistro.py	2011-07-19 12:32:37 +0000
+++ lib/lp/soyuz/scripts/publishdistro.py	2011-07-21 13:04:29 +0000
@@ -62,7 +62,11 @@
 
         self.parser.add_option(
             "-d", "--distribution", dest="distribution", metavar="DISTRO",
-            default="ubuntu", help="The distribution to publish.")
+            default=None, help="The distribution to publish.")
+
+        self.parser.add_option(
+            "-a", "--all-derived", action="store_true", dest="all_derived",
+            default=False, help="Publish all Ubuntu-derived distributions.")
 
         self.parser.add_option(
             '-s', '--suite', metavar='SUITE', dest='suite', action='append',
@@ -166,83 +170,104 @@
                 "Can only specify one of partner, ppa, private-ppa, "
                 "copy-archive and primary-debug.")
 
+        if self.options.all_derived and self.options.distribution is not None:
+                raise OptionValueError(
+                    "Specify --distribution or --all-derived, but not both.")
+
         for_ppa = (self.options.ppa or self.options.private_ppa)
         if for_ppa and self.options.distsroot:
             raise OptionValueError(
                 "We should not define 'distsroot' in PPA mode!", )
 
-    def findDistro(self):
-        """Find the selected distribution."""
+    def findSelectedDistro(self):
+        """Find the `Distribution` named by the --distribution option.
+
+        Defaults to Ubuntu if no name was given.
+        """
         self.logger.debug("Finding distribution object.")
         name = self.options.distribution
+        if name is None:
+            # Default to publishing Ubuntu.
+            name = "ubuntu"
         distro = getUtility(IDistributionSet).getByName(name)
         if distro is None:
             raise OptionValueError("Distribution '%s' not found." % name)
         return distro
 
-    def findSuite(self, suite):
+    def findDerivedDistros(self):
+        """Find all Ubuntu-derived distributions."""
+        self.logger.debug("Finding derived distributions.")
+        return getUtility(IDistributionSet).getDerivedDistributions()
+
+    def findDistros(self):
+        """Find the selected distribution(s)."""
+        if self.options.all_derived:
+            return self.findDerivedDistros()
+        else:
+            return [self.findSelectedDistro()]
+
+    def findSuite(self, distribution, suite):
         """Find the named `suite` in the selected `Distribution`.
 
         :param suite: The suite name to look for.
         :return: A tuple of distroseries name and pocket.
         """
         try:
-            series, pocket = self.distribution.getDistroSeriesAndPocket(suite)
+            series, pocket = distribution.getDistroSeriesAndPocket(suite)
         except NotFoundError, e:
             raise OptionValueError(e)
         return series.name, pocket
 
-    def findAllowedSuites(self):
+    def findAllowedSuites(self, distribution):
         """Find the selected suite(s)."""
-        return set([self.findSuite(suite) for suite in self.options.suite])
+        return set([
+            self.findSuite(distribution, suite)
+            for suite in self.options.suite])
 
-    def getDebugArchive(self):
+    def getDebugArchive(self, distribution):
         """Find the debug archive for the selected distribution, as a list."""
         debug_archive = getUtility(IArchiveSet).getByDistroPurpose(
-            self.distribution, ArchivePurpose.DEBUG)
+            distribution, ArchivePurpose.DEBUG)
         if debug_archive is None:
             raise OptionValueError(
-                "Could not find DEBUG archive for %s"
-                % self.distribution.name)
+                "Could not find DEBUG archive for %s" % distribution.name)
         return [debug_archive]
 
-    def getCopyArchives(self):
+    def getCopyArchives(self, distribution):
         """Find copy archives for the selected distribution."""
         copy_archives = list(
             getUtility(IArchiveSet).getArchivesForDistribution(
-                self.distribution, purposes=[ArchivePurpose.COPY]))
+                distribution, purposes=[ArchivePurpose.COPY]))
         if copy_archives == []:
             raise LaunchpadScriptFailure("Could not find any COPY archives")
         return copy_archives
 
-    def getPPAs(self):
+    def getPPAs(self, distribution):
         """Find private package archives for the selected distribution."""
         if self.isCareful(self.options.careful_publishing):
-            return self.distribution.getAllPPAs()
+            return distribution.getAllPPAs()
         else:
-            return self.distribution.getPendingPublicationPPAs()
+            return distribution.getPendingPublicationPPAs()
 
-    def getTargetArchives(self):
+    def getTargetArchives(self, distribution):
         """Find the archive(s) selected by the script's options."""
         if self.options.partner:
-            return [self.distribution.getArchiveByComponent('partner')]
+            return [distribution.getArchiveByComponent('partner')]
         elif self.options.ppa:
-            return filter(is_ppa_public, self.getPPAs())
+            return filter(is_ppa_public, self.getPPAs(distribution))
         elif self.options.private_ppa:
-            return filter(is_ppa_private, self.getPPAs())
+            return filter(is_ppa_private, self.getPPAs(distribution))
         elif self.options.primary_debug:
-            return self.getDebugArchive()
+            return self.getDebugArchive(distribution)
         elif self.options.copy_archive:
-            return self.getCopyArchives()
+            return self.getCopyArchives(distribution)
         else:
-            return [self.distribution.main_archive]
+            return [distribution.main_archive]
 
-    def getPublisher(self, archive, allowed_suites):
+    def getPublisher(self, distribution, archive, allowed_suites):
         """Get a publisher for the given options."""
         if archive.purpose in MAIN_ARCHIVE_PURPOSES:
-            description = "%s %s" % (
-                self.distribution.name,
-                archive.displayname)
+            description = "%s %s" % (distribution.name, archive.displayname)
             # Only let the primary/partner archives override the distsroot.
             distsroot = self.options.distsroot
         else:
@@ -294,21 +319,22 @@
         """See `LaunchpadScript`."""
         self.validateOptions()
         self.logOptions()
-        self.distribution = self.findDistro()
-        allowed_suites = self.findAllowedSuites()
-
-        for archive in self.getTargetArchives():
-            publisher = self.getPublisher(archive, allowed_suites)
-
-            if archive.status == ArchiveStatus.DELETING:
-                work_done = self.deleteArchive(archive, publisher)
-            elif archive.publish:
-                self.publishArchive(archive, publisher)
-                work_done = True
-            else:
-                work_done = False
-
-            if work_done:
-                self.txn.commit()
+
+        for distribution in self.findDistros():
+            allowed_suites = self.findAllowedSuites(distribution)
+            for archive in self.getTargetArchives(distribution):
+                publisher = self.getPublisher(
+                    distribution, archive, allowed_suites)
+
+                if archive.status == ArchiveStatus.DELETING:
+                    work_done = self.deleteArchive(archive, publisher)
+                elif archive.publish:
+                    self.publishArchive(archive, publisher)
+                    work_done = True
+                else:
+                    work_done = False
+
+                if work_done:
+                    self.txn.commit()
 
         self.logger.debug("Ciao")

=== modified file 'lib/lp/soyuz/scripts/tests/test_publishdistro.py'
--- lib/lp/soyuz/scripts/tests/test_publishdistro.py	2011-07-19 13:39:28 +0000
+++ lib/lp/soyuz/scripts/tests/test_publishdistro.py	2011-07-21 13:04:29 +0000
@@ -15,6 +15,7 @@
 
 from canonical.config import config
 from canonical.testing.layers import ZopelessDatabaseLayer
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.archivepublisher.config import getPubConfig
 from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
 from lp.archivepublisher.publishing import Publisher
@@ -28,6 +29,7 @@
 from lp.services.scripts.base import LaunchpadScriptFailure
 from lp.soyuz.enums import (
     ArchivePurpose,
+    ArchiveStatus,
     BinaryPackageFormat,
     PackagePublishingStatus,
     )
@@ -399,7 +401,9 @@
 class FakeArchive:
     """A very simple fake `Archive`."""
     def __init__(self, purpose=ArchivePurpose.PRIMARY):
+        self.publish = True
         self.purpose = purpose
+        self.status = ArchiveStatus.ACTIVE
 
 
 class FakePublisher:
@@ -418,11 +422,24 @@
 
     layer = ZopelessDatabaseLayer
 
-    def makeScript(self, distribution=None, args=[]):
+    def makeDistro(self):
+        """Create a distribution."""
+        # Set up a temporary directory as publish_root_dir.  Without
+        # this, getPublisher will create archives in the current
+        # directory.
+        return self.factory.makeDistribution(
+            publish_root_dir=unicode(self.makeTemporaryDirectory()))
+
+    def makeScript(self, distribution=None, args=[], all_derived=False):
         """Create a `PublishDistro` for `distribution`."""
-        if distribution is None:
-            distribution = self.factory.makeDistribution()
-        full_args = ['-d', distribution.name] + args
+        if distribution is None and not all_derived:
+            distribution = self.makeDistro()
+        distro_args = []
+        if distribution is not None:
+            distro_args.extend(['--distribution', distribution.name])
+        if all_derived:
+            distro_args.append('--all-derived')
+        full_args = args + distro_args
         script = PublishDistro(test_args=full_args)
         script.distribution = distribution
         script.logger = DevNullLogger()
@@ -522,19 +539,67 @@
         script = self.makeScript(args=['--private-ppa', '--distsroot=/tmp'])
         self.assertRaises(OptionValueError, script.validateOptions)
 
-    def test_findDistro_finds_distribution(self):
-        # findDistro looks up and returns the distribution named on the
+    def test_validateOptions_accepts_all_derived_without_distro(self):
+        # If --all-derived is given, the --distribution option is not
+        # required.
+        PublishDistro(test_args=['--all-derived']).validateOptions()
+        # The test is that we get here without error.
+        pass
+
+    def test_validateOptions_does_not_accept_distro_with_all_derived(self):
+        # The --all-derived option conflicts with the --distribution
+        # option.
+        distro = self.makeDistro()
+        script = PublishDistro(test_args=['-d', distro.name, '--all-derived'])
+        self.assertRaises(OptionValueError, script.validateOptions)
+
+    def test_findDistros_finds_selected_distribution(self):
+        # findDistros looks up and returns the distribution named on the
         # command line.
-        distro = self.factory.makeDistribution()
-        self.assertEqual(distro, self.makeScript(distro).findDistro())
-
-    def test_findDistro_raises_if_distro_not_found(self):
+        distro = self.makeDistro()
+        self.assertEqual([distro], self.makeScript(distro).findDistros())
+
+    def test_findDistros_finds_ubuntu_by_default(self):
+        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+        self.assertContentEqual(
+            [ubuntu], PublishDistro(test_args=[]).findDistros())
+
+    def test_findDistros_raises_if_selected_distro_not_found(self):
         # If findDistro can't find the distribution, that's an
         # OptionValueError.
         wrong_name = self.factory.getUniqueString()
         self.assertRaises(
             OptionValueError,
-            PublishDistro(test_args=['-d', wrong_name]).findDistro)
+            PublishDistro(test_args=['-d', wrong_name]).findDistros)
+
+    def test_findDistros_for_all_derived_distros_may_return_empty(self):
+        # If the --all-derived option is given but there are no derived
+        # distributions to publish, findDistros returns no distributions
+        # (but it does return normally).
+        self.assertContentEqual(
+            [], self.makeScript(all_derived=True).findDistros())
+
+    def test_findDistros_for_all_derived_finds_derived_distros(self):
+        # If --all-derived is given, findDistros finds all derived
+        # distributions.
+        dsp = self.factory.makeDistroSeriesParent()
+        self.assertContentEqual(
+            [dsp.derived_series.distribution],
+            self.makeScript(all_derived=True).findDistros())
+
+    def test_findDistros_for_all_derived_ignores_ubuntu(self):
+        # The --all-derived option does not include Ubuntu, even if it
+        # is itself a derived distribution.
+        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+        self.factory.makeDistroSeriesParent(
+            parent_series=ubuntu.currentseries)
+        self.assertNotIn(
+            ubuntu, self.makeScript(all_derived=True).findDistros())
+
+    def test_findDistros_for_all_derived_ignores_nonderived_distros(self):
+        self.makeDistro()
+        self.assertContentEqual(
+            [], self.makeScript(all_derived=True).findDistros())
 
     def test_findSuite_finds_release_pocket(self):
         # Despite its lack of a suffix, a release suite shows up
@@ -543,27 +608,32 @@
         distro = series.distribution
         self.assertEqual(
             (series.name, PackagePublishingPocket.RELEASE),
-            self.makeScript(distro).findSuite(series.name))
+            self.makeScript(distro).findSuite(distro, series.name))
 
     def test_findSuite_finds_other_pocket(self):
         # Suites that are not in the release pocket have their pocket
         # name as a suffix.  These show up in findSuite results.
         series = self.factory.makeDistroSeries()
         distro = series.distribution
+        script = self.makeScript(distro)
         self.assertEqual(
             (series.name, PackagePublishingPocket.UPDATES),
-            self.makeScript(distro).findSuite(series.name + "-updates"))
+            script.findSuite(distro, series.name + "-updates"))
 
     def test_findSuite_raises_if_not_found(self):
         # If findSuite can't find its suite, that's an OptionValueError.
+        distro = self.makeDistro()
+        script = self.makeScript(distro)
         self.assertRaises(
             OptionValueError,
-            self.makeScript().findSuite, self.factory.getUniqueString())
+            script.findSuite, distro, self.factory.getUniqueString())
 
     def test_findAllowedSuites_finds_nothing_if_no_suites_given(self):
         # If no suites are given, findAllowedSuites returns an empty
         # sequence.
-        self.assertContentEqual([], self.makeScript().findAllowedSuites())
+        distro = self.makeDistro()
+        script = self.makeScript(distro)
+        self.assertContentEqual([], script.findAllowedSuites(distro))
 
     def test_findAllowedSuites_finds_series_and_pocket(self):
         # findAllowedSuites looks up the requested suites.
@@ -572,7 +642,7 @@
         script = self.makeScript(series.distribution, ['--suite', suite])
         self.assertContentEqual(
             [(series.name, PackagePublishingPocket.UPDATES)],
-            script.findAllowedSuites())
+            script.findAllowedSuites(series.distribution))
 
     def test_findAllowedSuites_finds_multiple(self):
         # Multiple suites may be requested; findAllowedSuites looks them
@@ -585,206 +655,218 @@
             (series.name, PackagePublishingPocket.UPDATES),
             (series.name, PackagePublishingPocket.RELEASE),
             ]
-        self.assertContentEqual(expected_suites, script.findAllowedSuites())
+        self.assertContentEqual(
+            expected_suites, script.findAllowedSuites(series.distribution))
 
     def test_getDebugArchive_returns_list(self):
         # getDebugArchive returns a list of one archive.  Fits in more
         # regularly with the other methods to find archives.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         script = self.makeScript(distro)
         debug_archive = self.factory.makeArchive(
             distro, purpose=ArchivePurpose.DEBUG)
-        self.assertEqual([debug_archive], script.getDebugArchive())
+        self.assertEqual([debug_archive], script.getDebugArchive(distro))
 
     def test_getDebugArchive_raises_if_not_found(self):
         # If getDebugArchive doesn't find a debug archive, that's an
         # OptionValueError.
-        self.assertRaises(OptionValueError, self.makeScript().getDebugArchive)
+        distro = self.makeDistro()
+        script = self.makeScript(distro)
+        self.assertRaises(OptionValueError, script.getDebugArchive, distro)
 
     def test_getDebugArchive_ignores_other_archive_purposes(self):
         # getDebugArchive does not return archives that aren't debug
         # archives.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         script = self.makeScript(distro)
         self.factory.makeArchive(distro, purpose=ArchivePurpose.PARTNER)
-        self.assertRaises(OptionValueError, script.getDebugArchive)
+        self.assertRaises(OptionValueError, script.getDebugArchive, distro)
 
     def test_getDebugArchive_ignores_other_distros(self):
         # getDebugArchive won't return an archive for the wrong
         # distribution.
+        distro = self.makeDistro()
         self.factory.makeArchive(purpose=ArchivePurpose.DEBUG)
-        self.assertRaises(OptionValueError, self.makeScript().getDebugArchive)
+        script = self.makeScript(distro)
+        self.assertRaises(OptionValueError, script.getDebugArchive, distro)
 
     def test_getCopyArchives_returns_list(self):
         # getCopyArchives returns a list of archives.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         script = self.makeScript(distro)
         copy_archive = self.factory.makeArchive(
             distro, purpose=ArchivePurpose.COPY)
-        self.assertEqual([copy_archive], script.getCopyArchives())
+        self.assertEqual([copy_archive], script.getCopyArchives(distro))
 
     def test_getCopyArchives_raises_if_not_found(self):
         # If the distribution has no copy archives, that's a script
         # failure.
+        distro = self.makeDistro()
+        script = self.makeScript(distro)
         self.assertRaises(
-            LaunchpadScriptFailure, self.makeScript().getCopyArchives)
+            LaunchpadScriptFailure, script.getCopyArchives, distro)
 
     def test_getCopyArchives_ignores_other_archive_purposes(self):
         # getCopyArchives won't return archives that aren't copy
         # archives.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         script = self.makeScript(distro)
         self.factory.makeArchive(distro, purpose=ArchivePurpose.PARTNER)
-        self.assertRaises(LaunchpadScriptFailure, script.getCopyArchives)
+        self.assertRaises(
+            LaunchpadScriptFailure, script.getCopyArchives, distro)
 
     def test_getCopyArchives_ignores_other_distros(self):
         # getCopyArchives won't return an archive for the wrong
         # distribution.
+        distro = self.makeDistro()
+        script = self.makeScript(distro)
         self.factory.makeArchive(purpose=ArchivePurpose.COPY)
         self.assertRaises(
-            LaunchpadScriptFailure, self.makeScript().getCopyArchives)
+            LaunchpadScriptFailure, script.getCopyArchives, distro)
 
     def test_getPPAs_gets_pending_distro_PPAs_if_careful(self):
         # In careful mode, getPPAs includes PPAs for the distribution
         # that are pending pulication.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         script = self.makeScript(distro, ['--careful'])
         ppa = self.factory.makeArchive(distro, purpose=ArchivePurpose.PPA)
         self.factory.makeSourcePackagePublishingHistory(archive=ppa)
-        self.assertContentEqual([ppa], script.getPPAs())
+        self.assertContentEqual([ppa], script.getPPAs(distro))
 
     def test_getPPAs_gets_nonpending_distro_PPAs_if_careful(self):
         # In careful mode, getPPAs includes PPAs for the distribution
         # that are not pending pulication.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         script = self.makeScript(distro, ['--careful'])
         ppa = self.factory.makeArchive(distro, purpose=ArchivePurpose.PPA)
-        self.assertContentEqual([ppa], script.getPPAs())
+        self.assertContentEqual([ppa], script.getPPAs(distro))
 
     def test_getPPAs_gets_pending_distro_PPAs_if_not_careful(self):
         # In non-careful mode, getPPAs includes PPAs that are pending
         # pulication.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         script = self.makeScript(distro)
         ppa = self.factory.makeArchive(distro, purpose=ArchivePurpose.PPA)
         self.factory.makeSourcePackagePublishingHistory(archive=ppa)
-        self.assertContentEqual([ppa], script.getPPAs())
+        self.assertContentEqual([ppa], script.getPPAs(distro))
 
     def test_getPPAs_ignores_nonpending_distro_PPAs_if_not_careful(self):
         # In non-careful mode, getPPAs does not include PPAs that are
         # not pending pulication.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         script = self.makeScript(distro)
         self.factory.makeArchive(distro, purpose=ArchivePurpose.PPA)
-        self.assertContentEqual([], script.getPPAs())
+        self.assertContentEqual([], script.getPPAs(distro))
 
     def test_getPPAs_returns_empty_if_careful_and_no_PPAs_found(self):
         # If, in careful mode, getPPAs finds no archives it returns an
         # empty sequence.
-        self.assertContentEqual(
-            [], self.makeScript(args=['--careful']).getPPAs())
+        distro = self.makeDistro()
+        script = self.makeScript(distro, ['--careful'])
+        self.assertContentEqual([], script.getPPAs(distro))
 
     def test_getPPAs_returns_empty_if_not_careful_and_no_PPAs_found(self):
         # If, in non-careful mode, getPPAs finds no archives it returns
         # an empty sequence.
-        self.assertContentEqual([], self.makeScript().getPPAs())
+        distro = self.makeDistro()
+        self.assertContentEqual([], self.makeScript(distro).getPPAs(distro))
 
     def test_getTargetArchives_gets_partner_archive(self):
         # If the selected exclusive option is "partner,"
         # getTargetArchives looks for a partner archive.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         partner = self.factory.makeArchive(
             distro, purpose=ArchivePurpose.PARTNER)
         script = self.makeScript(distro, ['--partner'])
-        self.assertContentEqual([partner], script.getTargetArchives())
+        self.assertContentEqual([partner], script.getTargetArchives(distro))
 
     def test_getTargetArchives_ignores_public_ppas_if_private(self):
         # If the selected exclusive option is "private-ppa,"
         # getTargetArchives looks for PPAs but leaves out public ones.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         self.factory.makeArchive(
             distro, purpose=ArchivePurpose.PPA, private=False)
         script = self.makeScript(distro, ['--private-ppa'])
-        self.assertContentEqual([], script.getTargetArchives())
+        self.assertContentEqual([], script.getTargetArchives(distro))
 
     def test_getTargetArchives_gets_private_ppas_if_private(self):
         # If the selected exclusive option is "private-ppa,"
         # getTargetArchives looks for private PPAs.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         ppa = self.factory.makeArchive(
             distro, purpose=ArchivePurpose.PPA, private=True)
         script = self.makeScript(distro, ['--private-ppa', '--careful'])
-        self.assertContentEqual([ppa], script.getTargetArchives())
+        self.assertContentEqual([ppa], script.getTargetArchives(distro))
 
     def test_getTargetArchives_gets_public_ppas_if_not_private(self):
         # If the selected exclusive option is "ppa," getTargetArchives
         # looks for public PPAs.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         ppa = self.factory.makeArchive(
             distro, purpose=ArchivePurpose.PPA, private=False)
         script = self.makeScript(distro, ['--ppa', '--careful'])
-        self.assertContentEqual([ppa], script.getTargetArchives())
+        self.assertContentEqual([ppa], script.getTargetArchives(distro))
 
     def test_getTargetArchives_ignores_private_ppas_if_not_private(self):
         # If the selected exclusive option is "ppa," getTargetArchives
         # leaves out private PPAs.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         self.factory.makeArchive(
             distro, purpose=ArchivePurpose.PPA, private=True)
         script = self.makeScript(distro, ['--ppa'])
-        self.assertContentEqual([], script.getTargetArchives())
+        self.assertContentEqual([], script.getTargetArchives(distro))
 
     def test_getTargetArchives_gets_primary_debug_archive(self):
         # If the selected exclusive option is "primary-debug,"
         # getTargetArchives looks for a debug archive.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         debug = self.factory.makeArchive(distro, purpose=ArchivePurpose.DEBUG)
         script = self.makeScript(distro, ['--primary-debug'])
-        self.assertContentEqual([debug], script.getTargetArchives())
+        self.assertContentEqual([debug], script.getTargetArchives(distro))
 
     def test_getTargetArchives_gets_copy_archives(self):
         # If the selected exclusive option is "copy-archive,"
         # getTargetArchives looks for a copy archive.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         copy = self.factory.makeArchive(distro, purpose=ArchivePurpose.COPY)
         script = self.makeScript(distro, ['--copy-archive'])
-        self.assertContentEqual([copy], script.getTargetArchives())
+        self.assertContentEqual([copy], script.getTargetArchives(distro))
 
     def test_getPublisher_returns_publisher(self):
         # getPublisher produces a Publisher instance.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         script = self.makeScript(distro)
-        publisher = script.getPublisher(distro.main_archive, None)
+        publisher = script.getPublisher(distro, distro.main_archive, None)
         self.assertIsInstance(publisher, Publisher)
 
     def test_deleteArchive_deletes_ppa(self):
         # If fed a PPA, deleteArchive will properly delete it (and
         # return True to indicate it's done something that needs
         # committing).
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         ppa = self.factory.makeArchive(distro, purpose=ArchivePurpose.PPA)
         script = self.makeScript(distro)
         deletion_done = script.deleteArchive(
-            ppa, script.getPublisher(ppa, []))
+            ppa, script.getPublisher(distro, ppa, []))
         self.assertTrue(deletion_done)
-        self.assertContentEqual([], script.getPPAs())
+        self.assertContentEqual([], script.getPPAs(distro))
 
     def test_deleteArchive_ignores_non_ppa(self):
         # If fed an archive that's not a PPA, deleteArchive will do
         # nothing and return False to indicate the fact.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         archive = self.factory.makeArchive(
             distro, purpose=ArchivePurpose.DEBUG)
         script = self.makeScript(distro)
         deletion_done = script.deleteArchive(archive, None)
         self.assertFalse(deletion_done)
-        self.assertContentEqual([archive], script.getDebugArchive())
+        self.assertContentEqual([archive], script.getDebugArchive(distro))
 
     def test_publishArchive_drives_publisher(self):
         # publishArchive puts a publisher through its paces.  This work
         # ought to be in the publisher itself, so if you find this way
         # of doing things annoys you, that's your cue to help clean up!
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         script = self.makeScript(distro)
         script.txn = FakeTransaction()
         publisher = FakePublisher()
@@ -799,7 +881,7 @@
         # For some types of archive, publishArchive invokes the
         # publisher's C_doFTPArchive method as a way of generating
         # indexes.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         script = self.makeScript(distro)
         script.txn = FakeTransaction()
         publisher = FakePublisher()
@@ -811,10 +893,34 @@
         # For some types of archive, publishArchive invokes the
         # publisher's C_writeIndexes as an alternative to
         # C_doFTPArchive.
-        distro = self.factory.makeDistribution()
+        distro = self.makeDistro()
         script = self.makeScript(distro)
         script.txn = FakeTransaction()
         publisher = FakePublisher()
         script.publishArchive(FakeArchive(ArchivePurpose.PPA), publisher)
         self.assertEqual(0, publisher.C_doFTPArchive.call_count)
         self.assertEqual(1, publisher.C_writeIndexes.call_count)
+
+    def test_publishes_only_selected_archives(self):
+        # The script publishes only the archives returned by
+        # getTargetArchives, for the distributions returned by
+        # findDistros.
+        distro = self.makeDistro()
+        # The script gets a distribution and archive of its own, to
+        # prove that any distros and archives other than what
+        # findDistros and getTargetArchives return are ignored.
+        script = self.makeScript()
+        script.txn = FakeTransaction()
+        script.findDistros = FakeMethod([distro])
+        archive = FakeArchive()
+        script.getTargetArchives = FakeMethod([archive])
+        publisher = FakePublisher()
+        script.getPublisher = FakeMethod(publisher)
+        script.publishArchive = FakeMethod()
+        script.main()
+        [(args, kwargs)] = script.getPublisher.calls
+        distro_arg, archive_arg = args[:2]
+        self.assertEqual(distro, distro_arg)
+        self.assertEqual(archive, archive_arg)
+        self.assertEqual(
+            [((archive, publisher), {})], script.publishArchive.calls)

=== modified file 'lib/lp/soyuz/tests/test_processaccepted.py'
--- lib/lp/soyuz/tests/test_processaccepted.py	2011-07-14 16:54:41 +0000
+++ lib/lp/soyuz/tests/test_processaccepted.py	2011-07-21 13:04:29 +0000
@@ -4,7 +4,6 @@
 """Test process-accepted.py"""
 
 from cStringIO import StringIO
-from zope.component import getUtility
 
 from canonical.launchpad.interfaces.lpstorm import IStore
 from debian.deb822 import Changes
@@ -14,7 +13,6 @@
 from canonical.config import config
 from canonical.launchpad.webapp.errorlog import ErrorReportingUtility
 from canonical.testing.layers import LaunchpadZopelessLayer
-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.log.logger import BufferLogger
 from lp.services.scripts.base import LaunchpadScriptFailure
@@ -223,32 +221,7 @@
         dsp = self.factory.makeDistroSeriesParent()
         script = ProcessAccepted(test_args=['--derived'])
         self.assertIn(
-            dsp.derived_series.distribution, script.findDerivedDistros())
-
-    def test_findDerivedDistros_for_derived_ignores_non_derived_distros(self):
-        distro = self.factory.makeDistribution()
-        script = ProcessAccepted(test_args=['--derived'])
-        self.assertNotIn(distro, script.findDerivedDistros())
-
-    def test_findDerivedDistros_ignores_ubuntu_even_if_derived(self):
-        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
-        self.factory.makeDistroSeriesParent(
-            derived_series=ubuntu.currentseries)
-        script = ProcessAccepted(test_args=['--derived'])
-        self.assertNotIn(ubuntu, script.findDerivedDistros())
-
-    def test_findDerivedDistros_finds_each_distro_just_once(self):
-        # Derived distros are not duplicated in the output of
-        # findDerivedDistros, even if they have multiple parents and
-        # multiple derived series.
-        dsp = self.factory.makeDistroSeriesParent()
-        distro = dsp.derived_series.distribution
-        other_series = self.factory.makeDistroSeries(distribution=distro)
-        self.factory.makeDistroSeriesParent(derived_series=other_series)
-        script = ProcessAccepted(test_args=['--derived'])
-        derived_distros = list(script.findDerivedDistros())
-        self.assertEqual(
-            1, derived_distros.count(dsp.derived_series.distribution))
+            dsp.derived_series.distribution, script.findTargetDistros())
 
 
 class TestBugsFromChangesFile(TestCaseWithFactory):