Merge ~cjwatson/launchpad:snap-arch-all into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 43f8ff2bb8deead319a60cccf6807821915002e3
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:snap-arch-all
Merge into: launchpad:master
Diff against target: 135 lines (+77/-4)
2 files modified
lib/lp/snappy/adapters/buildarch.py (+35/-4)
lib/lp/snappy/adapters/tests/test_buildarch.py (+42/-0)
Reviewer Review Type Date Requested Status
Jürgen Gmach Approve
Review via email: mp+437624@code.launchpad.net

Commit message

Handle "all" in snapcraft architectures declaration

Description of the change

This has apparently been supported by snapcraft for quite some time, but we overlooked it while implementing `architectures` in Launchpad. Reported by James Henstridge.

To post a comment you must log in.
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/snappy/adapters/buildarch.py b/lib/lp/snappy/adapters/buildarch.py
2index eb87a7d..d2dc8fd 100644
3--- a/lib/lp/snappy/adapters/buildarch.py
4+++ b/lib/lp/snappy/adapters/buildarch.py
5@@ -39,6 +39,24 @@ class IncompatibleArchitecturesStyleError(SnapArchitecturesParserError):
6 )
7
8
9+class AllConflictInBuildForError(SnapArchitecturesParserError):
10+ """Error for when `build-for` contains `all` and another architecture."""
11+
12+ def __init__(self):
13+ super().__init__(
14+ "'build-for' contains both 'all' and another architecture name"
15+ )
16+
17+
18+class AllConflictInBuildOnError(SnapArchitecturesParserError):
19+ """Error for when `build-on` contains `all` and another architecture."""
20+
21+ def __init__(self):
22+ super().__init__(
23+ "'build-on' contains both 'all' and another architecture name"
24+ )
25+
26+
27 class DuplicateBuildOnError(SnapArchitecturesParserError):
28 """Error for when multiple `build-on`s include the same architecture."""
29
30@@ -134,14 +152,21 @@ class SnapBuildInstance:
31 :param supported_architectures: List of supported architectures,
32 sorted by priority.
33 """
34+ build_on = architecture.build_on
35+ # "all" indicates that the architecture doesn't matter. Try to pick
36+ # an appropriate architecture in this case.
37+ # `Snap.requestBuildsFromJob` orders `supported_architectures` such
38+ # that we can reasonably pick the first one if all else fails.
39+ if "all" in build_on:
40+ build_on = architecture.build_for
41+ if "all" in build_on:
42+ build_on = supported_architectures[0]
43 try:
44 self.architecture = next(
45- arch
46- for arch in supported_architectures
47- if arch in architecture.build_on
48+ arch for arch in supported_architectures if arch in build_on
49 )
50 except StopIteration:
51- raise UnsupportedBuildOnError(architecture.build_on)
52+ raise UnsupportedBuildOnError(build_on)
53
54 self.target_architectures = architecture.build_for
55 self.required = architecture.build_error != "ignore"
56@@ -186,6 +211,12 @@ def determine_architectures_to_build(
57 SnapArchitecture(build_on=a) for a in supported_arches
58 ]
59
60+ for arch in architectures:
61+ if "all" in arch.build_on and len(arch.build_on) > 1:
62+ raise AllConflictInBuildOnError()
63+ if "all" in arch.build_for and len(arch.build_for) > 1:
64+ raise AllConflictInBuildForError()
65+
66 allow_duplicate_build_on = (
67 snap_base
68 and snap_base.features.get(SnapBaseFeature.ALLOW_DUPLICATE_BUILD_ON)
69diff --git a/lib/lp/snappy/adapters/tests/test_buildarch.py b/lib/lp/snappy/adapters/tests/test_buildarch.py
70index 8a9ef2e..399ea76 100644
71--- a/lib/lp/snappy/adapters/tests/test_buildarch.py
72+++ b/lib/lp/snappy/adapters/tests/test_buildarch.py
73@@ -5,6 +5,8 @@ from testscenarios import WithScenarios, load_tests_apply_scenarios
74 from testtools.matchers import HasLength
75
76 from lp.snappy.adapters.buildarch import (
77+ AllConflictInBuildForError,
78+ AllConflictInBuildOnError,
79 DuplicateBuildOnError,
80 SnapArchitecture,
81 SnapBuildInstance,
82@@ -154,6 +156,28 @@ class TestSnapBuildInstance(WithScenarios, TestCase):
83 "expected_required": False,
84 },
85 ),
86+ (
87+ "build on all",
88+ {
89+ "architecture": SnapArchitecture(build_on=["all"]),
90+ "supported_architectures": ["amd64", "i386", "armhf"],
91+ "expected_architecture": "amd64",
92+ "expected_target_architectures": ["all"],
93+ "expected_required": True,
94+ },
95+ ),
96+ (
97+ "build on all, build for i386",
98+ {
99+ "architecture": SnapArchitecture(
100+ build_on=["all"], build_for="i386"
101+ ),
102+ "supported_architectures": ["amd64", "i386", "armhf"],
103+ "expected_architecture": "i386",
104+ "expected_target_architectures": ["i386"],
105+ "expected_required": True,
106+ },
107+ ),
108 ]
109
110 def test_build_instance(self):
111@@ -397,6 +421,24 @@ class TestDetermineArchitecturesToBuild(WithScenarios, TestCaseWithFactory):
112 },
113 ),
114 (
115+ "build-on contains both all and another architecture",
116+ {
117+ "architectures": [{"build-on": ["all", "amd64"]}],
118+ "supported_architectures": ["amd64"],
119+ "expected_exception": AllConflictInBuildOnError,
120+ },
121+ ),
122+ (
123+ "build-for contains both all and another architecture",
124+ {
125+ "architectures": [
126+ {"build-on": "amd64", "build-for": ["amd64", "all"]}
127+ ],
128+ "supported_architectures": ["amd64"],
129+ "expected_exception": AllConflictInBuildForError,
130+ },
131+ ),
132+ (
133 "multiple build-for for the same build-on",
134 {
135 "snap_base_features": {

Subscribers

People subscribed via source and target branches

to status/vote changes: