Launchpad itself

Merge lp:~wgrant/launchpad/bug-655614-disabled-arch-indices into lp:launchpad/db-devel

  1. bug-655614-disabled-arch-indices
  2. Merge into db-devel
Proposed by William Grant
Status: Merged
Approved by: Julian Edwards
Approved revision: no longer in the source branch.
Merged at revision: 9875
Proposed branch: lp:~wgrant/launchpad/bug-655614-disabled-arch-indices
Merge into: lp:launchpad/db-devel
Diff against target: 211 lines (+100/-23)
4 files modified
lib/lp/archivepublisher/config.py (+3/-2)
lib/lp/archivepublisher/ftparchive.py (+7/-0)
lib/lp/archivepublisher/publishing.py (+10/-7)
lib/lp/archivepublisher/tests/test_publisher.py (+80/-14)
To merge this branch: bzr merge lp:~wgrant/launchpad/bug-655614-disabled-arch-indices
Reviewer Review Type Date Requested Status
Julian Edwards (community) Approve
Review via email: mp+37849@code.launchpad.net

Commit message

Don't publish indices for disabled architectures.

Description of the change

Both the publication methods (NMAF and a-f) still publish indices for disabled architectures every time the pocket is dirtied. This is pointless pollution, as these indices will be empty.

This branch fixes both publishers to not publish any indices for disabled archs, and also to exclude them from the series Release file. It adds tests to confirm that neither publisher writes disabled indices, or even creates directories for them.

The revolting hack in lib/lp/archivepublisher/ftparchive.py is largely stolen from another part of the publisher, and can't be avoided without a substantial refactor.

To post a comment you must log in.
Revision history for this message
Julian Edwards (julian-edwards) :
review: Approve
Revision history for this message
Julian Edwards (julian-edwards) wrote :

I am testing and landing. If it works on on dogfood I'll cherry pick too, thanks for writing this William.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/archivepublisher/config.py'
2--- lib/lp/archivepublisher/config.py 2010-08-23 16:51:11 +0000
3+++ lib/lp/archivepublisher/config.py 2010-10-07 14:15:57 +0000
4@@ -117,8 +117,9 @@
5 }
6
7 for dar in dr.architectures:
8- config_segment["archtags"].append(
9- dar.architecturetag.encode('utf-8'))
10+ if dar.enabled:
11+ config_segment["archtags"].append(
12+ dar.architecturetag.encode('utf-8'))
13
14 if dr.lucilleconfig:
15 strio = StringIO(dr.lucilleconfig.encode('utf-8'))
16
17=== modified file 'lib/lp/archivepublisher/ftparchive.py'
18--- lib/lp/archivepublisher/ftparchive.py 2010-08-24 15:29:01 +0000
19+++ lib/lp/archivepublisher/ftparchive.py 2010-10-07 14:15:57 +0000
20@@ -699,6 +699,13 @@
21 self.log.debug("Writing file lists for %s" % suite)
22 for component, architectures in components.items():
23 for architecture, file_names in architectures.items():
24+ # XXX wgrant 2010-10-06: There must be a better place to
25+ # do this.
26+ series, pocket = (
27+ self.distro.getDistroSeriesAndPocket(suite))
28+ if (architecture != 'source' and
29+ not series.getDistroArchSeries(architecture[7:]).enabled):
30+ continue
31 self.writeFileList(architecture, file_names,
32 suite, component)
33
34
35=== modified file 'lib/lp/archivepublisher/publishing.py'
36--- lib/lp/archivepublisher/publishing.py 2010-08-24 15:29:01 +0000
37+++ lib/lp/archivepublisher/publishing.py 2010-10-07 14:15:57 +0000
38@@ -353,8 +353,16 @@
39 source_index.close()
40
41 for arch in distroseries.architectures:
42+ if not arch.enabled:
43+ continue
44+
45 arch_path = 'binary-%s' % arch.architecturetag
46
47+ # XXX wgrant 2010-10-06 bug=655690: Using FTPArchiveHandler
48+ # for NMAF is wrong.
49+ self.apt_handler.requestReleaseFile(
50+ suite_name, component.name, arch_path)
51+
52 self.log.debug("Generating Packages for %s" % arch_path)
53
54 package_index_root = os.path.join(
55@@ -386,15 +394,10 @@
56 package_index.close()
57 di_index.close()
58
59- # Inject static requests for Release files into self.apt_handler
60- # in a way which works for NoMoreAptFtpArchive without changing
61- # much of the rest of the code, specially D_writeReleaseFiles.
62+ # XXX wgrant 2010-10-06 bug=655690: Using FTPArchiveHandler
63+ # is wrong here too.
64 self.apt_handler.requestReleaseFile(
65 suite_name, component.name, 'source')
66- for arch in distroseries.architectures:
67- arch_name = "binary-" + arch.architecturetag
68- self.apt_handler.requestReleaseFile(
69- suite_name, component.name, arch_name)
70
71 def cannotModifySuite(self, distroseries, pocket):
72 """Return True if the distroseries is stable and pocket is release."""
73
74=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
75--- lib/lp/archivepublisher/tests/test_publisher.py 2010-08-31 11:11:09 +0000
76+++ lib/lp/archivepublisher/tests/test_publisher.py 2010-10-07 14:15:57 +0000
77@@ -22,7 +22,10 @@
78 from canonical.launchpad.ftests.keys_for_tests import gpgkeysdir
79 from canonical.launchpad.interfaces.gpghandler import IGPGHandler
80 from canonical.zeca.ftests.harness import ZecaTestSetup
81-from lp.archivepublisher.config import getPubConfig
82+from lp.archivepublisher.config import (
83+ Config,
84+ getPubConfig,
85+ )
86 from lp.archivepublisher.diskpool import DiskPool
87 from lp.archivepublisher.interfaces.archivesigningkey import (
88 IArchiveSigningKey,
89@@ -767,7 +770,7 @@
90 self.checkDirtyPockets(publisher, expected=allowed_suites)
91
92 def assertReleaseFileRequested(self, publisher, suite_name,
93- component_name, arch_name):
94+ component_name, archs):
95 """Assert the given context will have it's Release file regenerated.
96
97 Check if a request for the given context is correctly stored in:
98@@ -779,12 +782,10 @@
99 self.assertTrue(
100 component_name in suite,
101 'Component %s/%s not requested' % (suite_name, component_name))
102- self.assertTrue(
103- arch_name in suite[component_name],
104- 'Arch %s/%s/%s not requested' % (
105- suite_name, component_name, arch_name))
106+ self.assertEquals(archs, suite[component_name])
107
108- def checkAllRequestedReleaseFiles(self, publisher):
109+ def checkAllRequestedReleaseFiles(self, publisher,
110+ architecturetags=('hppa', 'i386')):
111 """Check if all expected Release files are going to be regenerated.
112
113 'source', 'binary-i386' and 'binary-hppa' Release Files should be
114@@ -795,19 +796,17 @@
115 self.assertEqual(available_components,
116 ['main', 'multiverse', 'restricted', 'universe'])
117
118- available_archs = ['binary-%s' % a.architecturetag
119- for a in self.breezy_autotest.architectures]
120- self.assertEqual(available_archs, ['binary-hppa', 'binary-i386'])
121+ available_archs = ['binary-%s' % arch for arch in architecturetags]
122
123 # XXX cprov 20071210: Include the artificial component 'source' as a
124 # location to check for generated indexes. Ideally we should
125 # encapsulate this task in publishing.py and this common method
126 # in tests as well.
127- dists = ['source'] + available_archs
128+ all_archs = set(available_archs)
129+ all_archs.add('source')
130 for component in available_components:
131- for dist in dists:
132- self.assertReleaseFileRequested(
133- publisher, 'breezy-autotest', component, dist)
134+ self.assertReleaseFileRequested(
135+ publisher, 'breezy-autotest', component, all_archs)
136
137 def _getReleaseFileOrigin(self, contents):
138 origin_header = 'Origin: '
139@@ -1082,6 +1081,73 @@
140 # The Label: field should be set to the archive displayname
141 self.assertEqual(release_contents[1], 'Label: Partner archive')
142
143+ def assertIndicesForArchitectures(self, publisher, present, absent):
144+ """Assert that the correct set of archs has indices.
145+
146+ Checks that the architecture tags in 'present' have Packages and
147+ Release files and are in the series' Release file, and confirms
148+ that those in 'absent' are not.
149+ """
150+
151+ self.checkAllRequestedReleaseFiles(
152+ publisher, architecturetags=present)
153+
154+ arch_template = os.path.join(
155+ publisher._config.distsroot, 'breezy-autotest/main/binary-%s')
156+ release_template = os.path.join(arch_template, 'Release')
157+ packages_template = os.path.join(arch_template, 'Packages')
158+ release_content = open(os.path.join(
159+ publisher._config.distsroot,
160+ 'breezy-autotest/Release')).read()
161+
162+ for arch in present:
163+ self.assertTrue(os.path.exists(arch_template % arch))
164+ self.assertTrue(os.path.exists(release_template % arch))
165+ self.assertTrue(os.path.exists(packages_template % arch))
166+ self.assertTrue(arch in release_content)
167+
168+ for arch in absent:
169+ self.assertFalse(os.path.exists(arch_template % arch))
170+ self.assertFalse(arch in release_content)
171+
172+ def testNativeNoIndicesForDisabledArchitectures(self):
173+ """Test that no indices are created for disabled archs."""
174+ self.getPubBinaries()
175+
176+ ds = self.ubuntutest.getSeries('breezy-autotest')
177+ ds.getDistroArchSeries('i386').enabled = False
178+ self.config = Config(self.ubuntutest)
179+
180+ publisher = Publisher(
181+ self.logger, self.config, self.disk_pool,
182+ self.ubuntutest.main_archive)
183+
184+ publisher.A_publish(False)
185+ publisher.C_writeIndexes(False)
186+ publisher.D_writeReleaseFiles(False)
187+
188+ self.assertIndicesForArchitectures(
189+ publisher, present=['hppa'], absent=['i386'])
190+
191+ def testAptFtparchiveNoIndicesForDisabledArchitectures(self):
192+ """Test that no indices are created for disabled archs."""
193+ self.getPubBinaries()
194+
195+ ds = self.ubuntutest.getSeries('breezy-autotest')
196+ ds.getDistroArchSeries('i386').enabled = False
197+ self.config = Config(self.ubuntutest)
198+
199+ publisher = Publisher(
200+ self.logger, self.config, self.disk_pool,
201+ self.ubuntutest.main_archive)
202+
203+ publisher.A_publish(False)
204+ publisher.C_doFTPArchive(False)
205+ publisher.D_writeReleaseFiles(False)
206+
207+ self.assertIndicesForArchitectures(
208+ publisher, present=['hppa'], absent=['i386'])
209+
210 def testWorldAndGroupReadablePackagesAndSources(self):
211 """Test Packages.gz and Sources.gz files are world and group readable.
212

Subscribers

People subscribed via source and target branches

to status/vote changes: