launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #31275
[Merge] ~ruinedyourlife/launchpad:feat-snapcraft-base-unification into launchpad:master
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"):
