Skip to content

Commit

Permalink
Port the cli aliasing feature to rust (#21695)
Browse files Browse the repository at this point in the history
This is a step on the road to getting rid of the 
legacy options parser code, and greatly simplifying
the OptionsBootstrapper.

A followup change will remove the legacy CliAlias code.
  • Loading branch information
benjyw authored Dec 11, 2024
1 parent 333c239 commit a0bcae9
Show file tree
Hide file tree
Showing 22 changed files with 620 additions and 123 deletions.
6 changes: 4 additions & 2 deletions pants-plugins/internal_plugins/releases/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from pants.engine.rules import Get, collect_rules, goal_rule, rule
from pants.engine.target import Target
from pants.engine.unions import UnionRule
from pants.option.alias import CliAlias
from pants.option.config import Config
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.util.strutil import softwrap
Expand Down Expand Up @@ -132,7 +131,10 @@ async def check_default_tools(
SessionValues(
{
OptionsBootstrapper: OptionsBootstrapper(
tuple(), ("./pants",), args, Config(tuple()), CliAlias()
tuple(),
("./pants",),
args,
Config(tuple()),
)
}
),
Expand Down
2 changes: 2 additions & 0 deletions src/python/pants/engine/internals/native_engine.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,7 @@ class PyOptionParser:
configs: Optional[Sequence[PyConfigSource]],
allow_pantsrc: bool,
include_derivation: bool,
known_scopes_to_flags: Optional[dict[str, frozenset[str]]],
) -> None: ...
def get_bool(self, option_id: PyOptionId, default: Optional[bool]) -> OptionValue[bool]: ...
def get_int(self, option_id: PyOptionId, default: Optional[int]) -> OptionValue[int]: ...
Expand All @@ -692,6 +693,7 @@ class PyOptionParser:
self, option_id: PyOptionId, default: list[str]
) -> OptionValue[list[str]]: ...
def get_dict(self, option_id: PyOptionId, default: dict[str, Any]) -> OptionValue[dict]: ...
def get_args(self) -> list[str]: ...
def get_passthrough_args(self) -> Optional[list[str]]: ...
def get_unconsumed_flags(self) -> dict[str, list[str]]: ...
def validate_config(self, valid_keys: dict[str, set[str]]) -> list[str]: ...
Expand Down
7 changes: 6 additions & 1 deletion src/python/pants/help/help_formatter_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,12 @@ def _get_registrar_and_parser(cls) -> Tuple[OptionRegistrar, NativeOptionParser]
return OptionRegistrar(
scope=GlobalOptions.options_scope,
), NativeOptionParser(
args=[], env={}, config_sources=[], allow_pantsrc=False, include_derivation=True
args=[],
env={},
config_sources=[],
allow_pantsrc=False,
include_derivation=True,
known_scopes_to_flags={},
)

@classmethod
Expand Down
8 changes: 6 additions & 2 deletions src/python/pants/help/help_info_extracter_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,9 @@ def do_test(args, kwargs, expected_default_str):
registrar = OptionRegistrar(
scope=GlobalOptions.options_scope,
)
native_parser = NativeOptionParser([], {}, [], allow_pantsrc=False, include_derivation=True)
native_parser = NativeOptionParser(
[], {}, [], allow_pantsrc=False, include_derivation=True, known_scopes_to_flags={}
)
registrar.register(*args, **kwargs)
oshi = HelpInfoExtracter(registrar.scope).get_option_scope_help_info(
"description", registrar, native_parser, False, "provider"
Expand Down Expand Up @@ -199,7 +201,9 @@ def exp_to_len(exp):
return int(exp) # True -> 1, False -> 0.

registrar = OptionRegistrar(scope=GlobalOptions.options_scope)
native_parser = NativeOptionParser([], {}, [], allow_pantsrc=False, include_derivation=True)
native_parser = NativeOptionParser(
[], {}, [], allow_pantsrc=False, include_derivation=True, known_scopes_to_flags={}
)
registrar.register("--foo", **kwargs)
oshi = HelpInfoExtracter("").get_option_scope_help_info(
"", registrar, native_parser, False, ""
Expand Down
4 changes: 3 additions & 1 deletion src/python/pants/help/help_tools_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ def registrar() -> OptionRegistrar:

@pytest.fixture
def native_parser() -> NativeOptionParser:
return NativeOptionParser([], {}, [], allow_pantsrc=False, include_derivation=True)
return NativeOptionParser(
[], {}, [], allow_pantsrc=False, include_derivation=True, known_scopes_to_flags={}
)


@pytest.fixture
Expand Down
15 changes: 14 additions & 1 deletion src/python/pants/option/native_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,21 @@ def __init__(
config_sources: Optional[Sequence[ConfigSource]],
allow_pantsrc: bool,
include_derivation: bool,
known_scopes_to_flags: dict[str, frozenset[str]],
):
# Remember these args so this object can clone itself in with_derivation() below.
self._args, self._env, self._config_sources, self._allow_pantsrc = (
(
self._args,
self._env,
self._config_sources,
self._allow_pantsrc,
self._known_scopes_to_flags,
) = (
args,
env,
config_sources,
allow_pantsrc,
known_scopes_to_flags,
)

py_config_sources = (
Expand All @@ -82,6 +90,7 @@ def __init__(
py_config_sources,
allow_pantsrc,
include_derivation,
known_scopes_to_flags,
)

# (type, member_type) -> native get for that type.
Expand All @@ -108,6 +117,7 @@ def with_derivation(self) -> NativeOptionParser:
config_sources=None if self._config_sources is None else tuple(self._config_sources),
allow_pantsrc=self._allow_pantsrc,
include_derivation=True,
known_scopes_to_flags=self._known_scopes_to_flags,
)

def get_value(self, *, scope, registration_args, registration_kwargs) -> Tuple[Any, Rank]:
Expand Down Expand Up @@ -272,6 +282,9 @@ def check_scalar_value(val, choices):

return (val, rank, derivation)

def get_args(self) -> tuple[str, ...]:
return tuple(self._native_parser.get_args())

def get_unconsumed_flags(self) -> dict[str, tuple[str, ...]]:
return {k: tuple(v) for k, v in self._native_parser.get_unconsumed_flags().items()}

Expand Down
40 changes: 24 additions & 16 deletions src/python/pants/option/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,28 @@ def create(
# We need registrars for all the intermediate scopes, so inherited option values
# can propagate through them.
complete_known_scope_infos = cls.complete_scopes(known_scope_infos)

registrar_by_scope = {
si.scope: OptionRegistrar(si.scope) for si in complete_known_scope_infos
}
known_scope_to_info = {s.scope: s for s in complete_known_scope_infos}
known_scope_to_flags = {
scope: registrar.known_scoped_args for scope, registrar in registrar_by_scope.items()
}

config_to_pass = None if native_options_config_discovery else config.sources()
native_parser = NativeOptionParser(
args[1:], # The native parser expects args without the sys.argv[0] binary name.
env,
config_sources=config_to_pass,
allow_pantsrc=True,
include_derivation=include_derivation,
known_scopes_to_flags=known_scope_to_flags,
)

splitter = ArgSplitter(complete_known_scope_infos, get_buildroot())
split_args = splitter.split_args(args)
# We take the cli alias-expanded args[1:] from the native parser.
split_args = splitter.split_args([args[0], *native_parser.get_args()])

if split_args.passthru and len(split_args.goals) > 1:
raise cls.AmbiguousPassthroughError(
Expand All @@ -166,21 +186,6 @@ def create(
[line for line in [line.strip() for line in f] if line]
)

registrar_by_scope = {
si.scope: OptionRegistrar(si.scope) for si in complete_known_scope_infos
}
known_scope_to_info = {s.scope: s for s in complete_known_scope_infos}

config_to_pass = None if native_options_config_discovery else config.sources()

native_parser = NativeOptionParser(
args,
env,
config_sources=config_to_pass,
allow_pantsrc=True,
include_derivation=include_derivation,
)

return cls(
builtin_or_auxiliary_goal=split_args.builtin_or_auxiliary_goal,
goals=split_args.goals,
Expand Down Expand Up @@ -300,6 +305,9 @@ def verify_configs(self, global_config: Config) -> None:
)
global_config.verify(section_to_valid_options)

def get_args(self) -> tuple[str, ...]:
return self._native_parser.get_args()

def verify_args(self):
# Consume all known args, and see if any are left.
# This will have the side-effect of precomputing (and memoizing) options for all scopes.
Expand Down
18 changes: 2 additions & 16 deletions src/python/pants/option/options_bootstrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@
from pants.base.build_environment import get_buildroot, get_default_pants_config_file, pants_version
from pants.base.exceptions import BuildConfigurationError
from pants.engine.unions import UnionMembership
from pants.option.alias import CliAlias
from pants.option.config import Config
from pants.option.custom_types import DictValueComponent, ListValueComponent
from pants.option.custom_types import ListValueComponent
from pants.option.global_options import BootstrapOptions, GlobalOptions
from pants.option.option_types import collect_options_info
from pants.option.options import Options
Expand All @@ -39,7 +38,6 @@ 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}
Expand Down Expand Up @@ -121,6 +119,7 @@ def create(
absolute paths. Production use-cases should pass True to allow options values to make the
decision of whether to respect pantsrc files.
"""
args = tuple(args)
with warnings.catch_warnings(record=True):
# We can't use pants.engine.fs.FileContent here because it would cause a circular dep.
@dataclass(frozen=True)
Expand Down Expand Up @@ -162,15 +161,6 @@ def filecontent_for(path: str) -> FileContent:
env=env,
)

# Finally, we expand any aliases and re-populate the bootstrap args, in case there
# were any from aliases.
# stuhood: This could potentially break the rust client when aliases are used:
# https://github.com/pantsbuild/pants/pull/13228#discussion_r728223889
alias_vals = post_bootstrap_config.get("cli", "alias")
val = DictValueComponent.merge([DictValueComponent.create(v) for v in alias_vals]).val
alias = CliAlias.from_dict(val)

args = alias.expand_args(tuple(args))
bargs = cls._get_bootstrap_args(args)

# We need to set this env var to allow various static help strings to reference the
Expand All @@ -196,7 +186,6 @@ def filecontent_for(path: str) -> FileContent:
bootstrap_args=bargs,
args=args,
config=post_bootstrap_config,
alias=alias,
)

@classmethod
Expand Down Expand Up @@ -310,9 +299,6 @@ def full_options(
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, options.known_scope_to_scoped_args
)
return options


Expand Down
66 changes: 1 addition & 65 deletions src/python/pants/option/options_bootstrapper_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def test_create_bootstrapped_options(self) -> None:
)
)
fp.close()
args = ["--pants-workdir=/qux"] + self._config_path(fp.name)
args = ["pants", "--pants-workdir=/qux"] + self._config_path(fp.name)
bootstrapper = OptionsBootstrapper.create(
env={"PANTS_DISTDIR": "/pear"}, args=args, allow_pantsrc=False
)
Expand Down Expand Up @@ -407,70 +407,6 @@ def test_setting_pants_config_in_config(self, tmp_path: Path) -> None:
logdir = ob.get_bootstrap_options().for_global_scope().logdir
assert "logdir1" == logdir

def test_alias(self, tmp_path: Path) -> None:
config0 = tmp_path / "config0"
config0.write_text(
dedent(
"""\
[cli.alias]
pyupgrade = "--backend-packages=pants.backend.python.lint.pyupgrade fmt"
green = "lint test"
"""
)
)

config1 = tmp_path / "config1"
config1.write_text(
dedent(
"""\
[cli]
alias.add = {green = "lint test --force check"}
"""
)
)

config2 = tmp_path / "config2"
config2.write_text(
dedent(
"""\
[cli]
alias = "+{'shell': 'repl'}"
"""
)
)

config_arg = (
f"--pants-config-files=["
f"'{config0.as_posix()}','{config1.as_posix()}','{config2.as_posix()}']"
)
ob = OptionsBootstrapper.create(
env={}, args=[config_arg, "pyupgrade", "green"], allow_pantsrc=False
)
assert (
config_arg,
"--backend-packages=pants.backend.python.lint.pyupgrade",
"fmt",
"lint",
"test",
"--force",
"check",
) == ob.args
assert (
"<ignored>",
config_arg,
"--backend-packages=pants.backend.python.lint.pyupgrade",
) == ob.bootstrap_args

ob = OptionsBootstrapper.create(env={}, args=[config_arg, "shell"], allow_pantsrc=False)
assert (
config_arg,
"repl",
) == ob.args
assert (
"<ignored>",
config_arg,
) == ob.bootstrap_args


def test_munge_bin_name():
build_root = "/my/repo"
Expand Down
Loading

0 comments on commit a0bcae9

Please sign in to comment.