Colin Watson has proposed merging ~cjwatson/launchpad:snap-arch-all into launchpad:master.
Commit message: Handle "all" in snapcraft architectures declaration Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/437624 This has apparently been supported by snapcraft for quite some time, but we overlooked it while implementing `architectures` in Launchpad. Reported by James Henstridge. -- Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:snap-arch-all into launchpad:master.
diff --git a/lib/lp/snappy/adapters/buildarch.py b/lib/lp/snappy/adapters/buildarch.py index eb87a7d..d2dc8fd 100644 --- a/lib/lp/snappy/adapters/buildarch.py +++ b/lib/lp/snappy/adapters/buildarch.py @@ -39,6 +39,24 @@ class IncompatibleArchitecturesStyleError(SnapArchitecturesParserError): ) +class AllConflictInBuildForError(SnapArchitecturesParserError): + """Error for when `build-for` contains `all` and another architecture.""" + + def __init__(self): + super().__init__( + "'build-for' contains both 'all' and another architecture name" + ) + + +class AllConflictInBuildOnError(SnapArchitecturesParserError): + """Error for when `build-on` contains `all` and another architecture.""" + + def __init__(self): + super().__init__( + "'build-on' contains both 'all' and another architecture name" + ) + + class DuplicateBuildOnError(SnapArchitecturesParserError): """Error for when multiple `build-on`s include the same architecture.""" @@ -134,14 +152,21 @@ class SnapBuildInstance: :param supported_architectures: List of supported architectures, sorted by priority. """ + build_on = architecture.build_on + # "all" indicates that the architecture doesn't matter. Try to pick + # an appropriate architecture in this case. + # `Snap.requestBuildsFromJob` orders `supported_architectures` such + # that we can reasonably pick the first one if all else fails. + if "all" in build_on: + build_on = architecture.build_for + if "all" in build_on: + build_on = supported_architectures[0] try: self.architecture = next( - arch - for arch in supported_architectures - if arch in architecture.build_on + arch for arch in supported_architectures if arch in build_on ) except StopIteration: - raise UnsupportedBuildOnError(architecture.build_on) + raise UnsupportedBuildOnError(build_on) self.target_architectures = architecture.build_for self.required = architecture.build_error != "ignore" @@ -186,6 +211,12 @@ def determine_architectures_to_build( SnapArchitecture(build_on=a) for a in supported_arches ] + for arch in architectures: + if "all" in arch.build_on and len(arch.build_on) > 1: + raise AllConflictInBuildOnError() + if "all" in arch.build_for and len(arch.build_for) > 1: + raise AllConflictInBuildForError() + allow_duplicate_build_on = ( snap_base and snap_base.features.get(SnapBaseFeature.ALLOW_DUPLICATE_BUILD_ON) diff --git a/lib/lp/snappy/adapters/tests/test_buildarch.py b/lib/lp/snappy/adapters/tests/test_buildarch.py index 8a9ef2e..399ea76 100644 --- a/lib/lp/snappy/adapters/tests/test_buildarch.py +++ b/lib/lp/snappy/adapters/tests/test_buildarch.py @@ -5,6 +5,8 @@ from testscenarios import WithScenarios, load_tests_apply_scenarios from testtools.matchers import HasLength from lp.snappy.adapters.buildarch import ( + AllConflictInBuildForError, + AllConflictInBuildOnError, DuplicateBuildOnError, SnapArchitecture, SnapBuildInstance, @@ -154,6 +156,28 @@ class TestSnapBuildInstance(WithScenarios, TestCase): "expected_required": False, }, ), + ( + "build on all", + { + "architecture": SnapArchitecture(build_on=["all"]), + "supported_architectures": ["amd64", "i386", "armhf"], + "expected_architecture": "amd64", + "expected_target_architectures": ["all"], + "expected_required": True, + }, + ), + ( + "build on all, build for i386", + { + "architecture": SnapArchitecture( + build_on=["all"], build_for="i386" + ), + "supported_architectures": ["amd64", "i386", "armhf"], + "expected_architecture": "i386", + "expected_target_architectures": ["i386"], + "expected_required": True, + }, + ), ] def test_build_instance(self): @@ -397,6 +421,24 @@ class TestDetermineArchitecturesToBuild(WithScenarios, TestCaseWithFactory): }, ), ( + "build-on contains both all and another architecture", + { + "architectures": [{"build-on": ["all", "amd64"]}], + "supported_architectures": ["amd64"], + "expected_exception": AllConflictInBuildOnError, + }, + ), + ( + "build-for contains both all and another architecture", + { + "architectures": [ + {"build-on": "amd64", "build-for": ["amd64", "all"]} + ], + "supported_architectures": ["amd64"], + "expected_exception": AllConflictInBuildForError, + }, + ), + ( "multiple build-for for the same build-on", { "snap_base_features": {
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp