Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the concept of a scope category. #10224

Merged
merged 20 commits into from Jul 3, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 2 additions & 19 deletions src/python/pants/engine/goal.py
Expand Up @@ -8,8 +8,6 @@

from pants.cache.cache_setup import CacheSetup
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

Expand All @@ -32,8 +30,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, ...] = ()
Expand All @@ -54,22 +50,9 @@ def deprecated_cache_setup_removal_version(cls):
"""
return None

@classmethod
def conflict_free_name(cls):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, v2 setup-py is still setup-py2:

name = "setup-py2"
@classmethod
def register_options(cls, register):
super().register_options(register)
register(
"--args",
type=list,
member_type=shell_str,
passthrough=True,
help="Arguments to pass directly to setup.py, e.g. "
'`--setup-py2-args="bdist_wheel --python-tag py36.py37"`. If unspecified, Pants will '
"dump the setup.py chroot.",
)

# 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

@classmethod
def subsystem_dependencies(cls):
Expand Down Expand Up @@ -126,7 +109,7 @@ class List(Goal):

@classproperty
def name(cls):
return cls.subsystem_cls.conflict_free_name()
return cls.subsystem_cls.name


class Outputting:
Expand Down
29 changes: 8 additions & 21 deletions src/python/pants/help/help_printer.py
Expand Up @@ -9,9 +9,8 @@
from typing_extensions import Literal

from pants.base.build_environment import pants_release, pants_version
from pants.engine.goal import GoalSubsystem
from pants.engine.goal import Goal, 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 (
Expand Down Expand Up @@ -75,7 +74,7 @@ def _print_goals_help(self) -> None:
goal_scope_infos = [
scope_info
for scope_info in self._options.known_scope_to_info.values()
if scope_info.category == ScopeInfo.GOAL
if issubclass(scope_info.optionable_cls, Goal) # type: ignore[arg-type]
]
for scope_info in goal_scope_infos:
optionable_cls = scope_info.optionable_cls
Expand All @@ -88,10 +87,6 @@ def _print_goals_help(self) -> None:
continue
description = scope_info.description or "<no description>"
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)}"
Expand All @@ -101,18 +96,18 @@ def _print_goals_help(self) -> None:
max_width = max(len(name) for name in goal_descriptions.keys())
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",
Expand All @@ -134,20 +129,14 @@ 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())
else:
# 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_info_iterator = ScopeInfoIterator(scope_to_info=self._options.known_scope_to_info)

scope_infos = list(scope_info_iterator.iterate(help_scopes))

Expand Down Expand Up @@ -188,9 +177,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.
Expand Down
30 changes: 2 additions & 28 deletions src/python/pants/help/scope_info_iterator.py
Expand Up @@ -15,7 +15,6 @@ 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.
Expand All @@ -34,33 +33,11 @@ def iterate(self, scopes: Set[str]) -> Iterator[ScopeInfo]:
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))
scope_infos: List[ScopeInfo] = sorted(self.scope_to_info[s] for s in 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."""

Expand All @@ -80,10 +57,7 @@ def subsys_deps(subsystem_client_cls: Type[Any]) -> Iterator[ScopeInfo]:
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
):
if info.scope != GLOBAL_SCOPE and enclosing_scope(scope) == GLOBAL_SCOPE:
yield info
if info.optionable_cls is not None:
for subsys_dep in subsys_deps(info.optionable_cls):
Expand Down
26 changes: 13 additions & 13 deletions src/python/pants/help/scope_info_iterator_test.py
Expand Up @@ -33,19 +33,19 @@ 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),
ScopeInfo(GLOBAL_SCOPE, GlobalOptions),
ScopeInfo("subsys2", Subsys2),
ScopeInfo("subsys1.subsys2", Subsys1),
ScopeInfo("goal1"),
ScopeInfo("goal1.task11"),
ScopeInfo("goal1.task12", Goal1Task2),
ScopeInfo("subsys1.goal1.task12", Subsys1),
ScopeInfo("goal2"),
ScopeInfo("goal2.task21"),
ScopeInfo("goal2.task22"),
ScopeInfo("goal3"),
ScopeInfo("goal3.task31"),
ScopeInfo("goal3.task32"),
]

scope_to_infos = dict((x.scope, x) for x in infos)
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/option/arg_splitter.py
Expand Up @@ -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
Expand Down
28 changes: 8 additions & 20 deletions src/python/pants/option/arg_splitter_test.py
Expand Up @@ -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(
Expand Down
3 changes: 1 addition & 2 deletions src/python/pants/option/global_options.py
Expand Up @@ -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

Expand Down Expand Up @@ -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):
Expand Down
7 changes: 3 additions & 4 deletions src/python/pants/option/optionable.py
Expand Up @@ -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.
Expand Down Expand Up @@ -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):
Expand Down
7 changes: 3 additions & 4 deletions src/python/pants/option/options.py
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 = (
Expand Down Expand Up @@ -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)

Expand Down
22 changes: 0 additions & 22 deletions src/python/pants/option/options_bootstrapper.py
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down