chore: finalize feature and feedback from review

Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>

snapcraft/application.py

@@ -22,7 +22,7 @@
 import pathlib
 import signal
 import sys
-from typing import Any, List
+from typing import Any, List, Sequence
 
 import craft_application.commands as craft_app_commands
 import craft_application.models
@@ -37,7 +37,7 @@
 
 from snapcraft import cli, commands, errors, models, services
 from snapcraft.commands import unimplemented
-from snapcraft.const import SUPPORTED_ARCHS
+from snapcraft.const import SUPPORTED_ARCHS, SnapArch
 from snapcraft.extensions import apply_extensions
 from snapcraft.models import Platform
 from snapcraft.models.project import apply_root_packages
@@ -51,37 +51,37 @@ class SnapcraftBuildPlanner(craft_application.models.BuildPlanner):
     base: str | None
     build_base: str | None = None
     name: str
-    platforms: dict[str, Any]
+    platforms: dict[str, Any] | None = None
     project_type: str | None = pydantic.Field(default=None, alias="type")
 
     @pydantic.validator("platforms")
     @classmethod
     def _validate_all_platforms(cls, platforms: dict[str, Any]) -> dict[str, Any]:
-        """Make sure all provided platforms are tangible and sane."""
+        """Validate and convert platform data to a dict of Platforms."""
         for platform_label in platforms:
-            platform: dict[str, Any] = (
+            platform_data: dict[str, Any] = (
                 platforms[platform_label] if platforms[platform_label] else {}
             )
             error_prefix = f"Error for platform entry '{platform_label}'"
 
             # Make sure the provided platform_set is valid
             try:
-                platform = Platform(**platform).dict()
+                platform = Platform(**platform_data)
             except CraftValidationError as err:
-                # pylint: disable=raise-missing-from
-                raise CraftValidationError(f"{error_prefix}: {str(err)}")
+                raise CraftValidationError(f"{error_prefix}: {str(err)}") from None
 
             # build_on and build_for are validated
             # let's also validate the platform label
-            build_on_one_of = (
-                platform["build_on"] if platform["build_on"] else [platform_label]
-            )
+            if platform.build_on:
+                build_on_one_of: Sequence[SnapArch | str] = platform.build_on
+            else:
+                build_on_one_of = [platform_label]
 
             # If the label maps to a valid architecture and
             # `build-for` is present, then both need to have the same value,
             # otherwise the project is invalid.
-            if platform["build_for"]:
-                build_target = platform["build_for"][0]
+            if platform.build_for:
+                build_target = platform.build_for[0]
                 if platform_label in SUPPORTED_ARCHS and platform_label != build_target:
                     raise CraftValidationError(
                         str(
@@ -90,24 +90,21 @@ def _validate_all_platforms(cls, platforms: dict[str, Any]) -> dict[str, Any]:
                             f"both values must match. {platform_label} != {build_target}"
                         )
                     )
-            else:
-                build_target = platform_label
-
-            # Both build and target architectures must be supported
-            if not any(b_o in SUPPORTED_ARCHS for b_o in build_on_one_of):
+            # if no build-for is present, then the platform label needs to be a valid architecture
+            elif platform_label not in SUPPORTED_ARCHS:
                 raise CraftValidationError(
                     str(
-                        f"{error_prefix}: trying to build snap in one of "
-                        f"{build_on_one_of}, but none of these build architectures is supported. "
-                        f"Supported architectures: {SUPPORTED_ARCHS}"
+                        f"{error_prefix}: platform entry label must correspond to a "
+                        "valid architecture if 'build-for' is not provided."
                     )
                 )
 
-            if build_target not in SUPPORTED_ARCHS:
+            # Both build and target architectures must be supported
+            if not any(b_o in SUPPORTED_ARCHS for b_o in build_on_one_of):
                 raise CraftValidationError(
                     str(
-                        f"{error_prefix}: trying to build snap for target "
-                        f"architecture {build_target}, which is not supported. "
+                        f"{error_prefix}: trying to build snap in one of "
+                        f"{build_on_one_of}, but none of these build architectures are supported. "
                         f"Supported architectures: {SUPPORTED_ARCHS}"
                     )
                 )
@@ -132,9 +129,12 @@ def get_build_plan(self) -> List[BuildInfo]:
 
         base = bases.BaseName("ubuntu", effective_base)
 
+        if self.platforms is None:
+            raise CraftValidationError("Must define at least one platform.")
+
         for platform_entry, platform in self.platforms.items():
-            for build_for in platform.get("build_for") or [platform_entry]:
-                for build_on in platform.get("build_on") or [platform_entry]:
+            for build_for in platform.build_for or [platform_entry]:
+                for build_on in platform.build_on or [platform_entry]:
                     build_infos.append(
                         BuildInfo(
                             platform=platform_entry,

snapcraft/meta/snap_yaml.py

@@ -1,6 +1,6 @@
 # -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
 #
-# Copyright 2022-2023 Canonical Ltd.
+# Copyright 2022-2024 Canonical Ltd.
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as

snapcraft/models/__init__.py

@@ -1,6 +1,6 @@
 # -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
 #
-# Copyright 2022-2023 Canonical Ltd.
+# Copyright 2022-2024 Canonical Ltd.
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as

snapcraft/models/project.py

@@ -23,7 +23,6 @@
 from typing import (
     TYPE_CHECKING,
     Any,
-    Callable,
     Dict,
     List,
     Literal,
@@ -445,47 +444,30 @@ def _validate_default_provider(cls, default_provider):
         return default_provider
 
 
-class Platform(pydantic.BaseModel):
+class Platform(models.CraftBaseModel):
     """Snapcraft project platform definition."""
 
     build_on: list[SnapArch] | None = pydantic.Field(min_items=1, unique_items=True)
-    build_for: list[SnapArch] | None = pydantic.Field(min_items=1, unique_items=True)
-
-    class Config:  # pylint: disable=too-few-public-methods
-        """Pydantic model configuration."""
-
-        allow_population_by_field_name = True
-        alias_generator: Callable[[str], str] = lambda s: s.replace(  # noqa: E731
-            "_", "-"
-        )
+    build_for: list[SnapArch] | None = pydantic.Field(
+        min_items=1, max_items=1, unique_items=True
+    )
 
-    @pydantic.validator("build_for", pre=True)
+    @pydantic.validator("build_on", "build_for", pre=True)
     @classmethod
     def _vectorise_build_for(cls, val: str | list[str]) -> list[str]:
-        """Vectorise target architecture if needed."""
+        """Vectorise target architectures if needed."""
         if isinstance(val, str):
             val = [val]
         return val
 
     @pydantic.root_validator(skip_on_failure=True)
     @classmethod
     def _validate_platform_set(cls, values: Mapping[str, Any]) -> Mapping[str, Any]:
-        """Validate the build_on build_for combination."""
-        build_for: list[str] = values["build_for"] if values.get("build_for") else []
-        build_on: list[str] = values["build_on"] if values.get("build_on") else []
+        """If build_for is provided, then build_on must also be.
 
-        # We can only build for 1 arch at the moment
-        if len(build_for) > 1:
-            raise CraftValidationError(
-                str(
-                    f"Trying to build a rock for {build_for} "
-                    "but multiple target architectures are not "
-                    "currently supported. Please specify only 1 value."
-                )
-            )
-
-        # If build_for is provided, then build_on must also be
-        if not build_on and build_for:
+        This aligns with the precedent set by the `architectures` keyword.
+        """
+        if not values.get("build_on") and values.get("build_for"):
             raise CraftValidationError(
                 "'build_for' expects 'build_on' to also be provided."
             )
@@ -522,7 +504,6 @@ class Project(models.Project):
     ]
     grade: Optional[Literal["stable", "devel"]]
     architectures: List[Union[str, Architecture]] | None = None
-    # TODO: set architectures to [get_host_architecture()] if base is core22
     assumes: UniqueStrList = cast(UniqueStrList, [])
     package_repositories: Optional[List[Dict[str, Any]]]
     hooks: Optional[Dict[str, Hook]]
@@ -669,7 +650,7 @@ def _validate_platforms_and_architectures(cls, values):
 
         core24 and newer bases:
          - cannot define architectures
-         - must define platforms
+         - can optionally define platforms
         """
         base = get_effective_base(
             base=values.get("base"),
@@ -678,26 +659,24 @@ def _validate_platforms_and_architectures(cls, values):
             name=values.get("name"),
         )
 
-        if base == "core24":
-            if values.get("architectures"):
-                raise ValueError(
-                    (
-                        "'architectures' keyword is not supported for base %r. "
-                        "Use 'platforms' keyword instead.",
-                        base,
-                    ),
-                )
-            if not values.get("platforms"):
+        if base == "core22":
+            if values.get("platforms"):
                 raise ValueError(
-                    ("The 'platforms' keyword must be defined for base %r. ", base)
+                    f"'platforms' keyword is not supported for base {base!r}. "
+                    "Use 'architectures' keyword instead."
                 )
-        else:
+            # set default value
             if not values.get("architectures"):
                 values["architectures"] = [get_host_architecture()]
-            if values.get("platforms"):
+        else:
+            if values.get("architectures"):
                 raise ValueError(
-                    "'platforms' keyword is not supported with 'base=core22'"
+                    f"'architectures' keyword is not supported for base {base!r}. "
+                    "Use 'platforms' keyword instead."
                 )
+            # set default value
+            if not values.get("platforms"):
+                values["platforms"] = {get_host_architecture(): None}
 
         return values
 

spread.yaml

@@ -272,10 +272,6 @@ suites:
    systems:
      - ubuntu-22.04*
 
- # TODO:
- # - add `platforms` keyword to all core24 snapcraft.yaml files
- # - add core24 platforms tests
- # - remove core24 architecture tests
  tests/spread/core24/:
    summary: core24 specific tests
    systems:

tests/spread/core24/grammar/snap/snapcraft.yaml

@@ -3,14 +3,16 @@ version: "1.0"
 summary: test
 description: |
   Exercise snapcraft's advanced grammar keywords, `on`, and `to`.
-  This test leverages architecture keywords, `build-on` and `build-for`.
+  This test leverages the platform keywords, `build-on` and `build-for`.
 grade: devel
 confinement: strict
 base: core22
-architectures:
-  - build-on: amd64
+platforms:
+  platform1:
+    build-on: amd64
     build-for: amd64
-  - build-on: amd64
+  platform2:
+    build-on: amd64
     build-for: arm64
 
 parts:

tests/spread/core24/package-repositories/test-foreign-armhf/snapcraft.yaml

@@ -6,8 +6,8 @@ grade: devel
 confinement: strict
 base: core24
 build-base: devel
-architectures:
-  - build-on: amd64
+platforms:
+  amd64:
 
 package-repositories:
   # The repo that contains libpython3.11-minimal:armhf

tests/spread/core24/package-repositories/test-foreign-i386/snapcraft.yaml

@@ -6,8 +6,8 @@ grade: devel
 confinement: strict
 base: core24
 build-base: devel
-architectures:
-  - build-on: amd64
+platforms:
+  amd64:
 
 package-repositories:
   # The repo that contains libpython3.11-minimal:i386

tests/spread/core24/architectures/snaps/arg-match/snap/snapcraft.yaml → tests/spread/core24/platforms/snaps/arg-match/snap/snapcraft.yaml

@@ -8,10 +8,12 @@ grade: devel
 confinement: strict
 base: core24
 build-base: devel
-architectures:
-  - build-on: amd64
+platforms:
+  platform1:
+    build-on: amd64
     build-for: amd64
-  - build-on: [amd64, arm64]
+  platform2:
+    build-on: [amd64, arm64]
     build-for: arm64
 
 parts:

tests/spread/core24/architectures/snaps/arg-no-match/snap/snapcraft.yaml → tests/spread/core24/platforms/snaps/arg-no-match/snap/snapcraft.yaml

@@ -8,10 +8,12 @@ grade: devel
 confinement: strict
 base: core24
 build-base: devel
-architectures:
-  - build-on: amd64
+platforms:
+  platform1:
+    build-on: amd64
     build-for: amd64
-  - build-on: [amd64, arm64]
+  platform2:
+    build-on: [amd64, arm64]
     build-for: arm64
 
 parts:

tests/spread/core24/architectures/snaps/build-on-no-match/snap/snapcraft.yaml → tests/spread/core24/platforms/snaps/build-on-no-match/snap/snapcraft.yaml

@@ -8,10 +8,12 @@ grade: devel
 confinement: strict
 base: core24
 build-base: devel
-architectures:
-  - build-on: arm64
+platforms:
+  platform1:
+    build-on: arm64
     build-for: arm64
-  - build-on: armhf
+  platform2:
+    build-on: armhf
     build-for: armhf
 
 parts:

tests/spread/core24/architectures/snaps/env-var-match/snap/snapcraft.yaml → tests/spread/core24/platforms/snaps/env-var-match/snap/snapcraft.yaml

@@ -8,10 +8,12 @@ grade: devel
 confinement: strict
 base: core24
 build-base: devel
-architectures:
-  - build-on: amd64
+platforms:
+  platform1:
+    build-on: amd64
     build-for: amd64
-  - build-on: [amd64, arm64]
+  platform2:
+    build-on: [amd64, arm64]
     build-for: arm64
 
 parts:

tests/spread/core24/architectures/snaps/env-var-no-match/snap/snapcraft.yaml → tests/spread/core24/platforms/snaps/env-var-no-match/snap/snapcraft.yaml

@@ -8,10 +8,12 @@ grade: devel
 confinement: strict
 base: core24
 build-base: devel
-architectures:
-  - build-on: amd64
+platforms:
+  platform1:
+    build-on: amd64
     build-for: amd64
-  - build-on: [amd64, arm64]
+  platform2:
+    build-on: [amd64, arm64]
     build-for: arm64
 
 parts: