Merge lp:~cjwatson/launchpad/snap-daily-builds-request-builds into lp:launchpad
- snap-daily-builds-request-builds
- Merge into devel
Proposed by
Colin Watson
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 18740 | ||||
Proposed branch: | lp:~cjwatson/launchpad/snap-daily-builds-request-builds | ||||
Merge into: | lp:launchpad | ||||
Prerequisite: | lp:~cjwatson/launchpad/git-ref-remote-blob | ||||
Diff against target: |
353 lines (+128/-54) 4 files modified
lib/lp/snappy/adapters/buildarch.py (+4/-4) lib/lp/snappy/interfaces/snap.py (+14/-1) lib/lp/snappy/model/snap.py (+28/-36) lib/lp/snappy/tests/test_snap.py (+82/-13) |
||||
To merge this branch: | bzr merge lp:~cjwatson/launchpad/snap-daily-builds-request-builds | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
William Grant | code | Approve | |
Review via email: mp+348187@code.launchpad.net |
Commit message
Make automatic builds of snaps honour build-on architectures.
Description of the change
To post a comment you must log in.
Revision history for this message
William Grant (wgrant) : | # |
review:
Approve
(code)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/lp/snappy/adapters/buildarch.py' |
2 | --- lib/lp/snappy/adapters/buildarch.py 2018-06-14 17:11:56 +0000 |
3 | +++ lib/lp/snappy/adapters/buildarch.py 2018-07-30 09:08:46 +0000 |
4 | @@ -122,12 +122,12 @@ |
5 | |
6 | |
7 | def determine_architectures_to_build(snapcraft_data, supported_arches): |
8 | - """Return a set of architectures to build based on snapcraft.yaml. |
9 | + """Return a list of architectures to build based on snapcraft.yaml. |
10 | |
11 | :param snapcraft_data: A parsed snapcraft.yaml. |
12 | :param supported_arches: An ordered list of all architecture tags that |
13 | we can create builds for. |
14 | - :return: a set of `SnapBuildInstance`s. |
15 | + :return: a list of `SnapBuildInstance`s. |
16 | """ |
17 | architectures_list = snapcraft_data.get("architectures") |
18 | |
19 | @@ -162,10 +162,10 @@ |
20 | if duplicates: |
21 | raise DuplicateBuildOnError(duplicates) |
22 | |
23 | - architectures_to_build = set() |
24 | + architectures_to_build = [] |
25 | for arch in architectures: |
26 | try: |
27 | - architectures_to_build.add( |
28 | + architectures_to_build.append( |
29 | SnapBuildInstance(arch, supported_arches)) |
30 | except UnsupportedBuildOnError: |
31 | # Snaps are allowed to declare that they build on architectures |
32 | |
33 | === modified file 'lib/lp/snappy/interfaces/snap.py' |
34 | --- lib/lp/snappy/interfaces/snap.py 2018-06-18 16:42:43 +0000 |
35 | +++ lib/lp/snappy/interfaces/snap.py 2018-07-30 09:08:46 +0000 |
36 | @@ -406,6 +406,7 @@ |
37 | """ |
38 | |
39 | def requestBuildsFromJob(requester, archive, pocket, channels=None, |
40 | + allow_failures=False, fetch_snapcraft_yaml=True, |
41 | logger=None): |
42 | """Synchronous part of `Snap.requestBuilds`. |
43 | |
44 | @@ -416,6 +417,13 @@ |
45 | :param pocket: The pocket that should be targeted. |
46 | :param channels: A dictionary mapping snap names to channels to use |
47 | for these builds. |
48 | + :param allow_failures: If True, log exceptions other than "already |
49 | + pending" from individual build requests; if False, raise them to |
50 | + the caller. |
51 | + :param fetch_snapcraft_yaml: If True, fetch snapcraft.yaml from the |
52 | + appropriate branch or repository and use it to decide which |
53 | + builds to request; if False, fall back to building for all |
54 | + supported architectures. |
55 | :param logger: An optional logger. |
56 | :return: A sequence of `ISnapBuild` instances. |
57 | """ |
58 | @@ -474,12 +482,17 @@ |
59 | @operation_returns_collection_of(Interface) |
60 | @export_write_operation() |
61 | @operation_for_version("devel") |
62 | - def requestAutoBuilds(allow_failures=False, logger=None): |
63 | + def requestAutoBuilds(allow_failures=False, fetch_snapcraft_yaml=False, |
64 | + logger=None): |
65 | """Create and return automatic builds for this snap package. |
66 | |
67 | :param allow_failures: If True, log exceptions other than "already |
68 | pending" from individual build requests; if False, raise them to |
69 | the caller. |
70 | + :param fetch_snapcraft_yaml: If True, fetch snapcraft.yaml from the |
71 | + appropriate branch or repository and use it to decide which |
72 | + builds to request; if False, fall back to building for all |
73 | + supported architectures. |
74 | :param logger: An optional logger. |
75 | :raises CannotRequestAutoBuilds: if no auto_build_archive or |
76 | auto_build_pocket is set. |
77 | |
78 | === modified file 'lib/lp/snappy/model/snap.py' |
79 | --- lib/lp/snappy/model/snap.py 2018-07-13 09:17:40 +0000 |
80 | +++ lib/lp/snappy/model/snap.py 2018-07-30 09:08:46 +0000 |
81 | @@ -552,18 +552,22 @@ |
82 | return self.getBuildRequest(job.job_id) |
83 | |
84 | def requestBuildsFromJob(self, requester, archive, pocket, channels=None, |
85 | + allow_failures=False, fetch_snapcraft_yaml=True, |
86 | logger=None): |
87 | """See `ISnap`.""" |
88 | - try: |
89 | - snapcraft_data = removeSecurityProxy( |
90 | - getUtility(ISnapSet).getSnapcraftYaml(self)) |
91 | - except CannotFetchSnapcraftYaml as e: |
92 | - if not e.unsupported_remote: |
93 | - raise |
94 | - # The only reason we can't fetch the file is because we don't |
95 | - # support fetching from this repository's host. In this case |
96 | - # the best thing is to fall back to building for all supported |
97 | - # architectures. |
98 | + if fetch_snapcraft_yaml: |
99 | + try: |
100 | + snapcraft_data = removeSecurityProxy( |
101 | + getUtility(ISnapSet).getSnapcraftYaml(self)) |
102 | + except CannotFetchSnapcraftYaml as e: |
103 | + if not e.unsupported_remote: |
104 | + raise |
105 | + # The only reason we can't fetch the file is because we |
106 | + # don't support fetching from this repository's host. In |
107 | + # this case the best thing is to fall back to building for |
108 | + # all supported architectures. |
109 | + snapcraft_data = {} |
110 | + else: |
111 | snapcraft_data = {} |
112 | # Sort by Processor.id for determinism. This is chosen to be the |
113 | # same order as in BinaryPackageBuildSet.createForSource, to |
114 | @@ -589,11 +593,18 @@ |
115 | builds.append(build) |
116 | except SnapBuildAlreadyPending as e: |
117 | pass |
118 | + except Exception as e: |
119 | + if not allow_failures: |
120 | + raise |
121 | + elif logger is not None: |
122 | + logger.exception( |
123 | + " - %s/%s/%s: %s", |
124 | + self.owner.name, self.name, arch, e) |
125 | return builds |
126 | |
127 | - def requestAutoBuilds(self, allow_failures=False, logger=None): |
128 | + def requestAutoBuilds(self, allow_failures=False, |
129 | + fetch_snapcraft_yaml=False, logger=None): |
130 | """See `ISnap`.""" |
131 | - builds = [] |
132 | if self.auto_build_archive is None: |
133 | raise CannotRequestAutoBuilds("auto_build_archive") |
134 | if self.auto_build_pocket is None: |
135 | @@ -603,29 +614,10 @@ |
136 | logger.debug( |
137 | "Scheduling builds of snap package %s/%s", |
138 | self.owner.name, self.name) |
139 | - for arch in self.getAllowedArchitectures(): |
140 | - try: |
141 | - build = self.requestBuild( |
142 | - self.owner, self.auto_build_archive, arch, |
143 | - self.auto_build_pocket, self.auto_build_channels) |
144 | - if logger is not None: |
145 | - logger.debug( |
146 | - " - %s/%s/%s: Build requested.", |
147 | - self.owner.name, self.name, arch.architecturetag) |
148 | - builds.append(build) |
149 | - except SnapBuildAlreadyPending as e: |
150 | - if logger is not None: |
151 | - logger.warning( |
152 | - " - %s/%s/%s: %s", |
153 | - self.owner.name, self.name, arch.architecturetag, e) |
154 | - except Exception as e: |
155 | - if not allow_failures: |
156 | - raise |
157 | - elif logger is not None: |
158 | - logger.exception( |
159 | - " - %s/%s/%s: %s", |
160 | - self.owner.name, self.name, arch.architecturetag, e) |
161 | - return builds |
162 | + return self.requestBuildsFromJob( |
163 | + self.owner, self.auto_build_archive, self.auto_build_pocket, |
164 | + channels=self.auto_build_channels, allow_failures=allow_failures, |
165 | + fetch_snapcraft_yaml=fetch_snapcraft_yaml, logger=logger) |
166 | |
167 | def getBuildRequest(self, job_id): |
168 | """See `ISnap`.""" |
169 | @@ -1113,7 +1105,7 @@ |
170 | builds = [] |
171 | for snap in snaps: |
172 | builds.extend(snap.requestAutoBuilds( |
173 | - allow_failures=True, logger=logger)) |
174 | + allow_failures=True, fetch_snapcraft_yaml=True, logger=logger)) |
175 | return builds |
176 | |
177 | def detachFromBranch(self, branch): |
178 | |
179 | === modified file 'lib/lp/snappy/tests/test_snap.py' |
180 | --- lib/lp/snappy/tests/test_snap.py 2018-06-18 16:42:43 +0000 |
181 | +++ lib/lp/snappy/tests/test_snap.py 2018-07-30 09:08:46 +0000 |
182 | @@ -1285,9 +1285,10 @@ |
183 | def makeAutoBuildableSnap(self, **kwargs): |
184 | processor = self.factory.makeProcessor(supports_virtualized=True) |
185 | das = self.makeBuildableDistroArchSeries(processor=processor) |
186 | + [git_ref] = self.factory.makeGitRefs() |
187 | snap = self.factory.makeSnap( |
188 | distroseries=das.distroseries, processors=[das.processor], |
189 | - auto_build=True, **kwargs) |
190 | + git_ref=git_ref, auto_build=True, **kwargs) |
191 | return das, snap |
192 | |
193 | def test_makeAutoBuilds(self): |
194 | @@ -1296,7 +1297,11 @@ |
195 | self.assertEqual([], getUtility(ISnapSet).makeAutoBuilds()) |
196 | das, snap = self.makeAutoBuildableSnap(is_stale=True) |
197 | logger = BufferLogger() |
198 | - [build] = getUtility(ISnapSet).makeAutoBuilds(logger=logger) |
199 | + with GitHostingFixture(blob=dedent("""\ |
200 | + architectures: |
201 | + - build-on: %s |
202 | + """) % das.architecturetag): |
203 | + [build] = getUtility(ISnapSet).makeAutoBuilds(logger=logger) |
204 | self.assertThat(build, MatchesStructure.byEquality( |
205 | requester=snap.owner, snap=snap, distro_arch_series=das, |
206 | status=BuildStatus.NEEDSBUILD, |
207 | @@ -1319,7 +1324,11 @@ |
208 | requester=snap.owner, snap=snap, archive=snap.auto_build_archive, |
209 | distroarchseries=das) |
210 | logger = BufferLogger() |
211 | - builds = getUtility(ISnapSet).makeAutoBuilds(logger=logger) |
212 | + with GitHostingFixture(blob=dedent("""\ |
213 | + architectures: |
214 | + - build-on: %s |
215 | + """) % das.architecturetag): |
216 | + builds = getUtility(ISnapSet).makeAutoBuilds(logger=logger) |
217 | self.assertEqual([], builds) |
218 | self.assertEqual([], logger.getLogBuffer().splitlines()) |
219 | |
220 | @@ -1340,7 +1349,13 @@ |
221 | channels={"snapcraft": "stable"}) |
222 | |
223 | logger = BufferLogger() |
224 | - builds = getUtility(ISnapSet).makeAutoBuilds(logger=logger) |
225 | + snapcraft_yaml = dedent("""\ |
226 | + architectures: |
227 | + - build-on: %s |
228 | + - build-on: %s |
229 | + """) % (das1.architecturetag, das2.architecturetag) |
230 | + with GitHostingFixture(blob=snapcraft_yaml): |
231 | + builds = getUtility(ISnapSet).makeAutoBuilds(logger=logger) |
232 | self.assertThat(builds, MatchesSetwise( |
233 | MatchesStructure( |
234 | requester=Equals(snap1.owner), snap=Equals(snap1), |
235 | @@ -1369,14 +1384,19 @@ |
236 | removeSecurityProxy(snap).is_stale = True |
237 | IStore(snap).flush() |
238 | logger = BufferLogger() |
239 | - builds = getUtility(ISnapSet).makeAutoBuilds(logger=logger) |
240 | + with GitHostingFixture(blob=snapcraft_yaml): |
241 | + builds = getUtility(ISnapSet).makeAutoBuilds(logger=logger) |
242 | self.assertEqual([], builds) |
243 | self.assertEqual([], logger.getLogBuffer().splitlines()) |
244 | |
245 | def test_makeAutoBuilds_skips_non_stale_snaps(self): |
246 | # ISnapSet.makeAutoBuilds skips snap packages that are not stale. |
247 | das, snap = self.makeAutoBuildableSnap(is_stale=False) |
248 | - self.assertEqual([], getUtility(ISnapSet).makeAutoBuilds()) |
249 | + with GitHostingFixture(blob=dedent("""\ |
250 | + architectures: |
251 | + - build-on: %s |
252 | + """) % das.architecturetag): |
253 | + self.assertEqual([], getUtility(ISnapSet).makeAutoBuilds()) |
254 | |
255 | def test_makeAutoBuilds_skips_pending(self): |
256 | # ISnapSet.makeAutoBuilds skips snap packages that already have |
257 | @@ -1389,14 +1409,15 @@ |
258 | distroarchseries=das, |
259 | date_created=datetime.now(pytz.UTC) - timedelta(days=1)) |
260 | logger = BufferLogger() |
261 | - builds = getUtility(ISnapSet).makeAutoBuilds(logger=logger) |
262 | + with GitHostingFixture(blob=dedent("""\ |
263 | + architectures: |
264 | + - build-on: %s |
265 | + """) % das.architecturetag): |
266 | + builds = getUtility(ISnapSet).makeAutoBuilds(logger=logger) |
267 | self.assertEqual([], builds) |
268 | expected_log_entries = [ |
269 | "DEBUG Scheduling builds of snap package %s/%s" % ( |
270 | snap.owner.name, snap.name), |
271 | - "WARNING - %s/%s/%s: An identical build of this snap package " |
272 | - "is already pending." % ( |
273 | - snap.owner.name, snap.name, das.architecturetag), |
274 | ] |
275 | self.assertEqual( |
276 | expected_log_entries, logger.getLogBuffer().splitlines()) |
277 | @@ -1410,7 +1431,11 @@ |
278 | distroarchseries=das, |
279 | date_created=datetime.now(pytz.UTC) - timedelta(days=1), |
280 | status=BuildStatus.FULLYBUILT, duration=timedelta(minutes=1)) |
281 | - builds = getUtility(ISnapSet).makeAutoBuilds() |
282 | + with GitHostingFixture(blob=dedent("""\ |
283 | + architectures: |
284 | + - build-on: %s |
285 | + """) % das.architecturetag): |
286 | + builds = getUtility(ISnapSet).makeAutoBuilds() |
287 | self.assertEqual(1, len(builds)) |
288 | |
289 | def test_makeAutoBuilds_with_older_and_newer_builds(self): |
290 | @@ -1423,7 +1448,11 @@ |
291 | archive=snap.auto_build_archive, distroarchseries=das, |
292 | date_created=datetime.now(pytz.UTC) - timediff, |
293 | status=BuildStatus.FULLYBUILT, duration=timedelta(minutes=1)) |
294 | - self.assertEqual([], getUtility(ISnapSet).makeAutoBuilds()) |
295 | + with GitHostingFixture(blob=dedent("""\ |
296 | + architectures: |
297 | + - build-on: %s |
298 | + """) % das.architecturetag): |
299 | + self.assertEqual([], getUtility(ISnapSet).makeAutoBuilds()) |
300 | |
301 | def test_makeAutoBuilds_with_recent_build_from_different_archive(self): |
302 | # If a snap package has been built recently but from an archive |
303 | @@ -1434,9 +1463,49 @@ |
304 | requester=snap.owner, snap=snap, distroarchseries=das, |
305 | date_created=datetime.now(pytz.UTC) - timedelta(minutes=30), |
306 | status=BuildStatus.FULLYBUILT, duration=timedelta(minutes=1)) |
307 | - builds = getUtility(ISnapSet).makeAutoBuilds() |
308 | + with GitHostingFixture(blob=dedent("""\ |
309 | + architectures: |
310 | + - build-on: %s |
311 | + """) % das.architecturetag): |
312 | + builds = getUtility(ISnapSet).makeAutoBuilds() |
313 | self.assertEqual(1, len(builds)) |
314 | |
315 | + def test_makeAutoBuilds_honours_explicit_architectures(self): |
316 | + # ISnapSet.makeAutoBuilds honours an explicit list of architectures |
317 | + # in the snap's snapcraft.yaml. |
318 | + das1, snap = self.makeAutoBuildableSnap(is_stale=True) |
319 | + das2 = self.makeBuildableDistroArchSeries( |
320 | + distroseries=das1.distroseries, |
321 | + processor=self.factory.makeProcessor(supports_virtualized=True)) |
322 | + das3 = self.makeBuildableDistroArchSeries( |
323 | + distroseries=das1.distroseries, |
324 | + processor=self.factory.makeProcessor(supports_virtualized=True)) |
325 | + snap.setProcessors([das1.processor, das2.processor, das3.processor]) |
326 | + logger = BufferLogger() |
327 | + with GitHostingFixture(blob=dedent("""\ |
328 | + architectures: |
329 | + - build-on: %s |
330 | + - build-on: %s |
331 | + """) % (das1.architecturetag, das2.architecturetag)): |
332 | + builds = getUtility(ISnapSet).makeAutoBuilds(logger=logger) |
333 | + self.assertThat(set(builds), MatchesSetwise(*[ |
334 | + MatchesStructure.byEquality( |
335 | + requester=snap.owner, snap=snap, distro_arch_series=das, |
336 | + status=BuildStatus.NEEDSBUILD) |
337 | + for das in (das1, das2) |
338 | + ])) |
339 | + expected_log_entries = [ |
340 | + "DEBUG Scheduling builds of snap package %s/%s" % ( |
341 | + snap.owner.name, snap.name), |
342 | + "DEBUG - %s/%s/%s: Build requested." % ( |
343 | + snap.owner.name, snap.name, das1.architecturetag), |
344 | + "DEBUG - %s/%s/%s: Build requested." % ( |
345 | + snap.owner.name, snap.name, das2.architecturetag), |
346 | + ] |
347 | + self.assertEqual( |
348 | + expected_log_entries, logger.getLogBuffer().splitlines()) |
349 | + self.assertFalse(snap.is_stale) |
350 | + |
351 | def test_detachFromBranch(self): |
352 | # ISnapSet.detachFromBranch clears the given Bazaar branch from all |
353 | # Snaps. |