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

feat!: hybrid commands! #677

Open
wants to merge 2 commits into
base: feature/hybrid-commands
Choose a base branch
from
Open
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
13 changes: 6 additions & 7 deletions craft_application/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from dataclasses import dataclass, field
from functools import cached_property
from importlib import metadata
from typing import TYPE_CHECKING, Any, cast, final
from typing import TYPE_CHECKING, Any, Literal, cast, final

import craft_cli
import craft_parts
Expand Down Expand Up @@ -145,7 +145,7 @@ def __init__(
# Whether the command execution should use the fetch-service
self._enable_fetch_service = False
# The kind of sessions that the fetch-service service should create
self._fetch_service_policy = "strict"
self._fetch_service_policy: Literal["strict", "permissive"] = "strict"

@final
def _load_plugins(self) -> None:
Expand Down Expand Up @@ -324,10 +324,6 @@ def _configure_services(self, provider_name: str | None) -> None:
work_dir=self._work_dir,
provider_name=provider_name,
)
self.services.update_kwargs(
"fetch",
session_policy=self._fetch_service_policy,
)

def get_project(
self,
Expand Down Expand Up @@ -377,6 +373,9 @@ def run_managed(self, platform: str | None, build_for: str | None) -> None:
if not plan:
raise errors.EmptyBuildPlanError

if self._enable_fetch_service:
self.services.get("fetch").set_policy(self._fetch_service_policy)

extra_args: dict[str, Any] = {}
for build_info in plan:
env = {
Expand Down Expand Up @@ -541,7 +540,7 @@ def _pre_run(self, dispatcher: craft_cli.Dispatcher) -> None:
fetch_service_policy: str | None = getattr(args, "fetch_service_policy", None)
if fetch_service_policy:
self._enable_fetch_service = True
self._fetch_service_policy = fetch_service_policy
self._fetch_service_policy = fetch_service_policy # type: ignore[reportAttributeAccessIssue]

def get_arg_or_config(self, parsed_args: argparse.Namespace, item: str) -> Any: # noqa: ANN401
"""Get a configuration option that could be overridden by a command argument.
Expand Down
97 changes: 41 additions & 56 deletions craft_application/commands/lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from __future__ import annotations

import argparse
import os
import pathlib
import subprocess
import textwrap
Expand All @@ -26,6 +25,7 @@
from craft_parts.features import Features
from typing_extensions import override

from craft_application import util
from craft_application.commands import base


Expand Down Expand Up @@ -77,32 +77,27 @@ def _fill_parser(self, parser: argparse.ArgumentParser) -> None:
help="Build in a LXD container.",
)

@override
def get_managed_cmd(self, parsed_args: argparse.Namespace) -> list[str]:
cmd = super().get_managed_cmd(parsed_args)

cmd.extend(parsed_args.parts)

return cmd

@override
def provider_name(self, parsed_args: argparse.Namespace) -> str | None:
return "lxd" if parsed_args.use_lxd else None

@override
def run_managed(self, parsed_args: argparse.Namespace) -> bool:
"""Return whether the command should run in managed mode or not.
def _run_manager_for_build_plan(self, fetch_service_policy: str | None) -> None:
"""Run this command in managed mode, iterating over the generated build plan."""
provider = self._services.get("provider")
for build in self._services.get("build_plan").plan():
provider.run_managed(build, bool(fetch_service_policy))

The command will run in managed mode unless the `--destructive-mode` flag
is passed OR `CRAFT_BUILD_ENVIRONMENT` is set to `host`.
"""
def _use_provider(self, parsed_args: argparse.Namespace) -> bool:
"""Determine whether to build in a managed provider."""
if util.is_managed_mode():
return False
if parsed_args.destructive_mode:
emit.debug(
"Not running managed mode because `--destructive-mode` was passed"
)
return False

build_env = os.getenv("CRAFT_BUILD_ENVIRONMENT")
build_env = self._services.get("config").get("build_environment")
if build_env and build_env.lower().strip() == "host":
emit.debug(
f"Not running managed mode because CRAFT_BUILD_ENVIRONMENT={build_env}"
Expand Down Expand Up @@ -156,23 +151,6 @@ def _fill_parser(self, parser: argparse.ArgumentParser) -> None:
help="Set architecture to build for",
)

@override
def get_managed_cmd(self, parsed_args: argparse.Namespace) -> list[str]:
"""Get the command to run in managed mode.
:param parsed_args: The parsed arguments used.
:returns: A list of strings ready to be passed into a craft-providers executor.
:raises: RuntimeError if this command is not supposed to run managed.
"""
cmd = super().get_managed_cmd(parsed_args)

if getattr(parsed_args, "shell", False):
cmd.append("--shell")
if getattr(parsed_args, "shell_after", False):
cmd.append("--shell-after")

return cmd

@override
def _run(
self,
Expand All @@ -183,6 +161,15 @@ def _run(
"""Run a lifecycle step command."""
super()._run(parsed_args)

if self._use_provider(parsed_args):
fetch_service_policy = getattr(parsed_args, "fetch_service_policy", None)
if fetch_service_policy:
self._services.get("fetch").set_policy(
fetch_service_policy # type: ignore[reportArgumentType]
)
self._run_manager_for_build_plan(fetch_service_policy)
return

shell = getattr(parsed_args, "shell", False)
shell_after = getattr(parsed_args, "shell_after", False)
debug = getattr(parsed_args, "debug", False)
Expand Down Expand Up @@ -363,14 +350,12 @@ def _fill_parser(self, parser: argparse.ArgumentParser) -> None:
dest="fetch_service_policy",
)

@override
def _run(
def _run_real(
self,
parsed_args: argparse.Namespace,
step_name: str | None = None,
**kwargs: Any,
) -> None:
"""Run the pack command."""
"""Run the actual pack command."""
if step_name not in ("pack", None):
raise RuntimeError(f"Step name {step_name} passed to pack command.")

Expand Down Expand Up @@ -414,6 +399,17 @@ def _run(
if shell_after:
_launch_shell()

@override
def _run(
self,
parsed_args: argparse.Namespace,
step_name: str | None = None,
**kwargs: Any,
) -> None:
if self._use_provider(parsed_args=parsed_args):
return super()._run(parsed_args=parsed_args, step_name=step_name)
return self._run_real(parsed_args=parsed_args, step_name=step_name)


class CleanCommand(_BaseLifecycleCommand):
"""Command to remove part assets."""
Expand Down Expand Up @@ -462,26 +458,15 @@ def _run(
"""
super()._run(parsed_args)

if not super().run_managed(parsed_args) or not self._should_clean_instances(
parsed_args
):
self._services.lifecycle.clean(parsed_args.parts)
else:
self._services.provider.clean_instances()

@override
def run_managed(self, parsed_args: argparse.Namespace) -> bool:
"""Return whether the command should run in managed mode or not.
build_managed = self._use_provider(parsed_args)
clean_instances = self._should_clean_instances(parsed_args)

Clean will run in managed mode unless:
- the `--destructive-mode` flag is provided OR
- `CRAFT_BUILD_ENVIRONMENT` is set to `host` OR
- a list of specific parts to clean is provided
"""
if not super().run_managed(parsed_args):
return False

return not self._should_clean_instances(parsed_args)
if build_managed and clean_instances:
self._services.provider.clean_instances()
elif build_managed:
self._run_manager_for_build_plan(fetch_service_policy=None)
else:
self._services.get("lifecycle").clean(parsed_args.parts)

@staticmethod
def _should_clean_instances(parsed_args: argparse.Namespace) -> bool:
Expand Down
16 changes: 13 additions & 3 deletions craft_application/services/fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

from __future__ import annotations

import atexit
import json
import pathlib
import subprocess
Expand Down Expand Up @@ -64,8 +65,6 @@ def __init__(
self,
app: AppMetadata,
services: service_factory.ServiceFactory,
*,
session_policy: str,
) -> None:
"""Create a new FetchService.
Expand All @@ -75,7 +74,7 @@ def __init__(
super().__init__(app, services)
self._fetch_process = None
self._session_data = None
self._session_policy = session_policy
self._session_policy: str = "strict" # Default to strict policy.
self._instance = None

@override
Expand All @@ -96,6 +95,17 @@ def setup(self) -> None:

self._fetch_process = fetch.start_service()

# When we exit the application, we'll shut down the fetch service.
atexit.register(self.shutdown, force=True)

def set_policy(self, policy: typing.Literal["strict", "permissive"]) -> None:
"""Set the policy for the fetch service."""
if self._fetch_process is not None:
raise RuntimeError(
"Attempted to set fetch service policy after starting the process."
)
self._session_policy = policy

def create_session(self, instance: craft_providers.Executor) -> dict[str, str]:
"""Create a new session.
Expand Down
55 changes: 52 additions & 3 deletions craft_application/services/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@
import os
import pathlib
import pkgutil
import subprocess
import sys
import urllib.request
from collections.abc import Generator, Iterable
from collections.abc import Generator, Iterable, Sequence
from pathlib import Path
from typing import TYPE_CHECKING

import craft_platforms
import craft_providers
from craft_cli import CraftError, emit
from craft_providers import bases
from craft_providers.actions.snap_installer import Snap
Expand All @@ -40,8 +42,6 @@
from craft_application.util import platforms, snap_config

if TYPE_CHECKING: # pragma: no cover
import craft_providers

from craft_application.application import AppMetadata
from craft_application.services import ServiceFactory

Expand Down Expand Up @@ -371,3 +371,52 @@ def _clean_instance(
instance_name = self._get_instance_name(work_dir, info, project_name)
emit.debug(f"Cleaning instance {instance_name}")
provider.clean_project_environments(instance_name=instance_name)

def run_managed(
self,
build_info: craft_platforms.BuildInfo,
enable_fetch_service: bool, # noqa: FBT001
command: Sequence[str] = (),
) -> None:
"""Create a managed instance and run a command in it.
:param build_info: The BuildInfo that defines what instance to use.
:enable_fetch_service: Whether to enable the fetch service.
:command: The command to run. Defaults to the current command.
"""
if not command:
command = [self._app.name, *sys.argv[1:]]
env = {
"CRAFT_PLATFORM": build_info.platform,
"CRAFT_VERBOSITY_LEVEL": emit.get_mode().name,
}
emit.debug(
f"Running managed {self._app.name} in managed {build_info.build_base} instance for platform {build_info.platform!r}"
)

with self.instance(
build_info=build_info,
work_dir=self._work_dir,
clean_existing=enable_fetch_service,
) as instance:
if enable_fetch_service:
session_env = self._services.get("fetch").create_session(instance)
env.update(session_env)

emit.debug(f"Running in instance: {command}")
try:
with emit.pause():
# Pyright doesn't fully understand craft_providers's CompletedProcess.
instance.execute_run( # pyright: ignore[reportUnknownMemberType,reportUnknownVariableType]
list(command),
cwd=self._app.managed_instance_project_path,
check=True,
env=env,
)
except subprocess.CalledProcessError as exc:
raise craft_providers.ProviderError(
f"Failed to run {self._app.name} in instance"
) from exc
finally:
if enable_fetch_service:
self._services.get("fetch").teardown_session()
2 changes: 1 addition & 1 deletion snap/snapcraft.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,4 @@ parts:
cp -r partitioncraft "${CRAFT_PART_INSTALL}/lib/python3.12/site-packages/"
rm "${CRAFT_PART_INSTALL}/bin/python"
ln -s ../usr/bin/python3 "${CRAFT_PART_INSTALL}/bin/python"
ln -s ../usr/bin/python3.12 "${CRAFT_PART_INSTALL}/bin/python"
4 changes: 3 additions & 1 deletion spread.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ backends:
halt-timeout: 3h
systems:
- ubuntu-24.04-64:
workers: 2
workers: 4
memory: 2G
storage: 10G
# lxd: # Disabled due to https://github.com/canonical/spread/issues/215
# systems:
# - ubuntu-24.04
Expand Down
12 changes: 8 additions & 4 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,12 @@ def get_partitions_for(
return ["default", *{platform, build_for}]
return None

def get_partitions(self) -> list[str] | None:
"""Make this flexible for whether we have partitions or not."""
if craft_parts.Features().enable_partitions:
return ["default", "a"]
return None

return FakeProjectService


Expand Down Expand Up @@ -432,10 +438,8 @@ def fake_services(
factory.update_kwargs(
"lifecycle", work_dir=tmp_path, cache_dir=tmp_path / "cache", build_plan=[]
)
factory.update_kwargs(
"project",
project_dir=project_path,
)
factory.update_kwargs("project", project_dir=project_path)
factory.update_kwargs("provider", work_dir=project_path)
platform = (
request.getfixturevalue("fake_platform")
if "fake_platform" in request.fixturenames
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/services/test_fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ def app_service(app_metadata, fake_services, fake_project):
fetch_service = services.FetchService(
app_metadata,
fake_services,
session_policy="permissive",
)
fetch_service.set_policy("permissive")
yield fetch_service
fetch_service.shutdown(force=True)

Expand Down
Loading
Loading