launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04601
[Merge] lp:~jtv/launchpad/bug-824499 into lp:launchpad
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-824499 into lp:launchpad with lp:~jtv/launchpad/pre-824499 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #824499 in Launchpad itself: ""All derived distros" option for publish-ftpmaster"
https://bugs.launchpad.net/launchpad/+bug/824499
For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-824499/+merge/71544
= Summary =
This adds an option to publish-ftpmaster to, rather than publishing a single distribution named on the command line (and let's face it, that's probably going to be Ubuntu) loops over all Ubuntu-derived distributions. That way we can (and will) set up a separate server to published derived distros, without needing admin intervention for each new distro.
== Proposed fix ==
Replace the script's self.distribution with a list, self.distributions, and loop over it. That gives us the same code path for a single distro (the existing mode of operation) and a loop over several. Next, add code for looking up the derived distros (dead easy since I've done it exactly twice before so it's in a reusable place) and checking that no more than one of the options is given.
== Pre-implementation notes ==
Julian expected me to do this as part of the same work on publish-distro, but as per Sod's Law I ended up forgetting that part. Understandable I hope, since publish-ftpmaster calls publish-distro instead of the other way around.
== Implementation details ==
The self.archives attribute is gone, and self.configs gets one extra layer of indirection. Removing self.archives also let me inline getArchives.
The ordering between setUpDirs and processAccepted has changed. I couldn't see any disadvantage to that, but it let me avoid calling setUpDirs separately for each distro. (This avoids a lot of noise in tests). Another method that stayed outside the distributions loop is recoverWorkingDists; restoring a sane working state of the filesystem now happens right up front for all distributions. The script's lock is tight enough to ensure that these directories aren't moved around later, while the loop is already running.
== Tests ==
{{{
./bin/test -vvc lp.archivepublisher.tests.test_publish_ftpmaster
}}}
== Demo and Q/A ==
Run publish-ftpmaster with the --all-derived option (instead of the usual --distribution=ubuntu). It should publish any derived distros, but not Ubuntu itself.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/archivepublisher/scripts/publish_ftpmaster.py
lib/lp/archivepublisher/tests/test_publish_ftpmaster.py
--
https://code.launchpad.net/~jtv/launchpad/bug-824499/+merge/71544
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-824499 into lp:launchpad.
=== modified file 'lib/lp/archivepublisher/scripts/publish_ftpmaster.py'
--- lib/lp/archivepublisher/scripts/publish_ftpmaster.py 2011-08-15 11:04:41 +0000
+++ lib/lp/archivepublisher/scripts/publish_ftpmaster.py 2011-08-15 11:04:41 +0000
@@ -31,12 +31,20 @@
from lp.soyuz.scripts.processaccepted import ProcessAccepted
from lp.soyuz.scripts.publishdistro import PublishDistro
-# XXX JeroenVermeulen 2011-03-31 bug=746229: to start publishing debug
-# archives, get rid of this list.
-ARCHIVES_TO_PUBLISH = [
- ArchivePurpose.PRIMARY,
- ArchivePurpose.PARTNER,
- ]
+
+def get_publishable_archives(distribution):
+ """Get all archives for `distribution` that should be published."""
+ # XXX JeroenVermeulen 2011-03-31 bug=746229: to start publishing
+ # debug archives, simply stop filtering them out here. It may be
+ # enough to return list(distribution.all_distro_archives) directly.
+ ARCHIVES_TO_PUBLISH = [
+ ArchivePurpose.PRIMARY,
+ ArchivePurpose.PARTNER,
+ ]
+ return [
+ archive
+ for archive in distribution.all_distro_archives
+ if archive.purpose in ARCHIVES_TO_PUBLISH]
def compose_shell_boolean(boolean_value):
@@ -166,6 +174,9 @@
def add_my_options(self):
"""See `LaunchpadScript`."""
self.parser.add_option(
+ '-a', '--all-derived', dest='all_derived', action='store_true',
+ default=False, help="Process all derived distributions.")
+ self.parser.add_option(
'-d', '--distribution', dest='distribution', default=None,
help="Distribution to publish.")
self.parser.add_option(
@@ -179,16 +190,25 @@
def processOptions(self):
"""Handle command-line options.
- Sets `self.distribution` to the `Distribution` to publish.
+ Sets `self.distributions` to the `Distribution`s to publish.
"""
- if self.options.distribution is None:
- raise LaunchpadScriptFailure("Specify a distribution.")
+ if self.options.distribution is None and not self.options.all_derived:
+ raise LaunchpadScriptFailure(
+ "Specify a distribution, or --all-derived.")
+ if self.options.distribution is not None and self.options.all_derived:
+ raise LaunchpadScriptFailure(
+ "Can't combine the --distribution and --all-derived options.")
- self.distribution = getUtility(IDistributionSet).getByName(
- self.options.distribution)
- if self.distribution is None:
- raise LaunchpadScriptFailure(
- "Distribution %s not found." % self.options.distribution)
+ if self.options.all_derived:
+ distro_set = getUtility(IDistributionSet)
+ self.distributions = distro_set.getDerivedDistributions()
+ else:
+ distro = getUtility(IDistributionSet).getByName(
+ self.options.distribution)
+ if distro is None:
+ raise LaunchpadScriptFailure(
+ "Distribution %s not found." % self.options.distribution)
+ self.distributions = [distro]
def executeShell(self, command_line, failure=None):
"""Run `command_line` through a shell.
@@ -210,25 +230,20 @@
self.logger.debug("Command failed: %s", failure)
raise failure
- def getArchives(self, distribution):
- """Find archives for `self.distribution` that should be published."""
- # XXX JeroenVermeulen 2011-03-31 bug=746229: to start publishing
- # debug archives, change this to return
- # list(distribution.all_distro_archives).
- return [
- archive
- for archive in distribution.all_distro_archives
- if archive.purpose in ARCHIVES_TO_PUBLISH]
-
def getConfigs(self):
"""Set up configuration objects for archives to be published.
- The configs dict maps the archive purposes that are relevant for
- publishing to the respective archives' configurations.
+ The configs dict maps each distribution to another dict that maps the
+ archive purposes that are relevant for publishing to the respective
+ archives' configurations.
+
+ So: getConfigs[distro][purpose] gives you a config.
"""
return dict(
- (archive.purpose, getPubConfig(archive))
- for archive in self.archives)
+ (distro, dict(
+ (archive.purpose, getPubConfig(archive))
+ for archive in get_publishable_archives(distro)))
+ for distro in self.distributions)
def locateIndexesMarker(self, distribution, suite):
"""Give path for marker file whose presence marks index creation.
@@ -237,8 +252,9 @@
have been created. This is how future runs will know that this
work is done.
"""
- archive_root = self.configs[ArchivePurpose.PRIMARY].archiveroot
- return os.path.join(archive_root, ".created-indexes-for-%s" % suite)
+ config = self.configs[distribution][ArchivePurpose.PRIMARY]
+ return os.path.join(
+ config.archiveroot, ".created-indexes-for-%s" % suite)
def listSuitesNeedingIndexes(self, distroseries):
"""Find suites in `distroseries` that need indexes created.
@@ -317,7 +333,7 @@
:param archive_purpose: The (purpose of the) archive to copy.
"""
- for purpose, archive_config in self.configs.iteritems():
+ for purpose, archive_config in self.configs[distribution].iteritems():
dists = get_dists(archive_config)
backup_dists = get_backup_dists(archive_config)
self.executeShell(
@@ -346,25 +362,28 @@
run died while in this state, restore the directory to its
permanent location.
"""
- for archive_config in self.configs.itervalues():
- self.recoverArchiveWorkingDir(archive_config)
+ for distro_configs in self.configs.itervalues():
+ for archive_config in distro_configs.itervalues():
+ self.recoverArchiveWorkingDir(archive_config)
def setUpDirs(self):
"""Create archive roots and such if they did not yet exist."""
- for archive_purpose, archive_config in self.configs.iteritems():
- archiveroot = archive_config.archiveroot
- if not file_exists(archiveroot):
- self.logger.debug("Creating archive root %s.", archiveroot)
- os.makedirs(archiveroot)
- dists = get_dists(archive_config)
- if not file_exists(dists):
- self.logger.debug("Creating dists root %s.", dists)
- os.makedirs(dists)
- distscopy = get_backup_dists(archive_config)
- if not file_exists(distscopy):
- self.logger.debug(
- "Creating backup dists directory %s", distscopy)
- os.makedirs(distscopy)
+ for distro_configs in self.configs.itervalues():
+ for archive_purpose, archive_config in distro_configs.iteritems():
+ archiveroot = archive_config.archiveroot
+ if not file_exists(archiveroot):
+ self.logger.debug(
+ "Creating archive root %s.", archiveroot)
+ os.makedirs(archiveroot)
+ dists = get_dists(archive_config)
+ if not file_exists(dists):
+ self.logger.debug("Creating dists root %s.", dists)
+ os.makedirs(dists)
+ distscopy = get_backup_dists(archive_config)
+ if not file_exists(distscopy):
+ self.logger.debug(
+ "Creating backup dists directory %s", distscopy)
+ os.makedirs(distscopy)
def runPublishDistro(self, distribution, args=[], suites=None):
"""Execute `publish-distro`."""
@@ -390,7 +409,7 @@
the publishing to.
"""
purpose = archive.purpose
- archive_config = self.configs[purpose]
+ archive_config = self.configs[distribution][purpose]
self.logger.debug(
"Publishing the %s %s...", distribution.name, purpose.title)
@@ -414,7 +433,7 @@
def runPublishDistroParts(self, distribution, archive):
"""Execute the publish-distro hooks."""
- archive_config = self.configs[archive.purpose]
+ archive_config = self.configs[distribution][archive.purpose]
env = {
'ARCHIVEROOT': shell_quote(archive_config.archiveroot),
'DISTSROOT': shell_quote(get_backup_dists(archive_config)),
@@ -430,7 +449,7 @@
backup dists directory around.
"""
self.logger.debug("Moving the new dists into place...")
- for archive_config in self.configs.itervalues():
+ for archive_config in self.configs[distribution].itervalues():
# Use the dists "working location" as a temporary place to
# move the current dists out of the way for the switch. If
# we die in this state, the next run will know to move the
@@ -448,7 +467,7 @@
self.logger.debug("Creating ls-lR.gz...")
lslr = "ls-lR.gz"
lslr_new = "." + lslr + ".new"
- for purpose, archive_config in self.configs.iteritems():
+ for purpose, archive_config in self.configs[distribution].iteritems():
lslr_file = os.path.join(archive_config.archiveroot, lslr)
new_lslr_file = os.path.join(archive_config.archiveroot, lslr_new)
if file_exists(new_lslr_file):
@@ -462,7 +481,7 @@
def clearEmptyDirs(self, distribution):
"""Clear out any redundant empty directories."""
- for archive_config in self.configs.itervalues():
+ for archive_config in self.configs[distribution].itervalues():
self.executeShell(
"find '%s' -type d -empty | xargs -r rmdir"
% archive_config.archiveroot)
@@ -470,6 +489,7 @@
def runParts(self, distribution, parts, env):
"""Execute run-parts.
+ :param distribution: `Distribution` to execute run-parts scripts for.
:param parts: The run-parts directory to execute:
"publish-distro.d" or "finalize.d".
:param env: A dict of environment variables to pass to the
@@ -489,7 +509,7 @@
"""Run the finalize.d parts to finalize publication."""
archive_roots = shell_quote(' '.join([
archive_config.archiveroot
- for archive_config in self.configs.itervalues()]))
+ for archive_config in self.configs[distribution].itervalues()]))
env = {
'SECURITY_UPLOAD_ONLY': compose_shell_boolean(security_only),
@@ -512,7 +532,7 @@
def publishDistroUploads(self, distribution):
"""Publish the distro's complete uploads."""
self.logger.debug("Full publication. This may take some time.")
- for archive in self.archives:
+ for archive in get_publishable_archives(distribution):
# This, for the main archive, is where the script spends
# most of its time.
self.publishDistroArchive(distribution, archive)
@@ -546,16 +566,10 @@
def setUp(self):
"""Process options, and set up internal state."""
self.processOptions()
- self.archives = self.getArchives(self.distribution)
self.configs = self.getConfigs()
- def main(self):
- """See `LaunchpadScript`."""
- self.setUp()
- self.recoverWorkingDists()
-
- distribution = self.distribution
-
+ def processDistro(self, distribution):
+ """Process `distribution`."""
for series in distribution.series:
suites_needing_indexes = self.listSuitesNeedingIndexes(series)
if len(suites_needing_indexes) > 0:
@@ -566,7 +580,6 @@
return
self.processAccepted(distribution)
- self.setUpDirs()
self.rsyncBackupDists(distribution)
self.publish(distribution, security_only=True)
@@ -585,3 +598,13 @@
# changes, and so it'll take the next run a little less
# time to publish its security updates.
self.rsyncBackupDists(distribution)
+
+ def main(self):
+ """See `LaunchpadScript`."""
+ self.setUp()
+ self.recoverWorkingDists()
+ self.setUpDirs()
+
+ for distribution in self.distributions:
+ self.processDistro(distribution)
+ self.txn.commit()
=== modified file 'lib/lp/archivepublisher/tests/test_publish_ftpmaster.py'
--- lib/lp/archivepublisher/tests/test_publish_ftpmaster.py 2011-08-15 11:04:41 +0000
+++ lib/lp/archivepublisher/tests/test_publish_ftpmaster.py 2011-08-15 11:04:41 +0000
@@ -290,6 +290,22 @@
self.SCRIPT_PATH + " -d ubuntu")
self.assertEqual(0, retval, "Script failure:\n" + stderr)
+ def test_getConfigs_maps_distro_and_purpose_to_matching_config(self):
+ distro = self.makeDistroWithPublishDirectory()
+ script = self.makeScript(distro)
+ script.setUp()
+ reference_config = getPubConfig(distro.main_archive)
+ config = script.getConfigs()[distro][ArchivePurpose.PRIMARY]
+ self.assertEqual(reference_config.temproot, config.temproot)
+ self.assertEqual(reference_config.distroroot, config.distroroot)
+ self.assertEqual(reference_config.archiveroot, config.archiveroot)
+
+ def test_getConfigs_maps_distros(self):
+ distro = self.makeDistroWithPublishDirectory()
+ script = self.makeScript(distro)
+ script.setUp()
+ self.assertEqual([distro], script.getConfigs().keys())
+
def test_script_is_happy_with_no_publications(self):
distro = self.makeDistroWithPublishDirectory()
self.makeScript(distro).main()
@@ -540,7 +556,19 @@
script = self.makeScript(distro)
script.processOptions()
self.assertEqual(distro.name, script.options.distribution)
- self.assertEqual(distro, script.distribution)
+ self.assertEqual([distro], script.distributions)
+
+ def test_processOptions_for_all_derived_finds_derived_distros(self):
+ dsp = self.factory.makeDistroSeriesParent()
+ script = PublishFTPMaster(test_args=['--all-derived'])
+ script.processOptions()
+ self.assertIn(dsp.derived_series.distribution, script.distributions)
+
+ def test_processOptions_for_all_derived_ignores_nonderived_distros(self):
+ distro = self.factory.makeDistribution()
+ script = PublishFTPMaster(test_args=['--all-derived'])
+ script.processOptions()
+ self.assertNotIn(distro, script.distributions)
def test_processOptions_complains_about_unknown_distribution(self):
script = self.makeScript()
@@ -551,7 +579,7 @@
self.enableRunParts()
script = self.makeScript(self.prepareUbuntu())
script.setUp()
- distro = script.distribution
+ distro = script.distributions[0]
script.executeShell = FakeMethod()
script.runParts(distro, "finalize.d", {})
self.assertEqual(1, script.executeShell.call_count)
@@ -566,7 +594,7 @@
self.enableRunParts()
script = self.makeScript(self.prepareUbuntu())
script.setUp()
- distro = script.distribution
+ distro = script.distributions[0]
script.executeShell = FakeMethod()
key = self.factory.getUniqueString()
value = self.factory.getUniqueString()
@@ -603,7 +631,7 @@
def test_runFinalizeParts_passes_parameters(self):
script = self.makeScript(self.prepareUbuntu())
script.setUp()
- distro = script.distribution
+ distro = script.distributions[0]
script.runParts = FakeMethod()
script.runFinalizeParts(distro)
args, kwargs = script.runParts.calls[0]
@@ -615,7 +643,7 @@
def test_publishSecurityUploads_skips_pub_if_no_security_updates(self):
script = self.makeScript()
script.setUp()
- distro = script.distribution
+ distro = script.distributions[0]
script.setUpDirs()
script.installDists = FakeMethod()
script.publishSecurityUploads(distro)
@@ -760,7 +788,7 @@
script.setUp()
self.assertRaisesWithContent(
MoonPhaseError, message,
- script.publish, script.distribution)
+ script.publish, script.distributions[0])
def test_publish_obeys_keyboard_interrupt(self):
# Similar to an Exception, a keyboard interrupt does not get
@@ -772,7 +800,7 @@
script.setUp()
self.assertRaisesWithContent(
KeyboardInterrupt, message,
- script.publish, script.distribution)
+ script.publish, script.distributions[0])
def test_publish_recovers_working_dists_on_exception(self):
# If an Exception comes up while publishing, the publish method
@@ -788,7 +816,7 @@
script.setUp()
try:
- script.publish(script.distribution)
+ script.publish(script.distributions[0])
except MoonPhaseError:
pass
@@ -805,7 +833,7 @@
script.setUp()
try:
- script.publish(script.distribution)
+ script.publish(script.distributions[0])
except KeyboardInterrupt:
pass
