From 8b16cae4514bcdb66e7b7876f95ffcf82921c6ac Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Wed, 7 Aug 2024 15:13:46 -0400 Subject: [PATCH] fix(package): Limit the bases in manifest.yaml (#1795) For a `bases` charm, this was previously writing out all run bases rather than the specific bases for this build. --- charmcraft/services/package.py | 15 +++- charmcraft/utils/yaml.py | 2 + tests/conftest.py | 15 +++- .../complex-legacy/prime/manifest.yaml | 9 --- .../prime/manifest.yaml | 9 --- tests/integration/test_charm_builder.py | 2 +- tests/unit/models/test_project.py | 24 +++++- tests/unit/services/test_package.py | 78 +++++++++++++++++-- 8 files changed, 119 insertions(+), 35 deletions(-) diff --git a/charmcraft/services/package.py b/charmcraft/services/package.py index fa7950a4f..5ee1f5b87 100644 --- a/charmcraft/services/package.py +++ b/charmcraft/services/package.py @@ -17,7 +17,6 @@ """Service class for packing.""" from __future__ import annotations -import itertools import json import os import pathlib @@ -193,7 +192,19 @@ def get_manifest(self, lint_results: Iterable[lint.CheckResult]) -> Manifest: def get_manifest_bases(self) -> list[models.Base]: """Get the bases used for a charm manifest from the project.""" if isinstance(self._project, BasesCharm): - return list(itertools.chain.from_iterable(base.run_on for base in self._project.bases)) + run_on_bases = [] + for project_base in self._project.bases: + for build_base in project_base.build_on: + if build_base.name != self._build_plan[0].base.name: + continue + if build_base.channel != self._build_plan[0].base.version: + continue + if self._build_plan[0].build_on not in build_base.architectures: + continue + run_on_bases.extend(project_base.run_on) + if not run_on_bases: + raise RuntimeError("Could not determine run-on bases.") + return run_on_bases if isinstance(self._project, PlatformCharm): if not self._platform: architectures = [util.get_host_architecture()] diff --git a/charmcraft/utils/yaml.py b/charmcraft/utils/yaml.py index 4e59ca8c4..4f1bcd194 100644 --- a/charmcraft/utils/yaml.py +++ b/charmcraft/utils/yaml.py @@ -17,6 +17,7 @@ from typing import Any +import pydantic import yaml from craft_cli import emit @@ -47,6 +48,7 @@ def _repr_str(dumper: yaml.SafeDumper, data: str) -> yaml.ScalarNode: def dump_yaml(data: Any) -> str: # noqa: ANN401: yaml.dump takes anything, so why can't we? """Dump a craft model to a YAML string.""" yaml.add_representer(str, _repr_str, Dumper=yaml.SafeDumper) + yaml.add_representer(pydantic.AnyHttpUrl, _repr_str, Dumper=yaml.SafeDumper) yaml.add_representer( const.CharmArch, yaml.representer.SafeRepresenter.represent_str, diff --git a/tests/conftest.py b/tests/conftest.py index 43c422af3..095fc7544 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -49,7 +49,18 @@ def simple_charm(): name="charmy-mccharmface", summary="Charmy!", description="Very charming!", - bases=[{"name": "ubuntu", "channel": "22.04", "architectures": ["arm64"]}], + bases=[ + { + "build-on": [ + { + "name": "ubuntu", + "channel": "22.04", + "architectures": [util.get_host_architecture()], + } + ], + "run-on": [{"name": "ubuntu", "channel": "22.04", "architectures": ["arm64"]}], + } + ], ) @@ -108,7 +119,7 @@ def default_build_plan(): models.BuildInfo( base=bases.BaseName("ubuntu", "22.04"), build_on=arch, - build_for=arch, + build_for="arm64", platform="distro-1-test64", ) ] diff --git a/tests/integration/services/sample_projects/complex-legacy/prime/manifest.yaml b/tests/integration/services/sample_projects/complex-legacy/prime/manifest.yaml index 6409b27c4..c6d9192db 100644 --- a/tests/integration/services/sample_projects/complex-legacy/prime/manifest.yaml +++ b/tests/integration/services/sample_projects/complex-legacy/prime/manifest.yaml @@ -13,15 +13,6 @@ bases: channel: '22.04' architectures: - arm64 -- name: ubuntu - channel: '20.04' - architectures: - - amd64 - - arm64 - - riscv64 - - s390x - - ppc64el - - armhf analysis: attributes: - name: language diff --git a/tests/integration/services/sample_projects/complex-self-contained/prime/manifest.yaml b/tests/integration/services/sample_projects/complex-self-contained/prime/manifest.yaml index 467326074..a4cb0bb56 100644 --- a/tests/integration/services/sample_projects/complex-self-contained/prime/manifest.yaml +++ b/tests/integration/services/sample_projects/complex-self-contained/prime/manifest.yaml @@ -13,15 +13,6 @@ bases: channel: '22.04' architectures: - arm64 -- name: ubuntu - channel: '20.04' - architectures: - - amd64 - - arm64 - - riscv64 - - s390x - - ppc64el - - armhf analysis: attributes: - name: language diff --git a/tests/integration/test_charm_builder.py b/tests/integration/test_charm_builder.py index 0b8cc5be7..16c84e877 100644 --- a/tests/integration/test_charm_builder.py +++ b/tests/integration/test_charm_builder.py @@ -35,7 +35,7 @@ ], ) def test_install_strict_dependencies_pip_check_error( - new_path: pathlib.Path, requirements: list[str] + monkeypatch, new_path: pathlib.Path, requirements: list[str] ): build_dir = new_path / "build" install_dir = new_path / "install" diff --git a/tests/unit/models/test_project.py b/tests/unit/models/test_project.py index 4ff7a00fc..e64d3b756 100644 --- a/tests/unit/models/test_project.py +++ b/tests/unit/models/test_project.py @@ -68,17 +68,33 @@ } BASIC_CHARM_PARTS = {"charm": {"plugin": "charm", "source": "."}} -MINIMAL_CHARMCRAFT_YAML = """\ +MINIMAL_CHARMCRAFT_YAML = f"""\ type: charm -bases: [{name: ubuntu, channel: "22.04", architectures: [arm64]}] +bases: + - build-on: + - name: ubuntu + channel: "22.04" + architectures: [{util.get_host_architecture()}] + run-on: + - name: ubuntu + channel: "22.04" + architectures: [arm64] """ SIMPLE_METADATA_YAML = "{name: charmy-mccharmface, summary: Charmy!, description: Very charming!}" -SIMPLE_CHARMCRAFT_YAML = """\ +SIMPLE_CHARMCRAFT_YAML = f"""\ type: charm name: charmy-mccharmface summary: Charmy! description: Very charming! -bases: [{name: ubuntu, channel: "22.04", architectures: [arm64]}] +bases: + - build-on: + - name: ubuntu + channel: "22.04" + architectures: [{util.get_host_architecture()}] + run-on: + - name: ubuntu + channel: "22.04" + architectures: [arm64] """ SIMPLE_CONFIG_YAML = "options: {admin: {default: root, description: Admin user, type: string}}" SIMPLE_CONFIG_DICT = { diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index c38e09941..d1501775e 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -16,21 +16,26 @@ """Tests for package service.""" import datetime +import pathlib import sys import zipfile +from typing import Any import craft_cli.pytest_plugin import pytest import pytest_check from craft_application import util +from craft_application.models import BuildInfo +from craft_providers.bases import BaseName from charmcraft import const, models, services from charmcraft.application.main import APP_METADATA +from charmcraft.models.project import BasesCharm SIMPLE_BUILD_BASE = models.charmcraft.Base(name="ubuntu", channel="22.04", architectures=["arm64"]) SIMPLE_MANIFEST = models.Manifest( charmcraft_started_at="1970-01-01T00:00:00+00:00", - bases=[models.Base(name="ubuntu", channel="22.04", architectures=["arm64"])], + bases=[SIMPLE_BUILD_BASE], ) MANIFEST_WITH_ATTRIBUTE = models.Manifest( **SIMPLE_MANIFEST.marshal(), @@ -70,7 +75,7 @@ def package_service(fake_path, simple_charm, service_factory, default_build_plan ), ], ) -def test_get_metadata(package_service, simple_charm, metadata): +def test_get_metadata(package_service, simple_charm: BasesCharm, metadata): package_service._project = simple_charm assert package_service.metadata == metadata @@ -135,10 +140,16 @@ def test_do_not_overwrite_metadata_yaml( # region Tests for getting bases for manifest.yaml @pytest.mark.parametrize( - ("bases", "expected"), + ("bases", "build_item", "expected"), [ ( [{"name": "ubuntu", "channel": "20.04"}], + BuildInfo( + platform=util.get_host_architecture(), + build_for=util.get_host_architecture(), + build_on=util.get_host_architecture(), + base=BaseName("ubuntu", "20.04"), + ), [ { "name": "ubuntu", @@ -149,27 +160,78 @@ def test_do_not_overwrite_metadata_yaml( ), ( [ - {"name": "ubuntu", "channel": "22.04", "architectures": ["all"]}, - {"name": "ubuntu", "channel": "20.04", "architectures": ["riscv64"]}, + { + "build-on": [ + {"name": "ubuntu", "channel": "22.04", "architectures": ["riscv64"]} + ], + "run-on": [ + {"name": "ubuntu", "channel": "22.04", "architectures": ["all"]}, + ], + }, ], + BuildInfo( + platform="riscv64", + build_for="riscv64", + build_on="riscv64", + base=BaseName("ubuntu", "22.04"), + ), [ {"name": "ubuntu", "channel": "22.04", "architectures": ["all"]}, - {"name": "ubuntu", "channel": "20.04", "architectures": ["riscv64"]}, ], ), + ( + [{"name": "centos", "channel": "7"}], + BuildInfo( + platform=util.get_host_architecture(), + build_on=util.get_host_architecture(), + build_for=util.get_host_architecture(), + base=BaseName("centos", "7"), + ), + [{"name": "centos", "channel": "7", "architectures": [util.get_host_architecture()]}], + ), + pytest.param( + [ + {"name": "centos", "channel": "7"}, + { + "build-on": [{"name": "ubuntu", "channel": "20.04"}], + "run-on": [{"name": "ubuntu", "channel": "20.04", "architectures": ["all"]}], + }, + { + "build-on": [ + {"name": "ubuntu", "channel": "22.04", "architectures": ["amd64", "arm64"]} + ], + "run-on": [{"name": "ubuntu", "channel": "22.04", "architectures": ["arm64"]}], + }, + ], + BuildInfo( + platform="amd64", + build_on="amd64", + build_for="arm64", + base=BaseName("ubuntu", "22.04"), + ), + [{"name": "ubuntu", "channel": "22.04", "architectures": ["arm64"]}], + id="cross-compile", + ), ], ) -def test_get_manifest_bases_from_bases(fake_path, package_service, bases, expected): +def test_get_manifest_bases_from_bases( + fake_path: pathlib.Path, + package_service: services.PackageService, + bases: list[dict[str, Any]], + build_item: BuildInfo, + expected: list[dict[str, Any]], +): charm = models.BasesCharm.parse_obj( { "name": "my-charm", "description": "", "summary": "", "type": "charm", - "bases": [{"build-on": bases, "run-on": bases}], + "bases": bases, } ) package_service._project = charm + package_service._build_plan = [build_item] assert package_service.get_manifest_bases() == [models.Base.parse_obj(b) for b in expected]