Merge lp:~jtv/launchpad/redo-uploads into lp:launchpad

[Jeroen T. Vermeulen]

[Jeroen T. Vermeulen (jtv)]

= Bug 439346 =

A stupid missing parameter in 3.0 caused the translations auto-approver
to approve uploads for the right packages but the wrong Ubuntu releases.
Due to some other bugs that we've fixed already, we don't know exactly
which translations will or won't be affected. So we ended up with a
fairly long list of packages that _may_ have had the wrong files
imported to them.

This branch provides a script that finds the latest Soyuz-generated
translations upload for a given package (a tarball containing, roughly
speaking, the upstream templates and translations) and re-uploads it.

Finding the latest translations upload for a package turns out to be...
in Julian's words, "easy." I'd probably pick another word myself.
After conferring with Celso (who helped me get it actually working) I
isolated that piece of work in SourcePackage.

You may wonder why the script keeps track of uploadless_packages. That
is there just to facilitate testing.

There is no single end-to-end test. It's just too hard to set up and
maintain a realistic testing situation for this. Test coverage consists
of several stretches that touch more than overlap:

 * A full script run, but without any uploads being found.
   This exercises everything except retrieving and uploading a tarball
   from the Librarian, and the tail end of getLatestTranslations in the
   success case.

 * A before-and-after demonstration of getLatestTranslationsUploads in
   the doctest. This does exercise the tail end in the success case.
   It also shows that a gzipped tarball containing the right files has
   gone into the Librarian.

 * A unit test for _findPackage, to cover up for the fact that other
   unit tests patch that method for mocking purposes.

 * A successful run of the LaunchpadScript object, but with
   getLatestTranslationsUploads mocked up to return a gzipped tarball
   that was stuffed straight into the Librarian by the test. This also
   tests the retrieval and uploading of that tarball.

The tar-file handling in the tests is terrible. I can't really help it;
blame the tarfile module's horrible API. Makes it hard to create
tarballs in-memory using StringIOs.

== Tests ==
{{{
./bin/test -vv -t distroseriesqueue-translations.txt
./bin/test -vv -t reupload_translations
}}}

== Demo and Q/A ==

Pick a source package in "main" for some Ubuntu release, one with
translations but not too much in its import queue—there has to be at
least one translation that doesn't have a Needs Review or Approved entry
in the queue. You can create a new translation by translating a string
into a dead language such as Sumerian (sux).

Run the script, passing it the name of the Ubuntu release (e.g. karmic)
and the name of the source package (e.g. tomboy). Try the --dry-run
option first; nothing will actually happen to the package's import
queue. Try the same run again without the --dry-run option, and now a
full set of upstream translations will appear on the queue in Needs
Review state.

Or if you wait too long, Approved or even Imported. The approval and
import scripts do run on staging.

No lint.

Jeroen

[Edwin Grubbs (edwin-grubbs)]

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.distroserie...

review: Approve (code)

[Jeroen T. Vermeulen (jtv)]

> Hi Jeroen,
>
> This looks good. I just have a few formatting comments.

Thanks. Whitespace problems all fixed.

"I have no idea how that tab got into my code, officer."

review: Abstain

=== modified file 'lib/lp/registry/interfaces/sourcepackage.py'
--- lib/lp/registry/interfaces/sourcepackage.py	2009-09-16 04:31:39 +0000
+++ lib/lp/registry/interfaces/sourcepackage.py	2009-09-30 16:53:17 +0000
@@ -225,6 +225,13 @@
         title=u'The component in which the package was last published.',
         schema=IComponent, readonly=True, required=False)
 
+    def getLatestTranslationsUploads():
+        """Find latest Translations tarballs as produced by Soyuz.
+
+        :return: A list of `ILibraryFileAlias`es, usually of size zero
+            or one.  If not, they are sorted from oldest to newest.
+        """
+
 
 class ISourcePackageFactory(Interface):
     """A creator of source packages."""
 
=== modified file 'lib/lp/registry/model/sourcepackage.py'
--- lib/lp/registry/model/sourcepackage.py	2009-09-25 17:00:20 +0000
+++ lib/lp/registry/model/sourcepackage.py	2009-09-30 16:53:17 +0000
@@ -54,6 +54,7 @@
 from lp.translations.interfaces.potemplate import IHasTranslationTemplates
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
+from lp.soyuz.interfaces.queue import PackageUploadCustomFormat
 from lp.answers.interfaces.questioncollection import (
     QUESTION_STATUS_DEFAULT_SEARCH)
 from lp.answers.interfaces.questiontarget import IQuestionTarget
@@ -655,3 +656,35 @@
             self.distribution.name,
             self.distroseries.getSuite(pocket),
             self.name)
+
+    def getLatestTranslationsUploads(self):
+        """See `ISourcePackage`."""
+        our_format = PackageUploadCustomFormat.ROSETTA_TRANSLATIONS
+
+        packagename = self.sourcepackagename.name
+        displayname = self.displayname
+        distro = self.distroseries.distribution
+
+        histories = distro.main_archive.getPublishedSources(
+            name=packagename, distroseries=self.distroseries,
+            status=PackagePublishingStatus.PUBLISHED, exact_match=True)
+        histories = list(histories)
+
+        builds = []
+        for history in histories:
+            builds += list(history.getBuilds())
+
+        uploads = [
+            build.package_upload
+            for build in builds
+            if build.package_upload
+            ]
+        custom_files = []
+        for upload in uploads:
+            custom_files += [
+                custom for custom in upload.customfiles
+                if custom.customformat == our_format
+                ]
+
+        custom_files.sort(key=attrgetter('id'))
+        return [custom.libraryfilealias for custom in custom_files]
 
=== modified file 'lib/lp/soyuz/doc/distroseriesqueue-translations.txt'
--- lib/lp/soyuz/doc/distroseriesqueue-translations.txt	2009-09-04 08:35:20 +0000
+++ lib/lp/soyuz/doc/distroseriesqueue-translations.txt	2009-09-30 16:53:17 +0000
@@ -97,6 +97,14 @@
   >>> pmount_upload.is_rejected
   False
 
+At this point, no translations uploads have been registered for this
+package.
+
+  >>> from lp.registry.model.sourcepackage import SourcePackage
+  >>> dapper_pmount = SourcePackage(pmount_sourcepackagename, dapper)
+  >>> print len(dapper_pmount.getLatestTranslationsUploads())
+  0
+
   >>> success = pmount_upload.do_accept()
   DEBUG: Creating queue entry
   DEBUG: Build ... found
@@ -115,6 +123,17 @@
   #NEW: pmount_0.9.7-2ubuntu2_amd64.deb
   #OK: pmount_0.9.7-2ubuntu2_amd64_translations.tar.gz
 
+The upload now shows up as the latest translations upload for the
+package.
+
+  >>> latest_translations_uploads = list(
+  ...     dapper_pmount.getLatestTranslationsUploads())
+  >>> print len(latest_translations_uploads)
+  1
+
+We'll get back to that uploaded file later.
+
+  >>> latest_translations_upload = latest_translations_uploads[0]
 
   # Check the import queue content, it should be empty.
   >>> from lp.translations.interfaces.translationimportqueue import (
@@ -259,6 +278,7 @@
   # component.
   >>> transaction.abort()
 
+
 == Translations from PPA build ==
 
 For now we simply ignore translations for archives other than the
@@ -376,3 +396,26 @@
   >>> translations_upload.packageupload = carlos_package_upload
   >>> translations_upload.publish_ROSETTA_TRANSLATIONS()
   Imported by: carlos
+
+
+=== Translations tarball ===
+
+The LibraryFileAlias returned by getLatestTranslationsUploads on the
+source package points to a tarball with translations files for the
+package.
+
+  >>> import tarfile
+  >>> from StringIO import StringIO
+  >>> tarball = StringIO(latest_translations_upload.read())
+  >>> archive = tarfile.open('', 'r|gz', tarball)
+  >>> translation_files = sorted([
+  ...     entry.name for entry in archive.getmembers()
+  ...     if entry.name.endswith('.po') or entry.name.endswith('.pot')
+  ...     ])
+  >>> for filename in translation_files:
+  ...     print filename
+  source/po/ca.po
+  source/po/cs.po
+  source/po/de.po
+  ...
+  source/po/pmount.pot
 
=== 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 16:53:17 +0000
@@ -0,0 +1,110 @@
+#! /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).
+
+__metaclass__ = type
+
+__all__ = [
+    'ReuploadPackageTranslations',
+    ]
+
+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.")
+
+    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 16:53:17 +0000
@@ -0,0 +1,168 @@
+#! /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`.
+    """
+    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):
+        # _processPackage will fetch the package's latest translations
+        # upload from the Librarian and re-import it.
+        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__)
 
=== added file 'scripts/rosetta/reupload-translations.py'
--- scripts/rosetta/reupload-translations.py	1970-01-01 00:00:00 +0000
+++ scripts/rosetta/reupload-translations.py	2009-09-30 16:53:17 +0000
@@ -0,0 +1,19 @@
+#! /usr/bin/env python2.4
+#
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+# pylint: disable-msg=W0403
+
+"""Re-upload translations from given packages."""
+
+__metaclass__ = type
+
+import _pythonpath
+
+from lp.translations.scripts.reupload_translations import (
+    ReuploadPackageTranslations)
+
+
+if __name__ == '__main__':
+    script = ReuploadPackageTranslations('reupload-translations')
+    script.run()