Merge ~xnox/launchpad:drop-i18n-index into launchpad:master

Proposed by Dimitri John Ledkov
Status: Merged
Approved by: Jürgen Gmach
Approved revision: ceb6ea5fb240c753ef54a72d26d3f6893bcfd9a8
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~xnox/launchpad:drop-i18n-index
Merge into: launchpad:master
Diff against target: 185 lines (+67/-4)
9 files modified
lib/lp/archivepublisher/publishing.py (+5/-4)
lib/lp/archivepublisher/tests/test_publisher.py (+23/-0)
lib/lp/registry/configure.zcml (+1/-0)
lib/lp/registry/interfaces/distroseries.py (+11/-0)
lib/lp/registry/model/distroseries.py (+10/-0)
lib/lp/registry/stories/webservice/xx-distroseries.rst (+1/-0)
lib/lp/registry/tests/test_distroseries.py (+11/-0)
lib/lp/soyuz/scripts/initialize_distroseries.py (+3/-0)
lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py (+2/-0)
Reviewer Review Type Date Requested Status
Jürgen Gmach Approve
Colin Watson (community) Approve
Julian Andres Klode (community) Approve
Ubuntu Package Archive Administrators Pending
Review via email: mp+453586@code.launchpad.net

Commit message

Add a new option to turn off i18n/Index file publishing

It is not signed, it contains SHA-1 hashes which apt doesn't trust anymore, and it is believed to be unused by apt, as it finds the underlying translation files from the the InRelease file.

Intention is to turn publishing i18n index file off during 24.04 release development

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) wrote :

Tools like debmirror used to use this. Have we confirmed that other common tools handling apt repositories no longer do?

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Doing it via toggle is one way to find out.

Also since this index is sha1 based, it no longer is secure. Any mirroring must use InRelease file and strong hashes.

And given strong hashes of these files exist in the InRelease, it is wasteful to repeat them.

Also, sort of hoping to maybe have this merged and sample published on qastaging or some such, and publish a PPA there to allow checking against a real thing.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

http://ftp.debian.org/debian/dists/bookworm/main/i18n/

Debian does not publish Index.

Reading debmirror code it can get them either via Release file, or via i18n/Index and I think tries both always.

And since it works for Debian, surely it works for us to drop it too.

Revision history for this message
Julian Andres Klode (juliank) wrote :

I concur with xnox

review: Approve
Revision history for this message
Colin Watson (cjwatson) :
review: Approve
~xnox/launchpad:drop-i18n-index updated
ceb6ea5... by Dimitri John Ledkov

Fix typo

Signed-off-by: Dimitri John Ledkov <email address hidden>

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

fix typo

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Launchpad team ping.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

clinton: The only thing missing that I want to see is +1 from an engineer whose problem it will be to land and support this in future.

The problem to land this would be me. The intention is to land this code into production launchpad and change the toggle off on noble. If everything is fine (publication works, mirroring works, no clients loose i18n access) - keep it off. Then once all Ubuntu releases are published without i18n index this code path can be ripped out.

Support in the future - I think this publisher code path was redundant since introduction, but also have not been touched much, if at all. Ultimately Ubuntu Archive team are the ones that care that Ubuntu Archive is published, and how it is published.

This is a dry run for a much more user-visible change to add toggles to turn off obsolete checksums generation & publication. And toggle those off. As was advised around discussions of how to land https://code.launchpad.net/~xnox/launchpad/+git/launchpad/+merge/452749

Revision history for this message
Jürgen Gmach (jugmac00) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/archivepublisher/publishing.py b/lib/lp/archivepublisher/publishing.py
2index 622a07f..165c030 100644
3--- a/lib/lp/archivepublisher/publishing.py
4+++ b/lib/lp/archivepublisher/publishing.py
5@@ -1682,11 +1682,12 @@ class Publisher:
6 # Schedule i18n files for inclusion in the Release file.
7 all_series_files.add(os.path.join(i18n_subpath, i18n_file))
8
9- with open(os.path.join(i18n_dir, "Index"), "wb") as f:
10- i18n_index.dump(f, "utf-8")
11+ if distroseries.publish_i18n_index:
12+ with open(os.path.join(i18n_dir, "Index"), "wb") as f:
13+ i18n_index.dump(f, "utf-8")
14
15- # Schedule this for inclusion in the Release file.
16- all_series_files.add(os.path.join(component, "i18n", "Index"))
17+ # Schedule this for inclusion in the Release file.
18+ all_series_files.add(os.path.join(component, "i18n", "Index"))
19
20 def _readIndexFileHashes(
21 self, suite, file_name, subpath=None, real_file_name=None
22diff --git a/lib/lp/archivepublisher/tests/test_publisher.py b/lib/lp/archivepublisher/tests/test_publisher.py
23index 354ca4a..a785065 100644
24--- a/lib/lp/archivepublisher/tests/test_publisher.py
25+++ b/lib/lp/archivepublisher/tests/test_publisher.py
26@@ -2847,6 +2847,29 @@ class TestPublisher(TestPublisherBase):
27
28 self.assertFalse(os.path.exists(os.path.join(i18n_root, "Index")))
29
30+ def testWriteSuiteI18nPublishI18nFalse(self):
31+ """i18n/Index is not generated when publish_i18n_index is False."""
32+ publisher = Publisher(
33+ self.logger,
34+ self.config,
35+ self.disk_pool,
36+ self.ubuntutest.main_archive,
37+ )
38+ i18n_root = os.path.join(
39+ self.config.distsroot, "breezy-autotest", "main", "i18n"
40+ )
41+
42+ self.ubuntu["breezy-autotest"].publish_i18n_index = False
43+
44+ publisher._writeSuiteI18n(
45+ self.ubuntutest["breezy-autotest"],
46+ PackagePublishingPocket.RELEASE,
47+ "main",
48+ set(),
49+ )
50+
51+ self.assertFalse(os.path.exists(os.path.join(i18n_root, "Index")))
52+
53 def testReadIndexFileHashesCompression(self):
54 """Test compressed file handling in _readIndexFileHashes."""
55 publisher = Publisher(
56diff --git a/lib/lp/registry/configure.zcml b/lib/lp/registry/configure.zcml
57index 5dda9d1..3cf36a8 100644
58--- a/lib/lp/registry/configure.zcml
59+++ b/lib/lp/registry/configure.zcml
60@@ -332,6 +332,7 @@
61 include_long_descriptions
62 index_compressors
63 publish_by_hash
64+ publish_i18n_index
65 advertise_by_hash
66 strict_supported_component_dependencies
67 inherit_overrides_from_parents"/>
68diff --git a/lib/lp/registry/interfaces/distroseries.py b/lib/lp/registry/interfaces/distroseries.py
69index bcb5a24..594c442 100644
70--- a/lib/lp/registry/interfaces/distroseries.py
71+++ b/lib/lp/registry/interfaces/distroseries.py
72@@ -585,6 +585,17 @@ class IDistroSeriesPublic(
73 as_of="devel",
74 )
75
76+ publish_i18n_index = exported(
77+ Bool(
78+ title=_("Publish I18n index"),
79+ required=True,
80+ description=_(
81+ """
82+ Publish archive i18n/Index file, which is believed to be unused."""
83+ ),
84+ )
85+ )
86+
87 inherit_overrides_from_parents = Bool(
88 title=_("Inherit overrides from parents"),
89 readonly=False,
90diff --git a/lib/lp/registry/model/distroseries.py b/lib/lp/registry/model/distroseries.py
91index 2e19dfb..f67348b 100644
92--- a/lib/lp/registry/model/distroseries.py
93+++ b/lib/lp/registry/model/distroseries.py
94@@ -285,6 +285,7 @@ class DistroSeries(
95 "publish_by_hash": False,
96 "advertise_by_hash": False,
97 "strict_supported_component_dependencies": True,
98+ "publish_i18n_index": True,
99 }
100
101 @property
102@@ -969,6 +970,15 @@ class DistroSeries(
103 "strict_supported_component_dependencies"
104 ] = value
105
106+ @property
107+ def publish_i18n_index(self):
108+ return self.publishing_options.get("publish_i18n_index", True)
109+
110+ @publish_i18n_index.setter
111+ def publish_i18n_index(self, value):
112+ assert isinstance(value, bool)
113+ self.publishing_options["publish_i18n_index"] = value
114+
115 def _customizeSearchParams(self, search_params):
116 """Customize `search_params` for this distribution series."""
117 search_params.setDistroSeries(self)
118diff --git a/lib/lp/registry/stories/webservice/xx-distroseries.rst b/lib/lp/registry/stories/webservice/xx-distroseries.rst
119index 4fe4c9b..51a089a 100644
120--- a/lib/lp/registry/stories/webservice/xx-distroseries.rst
121+++ b/lib/lp/registry/stories/webservice/xx-distroseries.rst
122@@ -94,6 +94,7 @@ For distroseries we publish a subset of its attributes.
123 parent_series_link: 'http://.../ubuntu/warty'
124 proposed_not_automatic: False
125 publish_by_hash: False
126+ publish_i18n_index: True
127 registrant_link: 'http://.../~mark'
128 resource_type_link: ...
129 self_link: 'http://.../ubuntu/hoary'
130diff --git a/lib/lp/registry/tests/test_distroseries.py b/lib/lp/registry/tests/test_distroseries.py
131index c6dd967..eafd665 100644
132--- a/lib/lp/registry/tests/test_distroseries.py
133+++ b/lib/lp/registry/tests/test_distroseries.py
134@@ -448,6 +448,17 @@ class TestDistroSeries(TestCaseWithFactory):
135 ]
136 )
137
138+ def test_publish_i18n_index(self):
139+ distroseries = self.factory.makeDistroSeries()
140+ self.assertTrue(distroseries.publish_i18n_index)
141+ with admin_logged_in():
142+ distroseries.publish_i18n_index = False
143+ self.assertFalse(distroseries.publish_i18n_index)
144+ naked_distroseries = removeSecurityProxy(distroseries)
145+ self.assertFalse(
146+ naked_distroseries.publishing_options["publish_i18n_index"]
147+ )
148+
149
150 class TestDistroSeriesPackaging(TestCaseWithFactory):
151 layer = DatabaseFunctionalLayer
152diff --git a/lib/lp/soyuz/scripts/initialize_distroseries.py b/lib/lp/soyuz/scripts/initialize_distroseries.py
153index 6a35b71..4097a10 100644
154--- a/lib/lp/soyuz/scripts/initialize_distroseries.py
155+++ b/lib/lp/soyuz/scripts/initialize_distroseries.py
156@@ -454,6 +454,9 @@ class InitializeDistroSeries:
157 parent.strict_supported_component_dependencies
158 for parent in self.derivation_parents
159 )
160+ self.distroseries.publish_i18n_index = any(
161+ parent.publish_i18n_index for parent in self.derivation_parents
162+ )
163
164 def _copy_architectures(self):
165 log.info("Copying distroarchseries from parents.")
166diff --git a/lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py b/lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py
167index aee00cb..c0d6b57 100644
168--- a/lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py
169+++ b/lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py
170@@ -96,6 +96,7 @@ class InitializationHelperTestCase(TestCaseWithFactory):
171 parent.publish_by_hash = True
172 parent.advertise_by_hash = True
173 parent.strict_supported_component_dependencies = False
174+ parent.publish_i18n_index = False
175 self._populate_parent(parent, parent_das, packages, pocket)
176 return parent, parent_das
177
178@@ -842,6 +843,7 @@ class TestInitializeDistroSeries(InitializationHelperTestCase):
179 self.assertTrue(child.publish_by_hash)
180 self.assertTrue(child.advertise_by_hash)
181 self.assertFalse(child.strict_supported_component_dependencies)
182+ self.assertFalse(child.publish_i18n_index)
183
184 def test_initialize(self):
185 # Test a full initialize with no errors.

Subscribers

People subscribed via source and target branches

to status/vote changes: