From 5239864d9ed0ff5b6f5a1964e8a2cda963757f37 Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Wed, 13 Oct 2021 22:37:38 +0200 Subject: [PATCH] Add `[cli.alias]` config section for command line alias support. (#13228) This allows you to run pants with commands that are easier to remember, even if there are long or many options involved. The following two invocations are equivalent, given the pants configuration below: ``` $ ./pants all-changed fmt $ ./pants --changed-since=HEAD --changed-dependees=transitive fmt ``` `pants.toml`: ``` [cli.alias] all-changed = "--changed-since=HEAD --changed-dependees=transitive" ``` # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- pants.toml | 7 + .../pants/build_graph/build_configuration.py | 3 +- src/python/pants/option/alias.py | 129 +++++++++++++++ src/python/pants/option/alias_test.py | 151 ++++++++++++++++++ src/python/pants/option/global_options.py | 31 +++- .../pants/option/options_bootstrapper.py | 73 +++++---- .../pants/option/options_bootstrapper_test.py | 35 ++++ src/python/pants/option/options_test.py | 7 +- 8 files changed, 400 insertions(+), 36 deletions(-) create mode 100644 src/python/pants/option/alias.py create mode 100644 src/python/pants/option/alias_test.py diff --git a/pants.toml b/pants.toml index d95769ea414b..b4e2d739bb56 100644 --- a/pants.toml +++ b/pants.toml @@ -58,10 +58,17 @@ remote_store_address = "grpcs://cache.toolchain.com:443" remote_instance_name = "main" remote_auth_plugin = "toolchain.pants.auth.plugin:toolchain_auth_plugin" + [anonymous-telemetry] enabled = true repo_id = "7775F8D5-FC58-4DBC-9302-D00AE4A1505F" + +[cli.alias] +all-changed = "--changed-since=HEAD --changed-dependees=transitive" +pyupgrade = "--backend-packages=pants.backend.python.lint.pyupgrade fmt" + + [source] root_patterns = [ "src/*", diff --git a/src/python/pants/build_graph/build_configuration.py b/src/python/pants/build_graph/build_configuration.py index e982c3186123..52798f460edf 100644 --- a/src/python/pants/build_graph/build_configuration.py +++ b/src/python/pants/build_graph/build_configuration.py @@ -15,6 +15,7 @@ from pants.engine.rules import Rule, RuleIndex from pants.engine.target import Target from pants.engine.unions import UnionRule +from pants.option.alias import CliOptions from pants.option.global_options import GlobalOptions from pants.option.scope import normalize_scope from pants.option.subsystem import Subsystem @@ -31,7 +32,7 @@ # Subsystems used outside of any rule. -_GLOBAL_SUBSYSTEMS: Set[Type[Subsystem]] = {GlobalOptions, Changed} +_GLOBAL_SUBSYSTEMS: Set[Type[Subsystem]] = {GlobalOptions, Changed, CliOptions} @dataclass(frozen=True) diff --git a/src/python/pants/option/alias.py b/src/python/pants/option/alias.py new file mode 100644 index 000000000000..58358e71340a --- /dev/null +++ b/src/python/pants/option/alias.py @@ -0,0 +1,129 @@ +# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import annotations + +import logging +import re +import shlex +from dataclasses import dataclass, field +from itertools import chain +from typing import Generator + +from pants.option.errors import OptionsError +from pants.option.scope import ScopeInfo +from pants.option.subsystem import Subsystem +from pants.util.frozendict import FrozenDict + +logger = logging.getLogger(__name__) + + +class CliAliasError(OptionsError): + pass + + +class CliAliasCycleError(CliAliasError): + pass + + +class CliAliasInvalidError(CliAliasError): + pass + + +class CliOptions(Subsystem): + options_scope = "cli" + help = "Options for configuring CLI behavior, such as command line aliases." + + @staticmethod + def register_options(register): + register( + "--alias", + type=dict, + default={}, + help=( + "Register command line aliases.\nExample:\n\n" + " [cli.alias]\n" + ' green = "fmt lint check"\n' + ' all-changed = "--changed-since=HEAD --changed-dependees=transitive"\n' + "\n" + "This would allow you to run `./pants green all-changed`, which is shorthand for " + "`./pants fmt lint check --changed-since=HEAD --changed-dependees=transitive`.\n\n" + "Notice: this option must be placed in a config file (e.g. `pants.toml` or " + "`pantsrc`) to have any effect." + ), + ) + + +@dataclass(frozen=True) +class CliAlias: + definitions: FrozenDict[str, tuple[str, ...]] = field(default_factory=FrozenDict) + + def __post_init__(self): + valid_alias_re = re.compile(r"\w(\w|-)*\w$", re.IGNORECASE) + for alias in self.definitions.keys(): + if not re.match(valid_alias_re, alias): + raise CliAliasInvalidError( + f"Invalid alias in `[cli].alias` option: {alias!r}. May only contain alpha " + "numerical letters and the separators `-` and `_`, and may not begin/end " + "with a `-`." + ) + + @classmethod + def from_dict(cls, aliases: dict[str, str]) -> CliAlias: + definitions = {key: tuple(shlex.split(value)) for key, value in aliases.items()} + + def expand( + definition: tuple[str, ...], *trail: str + ) -> Generator[tuple[str, ...], None, None]: + for arg in definition: + if arg not in definitions: + yield (arg,) + else: + if arg in trail: + raise CliAliasCycleError( + "CLI alias cycle detected in `[cli].alias` option: " + + " -> ".join([arg, *trail]) + ) + yield from expand(definitions[arg], arg, *trail) + + return cls( + FrozenDict( + { + alias: tuple(chain.from_iterable(expand(definition))) + for alias, definition in definitions.items() + } + ) + ) + + def check_name_conflicts(self, known_scopes: dict[str, ScopeInfo]) -> None: + for alias in self.definitions.keys(): + scope = known_scopes.get(alias) + if scope: + raise CliAliasInvalidError( + f"Invalid alias in `[cli].alias` option: {alias!r}. This is already a " + "registered " + ("goal." if scope.is_goal else "subsystem.") + ) + + def expand_args(self, args: tuple[str, ...]) -> tuple[str, ...]: + if not self.definitions: + return args + return tuple(self._do_expand_args(args)) + + def _do_expand_args(self, args: tuple[str, ...]) -> Generator[str, None, None]: + args_iter = iter(args) + for arg in args_iter: + if arg == "--": + # Do not expand pass through arguments. + yield arg + yield from args_iter + return + + expanded = self.maybe_expand(arg) + if expanded: + logger.debug(f"Expanded [cli.alias].{arg} => {' '.join(expanded)}") + yield from expanded + else: + yield arg + + def maybe_expand(self, arg: str) -> tuple[str, ...] | None: + return self.definitions.get(arg) diff --git a/src/python/pants/option/alias_test.py b/src/python/pants/option/alias_test.py new file mode 100644 index 000000000000..6a2e9de6f110 --- /dev/null +++ b/src/python/pants/option/alias_test.py @@ -0,0 +1,151 @@ +# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import annotations + +from typing import ContextManager + +import pytest + +from pants.option.alias import CliAlias, CliAliasCycleError, CliAliasInvalidError +from pants.option.scope import ScopeInfo +from pants.testutil.pytest_util import no_exception +from pants.util.frozendict import FrozenDict + + +def test_maybe_nothing() -> None: + cli_alias = CliAlias() + assert cli_alias.maybe_expand("arg") is None + + +@pytest.mark.parametrize( + "alias, expanded", + [ + ("--arg1", ("--arg1",)), + ("--arg1 --arg2", ("--arg1", "--arg2")), + ("--arg=value --option", ("--arg=value", "--option")), + ("--arg=value --option flag", ("--arg=value", "--option", "flag")), + ("--arg 'quoted value'", ("--arg", "quoted value")), + ], +) +def test_maybe_expand_alias(alias: str, expanded: tuple[str, ...] | None) -> None: + cli_alias = CliAlias.from_dict( + { + "alias": alias, + } + ) + assert cli_alias.maybe_expand("alias") == expanded + + +@pytest.mark.parametrize( + "args, expanded", + [ + ( + ("some", "alias", "target"), + ("some", "--flag", "goal", "target"), + ), + ( + # Don't touch pass through args. + ("some", "--", "alias", "target"), + ("some", "--", "alias", "target"), + ), + ], +) +def test_expand_args(args: tuple[str, ...], expanded: tuple[str, ...]) -> None: + cli_alias = CliAlias.from_dict( + { + "alias": "--flag goal", + } + ) + assert cli_alias.expand_args(args) == expanded + + +def test_no_expand_when_no_aliases() -> None: + args = ("./pants",) + cli_alias = CliAlias() + assert cli_alias.expand_args(args) is args + + +@pytest.mark.parametrize( + "alias, definitions", + [ + ( + { + "basic": "goal", + "nested": "--option=advanced basic", + }, + { + "basic": ("goal",), + "nested": ( + "--option=advanced", + "goal", + ), + }, + ), + ( + { + "multi-nested": "deep nested", + "basic": "goal", + "nested": "--option=advanced basic", + }, + { + "multi-nested": ("deep", "--option=advanced", "goal"), + "basic": ("goal",), + "nested": ( + "--option=advanced", + "goal", + ), + }, + ), + ( + { + "cycle": "other-alias", + "other-alias": "cycle", + }, + pytest.raises( + CliAliasCycleError, + match=( + r"CLI alias cycle detected in `\[cli\]\.alias` option: " + r"other-alias -> cycle -> other-alias" + ), + ), + ), + ], +) +def test_nested_alias(alias, definitions: dict | ContextManager) -> None: + expect: ContextManager = no_exception() if isinstance(definitions, dict) else definitions + with expect: + cli_alias = CliAlias.from_dict(alias) + if isinstance(definitions, dict): + assert cli_alias.definitions == FrozenDict(definitions) + + +@pytest.mark.parametrize( + "alias", + [ + # Check that we do not allow any alias that may resemble a valid option/spec. + "dir/spec", + "file.name", + "target:name", + "-o", + "--o", + "-option", + "--option", + ], +) +def test_invalid_alias_name(alias: str) -> None: + with pytest.raises( + CliAliasInvalidError, match=(f"Invalid alias in `\\[cli\\]\\.alias` option: {alias!r}\\.") + ): + CliAlias.from_dict({alias: ""}) + + +def test_banned_alias_names() -> None: + cli_alias = CliAlias.from_dict({"fmt": "--cleverness format"}) + with pytest.raises( + CliAliasInvalidError, + match=( + r"Invalid alias in `\[cli\]\.alias` option: 'fmt'\. This is already a registered goal\." + ), + ): + cli_alias.check_name_conflicts({"fmt": ScopeInfo("fmt", is_goal=True)}) diff --git a/src/python/pants/option/global_options.py b/src/python/pants/option/global_options.py index 91806883d468..49f29e4fb30a 100644 --- a/src/python/pants/option/global_options.py +++ b/src/python/pants/option/global_options.py @@ -14,7 +14,7 @@ from datetime import datetime from enum import Enum from pathlib import Path -from typing import Any, cast +from typing import Any, Type, cast from pants.base.build_environment import ( get_buildroot, @@ -36,7 +36,8 @@ from pants.util.dirutil import fast_relpath_optional from pants.util.docutil import doc_url from pants.util.logging import LogLevel -from pants.util.ordered_set import OrderedSet +from pants.util.memo import memoized_classmethod +from pants.util.ordered_set import FrozenOrderedSet, OrderedSet from pants.util.osutil import CPU_COUNT from pants.version import VERSION @@ -1599,3 +1600,29 @@ def compute_pantsd_invalidation_globs( ) return tuple(invalidation_globs) + + @memoized_classmethod + def get_options_flags(cls) -> GlobalOptionsFlags: + return GlobalOptionsFlags.create(cast("Type[GlobalOptions]", cls)) + + +@dataclass(frozen=True) +class GlobalOptionsFlags: + flags: FrozenOrderedSet[str] + short_flags: FrozenOrderedSet[str] + + @classmethod + def create(cls, GlobalOptionsType: Type[GlobalOptions]) -> GlobalOptionsFlags: + flags = set() + short_flags = set() + + def capture_the_flags(*args: str, **kwargs) -> None: + for arg in args: + flags.add(arg) + if len(arg) == 2: + short_flags.add(arg) + elif kwargs.get("type") == bool: + flags.add(f"--no-{arg[2:]}") + + GlobalOptionsType.register_bootstrap_options(capture_the_flags) + return cls(FrozenOrderedSet(flags), FrozenOrderedSet(short_flags)) diff --git a/src/python/pants/option/options_bootstrapper.py b/src/python/pants/option/options_bootstrapper.py index 839650c82c66..c519e660050e 100644 --- a/src/python/pants/option/options_bootstrapper.py +++ b/src/python/pants/option/options_bootstrapper.py @@ -13,6 +13,7 @@ from pants.base.build_environment import get_default_pants_config_file, pants_version from pants.base.exceptions import BuildConfigurationError from pants.build_graph.build_configuration import BuildConfiguration +from pants.option.alias import CliAlias from pants.option.config import Config from pants.option.custom_types import ListValueComponent from pants.option.global_options import GlobalOptions @@ -34,6 +35,7 @@ class OptionsBootstrapper: bootstrap_args: tuple[str, ...] args: tuple[str, ...] config: Config + alias: CliAlias def __repr__(self) -> str: env = {pair[0]: pair[1] for pair in self.env_tuples} @@ -114,12 +116,6 @@ def create( decision of whether to respect pantsrc files. """ with warnings.catch_warnings(record=True): - env = {k: v for k, v in env.items() if k.startswith("PANTS_")} - args = tuple(args) - - flags = set() - short_flags = set() - # We can't use pants.engine.fs.FileContent here because it would cause a circular dep. @dataclass(frozen=True) class FileContent: @@ -132,31 +128,8 @@ def filecontent_for(path: str) -> FileContent: read_file(path, binary_mode=True), ) - def capture_the_flags(*args: str, **kwargs) -> None: - for arg in args: - flags.add(arg) - if len(arg) == 2: - short_flags.add(arg) - elif kwargs.get("type") == bool: - flags.add(f"--no-{arg[2:]}") - - GlobalOptions.register_bootstrap_options(capture_the_flags) - - def is_bootstrap_option(arg: str) -> bool: - components = arg.split("=", 1) - if components[0] in flags: - return True - for flag in short_flags: - if arg.startswith(flag): - return True - return False - - # Take just the bootstrap args, so we don't choke on other global-scope args on the cmd line. - # Stop before '--' since args after that are pass-through and may have duplicate names to our - # bootstrap options. - bargs = ("./pants",) + tuple( - filter(is_bootstrap_option, itertools.takewhile(lambda arg: arg != "--", args)) - ) + env = {k: v for k, v in env.items() if k.startswith("PANTS_")} + bargs = cls._get_bootstrap_args(args) config_file_paths = cls.get_config_file_paths(env=env, args=args) config_files_products = [filecontent_for(p) for p in config_file_paths] @@ -185,10 +158,45 @@ def is_bootstrap_option(arg: str) -> bool: ) env_tuples = tuple(sorted(env.items(), key=lambda x: x[0])) + + # Finally, we expand any aliases and re-populates the bootstrap args, in case there was + # any from an alias. + # stuhood: This could potentially break the rust client when aliases are used: + # https://github.com/pantsbuild/pants/pull/13228#discussion_r728223889 + alias = CliAlias.from_dict(post_bootstrap_config.get("cli", "alias", dict, {})) + args = alias.expand_args(tuple(args)) + bargs = cls._get_bootstrap_args(args) + return cls( - env_tuples=env_tuples, bootstrap_args=bargs, args=args, config=post_bootstrap_config + env_tuples=env_tuples, + bootstrap_args=bargs, + args=args, + config=post_bootstrap_config, + alias=alias, ) + @classmethod + def _get_bootstrap_args(cls, args: Sequence[str]) -> tuple[str, ...]: + # TODO(13244): there is a typing issue with `memoized_classmethod`. + options = GlobalOptions.get_options_flags() # type: ignore[call-arg] + + def is_bootstrap_option(arg: str) -> bool: + components = arg.split("=", 1) + if components[0] in options.flags: + return True + for flag in options.short_flags: + if arg.startswith(flag): + return True + return False + + # Take just the bootstrap args, so we don't choke on other global-scope args on the cmd line. + # Stop before '--' since args after that are pass-through and may have duplicate names to our + # bootstrap options. + bargs = ("./pants",) + tuple( + filter(is_bootstrap_option, itertools.takewhile(lambda arg: arg != "--", args)) + ) + return bargs + @memoized_property def env(self) -> dict[str, str]: return dict(self.env_tuples) @@ -262,4 +270,5 @@ def full_options(self, build_configuration: BuildConfiguration) -> Options: known_scope_infos, allow_unknown_options=build_configuration.allow_unknown_options ) GlobalOptions.validate_instance(options.for_global_scope()) + self.alias.check_name_conflicts(options.known_scope_to_info) return options diff --git a/src/python/pants/option/options_bootstrapper_test.py b/src/python/pants/option/options_bootstrapper_test.py index c62ea1796882..2609e61e21a7 100644 --- a/src/python/pants/option/options_bootstrapper_test.py +++ b/src/python/pants/option/options_bootstrapper_test.py @@ -407,3 +407,38 @@ def test_setting_pants_config_in_config(self) -> None: ) logdir = ob.get_bootstrap_options().for_global_scope().logdir self.assertEqual("logdir1", logdir) + + def test_alias_pyupgrade(self) -> None: + with temporary_dir() as tmpdir: + config = os.path.join(tmpdir, "config") + with open(config, "w") as out: + out.write( + dedent( + """\ + [cli.alias] + pyupgrade = "--backend-packages=pants.backend.python.lint.pyupgrade fmt" + """ + ) + ) + + config_arg = f"--pants-config-files=['{config}']" + ob = OptionsBootstrapper.create( + env={}, args=[config_arg, "pyupgrade"], allow_pantsrc=False + ) + + self.assertEqual( + ( + config_arg, + "--backend-packages=pants.backend.python.lint.pyupgrade", + "fmt", + ), + ob.args, + ) + self.assertEqual( + ( + "./pants", + config_arg, + "--backend-packages=pants.backend.python.lint.pyupgrade", + ), + ob.bootstrap_args, + ) diff --git a/src/python/pants/option/options_test.py b/src/python/pants/option/options_test.py index 089610c4c1cb..4f7762e55852 100644 --- a/src/python/pants/option/options_test.py +++ b/src/python/pants/option/options_test.py @@ -504,6 +504,11 @@ def register(opts: Options) -> None: assert not caplog.records +# ---------------------------------------------------------------------------------------- +# Legacy Unittest TestCase. +# ---------------------------------------------------------------------------------------- + + class OptionsTest(unittest.TestCase): @staticmethod def _create_config(config: dict[str, dict[str, str]] | None = None) -> Config: @@ -1125,7 +1130,7 @@ def test_at_most_one_goal_with_passthru_args(self): with pytest.raises(Options.AmbiguousPassthroughError) as exc: Options.create( env={}, - config={}, + config=self._create_config(), known_scope_infos=[global_scope(), task("test"), task("fmt")], args=["./pants", "test", "fmt", "target", "--", "bar", "--baz"], )