Merge lp:~cjwatson/launchpad/refactor-cron-germinate into lp:launchpad

[Colin Watson]

[r=jtv][bug=899972] Reimplement most of cron.germinate in Python using the new python-germinate package. This no longer needs to recompute the dependency expansion of common seeds over and over again, so is much faster.

= Summary =

cron.germinate currently takes on the order of ten minutes, and really doesn't have a particularly good excuse for taking so long. Fixing this would probably be enough to let us move back to 30-minute publisher cycles for Ubuntu (as we used to have with dak, back in the dawn of time), which would increase our velocity and make some of us very happy.

The main reason it's so slow is that it runs germinate as a separate process once for each of eight flavours (Ubuntu, Kubuntu, etc., each with its own seed collection) on each of five architectures. There are a few inefficiencies inherent in this approach, but the most significant is that it has to expand dependencies and build-dependencies of the seeds that are common to all the flavours eight times as often as necessary. Since the build-dependency chain in particular of the base system winds its way through a good fraction of main, this winds up being rather a lot of duplicated work.

== Proposed fix ==

I've been working on this problem for some time now, and have just released germinate 2.0 to support solving it properly. The most important change here is that germinate can now process multiple seed collections for a single architecture in a single instance of the core Germinator class, which allows reusing the expansions of common seeds. While I could technically have extended the command-line interface further for this, I felt that the command-line interface was already far too complicated, and decided instead to export a public, documented, and stable Python interface which can be used for this purpose.

This branch is the other half of that fix: a LaunchpadScript that uses germinate's new Python interface to do the same work as the top part of cron.germinate (i.e. everything except for maintenance-check.py). A stripped-down subset of this with just the logic (germinating seeds and producing output) runs in about three minutes on my laptop.

== Pre-implementation notes ==

The Launchpad position for a while has been that cron.germinate is owned by Ubuntu Engineering, and indeed the vast bulk of the work here was in preparing a new germinate release in Ubuntu that does what we need. However, I have talked with various Launchpad people over the last few months so that it was clear that I was working on this, and I did need some help figuring out how to approach testing. Initially I'd been planning on doing a full Soyuz publisher run so that I had Packages and Sources files to work with, but Julian said this would be too slow and pointed me towards methods like factory.makeSourcePackagePublishingHistory, which allow me to use getIndexStanza et al so that I don't have to write out RFC822 data by hand.

== Implementation details ==

I had to add a new attribute on PublisherConfig pointing to the germinate output directory; previously this was only known to the shell script.

I added a 'generate_extra_overrides' user to security.cfg, which seemed to be in line with other archivepublisher scripts. Was this the right approach? Does it require any special deployment steps?

The actual override generation was a fairly straightforward translation from shell to Python.

Predictably, the bulk of the work was in writing tests, since cron.germinate was previously untested and I didn't think continuing with untested code was likely to fly once it was in Python. The top-level shell script is still untested, but there's a lot less in it and I expect that it can eventually be removed entirely.

I ran into a couple of existing bugs that slowed me down. Firstly, I'd hoped to use the fake librarian to speed things up a bit, but ran into bug 713764, which was beyond my ability to fix (although I tried). Secondly, bug 694140 seriously confused me for a while; I added much the same workaround as is found elsewhere in Launchpad for that problem so that germinate's log output doesn't get lost.

I wanted to include a test that the cronscript could be run standalone, but I couldn't see how to ensure that a suitable PublisherConfig was in place.

== Tests ==

bin/test -vvct generate_extra_overrides

This branch makes some small changes to other archivepublisher tests, so:

bin/test -vvct archivepublisher

== Demo and Q/A ==

The deployment steps listed in bug 899972 and https://rt.admin.canonical.com/Ticket/Display.html?id=49745 need to be completed before we can land this branch.

Once that's done, we can Q/A this by doing a timed control publisher run on mawson, rolling out this change, taking a backup copy of ubuntu-germinate and ubuntu-misc, and then doing another timed publisher run and comparing the ubuntu-germinate and ubuntu-misc directories before and after. (There may be some harmless ordering changes and such.)

== Lint ==

./cronscripts/generate-extra-overrides.py
       8: '_pythonpath' imported but unused

Seems to be standard practice in cronscripts.

./lib/lp/archivepublisher/tests/publisher-config.txt
       1: narrative uses a moin header.
      26: narrative uses a moin header.
      46: narrative uses a moin header.
     114: narrative uses a moin header.
     138: narrative uses a moin header.
     163: narrative uses a moin header.
     178: want exceeds 78 characters.
     179: want exceeds 78 characters.
     180: want exceeds 78 characters.
     182: want exceeds 78 characters.

This was pre-existing lint; I only made the minimal changes necessary to add the 'germinateroot' attribute, which haven't fundamentally made the situation any worse.

[Colin Watson (cjwatson)]

I've tested the core logic on mawson now (stripped out the database interaction and hardcoded paths to a local mirror so that I could run it in isolation). Compared to an output tree generated by the old cron.germinate with germinate 2.0, the substantive output is identical: various files are deliberately no longer generated, and the log output is different, but the actual germinate output and the more-extra.override.precise.main file are identical. It runs in about 7 minutes compared with about 14/15 minutes for the old cron.germinate, which I call a win (although these are single-run figures).

The weird part is that something is wrong that means we aren't even getting a lot of the benefit yet! It isn't actually sharing the results of germinating common seeds. Therefore, that 7/8 minutes of win must essentially come from avoiding duplicated seed parsing, avoiding duplicated Packages/Sources parsing, and a reduction in the number of files we write. I wasn't expecting that to account for so much of the runtime, but I'm not going to complain.

I will investigate further to find out why common seed output isn't being shared. That should bring the runtime down further.

[Colin Watson (cjwatson)]

Right. I've tracked down all the problems I could find and released germinate 2.1 to fix all of them; none of them were problems with this branch. The runtime is now down to about 4m50s on mawson; the only differences in output are (a) in places where it doesn't matter very much and (b) accountable to a pure-virtual dependency in libenchant1c2a, where germinate is entirely entitled to choose a provider at random, so I'm not worried about those.

[Jeroen T. Vermeulen (jtv)]

A note on the problem of ensuring that a PublisherConfig is in place: factory.makeDistribution creates one for you. Just make sure to pass it a publish_root_dir, or I think it will use default system config. This should probably be considered a bug in the factory.

[Jeroen T. Vermeulen (jtv)]

It looks to me like you did the right thing in creating a dedicated database user. We may ditch the per-user privileges, but we do want separate users for separate scripts.

[Jeroen T. Vermeulen (jtv)]

=== added file 'lib/lp/archivepublisher/scripts/generate_extra_overrides.py'
--- lib/lp/archivepublisher/scripts/generate_extra_overrides.py 1970-01-01 00:00:00 +0000
+++ lib/lp/archivepublisher/scripts/generate_extra_overrides.py 2011-12-06 15:03:58 +0000

+class AtomicFile:
+ """Facilitate atomic writing of files."""
+
+ def __init__(self, filename):
+ self.filename = filename
+ self.fd = open('%s.new' % self.filename, 'w')
+
+ def __enter__(self):
+ return self.fd
+
+ def __exit__(self, exc_type, exc_value, exc_tb):
+ self.fd.close()
+ os.rename('%s.new' % self.filename, self.filename)

Does it make sense to put the file in place if the context exits with an exception?

Actually this is what I don't like about the design of python context managers: it puts you, as the author of a context manager, in the position where you may have to worry about failures in the context you manage — and yet it's easy for the author of the context to hide them from you. So maybe stupid is just better.

[Jeroen T. Vermeulen (jtv)]

Next bit I looked at:

=== added file 'lib/lp/archivepublisher/scripts/generate_extra_overrides.py'

+ def processOptions(self):
+ """Handle command-line options."""
+ if self.options.distribution is None:
+ raise OptionValueError("Specify a distribution.")
+
+ self.distribution = getUtility(IDistributionSet).getByName(
+ self.options.distribution)
+ if self.distribution is None:
+ raise OptionValueError(
+ "Distribution '%s' not found." % self.options.distribution)
+
+ series = None
+ wanted_status = (SeriesStatus.DEVELOPMENT,
+ SeriesStatus.FROZEN)
+ for status in wanted_status:
+ series = self.distribution.getSeriesByStatus(status)
+ if series.count() > 0:
+ break
+ else:
+ raise LaunchpadScriptFailure(
+ 'There is no DEVELOPMENT distroseries for %s' %
+ self.options.distribution)
+ self.series = series[0]

The part from “series = None” onward seems to be an isolated unit of work. I think it looks for the first distroseries in development state, or failing that, the first in frozen state. But the structure of the code makes that hard to figure out, and then only afterwards I can start wondering why you do it exactly this way.

I fervently recommend extracting that code into a sensibly-named function. (It doesn't need to be a method: AFAICS a distribution goes in, a series comes out, and that is the full extent of its interaction with the rest of the world). Come to think of it, might there already be a suitable method in Distribution somewhere?

The structure of that loop is also a bit hard to follow. It gets easier if you have a single-purpose function that you can just return from instead of using a break!

Finally, a few cosmetic tips:

 * When line-breaking a tuple, list, or dict, our “house style” is to add a line break right after the opening parenthesis, bracket, or brace. Each entry ends in a comma and a newline — including the last entry.

    wanted_status = (
        SeriesStatus.DEVELOPMENT,
        SeriesStatus.FROZEN,
        )

 * You don't really need series.count(). In fact I think you can just retrieve the first matching series as series.first(), and you'll get None if there isn't any.

 * For Python strings I find consistent double quotes a good habit, at least for free-form texts that may just as well contain an apostrophe.

[Jeroen T. Vermeulen (jtv)]

I'm going through the diff sequentially at the moment; the parts I'm ignoring are the parts that I have nothing to say about. When I get to the end I can give you a very brief formal review. :)

=== added file 'lib/lp/archivepublisher/scripts/generate_extra_overrides.py'

+ def getConfig(self):
+ """Set up a configuration object for this archive."""
+ for archive in self.distribution.all_distro_archives:
+ # We only work on the primary archive.
+ if archive.purpose == ArchivePurpose.PRIMARY:
+ return getPubConfig(archive)
+ else:
+ raise LaunchpadScriptFailure(
+ 'There is no PRIMARY archive for %s' %
+ self.options.distribution)

Why not just use self.distribution.main_archive?

(Also, remember to punctuate that error message!)

[Jeroen T. Vermeulen (jtv)]

+ def outputPath(self, flavour, series_name, arch, base):
+ return os.path.join(
+ self.config.germinateroot,
+ '%s_%s_%s_%s' % (base, flavour, series_name, arch))

Our dromedary-cased method names normally start with a verb. Read that way, “outputPath” would suggest that the method prints a path.

So consider prefixing this method's name with a verb. The unimaginative catch-all verb is “get”; personally I prefer “compose” for this kind of thing.

[Jeroen T. Vermeulen (jtv)]

+ def runGerminate(self, override_file, series_name, arch, flavours,
+ structures):

Rightly or wrongly, the “run” in that name led me to expect that this method would fire up a separate process. Maybe a very short docstring to dispel that notion?

+ germinator = Germinator(arch)
+
+ # Read archive metadata.
+ archive = TagFile(
+ series_name, self.components, arch,
+ 'file://%s' % self.config.archiveroot, cleanup=True)
+ germinator.parse_archive(archive)
+
+ for flavour in flavours:

To me, your branch is of very high quality but this is its Achilles' heel. The following loop body is massive! I can't see it all on one page, so it becomes hard even to see just how long it is. If I judged the indentations right, it makes up the entire rest of the method.

Please extract it. It's well nigh impossible to see (now or in the future, after some maintenance) whether any variables are carried across loop iterations. That sort of thing can easily trip you up, especially when it happens by accident.

Once you've extracted the loop body, given its size, it's probably still worth extracting chunks from that.

According to some, the pattern of “comment, block of code, blank line, comment, block of code, blank line” is a strong indication that you need to split things up. It shows that you have already done so in your own mind, but leaves each individual reader to figure out what the dependencies between consecutive blocks are.

Also, of course, small methods with simple purposes are easier to specify in detail and cheaper to test thoroughly.

+ self.logger.info('Germinating for %s/%s/%s',
+ flavour, series_name, arch)

We follow a different rule for line-breaking function calls than we do for function definitions.

For calls, line-break right after the opening parenthesis and then indent the arguments list by 4 spaces:

    self.logger.info(
        "Germinating for %s/%s/%s.", flavour, series_name, arch)

+ # Add this to the germinate log as well so that that can be
+ # debugged more easily. Log a separator line first.
+ self.germinate_logger.info('', extra={'progress': True})
+ self.germinate_logger.info('Germinating for %s/%s/%s',
+ flavour, series_name, arch,
+ extra={'progress': True})

What does the “extra” parameter do?

+ # Expand dependencies.
+ structure = structures[flavour]
+ germinator.plant_seeds(structure)
+ germinator.grow(structure)
+ germinator.add_extras(structure)
+ # Write output files.
+
+ # The structure file makes it possible to figure out how the
+ # other output files relate to each other.
+ structure.write(self.outputPath(
+ flavour, series_name, arch, 'structure'))
+
+ # "all" and "all.sources" list the full set of binary and source
+ # packages respectively for a given flavour/suite/architecture
+ # combination.
+ all_path = ...

[Colin Watson (cjwatson)]

I think you're right that the file shouldn't be put in place if the
context exits with an exception. If I were writing this without context
managers, then rather than:

    with AtomicFile('foo') as handle:
        print >>handle, 'bar'

... I'd probably write:

    handle = open('foo.new', 'w')
    try:
        print >>handle, 'bar'
    finally:
        handle.close()
    os.rename('foo.new', 'foo')

And that's equivalent to the natural expression in (good) shell too:

    set -e
    echo bar >foo.new
    mv foo.new foo

That all suggests to me that skipping the rename on exception, and
leaving the .new file in place so that somebody with access can look at
it and find out how far the script got before it fell over, is a good
thing. I'll change the code to do that.

[Jeroen T. Vermeulen (jtv)]

Getting a better feel for the structure of runGerminate now. Which makes me wonder:

+ # Generate apt-ftparchive "extra overrides" for Build-Essential
+ # fields.
+ if ('build-essential' in structure.names and
+ flavour == flavours[0]):
+ writeOverrides('build-essential', 'Build-Essential', 'yes')

I don't suppose you could just do this at the end, outside of all loops?

    if 'build-essential' in structures[flavours[0]].names:
        writeOverrides('build-essential', 'Build-Essential', 'yes')

[Jeroen T. Vermeulen (jtv)]

Ah, I see now why you can't do that: writeOverrides is a closure. It picks up “structure” and “arch” from the surrounding method at its point of definition. There's another easy thing to break by accident! The structure should again be structures[flavours[0]], I guess.

Might it be worthwhile to separate the closure nature of writeOverrides from its functionality? If you had a separate function that takes all the necessary variables as arguments, it'd be easier to move that last paragraph out of the loop (still assuming that that makes sense at all) and it'd be easier to see that writeOverrides “borrows” some variables from runGerminate.

[Colin Watson (cjwatson)]

On Wed, Dec 07, 2011 at 07:20:29AM -0000, Jeroen T. Vermeulen wrote:
> The part from “series = None” onward seems to be an isolated unit of
> work. I think it looks for the first distroseries in development
> state, or failing that, the first in frozen state. But the structure
> of the code makes that hard to figure out, and then only afterwards I
> can start wondering why you do it exactly this way.
>
> I fervently recommend extracting that code into a sensibly-named
> function. (It doesn't need to be a method: AFAICS a distribution goes
> in, a series comes out, and that is the full extent of its interaction
> with the rest of the world). Come to think of it, might there already
> be a suitable method in Distribution somewhere?

Fair comment. Distribution.currentseries is nearly there: it just
sometimes returns series in statuses we don't want here; but if there's
one we can use then it will always return it, so we can just call
currentseries and then check the result.

> * For Python strings I find consistent double quotes a good habit, at
> least for free-form texts that may just as well contain an
> apostrophe.

This is an out-of-context habit of mine from Perl and shell programming:
since single and double quotes have different interpolation
characteristics there, I've trained myself to use single quotes unless
either I have an explicit reason to want interpolation or the string
contains an apostrophe and no double quotes. I realise this doesn't
make as much sense for Python, so I'll go through and amend this.

[Jeroen T. Vermeulen (jtv)]

Some very nice tests there! To continue:

=== added file 'lib/lp/archivepublisher/tests/test_generate_extra_overrides.py'

+def file_contents(path):
+ """Return the contents of the file at path."""
+ with open(path) as handle:
+ return handle.read()

Since you're only reading the file, there's probably no hurry to close this file handle. So you could just return “open(path).read().”

[Colin Watson (cjwatson)]

[I've not responded to your entire review point-by-point, but I believe
I've addressed all points where I haven't given an explicit response.]

On Wed, Dec 07, 2011 at 09:22:24AM -0000, Jeroen T. Vermeulen wrote:
> + def runGerminate(self, override_file, series_name, arch, flavours,
> + structures):
>
> Rightly or wrongly, the “run” in that name led me to expect that this
> method would fire up a separate process. Maybe a very short docstring
> to dispel that notion?

The name doesn't actually make a lot of sense; I think originally I was
in the mindset where this actually did fire up a separate process. I've
renamed the script instance to germinateArch (with a docstring), and the
test instance to fetchGerminatedOverrides. Does that help?

> To me, your branch is of very high quality but this is its Achilles'
> heel. The following loop body is massive! I can't see it all on one
> page, so it becomes hard even to see just how long it is. If I judged
> the indentations right, it makes up the entire rest of the method.

That's fair; I went back and forward a bit on this, but clearly landed
in the wrong place. Actually, now that I've renamed runGerminate to
germinateArch, it's easier in my mind to turn a block of it into
germinateArchFlavour in turn. (Regularised naming may be boring, but I
like it anyway.)

The methods do start ending up with quite a few parameters, but that's
probably better than the alternative.

> Once you've extracted the loop body, given its size, it's probably
> still worth extracting chunks from that.

I've done a good deal of this, although will probably end up
restructuring a bit further per your later comments.

> According to some, the pattern of “comment, block of code, blank line,
> comment, block of code, blank line” is a strong indication that you
> need to split things up. It shows that you have already done so in
> your own mind, but leaves each individual reader to figure out what
> the dependencies between consecutive blocks are.

I don't necessarily agree with this in all cases (e.g. I don't know that
the new writeGerminateOutput method would benefit much from being split
further), but I agree that the original version of this code was rather
too far in the other direction.

> + # Add this to the germinate log as well so that that can be
> + # debugged more easily. Log a separator line first.
> + self.germinate_logger.info('', extra={'progress': True})
> + self.germinate_logger.info('Germinating for %s/%s/%s',
> + flavour, series_name, arch,
> + extra={'progress': True})
>
> What does the “extra” parameter do?

That attaches a dictionary of extra attributes to the log message which
can then be picked up by the formatter. Germinate uses this for certain
messages (arguably a wart in germinate's design; I should probably look
into doing that more neatly at some point). I've moved this to its own
method so that it can have a clarifying docstring.

> That's a lot of string manipulation. It may be clearer as a regex:
>
> task_header_regex = re.compile("ta...

[Colin Watson (cjwatson)]

On Wed, Dec 07, 2011 at 09:56:26AM -0000, Jeroen T. Vermeulen wrote:
> Might it be worthwhile to separate the closure nature of
> writeOverrides from its functionality? If you had a separate function
> that takes all the necessary variables as arguments, it'd be easier to
> move that last paragraph out of the loop (still assuming that that
> makes sense at all) and it'd be easier to see that writeOverrides
> “borrows” some variables from runGerminate.

Good idea. Per our conversation on IRC, I've used functools.partial for
this. I also used it in writeGerminateOutput, which IMO this approach
notably improves. What do you think?

[Jeroen T. Vermeulen (jtv)]

Thank you for bearing with me, making so many changes after producing an already large and difficult branch. I particularly like the way you seized on method extraction as an opportunity to document the algorithm's steps.

This is a massive step forward in terms of code maintenance alone, and I'm looking forward to seeing how it affects performance.

review: Approve

[Colin Watson (cjwatson)]

On Wed, Dec 07, 2011 at 10:14:27AM -0000, Jeroen T. Vermeulen wrote:
> +def file_contents(path):
> + """Return the contents of the file at path."""
> + with open(path) as handle:
> + return handle.read()
>
> Since you're only reading the file, there's probably no hurry to close this file handle. So you could just return “open(path).read().”

For the record, there was some discussion on this on IRC, and most
people seemed to think that it was better to retain the immediate close,
in particular since implementations other than cPython may not GC-close
it with any alacrity; so I've left this the way it is. (There is a lot
of variation on this in the Launchpad test suite, though ...)

=== added file 'cronscripts/generate-extra-overrides.py'
--- cronscripts/generate-extra-overrides.py	1970-01-01 00:00:00 +0000
+++ cronscripts/generate-extra-overrides.py	2011-12-14 15:49:40 +0000
@@ -0,0 +1,18 @@
+#!/usr/bin/python -S
+#
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Generate extra overrides using Germinate."""
+
+import _pythonpath
+
+from lp.archivepublisher.scripts.generate_extra_overrides import (
+    GenerateExtraOverrides,
+    )
+
+
+if __name__ == '__main__':
+    script = GenerateExtraOverrides(
+        "generate-extra-overrides", dbuser='generate_extra_overrides')
+    script.lock_and_run()

=== modified file 'cronscripts/publishing/cron.germinate'
--- cronscripts/publishing/cron.germinate	2011-10-27 11:36:13 +0000
+++ cronscripts/publishing/cron.germinate	2011-12-14 15:49:40 +0000
@@ -12,8 +12,12 @@
 GERMINATEROOT=$ARCHIVEROOT/../ubuntu-germinate
 
 LAUNCHPADROOT=${TEST_LAUNCHPADROOT:-/srv/launchpad.net/codelines/current}
+GENERATE=$LAUNCHPADROOT/cronscripts/generate-extra-overrides.py
 MAINTAINCE_CHECK=$LAUNCHPADROOT/cronscripts/publishing/maintenance-check.py
 
+FLAVOURS="ubuntu kubuntu kubuntu-mobile edubuntu xubuntu mythbuntu lubuntu"
+FLAVOURS="$FLAVOURS ubuntustudio"
+
 ## Check to see if another germinate run is in progress
 
 LOCKFILE=$LOCKROOT/cron.germinate.lock
@@ -28,134 +32,9 @@
 
 trap cleanup EXIT
 
-suite=`$LAUNCHPADROOT/scripts/ftpmaster-tools/lp-query-distro.py development`
-archs=`$LAUNCHPADROOT/scripts/ftpmaster-tools/lp-query-distro.py -s "$suite" archs`
-
-echo -n "Running germinate... "
 cd $GERMINATEROOT
 
-# Clean up temporary files
-rm -f \
-    germinate.output ALL ALL.sources \
-    UBUNTU-* KUBUNTU-* EDUBUNTU-* XUBUNTU-* MYTHBUNTU-* LUBUNTU-* \
-    UBUNTUSTUDIO-*
-rm -f all_* all.sources_*
-rm -rf dists
-
-# Grab local copies of Sources and Packages files, to avoid problems in case
-# the archive changes under our feet.
-for component in main universe restricted multiverse; do
-  base="dists/$suite/$component"
-  mkdir -p "$base/source"
-  zcat "$ARCHIVEROOT/$base/source/Sources.gz" > "$base/source/Sources"
-  for arch in $archs; do
-    mkdir -p "$base/binary-$arch" "$base/debian-installer/binary-$arch"
-    zcat "$ARCHIVEROOT/$base/binary-$arch/Packages.gz" \
-      > "$base/binary-$arch/Packages"
-    zcat "$ARCHIVEROOT/$base/debian-installer/binary-$arch/Packages.gz" \
-      > "$base/debian-installer/binary-$arch/Packages"
-  done
-done
-
-> "$MISCROOT/more-extra.override.$suite.main.new"
-
-germinate_components=main,universe,restricted,multiverse
-for distro in \
-    ubuntu kubuntu kubuntu-mobile edubuntu xubuntu mythbuntu lubuntu \
-    ubuntustudio
-do
-  DISTRO="$(echo $distro | tr a-z A-Z)"
-  germinate_suite="$distro.$suite"
-  for arch in $archs; do
-    # Run germinate
-    echo " **************** $distro/$suite/$arch ********************* " >> germinate.output
-    germinate \
-        --no-rdepends \
-        -m "file://$(pwd)/" \
-        -s "$germinate_suite" \
-        -d "$suite" \
-        -c "$germinate_components" \
-        -a $arch >> germinate.output 2>&1
-
-    # The structure file is generally useful; keep per distro/suite/arch
-    # copies for convenience
-    cp structure structure_"$distro"_"$suite"_"$arch"
-
-    # Keep per distro/suite/arch copies of 'all' and 'all.sources' for
-    # anastacia.
-    cp all all_"$distro"_"$suite"_"$arch"
-    cp all.sources all.sources_"$distro"_"$suite"_"$arch"
-
-    # Keep per distro/suite/arch copies of 'minimal' and 'standard' for
-    # jessica.
-    cp minimal minimal_"$distro"_"$suite"_"$arch"
-    cp standard standard_"$distro"_"$suite"_"$arch"
-
-    # Keep amalgamated copies of 'all' and 'all.sources', just for convenience
-    cat all >> ALL; cat all.sources >> ALL.sources
-
-    # We need to fetch a number of seeds so that we can generate Task fields
-    # for them. This changes over time and differs from derivative to
-    # derivative, so it's best to just fetch them all.
-    taskseeds="$(cut -d: -f1 structure | xargs -n1 | sort -u)"
-
-    for seed in $taskseeds; do
-      cp "$seed" "$seed"_"$distro"_"$suite"_"$arch"
-    done
-    echo " ********************************************************************** " >> germinate.output
-    echo "" >> germinate.output
-    echo -n "."
-
-    ## Generate apt-ftparchive 'extra' overrides for Task: fields
-    for seed in $taskseeds; do
-      if ! grep -iq '^Task-' "$seed.seedtext"; then
-	continue
-      fi
-      # If the seed contains Task-Name header, override the normal behavior
-      if grep -iq '^Task-Name:' "$seed.seedtext"; then
-	task=$(grep '^Task-Name:' "$seed.seedtext" | cut -d: -f2)
-      elif grep -iq '^Task-Per-Derivative:' "$seed.seedtext"; then
-	task="$distro-$seed"
-      else
-	# If a seed is not per-derivative, then we only honour it for Ubuntu,
-	# and its task name is archive-global.
-	if [ "$distro" = ubuntu ]; then
-	  task="$seed"
-	else
-	  continue
-	fi
-      fi
-      if grep -iq '^Task-Seeds:' "$seed.seedtext"; then
-          scanseeds="$( (grep '^Task-Seeds:' "$seed.seedtext" | cut -d: -f2 | xargs -n1; echo "$seed") | sort -u )"
-      else
-	scanseeds="$seed"
-      fi
-      for scanseed in $scanseeds; do
-          egrep -v -- \
-              "^(-|Package| )" "$scanseed"_"$distro"_"$suite"_"$arch" |
-          awk '{print $1}' |
-          sed -e "s,$,/$arch  Task  $task," |
-          sort -u >> "$MISCROOT/more-extra.override.$suite.main.new"
-      done
-    done
-
-    # Generate apt-ftparchive 'extra' overrides for Build-Essential: fields
-    if [ -e build-essential ] && [ "$distro" = ubuntu ]; then
-        # Keep a copy, just for convenience
-        cp build-essential build-essential_"$distro"_"$suite"_"$arch"
-        egrep -v -- \
-            "^(-|Package| )" build-essential_"$distro"_"$suite"_"$arch" |
-        awk '{print $1}' |
-        sed -e "s,$,/$arch  Build-Essential  yes," |
-        sort -u >> "$MISCROOT/more-extra.override.$suite.main.new"
-    fi
-  done
-done
-echo " done."
-
-mv -f \
-    "$MISCROOT/more-extra.override.$suite.main.new" \
-    "$MISCROOT/more-extra.override.$suite.main"
+$GENERATE -d ubuntu $FLAVOURS
 
 # Now generate the Supported extra overrides for all supported distros.
 SUITES=`$LAUNCHPADROOT/scripts/ftpmaster-tools/lp-query-distro.py supported`

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-12-06 21:10:57 +0000
+++ database/schema/security.cfg	2011-12-14 15:49:40 +0000
@@ -2266,6 +2266,10 @@
 type=user
 groups=archivepublisher
 
+[generate_extra_overrides]
+type=user
+groups=archivepublisher
+
 [process_death_row]
 type=user
 groups=archivepublisher

=== modified file 'lib/lp/archivepublisher/config.py'
--- lib/lp/archivepublisher/config.py	2011-08-29 16:43:10 +0000
+++ lib/lp/archivepublisher/config.py	2011-12-14 15:49:40 +0000
@@ -74,10 +74,12 @@
         pubconf.overrideroot = pubconf.archiveroot + '-overrides'
         pubconf.cacheroot = pubconf.archiveroot + '-cache'
         pubconf.miscroot = pubconf.archiveroot + '-misc'
+        pubconf.germinateroot = pubconf.archiveroot + '-germinate'
     else:
         pubconf.overrideroot = None
         pubconf.cacheroot = None
         pubconf.miscroot = None
+        pubconf.germinateroot = None
 
     pubconf.poolroot = os.path.join(pubconf.archiveroot, 'pool')
     pubconf.distsroot = os.path.join(pubconf.archiveroot, 'dists')
@@ -106,6 +108,7 @@
             self.cacheroot,
             self.overrideroot,
             self.miscroot,
+            self.germinateroot,
             self.temproot,
             ]
 

=== added file 'lib/lp/archivepublisher/scripts/generate_extra_overrides.py'
--- lib/lp/archivepublisher/scripts/generate_extra_overrides.py	1970-01-01 00:00:00 +0000
+++ lib/lp/archivepublisher/scripts/generate_extra_overrides.py	2011-12-14 15:49:40 +0000
@@ -0,0 +1,339 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Generate extra overrides using Germinate."""
+
+__metaclass__ = type
+__all__ = [
+    'GenerateExtraOverrides',
+    ]
+
+from functools import partial
+import logging
+from optparse import OptionValueError
+import os
+import re
+
+from germinate.germinator import Germinator
+from germinate.archive import TagFile
+from germinate.log import GerminateFormatter
+from germinate.seeds import SeedStructure
+
+from zope.component import getUtility
+
+from canonical.launchpad.webapp.dbpolicy import (
+    DatabaseBlockedPolicy,
+    SlaveOnlyDatabasePolicy,
+    )
+from lp.archivepublisher.config import getPubConfig
+from lp.registry.interfaces.distribution import IDistributionSet
+from lp.registry.interfaces.series import SeriesStatus
+from lp.services.scripts.base import (
+    LaunchpadScript,
+    LaunchpadScriptFailure,
+    )
+from lp.services.utils import file_exists
+
+
+class AtomicFile:
+    """Facilitate atomic writing of files."""
+
+    def __init__(self, filename):
+        self.filename = filename
+        self.fd = open("%s.new" % self.filename, "w")
+
+    def __enter__(self):
+        return self.fd
+
+    def __exit__(self, exc_type, exc_value, exc_tb):
+        self.fd.close()
+        if exc_type is None:
+            os.rename("%s.new" % self.filename, self.filename)
+
+
+def find_operable_series(distribution):
+    """Find a series we can operate on in this distribution.
+
+    We are allowed to modify DEVELOPMENT or FROZEN series, but should leave
+    series with any other status alone.
+    """
+    series = distribution.currentseries
+    if series.status in (SeriesStatus.DEVELOPMENT, SeriesStatus.FROZEN):
+        return series
+    else:
+        return None
+
+
+class GenerateExtraOverrides(LaunchpadScript):
+    """Main class for scripts/ftpmaster-tools/generate-task-overrides.py."""
+
+    def __init__(self, *args, **kwargs):
+        super(GenerateExtraOverrides, self).__init__(*args, **kwargs)
+        self.germinate_logger = None
+
+    def add_my_options(self):
+        """Add a 'distribution' context option."""
+        self.parser.add_option(
+            "-d", "--distribution", dest="distribution",
+            help="Context distribution name.")
+
+    @property
+    def name(self):
+        """See `LaunchpadScript`."""
+        # Include distribution name.  Clearer to admins, but also
+        # puts runs for different distributions under separate
+        # locks so that they can run simultaneously.
+        return "%s-%s" % (self._name, self.options.distribution)
+
+    def processOptions(self):
+        """Handle command-line options."""
+        if self.options.distribution is None:
+            raise OptionValueError("Specify a distribution.")
+
+        self.distribution = getUtility(IDistributionSet).getByName(
+            self.options.distribution)
+        if self.distribution is None:
+            raise OptionValueError(
+                "Distribution '%s' not found." % self.options.distribution)
+
+        self.series = find_operable_series(self.distribution)
+        if self.series is None:
+            raise LaunchpadScriptFailure(
+                "There is no DEVELOPMENT distroseries for %s." %
+                self.options.distribution)
+
+        # Even if DistroSeries.component_names starts including partner, we
+        # don't want it; this applies to the primary archive only.
+        self.components = [component
+                           for component in self.series.component_names
+                           if component != "partner"]
+
+    def getConfig(self):
+        """Set up a configuration object for this archive."""
+        archive = self.distribution.main_archive
+        if archive:
+            return getPubConfig(archive)
+        else:
+            raise LaunchpadScriptFailure(
+                "There is no PRIMARY archive for %s." %
+                self.options.distribution)
+
+    def setUpDirs(self):
+        """Create output directories if they did not already exist."""
+        germinateroot = self.config.germinateroot
+        if not file_exists(germinateroot):
+            self.logger.debug("Creating germinate root %s.", germinateroot)
+            os.makedirs(germinateroot)
+        miscroot = self.config.miscroot
+        if not file_exists(miscroot):
+            self.logger.debug("Creating misc root %s.", miscroot)
+            os.makedirs(miscroot)
+
+    def addLogHandler(self):
+        """Send germinate's log output to a separate file."""
+        if self.germinate_logger is not None:
+            return
+
+        self.germinate_logger = logging.getLogger("germinate")
+        self.germinate_logger.setLevel(logging.INFO)
+        log_file = os.path.join(self.config.germinateroot, "germinate.output")
+        handler = logging.FileHandler(log_file, mode="w")
+        handler.setFormatter(GerminateFormatter())
+        self.germinate_logger.addHandler(handler)
+        self.germinate_logger.propagate = False
+
+    def setUp(self):
+        """Process options, and set up internal state."""
+        self.processOptions()
+        self.config = self.getConfig()
+        self.setUpDirs()
+        self.addLogHandler()
+
+    def makeSeedStructures(self, series_name, flavours, seed_bases=None):
+        structures = {}
+        for flavour in flavours:
+            structures[flavour] = SeedStructure(
+                "%s.%s" % (flavour, series_name), seed_bases=seed_bases)
+        return structures
+
+    def logGerminateProgress(self, *args):
+        """Log a "progress" entry to the germinate log file.
+
+        Germinate logs quite a bit of detailed information.  To make it
+        easier to see the structure of its operation, GerminateFormatter
+        allows tagging some log entries as "progress" entries, which are
+        printed without a prefix.
+        """
+        self.germinate_logger.info(*args, extra={"progress": True})
+
+    def composeOutputPath(self, flavour, series_name, arch, base):
+        return os.path.join(
+            self.config.germinateroot,
+            "%s_%s_%s_%s" % (base, flavour, series_name, arch))
+
+    def writeGerminateOutput(self, germinator, structure, flavour,
+                             series_name, arch):
+        """Write dependency-expanded output files.
+
+        These files are a reduced subset of those written by the germinate
+        command-line program.
+        """
+        path = partial(self.composeOutputPath, flavour, series_name, arch)
+
+        # The structure file makes it possible to figure out how the other
+        # output files relate to each other.
+        structure.write(path("structure"))
+
+        # "all" and "all.sources" list the full set of binary and source
+        # packages respectively for a given flavour/suite/architecture
+        # combination.
+        germinator.write_all_list(structure, path("all"))
+        germinator.write_all_source_list(structure, path("all.sources"))
+
+        # Write the dependency-expanded output for each seed.  Several of
+        # these are used by archive administration tools, and others are
+        # useful for debugging, so it's best to just write them all.
+        for seedname in structure.names:
+            germinator.write_full_list(structure, path(seedname), seedname)
+
+    def parseTaskHeaders(self, seedtext):
+        """Parse a seed for Task headers.
+
+        seedtext is a file-like object.  Return a dictionary of Task headers,
+        with keys canonicalised to lower-case.
+        """
+        task_headers = {}
+        task_header_regex = re.compile(
+            r"task-(.*?):(.*)", flags=re.IGNORECASE)
+        for line in seedtext:
+            match = task_header_regex.match(line)
+            if match is not None:
+                key, value = match.groups()
+                task_headers[key.lower()] = value.strip()
+        return task_headers
+
+    def getTaskName(self, task_headers, flavour, seedname, primary_flavour):
+        """Work out the name of the Task to be generated from this seed.
+
+        If there is a Task-Name header, it wins; otherwise, seeds with a
+        Task-Per-Derivative header are honoured for all flavours and put in
+        an appropriate namespace, while other seeds are only honoured for
+        the first flavour and have archive-global names.
+        """
+        if "name" in task_headers:
+            return task_headers["name"]
+        elif "per-derivative" in task_headers:
+            return "%s-%s" % (flavour, seedname)
+        elif primary_flavour:
+            return seedname
+        else:
+            return None
+
+    def getTaskSeeds(self, task_headers, seedname):
+        """Return the list of seeds used to generate a task from this seed.
+
+        The list of packages in this task comes from this seed plus any
+        other seeds listed in a Task-Seeds header.
+        """
+        scan_seeds = set([seedname])
+        if "seeds" in task_headers:
+            scan_seeds.update(task_headers["seeds"].split())
+        return sorted(scan_seeds)
+
+    def writeOverrides(self, override_file, germinator, structure, arch,
+                       seedname, key, value):
+        packages = germinator.get_full(structure, seedname)
+        for package in sorted(packages):
+            print >>override_file, "%s/%s  %s  %s" % (
+                package, arch, key, value)
+
+    def germinateArchFlavour(self, override_file, germinator, series_name,
+                             arch, flavour, structure, primary_flavour):
+        """Germinate seeds on a single flavour for a single architecture."""
+        # Expand dependencies.
+        germinator.plant_seeds(structure)
+        germinator.grow(structure)
+        germinator.add_extras(structure)
+
+        self.writeGerminateOutput(germinator, structure, flavour, series_name,
+                                  arch)
+
+        write_overrides = partial(
+            self.writeOverrides, override_file, germinator, structure, arch)
+
+        # Generate apt-ftparchive "extra overrides" for Task fields.
+        seednames = [name for name in structure.names if name != "extra"]
+        for seedname in seednames:
+            with structure[seedname] as seedtext:
+                task_headers = self.parseTaskHeaders(seedtext)
+            if task_headers:
+                task = self.getTaskName(
+                    task_headers, flavour, seedname, primary_flavour)
+                if task is not None:
+                    scan_seeds = self.getTaskSeeds(task_headers, seedname)
+                    for scan_seed in scan_seeds:
+                        write_overrides(scan_seed, "Task", task)
+
+        # Generate apt-ftparchive "extra overrides" for Build-Essential
+        # fields.
+        if "build-essential" in structure.names and primary_flavour:
+            write_overrides("build-essential", "Build-Essential", "yes")
+
+    def germinateArch(self, override_file, series_name, arch, flavours,
+                      structures):
+        """Germinate seeds on all flavours for a single architecture."""
+        germinator = Germinator(arch)
+
+        # Read archive metadata.
+        archive = TagFile(
+            series_name, self.components, arch,
+            "file://%s" % self.config.archiveroot, cleanup=True)
+        germinator.parse_archive(archive)
+
+        for flavour in flavours:
+            self.logger.info(
+                "Germinating for %s/%s/%s", flavour, series_name, arch)
+            # Add this to the germinate log as well so that that can be
+            # debugged more easily.  Log a separator line first.
+            self.logGerminateProgress("")
+            self.logGerminateProgress(
+                "Germinating for %s/%s/%s", flavour, series_name, arch)
+
+            self.germinateArchFlavour(
+                override_file, germinator, series_name, arch, flavour,
+                structures[flavour], flavour == flavours[0])
+
+    def generateExtraOverrides(self, series_name, series_architectures,
+                               flavours, seed_bases=None):
+        structures = self.makeSeedStructures(
+            series_name, flavours, seed_bases=seed_bases)
+
+        override_path = os.path.join(
+            self.config.miscroot,
+            "more-extra.override.%s.main" % series_name)
+        with AtomicFile(override_path) as override_file:
+            for arch in series_architectures:
+                self.germinateArch(
+                    override_file, series_name, arch, flavours, structures)
+
+    def process(self, seed_bases=None):
+        """Do the bulk of the work."""
+        self.setUp()
+
+        series_name = self.series.name
+        series_architectures = sorted(
+            [arch.architecturetag for arch in self.series.architectures])
+
+        # This takes a while.  Ensure that we do it without keeping a
+        # database transaction open.
+        self.txn.commit()
+        with DatabaseBlockedPolicy():
+            self.generateExtraOverrides(
+                series_name, series_architectures, self.args,
+                seed_bases=seed_bases)
+
+    def main(self):
+        """See `LaunchpadScript`."""
+        # This code has no need to alter the database.
+        with SlaveOnlyDatabasePolicy():
+            self.process()

=== modified file 'lib/lp/archivepublisher/tests/publisher-config.txt'
--- lib/lp/archivepublisher/tests/publisher-config.txt	2010-10-17 13:35:20 +0000
+++ lib/lp/archivepublisher/tests/publisher-config.txt	2011-12-14 15:49:40 +0000
@@ -14,6 +14,7 @@
     ...     'overrideroot',
     ...     'cacheroot',
     ...     'miscroot',
+    ...     'germinateroot',
     ...     'temproot',
     ... ]
 
@@ -38,6 +39,7 @@
     overrideroot:    /var/tmp/archive/ubuntutest-overrides
     cacheroot:       /var/tmp/archive/ubuntutest-cache
     miscroot:        /var/tmp/archive/ubuntutest-misc
+    germinateroot:   /var/tmp/archive/ubuntutest-germinate
     temproot:        /var/tmp/archive/ubuntutest-temp
 
 
@@ -80,6 +82,7 @@
     overrideroot:    None
     cacheroot:       None
     miscroot:        None
+    germinateroot:   None
     temproot:        /var/tmp/archive/ubuntutest-temp
 
 There is a separate location for private PPAs that is used if the
@@ -108,6 +111,7 @@
     overrideroot:    None
     cacheroot:       None
     miscroot:        None
+    germinateroot:   None
     temproot:        /var/tmp/archive/ubuntutest-temp
 
 
@@ -131,6 +135,7 @@
     overrideroot:    None
     cacheroot:       None
     miscroot:        None
+    germinateroot:   None
     temproot:        /var/tmp/archive/ubuntutest-temp
 
 
@@ -155,6 +160,7 @@
     overrideroot:    None
     cacheroot:       None
     miscroot:        None
+    germinateroot:   None
     temproot:        /var/tmp/archive/ubuntutest-temp
 
 
@@ -177,4 +183,5 @@
     overrideroot:    /var/tmp/archive/ubuntutest-rebuildtest99/ubuntutest-overrides
     cacheroot:       /var/tmp/archive/ubuntutest-rebuildtest99/ubuntutest-cache
     miscroot:        /var/tmp/archive/ubuntutest-rebuildtest99/ubuntutest-misc
+    germinateroot:   /var/tmp/archive/ubuntutest-rebuildtest99/ubuntutest-germinate
     temproot:        /var/tmp/archive/ubuntutest-rebuildtest99/ubuntutest-temp

=== added file 'lib/lp/archivepublisher/tests/test_generate_extra_overrides.py'
--- lib/lp/archivepublisher/tests/test_generate_extra_overrides.py	1970-01-01 00:00:00 +0000
+++ lib/lp/archivepublisher/tests/test_generate_extra_overrides.py	2011-12-14 15:49:40 +0000
@@ -0,0 +1,567 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test for the `generate-extra-overrides` script."""
+
+__metaclass__ = type
+
+import logging
+from optparse import OptionValueError
+import os
+import tempfile
+
+from germinate import (
+    archive,
+    germinator,
+    seeds,
+    )
+
+import transaction
+
+from canonical.testing.layers import (
+    LaunchpadZopelessLayer,
+    ZopelessDatabaseLayer,
+    )
+from lp.archivepublisher.scripts.generate_extra_overrides import (
+    AtomicFile,
+    GenerateExtraOverrides,
+    )
+from lp.archivepublisher.utils import RepositoryIndexFile
+from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.series import SeriesStatus
+from lp.services.log.logger import DevNullLogger
+from lp.services.osutils import (
+    ensure_directory_exists,
+    open_for_writing,
+    )
+from lp.services.scripts.base import LaunchpadScriptFailure
+from lp.services.utils import file_exists
+from lp.soyuz.enums import PackagePublishingStatus
+from lp.testing import TestCaseWithFactory
+from lp.testing.faketransaction import FakeTransaction
+
+
+def file_contents(path):
+    """Return the contents of the file at path."""
+    with open(path) as handle:
+        return handle.read()
+
+
+class TestAtomicFile(TestCaseWithFactory):
+    """Tests for the AtomicFile helper class."""
+
+    layer = ZopelessDatabaseLayer
+
+    def test_atomic_file_creates_file(self):
+        # AtomicFile creates the named file with the requested contents.
+        self.useTempDir()
+        filename = self.factory.getUniqueString()
+        text = self.factory.getUniqueString()
+        with AtomicFile(filename) as test:
+            test.write(text)
+        self.assertEqual(text, file_contents(filename))
+
+    def test_atomic_file_removes_dot_new(self):
+        # AtomicFile does not leave .new files lying around.
+        self.useTempDir()
+        filename = self.factory.getUniqueString()
+        with AtomicFile(filename):
+            pass
+        self.assertFalse(file_exists("%s.new" % filename))
+
+
+class TestGenerateExtraOverrides(TestCaseWithFactory):
+    """Tests for the actual `GenerateExtraOverrides` script."""
+
+    layer = LaunchpadZopelessLayer
+
+    def setUp(self):
+        super(TestGenerateExtraOverrides, self).setUp()
+        self.seeddir = self.makeTemporaryDirectory()
+        # XXX cjwatson 2011-12-06 bug=694140: Make sure germinate doesn't
+        # lose its loggers between tests, due to Launchpad's messing with
+        # global log state.
+        archive._logger = logging.getLogger("germinate.archive")
+        germinator._logger = logging.getLogger("germinate.germinator")
+        seeds._logger = logging.getLogger("germinate.seeds")
+
+    def assertFilesEqual(self, expected_path, observed_path):
+        self.assertEqual(
+            file_contents(expected_path), file_contents(observed_path))
+
+    def makeDistro(self):
+        """Create a distribution for testing.
+
+        The distribution will have a root directory set up, which will
+        be cleaned up after the test.  It will have an attached archive.
+        """
+        return self.factory.makeDistribution(
+            publish_root_dir=unicode(self.makeTemporaryDirectory()))
+
+    def makeScript(self, distribution, run_setup=True, extra_args=None):
+        """Create a script for testing."""
+        test_args = []
+        if distribution is not None:
+            test_args.extend(["-d", distribution.name])
+        if extra_args is not None:
+            test_args.extend(extra_args)
+        script = GenerateExtraOverrides(test_args=test_args)
+        script.logger = DevNullLogger()
+        script.txn = FakeTransaction()
+        if distribution is not None and run_setup:
+            script.setUp()
+        else:
+            script.distribution = distribution
+        return script
+
+    def makePackage(self, component, dases, **kwargs):
+        """Create a published source and binary package for testing."""
+        package = self.factory.makeDistributionSourcePackage(
+            distribution=dases[0].distroseries.distribution)
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=dases[0].distroseries,
+            pocket=PackagePublishingPocket.RELEASE,
+            status=PackagePublishingStatus.PUBLISHED,
+            sourcepackagename=package.name, component=component)
+        for das in dases:
+            build = self.factory.makeBinaryPackageBuild(
+                source_package_release=spph.sourcepackagerelease,
+                distroarchseries=das, processor=das.default_processor)
+            bpr = self.factory.makeBinaryPackageRelease(
+                binarypackagename=package.name, build=build,
+                component=component, **kwargs)
+            lfa = self.factory.makeLibraryFileAlias(
+                filename="%s.deb" % package.name)
+            transaction.commit()
+            bpr.addFile(lfa)
+            self.factory.makeBinaryPackagePublishingHistory(
+                binarypackagerelease=bpr, distroarchseries=das,
+                pocket=PackagePublishingPocket.RELEASE,
+                status=PackagePublishingStatus.PUBLISHED)
+        return package
+
+    def makeIndexFiles(self, script, distroseries):
+        """Create a limited subset of index files for testing."""
+        ensure_directory_exists(script.config.temproot)
+
+        for component in distroseries.components:
+            index_root = os.path.join(
+                script.config.distsroot, distroseries.name, component.name)
+
+            source_index_root = os.path.join(index_root, "source")
+            source_index = RepositoryIndexFile(
+                source_index_root, script.config.temproot, "Sources")
+            for spp in distroseries.getSourcePackagePublishing(
+                PackagePublishingStatus.PUBLISHED,
+                PackagePublishingPocket.RELEASE, component=component):
+                stanza = spp.getIndexStanza().encode("utf-8") + "\n\n"
+                source_index.write(stanza)
+            source_index.close()
+
+            for arch in distroseries.architectures:
+                package_index_root = os.path.join(
+                    index_root, "binary-%s" % arch.architecturetag)
+                package_index = RepositoryIndexFile(
+                    package_index_root, script.config.temproot, "Packages")
+                for bpp in distroseries.getBinaryPackagePublishing(
+                    archtag=arch.architecturetag,
+                    pocket=PackagePublishingPocket.RELEASE,
+                    component=component):
+                    stanza = bpp.getIndexStanza().encode("utf-8") + "\n\n"
+                    package_index.write(stanza)
+                package_index.close()
+
+    def composeSeedPath(self, flavour, series_name, seed_name):
+        return os.path.join(
+            self.seeddir, "%s.%s" % (flavour, series_name), seed_name)
+
+    def makeSeedStructure(self, flavour, series_name, seed_names,
+                          seed_inherit=None):
+        """Create a simple seed structure file."""
+        if seed_inherit is None:
+            seed_inherit = {}
+
+        structure_path = self.composeSeedPath(
+            flavour, series_name, "STRUCTURE")
+        with open_for_writing(structure_path, "w") as structure:
+            for seed_name in seed_names:
+                inherit = seed_inherit.get(seed_name, [])
+                line = "%s: %s" % (seed_name, " ".join(inherit))
+                print >>structure, line.strip()
+
+    def makeSeed(self, flavour, series_name, seed_name, entries,
+                 headers=None):
+        """Create a simple seed file."""
+        seed_path = self.composeSeedPath(flavour, series_name, seed_name)
+        with open_for_writing(seed_path, "w") as seed:
+            if headers is not None:
+                for header in headers:
+                    print >>seed, header
+                print >>seed
+            for entry in entries:
+                print >>seed, " * %s" % entry
+
+    def getTaskNameFromSeed(self, script, flavour, series_name, seed,
+                            primary_flavour):
+        """Use script to parse a seed and return its task name."""
+        seed_path = self.composeSeedPath(flavour, series_name, seed)
+        with open(seed_path) as seed_text:
+            task_headers = script.parseTaskHeaders(seed_text)
+        return script.getTaskName(
+            task_headers, flavour, seed, primary_flavour)
+
+    def getTaskSeedsFromSeed(self, script, flavour, series_name, seed):
+        """Use script to parse a seed and return its task seed list."""
+        seed_path = self.composeSeedPath(flavour, series_name, seed)
+        with open(seed_path) as seed_text:
+            task_headers = script.parseTaskHeaders(seed_text)
+        return script.getTaskSeeds(task_headers, seed)
+
+    def test_name_is_consistent(self):
+        # Script instances for the same distro get the same name.
+        distro = self.factory.makeDistribution()
+        self.assertEqual(
+            GenerateExtraOverrides(test_args=["-d", distro.name]).name,
+            GenerateExtraOverrides(test_args=["-d", distro.name]).name)
+
+    def test_name_is_unique_for_each_distro(self):
+        # Script instances for different distros get different names.
+        self.assertNotEqual(
+            GenerateExtraOverrides(
+                test_args=["-d", self.factory.makeDistribution().name]).name,
+            GenerateExtraOverrides(
+                test_args=["-d", self.factory.makeDistribution().name]).name)
+
+    def test_requires_distro(self):
+        # The --distribution or -d argument is mandatory.
+        script = self.makeScript(None)
+        self.assertRaises(OptionValueError, script.processOptions)
+
+    def test_requires_real_distro(self):
+        # An incorrect distribution name is flagged as an invalid option
+        # value.
+        script = self.makeScript(
+            None, extra_args=["-d", self.factory.getUniqueString()])
+        self.assertRaises(OptionValueError, script.processOptions)
+
+    def test_looks_up_distro(self):
+        # The script looks up and keeps the distribution named on the
+        # command line.
+        distro = self.makeDistro()
+        self.factory.makeDistroSeries(distro)
+        script = self.makeScript(distro)
+        self.assertEqual(distro, script.distribution)
+
+    def test_prefers_development_distro_series(self):
+        # The script prefers a DEVELOPMENT series for the named
+        # distribution over CURRENT and SUPPORTED series.
+        distro = self.makeDistro()
+        self.factory.makeDistroSeries(distro, status=SeriesStatus.SUPPORTED)
+        self.factory.makeDistroSeries(distro, status=SeriesStatus.CURRENT)
+        development_distroseries = self.factory.makeDistroSeries(
+            distro, status=SeriesStatus.DEVELOPMENT)
+        script = self.makeScript(distro)
+        self.assertEqual(development_distroseries, script.series)
+
+    def test_permits_frozen_distro_series(self):
+        # If there is no DEVELOPMENT series, a FROZEN one will do.
+        distro = self.makeDistro()
+        self.factory.makeDistroSeries(distro, status=SeriesStatus.SUPPORTED)
+        self.factory.makeDistroSeries(distro, status=SeriesStatus.CURRENT)
+        frozen_distroseries = self.factory.makeDistroSeries(
+            distro, status=SeriesStatus.FROZEN)
+        script = self.makeScript(distro)
+        self.assertEqual(frozen_distroseries, script.series)
+
+    def test_requires_development_frozen_distro_series(self):
+        # If there is no DEVELOPMENT or FROZEN series, the script fails.
+        distro = self.makeDistro()
+        self.factory.makeDistroSeries(distro, status=SeriesStatus.SUPPORTED)
+        self.factory.makeDistroSeries(distro, status=SeriesStatus.CURRENT)
+        script = self.makeScript(distro, run_setup=False)
+        self.assertRaises(LaunchpadScriptFailure, script.processOptions)
+
+    def test_components_exclude_partner(self):
+        # If a 'partner' component exists, it is excluded.
+        distro = self.makeDistro()
+        distroseries = self.factory.makeDistroSeries(distro)
+        self.factory.makeComponentSelection(
+            distroseries=distroseries, component="main")
+        self.factory.makeComponentSelection(
+            distroseries=distroseries, component="partner")
+        script = self.makeScript(distro)
+        self.assertEqual(["main"], script.components)
+
+    def test_compose_output_path_in_germinateroot(self):
+        # Output files are written to the correct locations under
+        # germinateroot.
+        distro = self.makeDistro()
+        distroseries = self.factory.makeDistroSeries(distro)
+        script = self.makeScript(distro)
+        flavour = self.factory.getUniqueString()
+        arch = self.factory.getUniqueString()
+        base = self.factory.getUniqueString()
+        output = script.composeOutputPath(
+            flavour, distroseries.name, arch, base)
+        self.assertEqual(
+            "%s/%s_%s_%s_%s" % (
+                script.config.germinateroot, base, flavour, distroseries.name,
+                arch),
+            output)
+
+    def fetchGerminatedOverrides(self, script, series_name, arch, flavours):
+        """Helper to call script.germinateArch and return overrides."""
+        structures = script.makeSeedStructures(
+            series_name, flavours, seed_bases=["file://%s" % self.seeddir])
+
+        override_fd, override_path = tempfile.mkstemp()
+        with os.fdopen(override_fd, "w") as override_file:
+            script.germinateArch(
+                override_file, series_name, arch, flavours, structures)
+        return file_contents(override_path).splitlines()
+
+    def test_germinate_output(self):
+        # A single call to germinateArch produces output for all flavours on
+        # one architecture.
+        distro = self.makeDistro()
+        distroseries = self.factory.makeDistroSeries(distribution=distro)
+        series_name = distroseries.name
+        component = self.factory.makeComponent()
+        self.factory.makeComponentSelection(
+            distroseries=distroseries, component=component)
+        das = self.factory.makeDistroArchSeries(distroseries=distroseries)
+        arch = das.architecturetag
+        one = self.makePackage(component, [das])
+        two = self.makePackage(component, [das])
+        script = self.makeScript(distro)
+        self.makeIndexFiles(script, distroseries)
+
+        flavour_one = self.factory.getUniqueString()
+        flavour_two = self.factory.getUniqueString()
+        seed = self.factory.getUniqueString()
+        self.makeSeedStructure(flavour_one, series_name, [seed])
+        self.makeSeed(flavour_one, series_name, seed, [one.name])
+        self.makeSeedStructure(flavour_two, series_name, [seed])
+        self.makeSeed(flavour_two, series_name, seed, [two.name])
+
+        overrides = self.fetchGerminatedOverrides(
+            script, series_name, arch, [flavour_one, flavour_two])
+        self.assertEqual([], overrides)
+
+        seed_dir_one = os.path.join(
+            self.seeddir, "%s.%s" % (flavour_one, series_name))
+        self.assertFilesEqual(
+            os.path.join(seed_dir_one, "STRUCTURE"),
+            script.composeOutputPath(
+                flavour_one, series_name, arch, "structure"))
+        self.assertTrue(file_exists(script.composeOutputPath(
+            flavour_one, series_name, arch, "all")))
+        self.assertTrue(file_exists(script.composeOutputPath(
+            flavour_one, series_name, arch, "all.sources")))
+        self.assertTrue(file_exists(script.composeOutputPath(
+            flavour_one, series_name, arch, seed)))
+
+        seed_dir_two = os.path.join(
+            self.seeddir, "%s.%s" % (flavour_two, series_name))
+        self.assertFilesEqual(
+            os.path.join(seed_dir_two, "STRUCTURE"),
+            script.composeOutputPath(
+                flavour_two, series_name, arch, "structure"))
+        self.assertTrue(file_exists(script.composeOutputPath(
+            flavour_two, series_name, arch, "all")))
+        self.assertTrue(file_exists(script.composeOutputPath(
+            flavour_two, series_name, arch, "all.sources")))
+        self.assertTrue(file_exists(script.composeOutputPath(
+            flavour_two, series_name, arch, seed)))
+
+    def test_germinate_output_task(self):
+        # germinateArch produces Task extra overrides.
+        distro = self.makeDistro()
+        distroseries = self.factory.makeDistroSeries(distribution=distro)
+        series_name = distroseries.name
+        component = self.factory.makeComponent()
+        self.factory.makeComponentSelection(
+            distroseries=distroseries, component=component)
+        das = self.factory.makeDistroArchSeries(distroseries=distroseries)
+        arch = das.architecturetag
+        one = self.makePackage(component, [das])
+        two = self.makePackage(component, [das], depends=one.name)
+        three = self.makePackage(component, [das])
+        self.makePackage(component, [das])
+        script = self.makeScript(distro)
+        self.makeIndexFiles(script, distroseries)
+
+        flavour = self.factory.getUniqueString()
+        seed_one = self.factory.getUniqueString()
+        seed_two = self.factory.getUniqueString()
+        self.makeSeedStructure(flavour, series_name, [seed_one, seed_two])
+        self.makeSeed(
+            flavour, series_name, seed_one, [two.name],
+            headers=["Task-Description: one"])
+        self.makeSeed(
+            flavour, series_name, seed_two, [three.name],
+            headers=["Task-Description: two"])
+
+        overrides = self.fetchGerminatedOverrides(
+            script, series_name, arch, [flavour])
+        expected_overrides = [
+            "%s/%s  Task  %s" % (one.name, arch, seed_one),
+            "%s/%s  Task  %s" % (two.name, arch, seed_one),
+            "%s/%s  Task  %s" % (three.name, arch, seed_two),
+            ]
+        self.assertContentEqual(expected_overrides, overrides)
+
+    def test_task_name(self):
+        # The Task-Name field is honoured.
+        series_name = self.factory.getUniqueString()
+        package = self.factory.getUniqueString()
+        script = self.makeScript(None)
+
+        flavour = self.factory.getUniqueString()
+        seed = self.factory.getUniqueString()
+        task = self.factory.getUniqueString()
+        self.makeSeed(
+            flavour, series_name, seed, [package],
+            headers=["Task-Name: %s" % task])
+
+        observed_task = self.getTaskNameFromSeed(
+            script, flavour, series_name, seed, True)
+        self.assertEqual(task, observed_task)
+
+    def test_task_per_derivative(self):
+        # The Task-Per-Derivative field is honoured.
+        series_name = self.factory.getUniqueString()
+        package = self.factory.getUniqueString()
+        script = self.makeScript(None)
+
+        flavour_one = self.factory.getUniqueString()
+        flavour_two = self.factory.getUniqueString()
+        seed_one = self.factory.getUniqueString()
+        seed_two = self.factory.getUniqueString()
+        self.makeSeed(
+            flavour_one, series_name, seed_one, [package],
+            headers=["Task-Description: one"])
+        self.makeSeed(
+            flavour_one, series_name, seed_two, [package],
+            headers=["Task-Per-Derivative: 1"])
+        self.makeSeed(
+            flavour_two, series_name, seed_one, [package],
+            headers=["Task-Description: one"])
+        self.makeSeed(
+            flavour_two, series_name, seed_two, [package],
+            headers=["Task-Per-Derivative: 1"])
+
+        observed_task_one_one = self.getTaskNameFromSeed(
+            script, flavour_one, series_name, seed_one, True)
+        observed_task_one_two = self.getTaskNameFromSeed(
+            script, flavour_one, series_name, seed_two, True)
+        observed_task_two_one = self.getTaskNameFromSeed(
+            script, flavour_two, series_name, seed_one, False)
+        observed_task_two_two = self.getTaskNameFromSeed(
+            script, flavour_two, series_name, seed_two, False)
+
+        # seed_one is not per-derivative, so it is honoured only for
+        # flavour_one and has a global name.
+        self.assertEqual(seed_one, observed_task_one_one)
+        self.assertIsNone(observed_task_two_one)
+
+        # seed_two is per-derivative, so it is honoured for both flavours
+        # and has the flavour name prefixed.
+        self.assertEqual(
+            "%s-%s" % (flavour_one, seed_two), observed_task_one_two)
+        self.assertEqual(
+            "%s-%s" % (flavour_two, seed_two), observed_task_two_two)
+
+    def test_task_seeds(self):
+        # The Task-Seeds field is honoured.
+        series_name = self.factory.getUniqueString()
+        one = self.getUniqueString()
+        two = self.getUniqueString()
+        script = self.makeScript(None)
+
+        flavour = self.factory.getUniqueString()
+        seed_one = self.factory.getUniqueString()
+        seed_two = self.factory.getUniqueString()
+        self.makeSeed(flavour, series_name, seed_one, [one])
+        self.makeSeed(
+            flavour, series_name, seed_two, [two],
+            headers=["Task-Seeds: %s" % seed_one])
+
+        task_seeds = self.getTaskSeedsFromSeed(
+            script, flavour, series_name, seed_two)
+        self.assertContentEqual([seed_one, seed_two], task_seeds)
+
+    def test_germinate_output_build_essential(self):
+        # germinateArch produces Build-Essential extra overrides.
+        distro = self.makeDistro()
+        distroseries = self.factory.makeDistroSeries(distribution=distro)
+        series_name = distroseries.name
+        component = self.factory.makeComponent()
+        self.factory.makeComponentSelection(
+            distroseries=distroseries, component=component)
+        das = self.factory.makeDistroArchSeries(distroseries=distroseries)
+        arch = das.architecturetag
+        package = self.makePackage(component, [das])
+        script = self.makeScript(distro)
+        self.makeIndexFiles(script, distroseries)
+
+        flavour = self.factory.getUniqueString()
+        seed = "build-essential"
+        self.makeSeedStructure(flavour, series_name, [seed])
+        self.makeSeed(flavour, series_name, seed, [package.name])
+
+        overrides = self.fetchGerminatedOverrides(
+            script, series_name, arch, [flavour])
+        self.assertContentEqual(
+            ["%s/%s  Build-Essential  yes" % (package.name, arch)], overrides)
+
+    def test_main(self):
+        # If run end-to-end, the script generates override files containing
+        # output for all architectures, and sends germinate's log output to
+        # a file.
+        distro = self.makeDistro()
+        distroseries = self.factory.makeDistroSeries(distribution=distro)
+        series_name = distroseries.name
+        component = self.factory.makeComponent()
+        self.factory.makeComponentSelection(
+            distroseries=distroseries, component=component)
+        das_one = self.factory.makeDistroArchSeries(distroseries=distroseries)
+        arch_one = das_one.architecturetag
+        das_two = self.factory.makeDistroArchSeries(distroseries=distroseries)
+        arch_two = das_two.architecturetag
+        package = self.makePackage(component, [das_one, das_two])
+        flavour = self.factory.getUniqueString()
+        script = self.makeScript(distro, extra_args=[flavour])
+        self.makeIndexFiles(script, distroseries)
+
+        seed = self.factory.getUniqueString()
+        self.makeSeedStructure(flavour, series_name, [seed])
+        self.makeSeed(
+            flavour, series_name, seed, [package.name],
+            headers=["Task-Description: task"])
+
+        script.process(seed_bases=["file://%s" % self.seeddir])
+        override_path = os.path.join(
+            script.config.miscroot,
+            "more-extra.override.%s.main" % series_name)
+        expected_overrides = [
+            "%s/%s  Task  %s" % (package.name, arch_one, seed),
+            "%s/%s  Task  %s" % (package.name, arch_two, seed),
+            ]
+        self.assertContentEqual(
+            expected_overrides, file_contents(override_path).splitlines())
+
+        log_file = os.path.join(
+            script.config.germinateroot, "germinate.output")
+        self.assertIn("Downloading file://", file_contents(log_file))
+
+    def test_run_script(self):
+        # The script will run stand-alone.
+        from canonical.launchpad.scripts.tests import run_script
+        distro = self.makeDistro()
+        self.factory.makeDistroSeries(distro)
+        transaction.commit()
+        retval, out, err = run_script(
+            "cronscripts/generate-extra-overrides.py",
+            ["-d", distro.name, "-q"])
+        self.assertEqual(0, retval)

=== removed file 'lib/lp/soyuz/scripts/tests/germinate-test-data/mock-bin/germinate'
--- lib/lp/soyuz/scripts/tests/germinate-test-data/mock-bin/germinate	2010-10-20 13:23:05 +0000
+++ lib/lp/soyuz/scripts/tests/germinate-test-data/mock-bin/germinate	1970-01-01 00:00:00 +0000
@@ -1,5 +0,0 @@
-#!/bin/sh
-#
-# This is a mock germinate script that just produces enough (empty)
-# files so that the cron.germinate shell script can run.
-touch structure all all.sources minimal standard

=== added file 'lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/cronscripts/generate-extra-overrides.py'
--- lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/cronscripts/generate-extra-overrides.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/cronscripts/generate-extra-overrides.py	2011-12-14 15:49:40 +0000
@@ -0,0 +1,5 @@
+#! /usr/bin/python
+#
+# We don't need to run generate-extra-overrides.py when testing
+# maintenance-check.py.  We could, but it would slow down the tests and its
+# output is not interesting here.

=== modified file 'lib/lp/soyuz/scripts/tests/test_cron_germinate.py'
--- lib/lp/soyuz/scripts/tests/test_cron_germinate.py	2011-01-19 17:49:10 +0000
+++ lib/lp/soyuz/scripts/tests/test_cron_germinate.py	2011-12-14 15:49:40 +0000
@@ -148,10 +148,8 @@
             os.path.join(archive_dir, "ubuntu"))
         fake_environ["TEST_LAUNCHPADROOT"] = os.path.abspath(
             os.path.join(basepath, "germinate-test-data/mock-lp-root"))
-        # Set the PATH in the fake environment so that our mock germinate
-        # is used. We could use the real germinate as well, but that will
-        # slow down the tests a lot and its also not interessting for this
-        # test as we do not use any of the germinate information.
+        # Set the PATH in the fake environment so that our mock lockfile is
+        # used.
         fake_environ["PATH"] = "%s:%s" % (
             os.path.abspath(os.path.join(
                 basepath, "germinate-test-data/mock-bin")),