Quentin Debhi has proposed merging ~ruinedyourlife/launchpad:feat-snapcraft-base-unification into launchpad:master.
Commit message: Platform unification for Snapcraft with unified syntax Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~ruinedyourlife/launchpad/+git/launchpad/+merge/468357 This change will help support core24 syntax as well as older syntaxes by adding support for the new `platform` key. -- Your team Launchpad code reviewers is requested to review the proposed merge of ~ruinedyourlife/launchpad:feat-snapcraft-base-unification into launchpad:master.
diff --git a/lib/lp/snappy/adapters/buildarch.py b/lib/lp/snappy/adapters/buildarch.py index df5acfd..0588e02 100644 --- a/lib/lp/snappy/adapters/buildarch.py +++ b/lib/lp/snappy/adapters/buildarch.py @@ -1,9 +1,7 @@ # Copyright 2018 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). -__all__ = [ - "determine_architectures_to_build", -] +__all__ = ["determine_architectures_to_build", "BadPropertyError"] from collections import Counter from typing import Any, Dict, List, Optional, Union @@ -29,6 +27,10 @@ class MissingPropertyError(SnapArchitecturesParserError): self.property = prop +class BadPropertyError(Exception): + """Error for when a YAML property is malformed in some way.""" + + class IncompatibleArchitecturesStyleError(SnapArchitecturesParserError): """Error for when architectures mix incompatible styles.""" @@ -188,20 +190,9 @@ def determine_architectures_to_build( architectures_list: Optional[List] = snapcraft_data.get("architectures") if architectures_list: - # First, determine what style we're parsing. Is it a list of - # strings or a list of dicts? - if all(isinstance(a, str) for a in architectures_list): - # If a list of strings (old style), then that's only a single - # item. - architectures = [SnapArchitecture(build_on=architectures_list)] - elif all(isinstance(arch, dict) for arch in architectures_list): - # If a list of dicts (new style), then that's multiple items. - architectures = [ - SnapArchitecture.from_dict(a) for a in architectures_list - ] - else: - # If a mix of both, bail. We can't reasonably handle it. - raise IncompatibleArchitecturesStyleError() + architectures = parse_architectures_list(architectures_list) + elif "platforms" in snapcraft_data: + architectures = parse_platforms(snapcraft_data, supported_arches) else: # If no architectures are specified, build one for each supported # architecture. @@ -209,28 +200,97 @@ 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() + validate_architectures(architectures) allow_duplicate_build_on = ( snap_base and snap_base.features.get(SnapBaseFeature.ALLOW_DUPLICATE_BUILD_ON) ) or False + if not allow_duplicate_build_on: - # Ensure that multiple `build-on` items don't include the same - # architecture; this is ambiguous and forbidden by snapcraft prior - # to core22. Checking this here means that we don't get duplicate - # supported_arch results below. - build_ons = Counter() - for arch in architectures: - build_ons.update(arch.build_on) - duplicates = {arch for arch, count in build_ons.items() if count > 1} - if duplicates: - raise DuplicateBuildOnError(duplicates) + check_for_duplicate_build_on(architectures) + + return build_architectures_list(architectures, supported_arches) + + +def parse_architectures_list( + architectures_list: List, +) -> List[SnapArchitecture]: + # First, determine what style we're parsing. Is it a list of + # strings or a list of dicts? + if all(isinstance(a, str) for a in architectures_list): + # If a list of strings (old style), then that's only a single + # item. + return [SnapArchitecture(build_on=architectures_list)] + elif all(isinstance(arch, dict) for arch in architectures_list): + # If a list of dicts (new style), then that's multiple items. + return [SnapArchitecture.from_dict(a) for a in architectures_list] + else: + # If a mix of both, bail. We can't reasonably handle it. + raise IncompatibleArchitecturesStyleError() + + +def parse_platforms( + snapcraft_data: Dict[str, Any], supported_arches: List[str] +) -> List[SnapArchitecture]: + architectures = [] + supported_arch_names = supported_arches + + for platform, configuration in snapcraft_data["platforms"].items(): + # The 'platforms' property and its values look like + # platforms: + # ubuntu-amd64: + # build-on: [amd64] + # build-for: [amd64] + # 'ubuntu-amd64' will be the value of 'platform' and its value dict + # containing the keys 'build-on', 'build-for' will be the value of + # 'configuration'. + if configuration: + build_on = configuration.get("build-on", [platform]) + build_for = configuration.get("build-for", build_on) + architectures.append( + SnapArchitecture( + build_on=build_on, + build_for=build_for, + ) + ) + elif platform in supported_arch_names: + architectures.append( + SnapArchitecture(build_on=[platform], build_for=[platform]) + ) + else: + base = snapcraft_data.get("base", "unknown") + raise BadPropertyError( + f"'{platform}' is not a supported platform " f"for '{base}'." + ) + return architectures + + +def validate_architectures(architectures: List[SnapArchitecture]): + 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() + + +def check_for_duplicate_build_on(architectures: List[SnapArchitecture]): + # Ensure that multiple `build-on` items don't include the same + # architecture; this is ambiguous and forbidden by snapcraft prior + # to core22. Checking this here means that we don't get duplicate + # supported_arch results below. + build_ons = Counter() + for arch in architectures: + build_ons.update(arch.build_on) + duplicates = {arch for arch, count in build_ons.items() if count > 1} + if duplicates: + raise DuplicateBuildOnError(duplicates) + + +def build_architectures_list( + architectures: List[SnapArchitecture], supported_arches: List[str] +) -> List[SnapBuildInstance]: architectures_to_build = [] for arch in architectures: try: diff --git a/lib/lp/snappy/adapters/tests/test_buildarch.py b/lib/lp/snappy/adapters/tests/test_buildarch.py index a5ae901..1383ae2 100644 --- a/lib/lp/snappy/adapters/tests/test_buildarch.py +++ b/lib/lp/snappy/adapters/tests/test_buildarch.py @@ -7,6 +7,7 @@ from testtools.matchers import HasLength from lp.snappy.adapters.buildarch import ( AllConflictInBuildForError, AllConflictInBuildOnError, + BadPropertyError, DuplicateBuildOnError, SnapArchitecture, SnapBuildInstance, @@ -475,10 +476,221 @@ class TestDetermineArchitecturesToBuild(WithScenarios, TestCaseWithFactory): "expected_exception": DuplicateBuildOnError, }, ), + ( + "platforms with configuration", + { + "platforms": { + "ubuntu-amd64": { + "build-on": ["amd64"], + "build-for": ["amd64"], + }, + "ubuntu-i386": { + "build-on": ["i386"], + "build-for": ["i386"], + }, + }, + "supported_architectures": ["amd64", "i386", "armhf"], + "expected": [ + { + "architecture": "amd64", + "target_architectures": ["amd64"], + "required": True, + }, + { + "architecture": "i386", + "target_architectures": ["i386"], + "required": True, + }, + ], + }, + ), + ( + "platforms with shorthand configuration", + { + "platforms": { + "amd64": {}, + "i386": {}, + }, + "supported_architectures": ["amd64", "i386", "armhf"], + "expected": [ + { + "architecture": "amd64", + "target_architectures": ["amd64"], + "required": True, + }, + { + "architecture": "i386", + "target_architectures": ["i386"], + "required": True, + }, + ], + }, + ), + ( + "platforms with unsupported architecture", + { + "platforms": { + "ubuntu-unsupported": {}, + }, + "supported_architectures": ["amd64", "i386", "armhf"], + "expected_exception": BadPropertyError, + }, + ), + ( + "platforms with multiple architectures", + { + "platforms": { + "ubuntu-amd64-i386": { + "build-on": ["amd64", "i386"], + "build-for": ["amd64", "i386"], + }, + }, + "supported_architectures": ["amd64", "i386", "armhf"], + "expected": [ + { + "architecture": "amd64", + "target_architectures": ["amd64", "i386"], + "required": True, + }, + ], + }, + ), + ( + "platforms with conflict in build-on", + { + "platforms": { + "ubuntu-conflict": { + "build-on": ["all", "amd64"], + }, + }, + "supported_architectures": ["amd64", "i386", "armhf"], + "expected_exception": AllConflictInBuildOnError, + }, + ), + ( + "platforms with conflict in build-for", + { + "platforms": { + "ubuntu-conflict": { + "build-on": ["amd64"], + "build-for": ["all", "amd64"], + }, + }, + "supported_architectures": ["amd64", "i386", "armhf"], + "expected_exception": AllConflictInBuildForError, + }, + ), + ( + "platforms with unsupported architecture in build-on", + { + "platforms": { + "ubuntu-amd64": { + "build-on": ["unsupported"], + "build-for": ["amd64"], + }, + }, + "supported_architectures": ["amd64", "i386", "armhf"], + # Launchpad ignores architectures that it does not know about + "expected": [], + }, + ), + ( + "platforms with 1/2 unsupported architectures in build-on", + { + "platforms": { + "ubuntu-amd64": { + "build-on": ["unsupported", "amd64"], + "build-for": ["amd64"], + }, + }, + "supported_architectures": ["amd64", "i386", "armhf"], + "expected": [ + { + "architecture": "amd64", + "target_architectures": ["amd64"], + "required": True, + }, + ], + }, + ), + ( + "platforms with duplicate build-on", + { + "snap_base_features": { + SnapBaseFeature.ALLOW_DUPLICATE_BUILD_ON: False + }, + "platforms": { + "ubuntu-amd64": { + "build-on": ["amd64"], + "build-for": ["amd64"], + }, + "ubuntu-amd64-duplicate": { + "build-on": ["amd64"], + "build-for": ["i386"], + }, + }, + "supported_architectures": ["amd64", "i386", "armhf"], + "expected_exception": DuplicateBuildOnError, + }, + ), + ( + "platforms with multiple build-for for the same build-on", + { + "snap_base_features": { + SnapBaseFeature.ALLOW_DUPLICATE_BUILD_ON: True + }, + "platforms": { + "ubuntu-amd64": { + "build-on": ["amd64"], + "build-for": ["amd64"], + }, + "ubuntu-amd64-i386": { + "build-on": ["amd64"], + "build-for": ["i386"], + }, + }, + "supported_architectures": ["amd64", "i386", "armhf"], + "expected": [ + { + "architecture": "amd64", + "target_architectures": ["amd64"], + "required": True, + }, + { + "architecture": "amd64", + "target_architectures": ["i386"], + "required": True, + }, + ], + }, + ), + ( + "platforms with all keyword", + { + "platforms": { + "ubuntu-all": { + "build-on": ["all"], + "build-for": ["all"], + }, + }, + "supported_architectures": ["amd64", "i386", "armhf"], + "expected": [ + { + "architecture": "amd64", + "target_architectures": ["all"], + "required": True, + }, + ], + }, + ), ] def test_parser(self): - snapcraft_data = {"architectures": self.architectures} + snapcraft_data = {} + if hasattr(self, "architectures"): + snapcraft_data["architectures"] = self.architectures + if hasattr(self, "platforms"): + snapcraft_data["platforms"] = self.platforms snap_base_features = getattr(self, "snap_base_features", {}) snap_base = self.factory.makeSnapBase(features=snap_base_features) if hasattr(self, "expected_exception"):
_______________________________________________ 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