Merge lp:~wgrant/launchpad/xbia-refactor into lp:launchpad
- xbia-refactor
- Merge into devel
Proposed by
William Grant
Status: | Merged |
---|---|
Merged at revision: | 17337 |
Proposed branch: | lp:~wgrant/launchpad/xbia-refactor |
Merge into: | lp:launchpad |
Diff against target: |
379 lines (+154/-139) 4 files modified
lib/lp/soyuz/adapters/buildarch.py (+20/-47) lib/lp/soyuz/adapters/tests/test_buildarch.py (+42/-49) lib/lp/soyuz/model/binarypackagebuild.py (+25/-11) lib/lp/soyuz/tests/test_build_set.py (+67/-32) |
To merge this branch: | bzr merge lp:~wgrant/launchpad/xbia-refactor |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Colin Watson (community) | Approve | ||
Review via email: mp+249295@code.launchpad.net |
Commit message
Refactor build creation so buildarch and its tests can avoid the DB.
Description of the change
Refactor build creation so buildarch and its tests can avoid the DB. Makes things a bit less painfully slow and paves the way for the implementation of Build-Indep-
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/lp/soyuz/adapters/buildarch.py' |
2 | --- lib/lp/soyuz/adapters/buildarch.py 2014-11-06 05:01:05 +0000 |
3 | +++ lib/lp/soyuz/adapters/buildarch.py 2015-02-11 10:10:26 +0000 |
4 | @@ -8,7 +8,6 @@ |
5 | ] |
6 | |
7 | |
8 | -from operator import attrgetter |
9 | import os |
10 | import subprocess |
11 | |
12 | @@ -29,63 +28,37 @@ |
13 | return self._matches[(arch, wildcard)] |
14 | |
15 | def findAllMatches(self, arches, wildcards): |
16 | - return [arch for arch in arches for wildcard in wildcards |
17 | - if self.match(arch, wildcard)] |
18 | + return list(sorted(set( |
19 | + arch for arch in arches for wildcard in wildcards |
20 | + if self.match(arch, wildcard)))) |
21 | |
22 | |
23 | dpkg_architecture = DpkgArchitectureCache() |
24 | |
25 | |
26 | -def determine_architectures_to_build(hintlist, archive, distroseries, |
27 | - legal_archseries, need_arch_indep): |
28 | - """Return a list of architectures for which this publication should build. |
29 | - |
30 | - This function answers the question: given a list of architectures and |
31 | - an archive, what architectures should we build it for? It takes a set of |
32 | - legal distroarchseries and the distribution series for which we are |
33 | - building. |
34 | - |
35 | - For PPA publications we only consider architectures supported by PPA |
36 | - subsystem (`DistroArchSeries`.supports_virtualized flag). |
37 | - |
38 | - :param: hintlist: A string of the architectures this source package |
39 | +def determine_architectures_to_build(hintlist, valid_archs, |
40 | + nominated_arch_indep, need_arch_indep): |
41 | + """Return a set of architectures to build. |
42 | + |
43 | + :param hintlist: A string of the architectures this source package |
44 | specifies it builds for. |
45 | - :param: archive: The `IArchive` we are building into. |
46 | - :param: distroseries: the context `DistroSeries`. |
47 | - :param: legal_archseries: a list of all initialized `DistroArchSeries` |
48 | - to be considered. |
49 | - :return: a list of `DistroArchSeries` for which the source publication in |
50 | + :param valid_archs: a list of all architecture tags that we can |
51 | + create builds for. |
52 | + :param nominated_arch_indep: a preferred architecture tag for |
53 | + architecture-independent builds. May be None. |
54 | + :return: a set of architecture tags for which the source publication in |
55 | question should be built. |
56 | """ |
57 | - # The 'PPA supported' flag only applies to virtualized archives |
58 | - if archive.require_virtualized: |
59 | - legal_archseries = [ |
60 | - arch for arch in legal_archseries if arch.supports_virtualized] |
61 | - # Cope with no virtualization support at all. It usually happens when |
62 | - # a distroseries is created and initialized, by default no |
63 | - # architecture supports its. Distro-team might take some time to |
64 | - # decide which architecture will be allowed for PPAs and queue-builder |
65 | - # will continue to work meanwhile. |
66 | - if not legal_archseries: |
67 | - return [] |
68 | - |
69 | - legal_arch_tags = set( |
70 | - arch.architecturetag for arch in legal_archseries if arch.enabled) |
71 | - |
72 | hint_archs = set(hintlist.split()) |
73 | - build_tags = set(dpkg_architecture.findAllMatches( |
74 | - legal_arch_tags, hint_archs)) |
75 | + build_archs = set( |
76 | + dpkg_architecture.findAllMatches(valid_archs, hint_archs)) |
77 | |
78 | # 'all' is only used as a last resort, to create an arch-indep build |
79 | # where no builds would otherwise exist. |
80 | - if need_arch_indep and len(build_tags) == 0 and 'all' in hint_archs: |
81 | - nominated_arch = distroseries.nominatedarchindep |
82 | - if nominated_arch in legal_archseries: |
83 | - build_tags = set([nominated_arch.architecturetag]) |
84 | + if need_arch_indep and len(build_archs) == 0 and 'all' in hint_archs: |
85 | + if nominated_arch_indep in valid_archs: |
86 | + return set([nominated_arch_indep]) |
87 | else: |
88 | - build_tags = set() |
89 | + return set() |
90 | |
91 | - sorted_archseries = sorted( |
92 | - legal_archseries, key=attrgetter('architecturetag')) |
93 | - return [arch for arch in sorted_archseries |
94 | - if arch.architecturetag in build_tags] |
95 | + return build_archs |
96 | |
97 | === modified file 'lib/lp/soyuz/adapters/tests/test_buildarch.py' |
98 | --- lib/lp/soyuz/adapters/tests/test_buildarch.py 2014-11-06 05:01:05 +0000 |
99 | +++ lib/lp/soyuz/adapters/tests/test_buildarch.py 2015-02-11 10:10:26 +0000 |
100 | @@ -3,48 +3,52 @@ |
101 | |
102 | __metaclass__ = type |
103 | |
104 | -from lp.soyuz.adapters.buildarch import determine_architectures_to_build |
105 | -from lp.soyuz.tests.test_publishing import SoyuzTestPublisher |
106 | -from lp.testing import TestCaseWithFactory |
107 | -from lp.testing.layers import LaunchpadZopelessLayer |
108 | - |
109 | - |
110 | -class TestDetermineArchitecturesToBuild(TestCaseWithFactory): |
111 | +from lp.soyuz.adapters.buildarch import ( |
112 | + determine_architectures_to_build, |
113 | + DpkgArchitectureCache, |
114 | + ) |
115 | +from lp.testing import TestCase |
116 | + |
117 | + |
118 | +class TestDpkgArchitectureCache(TestCase): |
119 | + |
120 | + def test_multiple(self): |
121 | + self.assertContentEqual( |
122 | + ['amd64', 'armhf'], |
123 | + DpkgArchitectureCache().findAllMatches( |
124 | + ['amd64', 'i386', 'armhf'], ['amd64', 'armhf'])) |
125 | + |
126 | + def test_any(self): |
127 | + self.assertContentEqual( |
128 | + ['amd64', 'i386', 'kfreebsd-amd64'], |
129 | + DpkgArchitectureCache().findAllMatches( |
130 | + ['amd64', 'i386', 'kfreebsd-amd64'], ['any'])) |
131 | + |
132 | + def test_all(self): |
133 | + self.assertContentEqual( |
134 | + [], |
135 | + DpkgArchitectureCache().findAllMatches( |
136 | + ['amd64', 'i386', 'kfreebsd-amd64'], ['all'])) |
137 | + |
138 | + def test_partial_wildcards(self): |
139 | + self.assertContentEqual( |
140 | + ['amd64', 'i386', 'kfreebsd-amd64'], |
141 | + DpkgArchitectureCache().findAllMatches( |
142 | + ['amd64', 'i386', 'kfreebsd-amd64', 'kfreebsd-i386'], |
143 | + ['linux-any', 'any-amd64'])) |
144 | + |
145 | + |
146 | +class TestDetermineArchitecturesToBuild(TestCase): |
147 | """Test that determine_architectures_to_build correctly interprets hints. |
148 | """ |
149 | |
150 | - layer = LaunchpadZopelessLayer |
151 | - |
152 | - def setUp(self): |
153 | - super(TestDetermineArchitecturesToBuild, self).setUp() |
154 | - self.publisher = SoyuzTestPublisher() |
155 | - self.publisher.prepareBreezyAutotest() |
156 | - armel = self.factory.makeProcessor('armel', 'armel', 'armel') |
157 | - self.publisher.breezy_autotest.newArch( |
158 | - 'armel', armel, False, self.publisher.person) |
159 | - self.publisher.addFakeChroots() |
160 | - |
161 | - def assertArchitecturesToBuild(self, expected_arch_tags, pub, |
162 | - allowed_arch_tags=None): |
163 | + def assertArchsForHint(self, hint_string, expected_arch_tags, |
164 | + allowed_arch_tags=None): |
165 | if allowed_arch_tags is None: |
166 | - allowed_archs = self.publisher.breezy_autotest.architectures |
167 | - else: |
168 | - allowed_archs = [ |
169 | - arch for arch in self.publisher.breezy_autotest.architectures |
170 | - if arch.architecturetag in allowed_arch_tags] |
171 | - architectures = determine_architectures_to_build( |
172 | - pub.sourcepackagerelease.architecturehintlist, pub.archive, |
173 | - self.publisher.breezy_autotest, allowed_archs, True) |
174 | - self.assertContentEqual( |
175 | - expected_arch_tags, [a.architecturetag for a in architectures]) |
176 | - |
177 | - def assertArchsForHint(self, hint_string, expected_arch_tags, |
178 | - allowed_arch_tags=None, sourcename=None): |
179 | - """Assert that the given hint resolves to the expected archtags.""" |
180 | - pub = self.publisher.getPubSource( |
181 | - sourcename=sourcename, architecturehintlist=hint_string) |
182 | - self.assertArchitecturesToBuild( |
183 | - expected_arch_tags, pub, allowed_arch_tags=allowed_arch_tags) |
184 | + allowed_arch_tags = ['armel', 'hppa', 'i386'] |
185 | + arch_tags = determine_architectures_to_build( |
186 | + hint_string, allowed_arch_tags, 'i386', True) |
187 | + self.assertContentEqual(expected_arch_tags, arch_tags) |
188 | |
189 | def test_single_architecture(self): |
190 | # A hint string with a single arch resolves to just that arch. |
191 | @@ -112,17 +116,6 @@ |
192 | # it anyway. |
193 | self.assertArchsForHint('any-any', ['armel', 'hppa', 'i386']) |
194 | |
195 | - def test_disabled_architectures_omitted(self): |
196 | - # Disabled architectures are not buildable, so are excluded. |
197 | - self.publisher.breezy_autotest['hppa'].enabled = False |
198 | - self.assertArchsForHint('any', ['armel', 'i386']) |
199 | - |
200 | - def test_virtualized_archives_have_only_virtualized_archs(self): |
201 | - # For archives which must build on virtual builders, only |
202 | - # virtual archs are returned. |
203 | - self.publisher.breezy_autotest.main_archive.require_virtualized = True |
204 | - self.assertArchsForHint('any', ['i386']) |
205 | - |
206 | def test_no_all_builds_when_nominatedarchindep_not_permitted(self): |
207 | # Some archives (eg. armel rebuilds) don't want arch-indep |
208 | # builds. If the nominatedarchindep architecture (normally |
209 | |
210 | === modified file 'lib/lp/soyuz/model/binarypackagebuild.py' |
211 | --- lib/lp/soyuz/model/binarypackagebuild.py 2014-11-07 12:05:37 +0000 |
212 | +++ lib/lp/soyuz/model/binarypackagebuild.py 2015-02-11 10:10:26 +0000 |
213 | @@ -1385,9 +1385,16 @@ |
214 | """ |
215 | # Return all distroarches with unrestricted processors or with |
216 | # processors the archive is explicitly associated with. |
217 | - return [distroarch for distroarch in available_archs |
218 | - if not distroarch.processor.restricted or |
219 | - distroarch.processor in archive.enabled_restricted_processors] |
220 | + return [ |
221 | + das for das in available_archs |
222 | + if ( |
223 | + das.enabled |
224 | + and ( |
225 | + not das.processor.restricted |
226 | + or das.processor in archive.enabled_restricted_processors) |
227 | + and ( |
228 | + das.supports_virtualized |
229 | + or not archive.require_virtualized))] |
230 | |
231 | def createForSource(self, sourcepackagerelease, archive, distroseries, |
232 | pocket, architectures_available=None, logger=None): |
233 | @@ -1421,14 +1428,21 @@ |
234 | # For an architecturehintlist of just 'all', this will |
235 | # be the current nominatedarchindep if need_arch_indep, |
236 | # otherwise nothing. |
237 | - if architectures_available is None: |
238 | - architectures_available = list( |
239 | - distroseries.buildable_architectures) |
240 | - architectures_available = self._getAllowedArchitectures( |
241 | - archive, architectures_available) |
242 | - candidate_architectures = determine_architectures_to_build( |
243 | - sourcepackagerelease.architecturehintlist, archive, distroseries, |
244 | - architectures_available, need_arch_indep) |
245 | + valid_archs = list( |
246 | + architectures_available or distroseries.buildable_architectures) |
247 | + valid_archs = self._getAllowedArchitectures(archive, valid_archs) |
248 | + nominated_arch_indep_tag = ( |
249 | + distroseries.nominatedarchindep.architecturetag |
250 | + if distroseries.nominatedarchindep else None) |
251 | + |
252 | + # Filter the valid archs against the hint list. |
253 | + candidate_arch_tags = determine_architectures_to_build( |
254 | + sourcepackagerelease.architecturehintlist, |
255 | + [das.architecturetag for das in valid_archs], |
256 | + nominated_arch_indep_tag, need_arch_indep) |
257 | + candidate_architectures = set( |
258 | + das for das in valid_archs |
259 | + if das.architecturetag in candidate_arch_tags) |
260 | |
261 | # Filter out any architectures for which we earlier found sufficient |
262 | # builds. |
263 | |
264 | === modified file 'lib/lp/soyuz/tests/test_build_set.py' |
265 | --- lib/lp/soyuz/tests/test_build_set.py 2014-11-06 05:20:01 +0000 |
266 | +++ lib/lp/soyuz/tests/test_build_set.py 2015-02-11 10:10:26 +0000 |
267 | @@ -236,6 +236,73 @@ |
268 | self.builds[1].distro_arch_series)) |
269 | |
270 | |
271 | +class TestGetAllowedArchitectures(TestCaseWithFactory): |
272 | + """Tests for _getAllowedArchitectures.""" |
273 | + |
274 | + layer = ZopelessDatabaseLayer |
275 | + |
276 | + def setUp(self): |
277 | + super(TestGetAllowedArchitectures, self).setUp() |
278 | + self.avr = self.factory.makeProcessor(name="avr2001") |
279 | + self.sparc = self.factory.makeProcessor(name="sparc64") |
280 | + self.distroseries = self.factory.makeDistroSeries() |
281 | + for name, arch in (('avr', self.avr), ('sparc', self.sparc)): |
282 | + self.factory.makeDistroArchSeries( |
283 | + architecturetag=name, processor=arch, |
284 | + distroseries=self.distroseries, supports_virtualized=True) |
285 | + self.archive = self.factory.makeArchive( |
286 | + distribution=self.distroseries.distribution) |
287 | + |
288 | + def test_normal(self): |
289 | + self.assertContentEqual( |
290 | + [self.distroseries['sparc'], self.distroseries['avr']], |
291 | + BinaryPackageBuildSet()._getAllowedArchitectures( |
292 | + self.archive, self.distroseries.architectures)) |
293 | + |
294 | + def test_restricted(self): |
295 | + # Restricted architectures aren't returned by default. |
296 | + self.avr.restricted = True |
297 | + self.assertContentEqual( |
298 | + [self.distroseries['sparc']], |
299 | + BinaryPackageBuildSet()._getAllowedArchitectures( |
300 | + self.archive, self.distroseries.architectures)) |
301 | + |
302 | + def test_restricted_override(self): |
303 | + # Restricted architectures are returned if allowed by the archive. |
304 | + self.avr.restricted = True |
305 | + getUtility(IArchiveArchSet).new(self.archive, self.avr) |
306 | + self.assertContentEqual( |
307 | + [self.distroseries['sparc'], self.distroseries['avr']], |
308 | + BinaryPackageBuildSet()._getAllowedArchitectures( |
309 | + self.archive, self.distroseries.architectures)) |
310 | + |
311 | + def test_disabled_architectures_omitted(self): |
312 | + # Disabled architectures are not buildable, so are excluded. |
313 | + self.distroseries['sparc'].enabled = False |
314 | + self.assertContentEqual( |
315 | + [self.distroseries['avr']], |
316 | + BinaryPackageBuildSet()._getAllowedArchitectures( |
317 | + self.archive, self.distroseries.architectures)) |
318 | + |
319 | + def test_virt_archives_have_only_virt_archs(self): |
320 | + # For archives which must build on virtual builders, only |
321 | + # virtual archs are returned. |
322 | + self.distroseries['sparc'].supports_virtualized = False |
323 | + self.assertContentEqual( |
324 | + [self.distroseries['avr']], |
325 | + BinaryPackageBuildSet()._getAllowedArchitectures( |
326 | + self.archive, self.distroseries.architectures)) |
327 | + |
328 | + def test_nonvirt_archives_have_only_all_archs(self): |
329 | + # Non-virtual archives can build on all unrestricted architectures. |
330 | + self.distroseries['sparc'].supports_virtualized = False |
331 | + self.archive.require_virtualized = False |
332 | + self.assertContentEqual( |
333 | + [self.distroseries['sparc'], self.distroseries['avr']], |
334 | + BinaryPackageBuildSet()._getAllowedArchitectures( |
335 | + self.archive, self.distroseries.architectures)) |
336 | + |
337 | + |
338 | class BuildRecordCreationTests(TestNativePublishingBase): |
339 | """Test the creation of build records.""" |
340 | |
341 | @@ -302,38 +369,6 @@ |
342 | del success_map[build.distro_arch_series.architecturetag] |
343 | self.assertContentEqual([], success_map) |
344 | |
345 | - def test__getAllowedArchitectures_restricted(self): |
346 | - """Test _getAllowedArchitectures doesn't return unrestricted |
347 | - archs. |
348 | - |
349 | - For a normal archive, only unrestricted architectures should |
350 | - be used. |
351 | - """ |
352 | - self.avr.restricted = True |
353 | - available_archs = [ |
354 | - self.distroseries['sparc'], self.distroseries['avr']] |
355 | - pubrec = self.getPubSource(architecturehintlist='any') |
356 | - self.assertEqual( |
357 | - [self.distroseries['sparc']], |
358 | - BinaryPackageBuildSet()._getAllowedArchitectures( |
359 | - pubrec.archive, available_archs)) |
360 | - |
361 | - def test__getAllowedArchitectures_restricted_override(self): |
362 | - """Test _getAllowedArchitectures honors overrides of restricted archs. |
363 | - |
364 | - Restricted architectures should only be allowed if there is |
365 | - an explicit ArchiveArch association with the archive. |
366 | - """ |
367 | - self.avr.restricted = True |
368 | - available_archs = [ |
369 | - self.distroseries['sparc'], self.distroseries['avr']] |
370 | - getUtility(IArchiveArchSet).new(self.archive, self.avr) |
371 | - pubrec = self.getPubSource(architecturehintlist='any') |
372 | - self.assertEqual( |
373 | - [self.distroseries['sparc'], self.distroseries['avr']], |
374 | - BinaryPackageBuildSet()._getAllowedArchitectures( |
375 | - pubrec.archive, available_archs)) |
376 | - |
377 | def test_createForSource_restricts_any(self): |
378 | """createForSource() should limit builds targeted at 'any' |
379 | architecture to those allowed for the archive. |
determine_ architectures_ to_build is now unsorted, but the consequences of that are fixed up by https:/ /code.launchpad .net/~wgrant/ launchpad/ xbia/+merge/ 249296.