launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17273
Re: [Merge] lp:~cjohnston/launchpad/1171047-ppa-trans-en into lp:launchpad
Review: Needs Fixing code
Diff comments:
> === modified file 'lib/lp/archivepublisher/publishing.py'
> --- lib/lp/archivepublisher/publishing.py 2014-07-30 02:22:37 +0000
> +++ lib/lp/archivepublisher/publishing.py 2014-07-31 18:09:12 +0000
> @@ -55,6 +55,7 @@
> from lp.registry.model.distroseries import DistroSeries
> from lp.services.database.constants import UTC_NOW
> from lp.services.database.interfaces import IStore
> +from lp.services.features import getFeatureFlag
> from lp.services.librarian.client import LibrarianClient
> from lp.services.utils import file_exists
> from lp.soyuz.enums import (
> @@ -185,6 +186,12 @@
> return os.path.join(component_root, subcomp, arch_path, "Packages")
>
>
> +def get_translations_path(config, suite_name, component):
> + """Return path to Translations file for the given arguments."""
> + return os.path.join(
> + config.distsroot, suite_name, component.name, "i18n", "Translation-en")
This function is six extra lines to replace one. I'm not sure it's pulling its weight.
> +
> +
> class I18nIndex(_multivalued):
> """Represents an i18n/Index file."""
> _multivalued_fields = {
> @@ -640,6 +647,17 @@
>
> self.log.debug("Generating Sources")
>
> + long_descriptions = False
> + if (not distroseries.include_long_descriptions and
> + getFeatureFlag("soyuz.ppa.separate_long_descriptions")):
> + # If include_long_descriptions is False and the feature flag is
> + # enabled, create Translation-en file for PPAs.
> + long_descriptions = True
The long_descriptions flag is confusingly named here. It really means that the main indices should *not* include long descriptions -- perhaps separate_long_descriptions might make more sense.
Also, the comment shouldn't talk about PPAs (NMAF isn't just used for PPAs), and it should mention that getIndexStanza() will also omit the long description from Packages when the flag is set.
The comment would be entirely redundant if the variable name were more descriptive.
> + packages = {}
Does this need to be a dict rather than a set?
> + translation_en = RepositoryIndexFile(
> + get_translations_path(self._config, suite_name, component),
> + self._config.temproot)
> +
> source_index = RepositoryIndexFile(
> get_sources_path(self._config, suite_name, component),
> self._config.temproot)
> @@ -679,12 +697,24 @@
> # for, eg. ddebs where publish_debug_symbols is
> # disabled.
> continue
> - stanza = bpp.getIndexStanza().encode('utf-8') + '\n\n'
> + stanza = bpp.getIndexStanza(long_descriptions).encode(
> + 'utf-8') + '\n\n'
> indices[subcomp].write(stanza)
> + if long_descriptions:
> + # Write Package, Description-md5, and Description-en to
> + # Translation-en file.
This needs to indicate that getTranslationsStanza will record its details in the dict that is passed to it, and return None if the same stanza has already been written. Methods that mutate their arguments are particularly easy to misuse, so admonishments in affected code are critical.
> + translation_stanza = bpp.getTranslationsStanza(packages)
> + if translation_stanza:
> + translation_stanza = translation_stanza.encode(
> + 'utf-8') + '\n\n'
> + translation_en.write(translation_stanza)
>
> for index in indices.itervalues():
> index.close()
>
> + if long_descriptions:
> + translation_en.close()
> +
> def cannotModifySuite(self, distroseries, pocket):
> """Return True if the distroseries is stable and pocket is release."""
> return (not distroseries.isUnstable() and
>
> === modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
> --- lib/lp/archivepublisher/tests/test_publisher.py 2014-07-30 02:22:37 +0000
> +++ lib/lp/archivepublisher/tests/test_publisher.py 2014-07-31 18:09:12 +0000
> @@ -43,6 +43,8 @@
> from lp.registry.interfaces.series import SeriesStatus
> from lp.services.config import config
> from lp.services.database.constants import UTC_NOW
> +from lp.services.features import getFeatureFlag
> +from lp.services.features.testing import FeatureFixture
> from lp.services.gpg.interfaces import IGPGHandler
> from lp.services.log.logger import (
> BufferLogger,
> @@ -1043,8 +1045,9 @@
>
> return index_contents
>
> - def testPPAArchiveIndex(self):
> - """Building Archive Indexes from PPA publications."""
> + def setupPPAArchiveIndexTest(self, long_descriptions=True,
> + feature_flag=False):
> + # Setup for testPPAArchiveIndex tests
> allowed_suites = []
>
> cprov = getUtility(IPersonSet).getByName('cprov')
> @@ -1074,143 +1077,381 @@
> pub_source=ignored_source, binaryname='bingo',
> description='nice udeb', format=BinaryPackageFormat.UDEB)[0]
>
> + if feature_flag:
> + # Enabled corresponding feature flag.
> + self.useFixture(FeatureFixture({
> + 'soyuz.ppa.separate_long_descriptions': 'enabled'}))
> + self.assertEqual('enabled', getFeatureFlag(
> + 'soyuz.ppa.separate_long_descriptions'))
> +
> + if not long_descriptions:
> + # Make sure that NMAF generates i18n/Translation-en* files.
> + ds = self.ubuntutest.getSeries('breezy-autotest')
> + ds.include_long_descriptions = False
> +
> archive_publisher.A_publish(False)
> self.layer.txn.commit()
> archive_publisher.C_writeIndexes(False)
> -
> - # A compressed and uncompressed Sources file are written;
> - # ensure that they are the same after uncompressing the former.
> - index_contents = self._checkCompressedFile(
> - archive_publisher, os.path.join('source', 'Sources.bz2'),
> - os.path.join('source', 'Sources'))
> -
> - index_contents = self._checkCompressedFile(
> - archive_publisher, os.path.join('source', 'Sources.gz'),
> - os.path.join('source', 'Sources'))
> -
> - self.assertEqual(
> - ['Package: foo',
> - 'Binary: foo-bin',
> - 'Version: 666',
> - 'Section: base',
> - 'Maintainer: Foo Bar <foo@xxxxxxx>',
> - 'Architecture: all',
> - 'Standards-Version: 3.6.2',
> - 'Format: 1.0',
> - 'Directory: pool/main/f/foo',
> - 'Files:',
> - ' 3e25960a79dbc69b674cd4ec67a72c62 11 foo_1.dsc',
> - 'Checksums-Sha1:',
> - ' 7b502c3a1f48c8609ae212cdfb639dee39673f5e 11 foo_1.dsc',
> - 'Checksums-Sha256:',
> - ' 64ec88ca00b268e5ba1a35678a1b5316d212f4f366b2477232534a8aeca37f'
> - '3c 11 foo_1.dsc',
> -
> - ''],
> - index_contents)
> -
> - # A compressed and an uncompressed Packages file are written;
> - # ensure that they are the same after uncompressing the former.
> - index_contents = self._checkCompressedFile(
> - archive_publisher, os.path.join('binary-i386', 'Packages.bz2'),
> - os.path.join('binary-i386', 'Packages'))
> -
> - index_contents = self._checkCompressedFile(
> - archive_publisher, os.path.join('binary-i386', 'Packages.gz'),
> - os.path.join('binary-i386', 'Packages'))
> -
> - self.assertEqual(
> - ['Package: foo-bin',
> - 'Source: foo',
> - 'Priority: standard',
> - 'Section: base',
> - 'Installed-Size: 100',
> - 'Maintainer: Foo Bar <foo@xxxxxxx>',
> - 'Architecture: all',
> - 'Version: 666',
> - 'Filename: pool/main/f/foo/foo-bin_666_all.deb',
> - 'Size: 18',
> - 'MD5sum: 008409e7feb1c24a6ccab9f6a62d24c5',
> - 'SHA1: 30b7b4e583fa380772c5a40e428434628faef8cf',
> - 'SHA256: 006ca0f356f54b1916c24c282e6fd19961f4356441401f4b0966f2a'
> - '00bb3e945',
> - 'Description: Foo app is great',
> - ' My leading spaces are normalised to a single space but not '
> - 'trailing. ',
> - ' It does nothing, though',
> - ''],
> - index_contents)
> -
> - # A compressed and an uncompressed Packages file are written for
> - # 'debian-installer' section for each architecture. It will list
> - # the 'udeb' files.
> - index_contents = self._checkCompressedFile(
> - archive_publisher,
> - os.path.join('debian-installer', 'binary-i386', 'Packages.bz2'),
> - os.path.join('debian-installer', 'binary-i386', 'Packages'))
> -
> - index_contents = self._checkCompressedFile(
> - archive_publisher,
> - os.path.join('debian-installer', 'binary-i386', 'Packages.gz'),
> - os.path.join('debian-installer', 'binary-i386', 'Packages'))
> -
> - self.assertEqual(
> - ['Package: bingo',
> - 'Source: foo',
> - 'Priority: standard',
> - 'Section: base',
> - 'Installed-Size: 100',
> - 'Maintainer: Foo Bar <foo@xxxxxxx>',
> - 'Architecture: all',
> - 'Version: 666',
> - 'Filename: pool/main/f/foo/bingo_666_all.udeb',
> - 'Size: 18',
> - 'MD5sum: 008409e7feb1c24a6ccab9f6a62d24c5',
> - 'SHA1: 30b7b4e583fa380772c5a40e428434628faef8cf',
> - 'SHA256: 006ca0f356f54b1916c24c282e6fd19961f4356441401f4b0966f2a'
> - '00bb3e945',
> - 'Description: Foo app is great',
> - ' nice udeb',
> - ''],
> - index_contents)
> -
> - # 'debug' too, when publish_debug_symbols is enabled.
> - index_contents = self._checkCompressedFile(
> - archive_publisher,
> - os.path.join('debug', 'binary-i386', 'Packages.bz2'),
> - os.path.join('debug', 'binary-i386', 'Packages'))
> -
> - index_contents = self._checkCompressedFile(
> - archive_publisher,
> - os.path.join('debug', 'binary-i386', 'Packages.gz'),
> - os.path.join('debug', 'binary-i386', 'Packages'))
> -
> - self.assertEqual(
> - ['Package: foo-bin-dbgsym',
> - 'Source: foo',
> - 'Priority: standard',
> - 'Section: base',
> - 'Installed-Size: 100',
> - 'Maintainer: Foo Bar <foo@xxxxxxx>',
> - 'Architecture: all',
> - 'Version: 666',
> - 'Filename: pool/main/f/foo/foo-bin-dbgsym_666_all.ddeb',
> - 'Size: 18',
> - 'MD5sum: 008409e7feb1c24a6ccab9f6a62d24c5',
> - 'SHA1: 30b7b4e583fa380772c5a40e428434628faef8cf',
> - 'SHA256: 006ca0f356f54b1916c24c282e6fd19961f4356441401f4b0966f2a'
> - '00bb3e945',
> - 'Description: Foo app is great',
> - ' My leading spaces are normalised to a single space but not '
> - 'trailing. ',
> - ' It does nothing, though',
> - ''],
> - index_contents)
> -
> - # We always regenerate all Releases file for a given suite.
> - self.assertTrue(
> - ('breezy-autotest', PackagePublishingPocket.RELEASE) in
> - archive_publisher.release_files_needed)
> + archive_publisher.D_writeReleaseFiles(False)
> + return archive_publisher
> +
> + def testPPAArchiveIndex(self):
> + """Building Archive Indexes from PPA publications."""
> + archive_publisher = self.setupPPAArchiveIndexTest()
> +
> + # A compressed and uncompressed Sources file are written;
> + # ensure that they are the same after uncompressing the former.
> + index_contents = self._checkCompressedFile(
> + archive_publisher, os.path.join('source', 'Sources.bz2'),
> + os.path.join('source', 'Sources'))
> +
> + index_contents = self._checkCompressedFile(
> + archive_publisher, os.path.join('source', 'Sources.gz'),
> + os.path.join('source', 'Sources'))
> +
> + self.assertEqual(
> + ['Package: foo',
> + 'Binary: foo-bin',
> + 'Version: 666',
> + 'Section: base',
> + 'Maintainer: Foo Bar <foo@xxxxxxx>',
> + 'Architecture: all',
> + 'Standards-Version: 3.6.2',
> + 'Format: 1.0',
> + 'Directory: pool/main/f/foo',
> + 'Files:',
> + ' 3e25960a79dbc69b674cd4ec67a72c62 11 foo_1.dsc',
> + 'Checksums-Sha1:',
> + ' 7b502c3a1f48c8609ae212cdfb639dee39673f5e 11 foo_1.dsc',
> + 'Checksums-Sha256:',
> + ' 64ec88ca00b268e5ba1a35678a1b5316d212f4f366b2477232534a8aeca37f'
> + '3c 11 foo_1.dsc',
> +
> + ''],
> + index_contents)
> +
> + # A compressed and an uncompressed Packages file are written;
> + # ensure that they are the same after uncompressing the former.
> + index_contents = self._checkCompressedFile(
> + archive_publisher, os.path.join('binary-i386', 'Packages.bz2'),
> + os.path.join('binary-i386', 'Packages'))
> +
> + index_contents = self._checkCompressedFile(
> + archive_publisher, os.path.join('binary-i386', 'Packages.gz'),
> + os.path.join('binary-i386', 'Packages'))
> +
> + self.assertEqual(
> + ['Package: foo-bin',
> + 'Source: foo',
> + 'Priority: standard',
> + 'Section: base',
> + 'Installed-Size: 100',
> + 'Maintainer: Foo Bar <foo@xxxxxxx>',
> + 'Architecture: all',
> + 'Version: 666',
> + 'Filename: pool/main/f/foo/foo-bin_666_all.deb',
> + 'Size: 18',
> + 'MD5sum: 008409e7feb1c24a6ccab9f6a62d24c5',
> + 'SHA1: 30b7b4e583fa380772c5a40e428434628faef8cf',
> + 'SHA256: 006ca0f356f54b1916c24c282e6fd19961f4356441401f4b0966f2a'
> + '00bb3e945',
> + 'Description: Foo app is great',
> + ' My leading spaces are normalised to a single space but not '
> + 'trailing. ',
> + ' It does nothing, though',
> + ''],
> + index_contents)
> +
> + # A compressed and an uncompressed Packages file are written for
> + # 'debian-installer' section for each architecture. It will list
> + # the 'udeb' files.
> + index_contents = self._checkCompressedFile(
> + archive_publisher,
> + os.path.join('debian-installer', 'binary-i386', 'Packages.bz2'),
> + os.path.join('debian-installer', 'binary-i386', 'Packages'))
> +
> + index_contents = self._checkCompressedFile(
> + archive_publisher,
> + os.path.join('debian-installer', 'binary-i386', 'Packages.gz'),
> + os.path.join('debian-installer', 'binary-i386', 'Packages'))
> +
> + self.assertEqual(
> + ['Package: bingo',
> + 'Source: foo',
> + 'Priority: standard',
> + 'Section: base',
> + 'Installed-Size: 100',
> + 'Maintainer: Foo Bar <foo@xxxxxxx>',
> + 'Architecture: all',
> + 'Version: 666',
> + 'Filename: pool/main/f/foo/bingo_666_all.udeb',
> + 'Size: 18',
> + 'MD5sum: 008409e7feb1c24a6ccab9f6a62d24c5',
> + 'SHA1: 30b7b4e583fa380772c5a40e428434628faef8cf',
> + 'SHA256: 006ca0f356f54b1916c24c282e6fd19961f4356441401f4b0966f2a'
> + '00bb3e945',
> + 'Description: Foo app is great',
> + ' nice udeb',
> + ''],
> + index_contents)
> +
> + # 'debug' too, when publish_debug_symbols is enabled.
> + index_contents = self._checkCompressedFile(
> + archive_publisher,
> + os.path.join('debug', 'binary-i386', 'Packages.bz2'),
> + os.path.join('debug', 'binary-i386', 'Packages'))
> +
> + index_contents = self._checkCompressedFile(
> + archive_publisher,
> + os.path.join('debug', 'binary-i386', 'Packages.gz'),
> + os.path.join('debug', 'binary-i386', 'Packages'))
> +
> + self.assertEqual(
> + ['Package: foo-bin-dbgsym',
> + 'Source: foo',
> + 'Priority: standard',
> + 'Section: base',
> + 'Installed-Size: 100',
> + 'Maintainer: Foo Bar <foo@xxxxxxx>',
> + 'Architecture: all',
> + 'Version: 666',
> + 'Filename: pool/main/f/foo/foo-bin-dbgsym_666_all.ddeb',
> + 'Size: 18',
> + 'MD5sum: 008409e7feb1c24a6ccab9f6a62d24c5',
> + 'SHA1: 30b7b4e583fa380772c5a40e428434628faef8cf',
> + 'SHA256: 006ca0f356f54b1916c24c282e6fd19961f4356441401f4b0966f2a'
> + '00bb3e945',
> + 'Description: Foo app is great',
> + ' My leading spaces are normalised to a single space but not '
> + 'trailing. ',
> + ' It does nothing, though',
> + ''],
> + index_contents)
> +
> + # We always regenerate all Releases file for a given suite.
> + self.assertTrue(
> + ('breezy-autotest', PackagePublishingPocket.RELEASE) in
> + archive_publisher.release_files_needed)
> +
> + # Confirm that i18n files are not created
> + i18n_path = os.path.join(archive_publisher._config.distsroot,
> + 'breezy-autotest', 'main', 'i18n')
> + self.assertFalse(os.path.exists(
> + os.path.join(i18n_path, 'Translation-en')))
> + self.assertFalse(os.path.exists(
> + os.path.join(i18n_path, 'Translation-en.gz')))
> + self.assertFalse(os.path.exists(
> + os.path.join(i18n_path, 'Translation-en.bz2')))
> +
> + # remove PPA root
> + shutil.rmtree(config.personalpackagearchive.root)
> +
> + def testPPAArchiveIndexLongDescriptionsFalseFeatureFlagDisabled(self):
> + # Building Archive Indexes from PPA publications with
> + # include_long_descriptions = False but the feature flag being disabled
> + archive_publisher = self.setupPPAArchiveIndexTest(
> + long_descriptions=False)
> +
> + # Confirm that i18n files are not created
> + i18n_path = os.path.join(archive_publisher._config.distsroot,
> + 'breezy-autotest', 'main', 'i18n')
> + self.assertFalse(os.path.exists(
> + os.path.join(i18n_path, 'Translation-en')))
> + self.assertFalse(os.path.exists(
> + os.path.join(i18n_path, 'Translation-en.gz')))
> + self.assertFalse(os.path.exists(
> + os.path.join(i18n_path, 'Translation-en.bz2')))
> +
> + # remove PPA root
> + shutil.rmtree(config.personalpackagearchive.root)
> +
> + def testPPAArchiveIndexLongDescriptionsFalse(self):
> + # Building Archive Indexes from PPA publications with
> + # include_long_descriptions = False.
> + archive_publisher = self.setupPPAArchiveIndexTest(
> + long_descriptions=False, feature_flag=True)
> +
> + # A compressed and uncompressed Sources file are written;
> + # ensure that they are the same after uncompressing the former.
> + index_contents = self._checkCompressedFile(
> + archive_publisher, os.path.join('source', 'Sources.bz2'),
> + os.path.join('source', 'Sources'))
> +
> + index_contents = self._checkCompressedFile(
> + archive_publisher, os.path.join('source', 'Sources.gz'),
> + os.path.join('source', 'Sources'))
> +
> + self.assertEqual(
> + ['Package: foo',
> + 'Binary: foo-bin',
> + 'Version: 666',
> + 'Section: base',
> + 'Maintainer: Foo Bar <foo@xxxxxxx>',
> + 'Architecture: all',
> + 'Standards-Version: 3.6.2',
> + 'Format: 1.0',
> + 'Directory: pool/main/f/foo',
> + 'Files:',
> + ' 3e25960a79dbc69b674cd4ec67a72c62 11 foo_1.dsc',
> + 'Checksums-Sha1:',
> + ' 7b502c3a1f48c8609ae212cdfb639dee39673f5e 11 foo_1.dsc',
> + 'Checksums-Sha256:',
> + ' 64ec88ca00b268e5ba1a35678a1b5316d212f4f366b2477232534a8aeca37f'
> + '3c 11 foo_1.dsc',
> +
> + ''],
> + index_contents)
> +
> + # A compressed and an uncompressed Packages file are written;
> + # ensure that they are the same after uncompressing the former.
> + index_contents = self._checkCompressedFile(
> + archive_publisher, os.path.join('binary-i386', 'Packages.bz2'),
> + os.path.join('binary-i386', 'Packages'))
> +
> + index_contents = self._checkCompressedFile(
> + archive_publisher, os.path.join('binary-i386', 'Packages.gz'),
> + os.path.join('binary-i386', 'Packages'))
> +
> + self.assertEqual(
> + ['Package: foo-bin',
> + 'Source: foo',
> + 'Priority: standard',
> + 'Section: base',
> + 'Installed-Size: 100',
> + 'Maintainer: Foo Bar <foo@xxxxxxx>',
> + 'Architecture: all',
> + 'Version: 666',
> + 'Filename: pool/main/f/foo/foo-bin_666_all.deb',
> + 'Size: 18',
> + 'MD5sum: 008409e7feb1c24a6ccab9f6a62d24c5',
> + 'SHA1: 30b7b4e583fa380772c5a40e428434628faef8cf',
> + 'SHA256: 006ca0f356f54b1916c24c282e6fd19961f4356441401f4b0966f2a'
> + '00bb3e945',
> + 'Description: Foo app is great',
> + 'Description-md5: 6f43b8a2c247729beb1a59e5c336ffe2',
> + ''],
> + index_contents)
> +
> + # A compressed and an uncompressed Packages file are written for
> + # 'debian-installer' section for each architecture. It will list
> + # the 'udeb' files.
> + index_contents = self._checkCompressedFile(
> + archive_publisher,
> + os.path.join('debian-installer', 'binary-i386', 'Packages.bz2'),
> + os.path.join('debian-installer', 'binary-i386', 'Packages'))
> +
> + index_contents = self._checkCompressedFile(
> + archive_publisher,
> + os.path.join('debian-installer', 'binary-i386', 'Packages.gz'),
> + os.path.join('debian-installer', 'binary-i386', 'Packages'))
> +
> + self.assertEqual(
> + ['Package: bingo',
> + 'Source: foo',
> + 'Priority: standard',
> + 'Section: base',
> + 'Installed-Size: 100',
> + 'Maintainer: Foo Bar <foo@xxxxxxx>',
> + 'Architecture: all',
> + 'Version: 666',
> + 'Filename: pool/main/f/foo/bingo_666_all.udeb',
> + 'Size: 18',
> + 'MD5sum: 008409e7feb1c24a6ccab9f6a62d24c5',
> + 'SHA1: 30b7b4e583fa380772c5a40e428434628faef8cf',
> + 'SHA256: 006ca0f356f54b1916c24c282e6fd19961f4356441401f4b0966f2a'
> + '00bb3e945',
> + 'Description: Foo app is great',
> + 'Description-md5: f98d4e4b123ac43e12f6848e15b67246',
> + ''],
> + index_contents)
> +
> + # 'debug' too, when publish_debug_symbols is enabled.
> + index_contents = self._checkCompressedFile(
> + archive_publisher,
> + os.path.join('debug', 'binary-i386', 'Packages.bz2'),
> + os.path.join('debug', 'binary-i386', 'Packages'))
> +
> + index_contents = self._checkCompressedFile(
> + archive_publisher,
> + os.path.join('debug', 'binary-i386', 'Packages.gz'),
> + os.path.join('debug', 'binary-i386', 'Packages'))
> +
> + self.assertEqual(
> + ['Package: foo-bin-dbgsym',
> + 'Source: foo',
> + 'Priority: standard',
> + 'Section: base',
> + 'Installed-Size: 100',
> + 'Maintainer: Foo Bar <foo@xxxxxxx>',
> + 'Architecture: all',
> + 'Version: 666',
> + 'Filename: pool/main/f/foo/foo-bin-dbgsym_666_all.ddeb',
> + 'Size: 18',
> + 'MD5sum: 008409e7feb1c24a6ccab9f6a62d24c5',
> + 'SHA1: 30b7b4e583fa380772c5a40e428434628faef8cf',
> + 'SHA256: 006ca0f356f54b1916c24c282e6fd19961f4356441401f4b0966f2a'
> + '00bb3e945',
> + 'Description: Foo app is great',
> + 'Description-md5: 6f43b8a2c247729beb1a59e5c336ffe2',
> + ''],
> + index_contents)
> +
> + # We always regenerate all Releases file for a given suite.
> + self.assertTrue(
> + ('breezy-autotest', PackagePublishingPocket.RELEASE) in
> + archive_publisher.release_files_needed)
> +
> + # A compressed and an uncompressed Translation-en file is written.
> + # ensure that they are the same after uncompressing the former.
> + index_contents = self._checkCompressedFile(
> + archive_publisher, os.path.join('i18n', 'Translation-en.gz'),
> + os.path.join('i18n', 'Translation-en'))
> +
> + index_contents = self._checkCompressedFile(
> + archive_publisher, os.path.join('i18n', 'Translation-en.bz2'),
> + os.path.join('i18n', 'Translation-en'))
> +
> + self.assertEqual(
> + ['Package: bingo',
> + 'Description-md5: f98d4e4b123ac43e12f6848e15b67246',
> + 'Description-en: Foo app is great',
> + ' nice udeb',
> + '',
> + 'Package: foo-bin',
> + 'Description-md5: 6f43b8a2c247729beb1a59e5c336ffe2',
> + 'Description-en: Foo app is great',
> + ' My leading spaces are normalised to a single space but not '
> + 'trailing. ',
> + ' It does nothing, though',
> + '',
> + 'Package: foo-bin-dbgsym',
> + 'Description-md5: 6f43b8a2c247729beb1a59e5c336ffe2',
> + 'Description-en: Foo app is great',
> + ' My leading spaces are normalised to a single space but not '
> + 'trailing. ',
> + ' It does nothing, though',
> + '',
> + ],
> + index_contents)
> +
> + series = os.path.join(archive_publisher._config.distsroot,
> + 'breezy-autotest')
> + i18n_index = os.path.join(series, 'main', 'i18n', 'Index')
> +
> + # The i18n/Index file has been generated.
> + self.assertTrue(os.path.exists(i18n_index))
> +
> + # It is listed correctly in Release.
> + release_path = os.path.join(series, 'Release')
> + release = self.parseRelease(release_path)
> + with open(i18n_index) as i18n_index_file:
> + self.assertReleaseContentsMatch(
> + release, 'main/i18n/Index', i18n_index_file.read())
> +
> + release_path = os.path.join(series, 'Release')
> + with open(release_path) as release_file:
> + content = release_file.read()
> + self.assertIn('main/i18n/Translation-en.bz2', content)
>
> # remove PPA root
> shutil.rmtree(config.personalpackagearchive.root)
> @@ -1696,7 +1937,7 @@
> os.path.join('main', 'i18n', 'Index'), all_files.pop())
> self.assertEqual(
> os.path.join('main', 'i18n', 'Translation-en.bz2'),
> - all_files.pop())
> + all_files.pop())
>
> def testWriteSuiteI18nMissingDirectory(self):
> """i18n/Index is not generated when the i18n directory is missing."""
>
> === modified file 'lib/lp/services/features/flags.py'
> --- lib/lp/services/features/flags.py 2014-05-29 15:02:53 +0000
> +++ lib/lp/services/features/flags.py 2014-07-31 18:09:12 +0000
> @@ -1,4 +1,4 @@
> -# Copyright 2010-2013 Canonical Ltd. This software is licensed under the
> +# Copyright 2010-2014 Canonical Ltd. This software is licensed under the
> # GNU Affero General Public License version 3 (see the file LICENSE).
>
> __all__ = [
> @@ -227,6 +227,12 @@
> 'disabled',
> 'Inline diff comments',
> ''),
> + ('soyuz.ppa.separate_long_descriptions',
> + 'boolean',
> + 'If true, PPAs will create an i18n/Translations-en file',
> + 'disabled',
> + 'PPA Separate Long Descriptions',
> + ''),
> ])
>
> # The set of all flag names that are documented.
>
> === modified file 'lib/lp/soyuz/interfaces/publishing.py'
> --- lib/lp/soyuz/interfaces/publishing.py 2014-07-08 22:31:49 +0000
> +++ lib/lp/soyuz/interfaces/publishing.py 2014-07-31 18:09:12 +0000
> @@ -182,6 +182,20 @@
> the field name and value is the value string.
> """
>
> + def getTranslationsStanza():
> + """Return archive Translation-en stanza contents
> +
> + It's based on the locally provided buildTranslationsStanzaTemplate
> + method, which differs for binary and source instances.
> + """
> +
> + def buildTranslationsStanzaFields():
> + """Build a map of fields and values to be in the Translation-en file.
> +
> + The fields and values ae mapped into a dictionary, where the key is
> + the field name and value is the value string.
> + """
The packages argument for both of these methods should be documented, particularly since it mutates the given object.
Also, getIndexStanza and buildIndexStanzaFields need their new argument documented, particularly since its meaning is inverted from what is intuitive.
> +
> def requestObsolescence():
> """Make this publication obsolete.
>
>
> === modified file 'lib/lp/soyuz/model/publishing.py'
> --- lib/lp/soyuz/model/publishing.py 2014-07-29 07:29:32 +0000
> +++ lib/lp/soyuz/model/publishing.py 2014-07-31 18:09:12 +0000
> @@ -17,6 +17,7 @@
>
> from collections import defaultdict
> from datetime import datetime
> +import hashlib
> from operator import attrgetter
> import os
> import re
> @@ -310,11 +311,16 @@
> else:
> self.setPublished()
>
> - def getIndexStanza(self):
> + def getIndexStanza(self, long_descriptions=False):
> """See `IPublishing`."""
> - fields = self.buildIndexStanzaFields()
> + fields = self.buildIndexStanzaFields(long_descriptions)
> return fields.makeOutput()
>
> + def getTranslationsStanza(self, packages):
> + """See `IPublishing`."""
> + fields = self.buildTranslationsStanzaFields(packages)
> + return fields
This is the identity function.
> +
> def setSuperseded(self):
> """Set to SUPERSEDED status."""
> self.status = PackagePublishingStatus.SUPERSEDED
> @@ -699,7 +705,7 @@
> def _formatFileList(self, l):
> return ''.join('\n %s %s %s' % ((h,) + f) for (h, f) in l)
>
> - def buildIndexStanzaFields(self):
> + def buildIndexStanzaFields(self, long_descriptions):
> """See `IPublishing`."""
> # Special fields preparation.
> spr = self.sourcepackagerelease
> @@ -991,7 +997,7 @@
> else:
> super(BinaryPackagePublishingHistory, self).publish(diskpool, log)
>
> - def buildIndexStanzaFields(self):
> + def buildIndexStanzaFields(self, long_descriptions=False):
> """See `IPublishing`."""
> bpr = self.binarypackagerelease
> spr = bpr.build.source_package_release
> @@ -1006,14 +1012,22 @@
> bin_filepath = os.path.join(
> makePoolPath(spr.name, self.component.name), bin_filename)
> # description field in index is an association of summary and
> - # description, as:
> + # description or the summary only if include_long_descriptions
> + # is false, as:
> #
> # Descrition: <SUMMARY>\n
> # <DESCRIPTION L1>
> # ...
> # <DESCRIPTION LN>
> descr_lines = [line.lstrip() for line in bpr.description.splitlines()]
> - bin_description = '%s\n %s' % (bpr.summary, '\n '.join(descr_lines))
> + description = '%s\n %s' % (bpr.summary, '\n '.join(descr_lines))
> + bin_description_md5 = hashlib.md5(description).hexdigest()
> + if long_descriptions:
> + # If include_long_descriptions is False, the description should be
> + # the summary
> + bin_description = bpr.summary
> + else:
> + bin_description = description
>
> # Dealing with architecturespecific field.
> # Present 'all' in every archive index for architecture
> @@ -1060,6 +1074,8 @@
> fields.append(
> 'Phased-Update-Percentage', self.phased_update_percentage)
> fields.append('Description', bin_description)
> + if long_descriptions:
> + fields.append('Description-md5', bin_description_md5)
> if bpr.user_defined_fields:
> fields.extend(bpr.user_defined_fields)
>
> @@ -1069,6 +1085,31 @@
>
> return fields
>
> + def buildTranslationsStanzaFields(self, packages):
> + """See `IPublishing`."""
> + bpr = self.binarypackagerelease
> +
> + # description field in index is an association of summary and
> + # description or the summary only if include_long_descriptions
> + # is false, as:
> + #
> + # Descrition: <SUMMARY>\n
> + # <DESCRIPTION L1>
> + # ...
> + # <DESCRIPTION LN>
> + descr_lines = [line.lstrip() for line in bpr.description.splitlines()]
> + bin_description = '%s\n %s' % (bpr.summary, '\n '.join(descr_lines))
> + bin_description_md5 = hashlib.md5(bin_description).hexdigest()
descr_lines, bin_description, bin_description_md5 and the big comment are almost a direct copy from buildIndexStanzaFields. Can this chunk be factored out to avoid mistakes?
> + if not packages.get((bpr.name, bin_description_md5)):
> + fields = IndexStanzaFields()
> + fields.append('Package', bpr.name)
> + fields.append('Description-md5', bin_description_md5)
> + fields.append('Description-en', bin_description)
> + packages[(bpr.name, bin_description_md5)] = True
> + return fields.makeOutput()
This method returns a stanza string, not fields like its name says and its sibling does.
> + else:
> + return None
> +
> def _getOtherPublications(self):
> """Return remaining publications with the same overrides.
>
>
--
https://code.launchpad.net/~cjohnston/launchpad/1171047-ppa-trans-en/+merge/228978
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
References
