diff --git a/rockcraft/models/__init__.py b/rockcraft/models/__init__.py index f0b68c4be..c063daa84 100644 --- a/rockcraft/models/__init__.py +++ b/rockcraft/models/__init__.py @@ -17,11 +17,6 @@ """Rockcraft models.""" -from rockcraft.models.project import ( - Project, - RockcraftBuildInfo, - load_project, - transform_yaml, -) +from rockcraft.models.project import Project, load_project, transform_yaml -__all__ = ["Project", "RockcraftBuildInfo", "load_project", "transform_yaml"] +__all__ = ["Project", "load_project", "transform_yaml"] diff --git a/rockcraft/models/project.py b/rockcraft/models/project.py index 4b01de960..f79087245 100644 --- a/rockcraft/models/project.py +++ b/rockcraft/models/project.py @@ -17,7 +17,6 @@ """Project definition and helpers.""" import dataclasses import operator -import platform as host_platform import re from builtins import super from functools import reduce @@ -39,6 +38,7 @@ import pydantic import spdx_lookup # type: ignore import yaml +from craft_application import util from craft_application.models import BuildInfo from craft_application.models import Project as BaseProject from craft_archives import repo @@ -55,64 +55,50 @@ from pydantic.error_wrappers import ErrorDict -@dataclasses.dataclass -class RockcraftBuildInfo(BuildInfo): - """BuildInfo with Rockcraft-specific entries.""" - - build_for_variant: Optional[str] = None - """Used for arm archs""" - - -class ArchitectureMapping(pydantic.BaseModel): +@dataclasses.dataclass(frozen=True) +class ArchitectureMapping: """Maps different denominations of the same architecture.""" description: str - deb_arch: str - compatible_uts_machine_archs: List[str] + compatible_deb_archs: List[str] go_arch: str +# The keys are valid debian architectures. _SUPPORTED_ARCHS: Dict[str, ArchitectureMapping] = { "amd64": ArchitectureMapping( description="Intel 64", - deb_arch="amd64", - compatible_uts_machine_archs=["amd64", "x86_64"], + compatible_deb_archs=["amd64"], go_arch="amd64", ), - "arm": ArchitectureMapping( + "armhf": ArchitectureMapping( description="ARM 32-bit", - deb_arch="armhf", - compatible_uts_machine_archs=["arm"], + compatible_deb_archs=["armhf"], go_arch="arm", ), "arm64": ArchitectureMapping( description="ARM 64-bit", - deb_arch="arm64", - compatible_uts_machine_archs=["aarch64"], + compatible_deb_archs=["arm64"], go_arch="arm64", ), "i386": ArchitectureMapping( description="Intel 386", - deb_arch="i386", - compatible_uts_machine_archs=["i386"], # TODO: also include "i686", "x86_64"? + compatible_deb_archs=["i386"], go_arch="386", ), - "ppc64le": ArchitectureMapping( + "ppc64el": ArchitectureMapping( description="PowerPC 64-bit", - deb_arch="ppc64el", - compatible_uts_machine_archs=["ppc64le"], + compatible_deb_archs=["ppc64el"], go_arch="ppc64le", ), "riscv64": ArchitectureMapping( description="RISCV 64-bit", - deb_arch="riscv64", - compatible_uts_machine_archs=["riscv64"], + compatible_deb_archs=["riscv64"], go_arch="riscv64", ), "s390x": ArchitectureMapping( description="IBM Z 64-bit", - deb_arch="s390x", - compatible_uts_machine_archs=["s390x"], + compatible_deb_archs=["s390x"], go_arch="s390x", ), } @@ -290,7 +276,7 @@ def _validate_build_base(cls, build_base: Optional[str], values: Any) -> str: @classmethod def _validate_all_platforms(cls, platforms: Dict[str, Any]) -> Dict[str, Any]: """Make sure all provided platforms are tangible and sane.""" - _self_uts_machine = host_platform.machine().lower() + host_arch = util.get_host_architecture() for platform_label in platforms: platform = platforms[platform_label] if platforms[platform_label] else {} @@ -352,42 +338,36 @@ def _validate_all_platforms(cls, platforms: Dict[str, Any]) -> Dict[str, Any]: # TODO: in the future, this may be removed # as Rockcraft gains the ability to natively build # for multiple architectures - build_for_compatible_uts = _SUPPORTED_ARCHS[ + build_for_compatible_deb_archs = _SUPPORTED_ARCHS[ build_target - ].compatible_uts_machine_archs - if _self_uts_machine not in build_for_compatible_uts: + ].compatible_deb_archs + if host_arch not in build_for_compatible_deb_archs: raise ProjectValidationError( str( - f"{error_prefix}: this machine's architecture ({_self_uts_machine}) " + f"{error_prefix}: this machine's architecture ({host_arch}) " "is not compatible with the ROCK's target architecture. Can only " - f"build a ROCK for {build_target} if the host is compatible with {build_for_compatible_uts}." + f"build a ROCK for {build_target} if the host is compatible with {build_for_compatible_deb_archs}." ) ) - build_on_compatible_uts = list( + build_on_compatible_deb_archs = list( reduce( operator.add, map( - lambda m: _SUPPORTED_ARCHS[m].compatible_uts_machine_archs, + lambda m: _SUPPORTED_ARCHS[m].compatible_deb_archs, build_on_one_of, ), ) ) - if _self_uts_machine not in build_on_compatible_uts: + if host_arch not in build_on_compatible_deb_archs: raise ProjectValidationError( str( f"{error_prefix}: this ROCK must be built on one of the " - f"following architectures: {build_on_compatible_uts}. " - f"This machine ({_self_uts_machine}) is not one of those." + f"following architectures: {build_on_compatible_deb_archs}. " + f"This machine ({host_arch}) is not one of those." ) ) - # Add variant, if needed, and return sanitized platform - if build_target == "arm": - platform["build_for_variant"] = "v7" - elif build_target == "arm64": - platform["build_for_variant"] = "v8" - platforms[platform_label] = platform return platforms @@ -522,14 +502,12 @@ def get_build_plan(self) -> List[BuildInfo]: 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]: - build_for_variant = platform.get("build_for_variant") build_infos.append( - RockcraftBuildInfo( + BuildInfo( platform=platform_entry, build_on=build_on, build_for=build_for, base=base, - build_for_variant=build_for_variant, ) ) diff --git a/rockcraft/oci.py b/rockcraft/oci.py index 44658e34b..8c9af3841 100644 --- a/rockcraft/oci.py +++ b/rockcraft/oci.py @@ -49,6 +49,17 @@ # The number of times to try downloading an image from `REGISTRY_URL`. MAX_DOWNLOAD_RETRIES = 5 +# A mapping from arch in Debian format to a pair of (arch in Docker format, arch variant). +# The values are taken from the supported architectures in the registry that we +# currently use (see https://gallery.ecr.aws/ubuntu/ubuntu). +_DEB_TO_DOCKER = { + "amd64": ("amd64", None), + "arm64": ("arm64", "v8"), + "armhf": ("arm", "v7"), + "ppc64el": ("ppc64le", None), + "s390x": ("s390x", None), +} + @dataclass(frozen=True) class Image: @@ -68,7 +79,6 @@ def from_docker_registry( *, image_dir: Path, arch: str, - variant: Optional[str] = None, ) -> Tuple["Image", str]: """Obtain an image from a docker registry. @@ -76,7 +86,7 @@ def from_docker_registry( :param image_name: The image to retrieve, in ``name:tag`` format. :param image_dir: The directory to store local OCI images. - :param arch: The architecture of the Docker image to fetch. + :param arch: The architecture of the Docker image to fetch, in debian format. :param variant: The variant, if any, of the Docker image to fetch. @@ -87,9 +97,12 @@ def from_docker_registry( source_image = f"docker://{REGISTRY_URL}/{image_name}" copy_params = ["--retry-times", str(MAX_DOWNLOAD_RETRIES)] + + docker_arch, variant = _DEB_TO_DOCKER[arch] + platform_params = [ "--override-arch", - arch, + docker_arch, ] if variant: platform_params += ["--override-variant", variant] @@ -108,13 +121,12 @@ def new_oci_image( image_name: str, image_dir: Path, arch: str, - variant: Optional[str] = None, ) -> Tuple["Image", str]: """Create a new OCI image out of thin air. :param image_name: The image to initiate, in ``name:tag`` format. :param image_dir: The directory to store the local OCI image. - :param arch: The architecture of the OCI image to create. + :param arch: The architecture of the OCI image to create, in debian format. :param variant: The variant, if any, of the OCI image to create. :returns: The new image object and it's corresponding source image @@ -130,6 +142,9 @@ def new_oci_image( # with arch and variant. We can configure the arch via # umoci config, but not the variant. Need to do it manually _config_image(image_target, ["--architecture", arch, "--no-history"]) + + variant = {"armhf": "v7", "arm64": "v8"}.get(arch) + if variant: _inject_architecture_variant(Path(image_target_no_tag), variant) diff --git a/rockcraft/services/image.py b/rockcraft/services/image.py index 8a050ab3a..105ffe4ee 100644 --- a/rockcraft/services/image.py +++ b/rockcraft/services/image.py @@ -72,7 +72,6 @@ def _create_image_info(self) -> ImageInfo: f"{project.base}:latest", image_dir=image_dir, arch=self._build_for, - variant=None, # TODO ) else: emit.progress(f"Retrieving base {project.base} for {build_for}") @@ -80,7 +79,6 @@ def _create_image_info(self) -> ImageInfo: project.base, image_dir=image_dir, arch=self._build_for, - variant=None, # TODO ) emit.progress(f"Retrieved base {project.base} for {build_for}") diff --git a/tests/unit/test_oci.py b/tests/unit/test_oci.py index 3fd485d35..746523b73 100644 --- a/tests/unit/test_oci.py +++ b/tests/unit/test_oci.py @@ -20,6 +20,7 @@ import json import os import tarfile +from collections import namedtuple from pathlib import Path from typing import List, Tuple from unittest.mock import ANY, call, mock_open, patch @@ -125,7 +126,7 @@ def test_attributes(self): def test_from_docker_registry(self, mock_run, new_dir): image, source_image = oci.Image.from_docker_registry( - "a:b", image_dir=Path("images/dir"), arch="amd64", variant=None + "a:b", image_dir=Path("images/dir"), arch="amd64" ) assert Path("images/dir").is_dir() assert image.image_name == "a:b" @@ -148,7 +149,7 @@ def test_from_docker_registry(self, mock_run, new_dir): ] mock_run.reset_mock() _ = oci.Image.from_docker_registry( - "a:b", image_dir=Path("images/dir"), arch="arm64", variant="v8" + "a:b", image_dir=Path("images/dir"), arch="arm64" ) assert mock_run.mock_calls == [ call( @@ -168,6 +169,46 @@ def test_from_docker_registry(self, mock_run, new_dir): ) ] + def _get_arch_from_call(self, mock_call): + ArchData = namedtuple("ArchData", ["override_arch", "override_variant"]) + + call_args = mock_call.args[0] + override_arch_index = call_args.index("--override-arch") + override_arch = call_args[override_arch_index + 1] + + override_variant = None + try: + override_variant_index = call_args.index("--override-variant") + override_variant = call_args[override_variant_index + 1] + except ValueError: + pass + + return ArchData(override_arch, override_variant) + + # The archs here were taken from the supported architectures in the registry + # that we currently use (https://gallery.ecr.aws/ubuntu/ubuntu) + @pytest.mark.parametrize( + ["deb_arch", "expected_arch", "expected_variant"], + [ + ("amd64", "amd64", None), + ("arm64", "arm64", "v8"), + ("armhf", "arm", "v7"), + ("ppc64el", "ppc64le", None), + ("s390x", "s390x", None), + ], + ) + def test_from_docker_registry_arch( + self, mock_run, new_dir, deb_arch, expected_arch, expected_variant + ): + """Test that the correct arch-related parameters are passed to skopeo.""" + oci.Image.from_docker_registry( + "a:b", image_dir=Path("images/dir"), arch=deb_arch + ) + arch_data = self._get_arch_from_call(mock_run.mock_calls[0]) + + assert arch_data.override_arch == expected_arch + assert arch_data.override_variant == expected_variant + def test_new_oci_image(self, mock_inject_variant, mock_run): image_dir = Path("images/dir") image, source_image = oci.Image.new_oci_image( @@ -193,10 +234,8 @@ def test_new_oci_image(self, mock_inject_variant, mock_run): ), ] mock_inject_variant.assert_not_called() - _ = oci.Image.new_oci_image( - "bare:latest", image_dir=image_dir, arch="foo", variant="bar" - ) - mock_inject_variant.assert_called_once_with(image_dir / "bare", "bar") + _ = oci.Image.new_oci_image("bare:latest", image_dir=image_dir, arch="armhf") + mock_inject_variant.assert_called_once_with(image_dir / "bare", "v7") def test_copy_to(self, mock_run): image = oci.Image("a:b", Path("/c")) diff --git a/tests/unit/test_project.py b/tests/unit/test_project.py index fb775aace..707fc3df8 100644 --- a/tests/unit/test_project.py +++ b/tests/unit/test_project.py @@ -20,15 +20,15 @@ import textwrap from pathlib import Path from typing import Any, Dict -from unittest.mock import patch import pydantic import pytest import yaml +from craft_application.models import BuildInfo from craft_providers.bases import BaseName from rockcraft.errors import ProjectLoadError, ProjectValidationError -from rockcraft.models import Project, RockcraftBuildInfo, load_project +from rockcraft.models import Project, load_project from rockcraft.models.project import INVALID_NAME_MESSAGE, ArchitectureMapping, Platform from rockcraft.pebble import Service @@ -262,8 +262,7 @@ def test_project_build_base(yaml_loaded_data): def test_architecture_mapping(): _ = ArchitectureMapping( description="mock arch", - deb_arch="amd64", - compatible_uts_machine_archs=["x86_64"], + compatible_deb_archs=["x86_64"], go_arch="amd64", ) @@ -299,36 +298,6 @@ def load_platform(platform, raises): ) -@patch("platform.machine") -def test_project_platform_variants(mock_uts_machine, yaml_loaded_data): - def load_project_for_arch(uts_arch: str, arch: str) -> Project: - mock_uts_machine.return_value = uts_arch - yaml_loaded_data["platforms"] = {arch: None} - return Project.unmarshal(yaml_loaded_data) - - # arm/v7 - assert ( - load_project_for_arch("arm", "arm").platforms["arm"]["build_for_variant"] - == "v7" - ) - - # arm64/v8 - assert ( - load_project_for_arch("aarch64", "arm64").platforms["arm64"][ - "build_for_variant" - ] - == "v8" - ) - - # others - assert ( - load_project_for_arch("x86_64", "amd64") - .platforms["amd64"] - .get("build_for_variant") - is None - ) - - def test_project_all_platforms_invalid(yaml_loaded_data): def reload_project_platforms(new_platforms=None): if new_platforms: @@ -367,7 +336,7 @@ def reload_project_platforms(new_platforms=None): # The underlying build machine must be compatible # with both build_on and build_for - other_arch = "arm" if BUILD_ON_ARCH == "amd64" else "amd64" + other_arch = "arm64" if BUILD_ON_ARCH == "amd64" else "amd64" mock_platforms = {"mock": {"build-on": [other_arch], "build-for": other_arch}} assert "if the host is compatible with" in reload_project_platforms(mock_platforms) @@ -598,12 +567,11 @@ def test_project_yaml(yaml_loaded_data): "amd64": None, }, [ - RockcraftBuildInfo( + BuildInfo( build_on="amd64", build_for="amd64", base=BaseName(name="ubuntu", version="20.04"), platform="amd64", - build_for_variant=None, ) ], ), @@ -615,19 +583,17 @@ def test_project_yaml(yaml_loaded_data): }, }, [ - RockcraftBuildInfo( + BuildInfo( build_on="amd64", build_for="amd64", base=BaseName(name="ubuntu", version="20.04"), platform="amd64", - build_for_variant=None, ), - RockcraftBuildInfo( + BuildInfo( build_on="i386", build_for="amd64", base=BaseName(name="ubuntu", version="20.04"), platform="amd64", - build_for_variant=None, ), ], ), @@ -639,12 +605,11 @@ def test_project_yaml(yaml_loaded_data): }, }, [ - RockcraftBuildInfo( + BuildInfo( build_on="amd64", build_for="amd64", base=BaseName(name="ubuntu", version="20.04"), platform="amd64v2", - build_for_variant=None, ) ], ),