Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use craft-platforms for build plans #1894

Merged
merged 1 commit into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 24 additions & 32 deletions charmcraft/models/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@
cast,
)

import craft_platforms
import pydantic
import pydantic.v1
from craft_application import errors, models, util
from craft_application.util import safe_yaml_load
from craft_cli import CraftError, emit
from craft_platforms import charm
from craft_providers import bases
from pydantic import dataclasses
from typing_extensions import Self
Expand Down Expand Up @@ -345,38 +347,28 @@ def get_build_plan(self) -> list[models.BuildInfo]:

if self.platforms is None:
raise CraftError("Must define at least one platform.")
build_infos = []
for platform_name, platform in self.platforms.items():
if platform is None:
if platform_name not in const.SUPPORTED_ARCHITECTURES:
raise CraftError(
f"Invalid platform {platform_name}.",
details="A platform name must either be a valid architecture name or the "
"platform must specify one or more build-on and build-for architectures.",
)
build_infos.append(
models.BuildInfo(
platform_name,
build_on=platform_name,
build_for=platform_name,
base=base,
)
)
else:
# TODO: this should go to craft-platforms, so silence mypy for now.
for build_on in platform.build_on: # type: ignore[union-attr]
build_infos.extend(
[
models.BuildInfo(
platform_name,
build_on=str(build_on),
build_for=str(build_for),
base=base,
)
for build_for in platform.build_for # type: ignore[union-attr]
]
)
return build_infos
platforms = cast(
# https://github.com/canonical/craft-platforms/issues/43
craft_platforms.Platforms, # pyright: ignore[reportPrivateImportUsage]
{
name: (platform.marshal() if platform else None)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is just me rambling but it bothers me a bit that we have to do this 'round-trip' where we unmarshal the project, which includes validation (necessary) and possibly transformation (dangerous), and then re-marshal to pass to craft-platforms. This makes the assembling of the build plan a little different between the application and systems that just use craft-platforms (like launchpad), and makes us still need "all" of this code here.

I don't have any good suggestions to address this though; maybe the base BuildPlanner class should retain the original values of base, build-base and platforms somewhere?

for name, platform in self.platforms.items()
},
)
build_infos = charm.get_platforms_charm_build_plan(
base=self.base,
build_base=self.build_base,
platforms=platforms,
)
return [
models.BuildInfo(
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: maybe craft-application's BuildInfo should have a staticmethod to convert a craft-platform's BuildInfo

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Both of your comments here are things I'd like to address in canonical/craft-application#391 :-)

platform=info.platform,
build_on=str(info.build_on),
build_for=str(info.build_for),
base=base,
)
for info in build_infos
]


class CharmcraftProject(models.Project, metaclass=abc.ABCMeta):
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ dependencies = [
"craft-grammar>=2.0.0",
"craft-parts>=2.0.0",
"craft-providers>=2.0.0",
"craft-platforms~=0.1",
"craft-platforms~=0.3",
"craft-providers>=2.0.0",
"craft-store>=3.0.0",
"distro>=1.3.0",
Expand Down
Loading