From 85003848427566f9611a79009c083600f22d9043 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Thu, 2 Jul 2020 17:04:32 -0700 Subject: [PATCH] Remove the concept of a scope category. (#10224) In v2 all optionables are subsystems, even global options. There is no meaning to categories any more. [ci skip-rust-tests] --- src/python/pants/engine/goal.py | 21 +---- src/python/pants/help/help_printer.py | 38 ++------ .../pants/help/list_goals_integration_test.py | 2 +- src/python/pants/help/scope_info_iterator.py | 93 ------------------- .../pants/help/scope_info_iterator_test.py | 72 -------------- src/python/pants/option/arg_splitter.py | 2 +- src/python/pants/option/arg_splitter_test.py | 28 ++---- src/python/pants/option/global_options.py | 3 +- src/python/pants/option/optionable.py | 7 +- src/python/pants/option/options.py | 7 +- .../pants/option/options_bootstrapper.py | 22 ----- .../pants/option/options_bootstrapper_test.py | 44 ++------- src/python/pants/option/options_test.py | 17 +--- src/python/pants/option/parser.py | 4 +- src/python/pants/option/scope.py | 13 +-- src/python/pants/subsystem/subsystem.py | 4 +- .../pants/subsystem/subsystem_client_mixin.py | 6 +- src/python/pants/subsystem/subsystem_test.py | 2 +- src/python/pants/task/task.py | 3 - 19 files changed, 48 insertions(+), 340 deletions(-) delete mode 100644 src/python/pants/help/scope_info_iterator.py delete mode 100644 src/python/pants/help/scope_info_iterator_test.py diff --git a/src/python/pants/engine/goal.py b/src/python/pants/engine/goal.py index b4c6803e401..7b4e2ebad80 100644 --- a/src/python/pants/engine/goal.py +++ b/src/python/pants/engine/goal.py @@ -7,8 +7,6 @@ from typing import ClassVar, Tuple, Type from pants.option.optionable import Optionable -from pants.option.options_bootstrapper import is_v2_exclusive -from pants.option.scope import ScopeInfo from pants.subsystem.subsystem_client_mixin import SubsystemClientMixin from pants.util.meta import classproperty @@ -31,8 +29,6 @@ def list(console: Console, options: ListOptions) -> List: ``` """ - options_scope_category = ScopeInfo.GOAL - # If the goal requires downstream implementations to work properly, such as `test` and `run`, # it should declare the union types that must have members. required_union_implementations: Tuple[Type, ...] = () @@ -43,22 +39,9 @@ def name(cls): """The name used to select the corresponding Goal on the commandline and the options_scope for its options.""" - @classmethod - def conflict_free_name(cls): - # v2 goal names ending in '2' are assumed to be so-named to avoid conflict with a v1 goal. - # If we're in v2-exclusive mode, strip off the '2', so we can use the more natural name. - # Note that this implies a change of options scope when changing to v2-only mode: options - # on the foo2 scope will need to be moved to the foo scope. But we already expect options - # changes when switching to v2-exclusive mode (e.g., some v1 options aren't even registered - # in that mode), so this is in keeping with existing expectations. And this provides - # a much better experience to new users who never encountered v1 anyway. - if is_v2_exclusive and cls.name.endswith("2"): - return cls.name[:-1] - return cls.name - @classproperty def options_scope(cls): - return cls.conflict_free_name() + return cls.name def __init__(self, scope, scoped_options): # NB: This constructor is shaped to meet the contract of `Optionable(Factory).signature`. @@ -97,7 +80,7 @@ class List(Goal): @classproperty def name(cls): - return cls.subsystem_cls.conflict_free_name() + return cls.subsystem_cls.name class Outputting: diff --git a/src/python/pants/help/help_printer.py b/src/python/pants/help/help_printer.py index 447a40687eb..9c4c9a7e746 100644 --- a/src/python/pants/help/help_printer.py +++ b/src/python/pants/help/help_printer.py @@ -11,9 +11,7 @@ from pants.base.build_environment import pants_release, pants_version from pants.engine.goal import GoalSubsystem from pants.engine.unions import UnionMembership -from pants.goal.goal import Goal from pants.help.help_formatter import HelpFormatter -from pants.help.scope_info_iterator import ScopeInfoIterator from pants.option.arg_splitter import ( GoalsHelp, HelpRequest, @@ -72,12 +70,7 @@ def _print_goals_help(self) -> None: global_options = self._options.for_global_scope() goal_descriptions: Dict[str, str] = {} if global_options.v2: - goal_scope_infos = [ - scope_info - for scope_info in self._options.known_scope_to_info.values() - if scope_info.category == ScopeInfo.GOAL - ] - for scope_info in goal_scope_infos: + for scope_info in self._options.known_scope_to_info.values(): optionable_cls = scope_info.optionable_cls if optionable_cls is None or not issubclass(optionable_cls, GoalSubsystem): continue @@ -88,31 +81,27 @@ def _print_goals_help(self) -> None: continue description = scope_info.description or "" goal_descriptions[scope_info.scope] = description - if global_options.v1: - goal_descriptions.update( - {goal.name: goal.description_first_line for goal in Goal.all() if goal.description} - ) title_text = "Goals" title = f"{title_text}\n{'-' * len(title_text)}" if self._use_color: title = green(title) - max_width = max(len(name) for name in goal_descriptions.keys()) + max_width = max((len(name) for name in goal_descriptions.keys()), default=0) chars_before_description = max_width + 2 - def format_goal(name: str, description: str) -> str: + def format_goal(name: str, descr: str) -> str: name = name.ljust(chars_before_description) if self._use_color: name = cyan(name) - description_lines = textwrap.wrap(description, 80 - chars_before_description) + description_lines = textwrap.wrap(descr, 80 - chars_before_description) if len(description_lines) > 1: description_lines = [ description_lines[0], *(f"{' ' * chars_before_description}{line}" for line in description_lines[1:]), ] - description = "\n".join(description_lines) - return f"{name}{description}\n" + formatted_descr = "\n".join(description_lines) + return f"{name}{formatted_descr}\n" lines = [ f"\n{title}\n", @@ -134,7 +123,6 @@ def _print_options_help(self) -> None: """ help_request = cast(OptionsHelp, self._help_request) - global_options = self._options.for_global_scope() if help_request.all_scopes: help_scopes = set(self._options.known_scope_to_info.keys()) @@ -142,15 +130,7 @@ def _print_options_help(self) -> None: # The scopes explicitly mentioned by the user on the cmd line. help_scopes = set(self._options.scope_to_flags.keys()) - {GLOBAL_SCOPE} - # If --v1 is enabled at all, don't use v2_help, even if --v2 is also enabled. - v2_help = global_options.v2 and not global_options.v1 - - scope_info_iterator = ScopeInfoIterator( - scope_to_info=self._options.known_scope_to_info, v2_help=v2_help - ) - - scope_infos = list(scope_info_iterator.iterate(help_scopes)) - + scope_infos = list(self._options.known_scope_to_info[scope] for scope in help_scopes) if scope_infos: for scope_info in scope_infos: help_str = self._format_help(scope_info, help_request.advanced) @@ -188,9 +168,7 @@ def _print_options_help(self) -> None: print(" dir:: to include all targets found recursively under the directory.") print("\nFriendly docs:\n http://pantsbuild.org/") - print( - self._format_help(ScopeInfo(GLOBAL_SCOPE, ScopeInfo.GLOBAL), help_request.advanced) - ) + print(self._format_help(ScopeInfo(GLOBAL_SCOPE), help_request.advanced)) def _format_help(self, scope_info: ScopeInfo, show_advanced_and_deprecated: bool) -> str: """Return a help message for the options registered on this object. diff --git a/src/python/pants/help/list_goals_integration_test.py b/src/python/pants/help/list_goals_integration_test.py index 73180c148d1..4a8209af38f 100644 --- a/src/python/pants/help/list_goals_integration_test.py +++ b/src/python/pants/help/list_goals_integration_test.py @@ -20,7 +20,7 @@ def test_only_show_implemented_goals(self) -> None: goals_that_need_implementation = ["binary", "fmt", "lint", "run", "test"] command = ["--pants-config-files=[]", "goals"] - not_implemented_run = self.run_pants(command) + not_implemented_run = self.run_pants(["--backend-packages=[]", *command,]) self.assert_success(not_implemented_run) for goal in goals_that_need_implementation: assert goal not in not_implemented_run.stdout_data diff --git a/src/python/pants/help/scope_info_iterator.py b/src/python/pants/help/scope_info_iterator.py deleted file mode 100644 index 47cdaecd436..00000000000 --- a/src/python/pants/help/scope_info_iterator.py +++ /dev/null @@ -1,93 +0,0 @@ -# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -from dataclasses import dataclass -from typing import Any, Dict, Iterator, List, Set, Type - -from pants.option.global_options import GlobalOptions -from pants.option.parser_hierarchy import enclosing_scope -from pants.option.scope import GLOBAL_SCOPE, ScopeInfo -from pants.subsystem.subsystem_client_mixin import SubsystemClientMixin - - -@dataclass(frozen=True) -class ScopeInfoIterator: - """Provides relevant ScopeInfo instances in a useful order.""" - - scope_to_info: Dict[str, ScopeInfo] - v2_help: bool = False - - def iterate(self, scopes: Set[str]) -> Iterator[ScopeInfo]: - """Yields ScopeInfo instances for the specified scopes, plus relevant related scopes. - - Relevant scopes are: - - All tasks in a requested goal. - - All subsystems tied to a request scope. - - Yields in a sensible order: Sorted by scope, but with subsystems tied to a request scope - following that scope, e.g., - - goal1 - goal1.task11 - subsys.goal1.task11 - goal1.task12 - goal2.task21 - ... - """ - - scope_infos: List[ScopeInfo] = [] - - if self.v2_help: - scope_infos.extend(sorted(self.scope_to_info[s] for s in scopes)) - else: - scope_infos.extend(self.scope_to_info[s] for s in self._expand_tasks(scopes)) - - for info in self._expand_subsystems(scope_infos): - yield info - - def _expand_tasks(self, scopes: Set[str]) -> List[str]: - """Add all tasks in any requested goals. - - Returns the requested scopes, plus the added tasks, sorted by scope name. - """ - expanded_scopes = set(scopes) - for scope, info in self.scope_to_info.items(): - if info.category == ScopeInfo.TASK: - outer = enclosing_scope(scope) - while outer != GLOBAL_SCOPE: - if outer in expanded_scopes: - expanded_scopes.add(scope) - break - outer = enclosing_scope(outer) - return sorted(expanded_scopes) - - def _expand_subsystems(self, scope_infos: List[ScopeInfo]) -> Iterator[ScopeInfo]: - """Add all subsystems tied to a scope, right after that scope.""" - - # Get non-global subsystem dependencies of the specified subsystem client. - def subsys_deps(subsystem_client_cls: Type[Any]) -> Iterator[ScopeInfo]: - for dep in subsystem_client_cls.subsystem_dependencies_iter(): - if dep.scope != GLOBAL_SCOPE: - yield self.scope_to_info[dep.options_scope] - for x in subsys_deps(dep.subsystem_cls): - yield x - - for scope_info in scope_infos: - yield scope_info - if scope_info.optionable_cls is not None: - # We don't currently subclass GlobalOptions, and I can't think of any reason why - # we would, but might as well be robust. - if issubclass(scope_info.optionable_cls, GlobalOptions): - # We were asked for global help, so also yield for all global subsystems. - for scope, info in self.scope_to_info.items(): - if ( - info.category == ScopeInfo.SUBSYSTEM - and enclosing_scope(scope) == GLOBAL_SCOPE - ): - yield info - if info.optionable_cls is not None: - for subsys_dep in subsys_deps(info.optionable_cls): - yield subsys_dep - elif issubclass(scope_info.optionable_cls, SubsystemClientMixin): - for subsys_dep in subsys_deps(scope_info.optionable_cls): - yield subsys_dep diff --git a/src/python/pants/help/scope_info_iterator_test.py b/src/python/pants/help/scope_info_iterator_test.py deleted file mode 100644 index 7d0c40e76cf..00000000000 --- a/src/python/pants/help/scope_info_iterator_test.py +++ /dev/null @@ -1,72 +0,0 @@ -# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -import unittest - -from pants.help.scope_info_iterator import ScopeInfoIterator -from pants.option.global_options import GlobalOptions -from pants.option.scope import GLOBAL_SCOPE, ScopeInfo -from pants.subsystem.subsystem import Subsystem -from pants.subsystem.subsystem_client_mixin import SubsystemDependency -from pants.task.task import Task - - -class ScopeInfoIteratorTest(unittest.TestCase): - def test_iteration(self): - self.maxDiff = None - - class Subsys1(Subsystem): - options_scope = "subsys1" - - class Subsys2(Subsystem): - options_scope = "subsys2" - - @classmethod - def subsystem_dependencies(cls): - return (SubsystemDependency(Subsys1, "subsys2"),) - - class Goal1Task2(Task): - options_scope = "goal1.task12" - - @classmethod - def subsystem_dependencies(cls): - return (SubsystemDependency(Subsys1, "goal1.task12"),) - - infos = [ - ScopeInfo(GLOBAL_SCOPE, ScopeInfo.GLOBAL, GlobalOptions), - ScopeInfo("subsys2", ScopeInfo.SUBSYSTEM, Subsys2), - ScopeInfo("subsys1.subsys2", ScopeInfo.SUBSYSTEM, Subsys1), - ScopeInfo("goal1", ScopeInfo.INTERMEDIATE), - ScopeInfo("goal1.task11", ScopeInfo.TASK), - ScopeInfo("goal1.task12", ScopeInfo.TASK, Goal1Task2), - ScopeInfo("subsys1.goal1.task12", ScopeInfo.SUBSYSTEM, Subsys1), - ScopeInfo("goal2", ScopeInfo.INTERMEDIATE), - ScopeInfo("goal2.task21", ScopeInfo.TASK), - ScopeInfo("goal2.task22", ScopeInfo.TASK), - ScopeInfo("goal3", ScopeInfo.INTERMEDIATE), - ScopeInfo("goal3.task31", ScopeInfo.TASK), - ScopeInfo("goal3.task32", ScopeInfo.TASK), - ] - - scope_to_infos = dict((x.scope, x) for x in infos) - - it = ScopeInfoIterator(scope_to_infos) - actual = list(it.iterate({GLOBAL_SCOPE, "goal1", "goal2.task21", "goal3"})) - - expected_scopes = [ - GLOBAL_SCOPE, - "subsys2", - "subsys1.subsys2", - "goal1", - "goal1.task11", - "goal1.task12", - "subsys1.goal1.task12", - "goal2.task21", - "goal3", - "goal3.task31", - "goal3.task32", - ] - - expected_scope_infos = [scope_to_infos[x] for x in expected_scopes] - - self.assertEqual(expected_scope_infos, actual) diff --git a/src/python/pants/option/arg_splitter.py b/src/python/pants/option/arg_splitter.py index 9af03291ed2..299a9824609 100644 --- a/src/python/pants/option/arg_splitter.py +++ b/src/python/pants/option/arg_splitter.py @@ -270,7 +270,7 @@ def _descope_flag(self, flag: str, default_scope: str) -> Tuple[str, str]: prefix = flag_prefix + scope_prefix if flag.startswith(prefix): scope = scope_info.scope - if scope_info.category == ScopeInfo.SUBSYSTEM and default_scope != GLOBAL_SCOPE: + if scope != GLOBAL_SCOPE and default_scope != GLOBAL_SCOPE: # We allow goal.task --subsystem-foo to refer to the task-level subsystem instance, # i.e., as if qualified by --subsystem-goal-task-foo. # Note that this means that we can't set a task option on the cmd-line if its diff --git a/src/python/pants/option/arg_splitter_test.py b/src/python/pants/option/arg_splitter_test.py index 02fe5da2949..7580473c6f5 100644 --- a/src/python/pants/option/arg_splitter_test.py +++ b/src/python/pants/option/arg_splitter_test.py @@ -18,28 +18,16 @@ from pants.util.contextutil import pushd, temporary_dir -def task(scope: str) -> ScopeInfo: - return ScopeInfo(scope, ScopeInfo.TASK) - - -def intermediate(scope: str) -> ScopeInfo: - return ScopeInfo(scope, ScopeInfo.INTERMEDIATE) - - -def subsys(scope: str) -> ScopeInfo: - return ScopeInfo(scope, ScopeInfo.SUBSYSTEM) - - class ArgSplitterTest(unittest.TestCase): _known_scope_infos = [ - intermediate("compile"), - task("compile.java"), - task("compile.scala"), - subsys("jvm"), - subsys("jvm.test.junit"), - subsys("reporting"), - intermediate("test"), - task("test.junit"), + ScopeInfo("compile"), + ScopeInfo("compile.java"), + ScopeInfo("compile.scala"), + ScopeInfo("jvm"), + ScopeInfo("jvm.test.junit"), + ScopeInfo("reporting"), + ScopeInfo("test"), + ScopeInfo("test.junit"), ] def assert_valid_split( diff --git a/src/python/pants/option/global_options.py b/src/python/pants/option/global_options.py index 2a0cb1cc150..9aab573cd24 100644 --- a/src/python/pants/option/global_options.py +++ b/src/python/pants/option/global_options.py @@ -18,7 +18,7 @@ ) from pants.option.custom_types import dir_option from pants.option.errors import OptionsError -from pants.option.scope import GLOBAL_SCOPE, ScopeInfo +from pants.option.scope import GLOBAL_SCOPE from pants.subsystem.subsystem import Subsystem from pants.util.logging import LogLevel @@ -146,7 +146,6 @@ def from_bootstrap_options(cls, bootstrap_options): class GlobalOptions(Subsystem): options_scope = GLOBAL_SCOPE - options_scope_category = ScopeInfo.GLOBAL @classmethod def register_bootstrap_options(cls, register): diff --git a/src/python/pants/option/optionable.py b/src/python/pants/option/optionable.py index a8ea44798fb..0f5443d72e5 100644 --- a/src/python/pants/option/optionable.py +++ b/src/python/pants/option/optionable.py @@ -70,7 +70,6 @@ class Optionable(OptionableFactory, metaclass=ABCMeta): # Subclasses must override. options_scope: Optional[str] = None - options_scope_category: Optional[str] = None # Subclasses may override these to specify a deprecated former name for this Optionable's scope. # Option values can be read from the deprecated scope, but a deprecation warning will be issued. @@ -101,9 +100,9 @@ def validate_scope_name_component(cls, s: str) -> None: @classmethod def get_scope_info(cls): """Returns a ScopeInfo instance representing this Optionable's options scope.""" - if cls.options_scope is None or cls.options_scope_category is None: - raise OptionsError(f"{cls.__name__} must set options_scope and options_scope_category.") - return ScopeInfo(cls.options_scope, cls.options_scope_category, cls) + if cls.options_scope is None: + raise OptionsError(f"{cls.__name__} must set options_scope.") + return ScopeInfo(cls.options_scope, cls) @classmethod def subscope(cls, scope): diff --git a/src/python/pants/option/options.py b/src/python/pants/option/options.py index 1839bf76d67..8644b6b0e34 100644 --- a/src/python/pants/option/options.py +++ b/src/python/pants/option/options.py @@ -98,7 +98,7 @@ def complete_scopes(cls, scope_infos: Iterable[ScopeInfo]) -> FrozenOrderedSet[S ) original_scopes[si.scope] = si if si.deprecated_scope: - ret.add(ScopeInfo(si.deprecated_scope, si.category, si.optionable_cls)) + ret.add(ScopeInfo(si.deprecated_scope, si.optionable_cls)) original_scopes[si.deprecated_scope] = si # TODO: Once scope name validation is enforced (so there can be no dots in scope name @@ -107,7 +107,7 @@ def complete_scopes(cls, scope_infos: Iterable[ScopeInfo]) -> FrozenOrderedSet[S for si in copy.copy(ret): for scope in all_enclosing_scopes(si.scope, allow_global=False): if scope not in original_scopes: - ret.add(ScopeInfo(scope, ScopeInfo.INTERMEDIATE)) + ret.add(ScopeInfo(scope)) return FrozenOrderedSet(ret) @classmethod @@ -454,7 +454,6 @@ def _make_parse_args_request( self, flags_in_scope, namespace: OptionValueContainer, - scope: str, include_passive_options: bool = False, ) -> Parser.ParseArgsRequest: levenshtein_max_distance = ( @@ -497,7 +496,7 @@ def for_scope( # Now add our values. flags_in_scope = self._scope_to_flags.get(scope, []) parse_args_request = self._make_parse_args_request( - flags_in_scope, values, scope, include_passive_options + flags_in_scope, values, include_passive_options ) self._parser_hierarchy.get_parser_by_scope(scope).parse_args(parse_args_request) diff --git a/src/python/pants/option/options_bootstrapper.py b/src/python/pants/option/options_bootstrapper.py index bafd62ecb44..49f9efe082c 100644 --- a/src/python/pants/option/options_bootstrapper.py +++ b/src/python/pants/option/options_bootstrapper.py @@ -21,25 +21,6 @@ from pants.util.strutil import ensure_text -# This is a temporary hack that allows us to note the fact that we're in v2-exclusive mode -# in a static location, as soon as we know it. This way code that cannot access options -# can still use this information to customize behavior. Again, this is a temporary hack -# to provide a better v2 experience to users who are not (and possibly never have been) -# running v1, and should go away ASAP. -class IsV2Exclusive: - def __init__(self): - self._value = False - - def set(self): - self._value = True - - def __bool__(self): - return self._value - - -is_v2_exclusive = IsV2Exclusive() - - @dataclass(frozen=True) class OptionsBootstrapper: """Holds the result of the first stage of options parsing, and assists with parsing full @@ -110,9 +91,6 @@ def register_global(*args, **kwargs): bootstrap_options.register(GLOBAL_SCOPE, *args, **kwargs) GlobalOptions.register_bootstrap_options(register_global) - opts = bootstrap_options.for_global_scope() - if opts.v2 and not opts.v1 and opts.backend_packages == []: - is_v2_exclusive.set() return bootstrap_options @classmethod diff --git a/src/python/pants/option/options_bootstrapper_test.py b/src/python/pants/option/options_bootstrapper_test.py index ea857b2dcc4..e32eb3b07e8 100644 --- a/src/python/pants/option/options_bootstrapper_test.py +++ b/src/python/pants/option/options_bootstrapper_test.py @@ -16,7 +16,8 @@ class OptionsBootstrapperTest(unittest.TestCase): - def _config_path(self, path: Optional[str]) -> List[str]: + @staticmethod + def _config_path(path: Optional[str]) -> List[str]: if path is None: return ["--pants-config-files=[]"] return [f"--pants-config-files=['{path}']"] @@ -136,11 +137,7 @@ def test_create_bootstrapped_options(self) -> None: args = ["--pants-workdir=/qux"] + self._config_path(fp.name) bootstrapper = OptionsBootstrapper.create(env={"PANTS_SUPPORTDIR": "/pear"}, args=args) opts = bootstrapper.get_full_options( - known_scope_infos=[ - ScopeInfo("", ScopeInfo.GLOBAL), - ScopeInfo("foo", ScopeInfo.TASK), - ScopeInfo("fruit", ScopeInfo.TASK), - ] + known_scope_infos=[ScopeInfo(""), ScopeInfo("foo"), ScopeInfo("fruit"),] ) # So we don't choke on these on the cmd line. opts.register("", "--pants-workdir") @@ -171,11 +168,7 @@ def assert_config_read_correctly( options_bootstrapper: OptionsBootstrapper, *, expected_worker_count: int, ) -> None: options = options_bootstrapper.get_full_options( - known_scope_infos=[ - ScopeInfo("", ScopeInfo.GLOBAL), - ScopeInfo("compile.apt", ScopeInfo.TASK), - ScopeInfo("fruit", ScopeInfo.TASK), - ], + known_scope_infos=[ScopeInfo(""), ScopeInfo("compile.apt"), ScopeInfo("fruit"),], ) # So we don't choke on these on the cmd line. options.register("", "--pants-config-files", type=list) @@ -237,10 +230,7 @@ def create_options_bootstrapper(*config_paths: str) -> OptionsBootstrapper: fp.close() bootstrapped_options = create_options_bootstrapper(fp.name) opts_single_config = bootstrapped_options.get_full_options( - known_scope_infos=[ - ScopeInfo("", ScopeInfo.GLOBAL), - ScopeInfo("resolver", ScopeInfo.TASK), - ] + known_scope_infos=[ScopeInfo(""), ScopeInfo("resolver"),] ) opts_single_config.register("", "--pantsrc-files", type=list) opts_single_config.register("resolver", "--resolver") @@ -252,36 +242,22 @@ def test_full_options_caching(self) -> None: bootstrapper = OptionsBootstrapper.create(env={}, args=args) opts1 = bootstrapper.get_full_options( - known_scope_infos=[ - ScopeInfo("", ScopeInfo.GLOBAL), - ScopeInfo("foo", ScopeInfo.TASK), - ] + known_scope_infos=[ScopeInfo(""), ScopeInfo("foo"),] ) opts2 = bootstrapper.get_full_options( - known_scope_infos=[ - ScopeInfo("foo", ScopeInfo.TASK), - ScopeInfo("", ScopeInfo.GLOBAL), - ] + known_scope_infos=[ScopeInfo("foo"), ScopeInfo(""),] ) assert opts1 is opts2 opts3 = bootstrapper.get_full_options( - known_scope_infos=[ - ScopeInfo("", ScopeInfo.GLOBAL), - ScopeInfo("foo", ScopeInfo.TASK), - ScopeInfo("", ScopeInfo.GLOBAL), - ] + known_scope_infos=[ScopeInfo(""), ScopeInfo("foo"), ScopeInfo(""),] ) assert opts1 is opts3 - opts4 = bootstrapper.get_full_options( - known_scope_infos=[ScopeInfo("", ScopeInfo.GLOBAL)] - ) + opts4 = bootstrapper.get_full_options(known_scope_infos=[ScopeInfo("")]) assert opts1 is not opts4 - opts5 = bootstrapper.get_full_options( - known_scope_infos=[ScopeInfo("", ScopeInfo.GLOBAL)] - ) + opts5 = bootstrapper.get_full_options(known_scope_infos=[ScopeInfo("")]) assert opts4 is opts5 assert opts1 is not opts5 diff --git a/src/python/pants/option/options_test.py b/src/python/pants/option/options_test.py index d9b23358124..83bd36ea604 100644 --- a/src/python/pants/option/options_test.py +++ b/src/python/pants/option/options_test.py @@ -54,19 +54,19 @@ def global_scope() -> ScopeInfo: - return ScopeInfo(GLOBAL_SCOPE, ScopeInfo.GLOBAL, GlobalOptions) + return ScopeInfo(GLOBAL_SCOPE, GlobalOptions) def task(scope: str) -> ScopeInfo: - return ScopeInfo(scope, ScopeInfo.TASK) + return ScopeInfo(scope) def intermediate(scope: str) -> ScopeInfo: - return ScopeInfo(scope, ScopeInfo.INTERMEDIATE) + return ScopeInfo(scope) def subsystem(scope: str) -> ScopeInfo: - return ScopeInfo(scope, ScopeInfo.SUBSYSTEM) + return ScopeInfo(scope) class OptionsTest(TestBase): @@ -218,7 +218,7 @@ def register_global(*args, **kwargs): register_global("--old-name", mutually_exclusive_group="new_name") # For the design doc example test. - options.register("compile", "--c", type=int, recursive=True) + options.register("compile", "--c", type=int) # Test deprecated options with a scope options.register("stale", "--still-good") @@ -1576,13 +1576,11 @@ def test_scope_deprecation(self) -> None: # old scope. I.e., it's possible to split an old scope's options among multiple new scopes. class DummyOptionable1(Optionable): options_scope = "new-scope1" - options_scope_category = ScopeInfo.SUBSYSTEM deprecated_options_scope = "deprecated-scope" deprecated_options_scope_removal_version = "9999.9.9.dev0" class DummyOptionable2(Optionable): options_scope = "new-scope2" - options_scope_category = ScopeInfo.SUBSYSTEM deprecated_options_scope = "deprecated-scope" deprecated_options_scope_removal_version = "9999.9.9.dev0" @@ -1645,7 +1643,6 @@ def test_scope_deprecation_parent(self) -> None: # another scope. class DummyOptionable1(Optionable): options_scope = "test" - options_scope_category = ScopeInfo.SUBSYSTEM @classmethod def register_options(cls, register): @@ -1654,7 +1651,6 @@ def register_options(cls, register): class DummyOptionable2(Optionable): options_scope = "lint" - options_scope_category = ScopeInfo.SUBSYSTEM deprecated_options_scope = "test.a-bit-linty" deprecated_options_scope_removal_version = "9999.9.9.dev0" @@ -1692,7 +1688,6 @@ def test_scope_deprecation_defaults(self) -> None: # Confirms that a DEFAULT option does not trigger deprecation warnings for a deprecated scope. class DummyOptionable1(Optionable): options_scope = "new-scope1" - options_scope_category = ScopeInfo.SUBSYSTEM deprecated_options_scope = "deprecated-scope" deprecated_options_scope_removal_version = "9999.9.9.dev0" @@ -1718,7 +1713,6 @@ def test_scope_dependency_deprecation(self) -> None: # Test that a dependency scope can be deprecated. class DummyOptionable1(Optionable): options_scope = "scope" - options_scope_category = ScopeInfo.SUBSYSTEM options = Options.create( env={}, @@ -1730,7 +1724,6 @@ class DummyOptionable1(Optionable): # imitates the construction of SubsystemClientMixin.known_scope_infos. ScopeInfo( DummyOptionable1.subscope("sub"), - ScopeInfo.SUBSYSTEM, DummyOptionable1, removal_version="9999.9.9.dev0", removal_hint="Sayonara!", diff --git a/src/python/pants/option/parser.py b/src/python/pants/option/parser.py index 5908e3e254a..a7a81696aad 100644 --- a/src/python/pants/option/parser.py +++ b/src/python/pants/option/parser.py @@ -492,7 +492,7 @@ def _unnormalized_option_registrations_iter(self): yield args, kwargs # Then yield our directly-registered options. for args, kwargs in self._option_registrations: - if "recursive" in kwargs and self._scope_info.category == ScopeInfo.SUBSYSTEM: + if "recursive" in kwargs and self._scope_info.scope != GLOBAL_SCOPE: raise RecursiveSubsystemOption(self.scope, args[0]) yield args, kwargs @@ -508,7 +508,7 @@ def _recursive_option_registration_args(self): # Note that all subsystem options are implicitly recursive: a subscope of a subsystem # scope is another (optionable-specific) instance of the same subsystem, so it needs # all the same options. - if self._scope_info.category == ScopeInfo.SUBSYSTEM or "recursive" in kwargs: + if self._scope_info.scope != GLOBAL_SCOPE or "recursive" in kwargs: yield args, kwargs def register(self, *args, **kwargs) -> None: diff --git a/src/python/pants/option/scope.py b/src/python/pants/option/scope.py index 7dee14f7e93..6c5933ea4c1 100644 --- a/src/python/pants/option/scope.py +++ b/src/python/pants/option/scope.py @@ -2,7 +2,7 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from dataclasses import dataclass -from typing import TYPE_CHECKING, ClassVar, Optional, Type, cast +from typing import TYPE_CHECKING, Optional, Type, cast from pants.option.option_value_container import OptionValueContainer @@ -25,7 +25,6 @@ class ScopeInfo: """Information about a scope.""" scope: str - category: str optionable_cls: Optional[Type["Optionable"]] = None # A ScopeInfo may have a deprecated_scope (from its associated optionable_cls), which represents a # previous/deprecated name for a current/non-deprecated ScopeInfo. It may also be directly @@ -34,16 +33,6 @@ class ScopeInfo: removal_version: Optional[str] = None removal_hint: Optional[str] = None - # Symbolic constants for different categories of scope. - GLOBAL: ClassVar[str] = "GLOBAL" - GOAL: ClassVar[str] = "GOAL" - GOAL_V1: ClassVar[str] = "GOAL_V1" - TASK: ClassVar[str] = "TASK" - SUBSYSTEM: ClassVar[str] = "SUBSYSTEM" - INTERMEDIATE: ClassVar[ - str - ] = "INTERMEDIATE" # Scope added automatically to fill out the scope hierarchy. - @property def description(self) -> str: return cast(str, self._optionable_cls_attr("get_description", lambda: "")()) diff --git a/src/python/pants/subsystem/subsystem.py b/src/python/pants/subsystem/subsystem.py index edabfab398d..82e3abd3b35 100644 --- a/src/python/pants/subsystem/subsystem.py +++ b/src/python/pants/subsystem/subsystem.py @@ -45,8 +45,6 @@ class Subsystem(SubsystemClientMixin, Optionable): :API: public """ - options_scope_category = ScopeInfo.SUBSYSTEM - class UninitializedSubsystemError(SubsystemError): def __init__(self, class_name, scope): super().__init__( @@ -75,7 +73,7 @@ def get_scope_info(cls, subscope=None): if subscope is None: return super().get_scope_info() else: - return ScopeInfo(cls.subscope(subscope), ScopeInfo.SUBSYSTEM, cls) + return ScopeInfo(cls.subscope(subscope), cls) # The full Options object for this pants run. Will be set after options are parsed. # TODO: A less clunky way to make option values available? diff --git a/src/python/pants/subsystem/subsystem_client_mixin.py b/src/python/pants/subsystem/subsystem_client_mixin.py index 56cddee81ef..c3aac48ff8a 100644 --- a/src/python/pants/subsystem/subsystem_client_mixin.py +++ b/src/python/pants/subsystem/subsystem_client_mixin.py @@ -142,11 +142,7 @@ def collect_scope_infos(optionable_cls, scoped_to, removal_version=None, removal else optionable_cls.subscope(scoped_to) ) scope_info = ScopeInfo( - scope, - optionable_cls.options_scope_category, - optionable_cls, - removal_version=removal_version, - removal_hint=removal_hint, + scope, optionable_cls, removal_version=removal_version, removal_hint=removal_hint, ) if scope_info not in known_scope_infos: diff --git a/src/python/pants/subsystem/subsystem_test.py b/src/python/pants/subsystem/subsystem_test.py index dad7da23096..093498bd9de 100644 --- a/src/python/pants/subsystem/subsystem_test.py +++ b/src/python/pants/subsystem/subsystem_test.py @@ -44,7 +44,7 @@ def subsystem_dependencies(cls): def si(scope, subsystem_cls): """Shorthand helper.""" - return ScopeInfo(scope, ScopeInfo.SUBSYSTEM, subsystem_cls) + return ScopeInfo(scope, subsystem_cls) class SubsystemTest(TestBase): diff --git a/src/python/pants/task/task.py b/src/python/pants/task/task.py index fb718777868..c8ab7d1a73e 100644 --- a/src/python/pants/task/task.py +++ b/src/python/pants/task/task.py @@ -22,7 +22,6 @@ from pants.invalidation.cache_manager import InvalidationCacheManager, InvalidationCheck from pants.option.optionable import Optionable from pants.option.options_fingerprinter import OptionsFingerprinter -from pants.option.scope import ScopeInfo from pants.reporting.reporting_utils import items_to_report_element from pants.source.source_root import SourceRootConfig from pants.subsystem.subsystem_client_mixin import SubsystemClientMixin @@ -57,8 +56,6 @@ class TaskBase(SubsystemClientMixin, Optionable, metaclass=ABCMeta): example. """ - options_scope_category = ScopeInfo.TASK - # We set this explicitly on the synthetic subclass, so that it shares a stable name with # its superclass, which is not necessary for regular use, but can be convenient in tests. _stable_name: Optional[str] = None