From 68def0c5a5a4338c9822330cec87625afa9ac351 Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Mon, 21 Oct 2024 17:08:15 -0300 Subject: [PATCH 1/4] feat: pass the app_config to pre_parse_args() This lets commands be able to always rely on `self.config` existing, even when running `fill_parser()` for help output. Fixes #530 --- craft_application/application.py | 9 +++++--- craft_application/commands/base.py | 8 +------- docs/reference/changelog.rst | 9 ++++++++ pyproject.toml | 2 +- tests/unit/commands/test_base.py | 13 ------------ tests/unit/test_application.py | 33 ++++++++++++++++++++++++++++++ 6 files changed, 50 insertions(+), 24 deletions(-) diff --git a/craft_application/application.py b/craft_application/application.py index 77addcf8..442db879 100644 --- a/craft_application/application.py +++ b/craft_application/application.py @@ -425,15 +425,18 @@ def _get_dispatcher(self) -> craft_cli.Dispatcher: try: craft_cli.emit.trace("pre-parsing arguments...") + app_config = self.app_config # Workaround for the fact that craft_cli requires a command. # https://github.com/canonical/craft-cli/issues/141 if "--version" in sys.argv or "-V" in sys.argv: try: - global_args = dispatcher.pre_parse_args(["pull", *sys.argv[1:]]) + global_args = dispatcher.pre_parse_args( + ["pull", *sys.argv[1:]], app_config + ) except craft_cli.ArgumentParsingError: - global_args = dispatcher.pre_parse_args(sys.argv[1:]) + global_args = dispatcher.pre_parse_args(sys.argv[1:], app_config) else: - global_args = dispatcher.pre_parse_args(sys.argv[1:]) + global_args = dispatcher.pre_parse_args(sys.argv[1:], app_config) if global_args.get("version"): craft_cli.emit.message(f"{self.app.name} {self.app.version}") diff --git a/craft_application/commands/base.py b/craft_application/commands/base.py index 4b6d9f9b..f2aa6f24 100644 --- a/craft_application/commands/base.py +++ b/craft_application/commands/base.py @@ -53,13 +53,7 @@ class AppCommand(BaseCommand): always_load_project: bool = False """The project is also loaded in non-managed mode.""" - def __init__(self, config: dict[str, Any] | None) -> None: - if config is None: - # This should only be the case when the command is not going to be run. - # For example, when requesting help on the command. - emit.trace("Not completing command configuration") - return - + def __init__(self, config: dict[str, Any]) -> None: super().__init__(config) self._app: application.AppMetadata = config["app"] diff --git a/docs/reference/changelog.rst b/docs/reference/changelog.rst index b8447809..ba143815 100644 --- a/docs/reference/changelog.rst +++ b/docs/reference/changelog.rst @@ -4,6 +4,15 @@ Changelog ********* +4.4.0 (2024-Oct-XX) +------------------- + +Application +=========== + +- ``AppCommand`` subclasses now will always receive a valid ``app_config`` + dict. + 4.3.0 (2024-Oct-11) ------------------- diff --git a/pyproject.toml b/pyproject.toml index 8aa28dc4..e4ab933c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ description = "A framework for *craft applications." dynamic = ["version", "readme"] dependencies = [ "craft-archives>=2.0.0", - "craft-cli>=2.6.0", + "craft-cli @ git+https://github.com/canonical/craft-cli@work/CRAFT-3585-command-help-app-config", "craft-grammar>=2.0.0", "craft-parts>=2.1.1", "craft-platforms>=0.3.1", diff --git a/tests/unit/commands/test_base.py b/tests/unit/commands/test_base.py index 8b94bae0..d860e447 100644 --- a/tests/unit/commands/test_base.py +++ b/tests/unit/commands/test_base.py @@ -63,19 +63,6 @@ def test_get_managed_cmd(fake_command, verbosity, app_metadata): ] -def test_without_config(emitter): - """Test that a command can be initialised without a config. - - This is necessary for providing per-command help. - """ - - command = base.AppCommand(None) - - emitter.assert_trace("Not completing command configuration") - assert not hasattr(command, "_app") - assert not hasattr(command, "_services") - - @pytest.mark.parametrize("always_load_project", [True, False]) def test_needs_project(fake_command, always_load_project): """`needs_project()` defaults to `always_load_project`.""" diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index 17df6c69..13f24343 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -49,6 +49,7 @@ services, util, ) +from craft_application.commands import AppCommand from craft_application.models import BuildInfo from craft_application.util import ( get_host_architecture, # pyright: ignore[reportGeneralTypeIssues] @@ -2141,3 +2142,35 @@ def test_clean_platform(monkeypatch, tmp_path, app_metadata, fake_services, mock base=bases.BaseName("ubuntu", "24.04"), ) mocked_clean.assert_called_once_with(mocker.ANY, mocker.ANY, expected_info) + + +class AppConfigCommand(AppCommand): + + name: str = "app-config" + help_msg: str = "Help text" + overview: str = "Overview" + + def fill_parser(self, parser: argparse.ArgumentParser) -> None: + + name = self._app.name + parser.add_argument( + "app-name", + help=f"The name of the app, which is {name!r}.", + ) + + +@pytest.mark.usefixtures("emitter") +def test_app_config_in_help( + monkeypatch, + capsys, + app, +): + app.add_command_group("Test", [AppConfigCommand]) + monkeypatch.setattr(sys, "argv", ["testcraft", "app-config", "-h"]) + + with pytest.raises(SystemExit): + app.run() + + expected = "app-name: The name of the app, which is 'testcraft'." + _, err = capsys.readouterr() + assert expected in err From 3affa99fae35cdba832a98c0ce94b4bb5d8ae2c4 Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Tue, 22 Oct 2024 10:31:55 -0300 Subject: [PATCH 2/4] fixup! feat: pass the app_config to pre_parse_args() --- craft_application/commands/base.py | 11 ++++++++++- tests/unit/commands/test_base.py | 13 +++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/craft_application/commands/base.py b/craft_application/commands/base.py index f2aa6f24..5ad5b1e5 100644 --- a/craft_application/commands/base.py +++ b/craft_application/commands/base.py @@ -18,6 +18,7 @@ import abc import argparse +import warnings from typing import Any, Optional, Protocol, final from craft_cli import BaseCommand, emit @@ -53,7 +54,15 @@ class AppCommand(BaseCommand): always_load_project: bool = False """The project is also loaded in non-managed mode.""" - def __init__(self, config: dict[str, Any]) -> None: + def __init__(self, config: dict[str, Any] | None) -> None: + if config is None: + warnings.warn( + "Creating an AppCommand without a config dict is pending deprecation.", + PendingDeprecationWarning, + ) + emit.trace("Not completing command configuration") + return + super().__init__(config) self._app: application.AppMetadata = config["app"] diff --git a/tests/unit/commands/test_base.py b/tests/unit/commands/test_base.py index d860e447..5a0aea65 100644 --- a/tests/unit/commands/test_base.py +++ b/tests/unit/commands/test_base.py @@ -63,6 +63,19 @@ def test_get_managed_cmd(fake_command, verbosity, app_metadata): ] +def test_without_config(emitter): + """Test that a command can be initialised without a config. + This is pending deprecation but still supported. + """ + + with pytest.deprecated_call(): + command = base.AppCommand(None) + + emitter.assert_trace("Not completing command configuration") + assert not hasattr(command, "_app") + assert not hasattr(command, "_services") + + @pytest.mark.parametrize("always_load_project", [True, False]) def test_needs_project(fake_command, always_load_project): """`needs_project()` defaults to `always_load_project`.""" From 2a83ad8b8255fb2e8a867b6c67f48b5c96539ffe Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Tue, 22 Oct 2024 10:37:21 -0300 Subject: [PATCH 3/4] fixup! fixup! feat: pass the app_config to pre_parse_args() --- craft_application/commands/base.py | 1 + 1 file changed, 1 insertion(+) diff --git a/craft_application/commands/base.py b/craft_application/commands/base.py index 5ad5b1e5..33056c05 100644 --- a/craft_application/commands/base.py +++ b/craft_application/commands/base.py @@ -59,6 +59,7 @@ def __init__(self, config: dict[str, Any] | None) -> None: warnings.warn( "Creating an AppCommand without a config dict is pending deprecation.", PendingDeprecationWarning, + stacklevel=3, ) emit.trace("Not completing command configuration") return From 685c7e17dc72e0605968ff09435d3754e5ef6535 Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Tue, 22 Oct 2024 14:46:42 -0300 Subject: [PATCH 4/4] fixup! fixup! fixup! feat: pass the app_config to pre_parse_args() --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index e4ab933c..c185a502 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ description = "A framework for *craft applications." dynamic = ["version", "readme"] dependencies = [ "craft-archives>=2.0.0", - "craft-cli @ git+https://github.com/canonical/craft-cli@work/CRAFT-3585-command-help-app-config", + "craft-cli>=2.9.0", "craft-grammar>=2.0.0", "craft-parts>=2.1.1", "craft-platforms>=0.3.1",