Skip to content

Commit

Permalink
fix(package): Limit the bases in manifest.yaml (#1795)
Browse files Browse the repository at this point in the history
For a `bases` charm, this was previously writing out all run bases
rather than the specific bases for this build.
  • Loading branch information
lengau authored Aug 7, 2024
1 parent affc209 commit 8b16cae
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 35 deletions.
15 changes: 13 additions & 2 deletions charmcraft/services/package.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
"""Service class for packing."""
from __future__ import annotations

import itertools
import json
import os
import pathlib
Expand Down Expand Up @@ -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()]
Expand Down
2 changes: 2 additions & 0 deletions charmcraft/utils/yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

from typing import Any

import pydantic
import yaml
from craft_cli import emit

Expand Down Expand Up @@ -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,
Expand Down
15 changes: 13 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]}],
}
],
)


Expand Down Expand Up @@ -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",
)
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/test_charm_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
24 changes: 20 additions & 4 deletions tests/unit/models/test_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
78 changes: 70 additions & 8 deletions tests/unit/services/test_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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]

Expand Down

0 comments on commit 8b16cae

Please sign in to comment.