diff --git a/AGENTS.md b/AGENTS.md index 38a67757..ad0bf6d8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,7 +30,7 @@ The CLI is a Click app that wraps the auto-generated `cloudsmith-api` Python SDK ### Layered layout - `cloudsmith_cli/cli/` — Click commands, decorators, output formatting, validators, config-file parsing, SAML browser-callback webserver. API **call sites** belong in `core/api/*`; commands invoke those wrappers rather than calling the SDK directly. (A few commands — `login`, `logout`, `check` — do import `cloudsmith_api` for `Configuration` defaults or exception classes; that's fine, but new request code should go through `core/api/*`.) - - `cli/commands/main.py` defines the top-level `main` Click group (`cls=AliasGroup`). Every other command module imports `main` and registers itself with `@main.command(...)` or `@main.group(...)`. `cli/commands/__init__.py` imports every module so registration happens on import. + - `cli/commands/main.py` defines the top-level `main` Click group (`cls=AliasGroup`). Every other command module imports `main` and registers itself with `@main.command(...)` or `@main.group(...)`. Command modules are imported lazily: `cli/commands/registry.py` maps each command name (and alias) to its module, and `AliasGroup` imports the module on first use. When you add, rename or alias a top-level command, update the registry; `cli/tests/test_lazy_commands.py` fails on drift. - `cli/command.py` provides `AliasGroup` (DYM + alias support) and JSON-aware Click exception formatting — Click errors are serialized to JSON when `-F json|pretty_json` is in effect (checked from both Click context and `sys.argv`). - `cli/decorators.py` is the seam between CLI and API: `@common_cli_config_options`, `@common_cli_output_options`, `@common_api_auth_options`, and `@initialise_api` (which calls `core.api.init.initialise_api` to configure the `cloudsmith_api` SDK with key/host/proxy/SSL/retry/SAML token). `@initialise_mcp` wires up the MCP server. - `cli/config.py` reads `config.ini` and `credentials.ini` via `click-configfile`. Search path: cwd, `click.get_app_dir("cloudsmith")`, `~/.cloudsmith`. Profiles use `[profile:NAME]` sections. diff --git a/cloudsmith_cli/cli/command.py b/cloudsmith_cli/cli/command.py index 7332e26c..4ab0ef3a 100644 --- a/cloudsmith_cli/cli/command.py +++ b/cloudsmith_cli/cli/command.py @@ -1,5 +1,6 @@ """CLI - Group/Command classes.""" +import importlib from collections import OrderedDict import click.exceptions @@ -50,12 +51,25 @@ def _format_click_exception_as_json(exception): class AliasGroup(DYMGroup): - """A command group with DYM and alias support.""" + """A command group with DYM, alias and lazy-subcommand support. + + ``lazy_commands`` maps a command name to the module that registers it. + The module is imported on first use, which keeps startup fast because + an invocation imports only the module of the invoked command. + ``lazy_aliases`` supplies the alias map for those commands up front. + """ def __init__(self, *args, **kwargs): + lazy_commands = kwargs.pop("lazy_commands", None) + lazy_aliases = kwargs.pop("lazy_aliases", None) super().__init__(*args, **kwargs) + self.lazy_commands = dict(lazy_commands or {}) self.aliases = OrderedDict() self.inverse = {} + for name, aliases in (lazy_aliases or {}).items(): + self.aliases[name] = aliases + for alias in aliases: + self.inverse[alias] = name def resolve_command(self, ctx, args): try: @@ -74,7 +88,7 @@ def resolve_command(self, ctx, args): raise def list_commands(self, ctx): - commands = super().list_commands(ctx) + commands = sorted(set(super().list_commands(ctx)) | set(self.lazy_commands)) if getattr(ctx, "showing_help", False): for k, v in enumerate(commands): @@ -99,7 +113,11 @@ def get_command(self, ctx, cmd_name): except KeyError: pass - return super().get_command(ctx, cmd_name) + cmd = super().get_command(ctx, cmd_name) + if cmd is None and cmd_name in self.lazy_commands: + importlib.import_module(self.lazy_commands[cmd_name]) + cmd = super().get_command(ctx, cmd_name) + return cmd def command(self, *args, **kwargs): def decorator(f): diff --git a/cloudsmith_cli/cli/commands/__init__.py b/cloudsmith_cli/cli/commands/__init__.py index 0ad6fc2b..72bbd338 100644 --- a/cloudsmith_cli/cli/commands/__init__.py +++ b/cloudsmith_cli/cli/commands/__init__.py @@ -1,34 +1,6 @@ -"""CLI/Commands - Import all commands.""" +"""CLI/Commands. -from . import ( - auth, - check, - copy, - credential_helper, - delete, - dependencies, - docs, - domains, - download, - entitlements, - help_, - list_, - login, - logout, - mcp, - metadata, - metrics, - move, - policy, - push, - quarantine, - quota, - repos, - resync, - status, - tags, - tokens, - upstream, - vulnerabilities, - whoami, -) +Command modules register themselves with the ``main`` group on import. +The ``main`` group imports them lazily through ``registry.LAZY_COMMANDS``, +so this package must not import them here. +""" diff --git a/cloudsmith_cli/cli/commands/main.py b/cloudsmith_cli/cli/commands/main.py index e476b8ca..be7b3250 100644 --- a/cloudsmith_cli/cli/commands/main.py +++ b/cloudsmith_cli/cli/commands/main.py @@ -6,6 +6,7 @@ from ...core.utils import get_github_website, get_help_website from ...core.version import get_version as get_cli_version from .. import command, decorators, utils +from .registry import LAZY_ALIASES, LAZY_COMMANDS CONTEXT_SETTINGS = {"help_option_names": ["-h", "--help"]} @@ -28,6 +29,8 @@ def print_version(opts): @click.group( cls=command.AliasGroup, + lazy_commands=LAZY_COMMANDS, + lazy_aliases=LAZY_ALIASES, context_settings=CONTEXT_SETTINGS, invoke_without_command=True, help="""\b diff --git a/cloudsmith_cli/cli/commands/registry.py b/cloudsmith_cli/cli/commands/registry.py new file mode 100644 index 00000000..f1d28b9a --- /dev/null +++ b/cloudsmith_cli/cli/commands/registry.py @@ -0,0 +1,59 @@ +"""CLI/Commands - Lazy command registry. + +Maps each top-level command name to the module that registers it, and each +command to its aliases. The AliasGroup imports a module only when its +command runs, which keeps CLI startup fast. The tests in +``cli/tests/test_lazy_commands.py`` verify that this registry matches what +the modules declare. +""" + +_PACKAGE = "cloudsmith_cli.cli.commands" + +LAZY_COMMANDS = { + "authenticate": f"{_PACKAGE}.auth", + "check": f"{_PACKAGE}.check", + "copy": f"{_PACKAGE}.copy", + "credential-helper": f"{_PACKAGE}.credential_helper", + "delete": f"{_PACKAGE}.delete", + "dependencies": f"{_PACKAGE}.dependencies", + "docs": f"{_PACKAGE}.docs", + "domains": f"{_PACKAGE}.domains", + "download": f"{_PACKAGE}.download", + "entitlements": f"{_PACKAGE}.entitlements", + "help": f"{_PACKAGE}.help_", + "list": f"{_PACKAGE}.list_", + "login": f"{_PACKAGE}.login", + "logout": f"{_PACKAGE}.logout", + "mcp": f"{_PACKAGE}.mcp", + "metadata": f"{_PACKAGE}.metadata", + "metrics": f"{_PACKAGE}.metrics", + "move": f"{_PACKAGE}.move", + "policy": f"{_PACKAGE}.policy", + "push": f"{_PACKAGE}.push", + "quarantine": f"{_PACKAGE}.quarantine", + "quota": f"{_PACKAGE}.quota", + "repositories": f"{_PACKAGE}.repos", + "resync": f"{_PACKAGE}.resync", + "status": f"{_PACKAGE}.status", + "tags": f"{_PACKAGE}.tags", + "tokens": f"{_PACKAGE}.tokens", + "upstream": f"{_PACKAGE}.upstream", + "vulnerabilities": f"{_PACKAGE}.vulnerabilities", + "whoami": f"{_PACKAGE}.whoami", +} + +LAZY_ALIASES = { + "authenticate": ["auth"], + "copy": ["cp"], + "delete": ["rm"], + "dependencies": ["deps"], + "domains": ["domain"], + "entitlements": ["ents"], + "list": ["ls"], + "login": ["token"], + "move": ["mv", "promote"], + "push": ["upload", "deploy"], + "quarantine": ["block"], + "repositories": ["repos"], + "tags": ["tag"], +} diff --git a/cloudsmith_cli/cli/tests/test_lazy_commands.py b/cloudsmith_cli/cli/tests/test_lazy_commands.py new file mode 100644 index 00000000..68ee20d6 --- /dev/null +++ b/cloudsmith_cli/cli/tests/test_lazy_commands.py @@ -0,0 +1,38 @@ +"""Tests that the lazy command registry matches the command modules. + +The registry duplicates the command names and aliases that the modules +declare, so the CLI can resolve a command without importing every module. +These tests import every command module and compare the result with the +registry to catch drift. +""" + +import importlib +import pkgutil + +from cloudsmith_cli.cli import commands +from cloudsmith_cli.cli.commands.main import main +from cloudsmith_cli.cli.commands.registry import LAZY_ALIASES, LAZY_COMMANDS + + +def import_every_command_module(): + prefix = commands.__name__ + "." + for module_info in pkgutil.walk_packages(commands.__path__, prefix=prefix): + importlib.import_module(module_info.name) + + +def test_registry_matches_registered_commands(): + import_every_command_module() + assert set(main.commands) == set(LAZY_COMMANDS) + + +def test_registry_matches_registered_aliases(): + import_every_command_module() + assert {name: list(aliases) for name, aliases in main.aliases.items()} == ( + LAZY_ALIASES + ) + + +def test_every_command_and_alias_resolves(): + ctx = main.make_context("cloudsmith", [], resilient_parsing=True) + for name in list(LAZY_COMMANDS) + [a for al in LAZY_ALIASES.values() for a in al]: + assert main.get_command(ctx, name) is not None, name diff --git a/cloudsmith_cli/cli/tests/test_startup_imports.py b/cloudsmith_cli/cli/tests/test_startup_imports.py index ea0553a1..0e117b4e 100644 --- a/cloudsmith_cli/cli/tests/test_startup_imports.py +++ b/cloudsmith_cli/cli/tests/test_startup_imports.py @@ -15,7 +15,7 @@ def modules_loaded_by_cli_import(): code = ( "import json, sys\n" - "import cloudsmith_cli.cli.commands\n" + "import cloudsmith_cli.cli.commands.main\n" "print(json.dumps(sorted(sys.modules)))\n" ) result = subprocess.run( @@ -35,3 +35,14 @@ def test_cli_import_does_not_load_heavy_modules(): if any(name == p or name.startswith(p + ".") for p in HEAVY_PREFIXES) ] assert heavy == [] + + +def test_cli_import_does_not_load_command_modules(): + package = "cloudsmith_cli.cli.commands." + allowed = {package + "main", package + "registry"} + loaded = [ + name + for name in modules_loaded_by_cli_import() + if name.startswith(package) and name not in allowed + ] + assert loaded == [] diff --git a/packaging/pyinstaller/cloudsmith.spec b/packaging/pyinstaller/cloudsmith.spec index 70a2415c..412d6e16 100644 --- a/packaging/pyinstaller/cloudsmith.spec +++ b/packaging/pyinstaller/cloudsmith.spec @@ -24,6 +24,7 @@ hiddenimports += collect_submodules( "mcp", filter=lambda name: name != "mcp.cli" and not name.startswith("mcp.cli."), ) +hiddenimports += collect_submodules("cloudsmith_cli") hiddenimports += collect_submodules("keyring.backends") hiddenimports += collect_submodules("keyrings.cryptfile") hiddenimports += collect_submodules("keyrings.alt") @@ -56,7 +57,9 @@ a = Analysis( "isort", "mcp.cli", "cloudsmith_cli.cli.tests", + "cloudsmith_cli.conftest", "cloudsmith_cli.core.tests", + "cloudsmith_cli.credential_helpers.pnpm.tests", "keyrings.cryptfile.tests", ], )