launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17271
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 04:59:22 +0000
> @@ -10,8 +10,10 @@
>
> __metaclass__ = type
>
> +import bz2
> from datetime import datetime
> import errno
> +import gzip
> import hashlib
> from itertools import (
> chain,
> @@ -42,7 +44,10 @@
> from lp.archivepublisher.interfaces.archivesigningkey import (
> IArchiveSigningKey,
> )
> -from lp.archivepublisher.model.ftparchive import FTPArchiveHandler
> +from lp.archivepublisher.model.ftparchive import (
> + FTPArchiveHandler,
> + safe_mkdir,
> + )
> from lp.archivepublisher.utils import (
> get_ppa_reference,
> RepositoryIndexFile,
> @@ -55,6 +60,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 (
> @@ -640,6 +646,17 @@
>
> self.log.debug("Generating Sources")
>
> + flag_enabled = getFeatureFlag("soyuz.ppa.separate_long_descriptions")
> + long_descriptions = distroseries.include_long_descriptions
> + if not long_descriptions and flag_enabled:
> + i18n_path = os.path.join(
> + self._config.distsroot, suite_name, component.name, "i18n")
> + safe_mkdir(i18n_path)
RepositoryIndexFile handles this for you.
> + translations_file = os.path.join(i18n_path, "Translation-en")
> + # create empty Translations-en file
> + open(translations_file, "w")
> + packages = []
> +
> source_index = RepositoryIndexFile(
> get_sources_path(self._config, suite_name, component),
> self._config.temproot)
> @@ -681,6 +698,36 @@
> continue
> stanza = bpp.getIndexStanza().encode('utf-8') + '\n\n'
> indices[subcomp].write(stanza)
> + if not long_descriptions and flag_enabled:
> + # Write Package, Description-md5, and Description-en to
> + # Translation-en file.
> + bpr = bpp.binarypackagerelease
> + if bpr.name not in packages:
What if a package has different descriptions on different architectures?
Also, packages is a list. That's going to be very slow on large sets of packages.
> + md5 = hashlib.md5(bpr.description).hexdigest()
The hash needs to be over the entire Description field: that includes BPR.summary, and formats BPR.description differently.
> + descr_lines = [line.lstrip() for line in
> + bpr.description.splitlines()]
> + description = '%s\n %s' % (
> + bpr.summary, '\n '.join(descr_lines))
> + package = (
> + "Package: %s\n"
> + "Description-md5: %s\n"
> + "Description-en: %s\n\n" % (
> + bpr.name, md5, description))
Did you consider using IndexStanzaFields like all the other bits of the publisher?
> + with open(translations_file, "a") as translation_file:
> + translation_file.write(package)
Opening and closing the same file tens of thousands of times isn't going to be particularly fast. Why does it need to be reopened?
> + packages.append(bpr.name)
> +
> + if not long_descriptions and flag_enabled:
> + # Create .gz and .bz3 Translation-en files
> + with open(translations_file, "rb") as translation_file:
> + tf = translation_file.read()
> + gz_out = gzip.open(translations_file + '.gz', "wb")
> + gz_out.writelines(tf)
read() returns a single string, not a set of lines, so writelines() is pointless.
> + gz_out.close()
> + bz2_out = bz2.BZ2File(translations_file + '.bz2', "wb")
> + for line in tf.splitlines(True):
> + bz2_out.write(line)
Why split it up?
> + bz2_out.close()
RepositoryIndexFile handles all of the compression for you.
>
> for index in indices.itervalues():
> index.close()
>
> === 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 04:59:22 +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: 17ba15f011398a911aa9017c653a0760',
> + ''],
> + 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: 51c65d3d44bbd01933c82126af441c77',
> + ''],
> + 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: 17ba15f011398a911aa9017c653a0760',
> + ''],
> + 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: 51c65d3d44bbd01933c82126af441c77',
> + 'Description-en: Foo app is great',
> + ' nice udeb',
> + '',
> + 'Package: foo-bin',
> + 'Description-md5: 17ba15f011398a911aa9017c653a0760',
> + '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: 17ba15f011398a911aa9017c653a0760',
> + '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)
>
> === 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 04:59:22 +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/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 04:59:22 +0000
> @@ -17,6 +17,7 @@
>
> from collections import defaultdict
> from datetime import datetime
> +import hashlib
> from operator import attrgetter
> import os
> import re
> @@ -62,6 +63,7 @@
> )
> from lp.services.database.sqlbase import SQLBase
> from lp.services.database.stormexpr import IsDistinctFrom
> +from lp.services.features import getFeatureFlag
> from lp.services.librarian.browser import ProxiedLibraryFileAlias
> from lp.services.librarian.model import (
> LibraryFileAlias,
> @@ -1005,16 +1007,23 @@
> bin_sha256 = bin_file.libraryfile.content.sha256
> bin_filepath = os.path.join(
> makePoolPath(spr.name, self.component.name), bin_filename)
> + bin_description_md5 = hashlib.md5(bpr.description).hexdigest()
As above, the hash needs to include bpr.summary, and have a properly formatted bpr.description.
> # 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))
> -
> + flag_enabled = getFeatureFlag("soyuz.ppa.separate_long_descriptions")
> + if (not self.distroarchseries.distroseries.include_long_descriptions
> + and flag_enabled):
> + bin_description = bpr.summary
> + else:
> + bin_description = '%s\n %s' % (
> + bpr.summary, '\n '.join(descr_lines))
The check for the two flags is done in four places now. Consider calculating whether we're using the new behaviour once, and passing it down into functions that need it. That'll make testing easier too.
> # Dealing with architecturespecific field.
> # Present 'all' in every archive index for architecture
> # independent binaries.
> @@ -1060,6 +1069,9 @@
> fields.append(
> 'Phased-Update-Percentage', self.phased_update_percentage)
> fields.append('Description', bin_description)
> + if (not self.distroarchseries.distroseries.include_long_descriptions
> + and flag_enabled):
> + fields.append('Description-md5', bin_description_md5)
> if bpr.user_defined_fields:
> fields.extend(bpr.user_defined_fields)
>
>
--
https://code.launchpad.net/~cjohnston/launchpad/1171047-ppa-trans-en/+merge/228978
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
References