Merge lp:~cjwatson/launchpad/multiarch-translations into lp:launchpad

[Colin Watson]

[r=jtv][bug=845475] Set LongDescription "true" in apt.conf for series that want it.

(Resubmitted against devel now that multiarch-translations-schema has been rolled out to the database.)

Summary: https://blueprints.launchpad.net/ubuntu/+spec/foundations-o-multiarch-next-steps requires that some clients start downloading Packages files for multiple architectures on 'apt-get update'. As it stands, this will double users' bandwidth costs and time.

Proposed fix: Move the long descriptions from the Packages files (which are in practice common across architectures) out to the Translation-en file. apt-ftparchive already knows how to do this with a configuration option, and apt-get already knows to fetch the Translation-* files, so all we need is a small change to Launchpad to start doing this for Ubuntu Oneiric and beyond.

Pre-implementation notes: I discussed this with William Grant, and agreed that we could follow much the same path as he took recently with the NotAutomatic flag for backports: add a new column to DistroSeries and make the new behaviour depend on that, so that we don't change older series. He also suggested that I export DistroSeries.include_long_descriptions over the API so that we don't need SQL to change it for Oneiric. William has allocated me a db patch number.

Tests: 'testr run -- -t archivepublisher; testr run -- -t soyuz' should cover it. I adjusted lib/lp/archivepublisher/tests/test_ftparchive.py and its data to ensure that the correct apt.conf text is generated, and lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py to ensure that include_long_descriptions is copied across initialising a new distroseries.

QA: On a dogfood Soyuz instance, use the API to set include_long_descriptions to False on a development distroseries of Ubuntu. Perform a publisher run (either making a fake upload, or using the publish-distro.py -A option to force careful apt generation). Make sure that long descriptions (everything after the first line) are written to Translation-en files in the output dists/ tree rather than to Packages files. Set up a client-side apt instance to talk to this, run 'apt-get update', and make sure that 'apt-cache show some-package-name-in-the-archive' still shows a sensible long description.

lint: Nothing introduced by my branch as far as I can see.

[Stuart Bishop (stub)]

DB patch looks fine. Please add a short description of the new column to database/schema/comments.sql ('Split long descriptions out of Packages into Translations-en' is fine per the Interface).

review: Approve (db)

[Colin Watson (cjwatson)]

comments.sql change done. Per a conversation with William, I renamed
the column to include_long_descriptions (as I'd got the sense of the
underlying apt.conf variable wrong, and wanted to keep the boolean
values the same way round), and adjusted the description accordingly; I
hope that doesn't invalidate the DB review.

For William: I checked with Michael Vogt, and apparently apt will fetch
Translation-en files in the LANG=C case, so I don't think we need to
worry about Translation-C files.

I also noticed that apt-ftparchive won't create the i18n directories
itself, so I added a conditional safe_mkdir call to
generateConfigForPocket.

I've asked Michael to prepare an apt backport with sufficient
LongDescription support, as lucid-updates doesn't have everything we
need.

[Henning Eggers (henninge)]

Looks good. Thank you very much. ;-)

review: Approve

[Colin Watson (cjwatson)]

Deployment: There are separate apt and python-apt changes working their way through lucid-proposed at the moment, which should land in a day or two. After that we'll need to get a patched apt plus modified meta-lp-deps into the Launchpad PPA, and then it will be possible to deploy this.

=== modified file 'lib/lp/archivepublisher/model/ftparchive.py'
--- lib/lp/archivepublisher/model/ftparchive.py	2011-08-03 15:06:39 +0000
+++ lib/lp/archivepublisher/model/ftparchive.py	2011-09-09 15:15:53 +0000
@@ -96,6 +96,7 @@
     Packages::Extensions "%(EXTENSIONS)s";
     BinCacheDB "packages-%(CACHEINSERT)s$(ARCH).db";
     Contents " ";
+    LongDescription "%(LONGDESCRIPTION)s";
 }
 
 """
@@ -802,7 +803,11 @@
                          "EXTENSIONS": ".deb",
                          "CACHEINSERT": "",
                          "DISTS": os.path.basename(self._config.distsroot),
-                         "HIDEEXTRA": ""})
+                         "HIDEEXTRA": "",
+                         "LONGDESCRIPTION":
+                             "true" if distroseries.include_long_descriptions
+                                    else "false",
+                         })
 
         if archs and suite in self._di_release_components:
             for component in self._di_release_components[suite]:
@@ -817,6 +822,7 @@
                     "CACHEINSERT": "debian-installer-",
                     "DISTS": os.path.basename(self._config.distsroot),
                     "HIDEEXTRA": "// ",
+                    "LONGDESCRIPTION": "true",
                     })
 
         # XXX: 2006-08-24 kiko: Why do we do this directory creation here?
@@ -831,5 +837,7 @@
             for base_path in base_paths:
                 if "debian-installer" not in base_path:
                     safe_mkdir(os.path.join(base_path, "source"))
+                    if not distroseries.include_long_descriptions:
+                        safe_mkdir(os.path.join(base_path, "i18n"))
                 for arch in archs:
                     safe_mkdir(os.path.join(base_path, "binary-"+arch))

=== modified file 'lib/lp/archivepublisher/tests/apt-data/apt.conf'
--- lib/lp/archivepublisher/tests/apt-data/apt.conf	2010-10-15 05:36:31 +0000
+++ lib/lp/archivepublisher/tests/apt-data/apt.conf	2011-09-09 15:15:53 +0000
@@ -34,6 +34,7 @@
     Packages::Extensions ".deb";
     BinCacheDB "packages-$(ARCH).db";
     Contents " ";
+    LongDescription "true";
 }
 
 
@@ -49,6 +50,7 @@
     Packages::Extensions ".udeb";
     BinCacheDB "packages-debian-installer-$(ARCH).db";
     Contents " ";
+    LongDescription "true";
 }
 
 
@@ -64,6 +66,7 @@
     Packages::Extensions ".udeb";
     BinCacheDB "packages-debian-installer-$(ARCH).db";
     Contents " ";
+    LongDescription "true";
 }
 
 
@@ -79,6 +82,7 @@
     Packages::Extensions ".udeb";
     BinCacheDB "packages-debian-installer-$(ARCH).db";
     Contents " ";
+    LongDescription "true";
 }
 
 
@@ -94,6 +98,7 @@
     Packages::Extensions ".udeb";
     BinCacheDB "packages-debian-installer-$(ARCH).db";
     Contents " ";
+    LongDescription "true";
 }
 
 
@@ -109,6 +114,7 @@
     Packages::Extensions ".deb";
     BinCacheDB "packages-$(ARCH).db";
     Contents " ";
+    LongDescription "true";
 }
 
 
@@ -124,6 +130,7 @@
     Packages::Extensions ".deb";
     BinCacheDB "packages-$(ARCH).db";
     Contents " ";
+    LongDescription "true";
 }
 
 
@@ -139,6 +146,7 @@
     Packages::Extensions ".deb";
     BinCacheDB "packages-$(ARCH).db";
     Contents " ";
+    LongDescription "true";
 }
 
 
@@ -154,6 +162,7 @@
     Packages::Extensions ".deb";
     BinCacheDB "packages-$(ARCH).db";
     Contents " ";
+    LongDescription "true";
 }
 
 
@@ -169,6 +178,7 @@
     Packages::Extensions ".deb";
     BinCacheDB "packages-$(ARCH).db";
     Contents " ";
+    LongDescription "false";
 }
 
 
@@ -184,6 +194,7 @@
     Packages::Extensions ".deb";
     BinCacheDB "packages-$(ARCH).db";
     Contents " ";
+    LongDescription "false";
 }
 
 
@@ -199,6 +210,7 @@
     Packages::Extensions ".deb";
     BinCacheDB "packages-$(ARCH).db";
     Contents " ";
+    LongDescription "false";
 }
 
 
@@ -214,6 +226,7 @@
     Packages::Extensions ".deb";
     BinCacheDB "packages-$(ARCH).db";
     Contents " ";
+    LongDescription "false";
 }
 
 
@@ -229,5 +242,6 @@
     Packages::Extensions ".deb";
     BinCacheDB "packages-$(ARCH).db";
     Contents " ";
+    LongDescription "false";
 }
 

=== modified file 'lib/lp/archivepublisher/tests/apt-data/apt_conf_single_empty_suite_test'
--- lib/lp/archivepublisher/tests/apt-data/apt_conf_single_empty_suite_test	2007-02-06 13:56:30 +0000
+++ lib/lp/archivepublisher/tests/apt-data/apt_conf_single_empty_suite_test	2011-09-09 15:15:53 +0000
@@ -34,5 +34,6 @@
     Packages::Extensions ".deb";
     BinCacheDB "packages-$(ARCH).db";
     Contents " ";
+    LongDescription "true";
 }
 

=== modified file 'lib/lp/archivepublisher/tests/test_ftparchive.py'
--- lib/lp/archivepublisher/tests/test_ftparchive.py	2011-02-25 11:54:05 +0000
+++ lib/lp/archivepublisher/tests/test_ftparchive.py	2011-09-09 15:15:53 +0000
@@ -286,6 +286,13 @@
         self._addRepositoryFile('main', 'foo', 'foo_1.dsc')
         self._addRepositoryFile('main', 'foo', 'foo_1_i386.deb')
 
+        # When include_long_descriptions is set, apt.conf has
+        # LongDescription "true" for that series.
+        hoary_test = self._distribution.getSeries('hoary-test')
+        self.assertTrue(hoary_test.include_long_descriptions)
+        breezy_autotest = self._distribution.getSeries('breezy-autotest')
+        breezy_autotest.include_long_descriptions = False
+
         # XXX cprov 2007-03-21: Relying on byte-to-byte configuration file
         # comparing is weak. We should improve this methodology to avoid
         # wasting time on test failures due to irrelevant format changes.

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2011-08-25 13:10:45 +0000
+++ lib/lp/registry/configure.zcml	2011-09-09 15:15:53 +0000
@@ -233,7 +233,8 @@
         <require
             permission="launchpad.Edit"
             set_attributes="displayname title summary description driver
-                            backports_not_automatic"/>
+                            backports_not_automatic
+                            include_long_descriptions"/>
 
         <!-- NB: check with SABDFL before modifying these, there is potential to
                                  make a huge mess if these get changed and Soyuz has to republish

=== modified file 'lib/lp/registry/interfaces/distroseries.py'
--- lib/lp/registry/interfaces/distroseries.py	2011-08-25 06:18:33 +0000
+++ lib/lp/registry/interfaces/distroseries.py	2011-09-09 15:15:53 +0000
@@ -360,6 +360,21 @@
             automatically upgrade within backports, but not into it.
             """))
 
+    include_long_descriptions = exported(
+        Bool(
+            title=_(
+                "Include long descriptions in Packages rather than in "
+                "Translation-en"),
+            default=True,
+            required=True,
+            description=_("""
+                If True, write long descriptions to the per-architecture
+                Packages files; if False, write them to a Translation-en
+                file common across architectures instead. Using a common
+                file reduces the bandwidth footprint of enabling multiarch
+                on clients, which requires downloading Packages files for
+                multiple architectures.""")))
+
     def priorReleasedSeries():
         """Prior series *by date* from the same distribution."""
 

=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2011-08-25 06:18:33 +0000
+++ lib/lp/registry/model/distroseries.py	2011-09-09 15:15:53 +0000
@@ -256,6 +256,7 @@
         notNull=False, default=None)
     language_pack_full_export_requested = BoolCol(notNull=True, default=False)
     backports_not_automatic = BoolCol(notNull=True, default=False)
+    include_long_descriptions = BoolCol(notNull=True, default=True)
 
     language_packs = SQLMultipleJoin(
         'LanguagePack', joinColumn='distroseries', orderBy='-date_exported')

=== modified file 'lib/lp/registry/stories/webservice/xx-distroseries.txt'
--- lib/lp/registry/stories/webservice/xx-distroseries.txt	2011-06-02 09:31:49 +0000
+++ lib/lp/registry/stories/webservice/xx-distroseries.txt	2011-09-09 15:15:53 +0000
@@ -74,6 +74,7 @@
     driver_link: None
     drivers_collection_link: u'http://.../ubuntu/hoary/drivers'
     fullseriesname: u'Ubuntu Hoary'
+    include_long_descriptions: True
     main_archive_link: u'http://.../ubuntu/+archive/primary'
     name: u'hoary'
     official_bug_tags: []

=== modified file 'lib/lp/soyuz/scripts/initialize_distroseries.py'
--- lib/lp/soyuz/scripts/initialize_distroseries.py	2011-09-06 12:10:33 +0000
+++ lib/lp/soyuz/scripts/initialize_distroseries.py	2011-09-09 15:15:53 +0000
@@ -331,6 +331,9 @@
         self.distroseries.backports_not_automatic = any(
             parent.backports_not_automatic
                 for parent in self.derivation_parents)
+        self.distroseries.include_long_descriptions = any(
+            parent.include_long_descriptions
+                for parent in self.derivation_parents)
 
     def _copy_architectures(self):
         das_filter = ' AND distroseries IN %s ' % (

=== modified file 'lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py'
--- lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py	2011-09-06 12:10:33 +0000
+++ lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py	2011-09-09 15:15:53 +0000
@@ -88,6 +88,7 @@
         if existing_format_selection is None:
             spfss_utility.add(parent, format_selection)
         parent.backports_not_automatic = True
+        parent.include_long_descriptions = False
         self._populate_parent(parent, parent_das, packages, pocket)
         return parent, parent_das
 
@@ -582,6 +583,7 @@
             SourcePackageFormat.FORMAT_1_0))
         # Other configuration bits are copied too.
         self.assertTrue(child.backports_not_automatic)
+        self.assertFalse(child.include_long_descriptions)
 
     def test_initialize(self):
         # Test a full initialize with no errors.