Merge lp:~cjwatson/launchpad/germinate-stale-files into lp:launchpad

[Colin Watson]

Remove stale files from config.germinateroot.

== Summary ==

One of the problems identified in https://wiki.ubuntu.com/FoundationsTeam/ReplaceArchiveAdminShellAccess (and bug 1001517) is that stale files pile up in cocoplum:/srv/launchpad.net/ubuntu-archive/ubuntu-germinate/ from time to time.

== Proposed fix ==

Keep track of the current set of files written for the current series, and remove any not in that set.

== Implementation details ==

Easy, except for LoC. I did some refactoring of test_generate_extra_overrides, which was overdue anyway, but couldn't make it small enough to get the LoC delta non-positive; so I converted an unrelated but nearby doctest to unit tests, bringing me to -14.

== Tests ==

bin/test -vvct archivepublisher.tests.test_config -t archivepublisher.tests.test_generate_extra_overrides

== Demo and Q/A ==

On dogfood:

 cp -a /srv/launchpad.net/ubuntu-archive/ubuntu-germinate /srv/launchpad.net/ubuntu-archive/ubuntu-germinate.stale-files-backup
 touch /srv/launchpad.net/ubuntu-archive/ubuntu-germinate/nonexistentseed_ubuntu_quantal_i386

Then do a full publisher run, and verify that (a) /srv/launchpad.net/ubuntu-archive/ubuntu-germinate/nonexistentseed_ubuntu_quantal_i386 is removed; (b) no other files were removed relative to the backup; (c) many other *_*_quantal_* files are created (since we haven't had a publisher run on dogfood since the last DB restore).

[Benji York (benji)]

Looks good.

The with/pass in test_generate_extra_overrides.py (line 877 of the diff)
confused me for a second. I wonder if an old-fashioned open().close()
wouldn't be easier to understand at first blush. Or maybe even a
"touch" helper function.

review: Approve (code)

[Colin Watson (cjwatson)]

Maybe I'm weird, but I prefer the with/pass pattern myself; however, a
helper function makes sense, and perhaps can be moved somewhere more
common in future. Done.

=== modified file 'lib/lp/archivepublisher/scripts/generate_extra_overrides.py'
--- lib/lp/archivepublisher/scripts/generate_extra_overrides.py	2012-01-03 17:08:40 +0000
+++ lib/lp/archivepublisher/scripts/generate_extra_overrides.py	2012-05-21 19:07:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Generate extra overrides using Germinate."""
@@ -9,6 +9,7 @@
     ]
 
 from functools import partial
+import glob
 import logging
 from optparse import OptionValueError
 import os
@@ -130,8 +131,9 @@
 
         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")
+        self.log_file = os.path.join(
+            self.config.germinateroot, "germinate.output")
+        handler = logging.FileHandler(self.log_file, mode="w")
         handler.setFormatter(GerminateFormatter())
         self.germinate_logger.addHandler(handler)
         self.germinate_logger.propagate = False
@@ -186,8 +188,12 @@
             self.config.germinateroot,
             "%s_%s_%s_%s" % (base, flavour, series_name, arch))
 
+    def recordOutput(self, path, seed_outputs):
+        if seed_outputs is not None:
+            seed_outputs.add(os.path.basename(path))
+
     def writeGerminateOutput(self, germinator, structure, flavour,
-                             series_name, arch):
+                             series_name, arch, seed_outputs=None):
         """Write dependency-expanded output files.
 
         These files are a reduced subset of those written by the germinate
@@ -198,18 +204,22 @@
         # The structure file makes it possible to figure out how the other
         # output files relate to each other.
         structure.write(path("structure"))
+        self.recordOutput(path("structure"), seed_outputs)
 
         # "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"))
+        self.recordOutput(path("all"), seed_outputs)
         germinator.write_all_source_list(structure, path("all.sources"))
+        self.recordOutput(path("all.sources"), seed_outputs)
 
         # 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)
+            self.recordOutput(path(seedname), seed_outputs)
 
     def parseTaskHeaders(self, seedtext):
         """Parse a seed for Task headers.
@@ -263,15 +273,17 @@
                 package, arch, key, value)
 
     def germinateArchFlavour(self, override_file, germinator, series_name,
-                             arch, flavour, structure, primary_flavour):
+                             arch, flavour, structure, primary_flavour,
+                             seed_outputs=None):
         """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)
+        self.writeGerminateOutput(
+            germinator, structure, flavour, series_name, arch,
+            seed_outputs=seed_outputs)
 
         write_overrides = partial(
             self.writeOverrides, override_file, germinator, structure, arch)
@@ -295,7 +307,7 @@
             write_overrides("build-essential", "Build-Essential", "yes")
 
     def germinateArch(self, override_file, series_name, components, arch,
-                      flavours, structures):
+                      flavours, structures, seed_outputs=None):
         """Germinate seeds on all flavours for a single architecture."""
         germinator = Germinator(arch)
 
@@ -316,7 +328,19 @@
 
             self.germinateArchFlavour(
                 override_file, germinator, series_name, arch, flavour,
-                structures[flavour], flavour == flavours[0])
+                structures[flavour], flavour == flavours[0],
+                seed_outputs=seed_outputs)
+
+    def removeStaleOutputs(self, series_name, seed_outputs):
+        """Remove stale outputs for a series.
+
+        Any per-seed outputs not in seed_outputs are considered stale.
+        """
+        all_outputs = glob.glob(
+            os.path.join(self.config.germinateroot, "*_*_%s_*" % series_name))
+        for output in all_outputs:
+            if os.path.basename(output) not in seed_outputs:
+                os.remove(output)
 
     def generateExtraOverrides(self, series_name, components, architectures,
                                flavours, seed_bases=None):
@@ -324,6 +348,7 @@
             series_name, flavours, seed_bases=seed_bases)
 
         if structures:
+            seed_outputs = set()
             override_path = os.path.join(
                 self.config.miscroot,
                 "more-extra.override.%s.main" % series_name)
@@ -331,7 +356,8 @@
                 for arch in architectures:
                     self.germinateArch(
                         override_file, series_name, components, arch,
-                        flavours, structures)
+                        flavours, structures, seed_outputs=seed_outputs)
+            self.removeStaleOutputs(series_name, seed_outputs)
 
     def process(self, seed_bases=None):
         """Do the bulk of the work."""

=== removed file 'lib/lp/archivepublisher/tests/publisher-config.txt'
--- lib/lp/archivepublisher/tests/publisher-config.txt	2011-12-29 05:29:36 +0000
+++ lib/lp/archivepublisher/tests/publisher-config.txt	1970-01-01 00:00:00 +0000
@@ -1,195 +0,0 @@
-Publisher Configuration Provider
-================================
-
-The config module has a function to provide a modified publisher configuration
-that provides the right paths for its publication according to the given
-archive.
-
-We will use a helper function for dumping publisher configurations.
-
-    >>> config_attributes = [
-    ...     'distroroot',
-    ...     'archiveroot',
-    ...     'poolroot',
-    ...     'distsroot',
-    ...     'overrideroot',
-    ...     'cacheroot',
-    ...     'miscroot',
-    ...     'germinateroot',
-    ...     'temproot',
-    ... ]
-
-    >>> def dump_config(config):
-    ...     for attr_name in config_attributes:
-    ...         print '%s: %s' % (attr_name, getattr(config, attr_name))
-
-
-Primary
--------
-
-    >>> from lp.registry.interfaces.distribution import IDistributionSet
-    >>> ubuntutest = getUtility(IDistributionSet)['ubuntutest']
-
-    >>> from lp.archivepublisher.config import getPubConfig
-    >>> primary_config = getPubConfig(ubuntutest.main_archive)
-
-    >>> dump_config(primary_config)
-    distroroot:      /var/tmp/archive
-    archiveroot:     /var/tmp/archive/ubuntutest
-    poolroot:        /var/tmp/archive/ubuntutest/pool
-    distsroot:       /var/tmp/archive/ubuntutest/dists
-    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
-
-
-PPAs
-----
-
-Adjust Celso's PPA to point to a configured distribution and built its
-publisher configuration.
-
-    >>> # cprov 20061127: We should *never* be able to change a PPA
-    >>> # distribution, however 'ubuntu' is not prepared for publication, thus
-    >>> # we have to override the PPA to 'ubuntutest' in order to perform the
-    >>> # tests.
-
-    >>> from lp.registry.interfaces.person import IPersonSet
-    >>> cprov = getUtility(IPersonSet).getByName('cprov')
-    >>> cprov_archive = cprov.archive
-    >>> cprov_archive.distribution = ubuntutest
-
-    >>> ppa_config = getPubConfig(cprov_archive)
-
-The base Archive publication location is set in the current Launchpad
-configuration file:
-
-    >>> from lp.services.config import config
-    >>> ppa_config.distroroot == config.personalpackagearchive.root
-    True
-
-A PPA repository topology will follow:
-
-<PPA_BASE_DIR>/<PERSONNAME>/<DISTRIBUTION>
-
-And some paths are not used for PPA workflow, so they are set to
-None, so they won't get created:
-
-    >>> dump_config(ppa_config)
-    distroroot:      /var/tmp/ppa.test/
-    archiveroot:     /var/tmp/ppa.test/cprov/ppa/ubuntutest
-    poolroot:        /var/tmp/ppa.test/cprov/ppa/ubuntutest/pool
-    distsroot:       /var/tmp/ppa.test/cprov/ppa/ubuntutest/dists
-    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
-archive is marked as private:
-
-    >>> (config.personalpackagearchive.private_root !=
-    ...  config.personalpackagearchive.root)
-    True
-
-    >>> cprov_private_ppa = factory.makeArchive(
-    ...     owner=cprov, name='myprivateppa',
-    ...     distribution=cprov_archive.distribution)
-    >>> cprov_private_ppa.private = True
-    >>> cprov_private_ppa.buildd_secret = "secret"
-    >>> p3a_config = getPubConfig(cprov_private_ppa)
-
-    >>> (p3a_config.distroroot ==
-    ...  config.personalpackagearchive.private_root)
-    True
-
-    >>> dump_config(p3a_config)
-    distroroot:      /var/tmp/ppa
-    archiveroot:     /var/tmp/ppa/cprov/myprivateppa/ubuntutest
-    poolroot:        /var/tmp/ppa/cprov/myprivateppa/ubuntutest/pool
-    distsroot:       /var/tmp/ppa/cprov/myprivateppa/ubuntutest/dists
-    overrideroot:    None
-    cacheroot:       None
-    miscroot:        None
-    germinateroot:   None
-    temproot:        /var/tmp/archive/ubuntutest-temp
-
-
-Partner
--------
-
-The publisher config for PARTNER contains only 'partner' in its
-components.  This prevents non-partner being published in the partner
-archive.
-
-    >>> from lp.soyuz.interfaces.archive import IArchiveSet
-    >>> partner_archive = getUtility(IArchiveSet).getByDistroAndName(
-    ...     ubuntutest, 'partner')
-
-    >>> partner_config = getPubConfig(partner_archive)
-
-    >>> dump_config(partner_config)
-    distroroot:      /var/tmp/archive
-    archiveroot:     /var/tmp/archive/ubuntutest-partner
-    poolroot:        /var/tmp/archive/ubuntutest-partner/pool
-    distsroot:       /var/tmp/archive/ubuntutest-partner/dists
-    overrideroot:    None
-    cacheroot:       None
-    miscroot:        None
-    germinateroot:   None
-    temproot:        /var/tmp/archive/ubuntutest-temp
-
-
-DEBUG
------
-
-The publisher configuration for DEBUG archives points to directories
-besides PRIMARY repository ones, but the distribution part is
-modified to be clearly different than the PRIMARY one.
-
-    >>> from lp.soyuz.enums import ArchivePurpose
-    >>> debug_archive = getUtility(IArchiveSet).new(
-    ...     purpose=ArchivePurpose.DEBUG, owner=ubuntutest.owner,
-    ...     distribution=ubuntutest)
-
-    >>> debug_config = getPubConfig(debug_archive)
-
-    >>> dump_config(debug_config)
-    distroroot:      /var/tmp/archive
-    archiveroot:     /var/tmp/archive/ubuntutest-debug
-    poolroot:        /var/tmp/archive/ubuntutest-debug/pool
-    distsroot:       /var/tmp/archive/ubuntutest-debug/dists
-    overrideroot:    None
-    cacheroot:       None
-    miscroot:        None
-    germinateroot:   None
-    temproot:        /var/tmp/archive/ubuntutest-temp
-
-
-COPY
-----
-
-In the case of copy archives (used for rebuild testing) the archiveroot
-is of the form distroroot/distroname-archivename/distroname
-
-    >>> copy_archive = getUtility(IArchiveSet).new(
-    ...     purpose=ArchivePurpose.COPY, owner=ubuntutest.owner,
-    ...     distribution=ubuntutest, name="rebuildtest99")
-
-    >>> copy_config = getPubConfig(copy_archive)
-
-    >>> dump_config(copy_config)
-    distroroot:    /var/tmp/archive
-    archiveroot:   /var/tmp/archive/ubuntutest-rebuildtest99/ubuntutest
-    poolroot:      /var/tmp/archive/ubuntutest-rebuildtest99/ubuntutest/pool
-    distsroot:     /var/tmp/archive/ubuntutest-rebuildtest99/ubuntutest/dists
-    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

=== modified file 'lib/lp/archivepublisher/tests/test_config.py'
--- lib/lp/archivepublisher/tests/test_config.py	2012-01-01 02:58:52 +0000
+++ lib/lp/archivepublisher/tests/test_config.py	2012-05-21 19:07:38 +0000
@@ -1,11 +1,20 @@
-# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""Test publisher configs handling."""
+"""Test publisher configs handling.
+
+Publisher configuration provides archive-dependent filesystem paths.
+"""
 
 __metaclass__ = type
 
+from zope.component import getUtility
+
 from lp.archivepublisher.config import getPubConfig
+from lp.registry.interfaces.distribution import IDistributionSet
+from lp.services.config import config
+from lp.soyuz.enums import ArchivePurpose
+from lp.soyuz.interfaces.archive import IArchiveSet
 from lp.testing import TestCaseWithFactory
 from lp.testing.layers import ZopelessDatabaseLayer
 
@@ -14,6 +23,152 @@
 
     layer = ZopelessDatabaseLayer
 
+    def setUp(self):
+        super(TestGetPubConfig, self).setUp()
+        self.ubuntutest = getUtility(IDistributionSet)['ubuntutest']
+        self.root = "/var/tmp/archive"
+
     def test_getPubConfig_returns_None_if_no_publisherconfig_found(self):
         archive = self.factory.makeDistribution(no_pubconf=True).main_archive
         self.assertEqual(None, getPubConfig(archive))
+
+    def test_primary_config(self):
+        # Primary archive configuration is correct.
+        primary_config = getPubConfig(self.ubuntutest.main_archive)
+        self.assertEqual(self.root, primary_config.distroroot)
+        archiveroot = self.root + "/ubuntutest"
+        self.assertEqual(archiveroot, primary_config.archiveroot)
+        self.assertEqual(archiveroot + "/pool", primary_config.poolroot)
+        self.assertEqual(archiveroot + "/dists", primary_config.distsroot)
+        self.assertEqual(
+            archiveroot + "-overrides", primary_config.overrideroot)
+        self.assertEqual(archiveroot + "-cache", primary_config.cacheroot)
+        self.assertEqual(archiveroot + "-misc", primary_config.miscroot)
+        self.assertEqual(
+            archiveroot + "-germinate", primary_config.germinateroot)
+        self.assertEqual(
+            self.root + "/ubuntutest-temp", primary_config.temproot)
+
+    def test_partner_config(self):
+        # Partner archive configuration is correct.
+        # The publisher config for PARTNER contains only 'partner' in its
+        # components.  This prevents non-partner being published in the
+        # partner archive.
+        partner_archive = getUtility(IArchiveSet).getByDistroAndName(
+            self.ubuntutest, "partner")
+        partner_config = getPubConfig(partner_archive)
+        self.root = "/var/tmp/archive"
+        self.assertEqual(self.root, partner_config.distroroot)
+        archiveroot = self.root + "/ubuntutest-partner"
+        self.assertEqual(archiveroot, partner_config.archiveroot)
+        self.assertEqual(archiveroot + "/pool", partner_config.poolroot)
+        self.assertEqual(archiveroot + "/dists", partner_config.distsroot)
+        self.assertIsNone(partner_config.overrideroot)
+        self.assertIsNone(partner_config.cacheroot)
+        self.assertIsNone(partner_config.miscroot)
+        self.assertIsNone(partner_config.germinateroot)
+        self.assertEqual(
+            self.root + "/ubuntutest-temp", partner_config.temproot)
+
+    def test_debug_config(self):
+        # The publisher configuration for DEBUG archives points to
+        # directories beside PRIMARY repository ones, but the distribution
+        # part is modified to be clearly different than the PRIMARY one.
+        debug_archive = getUtility(IArchiveSet).new(
+            purpose=ArchivePurpose.DEBUG, owner=self.ubuntutest.owner,
+            distribution=self.ubuntutest)
+        debug_config = getPubConfig(debug_archive)
+        self.assertEqual(self.root, debug_config.distroroot)
+        archiveroot = self.root + "/ubuntutest-debug"
+        self.assertEqual(archiveroot, debug_config.archiveroot)
+        self.assertEqual(archiveroot + "/pool", debug_config.poolroot)
+        self.assertEqual(archiveroot + "/dists", debug_config.distsroot)
+        self.assertIsNone(debug_config.overrideroot)
+        self.assertIsNone(debug_config.cacheroot)
+        self.assertIsNone(debug_config.miscroot)
+        self.assertIsNone(debug_config.germinateroot)
+        self.assertEqual(self.root + "/ubuntutest-temp", debug_config.temproot)
+
+    def test_copy_config(self):
+        # In the case of copy archives (used for rebuild testing) the
+        # archiveroot is of the form
+        # DISTROROOT/DISTRONAME-ARCHIVENAME/DISTRONAME.
+        copy_archive = getUtility(IArchiveSet).new(
+            purpose=ArchivePurpose.COPY, owner=self.ubuntutest.owner,
+            distribution=self.ubuntutest, name="rebuildtest99")
+        copy_config = getPubConfig(copy_archive)
+        self.assertEqual(self.root, copy_config.distroroot)
+        archiveroot = self.root + "/ubuntutest-rebuildtest99/ubuntutest"
+        self.assertEqual(archiveroot, copy_config.archiveroot)
+        self.assertEqual(archiveroot + "/pool", copy_config.poolroot)
+        self.assertEqual(archiveroot + "/dists", copy_config.distsroot)
+        self.assertEqual(
+            archiveroot + "-overrides", copy_config.overrideroot)
+        self.assertEqual(archiveroot + "-cache", copy_config.cacheroot)
+        self.assertEqual(archiveroot + "-misc", copy_config.miscroot)
+        self.assertEqual(
+            archiveroot + "-germinate", copy_config.germinateroot)
+        self.assertEqual(archiveroot + "-temp", copy_config.temproot)
+
+
+class TestGetPubConfigPPA(TestCaseWithFactory):
+
+    layer = ZopelessDatabaseLayer
+
+    def setUp(self):
+        super(TestGetPubConfigPPA, self).setUp()
+        self.ubuntutest = getUtility(IDistributionSet)['ubuntutest']
+        self.ppa = self.factory.makeArchive(
+            distribution=self.ubuntutest, purpose=ArchivePurpose.PPA)
+        self.ppa_config = getPubConfig(self.ppa)
+
+    def test_ppa_root_matches_config(self):
+        # The base publication location is set by Launchpad configuration.
+        self.assertEqual(
+            config.personalpackagearchive.root, self.ppa_config.distroroot)
+
+    def test_ppa_config(self):
+        # PPA configuration matches the PPA repository topology:
+        #   <PPA_BASE_DIR>/<PERSONNAME>/<DISTRIBUTION>
+        # Some paths are not used in the PPA workflow, so they are set to
+        # None in order that they won't get created.
+        self.assertEqual("/var/tmp/ppa.test/", self.ppa_config.distroroot)
+        archiveroot = "%s%s/%s/ubuntutest" % (
+            self.ppa_config.distroroot, self.ppa.owner.name, self.ppa.name)
+        self.assertEqual(archiveroot, self.ppa_config.archiveroot)
+        self.assertEqual(archiveroot + "/pool", self.ppa_config.poolroot)
+        self.assertEqual(archiveroot + "/dists", self.ppa_config.distsroot)
+        self.assertIsNone(self.ppa_config.overrideroot)
+        self.assertIsNone(self.ppa_config.cacheroot)
+        self.assertIsNone(self.ppa_config.miscroot)
+        self.assertIsNone(self.ppa_config.germinateroot)
+        self.assertEqual(
+            "/var/tmp/archive/ubuntutest-temp", self.ppa_config.temproot)
+
+    def test_private_ppa_separate_root(self):
+        # Private PPAs are published to a different location.
+        self.assertNotEqual(
+            config.personalpackagearchive.private_root,
+            config.personalpackagearchive.root)
+
+    def test_private_ppa_config(self):
+        # Private PPA configuration uses the separate base location.
+        p3a = self.factory.makeArchive(
+            owner=self.ppa.owner, name="myprivateppa",
+            distribution=self.ubuntutest, purpose=ArchivePurpose.PPA)
+        p3a.private = True
+        p3a.buildd_secret = "secret"
+        p3a_config = getPubConfig(p3a)
+        self.assertEqual(
+            config.personalpackagearchive.private_root, p3a_config.distroroot)
+        archiveroot = "%s/%s/%s/ubuntutest" % (
+            p3a_config.distroroot, p3a.owner.name, p3a.name)
+        self.assertEqual(archiveroot, p3a_config.archiveroot)
+        self.assertEqual(archiveroot + "/pool", p3a_config.poolroot)
+        self.assertEqual(archiveroot + "/dists", p3a_config.distsroot)
+        self.assertIsNone(p3a_config.overrideroot)
+        self.assertIsNone(p3a_config.cacheroot)
+        self.assertIsNone(p3a_config.miscroot)
+        self.assertIsNone(p3a_config.germinateroot)
+        self.assertEqual(
+            "/var/tmp/archive/ubuntutest-temp", p3a_config.temproot)

=== modified file 'lib/lp/archivepublisher/tests/test_generate_extra_overrides.py'
--- lib/lp/archivepublisher/tests/test_generate_extra_overrides.py	2012-01-03 17:08:40 +0000
+++ lib/lp/archivepublisher/tests/test_generate_extra_overrides.py	2012-05-21 19:07:38 +0000
@@ -1,10 +1,11 @@
-# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2012 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
 
+from functools import partial
 import logging
 from optparse import OptionValueError
 import os
@@ -46,6 +47,12 @@
         return handle.read()
 
 
+def touch(path):
+    """Create an empty file at path."""
+    with open_for_writing(path, "a"):
+        pass
+
+
 class TestAtomicFile(TestCaseWithFactory):
     """Tests for the AtomicFile helper class."""
 
@@ -113,6 +120,24 @@
             script.distribution = distribution
         return script
 
+    def setUpDistroAndScript(self, series_statuses=["DEVELOPMENT"], **kwargs):
+        """Helper wrapping distro and script setup."""
+        self.distro = self.makeDistro()
+        self.distroseries = [
+            self.factory.makeDistroSeries(
+                distribution=self.distro, status=SeriesStatus.items[status])
+            for status in series_statuses]
+        self.script = self.makeScript(self.distro, **kwargs)
+
+    def setUpComponent(self, component=None):
+        """Create a component and attach it to all distroseries."""
+        if component is None:
+            component = self.factory.makeComponent()
+        for distroseries in self.distroseries:
+            self.factory.makeComponentSelection(
+                distroseries=distroseries, component=component)
+        return component
+
     def makePackage(self, component, dases, **kwargs):
         """Create a published source and binary package for testing."""
         package = self.factory.makeDistributionSourcePackage(
@@ -175,11 +200,8 @@
             self.seeddir, "%s.%s" % (flavour, series_name), seed_name)
 
     def makeSeedStructure(self, flavour, series_name, seed_names,
-                          seed_inherit=None):
+                          seed_inherit={}):
         """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:
@@ -246,130 +268,87 @@
     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)
+        self.setUpDistroAndScript()
+        self.assertEqual(self.distro, self.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)
-        observed_series = [series.name for series in script.series]
-        self.assertEqual([development_distroseries.name], observed_series)
+        self.setUpDistroAndScript(["SUPPORTED", "CURRENT", "DEVELOPMENT"])
+        self.assertEqual([self.distroseries[2]], self.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)
-        observed_series = [series.name for series in script.series]
-        self.assertEqual([frozen_distroseries.name], observed_series)
+        self.setUpDistroAndScript(["SUPPORTED", "CURRENT", "FROZEN"])
+        self.assertEqual([self.distroseries[2]], self.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)
+        self.setUpDistroAndScript(["SUPPORTED", "CURRENT"], run_setup=False)
+        self.assertRaises(LaunchpadScriptFailure, self.script.processOptions)
 
     def test_multiple_development_frozen_distro_series(self):
         # If there are multiple DEVELOPMENT or FROZEN series, they are all
         # used.
-        distro = self.makeDistro()
-        development_distroseries_one = self.factory.makeDistroSeries(
-            distro, status=SeriesStatus.DEVELOPMENT)
-        development_distroseries_two = self.factory.makeDistroSeries(
-            distro, status=SeriesStatus.DEVELOPMENT)
-        frozen_distroseries_one = self.factory.makeDistroSeries(
-            distro, status=SeriesStatus.FROZEN)
-        frozen_distroseries_two = self.factory.makeDistroSeries(
-            distro, status=SeriesStatus.FROZEN)
-        script = self.makeScript(distro)
-        expected_series = [
-            development_distroseries_one.name,
-            development_distroseries_two.name,
-            frozen_distroseries_one.name,
-            frozen_distroseries_two.name,
-            ]
-        observed_series = [series.name for series in script.series]
-        self.assertContentEqual(expected_series, observed_series)
+        self.setUpDistroAndScript(
+            ["DEVELOPMENT", "DEVELOPMENT", "FROZEN", "FROZEN"])
+        self.assertContentEqual(self.distroseries, self.script.series)
 
     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(1, len(script.series))
-        self.assertEqual(["main"], script.getComponents(script.series[0]))
+        self.setUpDistroAndScript()
+        self.setUpComponent(component="main")
+        self.setUpComponent(component="partner")
+        self.assertEqual(1, len(self.script.series))
+        self.assertEqual(
+            ["main"], self.script.getComponents(self.script.series[0]))
 
     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)
+        self.setUpDistroAndScript()
         flavour = self.factory.getUniqueString()
         arch = self.factory.getUniqueString()
         base = self.factory.getUniqueString()
-        output = script.composeOutputPath(
-            flavour, distroseries.name, arch, base)
+        output = self.script.composeOutputPath(
+            flavour, self.distroseries[0].name, arch, base)
         self.assertEqual(
             "%s/%s_%s_%s_%s" % (
-                script.config.germinateroot, base, flavour, distroseries.name,
-                arch),
+                self.script.config.germinateroot, base, flavour,
+                self.distroseries[0].name, arch),
             output)
 
     def test_make_seed_structures_missing_seeds(self):
         # makeSeedStructures ignores missing seeds.
-        distro = self.makeDistro()
-        distroseries = self.factory.makeDistroSeries(distribution=distro)
-        series_name = distroseries.name
-        script = self.makeScript(distro)
+        self.setUpDistroAndScript()
+        series_name = self.distroseries[0].name
         flavour = self.factory.getUniqueString()
 
-        structures = script.makeSeedStructures(
+        structures = self.script.makeSeedStructures(
             series_name, [flavour], seed_bases=["file://%s" % self.seeddir])
         self.assertEqual({}, structures)
 
     def test_make_seed_structures_empty_seed_structure(self):
         # makeSeedStructures ignores an empty seed structure.
-        distro = self.makeDistro()
-        distroseries = self.factory.makeDistroSeries(distribution=distro)
-        series_name = distroseries.name
-        script = self.makeScript(distro)
+        self.setUpDistroAndScript()
+        series_name = self.distroseries[0].name
         flavour = self.factory.getUniqueString()
         self.makeSeedStructure(flavour, series_name, [])
 
-        structures = script.makeSeedStructures(
+        structures = self.script.makeSeedStructures(
             series_name, [flavour], seed_bases=["file://%s" % self.seeddir])
         self.assertEqual({}, structures)
 
     def test_make_seed_structures_valid_seeds(self):
         # makeSeedStructures reads valid seeds successfully.
-        distro = self.makeDistro()
-        distroseries = self.factory.makeDistroSeries(distribution=distro)
-        series_name = distroseries.name
-        script = self.makeScript(distro)
+        self.setUpDistroAndScript()
+        series_name = self.distroseries[0].name
         flavour = self.factory.getUniqueString()
         seed = self.factory.getUniqueString()
         self.makeSeedStructure(flavour, series_name, [seed])
         self.makeSeed(flavour, series_name, seed, [])
 
-        structures = script.makeSeedStructures(
+        structures = self.script.makeSeedStructures(
             series_name, [flavour], seed_bases=["file://%s" % self.seeddir])
         self.assertIn(flavour, structures)
 
@@ -390,18 +369,15 @@
     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)
+        self.setUpDistroAndScript()
+        series_name = self.distroseries[0].name
+        component = self.setUpComponent()
+        das = self.factory.makeDistroArchSeries(
+            distroseries=self.distroseries[0])
         arch = das.architecturetag
         one = self.makePackage(component, [das])
         two = self.makePackage(component, [das])
-        script = self.makeScript(distro)
-        self.makeIndexFiles(script, distroseries)
+        self.makeIndexFiles(self.script, self.distroseries[0])
 
         flavour_one = self.factory.getUniqueString()
         flavour_two = self.factory.getUniqueString()
@@ -412,51 +388,49 @@
         self.makeSeed(flavour_two, series_name, seed, [two.name])
 
         overrides = self.fetchGerminatedOverrides(
-            script, distroseries, arch, [flavour_one, flavour_two])
+            self.script, self.distroseries[0], 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(
+            self.script.composeOutputPath(
                 flavour_one, series_name, arch, "structure"))
-        self.assertTrue(file_exists(script.composeOutputPath(
+        self.assertTrue(file_exists(self.script.composeOutputPath(
             flavour_one, series_name, arch, "all")))
-        self.assertTrue(file_exists(script.composeOutputPath(
+        self.assertTrue(file_exists(self.script.composeOutputPath(
             flavour_one, series_name, arch, "all.sources")))
-        self.assertTrue(file_exists(script.composeOutputPath(
+        self.assertTrue(file_exists(self.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(
+            self.script.composeOutputPath(
                 flavour_two, series_name, arch, "structure"))
-        self.assertTrue(file_exists(script.composeOutputPath(
+        self.assertTrue(file_exists(self.script.composeOutputPath(
             flavour_two, series_name, arch, "all")))
-        self.assertTrue(file_exists(script.composeOutputPath(
+        self.assertTrue(file_exists(self.script.composeOutputPath(
             flavour_two, series_name, arch, "all.sources")))
-        self.assertTrue(file_exists(script.composeOutputPath(
+        self.assertTrue(file_exists(self.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)
+        self.setUpDistroAndScript()
+        series_name = self.distroseries[0].name
+        component = self.setUpComponent()
+        das = self.factory.makeDistroArchSeries(
+            distroseries=self.distroseries[0])
         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)
+        self.makeIndexFiles(self.script, self.distroseries[0])
 
         flavour = self.factory.getUniqueString()
         seed_one = self.factory.getUniqueString()
@@ -470,7 +444,7 @@
             headers=["Task-Description: two"])
 
         overrides = self.fetchGerminatedOverrides(
-            script, distroseries, arch, [flavour])
+            self.script, self.distroseries[0], arch, [flavour])
         expected_overrides = [
             "%s/%s  Task  %s" % (one.name, arch, seed_one),
             "%s/%s  Task  %s" % (two.name, arch, seed_one),
@@ -560,17 +534,14 @@
 
     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)
+        self.setUpDistroAndScript()
+        series_name = self.distroseries[0].name
+        component = self.setUpComponent()
+        das = self.factory.makeDistroArchSeries(
+            distroseries=self.distroseries[0])
         arch = das.architecturetag
         package = self.makePackage(component, [das])
-        script = self.makeScript(distro)
-        self.makeIndexFiles(script, distroseries)
+        self.makeIndexFiles(self.script, self.distroseries[0])
 
         flavour = self.factory.getUniqueString()
         seed = "build-essential"
@@ -578,55 +549,89 @@
         self.makeSeed(flavour, series_name, seed, [package.name])
 
         overrides = self.fetchGerminatedOverrides(
-            script, distroseries, arch, [flavour])
+            self.script, self.distroseries[0], arch, [flavour])
         self.assertContentEqual(
             ["%s/%s  Build-Essential  yes" % (package.name, arch)], overrides)
 
+    def test_removes_only_stale_files(self):
+        # removeStaleOutputs removes only stale germinate output files.
+        self.setUpDistroAndScript()
+        series_name = self.distroseries[0].name
+        seed_old_file = "old_flavour_%s_i386" % series_name
+        seed_new_file = "new_flavour_%s_i386" % series_name
+        other_file = "other-file"
+        output = partial(os.path.join, self.script.config.germinateroot)
+        for base in (seed_old_file, seed_new_file, other_file):
+            touch(output(base))
+        self.script.removeStaleOutputs(series_name, set([seed_new_file]))
+        self.assertFalse(os.path.exists(output(seed_old_file)))
+        self.assertTrue(os.path.exists(output(seed_new_file)))
+        self.assertTrue(os.path.exists(output(other_file)))
+
     def test_process_missing_seeds(self):
         # The script ignores series with no seed structures.
-        distro = self.makeDistro()
-        distroseries_one = self.factory.makeDistroSeries(distribution=distro)
-        distroseries_two = self.factory.makeDistroSeries(distribution=distro)
-        component = self.factory.makeComponent()
-        self.factory.makeComponentSelection(
-            distroseries=distroseries_one, component=component)
-        self.factory.makeComponentSelection(
-            distroseries=distroseries_two, component=component)
-        self.factory.makeDistroArchSeries(distroseries=distroseries_one)
-        self.factory.makeDistroArchSeries(distroseries=distroseries_two)
         flavour = self.factory.getUniqueString()
-        script = self.makeScript(distro, extra_args=[flavour])
-        self.makeIndexFiles(script, distroseries_two)
+        self.setUpDistroAndScript(
+            ["DEVELOPMENT", "DEVELOPMENT"], extra_args=[flavour])
+        self.setUpComponent()
+        self.factory.makeDistroArchSeries(distroseries=self.distroseries[0])
+        self.factory.makeDistroArchSeries(distroseries=self.distroseries[1])
+        self.makeIndexFiles(self.script, self.distroseries[1])
         seed = self.factory.getUniqueString()
-        self.makeSeedStructure(flavour, distroseries_two.name, [seed])
-        self.makeSeed(flavour, distroseries_two.name, seed, [])
+        self.makeSeedStructure(flavour, self.distroseries[1].name, [seed])
+        self.makeSeed(flavour, self.distroseries[1].name, seed, [])
 
-        script.process(seed_bases=["file://%s" % self.seeddir])
+        self.script.process(seed_bases=["file://%s" % self.seeddir])
         self.assertFalse(os.path.exists(os.path.join(
-            script.config.miscroot,
-            "more-extra.override.%s.main" % distroseries_one.name)))
+            self.script.config.miscroot,
+            "more-extra.override.%s.main" % self.distroseries[0].name)))
         self.assertTrue(os.path.exists(os.path.join(
-            script.config.miscroot,
-            "more-extra.override.%s.main" % distroseries_two.name)))
+            self.script.config.miscroot,
+            "more-extra.override.%s.main" % self.distroseries[1].name)))
+
+    def test_process_removes_only_stale_files(self):
+        # The script removes only stale germinate output files.
+        flavour = self.factory.getUniqueString()
+        self.setUpDistroAndScript(extra_args=[flavour])
+        series_name = self.distroseries[0].name
+        self.setUpComponent()
+        das = self.factory.makeDistroArchSeries(
+            distroseries=self.distroseries[0])
+        arch = das.architecturetag
+        self.makeIndexFiles(self.script, self.distroseries[0])
+
+        seed_old = self.factory.getUniqueString()
+        seed_new = self.factory.getUniqueString()
+        self.makeSeedStructure(flavour, series_name, [seed_old])
+        self.makeSeed(flavour, series_name, seed_old, [])
+        self.script.process(seed_bases=["file://%s" % self.seeddir])
+        output = partial(
+            self.script.composeOutputPath, flavour, series_name, arch)
+        self.assertTrue(os.path.exists(output(seed_old)))
+        self.makeSeedStructure(flavour, series_name, [seed_new])
+        self.makeSeed(flavour, series_name, seed_new, [])
+        self.script.process(seed_bases=["file://%s" % self.seeddir])
+        self.assertTrue(os.path.exists(os.path.join(self.script.log_file)))
+        self.assertTrue(os.path.exists(output("structure")))
+        self.assertTrue(os.path.exists(output("all")))
+        self.assertTrue(os.path.exists(output("all.sources")))
+        self.assertTrue(os.path.exists(output(seed_new)))
+        self.assertFalse(os.path.exists(output(seed_old)))
 
     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
+        flavour = self.factory.getUniqueString()
+        self.setUpDistroAndScript(extra_args=[flavour])
+        series_name = self.distroseries[0].name
+        component = self.setUpComponent()
+        das_one = self.factory.makeDistroArchSeries(
+            distroseries=self.distroseries[0])
+        das_two = self.factory.makeDistroArchSeries(
+            distroseries=self.distroseries[0])
         package = self.makePackage(component, [das_one, das_two])
-        flavour = self.factory.getUniqueString()
-        script = self.makeScript(distro, extra_args=[flavour])
-        self.makeIndexFiles(script, distroseries)
+        self.makeIndexFiles(self.script, self.distroseries[0])
 
         seed = self.factory.getUniqueString()
         self.makeSeedStructure(flavour, series_name, [seed])
@@ -634,19 +639,19 @@
             flavour, series_name, seed, [package.name],
             headers=["Task-Description: task"])
 
-        script.process(seed_bases=["file://%s" % self.seeddir])
+        self.script.process(seed_bases=["file://%s" % self.seeddir])
         override_path = os.path.join(
-            script.config.miscroot,
+            self.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),
+            "%s/%s  Task  %s" % (package.name, das_one.architecturetag, seed),
+            "%s/%s  Task  %s" % (package.name, das_two.architecturetag, seed),
             ]
         self.assertContentEqual(
             expected_overrides, file_contents(override_path).splitlines())
 
         log_file = os.path.join(
-            script.config.germinateroot, "germinate.output")
+            self.script.config.germinateroot, "germinate.output")
         self.assertIn("Downloading file://", file_contents(log_file))
 
     def test_run_script(self):