Skip to content

Commit

Permalink
feat(remotebuild): filter build-plan with --platform or --build-for
Browse files Browse the repository at this point in the history
Allows using '--platform' or '--build-for' to filter entries in the
'architectures' or 'platforms' keys in a project file.

If the project file doesn't contain an 'architectures' or 'platforms'
entry, then '--platform' and '--build-for' define the architectures to
build for.

Signed-off-by: Callahan Kovacs <[email protected]>
  • Loading branch information
mr-cal committed Jan 23, 2025
1 parent 800823a commit 43ac71a
Show file tree
Hide file tree
Showing 13 changed files with 290 additions and 96 deletions.
19 changes: 13 additions & 6 deletions docs/explanation/remote-build.rst
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,20 @@ Current
``--platform`` and ``--build-for`` behave differently than they do for
:ref:`lifecycle commands<reference-lifecycle-commands>`.

``--platform`` or ``--build-for`` can only be provided when the ``platforms``
or ``architectures`` keywords are not defined in the project metadata
(`[12]`_).
If the project file contains a ``platforms`` or ``architectures`` key, then
``--platform`` or ``--build-for`` filter the build plan defined by that key.
For more information about build plans and filtering, see
:ref:`Build plans <build-plans>`.

These keywords are mutually exclusive and must be a comma-separated list of
debian architectures.
If the project file doesn't contain a ``platforms`` or ``architectures`` entry,
then ``--platform`` or ``--build-for`` define the architectures to build for.

It the project file doesn't contain a ``platforms`` or ``architectures`` entry
and ``--platform`` or ``--build-for`` are not provided, Snapcraft will build
for the host system's architecture.

These ``--platform`` and ``--build-for`` arguments are mutually exclusive and
must be a comma-separated list of debian architectures.

``core22`` snaps can only use ``--build-for``. ``core24`` and newer snaps
can use ``--platform`` or ``--build-for``.
Expand Down Expand Up @@ -180,6 +188,5 @@ Launchpad is not able to parse this notation (`[9]`_).
.. _`[8]`: https://bugs.launchpad.net/snapcraft/+bug/2007789
.. _`[9]`: https://bugs.launchpad.net/snapcraft/+bug/2042167
.. _`[10]`: https://github.com/canonical/snapcraft/issues/4885
.. _`[12]`: https://github.com/canonical/snapcraft/issues/4992
.. _`[13]`: https://github.com/canonical/snapcraft/issues/4996
.. _`[14]`: https://github.com/canonical/snapcraft/issues/4995
10 changes: 9 additions & 1 deletion snapcraft/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import craft_cli
import craft_parts
import craft_store
from craft_application import Application, AppMetadata, remote, util
from craft_application import Application, AppMetadata, launchpad, remote, util
from craft_application.commands import get_other_command_group
from craft_cli import emit
from craft_parts.plugins.plugins import PluginType
Expand Down Expand Up @@ -224,6 +224,14 @@ def _run_inner(self) -> int:
err.doc_slug = "/explanation/remote-build"
self._emit_error(err)
return_code = err.retcode
except launchpad.LaunchpadError as err:
self._emit_error(
craft_cli.errors.CraftError(
f"{err}", doc_slug="/explanation/remote-build"
),
cause=err,
)
return_code = 1

return return_code

Expand Down
180 changes: 114 additions & 66 deletions snapcraft/commands/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,17 @@
from pathlib import Path
from typing import Any, cast

import craft_application.errors
import lazr.restfulclient.errors
from craft_application import errors
from craft_application import errors, launchpad
from craft_application.application import filter_plan
from craft_application.commands import ExtensibleCommand
from craft_application.errors import RemoteBuildError
from craft_application.launchpad.models import Build, BuildState
from craft_application.remote.utils import get_build_id
from craft_application.util import humanize_list
from craft_cli import emit
from craft_platforms import DebianArchitecture
from overrides import overrides

from snapcraft import models
Expand All @@ -57,9 +60,16 @@ class RemoteBuildCommand(ExtensibleCommand):
architecture are retrieved and will be available in the
local filesystem.
If not specified in the snapcraft.yaml file, the list of
architectures to build can be set using the --platforms option.
If both are specified, an error will occur.
If the project contains a ``platforms`` or ``architectures`` key,
then project's build plan is used. The build plan can be filtered
using the ``--platform`` or ``--build-for`` arguments.
If the project doesn't contain a ``platforms`` or ``architectures`` key,
then the architectures to build for are defined by the ``--platform`` or
``--build-for`` arguments.
If there are no architectures defined in the project or as arguments,
then the default behavior is to build for the host architecture.
Interrupted remote builds can be resumed using the --recover
option, followed by the build number informed when the remote
Expand Down Expand Up @@ -154,56 +164,17 @@ def _validate(self, parsed_args: argparse.Namespace) -> None:
retcode=os.EX_NOPERM,
)

build_fors = parsed_args.remote_build_build_fors or None
platforms = parsed_args.remote_build_platforms or None
parameter = "--build-for" if build_fors else "--platform" if platforms else None
keyword = (
"architectures"
if self.project._architectures_in_yaml
else "platforms" if self.project.platforms else None
)

if keyword and parameter:
if (
parsed_args.remote_build_platforms
and self.project.get_effective_base() == "core22"
):
raise errors.RemoteBuildError(
f"{parameter!r} cannot be used when {keyword!r} is in the snapcraft.yaml.",
resolution=f"Remove {parameter!r} from the command line or remove {keyword!r} in the snapcraft.yaml.",
"'--platform' cannot be used for core22 snaps.",
resolution="Use '--build-for' instead.",
doc_slug="/explanation/remote-build.html",
retcode=os.EX_CONFIG,
)

if platforms:
if self.project.get_effective_base() == "core22":
raise errors.RemoteBuildError(
"'--platform' cannot be used for core22 snaps.",
resolution="Use '--build-for' instead.",
doc_slug="/explanation/remote-build.html",
retcode=os.EX_CONFIG,
)
for platform in platforms:
if platform not in SUPPORTED_ARCHS:
raise errors.RemoteBuildError(
f"Unsupported platform {platform!r}.",
resolution=(
"Use a supported debian architecture. Supported "
f"architectures are: {humanize_list(SUPPORTED_ARCHS, 'and')}"
),
doc_slug="/explanation/remote-build.html",
retcode=os.EX_CONFIG,
)

if build_fors:
for build_for in build_fors:
if build_for not in SUPPORTED_ARCHS:
raise errors.RemoteBuildError(
f"Unsupported build-for architecture {build_for!r}.",
resolution=(
"Use a supported debian architecture. Supported "
f"architectures are: {humanize_list(SUPPORTED_ARCHS, 'and')}"
),
doc_slug="/explanation/remote-build.html",
retcode=os.EX_CONFIG,
)

self._validate_single_artifact_per_build_on()

def _validate_single_artifact_per_build_on(self) -> None:
Expand Down Expand Up @@ -274,14 +245,10 @@ def _run( # noqa: PLR0915 [too-many-statements]
emit.trace(f"Project directory: {project_dir}")
self._validate(parsed_args)

if parsed_args.remote_build_build_fors:
architectures = parsed_args.remote_build_build_fors
elif parsed_args.remote_build_platforms:
architectures = parsed_args.remote_build_platforms
else:
architectures = self._get_project_build_fors()

emit.debug(f"Architectures to build for: {architectures}")
archs = self._get_archs(
build_fors=parsed_args.remote_build_build_fors,
platforms=parsed_args.remote_build_platforms,
)

if parsed_args.launchpad_timeout:
emit.debug(f"Setting timeout to {parsed_args.launchpad_timeout} seconds")
Expand All @@ -296,10 +263,8 @@ def _run( # noqa: PLR0915 [too-many-statements]
"Starting new build. It may take a while to upload large projects."
)
try:
builds = builder.start_builds(
project_dir, architectures=architectures or None
)
except RemoteBuildError:
builds = builder.start_builds(project_dir, architectures=archs)
except (RemoteBuildError, launchpad.LaunchpadError):
emit.progress("Starting build failed.", permanent=True)
emit.progress("Cleaning up")
builder.cleanup()
Expand Down Expand Up @@ -407,12 +372,95 @@ def _monitor_and_complete( # noqa: PLR0912, PLR0915
)
return return_code

def _get_project_build_fors(self) -> list[str]:
"""Get a unique list of build-for architectures from the project.
def _get_archs(self, build_fors: list[str], platforms: list[str]) -> list[str]:
"""Get the architectures to build for.
If the project contains a ``platforms`` or ``architectures`` key, then project's
build plan is used to determine the architectures to build for. The build plan
can be filtered using the ``--platform`` or ``--build-for`` arguments.
If the project doesn't contain a ``platforms`` or ``architectures`` key, then
the architectures to build for are defined by the ``--platform`` or
``--build-for`` arguments.
If there are no architectures defined in the project or as arguments, then the
default behavior is to build for the host architecture.
:param build_fors: A list of build-for entries.
:param platforms: A list of platforms.
:raises EmptyBuildPlanError: If the build plan is filtered to an empty list.
:raises RemoteBuildError: If an unsupported architecture is provided.
:returns: A list of architectures.
"""
build_fors = set({build_info.build_for for build_info in self.build_plan})
emit.debug(f"Parsed build-for architectures from build plan: {build_fors}")
archs: list[str] = []
# core22 projects with an `architectures` key will have a corresponding `platforms`
# key when the project is unmarshalled
if self.project.platforms or self.project._architectures_in_yaml:
# if the project has platforms, then the `--platforms` and `--build-for` arguments act as filters
if build_fors:
emit.debug("Filtering the build plan using the '--build-for' argument.")
for build_for in build_fors:
filtered_build_plan = filter_plan(
self.build_plan,
platform=None,
build_for=build_for,
host_arch=None,
)
archs.extend([info.build_for for info in filtered_build_plan])
if not archs:
raise craft_application.errors.EmptyBuildPlanError()
elif platforms:
emit.debug("Filtering the build plan using the '--platforms' argument.")
for platform in platforms:
filtered_build_plan = filter_plan(
self.build_plan,
platform=platform,
build_for=None,
host_arch=None,
)
archs.extend([info.build_for for info in filtered_build_plan])
if not archs:
raise craft_application.errors.EmptyBuildPlanError()
else:
emit.debug("Using the project's build plan")
archs = [build_info.build_for for build_info in self.build_plan]
# No platforms in the project means '--build-for' and '--platforms' no longer act as filters.
# Instead, they define the architectures to build for.
elif build_fors:
emit.debug("Using '--build-for' as the list of architectures to build for")
_validate_archs(build_fors, "build-for architecture")
archs = build_fors
elif platforms:
emit.debug("Using '--platforms' as the list of architectures to build for")
_validate_archs(platforms, "platform")
archs = platforms
# default is to build for the host architecture
else:
archs = [str(DebianArchitecture.from_host())]
emit.debug(
f"Using host architecture {archs[0]} because no architectures were "
"defined in the project or as a command-line argument."
)

emit.debug(f"Architectures to build for: {humanize_list(archs, 'and')}")
return archs

return list(build_fors)

def _validate_archs(archs: list[str], key_name: str) -> None:
"""Validate that a list of architectures are valid.
:raises RemoteBuildError: If an unsupported architecture is provided.
"""
for arch in archs:
if arch not in SUPPORTED_ARCHS:
raise errors.RemoteBuildError(
f"Unsupported {key_name} {arch!r}.",
resolution=(
"Use a supported debian architecture. Supported "
f"architectures are: {humanize_list(SUPPORTED_ARCHS, 'and')}"
),
doc_slug="/explanation/remote-build.html",
retcode=os.EX_CONFIG,
)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--build-for amd64
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test-snap-architectures-filtered-core22_1.0_amd64.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
name: test-snap-architectures-filtered-core22
base: core22
version: "1.0"
summary: Test snap for remote build
description: Test snap for remote build

grade: stable
confinement: strict

architectures:
- build-on: [amd64]
build-for: [amd64]
- build-on: [riscv64]
build-for: [riscv64]

parts:
my-part:
plugin: nil
1 change: 1 addition & 0 deletions tests/spread/core22/remote-build/task.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ environment:
SNAP/all: all
SNAP/no_architectures: no-architectures
SNAP/architectures: architectures
SNAP/architectures_filtered: architectures-filtered
SNAP/build_for_arg: build-for-arg
SNAPCRAFT_REMOTE_BUILD_STRATEGY: disable-fallback
CREDENTIALS_FILE: "$HOME/.local/share/snapcraft/launchpad-credentials"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--platform amd64
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test-snap-platforms-filtered-core24_1.0_amd64.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: test-snap-platforms-filtered-core24
base: core24
version: "1.0"
summary: Test snap for remote build
description: Test snap for remote build

grade: stable
confinement: strict

platforms:
amd64:
build-on: [amd64]
build-for: [amd64]
riscv64:
build-on: [riscv64]
build-for: [riscv64]

parts:
my-part:
plugin: nil
8 changes: 6 additions & 2 deletions tests/spread/core24/remote-build/task.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ environment:
SNAP/platforms: platforms
SNAP/no_platforms: no-platforms
SNAP/platform_arg: platform-arg
SNAP/platforms_filtered: platforms-filtered
SNAP/build_for_arg: build-for-arg
CREDENTIALS_FILE: "$HOME/.local/share/snapcraft/launchpad-credentials"
CREDENTIALS_FILE/new_credentials: "$HOME/.local/share/snapcraft/launchpad-credentials"
Expand All @@ -38,8 +39,11 @@ prepare: |
restore: |
cd "./snaps/$SNAP"
rm -f ./*.snap ./*.txt
rm -rf snap .git
# remove snaps and log files
rm -f ./*.snap ./snapcraft-*.txt
# remove the temporary git repository
rm -rf .git
execute: |
cd "./snaps/$SNAP"
Expand Down
Loading

0 comments on commit 43ac71a

Please sign in to comment.