From 9d06939d5ef2733da73aca7161e208830fc84985 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Tue, 28 May 2024 19:11:43 +0400 Subject: [PATCH] Add --no-* negative selection flags for each single manager Closes #882 --- changelog.md | 3 + meta_package_manager/cli.py | 128 ++++++++++++++++++++++++----------- meta_package_manager/pool.py | 14 +++- poetry.lock | 21 ++++-- pyproject.toml | 1 + tests/test_cli.py | 20 ++++-- tests/test_cli_backup.py | 10 ++- tests/test_cli_upgrade.py | 10 ++- tests/test_managers.py | 4 ++ tests/test_pool.py | 2 +- 10 files changed, 158 insertions(+), 55 deletions(-) diff --git a/changelog.md b/changelog.md index 9467773ad..55a8728b5 100644 --- a/changelog.md +++ b/changelog.md @@ -6,6 +6,9 @@ This version is not released yet and is under active development. ``` +- \[mpm\] Add `--no-*` negative selection flags for each single manager. Closes {issue}`882`. +- \[mpm\] Stop CLI execution if manager selection parameters ends up with no managers being retained. +- \[mpm\] Add dependency on `more-itertools`. - \[mpm\] Add metadata and icon to binaries produced by Nuitka. - \[mpm\] Mark all Python 3.13-dev tests as stable but on macOS. - \[bar-plugin\] Reactivate login shells invokation tests. diff --git a/meta_package_manager/cli.py b/meta_package_manager/cli.py index a7ed99030..4b6095ccf 100644 --- a/meta_package_manager/cli.py +++ b/meta_package_manager/cli.py @@ -25,6 +25,7 @@ from io import TextIOWrapper from operator import attrgetter from pathlib import Path +from typing import Iterable, Iterator from unittest.mock import patch import tomli_w @@ -90,37 +91,87 @@ """ -def add_manager_to_selection(ctx: Context, param: Parameter, selected: bool) -> None: - """Store singular manager flag selection in the context. +def update_manager_selection( + ctx: Context, param: Parameter, value: str | Iterable[str] | None +) -> None: + """Update global selection list of managers in the context. - .. important:: - Because the parameter's name is transformed into a Python identifier on - instantiation, we have to reverse the process to get our value. - - Example: ``--apt-mint`` => ``apt_mint`` => ``apt-mint`` + Accumulate and merge all manager selectors to form the initial population enforced by the user. """ - if selected: - if ctx.obj is None: - ctx.obj = {"single_manager_selector": []} - manager_id = param.name.replace("_", "-") # type: ignore[union-attr] - ctx.obj["single_manager_selector"].append(manager_id) + # Option has not been called. + if not value: + return + + # Is a list to keep the natural order of selection. + to_add: list[str] = [] + # Is a set because we removal takes precedence over addition, so we don't care about user's order. + to_remove: set[str] = set() + + # Add the value of --manager list. + if param.name == "manager": + to_add.extend(value) + # Add the value of --exclure list. + elif param.name == "exclude": + to_remove.update(value) + + # Update the list of managers with the XKCD preset. + elif param.name == "xkcd": + to_add.extend(XKCD_MANAGER_ORDER) + + # Update selection with single selectors. + else: + assert value in pool.all_manager_ids, f"{value!r} is not a recognized manage ID" + # Because the parameter's name is transformed into a Python identifier on + # instantiation, we have to reverse the process to get our value. + # Example: --apt-mint => apt_mint => apt-mint + assert ( + param.name.removeprefix("no_").replace("_", "-") == value + ), f"single manager selector {param.name!r} is not recognized" + if param.name.startswith("no_"): + to_remove.add(value) + else: + to_add.append(value) -def single_manager_selectors(): + # Initialize the shared context object to accumulate there the selection results. + if ctx.obj is None: + ctx.obj = {} + if to_add: + ctx.obj.setdefault("managers_to_add", []).extend(to_add) + if to_remove: + ctx.obj.setdefault("managers_to_remove", set()).update(to_remove) + + +def single_manager_selectors() -> Iterator[Parameter]: """Dynamiccaly creates a dedicated flag selector alias for each manager.""" + single_flags = [] + single_no_flags = [] for manager_id in pool.all_manager_ids: manager = pool.get(manager_id) # Parameters do not have a deprecated flag. # See: https://github.com/pallets/click/issues/2263 deprecated_msg = " (deprecated)" if manager.deprecated else "" - yield option( - f"--{manager_id}", - is_flag=True, - default=False, - help=f"Alias to --manager {manager_id}{deprecated_msg}.", - expose_value=False, - callback=add_manager_to_selection, + single_flags.append( + option( + f"--{manager_id}", + flag_value=manager_id, + default=False, + help=f"Alias to --manager {manager_id}{deprecated_msg}.", + expose_value=False, + callback=update_manager_selection, + ) + ) + single_no_flags.append( + option( + f"--no-{manager_id}", + flag_value=manager_id, + default=False, + help=f"Alias to --exclude {manager_id}{deprecated_msg}.", + expose_value=False, + callback=update_manager_selection, + ) ) + return *single_flags, *single_no_flags def bar_plugin_path(ctx, param, value): @@ -177,6 +228,8 @@ def bar_plugin_path(ctx, param, value): "--manager", type=Choice(pool.all_manager_ids, case_sensitive=False), multiple=True, + expose_value=False, + callback=update_manager_selection, help="Restrict subcommand to a subset of managers. Repeat to " "select multiple managers. The order is preserved for priority-sensitive " "subcommands.", @@ -186,7 +239,11 @@ def bar_plugin_path(ctx, param, value): "--exclude", type=Choice(pool.all_manager_ids, case_sensitive=False), multiple=True, - help="Exclude a manager. Repeat to exclude multiple managers.", + expose_value=False, + callback=update_manager_selection, + help="Exclude a manager. Repeat to exclude multiple managers. Exclusion of a " + "manager always takes precedence over inclusion, whatever the parameter " + "position.", ), option( "-a", @@ -202,6 +259,8 @@ def bar_plugin_path(ctx, param, value): "--xkcd", is_flag=True, default=False, + expose_value=False, + callback=update_manager_selection, help="Preset manager selection as defined by XKCD #1654. Equivalent to: " "{}.".format(" ".join(f"--{mid}" for mid in XKCD_MANAGER_ORDER)), ), @@ -278,10 +337,7 @@ def bar_plugin_path(ctx, param, value): @pass_context def mpm( ctx, - manager, - exclude, all_managers, - xkcd, ignore_auto_updates, stop_on_error, dry_run, @@ -311,32 +367,28 @@ def remove_logging_override(): ctx.call_on_close(remove_logging_override) - # Merge all manager selectors to form the initial population enforced by the - # user. - initial_managers = list(manager) - # Update with single selectors. + user_selection = None + managers_to_remove = None if ctx.obj: - initial_managers.extend(ctx.obj.get("single_manager_selector", [])) - # Update the list of managers with the XKCD preset. - if xkcd: - initial_managers.extend(XKCD_MANAGER_ORDER) + user_selection = ctx.obj.get("managers_to_add", None) + managers_to_remove = ctx.obj.get("managers_to_remove", None) + # Normalize to None if no manager selectors have been used. This prevent the # pool.select_managers() method to iterate over an empty population of managers to # choose from. - if not initial_managers: - initial_managers = None + if not user_selection: logging.debug("No initial population of managers selected by user.") else: logging.debug( - "Initial population of user-selected managers: " - f"{' > '.join(map(theme.invoked_command, initial_managers))}", + "User-selected managers by priority: " + f"{' > '.join(map(theme.invoked_command, user_selection))}", ) # Select the subset of manager to target, and apply manager-level options. selected_managers = partial( pool.select_managers, - keep=initial_managers, - drop=exclude, + keep=user_selection, + drop=managers_to_remove, keep_deprecated=all_managers, # Should we include auto-update packages or not? ignore_auto_updates=ignore_auto_updates, diff --git a/meta_package_manager/pool.py b/meta_package_manager/pool.py index 5081518bd..cb9c76f32 100644 --- a/meta_package_manager/pool.py +++ b/meta_package_manager/pool.py @@ -22,7 +22,9 @@ from typing import TYPE_CHECKING, Final, Iterable, Iterator from boltons.iterutils import unique +from click_extra import get_current_context from click_extra.colorize import default_theme as theme +from more_itertools import peekable from .managers.apm import APM from .managers.apt import APT, APT_Mint @@ -181,7 +183,7 @@ def unsupported_manager_ids(self) -> tuple[str, ...]: if mid not in self.default_manager_ids ) - def select_managers( + def _select_managers( self, keep: Iterable[str] | None = None, drop: Iterable[str] | None = None, @@ -258,5 +260,15 @@ def select_managers( yield manager + def select_managers(self, *args, **kwargs) -> Iterator[PackageManager]: + """Wraps ``_select_managers()`` to stop CLI execution if no manager are selected.""" + managers = peekable(self._select_managers(*args, **kwargs)) + try: + managers.peek() + except StopIteration: + logging.critical("No manager selected.") + get_current_context().exit(2) + yield from managers + pool = ManagerPool() diff --git a/poetry.lock b/poetry.lock index cae6f2aef..518e4d0a2 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. [[package]] name = "alabaster" @@ -605,6 +605,17 @@ files = [ {file = "mergedeep-1.3.4.tar.gz", hash = "sha256:0096d52e9dad9939c3d975a774666af186eda617e6ca84df4c94dec30004f2a8"}, ] +[[package]] +name = "more-itertools" +version = "10.2.0" +description = "More routines for operating on iterables, beyond itertools" +optional = false +python-versions = ">=3.8" +files = [ + {file = "more-itertools-10.2.0.tar.gz", hash = "sha256:8fccb480c43d3e99a00087634c06dd02b0d50fbf088b380de5a41a015ec239e1"}, + {file = "more_itertools-10.2.0-py3-none-any.whl", hash = "sha256:686b06abe565edfab151cb8fd385a05651e1fdf8f0a14191e4439283421f8684"}, +] + [[package]] name = "myst-parser" version = "3.0.1" @@ -1435,13 +1446,13 @@ files = [ [[package]] name = "zipp" -version = "3.18.2" +version = "3.19.0" description = "Backport of pathlib-compatible object wrapper for zip files" optional = false python-versions = ">=3.8" files = [ - {file = "zipp-3.18.2-py3-none-any.whl", hash = "sha256:dce197b859eb796242b0622af1b8beb0a722d52aa2f57133ead08edd5bf5374e"}, - {file = "zipp-3.18.2.tar.gz", hash = "sha256:6278d9ddbcfb1f1089a88fde84481528b07b0e10474e09dcfe53dad4069fa059"}, + {file = "zipp-3.19.0-py3-none-any.whl", hash = "sha256:96dc6ad62f1441bcaccef23b274ec471518daf4fbbc580341204936a5a3dddec"}, + {file = "zipp-3.19.0.tar.gz", hash = "sha256:952df858fb3164426c976d9338d3961e8e8b3758e2e059e0f754b8c4262625ee"}, ] [package.extras] @@ -1451,4 +1462,4 @@ testing = ["big-O", "jaraco.functools", "jaraco.itertools", "jaraco.test", "more [metadata] lock-version = "2.0" python-versions = "^3.8" -content-hash = "9d8f785f44da48c652e13f3c88f6f3b0e1fa5732078d9a0cb171885a8bb9b6b7" +content-hash = "abb4bfa5f2d73ddfda952de06d643862f518dba5cd33794dff9f85abf590dd12" diff --git a/pyproject.toml b/pyproject.toml index 80b05b6df..93376f417 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -90,6 +90,7 @@ classifiers = [ python = "^3.8" boltons = "^24.0.0" click-extra = "^4.8.3" +more-itertools = "^10.2.0" packageurl-python = "^0.15.0" tabulate = { version = "^0.9.0", extras = ["widechars"] } tomli = { version = "^2.0.1", python = "< 3.11" } diff --git a/tests/test_cli.py b/tests/test_cli.py index c31288595..64596d250 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -283,20 +283,28 @@ def test_manager_shortcuts(self, invoke, manager_id): ), pytest.param( ("--manager", "pip", "--exclude", "pip"), - set(), - id="exclusion_override_ordered", + None, + id="exclusion_precedence_ordered", ), pytest.param( ("--exclude", "pip", "--manager", "pip"), - set(), - id="exclusion_override_reversed", + None, + id="exclusion_precedence_reversed", ), ), ) def test_manager_selection(self, invoke, args, expected): result = invoke(*args, "managers") - assert result.exit_code == 0 - self.check_manager_selection(result, expected) + if expected is None: + assert result.exit_code == 2 + assert not result.stdout + assert ( + result.stderr + == "\x1b[31m\x1b[1mcritical\x1b[0m: No manager selected.\n" + ) + else: + assert result.exit_code == 0 + self.check_manager_selection(result, expected) @pytest.mark.skip(reason="Generated config file is not isolated from other tests.") def test_conf_file_overrides_defaults(self, invoke, create_config): diff --git a/tests/test_cli_backup.py b/tests/test_cli_backup.py index 52edd8b9d..688e918e0 100644 --- a/tests/test_cli_backup.py +++ b/tests/test_cli_backup.py @@ -64,6 +64,12 @@ def test_single_manager_file_output(self, manager_id, invoke, subcmd): result = invoke( "--verbosity", "INFO", f"--{manager_id}", subcmd, "mpm-packages.toml" ) - assert result.exit_code == 0 assert "mpm-packages.toml" in result.stderr - self.check_manager_selection(result, {manager_id}) + if result.exit_code == 2: + assert not result.stdout + assert result.stderr.endswith( + "\x1b[31m\x1b[1mcritical\x1b[0m: No manager selected.\n" + ) + else: + assert result.exit_code == 0 + self.check_manager_selection(result, {manager_id}) diff --git a/tests/test_cli_upgrade.py b/tests/test_cli_upgrade.py index 80cf97879..fdac95552 100644 --- a/tests/test_cli_upgrade.py +++ b/tests/test_cli_upgrade.py @@ -73,10 +73,16 @@ def test_single_manager_dry_run_upgrade_all(self, invoke, manager_id, all_option result = invoke( f"--{manager_id}", "--dry-run", "--verbosity", "INFO", "upgrade", all_option ) - assert result.exit_code == 0 if not all_option: assert "assume -A/--all option" in result.stderr - self.check_manager_selection(result, {manager_id}) + if result.exit_code == 2: + assert not result.stdout + assert result.stderr.endswith( + "\x1b[31m\x1b[1mcritical\x1b[0m: No manager selected.\n" + ) + else: + assert result.exit_code == 0 + self.check_manager_selection(result, {manager_id}) @pytest.mark.destructive() @default_manager_ids diff --git a/tests/test_managers.py b/tests/test_managers.py index c0f55b898..3de9d04ce 100644 --- a/tests/test_managers.py +++ b/tests/test_managers.py @@ -61,7 +61,11 @@ def test_ascii_id(manager): """All package manager IDs should be short ASCII strings.""" assert manager.id assert manager.id.isascii() + assert manager.id[0] in ascii_lowercase + assert manager.id[-1] in ascii_lowercase + digits assert set(manager.id).issubset(ascii_lowercase + digits + "-") + # Make sure there is no potential conflix by prefixing a manager ID by "no", which is the convention for single manager flags in the CLI. + assert not manager.id.startswith("no") @all_managers diff --git a/tests/test_pool.py b/tests/test_pool.py index 6e6aeb377..45569e0d5 100644 --- a/tests/test_pool.py +++ b/tests/test_pool.py @@ -242,5 +242,5 @@ def test_extra_option_allowlist(): def test_select_managers(kwargs, expected): """We use tuple everywhere so we can check that select_managers() conserve the original order.""" - selection = pool.select_managers(**kwargs) + selection = pool._select_managers(**kwargs) assert tuple(m.id for m in selection) == expected