From 7969ee8d56faa4e501a45e57c650796d769a8bbc Mon Sep 17 00:00:00 2001 From: Matt Culler Date: Fri, 18 Oct 2024 18:25:25 -0400 Subject: [PATCH] feat: use `craft-platforms` for build plans (#5111) - [ ] Have you followed the [guidelines for contributing](https://github.com/canonical/snapcraft/blob/main/CONTRIBUTING.md)? - [ ] Have you signed the [CLA](http://www.ubuntu.com/legal/contributors/)? - [ ] Have you successfully run `tox run -m lint`? - [ ] Have you successfully run `tox run -e test-py310`? (supported versions: `py39`, `py310`, `py311`, `py312`) ----- --------- Co-authored-by: Alex Lowe --- requirements-devel.txt | 2 +- requirements-docs.txt | 2 +- requirements.txt | 2 +- setup.py | 2 +- snapcraft/models/project.py | 59 +++++++++++++++++++----------- tests/unit/models/test_projects.py | 34 ----------------- 6 files changed, 42 insertions(+), 59 deletions(-) diff --git a/requirements-devel.txt b/requirements-devel.txt index 951ca5ebbc..b34cf5f2ea 100644 --- a/requirements-devel.txt +++ b/requirements-devel.txt @@ -29,7 +29,7 @@ craft-archives==2.0.0 craft-cli==2.7.0 craft-grammar==2.0.1 craft-parts==2.1.2 -craft-platforms==0.1.1 +craft-platforms==0.4.0 craft-providers==2.0.4 craft-store==3.0.2 cryptography==43.0.1 diff --git a/requirements-docs.txt b/requirements-docs.txt index a91631c168..3c0d37570a 100644 --- a/requirements-docs.txt +++ b/requirements-docs.txt @@ -24,7 +24,7 @@ craft-archives==2.0.0 craft-cli==2.7.0 craft-grammar==2.0.1 craft-parts==2.1.2 -craft-platforms==0.1.1 +craft-platforms==0.4.0 craft-providers==2.0.4 craft-store==3.0.2 cryptography==43.0.1 diff --git a/requirements.txt b/requirements.txt index 0094f8ba12..f8f5854405 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,7 +12,7 @@ craft-archives==2.0.0 craft-cli==2.7.0 craft-grammar==2.0.1 craft-parts==2.1.2 -craft-platforms==0.1.1 +craft-platforms==0.4.0 craft-providers==2.0.4 craft-store==3.0.2 cryptography==43.0.1 diff --git a/setup.py b/setup.py index 545c7b11b9..275446acd9 100755 --- a/setup.py +++ b/setup.py @@ -103,7 +103,7 @@ def recursive_data_files(directory, install_directory): "craft-cli~=2.6", "craft-grammar>=2.0.1,<3.0.0", "craft-parts>=2.1.2,<3.0.0", - "craft-platforms~=0.1", + "craft-platforms~=0.4", "craft-providers>=2.0.4,<3.0.0", "craft-store>=3.0.2,<4.0.0", "docutils<0.20", # Frozen until we can update sphinx dependencies. diff --git a/snapcraft/models/project.py b/snapcraft/models/project.py index 507a1161cd..d748ef347f 100644 --- a/snapcraft/models/project.py +++ b/snapcraft/models/project.py @@ -32,6 +32,7 @@ ) from craft_cli import emit from craft_grammar.models import Grammar # type: ignore[import-untyped] +from craft_platforms import Platforms, snap from craft_providers import bases from pydantic import ConfigDict, PrivateAttr, StringConstraints from typing_extensions import Annotated, Self, override @@ -277,6 +278,14 @@ def _get_partitions_from_components( return None +def _validate_mandatory_base(base: str | None, snap_type: str | None) -> None: + """Validate that the base is specified, if required by the snap_type.""" + if (base is not None) ^ (snap_type not in ["base", "kernel", "snapd"]): + raise ValueError( + "Snap base must be declared when type is not base, kernel or snapd" + ) + + class Socket(models.CraftBaseModel): """Snapcraft app socket definition.""" @@ -714,12 +723,7 @@ def _validate_adoptable_fields(self) -> Self: @pydantic.model_validator(mode="after") def _validate_mandatory_base(self): - snap_type = self.type - base = self.base - if (base is not None) ^ (snap_type not in ["base", "kernel", "snapd"]): - raise ValueError( - "Snap base must be declared when type is not base, kernel or snapd" - ) + _validate_mandatory_base(self.base, self.type) return self @pydantic.field_validator("name") @@ -1138,6 +1142,7 @@ class SnapcraftBuildPlanner(models.BuildPlanner): base: str | None = None build_base: str | None = None name: str + type: Literal["app", "base", "gadget", "kernel", "snapd"] | None = None platforms: dict[str, Platform] | None = None # type: ignore[assignment] architectures: list[str | Architecture] | None = None project_type: str | None = pydantic.Field(default=None, alias="type") @@ -1192,7 +1197,6 @@ def _validate_all_platforms( def get_build_plan(self) -> list[BuildInfo]: """Get the build plan for this project.""" - build_infos: list[BuildInfo] = [] effective_base = SNAPCRAFT_BASE_TO_PROVIDER_BASE[ str( get_effective_base( @@ -1205,8 +1209,6 @@ def get_build_plan(self) -> list[BuildInfo]: ) ].value - base = bases.BaseName("ubuntu", effective_base) - # set default value if self.platforms is None: self.platforms = { @@ -1221,16 +1223,31 @@ def get_build_plan(self) -> list[BuildInfo]: Platform.from_architectures(self.architectures) ) - for platform_entry, platform in self.platforms.items(): - for build_for in platform.build_for or [SnapArch(platform_entry).value]: - for build_on in platform.build_on or [SnapArch(platform_entry).value]: - build_infos.append( - BuildInfo( - platform=platform_entry, - build_on=build_on, - build_for=build_for, - base=base, - ) - ) + platforms = cast( + Platforms, + {name: platform.marshal() for name, platform in self.platforms.items()}, + ) - return build_infos + # In _validate_mandatory_base, we ensure that the possible values of + # 'base' and 'snap_type' are narrowed so they'll always match one of + # the two overloads of get_platforms_snap_build_plan. But, pyright and + # mypy aren't smart enough to realize this, so we need the type checker + # ignores. + _validate_mandatory_base(self.base, self.type) + return [ + BuildInfo( + platform=buildinfo.platform, + build_on=str(buildinfo.build_on), + build_for=str(buildinfo.build_for), + base=bases.BaseName( + name=buildinfo.build_base.distribution, + version=buildinfo.build_base.series, + ), + ) + for buildinfo in snap.get_platforms_snap_build_plan( # pyright: ignore[reportCallIssue] + base=self.base, # type: ignore[arg-type] + build_base=self.build_base, + snap_type=self.type, # type: ignore[arg-type] + platforms=platforms, + ) + ] diff --git a/tests/unit/models/test_projects.py b/tests/unit/models/test_projects.py index 99ccc4c4ee..1cbbdabbec 100644 --- a/tests/unit/models/test_projects.py +++ b/tests/unit/models/test_projects.py @@ -2486,40 +2486,6 @@ def test_platform_default(): ] -def test_build_planner_get_build_plan_base(mocker): - """Test `get_build_plan()` uses the correct base.""" - mock_get_effective_base = mocker.patch( - "snapcraft.models.project.get_effective_base", return_value="core24" - ) - planner = snapcraft.models.project.SnapcraftBuildPlanner.model_validate( - { - "name": "test-snap", - "base": "test-base", - "build-base": "test-build-base", - "platforms": {"amd64": None}, - "project_type": "test-type", - } - ) - - actual_build_infos = planner.get_build_plan() - - assert actual_build_infos == [ - BuildInfo( - platform="amd64", - build_on="amd64", - build_for="amd64", - base=BaseName(name="ubuntu", version="24.04"), - ) - ] - mock_get_effective_base.assert_called_once_with( - base="test-base", - build_base="test-build-base", - project_type="test-type", - name="test-snap", - translate_devel=False, - ) - - def test_project_platform_error_has_context(): """Platform validation errors include which platform entry is invalid.""" error = r"build-on\n Field required"