launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04298
[Merge] lp:~jtv/launchpad/pre-788078 into lp:launchpad
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/pre-788078 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #189866 in Launchpad itself: "publish-distro should be a LaunchpadCronScript"
https://bugs.launchpad.net/launchpad/+bug/189866
For more details, see:
https://code.launchpad.net/~jtv/launchpad/pre-788078/+merge/68374
= Summary =
The publish-distro script needs to be cleaned up before we can sensibly extend it further. Bug 189866 says that it should also be turned into a proper LaunchpadScript; that alone saves a lot of boilerplate. The bug has been open for well over 3 years, and meanwhile we've merely continued to grow the hairball of code.
== Proposed fix ==
This turns the script into a proper LaunchpadCronScript, and adds fine-grained tests.
== Implementation details ==
We could conceivably drop more of the high-level testing, but especially with big refactorings I wanted to be conservative. Running the script body with multiple commits lots of times just to see that the script accepted the right options was going a bit far though; since it's isolated now I was able to test it at a much finer granularity.
== Tests ==
{{{
./bin/test -vvc lp.soyuz.scripts.tests.test_publishdistro
./bin/test -vvc lp.soyuz -t soyuz-upload.txt -t soyuz-set-of-uploads.txt
}}}
== Demo and Q/A ==
The publish-distro script should still work; we can test that on dogfood. The publish-ftpmaster script also invokes publish-distro so we ought to test that as well.
= Launchpad lint =
I cleaned up some of it but there's still some left that I can't do much about:
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
scripts/publish-distro.py
lib/lp/archivepublisher/scripts/publish_ftpmaster.py
lib/lp/soyuz/doc/soyuz-upload.txt
lib/lp/soyuz/doc/soyuz-set-of-uploads.txt
./scripts/publish-distro.py
8: '_pythonpath' imported but unused
./lib/lp/soyuz/doc/soyuz-upload.txt
522: want exceeds 78 characters.
523: want exceeds 78 characters.
524: want exceeds 78 characters.
620: want exceeds 78 characters.
621: want exceeds 78 characters.
622: want exceeds 78 characters.
684: want exceeds 78 characters.
687: want exceeds 78 characters.
712: want exceeds 78 characters.
713: want exceeds 78 characters.
714: want exceeds 78 characters.
715: want exceeds 78 characters.
--
https://code.launchpad.net/~jtv/launchpad/pre-788078/+merge/68374
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/pre-788078 into lp:launchpad.
=== modified file 'lib/lp/archivepublisher/scripts/publish_ftpmaster.py'
--- lib/lp/archivepublisher/scripts/publish_ftpmaster.py 2011-05-09 08:48:12 +0000
+++ lib/lp/archivepublisher/scripts/publish_ftpmaster.py 2011-07-19 12:44:38 +0000
@@ -9,7 +9,6 @@
]
from datetime import datetime
-from optparse import OptionParser
import os
from pytz import utc
from zope.component import getUtility
@@ -26,7 +25,7 @@
from lp.registry.interfaces.series import SeriesStatus
from lp.services.utils import file_exists
from lp.soyuz.enums import ArchivePurpose
-from lp.soyuz.scripts import publishdistro
+from lp.soyuz.scripts.publishdistro import PublishDistro
from lp.soyuz.scripts.ftpmaster import LpQueryDistro
from lp.soyuz.scripts.processaccepted import ProcessAccepted
@@ -363,10 +362,10 @@
args +
sum([['-s', suite] for suite in suites], []))
- parser = OptionParser()
- publishdistro.add_options(parser)
- options, args = parser.parse_args(arguments)
- publishdistro.run_publisher(options, self.txn, log=self.logger)
+ publish_distro = PublishDistro(test_args=arguments)
+ publish_distro.logger = self.logger
+ publish_distro.txn = self.txn
+ publish_distro.main()
def publishDistroArchive(self, archive, security_suites=None):
"""Publish the results for an archive.
=== modified file 'lib/lp/soyuz/doc/soyuz-set-of-uploads.txt'
--- lib/lp/soyuz/doc/soyuz-set-of-uploads.txt 2011-06-28 15:04:29 +0000
+++ lib/lp/soyuz/doc/soyuz-set-of-uploads.txt 2011-07-19 12:44:38 +0000
@@ -710,11 +710,15 @@
>>> fillLibrarianFile(70)
>>> run_publish_distro(careful=True)
- DEBUG Initializing zopeless.
+ DEBUG Enabled by DEFAULT section
DEBUG Distribution: ubuntutest
...
- DEBUG Added /var/tmp/archive/ubuntutest/pool/universe/b/bar/bar_1.0-2_i386.deb from library
- DEBUG Added /var/tmp/archive/ubuntutest/pool/universe/b/bar/bar_1.0-1_i386.deb from library
+ DEBUG Added
+ /var/tmp/archive/ubuntutest/pool/universe/b/bar/bar_1.0-2_i386.deb from
+ library
+ DEBUG Added
+ /var/tmp/archive/ubuntutest/pool/universe/b/bar/bar_1.0-1_i386.deb from
+ library
...
@@ -739,10 +743,11 @@
uploaded in the test above don't break the assumptions of this test.
>>> run_publish_distro(careful_publishing=True)
- DEBUG Initializing zopeless.
+ DEBUG Enabled by DEFAULT section
DEBUG Distribution: ubuntutest
...
- DEBUG /var/tmp/archive/ubuntutest/pool/universe/b/bar/bar_1.0-2_i386.deb is already in pool with the same content.
+ DEBUG /var/tmp/archive/ubuntutest/pool/universe/b/bar/bar_1.0-2_i386.deb
+ is already in pool with the same content.
...
DEBUG Skipping a-f stanza for breezy-autotest/RELEASE
...
=== modified file 'lib/lp/soyuz/doc/soyuz-upload.txt'
--- lib/lp/soyuz/doc/soyuz-upload.txt 2011-06-16 08:50:52 +0000
+++ lib/lp/soyuz/doc/soyuz-upload.txt 2011-07-19 12:44:38 +0000
@@ -517,7 +517,7 @@
>>> print stdout
<BLANKLINE>
>>> print stderr
- DEBUG Initializing zopeless.
+ DEBUG Enabled by DEFAULT section
DEBUG Distribution: ubuntutest
...
DEBUG Added /var/tmp/archive/ubuntutest/pool/universe/e/etherwake/etherwake_1.08.orig.tar.gz from library
@@ -615,7 +615,7 @@
already on disk, verify the contents are as expected.
>>> print stderr
- DEBUG Initializing zopeless.
+ DEBUG Enabled by DEFAULT section
DEBUG Distribution: ubuntutest
...
DEBUG /var/tmp/archive/ubuntutest/pool/universe/e/etherwake/etherwake_1.08.orig.tar.gz is already in pool with the same content.
=== modified file 'lib/lp/soyuz/scripts/publishdistro.py'
--- lib/lp/soyuz/scripts/publishdistro.py 2011-04-27 08:20:37 +0000
+++ lib/lp/soyuz/scripts/publishdistro.py 2011-07-19 12:44:38 +0000
@@ -1,23 +1,22 @@
# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-"""Publisher script functions."""
+"""Publisher script class."""
__all__ = [
- 'add_options',
- 'run_publisher',
+ 'PublishDistro',
]
+from optparse import OptionValueError
from zope.component import getUtility
-from canonical.launchpad.scripts import (
- logger,
- logger_options,
- )
from lp.app.errors import NotFoundError
from lp.archivepublisher.publishing import getPublisher
from lp.registry.interfaces.distribution import IDistributionSet
-from lp.services.scripts.base import LaunchpadScriptFailure
+from lp.services.scripts.base import (
+ LaunchpadCronScript,
+ LaunchpadScriptFailure,
+ )
from lp.soyuz.enums import (
ArchivePurpose,
ArchiveStatus,
@@ -27,194 +26,289 @@
MAIN_ARCHIVE_PURPOSES,
)
-# XXX Julian 2008-02-07 bug=189866:
-# These functions should be in a LaunchpadScript.
-
-def add_options(parser):
- logger_options(parser)
-
- parser.add_option("-C", "--careful", action="store_true",
- dest="careful", metavar="", default=False,
- help="Turns on all the below careful options.")
-
- parser.add_option("-P", "--careful-publishing", action="store_true",
- dest="careful_publishing", metavar="", default=False,
- help="Make the package publishing process careful.")
-
- parser.add_option("-D", "--careful-domination", action="store_true",
- dest="careful_domination", metavar="", default=False,
- help="Make the domination process careful.")
-
- parser.add_option("-A", "--careful-apt", action="store_true",
- dest="careful_apt", metavar="", default=False,
- help="Make index generation (e.g. apt-ftparchive) careful.")
-
- parser.add_option("-d", "--distribution",
- dest="distribution", metavar="DISTRO", default="ubuntu",
- help="The distribution to publish.")
-
- parser.add_option('-s', '--suite', metavar='SUITE', dest='suite',
- action='append', type='string', default=[],
- help='The suite to publish')
-
- parser.add_option("-R", "--distsroot",
- dest="distsroot", metavar="SUFFIX", default=None,
- help="Override the dists path for generation of the "
- "PRIMARY and PARTNER archives only.")
-
- parser.add_option("--ppa", action="store_true",
- dest="ppa", metavar="PPA", default=False,
- help="Only run over PPA archives.")
-
- parser.add_option("--private-ppa", action="store_true",
- dest="private_ppa", metavar="PRIVATEPPA", default=False,
- help="Only run over private PPA archives.")
-
- parser.add_option("--partner", action="store_true",
- dest="partner", metavar="PARTNER", default=False,
- help="Only run over the partner archive.")
-
- parser.add_option("--copy-archive", action="store_true",
- dest="copy_archive", metavar="COPYARCHIVE",
- default=False,
- help="Only run over the copy archives.")
-
- parser.add_option(
- "--primary-debug", action="store_true", default=False,
- dest="primary_debug", metavar="PRIMARYDEBUG",
- help="Only run over the debug-symbols for primary archive.")
-
-
-def run_publisher(options, txn, log=None):
- if not log:
- log = logger(options, "publish-distro")
-
- def careful_msg(what):
- """Quick handy util for the below."""
- if options.careful:
+
+def is_ppa_private(ppa):
+ """Is `ppa` private?"""
+ return ppa.private
+
+
+def is_ppa_public(ppa):
+ """Is `ppa` public?"""
+ return not ppa.private
+
+
+class PublishDistro(LaunchpadCronScript):
+ """Distro publisher."""
+
+ def add_my_options(self):
+ self.parser.add_option(
+ "-C", "--careful", action="store_true", dest="careful",
+ default=False, help="Turns on all the below careful options.")
+
+ self.parser.add_option(
+ "-P", "--careful-publishing", action="store_true",
+ dest="careful_publishing", default=False,
+ help="Make the package publishing process careful.")
+
+ self.parser.add_option(
+ "-D", "--careful-domination", action="store_true",
+ dest="careful_domination", default=False,
+ help="Make the domination process careful.")
+
+ self.parser.add_option(
+ "-A", "--careful-apt", action="store_true", dest="careful_apt",
+ default=False,
+ help="Make index generation (e.g. apt-ftparchive) careful.")
+
+ self.parser.add_option(
+ "-d", "--distribution", dest="distribution", metavar="DISTRO",
+ default="ubuntu", help="The distribution to publish.")
+
+ self.parser.add_option(
+ '-s', '--suite', metavar='SUITE', dest='suite', action='append',
+ type='string', default=[], help='The suite to publish')
+
+ self.parser.add_option(
+ "-R", "--distsroot", dest="distsroot", metavar="SUFFIX",
+ default=None,
+ help=(
+ "Override the dists path for generation of the PRIMARY and "
+ "PARTNER archives only."))
+
+ self.parser.add_option(
+ "--ppa", action="store_true", dest="ppa", default=False,
+ help="Only run over PPA archives.")
+
+ self.parser.add_option(
+ "--private-ppa", action="store_true", dest="private_ppa",
+ default=False, help="Only run over private PPA archives.")
+
+ self.parser.add_option(
+ "--partner", action="store_true", dest="partner", default=False,
+ help="Only run over the partner archive.")
+
+ self.parser.add_option(
+ "--copy-archive", action="store_true", dest="copy_archive",
+ default=False, help="Only run over the copy archives.")
+
+ self.parser.add_option(
+ "--primary-debug", action="store_true", default=False,
+ dest="primary_debug",
+ help="Only run over the debug-symbols for primary archive.")
+
+ def isCareful(self, option):
+ """Is the given "carefulness" option enabled?
+
+ Yes if the option is True, but also if the global "careful" option
+ is set.
+
+ :param option: The specific "careful" option to test, e.g.
+ `self.options.careful_publishing`.
+ :return: Whether the option should be treated as asking us to be
+ careful.
+ """
+ return option or self.options.careful
+
+ def describeCare(self, option):
+ """Helper: describe carefulness setting of given option.
+
+ Produces a human-readable string saying whether the option is set
+ to careful mode; or "overridden" to careful mode by the global
+ "careful" option; or is left in normal mode.
+ """
+ if self.options.careful:
return "Careful (Overridden)"
- if what:
+ elif option:
return "Careful"
- return "Normal"
-
- exclusive_options = (
- options.partner, options.ppa, options.private_ppa,
- options.primary_debug, options.copy_archive)
-
- num_exclusive = [flag for flag in exclusive_options if flag]
- if len(num_exclusive) > 1:
- raise LaunchpadScriptFailure(
- "Can only specify one of partner, ppa, private-ppa, copy-archive"
- " and primary-debug.")
-
- log.debug(" Distribution: %s" % options.distribution)
- log.debug(" Publishing: %s" % careful_msg(options.careful_publishing))
- log.debug(" Domination: %s" % careful_msg(options.careful_domination))
- if num_exclusive == 0:
- log.debug("Apt-FTPArchive: %s" % careful_msg(options.careful_apt))
- else:
- log.debug(" Indexing: %s" % careful_msg(options.careful_apt))
-
- log.debug("Finding distribution object.")
-
- try:
- distribution = getUtility(IDistributionSet).getByName(
- options.distribution)
- except NotFoundError, info:
- raise LaunchpadScriptFailure(info)
-
- allowed_suites = set()
- for suite in options.suite:
+ else:
+ return "Normal"
+
+ def logOption(self, name, value):
+ """Describe the state of `option` to the debug log."""
+ self.logger.debug("%14s: %s", name, value)
+
+ def countExclusiveOptions(self):
+ """Return the number of exclusive "mode" options that were set.
+
+ In valid use, at most one of them should be set.
+ """
+ exclusive_options = [
+ self.options.partner,
+ self.options.ppa,
+ self.options.private_ppa,
+ self.options.primary_debug,
+ self.options.copy_archive,
+ ]
+ return len(filter(None, exclusive_options))
+
+ def logOptions(self):
+ """Dump the selected options to the debug log."""
+ if self.countExclusiveOptions() == 0:
+ indexing_engine = "Apt-FTPArchive"
+ else:
+ indexing_engine = "Indexing"
+ self.logOption('Distribution', self.options.distribution)
+ log_items = [
+ ('Publishing', self.options.careful_publishing),
+ ('Domination', self.options.careful_domination),
+ (indexing_engine, self.options.careful_apt),
+ ]
+ for name, option in log_items:
+ self.logOption(name, self.describeCare(option))
+
+ def validateOptions(self):
+ """Check given options for user interface violations."""
+ if len(self.args) > 0:
+ raise OptionValueError(
+ "publish-distro takes no arguments, only options.")
+ if self.countExclusiveOptions() > 1:
+ raise OptionValueError(
+ "Can only specify one of partner, ppa, private-ppa, "
+ "copy-archive and primary-debug.")
+
+ 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."""
+ self.logger.debug("Finding distribution object.")
+ name = self.options.distribution
+ distro = getUtility(IDistributionSet).getByName(name)
+ if distro is None:
+ raise OptionValueError("Distribution '%s' not found." % name)
+ return distro
+
+ def findSuite(self, 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:
- distroseries, pocket = distribution.getDistroSeriesAndPocket(
- suite)
- except NotFoundError, info:
- raise LaunchpadScriptFailure(info)
- allowed_suites.add((distroseries.name, pocket))
-
- if options.partner:
- archives = [distribution.getArchiveByComponent('partner')]
- elif options.ppa or options.private_ppa:
- if options.careful or options.careful_publishing:
- archives = distribution.getAllPPAs()
- else:
- archives = distribution.getPendingPublicationPPAs()
-
- # Filter out non-private if we're publishing private PPAs only,
- # or filter out private if we're doing non-private.
- if options.private_ppa:
- archives = [archive for archive in archives if archive.private]
- else:
- archives = [
- archive for archive in archives if not archive.private]
-
- if options.distsroot is not None:
- raise LaunchpadScriptFailure(
- "We should not define 'distsroot' in PPA mode !")
- elif options.primary_debug:
+ series, pocket = self.distribution.getDistroSeriesAndPocket(suite)
+ except NotFoundError, e:
+ raise OptionValueError(e)
+ return series.name, pocket
+
+ def findAllowedSuites(self):
+ """Find the selected suite(s)."""
+ return set([self.findSuite(suite) for suite in self.options.suite])
+
+ def getDebugArchive(self):
+ """Find the debug archive for the selected distribution, as a list."""
debug_archive = getUtility(IArchiveSet).getByDistroPurpose(
- distribution, ArchivePurpose.DEBUG)
+ self.distribution, ArchivePurpose.DEBUG)
if debug_archive is None:
- raise LaunchpadScriptFailure(
- "Could not find DEBUG archive for %s" % distribution.name)
- archives = [debug_archive]
- elif options.copy_archive:
- archives = getUtility(IArchiveSet).getArchivesForDistribution(
- distribution, purposes=[ArchivePurpose.COPY])
- if not bool(archives):
+ raise OptionValueError(
+ "Could not find DEBUG archive for %s"
+ % self.distribution.name)
+ return [debug_archive]
+
+ def getCopyArchives(self):
+ """Find copy archives for the selected distribution."""
+ copy_archives = list(
+ getUtility(IArchiveSet).getArchivesForDistribution(
+ self.distribution, purposes=[ArchivePurpose.COPY]))
+ if copy_archives == []:
raise LaunchpadScriptFailure("Could not find any COPY archives")
- else:
- archives = [distribution.main_archive]
-
- # Consider only archives that have their "to be published" flag turned on
- # or are pending deletion.
- archives = [
- archive for archive in archives
- if archive.publish or archive.status == ArchiveStatus.DELETING]
-
- for archive in archives:
+ return copy_archives
+
+ def getPPAs(self):
+ """Find private package archives for the selected distribution."""
+ if self.isCareful(self.options.careful_publishing):
+ return self.distribution.getAllPPAs()
+ else:
+ return self.distribution.getPendingPublicationPPAs()
+
+ def getTargetArchives(self):
+ """Find the archive(s) selected by the script's options."""
+ if self.options.partner:
+ return [self.distribution.getArchiveByComponent('partner')]
+ elif self.options.ppa:
+ return filter(is_ppa_public, self.getPPAs())
+ elif self.options.private_ppa:
+ return filter(is_ppa_private, self.getPPAs())
+ elif self.options.primary_debug:
+ return self.getDebugArchive()
+ elif self.options.copy_archive:
+ return self.getCopyArchives()
+ else:
+ return [self.distribution.main_archive]
+
+ def getPublisher(self, archive, allowed_suites):
+ """Get a publisher for the given options."""
if archive.purpose in MAIN_ARCHIVE_PURPOSES:
- log.info(
- "Processing %s %s" % (distribution.name, archive.displayname))
+ description = "%s %s" % (
+ self.distribution.name,
+ archive.displayname)
# Only let the primary/partner archives override the distsroot.
- publisher = getPublisher(
- archive, allowed_suites, log, options.distsroot)
- else:
- log.info("Processing %s" % archive.archive_url)
- publisher = getPublisher(archive, allowed_suites, log)
-
- # Do we need to delete the archive or publish it?
- if archive.status == ArchiveStatus.DELETING:
- if archive.purpose == ArchivePurpose.PPA:
- publisher.deleteArchive()
- txn.commit()
- else:
- # Other types of archives do not currently support deletion.
- log.warning(
- "Deletion of %s skipped: operation not supported on %s"
- % archive.displayname)
- else:
- publisher.A_publish(options.careful or options.careful_publishing)
- txn.commit()
-
- # Flag dirty pockets for any outstanding deletions.
- publisher.A2_markPocketsWithDeletionsDirty()
- publisher.B_dominate(
- options.careful or options.careful_domination)
- txn.commit()
-
- # The primary and copy archives use apt-ftparchive to generate the
- # indexes, everything else uses the newer internal LP code.
- if archive.purpose in (ArchivePurpose.PRIMARY, ArchivePurpose.COPY):
- publisher.C_doFTPArchive(
- options.careful or options.careful_apt)
- else:
- publisher.C_writeIndexes(
- options.careful or options.careful_apt)
- txn.commit()
-
- publisher.D_writeReleaseFiles(
- options.careful or options.careful_apt)
- txn.commit()
-
- log.debug("Ciao")
+ distsroot = self.options.distsroot
+ else:
+ description = archive.archive_url
+ distsroot = None
+
+ self.logger.info("Processing %s", description)
+ return getPublisher(archive, allowed_suites, self.logger, distsroot)
+
+ def deleteArchive(self, archive, publisher):
+ """Ask `publisher` to delete `archive`."""
+ if archive.purpose == ArchivePurpose.PPA:
+ publisher.deleteArchive()
+ return True
+ else:
+ # Other types of archives do not currently support deletion.
+ self.logger.warning(
+ "Deletion of %s skipped: operation not supported on %s",
+ archive.displayname, archive.purpose.title)
+ return False
+
+ def publishArchive(self, archive, publisher):
+ """Ask `publisher` to publish `archive`.
+
+ Commits transactions along the way.
+ """
+ publisher.A_publish(self.isCareful(self.options.careful_publishing))
+ self.txn.commit()
+
+ # Flag dirty pockets for any outstanding deletions.
+ publisher.A2_markPocketsWithDeletionsDirty()
+ publisher.B_dominate(self.isCareful(self.options.careful_domination))
+ self.txn.commit()
+
+ # The primary and copy archives use apt-ftparchive to
+ # generate the indexes, everything else uses the newer
+ # internal LP code.
+ careful_indexing = self.isCareful(self.options.careful_apt)
+ if archive.purpose in (ArchivePurpose.PRIMARY, ArchivePurpose.COPY):
+ publisher.C_doFTPArchive(careful_indexing)
+ else:
+ publisher.C_writeIndexes(careful_indexing)
+ self.txn.commit()
+
+ publisher.D_writeReleaseFiles(careful_indexing)
+ # The caller will commit this last step.
+
+ def main(self):
+ """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()
+
+ self.logger.debug("Ciao")
=== modified file 'lib/lp/soyuz/scripts/tests/test_publishdistro.py'
--- lib/lp/soyuz/scripts/tests/test_publishdistro.py 2011-03-15 14:46:27 +0000
+++ lib/lp/soyuz/scripts/tests/test_publishdistro.py 2011-07-19 12:44:38 +0000
@@ -1,37 +1,42 @@
-# 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).
"""Functional tests for publish-distro.py script."""
__metaclass__ = type
-from optparse import OptionParser
+from optparse import OptionValueError
import os
import shutil
import subprocess
import sys
-import unittest
-
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
from canonical.config import config
+from canonical.testing.layers import ZopelessDatabaseLayer
from lp.archivepublisher.config import getPubConfig
from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
+from lp.archivepublisher.publishing import Publisher
from lp.registry.interfaces.distribution import IDistributionSet
from lp.registry.interfaces.person import IPersonSet
-from lp.services.log.logger import BufferLogger
+from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.log.logger import (
+ BufferLogger,
+ DevNullLogger,
+ )
from lp.services.scripts.base import LaunchpadScriptFailure
from lp.soyuz.enums import (
ArchivePurpose,
BinaryPackageFormat,
PackagePublishingStatus,
)
-from lp.soyuz.interfaces.archive import (
- IArchiveSet,
- )
-from lp.soyuz.scripts import publishdistro
+from lp.soyuz.interfaces.archive import IArchiveSet
+from lp.soyuz.scripts.publishdistro import PublishDistro
from lp.soyuz.tests.test_publishing import TestNativePublishingBase
+from lp.testing import TestCaseWithFactory
+from lp.testing.fakemethod import FakeMethod
+from lp.testing.faketransaction import FakeTransaction
class TestPublishDistro(TestNativePublishingBase):
@@ -46,12 +51,11 @@
args = ["-d", distribution]
if extra_args is not None:
args.extend(extra_args)
- parser = OptionParser()
- publishdistro.add_options(parser)
- options, args = parser.parse_args(args=args)
+ publish_distro = PublishDistro(test_args=args)
+ publish_distro.logger = BufferLogger()
+ publish_distro.txn = self.layer.txn
self.layer.switchDbUser(config.archivepublisher.dbuser)
- publishdistro.run_publisher(
- options, self.layer.txn, log=BufferLogger())
+ publish_distro.main()
self.layer.switchDbUser('launchpad')
def runPublishDistroScript(self):
@@ -250,7 +254,7 @@
private=True, distribution=ubuntutest)
# Publish something to the private PPA:
- pub_source = self.getPubSource(
+ pub_source = self.getPubSource(
sourcename='baz', filecontent='baz', archive=private_ppa)
self.layer.txn.commit()
@@ -271,7 +275,7 @@
# 'ubuntutest' (default testing distribution) has no DEBUG
# archive, Thus an error is raised.
self.assertRaises(
- LaunchpadScriptFailure,
+ OptionValueError,
self.runPublishDistro, ['--primary-debug'])
# The DEBUG repository path was not created.
@@ -338,7 +342,7 @@
removeSecurityProxy(copy_archive).publish = True
# Publish something.
- pub_source = self.getPubSource(
+ pub_source = self.getPubSource(
sourcename='baz', filecontent='baz', archive=copy_archive)
# Try a plain PPA run, to ensure the copy archive is not published.
@@ -391,37 +395,330 @@
"%s/hoary-test/main/binary-i386/Packages" % self.config.distsroot)
self.assertNotExists(index_path)
- def testExclusiveOptions(self):
- """Test that some command line options are mutually exclusive."""
- self.assertRaises(
- LaunchpadScriptFailure,
- self.runPublishDistro,
- ['--ppa', '--partner', '--primary-debug', '--copy-archive'])
- self.assertRaises(
- LaunchpadScriptFailure,
- self.runPublishDistro, ['--ppa', '--partner'])
- self.assertRaises(
- LaunchpadScriptFailure,
- self.runPublishDistro, ['--ppa', '--private-ppa'])
- self.assertRaises(
- LaunchpadScriptFailure,
- self.runPublishDistro, ['--ppa', '--primary-debug'])
- self.assertRaises(
- LaunchpadScriptFailure,
- self.runPublishDistro, ['--ppa', '--copy-archive'])
- self.assertRaises(
- LaunchpadScriptFailure,
- self.runPublishDistro, ['--partner', '--private-ppa'])
- self.assertRaises(
- LaunchpadScriptFailure,
- self.runPublishDistro, ['--partner', '--primary-debug'])
- self.assertRaises(
- LaunchpadScriptFailure,
- self.runPublishDistro, ['--partner', '--copy-archive'])
- self.assertRaises(
- LaunchpadScriptFailure,
- self.runPublishDistro, ['--primary-debug', '--copy-archive'])
-
-
-def test_suite():
- return unittest.TestLoader().loadTestsFromName(__name__)
+
+class FakeArchive:
+ def __init__(self, purpose=ArchivePurpose.PRIMARY):
+ self.purpose = purpose
+
+
+class FakePublisher:
+ """A very simple fake `Publisher`."""
+ def __init__(self):
+ self.A_publish = FakeMethod()
+ self.A2_markPocketsWithDeletionsDirty = FakeMethod()
+ self.B_dominate = FakeMethod()
+ self.C_doFTPArchive = FakeMethod()
+ self.C_writeIndexes = FakeMethod()
+ self.D_writeReleaseFiles = FakeMethod()
+
+
+class TestPublishDistroMethods(TestCaseWithFactory):
+ """Fine-grained unit tests for `PublishDistro`."""
+
+ layer = ZopelessDatabaseLayer
+
+ def makeScript(self, distribution=None, args=[]):
+ """Create a `PublishDistro` for `distribution`."""
+ if distribution is None:
+ distribution = self.factory.makeDistribution()
+ full_args = ['-d', distribution.name] + args
+ script = PublishDistro(test_args=full_args)
+ script.distribution = distribution
+ script.logger = DevNullLogger()
+ return script
+
+ def test_isCareful_is_false_if_option_not_set(self):
+ self.assertFalse(self.makeScript().isCareful(False))
+
+ def test_isCareful_is_true_if_option_is_set(self):
+ self.assertTrue(self.makeScript().isCareful(True))
+
+ def test_isCareful_is_true_if_global_careful_option_is_set(self):
+ self.assertTrue(self.makeScript(args=['--careful']).isCareful(False))
+
+ def test_describeCare_reports_non_careful_option(self):
+ self.assertEqual("Normal", self.makeScript().describeCare(False))
+
+ def test_describeCare_reports_careful_option(self):
+ self.assertEqual("Careful", self.makeScript().describeCare(True))
+
+ def test_describeCare_reports_careful_override(self):
+ self.assertEqual(
+ "Careful (Overridden)",
+ self.makeScript(args=['--careful']).describeCare(False))
+
+ def test_countExclusiveOptions_is_zero_if_none_set(self):
+ self.assertEqual(0, self.makeScript().countExclusiveOptions())
+
+ def test_countExclusiveOptions_counts_partner(self):
+ self.assertEqual(
+ 1, self.makeScript(args=['--partner']).countExclusiveOptions())
+
+ def test_countExclusiveOptions_counts_ppa(self):
+ self.assertEqual(
+ 1, self.makeScript(args=['--ppa']).countExclusiveOptions())
+
+ def test_countExclusiveOptions_counts_private_ppa(self):
+ self.assertEqual(
+ 1,
+ self.makeScript(args=['--private-ppa']).countExclusiveOptions())
+
+ def test_countExclusiveOptions_counts_primary_debug(self):
+ self.assertEqual(
+ 1,
+ self.makeScript(args=['--primary-debug']).countExclusiveOptions())
+
+ def test_countExclusiveOptions_counts_copy_archive(self):
+ self.assertEqual(
+ 1,
+ self.makeScript(args=['--copy-archive']).countExclusiveOptions())
+
+ def test_countExclusiveOptions_detects_conflict(self):
+ script = self.makeScript(args=['--ppa', '--partner'])
+ self.assertEqual(2, script.countExclusiveOptions())
+
+ def test_validateOptions_rejects_nonoption_arguments(self):
+ script = self.makeScript(args=['please'])
+ self.assertRaises(OptionValueError, script.validateOptions)
+
+ def test_validateOptions_rejects_exclusive_option_conflict(self):
+ # If more than one of the exclusive options are set,
+ # validateOptions raises that as an error.
+ script = self.makeScript()
+ script.countExclusiveOptions = FakeMethod(2)
+ self.assertRaises(OptionValueError, script.validateOptions)
+
+ def test_validateOptions_does_not_accept_distsroot_for_ppa(self):
+ script = self.makeScript(args=['--ppa', '--distsroot=/tmp'])
+ self.assertRaises(OptionValueError, script.validateOptions)
+
+ def test_validateOptions_does_not_accept_distsroot_for_private_ppa(self):
+ script = self.makeScript(args=['--private-ppa', '--distsroot=/tmp'])
+ self.assertRaises(OptionValueError, script.validateOptions)
+
+ def test_findDistro_finds_distribution(self):
+ distro = self.factory.makeDistribution()
+ self.assertEqual(distro, self.makeScript(distro).findDistro())
+
+ def test_findDistro_raises_if_distro_not_found(self):
+ wrong_name = self.factory.getUniqueString()
+ self.assertRaises(
+ OptionValueError,
+ PublishDistro(test_args=['-d', wrong_name]).findDistro)
+
+ def test_findSuite_finds_release_pocket(self):
+ series = self.factory.makeDistroSeries()
+ distro = series.distribution
+ self.assertEqual(
+ (series.name, PackagePublishingPocket.RELEASE),
+ self.makeScript(distro).findSuite(series.name))
+
+ def test_findSuite_finds_other_pocket(self):
+ series = self.factory.makeDistroSeries()
+ distro = series.distribution
+ self.assertEqual(
+ (series.name, PackagePublishingPocket.UPDATES),
+ self.makeScript(distro).findSuite(series.name + "-updates"))
+
+ def test_findSuite_raises_if_not_found(self):
+ self.assertRaises(
+ OptionValueError,
+ self.makeScript().findSuite, self.factory.getUniqueString())
+
+ def test_findAllowedSuites_finds_nothing_if_no_suites_given(self):
+ self.assertContentEqual([], self.makeScript().findAllowedSuites())
+
+ def test_findAllowedSuites_finds_series_and_pocket(self):
+ series = self.factory.makeDistroSeries()
+ suite = "%s-updates" % series.name
+ script = self.makeScript(series.distribution, ['--suite', suite])
+ self.assertContentEqual(
+ [(series.name, PackagePublishingPocket.UPDATES)],
+ script.findAllowedSuites())
+
+ def test_findAllowedSuites_finds_multiple(self):
+ series = self.factory.makeDistroSeries()
+ script = self.makeScript(series.distribution, [
+ '--suite', '%s-updates' % series.name,
+ '--suite', series.name])
+ expected_suites = [
+ (series.name, PackagePublishingPocket.UPDATES),
+ (series.name, PackagePublishingPocket.RELEASE),
+ ]
+ self.assertContentEqual(expected_suites, script.findAllowedSuites())
+
+ def test_getDebugArchive_returns_list(self):
+ distro = self.factory.makeDistribution()
+ script = self.makeScript(distro)
+ debug_archive = self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.DEBUG)
+ self.assertEqual([debug_archive], script.getDebugArchive())
+
+ def test_getDebugArchive_raises_if_not_found(self):
+ self.assertRaises(OptionValueError, self.makeScript().getDebugArchive)
+
+ def test_getDebugArchive_ignores_other_archive_purposes(self):
+ distro = self.factory.makeDistribution()
+ script = self.makeScript(distro)
+ self.factory.makeArchive(distro, purpose=ArchivePurpose.PARTNER)
+ self.assertRaises(OptionValueError, script.getDebugArchive)
+
+ def test_getDebugArchive_ignores_other_distros(self):
+ self.factory.makeArchive(purpose=ArchivePurpose.DEBUG)
+ self.assertRaises(OptionValueError, self.makeScript().getDebugArchive)
+
+ def test_getCopyArchives_returns_list(self):
+ distro = self.factory.makeDistribution()
+ script = self.makeScript(distro)
+ copy_archive = self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.COPY)
+ self.assertEqual([copy_archive], script.getCopyArchives())
+
+ def test_getCopyArchives_raises_if_not_found(self):
+ self.assertRaises(
+ LaunchpadScriptFailure, self.makeScript().getCopyArchives)
+
+ def test_getCopyArchives_ignores_other_archive_purposes(self):
+ distro = self.factory.makeDistribution()
+ script = self.makeScript(distro)
+ self.factory.makeArchive(distro, purpose=ArchivePurpose.PARTNER)
+ self.assertRaises(LaunchpadScriptFailure, script.getCopyArchives)
+
+ def test_getCopyArchives_ignores_other_distros(self):
+ self.factory.makeArchive(purpose=ArchivePurpose.COPY)
+ self.assertRaises(
+ LaunchpadScriptFailure, self.makeScript().getCopyArchives)
+
+ def test_getPPAs_gets_pending_distro_PPAs_if_careful(self):
+ distro = self.factory.makeDistribution()
+ script = self.makeScript(distro, ['--careful'])
+ ppa = self.factory.makeArchive(distro, purpose=ArchivePurpose.PPA)
+ self.factory.makeSourcePackagePublishingHistory(archive=ppa)
+ self.assertContentEqual([ppa], script.getPPAs())
+
+ def test_getPPAs_gets_nonpending_distro_PPAs_if_careful(self):
+ distro = self.factory.makeDistribution()
+ script = self.makeScript(distro, ['--careful'])
+ ppa = self.factory.makeArchive(distro, purpose=ArchivePurpose.PPA)
+ self.assertContentEqual([ppa], script.getPPAs())
+
+ def test_getPPAs_gets_pending_distro_PPAs_if_not_careful(self):
+ distro = self.factory.makeDistribution()
+ script = self.makeScript(distro)
+ ppa = self.factory.makeArchive(distro, purpose=ArchivePurpose.PPA)
+ self.factory.makeSourcePackagePublishingHistory(archive=ppa)
+ self.assertContentEqual([ppa], script.getPPAs())
+
+ def test_getPPAs_ignores_nonpending_distro_PPAs_if_not_careful(self):
+ distro = self.factory.makeDistribution()
+ script = self.makeScript(distro)
+ self.factory.makeArchive(distro, purpose=ArchivePurpose.PPA)
+ self.assertContentEqual([], script.getPPAs())
+
+ def test_getPPAs_returns_empty_if_careful_and_no_PPAs_found(self):
+ self.assertContentEqual(
+ [], self.makeScript(args=['--careful']).getPPAs())
+
+ def test_getPPAs_returns_empty_if_not_careful_and_no_PPAs_found(self):
+ self.assertContentEqual([], self.makeScript().getPPAs())
+
+ def test_getTargetArchives_gets_partner_archive(self):
+ distro = self.factory.makeDistribution()
+ partner = self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PARTNER)
+ script = self.makeScript(distro, ['--partner'])
+ self.assertContentEqual([partner], script.getTargetArchives())
+
+ def test_getTargetArchives_ignores_public_ppas_if_private(self):
+ distro = self.factory.makeDistribution()
+ self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=False)
+ script = self.makeScript(distro, ['--private-ppa'])
+ self.assertContentEqual([], script.getTargetArchives())
+
+ def test_getTargetArchives_gets_private_ppas_if_private(self):
+ distro = self.factory.makeDistribution()
+ ppa = self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=True)
+ script = self.makeScript(distro, ['--private-ppa', '--careful'])
+ self.assertContentEqual([ppa], script.getTargetArchives())
+
+ def test_getTargetArchives_gets_public_ppas_if_not_private(self):
+ distro = self.factory.makeDistribution()
+ ppa = self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=False)
+ script = self.makeScript(distro, ['--ppa', '--careful'])
+ self.assertContentEqual([ppa], script.getTargetArchives())
+
+ def test_getTargetArchives_ignores_private_ppas_if_not_private(self):
+ distro = self.factory.makeDistribution()
+ self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=True)
+ script = self.makeScript(distro, ['--ppa'])
+ self.assertContentEqual([], script.getTargetArchives())
+
+ def test_getTargetArchives_gets_primary_debug_archive(self):
+ distro = self.factory.makeDistribution()
+ debug = self.factory.makeArchive(distro, purpose=ArchivePurpose.DEBUG)
+ script = self.makeScript(distro, ['--primary-debug'])
+ self.assertContentEqual([debug], script.getTargetArchives())
+
+ def test_getTargetArchives_gets_copy_archives(self):
+ distro = self.factory.makeDistribution()
+ copy = self.factory.makeArchive(distro, purpose=ArchivePurpose.COPY)
+ script = self.makeScript(distro, ['--copy-archive'])
+ self.assertContentEqual([copy], script.getTargetArchives())
+
+ def test_getPublisher_returns_publisher(self):
+ distro = self.factory.makeDistribution()
+ script = self.makeScript(distro)
+ publisher = script.getPublisher(distro.main_archive, None)
+ self.assertIsInstance(publisher, Publisher)
+
+ def test_deleteArchive_deletes_ppa(self):
+ distro = self.factory.makeDistribution()
+ ppa = self.factory.makeArchive(distro, purpose=ArchivePurpose.PPA)
+ script = self.makeScript(distro)
+ deletion_done = script.deleteArchive(
+ ppa, script.getPublisher(ppa, []))
+ self.assertTrue(deletion_done)
+ self.assertContentEqual([], script.getPPAs())
+
+ def test_deleteArchive_ignores_non_ppa(self):
+ distro = self.factory.makeDistribution()
+ 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())
+
+ def test_publishArchive_drives_publisher(self):
+ distro = self.factory.makeDistribution()
+ script = self.makeScript(distro)
+ script.txn = FakeTransaction()
+ publisher = FakePublisher()
+ script.publishArchive(FakeArchive(), publisher)
+ self.assertEqual(1, publisher.A_publish.call_count)
+ self.assertEqual(
+ 1, publisher.A2_markPocketsWithDeletionsDirty.call_count)
+ self.assertEqual(1, publisher.B_dominate.call_count)
+ self.assertEqual(1, publisher.D_writeReleaseFiles.call_count)
+
+ def test_publishArchive_uses_apt_ftparchive_for_main_archive(self):
+ distro = self.factory.makeDistribution()
+ script = self.makeScript(distro)
+ script.txn = FakeTransaction()
+ publisher = FakePublisher()
+ script.publishArchive(FakeArchive(), publisher)
+ self.assertEqual(1, publisher.C_doFTPArchive.call_count)
+ self.assertEqual(0, publisher.C_writeIndexes.call_count)
+
+ def test_publishArchive_writes_own_indexes_for_ppa(self):
+ distro = self.factory.makeDistribution()
+ 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)
=== modified file 'scripts/publish-distro.py'
--- scripts/publish-distro.py 2011-06-09 10:50:25 +0000
+++ scripts/publish-distro.py 2011-07-19 12:44:38 +0000
@@ -1,34 +1,16 @@
#!/usr/bin/python -S
#
-# 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=C0103,W0403
import _pythonpath
-from optparse import OptionParser
-
from canonical.config import config
-from canonical.launchpad.scripts import (
- execute_zcml_for_scripts, logger)
-from lp.services.scripts.base import LaunchpadScriptFailure
-from canonical.lp import initZopeless
-from lp.soyuz.scripts import publishdistro
+from lp.soyuz.scripts.publishdistro import PublishDistro
if __name__ == "__main__":
- parser = OptionParser()
- publishdistro.add_options(parser)
- options, args = parser.parse_args()
- assert len(args) == 0, "publish-distro takes no arguments, only options."
-
- log = logger(options, "publish-distro")
- log.debug("Initializing zopeless.")
- execute_zcml_for_scripts()
- txn = initZopeless(dbuser=config.archivepublisher.dbuser)
-
- try:
- publishdistro.run_publisher(options, txn)
- except LaunchpadScriptFailure, err:
- log.error(err)
+ script = PublishDistro(dbuser=config.archivepublisher.dbuser)
+ script.run()
