launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03037
lp:~adeuring/launchpad/no-package-translation-uploads-when-sharing into lp:launchpad
Abel Deuring has proposed merging lp:~adeuring/launchpad/no-package-translation-uploads-when-sharing into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #732612 in Launchpad itself: "translation uploads from source package builds should stop when upstream sharing is enabled."
https://bugs.launchpad.net/launchpad/+bug/732612
For more details, see:
https://code.launchpad.net/~adeuring/launchpad/no-package-translation-uploads-when-sharing/+merge/54356
This branch fixes bug 732612: translation uploads from source package builds should stop when upstream sharing is enabled.
The fix is quite straightforward: Prevent a call of ITranslationImportQueue.addOrUpdateEntriesFromTarball() in SourcePackageRelease.attachTranslationFiles(), if translation sharing is enabled. I needed some time to figure out if attachTranslationFiles() is used anywhere: It is called by PackageUploadCustom.publishRosettaTranslations(), and grepping for publish_ROSETTA_TRANSLATIONS (as it was formerly alled) did not yield any results. The reason: The method was looked up in PackageUploadCustom.publish() by concatenating the string 'publish_' with the name of an enumeration value, and by using the resulting string in getattr(self, method_name). That works, but maked it a bit hard to figure out which method is used where, so I also changed the way how PackageUploadCustom.publish() find the right "worker method".
test: ./bin/test soyuz -vvt soyuz.tests.test_sourcepackagerelease
lint:
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/soyuz/doc/distroseriesqueue-translations.txt
lib/lp/soyuz/interfaces/queue.py
lib/lp/soyuz/model/queue.py
lib/lp/soyuz/model/sourcepackagerelease.py
lib/lp/soyuz/tests/test_sourcepackagerelease.py
./lib/lp/soyuz/doc/distroseriesqueue-translations.txt
7: source has bad indentation.
22: source has bad indentation.
25: source has bad indentation.
28: source has bad indentation.
32: source has bad indentation.
35: source has bad indentation.
40: source has bad indentation.
45: source has bad indentation.
53: source has bad indentation.
61: source has bad indentation.
73: source has bad indentation.
74: source exceeds 78 characters.
76: source has bad indentation.
79: source has bad indentation.
85: source has bad indentation.
97: source has bad indentation.
103: source has bad indentation.
108: source has bad indentation.
113: source has bad indentation.
117: want exceeds 78 characters.
128: source has bad indentation.
135: source has bad indentation.
138: source has bad indentation.
145: source has bad indentation.
150: source has bad indentation.
157: source has bad indentation.
174: source has bad indentation.
178: source has bad indentation.
183: source has bad indentation.
190: source has bad indentation.
193: want exceeds 78 characters.
195: source has bad indentation.
199: source has bad indentation.
207: source has bad indentation.
222: want exceeds 78 characters.
224: source has bad indentation.
228: source has bad indentation.
234: source has bad indentation.
240: source has bad indentation.
255: want exceeds 78 characters.
257: source has bad indentation.
261: source has bad indentation.
267: source has bad indentation.
273: source has bad indentation.
276: want exceeds 78 characters.
278: source has bad indentation.
287: source has bad indentation.
291: source has bad indentation.
295: source has bad indentation.
299: source has bad indentation.
300: want exceeds 78 characters.
305: source has bad indentation.
315: source has bad indentation.
318: source has bad indentation.
323: source has bad indentation.
327: source has bad indentation.
328: want exceeds 78 characters.
333: source has bad indentation.
344: source has bad indentation.
345: source exceeds 78 characters.
358: source has bad indentation.
363: source has bad indentation.
374: source exceeds 78 characters.
376: source has bad indentation.
389: source has bad indentation.
400: source has bad indentation.
404: source has bad indentation.
416: source has bad indentation.
426: source has bad indentation.
445: source has bad indentation.
./lib/lp/soyuz/model/sourcepackagerelease.py
201: redefinition of function 'copyright' from line 192
The complaint about a redefinition of "copyright" is bogus: There is a @property named "copyright" and a @copyright.setter named "copyright".
--
https://code.launchpad.net/~adeuring/launchpad/no-package-translation-uploads-when-sharing/+merge/54356
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/no-package-translation-uploads-when-sharing into lp:launchpad.
=== modified file 'lib/lp/soyuz/doc/distroseriesqueue-translations.txt'
--- lib/lp/soyuz/doc/distroseriesqueue-translations.txt 2010-12-22 20:46:21 +0000
+++ lib/lp/soyuz/doc/distroseriesqueue-translations.txt 2011-03-22 14:49:18 +0000
@@ -1,4 +1,5 @@
-== Upload processing queue with translations ==
+Upload processing queue with translations
+=========================================
This test covers the use case when a package includes translations and is
uploaded into the system.
@@ -277,7 +278,8 @@
>>> transaction.abort()
-== Translations from PPA build ==
+Translations from PPA build
+---------------------------
For now we simply ignore translations for archives other than the
Distribution archives (i.e. PPAs).
@@ -305,7 +307,8 @@
>>> transaction.abort()
-== Translations from a rebuild ==
+Translations from a rebuild
+---------------------------
Translations coming from rebuilt packages are also ignored.
@@ -331,7 +334,8 @@
0
-== Translations importer: publish_ROSETTA_TRANSLATIONS ==
+Translations importer: publishRosettaTranslations
+-------------------------------------------------
We create mock objects for SourcePackageRelease, PackageUpload and
PackageUploadCustom: these will emulate everything we need to document
@@ -404,7 +408,7 @@
True
>>> translations_upload = MockPackageUploadCustom()
>>> translations_upload.packageupload = sync_package_upload
- >>> translations_upload.publish_ROSETTA_TRANSLATIONS()
+ >>> translations_upload.publishRosettaTranslations()
Imported by: katie
Non-auto-sync uploads by 'katie' still indicate 'katie' as the uploader.
@@ -414,7 +418,7 @@
>>> non_sync_package_upload.isAutoSyncUpload()
False
>>> translations_upload.packageupload = non_sync_package_upload
- >>> translations_upload.publish_ROSETTA_TRANSLATIONS()
+ >>> translations_upload.publishRosettaTranslations()
Imported by: katie
Uploads by anyone else are treated as if importer is the packager.
@@ -427,11 +431,12 @@
>>> carlos_package_upload.isAutoSyncUpload()
False
>>> translations_upload.packageupload = carlos_package_upload
- >>> translations_upload.publish_ROSETTA_TRANSLATIONS()
+ >>> translations_upload.publishRosettaTranslations()
Imported by: carlos
-=== Translations tarball ===
+Translations tarball
+~~~~~~~~~~~~~~~~~~~~
The LibraryFileAlias returned by getLatestTranslationsUploads on the
source package points to a tarball with translations files for the
=== modified file 'lib/lp/soyuz/interfaces/queue.py'
--- lib/lp/soyuz/interfaces/queue.py 2011-01-31 16:01:21 +0000
+++ lib/lp/soyuz/interfaces/queue.py 2011-03-22 14:49:18 +0000
@@ -389,6 +389,7 @@
process will be logged to it.
"""
+
class IPackageUploadSource(Interface):
"""A Queue item's related sourcepackagereleases."""
@@ -517,7 +518,7 @@
process will be logged to it.
"""
- def publish_DEBIAN_INSTALLER(logger=None):
+ def publishDebianInstaller(logger=None):
"""Publish this custom item as a raw installer tarball.
This will write the installer tarball out to the right part of
@@ -527,7 +528,7 @@
process will be logged to it.
"""
- def publish_DIST_UPGRADER(logger=None):
+ def publishDistUpgrader(logger=None):
"""Publish this custom item as a raw dist-upgrader tarball.
This will write the dist-upgrader tarball out to the right part of
@@ -537,7 +538,7 @@
process will be logged to it.
"""
- def publish_DDTP_TARBALL(logger=None):
+ def publishDdtpTarball(logger=None):
"""Publish this custom item as a raw ddtp-tarball.
This will write the ddtp-tarball out to the right part of
@@ -547,7 +548,7 @@
process will be logged to it.
"""
- def publish_ROSETTA_TRANSLATIONS(logger=None):
+ def publishRosettaTranslations(logger=None):
"""Publish this custom item as a rosetta tarball.
Essentially this imports the tarball into rosetta.
@@ -556,14 +557,14 @@
process will be logged to it.
"""
- def publish_STATIC_TRANSLATIONS(logger):
+ def publishStaticTranslations(logger):
"""Publish this custom item as a static translations tarball.
This is currently a no-op as we don't publish these files, they only
reside in the librarian for later retrieval using the webservice.
"""
- def publish_META_DATA(logger):
+ def publishMetaData(logger):
"""Publish this custom item as a meta-data file.
This method writes the meta-data custom file to the archive in
@@ -662,7 +663,8 @@
return all pockets. It supports multiple pockets as a list.
If 'archive' is specified return only queue items targeted to this
- archive, if not restrict the results to the IDistribution.main_archive.
+ archive, if not restrict the results to the
+ IDistribution.main_archive.
Use 'exact_match' argument for precise results.
"""
=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py 2011-03-04 05:41:41 +0000
+++ lib/lp/soyuz/model/queue.py 2011-03-22 14:49:18 +0000
@@ -1655,13 +1655,7 @@
self.packageupload.distroseries.distribution.name,
self.packageupload.distroseries.name))
- name = "publish_" + self.customformat.name
- method = getattr(self, name, None)
- if method is not None:
- method(logger)
- else:
- raise NotFoundError("Unable to find a publisher method for %s" % (
- self.customformat.name))
+ self.publisher_dispatch[self.customformat](self, logger)
def temp_filename(self):
"""See `IPackageUploadCustom`."""
@@ -1692,7 +1686,7 @@
finally:
shutil.rmtree(os.path.dirname(temp_filename))
- def publish_DEBIAN_INSTALLER(self, logger=None):
+ def publishDebianInstaller(self, logger=None):
"""See `IPackageUploadCustom`."""
# XXX cprov 2005-03-03: We need to use the Zope Component Lookup
# to instantiate the object in question and avoid circular imports
@@ -1701,7 +1695,7 @@
self._publishCustom(process_debian_installer)
- def publish_DIST_UPGRADER(self, logger=None):
+ def publishDistUpgrader(self, logger=None):
"""See `IPackageUploadCustom`."""
# XXX cprov 2005-03-03: We need to use the Zope Component Lookup
# to instantiate the object in question and avoid circular imports
@@ -1710,7 +1704,7 @@
self._publishCustom(process_dist_upgrader)
- def publish_DDTP_TARBALL(self, logger=None):
+ def publishDdtpTarball(self, logger=None):
"""See `IPackageUploadCustom`."""
# XXX cprov 2005-03-03: We need to use the Zope Component Lookup
# to instantiate the object in question and avoid circular imports
@@ -1719,7 +1713,7 @@
self._publishCustom(process_ddtp_tarball)
- def publish_ROSETTA_TRANSLATIONS(self, logger=None):
+ def publishRosettaTranslations(self, logger=None):
"""See `IPackageUploadCustom`."""
sourcepackagerelease = self.packageupload.sourcepackagerelease
@@ -1756,7 +1750,7 @@
debug(logger, "Unable to fetch %s to import it into Rosetta" %
self.libraryfilealias.http_url)
- def publish_STATIC_TRANSLATIONS(self, logger=None):
+ def publishStaticTranslations(self, logger=None):
"""See `IPackageUploadCustom`."""
# Static translations are not published. Currently, they're
# only exposed via webservice methods so that third parties can
@@ -1764,7 +1758,7 @@
debug(logger, "Skipping publishing of static translations.")
return
- def publish_META_DATA(self, logger=None):
+ def publishMetaData(self, logger=None):
"""See `IPackageUploadCustom`."""
# In the future this could use the existing custom upload file
# processing which deals with versioning, etc., but that's too
@@ -1787,6 +1781,21 @@
self.libraryfilealias.open()
copy_and_close(self.libraryfilealias, file_obj)
+ publisher_dispatch = {
+ PackageUploadCustomFormat.DEBIAN_INSTALLER: publishDebianInstaller,
+ PackageUploadCustomFormat.ROSETTA_TRANSLATIONS:
+ publishRosettaTranslations,
+ PackageUploadCustomFormat.DIST_UPGRADER: publishDistUpgrader,
+ PackageUploadCustomFormat.DDTP_TARBALL: publishDdtpTarball,
+ PackageUploadCustomFormat.STATIC_TRANSLATIONS:
+ publishStaticTranslations,
+ PackageUploadCustomFormat.META_DATA: publishMetaData,
+ }
+
+ # publisher_dispatch must have an entry for each value of
+ # PackageUploadCustomFormat.
+ assert len(publisher_dispatch) == len(PackageUploadCustomFormat)
+
class PackageUploadSet:
"""See `IPackageUploadSet`"""
=== modified file 'lib/lp/soyuz/model/sourcepackagerelease.py'
--- lib/lp/soyuz/model/sourcepackagerelease.py 2011-03-10 07:53:15 +0000
+++ lib/lp/soyuz/model/sourcepackagerelease.py 2011-03-22 14:49:18 +0000
@@ -32,7 +32,6 @@
from zope.interface import implements
from canonical.database.constants import (
- DEFAULT,
UTC_NOW,
)
from canonical.database.datetimecol import UtcDateTimeCol
@@ -624,11 +623,17 @@
queue = getUtility(ITranslationImportQueue)
- queue.addOrUpdateEntriesFromTarball(
- tarball, by_maintainer, importer,
- sourcepackagename=self.sourcepackagename,
- distroseries=self.upload_distroseries,
- filename_filter=_filter_ubuntu_translation_file)
+ # We do not want to override upstream translations, if
+ # translation sharing is enabled.
+ # Avoid circular imports.
+ from lp.translations.utilities.translationsharinginfo import (
+ has_upstream_template)
+ if not has_upstream_template(self.sourcepackage):
+ queue.addOrUpdateEntriesFromTarball(
+ tarball, by_maintainer, importer,
+ sourcepackagename=self.sourcepackagename,
+ distroseries=self.upload_distroseries,
+ filename_filter=_filter_ubuntu_translation_file)
def getDiffTo(self, to_sourcepackagerelease):
"""See ISourcePackageRelease."""
=== modified file 'lib/lp/soyuz/tests/test_sourcepackagerelease.py'
--- lib/lp/soyuz/tests/test_sourcepackagerelease.py 2010-10-04 19:50:45 +0000
+++ lib/lp/soyuz/tests/test_sourcepackagerelease.py 2011-03-22 14:49:18 +0000
@@ -5,8 +5,21 @@
__metaclass__ = type
+import transaction
+from zope.component import getUtility
+
from canonical.testing.layers import LaunchpadFunctionalLayer
-from lp.testing import TestCaseWithFactory
+from lp.services.tarfile_helpers import LaunchpadWriteTarFile
+from lp.testing import (
+ TestCaseWithFactory,
+ person_logged_in,
+ )
+from lp.translations.interfaces.translationimportqueue import (
+ ITranslationImportQueue,
+ )
+from lp.translations.utilities.translationsharinginfo import (
+ has_upstream_template,
+ )
class TestSourcePackageRelease(TestCaseWithFactory):
@@ -54,3 +67,45 @@
# does not have to be valid.
spr = self.factory.makeSourcePackageRelease(homepage="<invalid<url")
self.assertEquals("<invalid<url", spr.homepage)
+
+ def makeTranslationsLFA(self):
+ """Create an LibraryFileAlias containing dummy translation data."""
+ test_tar_content = {
+ 'source/po/foo.pot': 'Foo template',
+ }
+ tarfile_content = LaunchpadWriteTarFile.files_to_string(
+ test_tar_content)
+ return self.factory.makeLibraryFileAlias(content=tarfile_content)
+
+ def test_attachTranslationFiles__no_translation_sharing(self):
+ # If translation sharing is disabled,
+ # SourcePackageRelease.attachTranslationFiles() creates a job
+ # in the translation import queue.
+ spr = self.factory.makeSourcePackageRelease()
+ self.assertFalse(has_upstream_template(spr.sourcepackage))
+ lfa = self.makeTranslationsLFA()
+ transaction.commit()
+ spr.attachTranslationFiles(lfa, True, spr.maintainer)
+ translation_import_queue = getUtility(ITranslationImportQueue)
+ entries_in_queue = translation_import_queue.getAllEntries(
+ target=spr.sourcepackage).count()
+ self.assertEqual(1, entries_in_queue)
+
+ def test_attachTranslationFiles__translation_sharing(self):
+ # If translation sharing is enabled,
+ # SourcePackageRelease.attachTranslationFiles() does nothing.
+ spr = self.factory.makeSourcePackageRelease()
+ sourcepackage = spr.sourcepackage
+ productseries = self.factory.makeProductSeries()
+ self.factory.makePOTemplate(productseries=productseries)
+ with person_logged_in(sourcepackage.distroseries.owner):
+ sourcepackage.setPackaging(
+ productseries, sourcepackage.distroseries.owner)
+ self.assertTrue(has_upstream_template(sourcepackage))
+ lfa = self.makeTranslationsLFA()
+ transaction.commit()
+ spr.attachTranslationFiles(lfa, True, spr.maintainer)
+ translation_import_queue = getUtility(ITranslationImportQueue)
+ entries_in_queue = translation_import_queue.getAllEntries(
+ target=sourcepackage).count()
+ self.assertEqual(0, entries_in_queue)
