Skip to content

chore: mark redundant functions as deprecated. #650

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
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
2 changes: 2 additions & 0 deletions craft_application/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
ServiceFactory,
)
from craft_application._config import ConfigModel
from craft_application.util import is_managed_mode

try:
from ._version import __version__
Expand All @@ -54,4 +55,5 @@
"PackageService",
"ProviderService",
"ServiceFactory",
"is_managed_mode",
]
24 changes: 16 additions & 8 deletions craft_application/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import subprocess
import sys
import traceback
import warnings
from collections.abc import Iterable, Sequence
from dataclasses import dataclass, field
from functools import cached_property
Expand All @@ -33,6 +34,7 @@

import craft_cli
import craft_parts
import craft_platforms
import craft_providers
from craft_parts.plugins.plugins import PluginType
from platformdirs import user_cache_path
Expand Down Expand Up @@ -152,7 +154,7 @@ def __init__(
# This may be overridden by specific application implementations.
self.project_dir = pathlib.Path.cwd()

if self.is_managed():
if util.is_managed_mode():
self._work_dir = pathlib.Path("/root")
else:
self._work_dir = pathlib.Path.cwd()
Expand Down Expand Up @@ -280,7 +282,7 @@ def _merge_defaults(
@property
def log_path(self) -> pathlib.Path | None:
"""Get the path to this process's log file, if any."""
if self.is_managed():
if util.is_managed_mode():
return util.get_managed_logpath(self.app)
return None

Expand Down Expand Up @@ -381,7 +383,7 @@ def get_project(
with project_path.open() as file:
yaml_data = util.safe_yaml_load(file)

host_arch = util.get_host_architecture()
host_arch = craft_platforms.DebianArchitecture.from_host().value
build_planner = self.app.BuildPlannerClass.from_yaml_data(
yaml_data, project_path
)
Expand Down Expand Up @@ -432,7 +434,13 @@ def project(self) -> models.Project:

def is_managed(self) -> bool:
"""Shortcut to tell whether we're running in managed mode."""
return self.services.get_class("provider").is_managed()
warnings.warn(
DeprecationWarning(
"app.is_managed is deprecated. Use craft_application.is_managed_mode() instead."
),
stacklevel=2,
)
return util.is_managed_mode()

def run_managed(self, platform: str | None, build_for: str | None) -> None:
"""Run the application in a managed instance."""
Expand Down Expand Up @@ -604,7 +612,7 @@ def _pre_run(self, dispatcher: craft_cli.Dispatcher) -> None:
# Some commands might have a project_dir parameter. Those commands and
# only those commands should get a project directory, but only when
# not managed.
if self.is_managed():
if util.is_managed_mode():
self.project_dir = pathlib.Path("/root/project")
elif project_dir := getattr(args, "project_dir", None):
self.project_dir = pathlib.Path(project_dir).expanduser().resolve()
Expand Down Expand Up @@ -669,7 +677,7 @@ def _run_inner(self) -> int:
# command runs in the outer instance
craft_cli.emit.debug(f"Running {self.app.name} {command.name} on host")
return_code = dispatcher.run() or os.EX_OK
elif not self.is_managed():
elif not util.is_managed_mode():
# command runs in inner instance, but this is the outer instance
self.run_managed(platform, build_for)
return_code = os.EX_OK
Expand Down Expand Up @@ -735,7 +743,7 @@ def _emit_error(
error.__cause__ = cause

# Do not report the internal logpath if running inside an instance
if self.is_managed():
if util.is_managed_mode():
error.logpath_report = False

craft_cli.emit.error(error)
Expand Down Expand Up @@ -843,7 +851,7 @@ def _set_global_environment(self, info: craft_parts.ProjectInfo) -> None:
def _render_secrets(self, yaml_data: dict[str, Any]) -> None:
"""Render build-secrets, in-place."""
secret_values = secrets.render_secrets(
yaml_data, managed_mode=self.is_managed()
yaml_data, managed_mode=util.is_managed_mode()
)

num_secrets = len(secret_values.secret_strings)
Expand Down
4 changes: 2 additions & 2 deletions craft_application/services/fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def setup(self) -> None:
"""Start the fetch-service process with proper arguments."""
super().setup()

if not self._services.get_class("provider").is_managed():
if not util.is_managed_mode():
# Early fail if the fetch-service is not installed.
fetch.verify_installed()

Expand Down Expand Up @@ -153,7 +153,7 @@ def create_project_manifest(self, artifacts: list[pathlib.Path]) -> None:

Only supports a single generated artifact, and only in managed runs.
"""
if not self._services.ProviderClass.is_managed():
if not util.is_managed_mode():
emit.debug("Unable to generate the project manifest on the host.")
return

Expand Down
11 changes: 9 additions & 2 deletions craft_application/services/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import pkgutil
import sys
import urllib.request
import warnings
from collections.abc import Generator, Iterable
from pathlib import Path
from typing import TYPE_CHECKING
Expand Down Expand Up @@ -85,7 +86,13 @@ def __init__(
@classmethod
def is_managed(cls) -> bool:
"""Determine whether we're running in managed mode."""
return os.getenv(cls.managed_mode_env_var) == "1"
warnings.warn(
DeprecationWarning(
"ProviderService.is_managed() is deprecated. Use craft_application.is_managed_mode() instead."
),
stacklevel=2,
)
return util.is_managed_mode()

def setup(self) -> None:
"""Application-specific service setup."""
Expand Down Expand Up @@ -223,7 +230,7 @@ def get_provider(self, name: str | None = None) -> craft_providers.Provider:
if self._provider is not None:
return self._provider

if self.is_managed():
if util.is_managed_mode():
raise CraftError("Cannot nest managed environments.")

# (1) use provider specified in the function argument,
Expand Down
27 changes: 20 additions & 7 deletions craft_application/util/platforms.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@
import functools
import os
import platform
import warnings
from typing import Final

from craft_parts.utils import os_utils
import craft_platforms
import distro
from craft_providers import bases

from .string import strtobool
Expand All @@ -33,8 +35,13 @@
@functools.lru_cache(maxsize=1)
def get_host_architecture() -> str:
"""Get host architecture in deb format."""
machine = platform.machine()
return _ARCH_TRANSLATIONS_PLATFORM_TO_DEB.get(machine, machine)
warnings.warn(
DeprecationWarning(
"get_host_architecture() is deprecated. Use craft_platforms.DebianArchitecture.from_host()"
),
stacklevel=2,
)
return craft_platforms.DebianArchitecture.from_host().value


def convert_architecture_deb_to_platform(arch: str) -> str:
Expand All @@ -54,10 +61,16 @@ def is_valid_architecture(arch: str) -> bool:
@functools.lru_cache(maxsize=1)
def get_host_base() -> bases.BaseName:
"""Get the craft-providers base for the running host."""
release = os_utils.OsRelease()
os_id = release.id()
version_id = release.version_id()
return bases.BaseName(os_id, version_id)
warnings.warn(
PendingDeprecationWarning(
"get_host_base() is pending deprecation. Use craft_platforms.DistroBase.from_linux_distribution(distro.LinuxDistribution())"
),
stacklevel=2,
)
distro_base = craft_platforms.DistroBase.from_linux_distribution(
distro.LinuxDistribution(include_lsb=False, include_uname=False)
)
return bases.BaseName(distro_base.distribution, distro_base.series)


def get_hostname(hostname: str | None = None) -> str:
Expand Down
2 changes: 1 addition & 1 deletion craft_application/util/string.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def strtobool(value: str) -> bool:
value = value.strip().lower()
if value in {"true", "t", "yes", "y", "on", "1"}:
return True
if value in {"false", "f", "no", "n", "off", "0"}:
if value in {"false", "f", "no", "n", "off", "0", "", "none"}:
return False
raise ValueError(f"Invalid boolean value: {value}")

Expand Down
8 changes: 8 additions & 0 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,15 @@

extensions = [
"canonical_sphinx",
"sphinx.ext.intersphinx",
]

intersphinx_mapping = {
"craft-platforms": (
"https://canonical-craft-platforms.readthedocs-hosted.com/en/latest",
None,
)
}
# endregion

# region Options for extensions
Expand Down
23 changes: 22 additions & 1 deletion docs/reference/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,27 @@
Changelog
*********

4.10.0 (YYYY-Mon-DD)
--------------------

Application
===========

- Deprecate ``Application.is_managed()`` in favour of :func:`is_managed_mode()`

Services
========

- Deprecate ``ProviderService.is_managed()`` in favour of :func:`is_managed_mode()`

Utilities
=========

- Deprecate ``get_host_architecture()`` in favour of
:external+craft-platforms:meth:`craft_platforms.DebianArchitecture.from_host()`
- Warn that ``get_host_base()`` is pending deprecation in favour of
:external+craft-platforms:meth:`craft_platforms.DistroBase.from_linux_distribution()`

4.9.1 (2025-Feb-12)
-------------------

Expand All @@ -23,7 +44,7 @@ Application
===========

- Add a feature to allow `Python plugins
https://packaging.python.org/en/latest/guides/creating-and-discovering-plugins/>`_
<https://packaging.python.org/en/latest/guides/creating-and-discovering-plugins/>`_
to extend or modify the behaviour of applications that use craft-application as a
framework. The plugin packages must be installed in the same virtual environment
as the application.
Expand Down
23 changes: 15 additions & 8 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@
import pathlib
import shutil
import subprocess
import warnings
from dataclasses import dataclass
from importlib import metadata
from typing import TYPE_CHECKING, Any
from unittest.mock import Mock

import craft_parts
import craft_platforms
import jinja2
import pydantic
import pytest
Expand All @@ -45,8 +47,12 @@

def _create_fake_build_plan(num_infos: int = 1) -> list[models.BuildInfo]:
"""Create a build plan that is able to execute on the running system."""
arch = util.get_host_architecture()
base = util.get_host_base()
with warnings.catch_warnings():
warnings.simplefilter("ignore", DeprecationWarning)
arch = util.get_host_architecture()
with warnings.catch_warnings():
warnings.simplefilter("ignore", PendingDeprecationWarning)
base = util.get_host_base()
return [models.BuildInfo("foo", arch, arch, base)] * num_infos


Expand Down Expand Up @@ -133,7 +139,7 @@ def app_metadata_docs(features) -> craft_application.AppMetadata:

@pytest.fixture
def fake_project() -> models.Project:
arch = util.get_host_architecture()
arch = craft_platforms.DebianArchitecture.from_host().value
return models.Project(
name="full-project", # pyright: ignore[reportArgumentType]
title="A fully-defined project", # pyright: ignore[reportArgumentType]
Expand Down Expand Up @@ -161,7 +167,7 @@ def fake_build_plan(request) -> list[models.BuildInfo]:
@pytest.fixture
def full_build_plan(mocker) -> list[models.BuildInfo]:
"""A big build plan with multiple bases and build-for targets."""
host_arch = util.get_host_architecture()
host_arch = craft_platforms.DebianArchitecture.from_host().value
build_plan = []
for release in ("20.04", "22.04", "24.04"):
build_plan.extend(
Expand Down Expand Up @@ -280,6 +286,8 @@ def pack(
def metadata(self) -> models.BaseMetadata:
return models.BaseMetadata()

services.ServiceFactory.register("package", FakePackageService)

return FakePackageService


Expand All @@ -305,6 +313,7 @@ def __init__(
**kwargs,
)

services.ServiceFactory.register("lifecycle", FakeLifecycleService)
return FakeLifecycleService


Expand All @@ -314,6 +323,7 @@ class FakeInitService(services.InitService):
def _get_loader(self, template_dir: pathlib.Path) -> jinja2.BaseLoader:
return FileSystemLoader(tmp_path / "templates" / template_dir)

services.ServiceFactory.register("init", FakeInitService)
return FakeInitService


Expand All @@ -324,6 +334,7 @@ class FakeRemoteBuild(services.RemoteBuildService):
def _get_lp_client(self) -> launchpad.Launchpad:
return Mock(spec=launchpad.Launchpad)

services.ServiceFactory.register("remote_build", FakeRemoteBuild)
return FakeRemoteBuild


Expand All @@ -337,10 +348,6 @@ def fake_services(
fake_init_service_class,
fake_remote_build_service_class,
):
services.ServiceFactory.register("package", fake_package_service_class)
services.ServiceFactory.register("lifecycle", fake_lifecycle_service_class)
services.ServiceFactory.register("init", fake_init_service_class)
services.ServiceFactory.register("remote_build", fake_remote_build_service_class)
factory = services.ServiceFactory(app_metadata, project=fake_project)
factory.update_kwargs(
"lifecycle", work_dir=tmp_path, cache_dir=tmp_path / "cache", build_plan=[]
Expand Down
8 changes: 3 additions & 5 deletions tests/integration/services/test_service_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,11 @@ def test_gets_dataclass_services(
fake_lifecycle_service_class,
fake_provider_service_class,
):
services.ServiceFactory.register("lifecycle", fake_lifecycle_service_class)
services.ServiceFactory.register("provider", fake_provider_service_class)
factory = services.ServiceFactory(
app_metadata,
project=fake_project,
PackageClass=fake_package_service_class,
LifecycleClass=fake_lifecycle_service_class,
ProviderClass=fake_provider_service_class,
)

check.is_instance(factory.package, services.PackageService)
Expand All @@ -49,7 +48,6 @@ def test_gets_registered_services(
fake_lifecycle_service_class,
fake_provider_service_class,
):
services.ServiceFactory.register("package", fake_package_service_class)
services.ServiceFactory.register("lifecycle", fake_lifecycle_service_class)
services.ServiceFactory.register("provider", fake_provider_service_class)
factory = services.ServiceFactory(
Expand All @@ -63,10 +61,10 @@ def test_gets_registered_services(


def test_real_service_error(app_metadata, fake_project):
services.ServiceFactory.register("package", services.PackageService)
factory = services.ServiceFactory(
app_metadata,
project=fake_project,
PackageClass=services.PackageService,
)

with pytest.raises(
Expand Down
Loading
Loading