From cddfaab851d5dc891674f63e9b05ff3c2df7fb34 Mon Sep 17 00:00:00 2001 From: Callahan Date: Thu, 5 Oct 2023 09:42:03 -0500 Subject: [PATCH] fix(project): improve error message for unsupported architectures (#4392) Signed-off-by: Callahan Kovacs --- snapcraft/models/project.py | 15 +++++++++++ snapcraft/utils.py | 18 +++++++++++++ tests/unit/models/test_projects.py | 18 +++++++++++++ tests/unit/parts/test_lifecycle.py | 42 +++++++++++++++--------------- tests/unit/test_utils.py | 32 +++++++++++++++++++++++ 5 files changed, 104 insertions(+), 21 deletions(-) diff --git a/snapcraft/models/project.py b/snapcraft/models/project.py index fe7705017df..43f91e1e702 100644 --- a/snapcraft/models/project.py +++ b/snapcraft/models/project.py @@ -34,6 +34,8 @@ convert_architecture_deb_to_platform, get_effective_base, get_host_architecture, + get_supported_architectures, + is_architecture_supported, ) # A workaround for mypy false positives @@ -101,6 +103,19 @@ def _validate_architectures(architectures): ) unique_build_fors.add(architecture) + # validate architectures are supported + if len(architectures): + for element in architectures: + for arch in element.build_for + element.build_on: + if arch != "all" and not is_architecture_supported(arch): + supported_archs = utils.humanize_list( + get_supported_architectures(), "and" + ) + raise ValueError( + f"Architecture {arch!r} is not supported. Supported " + f"architectures are {supported_archs}." + ) + return architectures diff --git a/snapcraft/utils.py b/snapcraft/utils.py index 84a5cd22e7c..77c76c6b0ed 100644 --- a/snapcraft/utils.py +++ b/snapcraft/utils.py @@ -127,6 +127,24 @@ def get_host_architecture(): ) +def is_architecture_supported(architecture: str) -> bool: + """Check if an debian-syntax architecture is supported. + + :param architecture: architecture to check + + :returns: True if the architecture is supported by snapcraft. + """ + return architecture in list(_ARCH_TRANSLATIONS_DEB_TO_PLATFORM) + + +def get_supported_architectures() -> List[str]: + """Get a list of architectures supported by snapcraft. + + :returns: A list of architectures. + """ + return list(_ARCH_TRANSLATIONS_DEB_TO_PLATFORM.keys()) + + def convert_architecture_deb_to_platform(architecture: str) -> str: """Convert an architecture from deb/snap syntax to platform syntax. diff --git a/tests/unit/models/test_projects.py b/tests/unit/models/test_projects.py index d078f261196..deee80a1035 100644 --- a/tests/unit/models/test_projects.py +++ b/tests/unit/models/test_projects.py @@ -1712,6 +1712,24 @@ def test_architecture_multiple_build_for_same_architecture_implicit( error.value ) + @pytest.mark.parametrize( + "architectures", + [ + "unknown", + {"build-on": ["unknown"]}, + {"build-on": ["unknown"], "build-for": ["amd64"]}, + {"build-on": ["amd64"], "build-for": ["unknown"]}, + ], + ) + def test_architecture_unsupported(self, architectures, project_yaml_data): + """Raise an error for unsupported architectures.""" + data = project_yaml_data(architectures=[architectures]) + + with pytest.raises(errors.ProjectValidationError) as error: + Project.unmarshal(data) + + assert "Architecture 'unknown' is not supported." in str(error.value) + def test_project_get_build_on(self, project_yaml_data): """Test `get_build_on()` returns the build-on string.""" data = project_yaml_data( diff --git a/tests/unit/parts/test_lifecycle.py b/tests/unit/parts/test_lifecycle.py index dad30009705..5f782b3b01b 100644 --- a/tests/unit/parts/test_lifecycle.py +++ b/tests/unit/parts/test_lifecycle.py @@ -1634,35 +1634,35 @@ def test_lifecycle_run_in_provider_devel_base( def test_get_build_plan_single_element_matching(snapcraft_yaml, mocker, new_dir): """Test get_build_plan with a single matching element.""" mocker.patch( - "snapcraft.parts.lifecycle.get_host_architecture", return_value="aarch64" + "snapcraft.parts.lifecycle.get_host_architecture", return_value="arm64" ) yaml_data = { "base": "core22", - "architectures": [{"build-on": "aarch64", "build-for": "aarch64"}], + "architectures": [{"build-on": "arm64", "build-for": "arm64"}], } snapcraft_yaml_data = snapcraft_yaml(**yaml_data) assert parts_lifecycle.get_build_plan( snapcraft_yaml_data, parsed_args=argparse.Namespace(build_for=None) - ) == [("aarch64", "aarch64")] + ) == [("arm64", "arm64")] def test_get_build_plan_build_for_all(snapcraft_yaml, mocker, new_dir): """Test get_build_plan with `build-for: all`.""" mocker.patch( - "snapcraft.parts.lifecycle.get_host_architecture", return_value="aarch64" + "snapcraft.parts.lifecycle.get_host_architecture", return_value="arm64" ) yaml_data = { "base": "core22", - "architectures": [{"build-on": "aarch64", "build-for": "all"}], + "architectures": [{"build-on": "arm64", "build-for": "all"}], } snapcraft_yaml_data = snapcraft_yaml(**yaml_data) assert parts_lifecycle.get_build_plan( snapcraft_yaml_data, parsed_args=argparse.Namespace(build_for=None) - ) == [("aarch64", "all")] + ) == [("arm64", "all")] def test_get_build_plan_with_matching_elements(snapcraft_yaml, mocker, new_dir): @@ -1670,13 +1670,13 @@ def test_get_build_plan_with_matching_elements(snapcraft_yaml, mocker, new_dir): the host architecture. """ mocker.patch( - "snapcraft.parts.lifecycle.get_host_architecture", return_value="aarch64" + "snapcraft.parts.lifecycle.get_host_architecture", return_value="arm64" ) yaml_data = { "base": "core22", "architectures": [ - {"build-on": "aarch64", "build-for": "aarch64"}, - {"build-on": "aarch64", "build-for": "arm64"}, + {"build-on": "arm64", "build-for": "amd64"}, + {"build-on": "arm64", "build-for": "arm64"}, {"build-on": "armhf", "build-for": "armhf"}, ], } @@ -1686,8 +1686,8 @@ def test_get_build_plan_with_matching_elements(snapcraft_yaml, mocker, new_dir): assert parts_lifecycle.get_build_plan( snapcraft_yaml_data, parsed_args=argparse.Namespace(build_for=None) ) == [ - ("aarch64", "aarch64"), - ("aarch64", "arm64"), + ("arm64", "amd64"), + ("arm64", "arm64"), ] @@ -1696,7 +1696,7 @@ def test_get_build_plan_list_without_matching_element(snapcraft_yaml, mocker, ne the host architecture. """ mocker.patch( - "snapcraft.parts.lifecycle.get_host_architecture", return_value="aarch64" + "snapcraft.parts.lifecycle.get_host_architecture", return_value="arm64" ) yaml_data = { "base": "core22", @@ -1718,21 +1718,21 @@ def test_get_build_plan_list_with_matching_element_and_env_var( ): """The build plan should be filtered down when `SNAPCRAFT_BUILD_FOR` is defined.""" mocker.patch( - "snapcraft.parts.lifecycle.get_host_architecture", return_value="aarch64" + "snapcraft.parts.lifecycle.get_host_architecture", return_value="arm64" ) yaml_data = { "base": "core22", "architectures": [ - {"build-on": "aarch64", "build-for": "aarch64"}, - {"build-on": "aarch64", "build-for": "armhf"}, + {"build-on": "arm64", "build-for": "arm64"}, + {"build-on": "arm64", "build-for": "armhf"}, ], } snapcraft_yaml_data = snapcraft_yaml(**yaml_data) assert parts_lifecycle.get_build_plan( - snapcraft_yaml_data, parsed_args=argparse.Namespace(build_for="aarch64") - ) == [("aarch64", "aarch64")] + snapcraft_yaml_data, parsed_args=argparse.Namespace(build_for="arm64") + ) == [("arm64", "arm64")] def test_get_build_plan_list_without_matching_element_and_build_for_arg( @@ -1741,13 +1741,13 @@ def test_get_build_plan_list_without_matching_element_and_build_for_arg( """The build plan should be empty when no plan has a matching `build_for` matching `SNAPCRAFT_BUILD_FOR.`""" mocker.patch( - "snapcraft.parts.lifecycle.get_host_architecture", return_value="aarch64" + "snapcraft.parts.lifecycle.get_host_architecture", return_value="arm64" ) yaml_data = { "base": "core22", "architectures": [ - {"build-on": "aarch64", "build-for": "aarch64"}, - {"build-on": "aarch64", "build-for": "armhf"}, + {"build-on": "arm64", "build-for": "arm64"}, + {"build-on": "arm64", "build-for": "armhf"}, ], } @@ -1755,7 +1755,7 @@ def test_get_build_plan_list_without_matching_element_and_build_for_arg( assert ( parts_lifecycle.get_build_plan( - snapcraft_yaml_data, parsed_args=argparse.Namespace(build_for="arm64") + snapcraft_yaml_data, parsed_args=argparse.Namespace(build_for="amd64") ) == [] ) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 8039a6bb9a8..9807f5ec5cc 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -478,6 +478,38 @@ def test_process_version_git(mocker): assert utils.process_version("git") == "1.2.3-dirty" +########################### +# Supported architectures # +########################### + + +@pytest.mark.parametrize("arch", utils.get_supported_architectures()) +def test_is_architecture_supported(arch): + """Supported architectures should return true.""" + assert utils.is_architecture_supported(arch) + + +def test_is_architecture_not_supported(): + """Unsupported architectures should return false.""" + assert not utils.is_architecture_supported("unknown") + + +def get_supported_architectures(): + """Validate list of supported architectures.""" + supported_archs = utils.get_supported_architectures() + + assert supported_archs == [ + "arm64", + "armhf", + "i386", + "powerpc", + "ppc64el", + "amd64", + "s390x", + "riscv64", + ] + + ######################### # Convert Architectures # #########################