Hi Jeroen,

This looks good. I just have a few formatting comments.

merge-conditional

-Edwin


>=== added file 'lib/lp/translations/scripts/reupload_translations.py'
>--- lib/lp/translations/scripts/reupload_translations.py	1970-01-01 00:00:00 +0000
>+++ lib/lp/translations/scripts/reupload_translations.py	2009-09-30 15:42:55 +0000
>@@ -0,0 +1,105 @@
>+__metaclass__ = type


Missing copyright notice.


>+
>+__all__ = [
>+	'ReuploadPackageTranslations',
>+	]


Indentation should be spaces.


>+
>+from zope.component import getUtility
>+
>+from lp.services.scripts.base import LaunchpadScript, LaunchpadScriptFailure
>+
>+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
>+from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
>+from lp.translations.interfaces.translationimportqueue import (
>+    ITranslationImportQueue)
>+
>+
>+class ReuploadPackageTranslations(LaunchpadScript):
>+    """Re-upload latest translations for given distribution packages."""
>+    description = "Re-upload latest translations uploads for package(s)."
>+
>+    def add_my_options(self):
>+        """See `LaunchpadScript`."""
>+        self.parser.add_option('-d', '--distribution', dest='distro',
>+            help="Distribution to upload for.", default='ubuntu')
>+        self.parser.add_option('-s', '--series', dest='distroseries',
>+            help="Distribution release series to upload for.")
>+        self.parser.add_option('-p', '--package', action='append',
>+            dest='packages', default=[],
>+            help="Name(s) of source package(s) to re-upload.")
>+        self.parser.add_option('-l', '--dry-run', dest='dryrun',
>+            action='store_true', default=False,
>+            help="Pretend to upload, but make no actual changes.")
>+ 


Trailing white space.


>+    def main(self):
>+        """See `LaunchpadScript`."""
>+        self.uploadless_packages = []
>+        self._setDistroDetails()
>+
>+        if len(self.options.packages) == 0:
>+            raise LaunchpadScriptFailure("No packages specified.")
>+
>+        if self.options.dryrun:
>+            self.logger.info("Dry run.  Not really uploading anything.")
>+
>+        for package_name in self.options.packages:
>+            self._processPackage(self._findPackage(package_name))
>+            self._commit()
>+
>+        self.logger.info("Done.")
>+
>+    def _commit(self):
>+        """Commit transaction (or abort if dry run)."""
>+        if self.txn:
>+            if self.options.dryrun:
>+                self.txn.abort()
>+            else:
>+                self.txn.commit()
>+
>+    def _setDistroDetails(self):
>+        """Figure out the `Distribution`/`DistroSeries` to act upon."""
>+        # Avoid circular imports.
>+        from lp.registry.interfaces.distribution import IDistributionSet
>+
>+        distroset = getUtility(IDistributionSet)
>+        self.distro = distroset.getByName(self.options.distro)
>+
>+        if not self.options.distroseries:
>+            raise LaunchpadScriptFailure(
>+                "Specify a distribution release series.")
>+
>+        self.distroseries = self.distro.getSeries(self.options.distroseries)
>+
>+    def _findPackage(self, name):
>+        """Find `SourcePackage` of given name."""
>+        # Avoid circular imports.
>+        from lp.registry.interfaces.sourcepackage import ISourcePackageFactory
>+
>+        factory = getUtility(ISourcePackageFactory)
>+        nameset = getUtility(ISourcePackageNameSet)
>+
>+        sourcepackagename = nameset.queryByName(name)
>+
>+        return factory.new(sourcepackagename, self.distroseries)
>+
>+    def _processPackage(self, package):
>+        """Get translations for `package` re-uploaded."""
>+        self.logger.info("Processing %s" % package.displayname)
>+        tarball_aliases = package.getLatestTranslationsUploads()
>+        queue = getUtility(ITranslationImportQueue)
>+        rosetta_team = getUtility(ILaunchpadCelebrities).rosetta_experts
>+
>+        have_uploads = False
>+        for alias in tarball_aliases:
>+            have_uploads = True
>+            self.logger.debug("Uploading file '%s' for %s." % (
>+                alias.filename, package.displayname))
>+            queue.addOrUpdateEntriesFromTarball(
>+                alias.read(), True, rosetta_team,
>+                sourcepackagename=package.sourcepackagename,
>+                distroseries=self.distroseries)
>+
>+        if not have_uploads:
>+            self.logger.warn(
>+                "Found no translations upload for %s." % package.displayname)
>+            self.uploadless_packages.append(package)
>
>=== added file 'lib/lp/translations/scripts/tests/test_reupload_translations.py'
>--- lib/lp/translations/scripts/tests/test_reupload_translations.py	1970-01-01 00:00:00 +0000
>+++ lib/lp/translations/scripts/tests/test_reupload_translations.py	2009-09-30 15:42:55 +0000
>@@ -0,0 +1,166 @@
>+#! /usr/bin/python2.4
>+#
>+# Copyright 2009 Canonical Ltd.  This software is licensed under the
>+# GNU Affero General Public License version 3 (see the file LICENSE).
>+
>+"""Test `reupload_translations` and `ReuploadPackageTranslations`."""
>+
>+__metaclass__ = type
>+
>+from unittest import TestLoader
>+
>+import re
>+from StringIO import StringIO
>+import tarfile
>+import transaction
>+
>+from zope.security.proxy import removeSecurityProxy
>+
>+from canonical.testing import LaunchpadZopelessLayer
>+from lp.testing import TestCaseWithFactory
>+from canonical.launchpad.scripts.tests import run_script
>+
>+from canonical.launchpad.database.librarian import LibraryFileAliasSet
>+from lp.registry.model.sourcepackage import SourcePackage
>+from lp.translations.model.translationimportqueue import (
>+    TranslationImportQueue)
>+
>+from lp.translations.scripts.reupload_translations import (
>+    ReuploadPackageTranslations)
>+
>+
>+class UploadInjector:
>+    def __init__(self, script, tar_alias):
>+        self.tar_alias = tar_alias
>+        self.script = script
>+        self.original_findPackage = script._findPackage
>+
>+    def __call__(self, name):
>+        package = self.original_findPackage(name)
>+        removeSecurityProxy(package).getLatestTranslationsUploads = (
>+            self._fakeTranslationsUpload)
>+        return package
>+
>+    def _fakeTranslationsUpload(self):
>+        return [self.tar_alias]
>+
>+
>+def upload_tarball(translation_files):
>+    """Create a tarball and upload it to the Librarian.
>+
>+    :param translation_files: A dict mapping filenames to file contents.
>+    :return: A `LibraryFileAlias`. 


Trailing white space.


>+    """
>+    buf = StringIO()
>+    tarball = tarfile.open('', 'w:gz', buf)
>+    for name, contents in translation_files.iteritems():
>+        pseudofile = StringIO(contents)
>+        tarinfo = tarfile.TarInfo()
>+        tarinfo.name = name
>+        tarinfo.size = len(contents)
>+        tarinfo.type = tarfile.REGTYPE
>+        tarball.addfile(tarinfo, pseudofile)
>+
>+    tarball.close()
>+    buf.flush()
>+    tarsize = buf.tell()
>+    buf.seek(0)
>+
>+    return LibraryFileAliasSet().create(
>+        'uploads.tar.gz', tarsize, buf, 'application/x-gtar')
>+
>+
>+def summarize_translations_queue(sourcepackage):
>+    """Describe queue entries for `sourcepackage` as a name/contents dict."""
>+    entries = TranslationImportQueue().getAllEntries(sourcepackage)
>+    return dict((entry.path, entry.content.read()) for entry in entries)
>+
>+
>+class TestReuploadPackageTranslations(TestCaseWithFactory):
>+    """Test `ReuploadPackageTranslations`."""
>+    layer = LaunchpadZopelessLayer
>+
>+    def setUp(self):
>+        super(TestReuploadPackageTranslations, self).setUp()
>+        sourcepackagename = self.factory.makeSourcePackageName()
>+        distroseries = self.factory.makeDistroRelease()
>+        self.sourcepackage = SourcePackage(sourcepackagename, distroseries)
>+        self.script = ReuploadPackageTranslations('reupload', test_args=[
>+            '-d', distroseries.distribution.name,
>+            '-s', distroseries.name,
>+            '-p', sourcepackagename.name,
>+            '-qqq'])
>+
>+    def test_findPackage(self):
>+        # _findPackage finds a SourcePackage by name.
>+        self.script._setDistroDetails()
>+        found_package = self.script._findPackage(
>+            self.sourcepackage.sourcepackagename.name)
>+        self.assertEqual(self.sourcepackage, found_package)
>+
>+    def test_processPackage_nothing(self):
>+        # A package need not have a translations upload.  The script
>+        # notices this but does nothing about it.
>+        self.script.main()
>+        self.assertEqual(
>+            [self.sourcepackage], self.script.uploadless_packages)
>+
>+    def test_processPackage(self):
>+        translation_files = {
>+            'po/messages.pot': '# pot',
>+            'po/nl.po': '# nl',
>+        }
>+        tar_alias = upload_tarball(translation_files)
>+
>+        # Force Librarian update
>+        transaction.commit()
>+
>+        self.script._findPackage = UploadInjector(self.script, tar_alias)
>+        self.script.main()
>+        self.assertEqual([], self.script.uploadless_packages)
>+
>+        # Force Librarian update
>+        transaction.commit()
>+
>+        queue_summary = summarize_translations_queue(self.sourcepackage)
>+        self.assertEqual(translation_files, queue_summary)
>+
>+
>+class TestReuploadScript(TestCaseWithFactory):
>+    """Test reupload-translations script."""
>+    layer = LaunchpadZopelessLayer
>+
>+    def setUp(self):
>+        super(TestReuploadScript, self).setUp()
>+        self.distroseries = self.factory.makeDistroRelease()
>+        self.sourcepackagename1 = self.factory.makeSourcePackageName()
>+        self.sourcepackagename2 = self.factory.makeSourcePackageName()
>+        transaction.commit()
>+
>+    def test_reupload_translations(self):
>+        """Test a run of the script."""
>+        retcode, stdout, stderr = run_script(
>+            'scripts/rosetta/reupload-translations.py', [
>+                '-d', self.distroseries.distribution.name,
>+                '-s', self.distroseries.name,
>+                '-p', self.sourcepackagename1.name,
>+                '-p', self.sourcepackagename2.name,
>+                '-vvv',
>+                '--dry-run',
>+            ])
>+
>+        self.assertEqual(0, retcode)
>+        self.assertEqual('', stdout)
>+
>+        expected_output = (
>+            "INFO\s*Dry run.  Not really uploading anything.\n"
>+            "INFO\s*Processing [^\s]+ in .*\n"
>+            "WARNING\s*Found no translations upload for .*\n"
>+            "INFO\s*Processing [^\s]+ in .*\n"
>+            "WARNING\s*Found no translations upload for .*\n"
>+            "INFO\s*Done.\n")
>+        self.assertTrue(re.match(expected_output, stderr))
>+
>+
>+def test_suite():
>+    return TestLoader().loadTestsFromName(__name__)
>