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: pass the app_config to pre_parse_args() #541

Merged
merged 5 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
9 changes: 6 additions & 3 deletions craft_application/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down
8 changes: 6 additions & 2 deletions craft_application/commands/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import abc
import argparse
import warnings
from typing import Any, Optional, Protocol, final

from craft_cli import BaseCommand, emit
Expand Down Expand Up @@ -55,8 +56,11 @@ class AppCommand(BaseCommand):

def __init__(self, config: dict[str, Any] | None) -> None:
tigarmo marked this conversation as resolved.
Show resolved Hide resolved
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.
warnings.warn(
"Creating an AppCommand without a config dict is pending deprecation.",
PendingDeprecationWarning,
stacklevel=3,
)
emit.trace("Not completing command configuration")
return

Expand Down
9 changes: 9 additions & 0 deletions docs/reference/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@
Changelog
*********

4.4.0 (2024-Oct-XX)
upils marked this conversation as resolved.
Show resolved Hide resolved
-------------------

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

- ``AppCommand`` subclasses now will always receive a valid ``app_config``
dict.

4.3.0 (2024-Oct-11)
-------------------

Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
tigarmo marked this conversation as resolved.
Show resolved Hide resolved
"craft-grammar>=2.0.0",
"craft-parts>=2.1.1",
"craft-platforms>=0.3.1",
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/commands/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ 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.
This is pending deprecation but still supported.
"""

command = base.AppCommand(None)
with pytest.deprecated_call():
command = base.AppCommand(None)

emitter.assert_trace("Not completing command configuration")
assert not hasattr(command, "_app")
Expand Down
33 changes: 33 additions & 0 deletions tests/unit/test_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
Loading