Skip to content
51 changes: 39 additions & 12 deletions craft_platforms/charm/_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"""Charmcraft-specific platforms information."""

import itertools
from typing import Any, Collection, Dict, Iterable, List, Optional, Sequence
from typing import Any, Collection, Dict, Iterable, List, Optional, Sequence, Set

from craft_platforms import (
_architectures,
Expand Down Expand Up @@ -86,12 +86,36 @@ def _validate_base_definition( # noqa: PLR0912
"the incompatible 'build-for' entry for the platform."
),
)
# create a set of the bases defined in the build-on and build-for entries
bases = set()
for entry in [
*_utils.vectorize(platform.get("build-on", [platform_name])),
*_utils.vectorize(platform.get("build-for", [platform_name])),
]:
# Collect build-on entries, separating devel-series from non-devel.
# Devel-series build-on entries are allowed to differ from the build-for base,
# but mixing devel with any non-devel entries (even base-less ones that inherit
# a stable top-level base) in build-on is not allowed, for the same reason
# that mixing two different stable bases in build-on is not allowed.
has_devel_build_on = False
non_devel_build_on_bases: Set[Optional[str]] = set()
for entry in _utils.vectorize(platform.get("build-on", [platform_name])):
distro_base, _ = _architectures.parse_base_and_architecture(arch=entry)
if distro_base is not None and distro_base.series == "devel":
has_devel_build_on = True
else:
non_devel_build_on_bases.add(str(distro_base) if distro_base else None)

if has_devel_build_on and non_devel_build_on_bases:
raise _errors.InvalidMultiBaseError(
Comment thread
lengau marked this conversation as resolved.
message=(
f"Platform {platform_name!r} has mismatched bases in the 'build-on' "
"and 'build-for' entries."
),
resolution=(
"Use the same base for all 'build-on' and 'build-for' entries for "
"the platform."
),
)

# Combine the non-devel build-on bases with the build-for bases to check
# overall consistency.
bases: Set[Optional[str]] = set(non_devel_build_on_bases)
for entry in _utils.vectorize(platform.get("build-for", [platform_name])):
distro_base, _ = _architectures.parse_base_and_architecture(arch=entry)
bases.add(str(distro_base) if distro_base else None)

Expand Down Expand Up @@ -166,8 +190,9 @@ def _get_base_from_build_data(
if platform_base:
return platform_base

# build-on and build-for entries all have the same base, so we only
# need to check one of them
# When build-on and build-for entries share a common base, use it.
# If build-on has a devel base and build-for has a stable base, the product
# loop in get_platforms_charm_build_plan will use the per-entry build-on base.
if platform:
build_for_base, _ = _architectures.parse_base_and_architecture(
arch=_utils.vectorize(platform["build-for"])[0]
Expand Down Expand Up @@ -281,8 +306,8 @@ def get_platforms_charm_build_plan(
_utils.vectorize(platform.get("build-on", [platform_name])),
_utils.vectorize(platform.get("build-for", [platform_name])),
):
_, build_on_arch = _architectures.parse_base_and_architecture(
arch=build_on
build_on_base, build_on_arch = (
_architectures.parse_base_and_architecture(arch=build_on)
)
if build_on_arch == "all":
raise ValueError(
Expand All @@ -298,7 +323,9 @@ def get_platforms_charm_build_plan(
platform=platform_name,
build_on=build_on_arch,
build_for=build_for_arch,
build_base=distro_base,
build_base=build_on_base
if build_on_base is not None
else distro_base,
),
)

Expand Down
127 changes: 121 additions & 6 deletions tests/unit/charm/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,63 @@ def test_build_plans_success(
],
id="multi-base-long-multi-build-on",
),
pytest.param(
None,
None,
{
"noble": {
"build-on": ["devel:amd64"],
"build-for": ["ubuntu@24.04:amd64"],
},
},
[
craft_platforms.BuildInfo(
"noble",
craft_platforms.DebianArchitecture("amd64"),
craft_platforms.DebianArchitecture("amd64"),
craft_platforms.DistroBase("ubuntu", "devel"),
),
],
id="multi-base-devel-build-on",
),
pytest.param(
None,
None,
{
"noble": {
"build-on": ["ubuntu@devel:amd64"],
"build-for": ["ubuntu@24.04:amd64"],
},
},
[
craft_platforms.BuildInfo(
"noble",
craft_platforms.DebianArchitecture("amd64"),
craft_platforms.DebianArchitecture("amd64"),
craft_platforms.DistroBase("ubuntu", "devel"),
),
],
id="multi-base-ubuntu-at-devel-build-on",
),
pytest.param(
None,
None,
{
"noble": {
"build-on": ["devel:amd64"],
"build-for": ["ubuntu@24.04:all"],
},
},
[
craft_platforms.BuildInfo(
"noble",
craft_platforms.DebianArchitecture("amd64"),
"all",
craft_platforms.DistroBase("ubuntu", "devel"),
),
],
id="multi-base-devel-build-on-all",
),
],
)
def test_build_plans_in_depth(base, build_base, platforms, expected):
Expand Down Expand Up @@ -687,6 +744,32 @@ def test_build_plans_in_depth(base, build_base, platforms, expected):
"Use the same base for all 'build-on' and 'build-for' entries for the platform.",
id="platform-base-with-incompatible-build-on",
),
pytest.param(
None,
None,
{
"noble": {
"build-on": ["devel:amd64", "ubuntu@24.04:arm64"],
"build-for": ["ubuntu@24.04:amd64"],
},
},
r"Platform 'noble' has mismatched bases in the 'build-on' and 'build-for' entries.",
"Use the same base for all 'build-on' and 'build-for' entries for the platform.",
id="devel-and-stable-mixed-build-on",
),
pytest.param(
"ubuntu@24.04",
None,
{
"my-platform": {
"build-on": ["amd64", "devel:arm64"],
"build-for": ["amd64"],
},
},
r"Platform 'my-platform' has mismatched bases in the 'build-on' and 'build-for' entries.",
"Use the same base for all 'build-on' and 'build-for' entries for the platform.",
id="devel-and-base-less-mixed-build-on",
),
],
)
def test_build_plans_bad_base(base, build_base, platforms, error_msg, error_res):
Expand Down Expand Up @@ -740,6 +823,43 @@ def _is_valid_platform(platforms):
return True


def _is_valid_multi_base_platform_dict(p):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels complex enough that it deserves its own test

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this is a pretty complex helper function for a test.

"""Return True if the platform dict is consistent for multi-base builds.

A valid multi-base platform dict must satisfy:
- All ``build-on`` entries either share the same base as ``build-for``,
or are devel-series entries (exactly ``"devel"`` or ``"*@devel"``).
- ``build-on`` must not mix devel-series entries with any non-devel entries
(including base-less entries that would inherit a stable top-level base),
for the same reason that two different stable bases in ``build-on`` are
rejected.
"""
build_ons = p["build-on"] if isinstance(p["build-on"], list) else [p["build-on"]]
build_fors = (
p["build-for"] if isinstance(p["build-for"], list) else [p["build-for"]]
)
build_for_base = build_fors[0].partition(":")[0]

devel_build_ons = [
on
for on in build_ons
if on.partition(":")[0] == "devel" or on.partition(":")[0].endswith("@devel")
]
# build-on entries that are not devel-series (includes base-less entries)
non_devel_build_ons = [on for on in build_ons if on not in devel_build_ons]

# Mixing devel with any non-devel entries in build-on is not allowed.
if devel_build_ons and non_devel_build_ons:
return False

# All non-devel entries that carry an explicit base must match build-for.
return all(
on.partition(":")[0] == build_for_base
for on in non_devel_build_ons
if "@" in on.partition(":")[0]
)


@given(
base=strategies.real_distro_base(),
platforms=strategies.platform(
Expand Down Expand Up @@ -773,12 +893,7 @@ def test_fuzz_get_platforms_build_plan_single_base(
values=strategies.platform_dict(
build_ons=strategies.distro_series_arch_str(strategies.any_distro_base()),
build_fors=strategies.distro_series_arch_str(strategies.any_distro_base()),
).filter(
lambda p: (
{p["build-for"][0].partition(":")[0]}
== {on.partition(":")[0] for on in p["build-on"]}
)
),
).filter(_is_valid_multi_base_platform_dict),
),
)
def test_fuzz_get_platforms_build_plan_multi_base(
Expand Down