From 56a9d94100ab37c045bd6b88b60f337164e250af Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Thu, 22 Oct 2020 13:53:01 -0400 Subject: [PATCH 1/3] Fold target help into the help system. Instead of it being the only help-related functionality implemented as a standalone goal. Also replaces `./pants goals` with `./pants help goals`, and makes various other improvements to help, such as not showing global option help when you just enter `./pants help`, so that the usage message fits in a single screen. Addresses https://github.com/pantsbuild/pants/issues/10999. [ci skip-rust] [ci skip-build-wheels] --- src/python/pants/backend/pants_info/BUILD | 6 - .../pants/backend/pants_info/__init__.py | 0 .../backend/pants_info/list_target_types.py | 287 ------------------ .../pants_info/list_target_types_test.py | 224 -------------- .../pants/backend/pants_info/register.py | 10 - src/python/pants/bin/local_pants_runner.py | 2 + src/python/pants/core/target_types.py | 29 +- .../pants/help/flag_error_help_printer.py | 4 +- src/python/pants/help/help_info_extracter.py | 135 +++++++- .../pants/help/help_info_extracter_test.py | 59 +++- .../pants/help/help_integration_test.py | 96 +++++- src/python/pants/help/help_printer.py | 187 +++++++----- .../pants/help/list_goals_integration_test.py | 42 --- src/python/pants/init/BUILD | 1 - src/python/pants/init/extension_loader.py | 4 +- .../init/load_backends_integration_test.py | 2 +- src/python/pants/option/arg_splitter.py | 27 +- src/python/pants/option/arg_splitter_test.py | 8 +- src/python/pants/option/global_options.py | 7 +- 19 files changed, 429 insertions(+), 701 deletions(-) delete mode 100644 src/python/pants/backend/pants_info/BUILD delete mode 100644 src/python/pants/backend/pants_info/__init__.py delete mode 100644 src/python/pants/backend/pants_info/list_target_types.py delete mode 100644 src/python/pants/backend/pants_info/list_target_types_test.py delete mode 100644 src/python/pants/backend/pants_info/register.py delete mode 100644 src/python/pants/help/list_goals_integration_test.py diff --git a/src/python/pants/backend/pants_info/BUILD b/src/python/pants/backend/pants_info/BUILD deleted file mode 100644 index 5d5f3c83125..00000000000 --- a/src/python/pants/backend/pants_info/BUILD +++ /dev/null @@ -1,6 +0,0 @@ -# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -python_library() - -python_tests(name = "tests") diff --git a/src/python/pants/backend/pants_info/__init__.py b/src/python/pants/backend/pants_info/__init__.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/python/pants/backend/pants_info/list_target_types.py b/src/python/pants/backend/pants_info/list_target_types.py deleted file mode 100644 index 143bcf98833..00000000000 --- a/src/python/pants/backend/pants_info/list_target_types.py +++ /dev/null @@ -1,287 +0,0 @@ -# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -import dataclasses -import json -import textwrap -from dataclasses import dataclass -from typing import Any, Dict, Generic, Optional, Sequence, Type, cast, get_type_hints - -from pants.core.util_rules.pants_bin import PantsBin -from pants.engine.console import Console -from pants.engine.goal import Goal, GoalSubsystem, LineOriented -from pants.engine.rules import collect_rules, goal_rule -from pants.engine.target import ( - AsyncField, - BoolField, - DictStringToStringField, - DictStringToStringSequenceField, - Field, - FloatField, - IntField, - PrimitiveField, - RegisteredTargetTypes, - ScalarField, - SequenceField, - StringField, - StringOrStringSequenceField, - StringSequenceField, - Target, -) -from pants.engine.unions import UnionMembership -from pants.util.objects import get_docstring, get_docstring_summary, pretty_print_type_hint - - -class TargetTypesSubsystem(LineOriented, GoalSubsystem): - """List all registered target types.""" - - name = "target-types" - - @classmethod - def register_options(cls, register): - super().register_options(register) - register( - "--details", - type=str, - metavar="", - help="List all of the target type's registered fields.", - ) - register( - "--all", - type=bool, - default=False, - help="List all target types with their full descriptions and fields as JSON.", - ) - - @property - def details(self) -> Optional[str]: - return cast(Optional[str], self.options.details) - - @property - def all(self) -> bool: - return cast(bool, self.options.all) - - -class TargetTypes(Goal): - subsystem_cls = TargetTypesSubsystem - - -@dataclass(frozen=True) -class AbbreviatedTargetInfo: - alias: str - description: Optional[str] - - @classmethod - def create(cls, target_type: Type[Target]) -> "AbbreviatedTargetInfo": - return cls(alias=target_type.alias, description=get_docstring_summary(target_type)) - - def format_for_cli(self, console: Console, *, longest_target_alias: int) -> str: - chars_before_description = longest_target_alias + 2 - alias = console.cyan(f"{self.alias}".ljust(chars_before_description)) - if not self.description: - description = "" - else: - description_lines = textwrap.wrap(self.description, 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"{alias}{description}\n" - - -@dataclass(frozen=True) -class FieldInfo: - alias: str - description: Optional[str] - type_hint: str - required: bool - default: Optional[str] - - @classmethod - def create(cls, field: Type[Field]) -> "FieldInfo": - # NB: It is very common (and encouraged) to subclass Fields to give custom behavior, e.g. - # `PythonSources` subclassing `Sources`. Here, we set `fallback_to_ancestors=True` so that - # we can still generate meaningful documentation for all these custom fields without - # requiring the Field author to rewrite the docstring. - # - # However, if the original `Field` author did not define docstring, then this means we - # would typically fall back to the docstring for `AsyncField`, `PrimitiveField`, or a - # helper class like `StringField`. This is a quirk of this heuristic and it's not - # intentional since these core `Field` types have documentation oriented to the custom - # `Field` author and not the end user filling in fields in a BUILD file target. - description = get_docstring( - field, - flatten=True, - fallback_to_ancestors=True, - ignored_ancestors={ - *Field.mro(), - AsyncField, - PrimitiveField, - BoolField, - DictStringToStringField, - DictStringToStringSequenceField, - FloatField, - Generic, # type: ignore[arg-type] - IntField, - ScalarField, - SequenceField, - StringField, - StringOrStringSequenceField, - StringSequenceField, - }, - ) - if issubclass(field, PrimitiveField): - raw_value_type = get_type_hints(field.compute_value)["raw_value"] - elif issubclass(field, AsyncField): - raw_value_type = get_type_hints(field.sanitize_raw_value)["raw_value"] - else: - raw_value_type = get_type_hints(field.__init__)["raw_value"] - type_hint = pretty_print_type_hint(raw_value_type) - - # Check if the field only allows for certain choices. - if issubclass(field, StringField) and field.valid_choices is not None: - valid_choices = sorted( - field.valid_choices - if isinstance(field.valid_choices, tuple) - else (choice.value for choice in field.valid_choices) - ) - type_hint = " | ".join([*(repr(c) for c in valid_choices), "None"]) - - if field.required: - # We hackily remove `None` as a valid option for the field when it's required. This - # greatly simplifies Field definitions because it means that they don't need to - # override the type hints for `PrimitiveField.compute_value()` and - # `AsyncField.sanitize_raw_value()` to indicate that `None` is an invalid type. - type_hint = type_hint.replace(" | None", "") - - return cls( - alias=field.alias, - description=description, - type_hint=type_hint, - required=field.required, - default=( - repr(field.default) if (not field.required and field.default is not None) else None - ), - ) - - def format_for_cli(self, console: Console) -> str: - field_alias = console.magenta(f"{self.alias}") - indent = " " - required_or_default = "required" if self.required else f"default: {self.default}" - type_info = console.cyan(f"{indent}type: {self.type_hint}, {required_or_default}") - lines = [field_alias, type_info] - if self.description: - lines.extend(f"{indent}{line}" for line in textwrap.wrap(self.description or "", 80)) - return "\n".join(f"{indent}{line}" for line in lines) - - def as_dict(self) -> Dict[str, Any]: - d = dataclasses.asdict(self) - del d["alias"] - return d - - -@dataclass(frozen=True) -class VerboseTargetInfo: - alias: str - description: Optional[str] - fields: Sequence[FieldInfo] - - @classmethod - def create( - cls, target_type: Type[Target], *, union_membership: UnionMembership - ) -> "VerboseTargetInfo": - return cls( - alias=target_type.alias, - description=get_docstring(target_type), - fields=[ - FieldInfo.create(field) - for field in target_type.class_field_types(union_membership=union_membership) - if not field.alias.startswith("_") and field.deprecated_removal_version is None - ], - ) - - def format_for_cli(self, console: Console) -> str: - output = [console.green(f"{self.alias}\n{'-' * len(self.alias)}\n")] - if self.description: - output.append(f"{self.description}\n") - output.extend( - [ - "Valid fields:\n", - *sorted(f"{field.format_for_cli(console)}\n" for field in self.fields), - ] - ) - return "\n".join(output).rstrip() - - def as_dict(self) -> Dict[str, Any]: - return { - "description": self.description, - "fields": {f.alias: f.as_dict() for f in self.fields}, - } - - -@goal_rule -def list_target_types( - registered_target_types: RegisteredTargetTypes, - union_membership: UnionMembership, - target_types_subsystem: TargetTypesSubsystem, - console: Console, - pants_bin: PantsBin, -) -> TargetTypes: - with target_types_subsystem.line_oriented(console) as print_stdout: - if target_types_subsystem.all: - all_target_types = { - alias: VerboseTargetInfo.create( - target_type, union_membership=union_membership - ).as_dict() - for alias, target_type in registered_target_types.aliases_to_types.items() - if not alias.startswith("_") and target_type.deprecated_removal_version is None - } - print_stdout(json.dumps(all_target_types, sort_keys=True, indent=4)) - elif target_types_subsystem.details: - alias = target_types_subsystem.details - target_type = registered_target_types.aliases_to_types.get(alias) - if target_type is None: - raise ValueError( - f"Unrecognized target type {repr(alias)}. All registered " - f"target types: {list(registered_target_types.aliases)}" - ) - verbose_target_info = VerboseTargetInfo.create( - target_type, union_membership=union_membership - ) - print_stdout("") - print_stdout(verbose_target_info.format_for_cli(console)) - else: - title_text = "Target types" - title = console.green(f"{title_text}\n{'-' * len(title_text)}") - target_infos = [ - AbbreviatedTargetInfo.create(target_type) - for target_type in registered_target_types.types - if ( - not target_type.alias.startswith("_") - and target_type.deprecated_removal_version is None - ) - ] - longest_target_alias = max( - len(target_type.alias) for target_type in registered_target_types.types - ) - lines = [ - f"\n{title}\n", - textwrap.fill( - f"Use `{pants_bin.render_command('target-types', '--details=$target_type')}` " - "to get detailed information for a particular target type.", - 80, - ), - "\n", - *( - target_info.format_for_cli(console, longest_target_alias=longest_target_alias) - for target_info in target_infos - ), - ] - print_stdout("\n".join(lines).rstrip()) - return TargetTypes(exit_code=0) - - -def rules(): - return collect_rules() diff --git a/src/python/pants/backend/pants_info/list_target_types_test.py b/src/python/pants/backend/pants_info/list_target_types_test.py deleted file mode 100644 index 1c81f54d312..00000000000 --- a/src/python/pants/backend/pants_info/list_target_types_test.py +++ /dev/null @@ -1,224 +0,0 @@ -# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -import json -from enum import Enum -from textwrap import dedent -from typing import Optional, cast - -from pants.backend.pants_info.list_target_types import TargetTypesSubsystem, list_target_types -from pants.core.util_rules.pants_bin import PantsBin -from pants.engine.target import BoolField, IntField, RegisteredTargetTypes, StringField, Target -from pants.engine.unions import UnionMembership -from pants.testutil.option_util import create_goal_subsystem -from pants.testutil.rule_runner import MockConsole, run_rule_with_mocks - - -# Note no docstring. -class FortranVersion(StringField): - alias = "fortran_version" - - -class GenericTimeout(IntField): - """The number of seconds to run before timing out.""" - - alias = "timeout" - - -# Note no docstring, but GenericTimeout has it, so we should end up using that. -class FortranTimeout(GenericTimeout): - pass - - -class FortranLibrary(Target): - """A library of Fortran code.""" - - alias = "fortran_library" - core_fields = (FortranVersion,) - - -# Note multiline docstring. -class FortranTests(Target): - """Tests for Fortran code. - - This assumes that you use the FRUIT test framework. - """ - - alias = "fortran_tests" - core_fields = (FortranVersion, FortranTimeout) - - -class ArchiveFormat(Enum): - TGZ = ".tgz" - TAR = ".tar" - - -class ArchiveFormatField(StringField): - alias = "archive_format" - valid_choices = ArchiveFormat - default = ArchiveFormat.TGZ.value - - -class ErrorBehavior(StringField): - alias = "error_behavior" - valid_choices = ("ignore", "warn", "error") - required = True - - -# Note no docstring. -class FortranBinary(Target): - alias = "fortran_binary" - core_fields = (FortranVersion, ArchiveFormatField, ErrorBehavior) - - -def run_goal( - *, - union_membership: Optional[UnionMembership] = None, - details_target: Optional[str] = None, - all: bool = False -) -> str: - console = MockConsole(use_colors=False) - run_rule_with_mocks( - list_target_types, - rule_args=[ - RegisteredTargetTypes.create([FortranBinary, FortranLibrary, FortranTests]), - union_membership or UnionMembership({}), - create_goal_subsystem( - TargetTypesSubsystem, sep="\\n", output_file=None, details=details_target, all=all - ), - console, - PantsBin(name="./BNF"), - ], - ) - return cast(str, console.stdout.getvalue()) - - -def test_list_all_abbreviated() -> None: - stdout = run_goal() - assert stdout == dedent( - """\ - - Target types - ------------ - - Use `./BNF target-types --details=$target_type` to get detailed information for - a particular target type. - - - fortran_binary - - fortran_library A library of Fortran code. - - fortran_tests Tests for Fortran code. - """ - ) - - -def test_list_all_json() -> None: - stdout = run_goal(all=True) - fortran_version = { - "default": None, - "description": None, - "required": False, - "type_hint": "str | None", - } - assert json.loads(stdout) == { - "fortran_binary": { - "description": None, - "fields": { - "archive_format": { - "default": "'.tgz'", - "description": None, - "required": False, - "type_hint": "'.tar' | '.tgz' | None", - }, - "error_behavior": { - "default": None, - "description": None, - "required": True, - "type_hint": "'error' | 'ignore' | 'warn'", - }, - "fortran_version": fortran_version, - }, - }, - "fortran_library": { - "description": "A library of Fortran code.", - "fields": {"fortran_version": fortran_version}, - }, - "fortran_tests": { - "description": ( - "Tests for Fortran code.\n\nThis assumes that you use the FRUIT test framework." - ), - "fields": { - "fortran_version": fortran_version, - "timeout": { - "default": None, - "description": "The number of seconds to run before timing out.", - "required": False, - "type_hint": "int | None", - }, - }, - }, - } - - -def test_list_single() -> None: - class CustomField(BoolField): - """My custom field! - - Use this field to... - """ - - alias = "custom_field" - required = True - - tests_target_stdout = run_goal( - union_membership=UnionMembership.from_rules( - [FortranTests.register_plugin_field(CustomField)] - ), - details_target=FortranTests.alias, - ) - assert tests_target_stdout == dedent( - """\ - - fortran_tests - ------------- - - Tests for Fortran code. - - This assumes that you use the FRUIT test framework. - - Valid fields: - - custom_field - type: bool, required - My custom field! Use this field to... - - fortran_version - type: str | None, default: None - - timeout - type: int | None, default: None - The number of seconds to run before timing out. - """ - ) - - binary_target_stdout = run_goal(details_target=FortranBinary.alias) - assert binary_target_stdout == dedent( - """\ - - fortran_binary - -------------- - - Valid fields: - - archive_format - type: '.tar' | '.tgz' | None, default: '.tgz' - - error_behavior - type: 'error' | 'ignore' | 'warn', required - - fortran_version - type: str | None, default: None - """ - ) diff --git a/src/python/pants/backend/pants_info/register.py b/src/python/pants/backend/pants_info/register.py deleted file mode 100644 index 64cc671ce12..00000000000 --- a/src/python/pants/backend/pants_info/register.py +++ /dev/null @@ -1,10 +0,0 @@ -# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -"""Information on Pants' internals, such as registered target types and goals.""" - -from pants.backend.pants_info import list_target_types - - -def rules(): - return list_target_types.rules() diff --git a/src/python/pants/bin/local_pants_runner.py b/src/python/pants/bin/local_pants_runner.py index 3b62a28c993..7feb72e11e8 100644 --- a/src/python/pants/bin/local_pants_runner.py +++ b/src/python/pants/bin/local_pants_runner.py @@ -17,6 +17,7 @@ from pants.engine.internals.native import Native from pants.engine.internals.scheduler import ExecutionError from pants.engine.internals.session import SessionValues +from pants.engine.target import RegisteredTargetTypes from pants.engine.unions import UnionMembership from pants.goal.run_tracker import RunTracker from pants.help.flag_error_help_printer import FlagErrorHelpPrinter @@ -226,6 +227,7 @@ def _print_help(self, request: HelpRequest) -> ExitCode: self.options, self.union_membership, self.graph_session.goal_consumed_subsystem_scopes, + RegisteredTargetTypes.create(self.build_config.target_types), ) help_printer = HelpPrinter( bin_name=global_options.pants_bin_name, diff --git a/src/python/pants/core/target_types.py b/src/python/pants/core/target_types.py index a1e13353eec..27379d99252 100644 --- a/src/python/pants/core/target_types.py +++ b/src/python/pants/core/target_types.py @@ -43,12 +43,11 @@ class FilesSources(Sources): class Files(Target): - """A collection of loose files which do not have their source roots stripped. + """Loose files that live outside code packages. - The sources of a `files` target can be accessed via language-specific APIs, such as Python's - `open()`. Unlike the similar `resources()` target type, Pants will not strip the source root of - `files()`, meaning that `src/python/project/f1.txt` will not be stripped down to - `project/f1.txt`. + Files are placed directly in archives, outside of code artifacts such as Python wheels or JVM + JARs. The sources of a `files` target are accessed via filesystem APIs, such as Python's + `open()`, via paths relative to the repo root. """ alias = "files" @@ -101,8 +100,10 @@ class RelocatedFilesDestField(StringField): class RelocatedFiles(Target): - """Relocate the paths for `files()` targets at runtime to something more convenient than the - default of their actual paths in your project. + """Loose files with path manipulation applied. + + Allows you to relocate the files at runtime to something more convenient than + their actual paths in your project. For example, you can relocate `src/resources/project1/data.json` to instead be `resources/data.json`. Your other target types can then add this target to their @@ -195,12 +196,11 @@ class ResourcesSources(Sources): class Resources(Target): - """A collection of loose files. + """Data emebdded in a code package and accessed in a location-independent manner. - The sources of a `resources` target can be accessed via language-specific APIs, such as Python's - `open()`. Resources are meant to be included in deployable units like JARs or Python wheels. - Unlike the similar `files()` target type, Pants will strip the source root of `resources()`, - meaning that `src/python/project/f1.txt` will be stripped down to `project/f1.txt`. + Resources are embedded in code artifacts such as Python wheels or JVM JARs. The sources of a + `resources` target are accessed via language-specific resource APIs, such as Python's pkgutil or + JVM's ClassLoader, via paths relative to the target's source root. """ alias = "resources" @@ -213,7 +213,7 @@ class Resources(Target): class GenericTarget(Target): - """A generic target with no specific target type. + """A generic target with no specific type. This can be used as a generic "bag of dependencies", i.e. you can group several different targets into one single target so that your other targets only need to depend on one thing. @@ -267,8 +267,7 @@ class ArchiveFormatField(StringField): class ArchiveTarget(Target): - """An archive (e.g. zip file) containing loose files and/or packages built via `./pants - package`.""" + """A ZIP or TAR file containing loose files and code packages.""" alias = "archive" core_fields = ( diff --git a/src/python/pants/help/flag_error_help_printer.py b/src/python/pants/help/flag_error_help_printer.py index eca717dde4f..3f4a1a0525f 100644 --- a/src/python/pants/help/flag_error_help_printer.py +++ b/src/python/pants/help/flag_error_help_printer.py @@ -3,6 +3,7 @@ import difflib +from pants.engine.target import RegisteredTargetTypes from pants.engine.unions import UnionMembership from pants.help.help_info_extracter import HelpInfoExtracter from pants.help.maybe_color import MaybeColor @@ -20,9 +21,10 @@ def __init__(self, options: Options): self._all_help_info = HelpInfoExtracter.get_all_help_info( options, # We only care about the options-related help info, so we pass in - # dummy values for union_membership and consumed_scopes_mapper. + # dummy values for the other arguments. UnionMembership({}), lambda x: tuple(), + RegisteredTargetTypes({}), ) def handle_unknown_flags(self, err: UnknownFlagsError): diff --git a/src/python/pants/help/help_info_extracter.py b/src/python/pants/help/help_info_extracter.py index 595a2f8601e..ffb6324a675 100644 --- a/src/python/pants/help/help_info_extracter.py +++ b/src/python/pants/help/help_info_extracter.py @@ -6,14 +6,32 @@ import json from dataclasses import dataclass from enum import Enum -from typing import Any, Callable, Dict, List, Optional, Tuple, Type, cast +from typing import Any, Callable, Dict, Generic, List, Optional, Tuple, Type, cast, get_type_hints from pants.base import deprecated from pants.engine.goal import GoalSubsystem +from pants.engine.target import ( + AsyncField, + BoolField, + DictStringToStringField, + DictStringToStringSequenceField, + Field, + FloatField, + IntField, + PrimitiveField, + RegisteredTargetTypes, + ScalarField, + SequenceField, + StringField, + StringOrStringSequenceField, + StringSequenceField, + Target, +) from pants.engine.unions import UnionMembership from pants.option.option_util import is_dict_option, is_list_option from pants.option.options import Options from pants.option.parser import OptionValueHistory, Parser +from pants.util.objects import get_docstring, get_docstring_summary, pretty_print_type_hint class HelpJSONEncoder(json.JSONEncoder): @@ -124,12 +142,116 @@ class GoalHelpInfo: consumed_scopes: Tuple[str, ...] # The scopes of subsystems consumed by this goal. +@dataclass(frozen=True) +class TargetFieldHelpInfo: + """A container for help information for a field in a target type.""" + + alias: str + description: Optional[str] + type_hint: str + required: bool + default: Optional[str] + + @classmethod + def create(cls, field: Type[Field]) -> "TargetFieldHelpInfo": + # NB: It is very common (and encouraged) to subclass Fields to give custom behavior, e.g. + # `PythonSources` subclassing `Sources`. Here, we set `fallback_to_ancestors=True` so that + # we can still generate meaningful documentation for all these custom fields without + # requiring the Field author to rewrite the docstring. + # + # However, if the original `Field` author did not define docstring, then this means we + # would typically fall back to the docstring for `AsyncField`, `PrimitiveField`, or a + # helper class like `StringField`. This is a quirk of this heuristic and it's not + # intentional since these core `Field` types have documentation oriented to the custom + # `Field` author and not the end user filling in fields in a BUILD file target. + description = get_docstring( + field, + flatten=True, + fallback_to_ancestors=True, + ignored_ancestors={ + *Field.mro(), + AsyncField, + PrimitiveField, + BoolField, + DictStringToStringField, + DictStringToStringSequenceField, + FloatField, + Generic, # type: ignore[arg-type] + IntField, + ScalarField, + SequenceField, + StringField, + StringOrStringSequenceField, + StringSequenceField, + }, + ) + if issubclass(field, PrimitiveField): + raw_value_type = get_type_hints(field.compute_value)["raw_value"] + elif issubclass(field, AsyncField): + raw_value_type = get_type_hints(field.sanitize_raw_value)["raw_value"] + else: + raw_value_type = get_type_hints(field.__init__)["raw_value"] + type_hint = pretty_print_type_hint(raw_value_type) + + # Check if the field only allows for certain choices. + if issubclass(field, StringField) and field.valid_choices is not None: + valid_choices = sorted( + field.valid_choices + if isinstance(field.valid_choices, tuple) + else (choice.value for choice in field.valid_choices) + ) + type_hint = " | ".join([*(repr(c) for c in valid_choices), "None"]) + + if field.required: + # We hackily remove `None` as a valid option for the field when it's required. This + # greatly simplifies Field definitions because it means that they don't need to + # override the type hints for `PrimitiveField.compute_value()` and + # `AsyncField.sanitize_raw_value()` to indicate that `None` is an invalid type. + type_hint = type_hint.replace(" | None", "") + + return cls( + alias=field.alias, + description=description, + type_hint=type_hint, + required=field.required, + default=( + repr(field.default) if (not field.required and field.default is not None) else None + ), + ) + + +@dataclass(frozen=True) +class TargetTypeHelpInfo: + """A container for help information for a target type.""" + + alias: str + summary: Optional[str] + description: Optional[str] + fields: Tuple[TargetFieldHelpInfo, ...] + + @classmethod + def create( + cls, target_type: Type[Target], *, union_membership: UnionMembership + ) -> "TargetTypeHelpInfo": + return cls( + alias=target_type.alias, + summary=get_docstring_summary(target_type), + description=get_docstring(target_type), + fields=tuple( + TargetFieldHelpInfo.create(field) + for field in target_type.class_field_types(union_membership=union_membership) + if not field.alias.startswith("_") and field.deprecated_removal_version is None + ), + ) + + @dataclass(frozen=True) class AllHelpInfo: """All available help info.""" scope_to_help_info: Dict[str, OptionScopeHelpInfo] name_to_goal_info: Dict[str, GoalHelpInfo] + name_to_target_type_info: Dict[str, TargetTypeHelpInfo] ConsumedScopesMapper = Callable[[str], Tuple[str, ...]] @@ -144,6 +266,7 @@ def get_all_help_info( options: Options, union_membership: UnionMembership, consumed_scopes_mapper: ConsumedScopesMapper, + registered_target_types: RegisteredTargetTypes, ) -> AllHelpInfo: scope_to_help_info = {} name_to_goal_info = {} @@ -180,8 +303,16 @@ def get_all_help_info( consumed_scopes_mapper(scope_info.scope), ) + name_to_target_type_info = { + alias: TargetTypeHelpInfo.create(target_type, union_membership=union_membership) + for alias, target_type in registered_target_types.aliases_to_types.items() + if not alias.startswith("_") and target_type.deprecated_removal_version is None + } + return AllHelpInfo( - scope_to_help_info=scope_to_help_info, name_to_goal_info=name_to_goal_info + scope_to_help_info=scope_to_help_info, + name_to_goal_info=name_to_goal_info, + name_to_target_type_info=name_to_target_type_info, ) @staticmethod diff --git a/src/python/pants/help/help_info_extracter_test.py b/src/python/pants/help/help_info_extracter_test.py index f0e630e7d78..169674adcf4 100644 --- a/src/python/pants/help/help_info_extracter_test.py +++ b/src/python/pants/help/help_info_extracter_test.py @@ -3,9 +3,10 @@ import dataclasses from enum import Enum -from typing import Tuple +from typing import Any, Optional, Tuple from pants.engine.goal import GoalSubsystem +from pants.engine.target import IntField, RegisteredTargetTypes, StringField, Target from pants.engine.unions import UnionMembership from pants.help.help_info_extracter import HelpInfoExtracter, to_help_str from pants.option.config import Config @@ -133,7 +134,7 @@ def do_test(args, kwargs, expected_default_str): def test_compute_default(): - def do_test(expected_default, **kwargs): + def do_test(expected_default: Optional[Any], **kwargs): kwargs["default"] = RankedValue(Rank.HARDCODED, kwargs["default"]) assert expected_default == HelpInfoExtracter.compute_default(**kwargs) @@ -227,6 +228,31 @@ class Bar(GoalSubsystem): name = "bar" + class QuxField(StringField): + """A qux string.""" + + alias = "qux" + default = "blahblah" + + class QuuxField(IntField): + """A quux int. + + Must be non-zero. Or zero. Whatever you like really. + """ + + alias = "quux" + required = True + + class BazLibrary(Target): + """A library of baz-es. + + Use it however you like. + """ + + alias = "baz_library" + + core_fields = [QuxField, QuuxField] + options = Options.create( env={}, config=Config.load_file_contents(""), @@ -242,7 +268,10 @@ def fake_consumed_scopes_mapper(scope: str) -> Tuple[str, ...]: return ("somescope", f"used_by_{scope or 'GLOBAL_SCOPE'}") all_help_info = HelpInfoExtracter.get_all_help_info( - options, UnionMembership({}), fake_consumed_scopes_mapper + options, + UnionMembership({}), + fake_consumed_scopes_mapper, + RegisteredTargetTypes({BazLibrary.alias: BazLibrary}), ) all_help_info_dict = dataclasses.asdict(all_help_info) expected_all_help_info_dict = { @@ -346,5 +375,29 @@ def fake_consumed_scopes_mapper(scope: str) -> Tuple[str, ...]: "is_implemented": True, } }, + "name_to_target_type_info": { + "baz_library": { + "alias": "baz_library", + "summary": "A library of baz-es.", + "description": "A library of baz-es.\n\nUse it however you like.", + "fields": ( + { + "alias": "qux", + "default": "'blahblah'", + "description": "A qux string.", + "required": False, + "type_hint": "str | None", + }, + { + "alias": "quux", + "default": None, + "description": "A quux int. Must be non-zero. Or zero. " + "Whatever you like really.", + "required": True, + "type_hint": "int", + }, + ), + } + }, } assert expected_all_help_info_dict == all_help_info_dict diff --git a/src/python/pants/help/help_integration_test.py b/src/python/pants/help/help_integration_test.py index 4b866177f96..24ecf702805 100644 --- a/src/python/pants/help/help_integration_test.py +++ b/src/python/pants/help/help_integration_test.py @@ -2,6 +2,7 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). import json +import textwrap from pants.testutil.pants_integration_test import run_pants @@ -10,19 +11,92 @@ def test_help() -> None: pants_run = run_pants(["help"]) pants_run.assert_success() assert "Usage:" in pants_run.stdout - # spot check to see that a public global option is printed + + +def test_help_global() -> None: + pants_run = run_pants(["help", "global"]) + pants_run.assert_success() assert "--level" in pants_run.stdout assert "Global options" in pants_run.stdout -def test_help_advanced() -> None: - pants_run = run_pants(["help-advanced"]) +def test_help_advanced_global() -> None: + pants_run = run_pants(["help-advanced", "global"]) pants_run.assert_success() assert "Global advanced options" in pants_run.stdout # Spot check to see that a global advanced option is printed assert "--pants-bootstrapdir" in pants_run.stdout +def test_help_targets() -> None: + pants_run = run_pants(["help", "targets"]) + pants_run.assert_success() + assert "archive A ZIP or TAR file containing loose files" in pants_run.stdout + assert "to get help for a specific target" in pants_run.stdout + + +def test_help_specific_target() -> None: + pants_run = run_pants(["help", "archive"]) + pants_run.assert_success() + + assert ( + textwrap.dedent( + """ + archive + ------- + + A ZIP or TAR file containing loose files and code packages. + + Valid fields: + """ + ) + in pants_run.stdout + ) + + assert ( + textwrap.dedent( + """ + format + type: 'tar' | 'tar.bz2' | 'tar.gz' | 'tar.xz' | 'zip' + required + The type of archive file to be generated. + """ + ) + in pants_run.stdout + ) + + +def test_help_goals() -> None: + pants_run = run_pants(["help", "goals"]) + pants_run.assert_success() + assert "to get help for a specific goal" in pants_run.stdout + # Spot check a few core goals. + for goal in ["filedeps", "list", "roots", "validate"]: + assert goal in pants_run.stdout + + +def test_help_goals_only_show_implemented() -> None: + # Some core goals, such as `./pants test`, require downstream implementations to work + # properly. We should only show those goals when an implementation is provided. + goals_that_need_implementation = ["binary", "fmt", "lint", "run", "test"] + command = ["--pants-config-files=[]", "help", "goals"] + + not_implemented_run = run_pants(["--backend-packages=[]", *command]) + not_implemented_run.assert_success() + for goal in goals_that_need_implementation: + assert goal not in not_implemented_run.stdout + + implemented_run = run_pants( + [ + "--backend-packages=['pants.backend.python', 'pants.backend.python.lint.isort']", + *command, + ], + ) + implemented_run.assert_success() + for goal in goals_that_need_implementation: + assert goal in implemented_run.stdout + + def test_help_all() -> None: pants_run = run_pants(["--backend-packages=pants.backend.python", "help-all"]) pants_run.assert_success() @@ -41,22 +115,22 @@ def test_help_all() -> None: def test_unknown_goal() -> None: pants_run = run_pants(["testx"]) pants_run.assert_failure() - assert "Unknown goal: testx" in pants_run.stdout_data - assert "Did you mean: test" in pants_run.stdout_data + assert "Unknown goal: testx" in pants_run.stdout + assert "Did you mean: test" in pants_run.stdout def test_unknown_global_flags() -> None: pants_run = run_pants(["--pants-workdirx", "goals"]) pants_run.assert_failure() - assert "Unknown flag --pants-workdirx on global scope" in pants_run.stdout_data - assert "Did you mean --pants-workdir" in pants_run.stdout_data + assert "Unknown flag --pants-workdirx on global scope" in pants_run.stdout + assert "Did you mean --pants-workdir" in pants_run.stdout def test_unknown_scoped_flags() -> None: pants_run = run_pants(["test", "--forcex"]) pants_run.assert_failure() - assert "Unknown flag --forcex on test scope" in pants_run.stdout_data - assert "Did you mean --force" in pants_run.stdout_data + assert "Unknown flag --forcex on test scope" in pants_run.stdout + assert "Did you mean --force" in pants_run.stdout def test_global_flag_in_scoped_position() -> None: @@ -64,5 +138,5 @@ def test_global_flag_in_scoped_position() -> None: ["test", "--pants-distdir=dist/"], ) pants_run.assert_failure() - assert "Unknown flag --pants-distdir on test scope" in pants_run.stdout_data - assert "Did you mean to use the global --pants-distdir?" in pants_run.stdout_data + assert "Unknown flag --pants-distdir on test scope" in pants_run.stdout + assert "Did you mean to use the global --pants-distdir?" in pants_run.stdout diff --git a/src/python/pants/help/help_printer.py b/src/python/pants/help/help_printer.py index 02ff30bf4f3..9a5ac83fc36 100644 --- a/src/python/pants/help/help_printer.py +++ b/src/python/pants/help/help_printer.py @@ -15,10 +15,9 @@ from pants.help.maybe_color import MaybeColor from pants.option.arg_splitter import ( AllHelp, - GoalsHelp, HelpRequest, NoGoalHelp, - OptionsHelp, + ThingHelp, UnknownGoalHelp, VersionHelp, ) @@ -45,17 +44,15 @@ def print_help(self) -> Literal[0, 1]: """Print help to the console.""" def print_hint() -> None: - print(f"Use `{self.maybe_green(self._bin_name + ' goals')}` to list goals.") print(f"Use `{self.maybe_green(self._bin_name + ' help')}` to get help.") + print(f"Use `{self.maybe_green(self._bin_name + ' help goals')}` to list goals.") if isinstance(self._help_request, VersionHelp): print(pants_version()) - elif isinstance(self._help_request, GoalsHelp): - self._print_goals_help() elif isinstance(self._help_request, AllHelp): self._print_all_help() - elif isinstance(self._help_request, OptionsHelp): - self._print_options_help() + elif isinstance(self._help_request, ThingHelp): + self._print_thing_help() elif isinstance(self._help_request, UnknownGoalHelp): # Only print help and suggestions for the first unknown goal. # It gets confusing to try and show suggestions for multiple cases. @@ -76,6 +73,40 @@ def print_hint() -> None: return 1 return 0 + def _print_title(self, title_text: str) -> None: + title = self.maybe_green(f"{title_text}\n{'-' * len(title_text)}") + print(f"\n{title}\n") + + def _print_all_help(self) -> None: + print(self._get_help_json()) + + def _print_thing_help(self) -> None: + """Print a help screen. + + Assumes that self._help_request is an instance of OptionsHelp. + + Note: Ony useful if called after options have been registered. + """ + help_request = cast(ThingHelp, self._help_request) + things = set(help_request.things) + + if things: + for thing in sorted(things): + if thing == "goals": + self._print_goals_help() + elif thing == "targets": + self._print_targets_help() + elif thing == "global": + self._print_options_help(GLOBAL_SCOPE, help_request.advanced) + elif thing in self._all_help_info.scope_to_help_info: + self._print_options_help(thing, help_request.advanced) + elif thing in self._all_help_info.name_to_target_type_info: + self._print_target_help(thing) + else: + print(self.maybe_red(f"Unknown entity: {thing}")) + else: + self._print_global_help() + def _print_goals_help(self) -> None: goal_descriptions: Dict[str, str] = {} @@ -83,8 +114,7 @@ def _print_goals_help(self) -> None: if goal_info.is_implemented: goal_descriptions[goal_info.name] = goal_info.description - title_text = "Goals" - title = self.maybe_green(f"{title_text}\n{'-' * len(title_text)}") + self._print_title("Goals") max_width = max((len(name) for name in goal_descriptions.keys()), default=0) chars_before_description = max_width + 2 @@ -100,74 +130,57 @@ def format_goal(name: str, descr: str) -> str: formatted_descr = "\n".join(description_lines) return f"{name}{formatted_descr}\n" - lines = [ - f"\n{title}\n", - f"Use `{self._bin_name} help $goal` to get help for a particular goal.", - "\n", - *( - format_goal(name, description) - for name, description in sorted(goal_descriptions.items()) - ), - ] - print("\n".join(lines)) - - def _print_all_help(self) -> None: - print(self._get_help_json()) - - def _print_options_help(self) -> None: - """Print a help screen. - - Assumes that self._help_request is an instance of OptionsHelp. - - Note: Ony useful if called after options have been registered. - """ - help_request = cast(OptionsHelp, self._help_request) - # The scopes explicitly mentioned by the user on the cmd line. - help_scopes = set(help_request.scopes) - if help_scopes: - for scope in sorted(help_scopes): - help_str = self._format_help(scope, help_request.advanced) - if help_str: - print(help_str) - return - else: - self._print_global_help(help_request.advanced) - - def _print_global_help(self, advanced: bool): - print(f"Pants {pants_version()} https://pypi.org/pypi/pantsbuild.pants/{pants_version()}") - print("\nUsage:") - print( - f" {self._bin_name} [option ...] [goal ...] [target/file ...] Attempt the specified goals." + for name, description in sorted(goal_descriptions.items()): + print(format_goal(name, description)) + specific_help_cmd = f"{self._bin_name} help $goal" + print(f"Use `{self.maybe_green(specific_help_cmd)}` to get help for a specific goal.\n") + + def _print_global_help(self): + def print_cmd(args: str, desc: str): + cmd = self.maybe_green(f"{self._bin_name} {args}".ljust(50)) + print(f" {cmd} {desc}") + + print(f"\nPants {pants_version()}") + print("\nUsage:\n") + print_cmd( + "[option ...] [goal ...] [file/target ...]", + "Attempt the specified goals on the specified files/targets.", ) - print(f" {self._bin_name} help Get global help.") - print( - f" {self._bin_name} help [goal/subsystem] Get help for a goal or subsystem." + print_cmd("help", "Display this usage message.") + print_cmd("help goals", "List all installed goals.") + print_cmd("help targets", "List all installed target types.") + print_cmd("help global", "Help for global options.") + print_cmd("help-advanced global", "Help for global advanced options.") + print_cmd("help [targettype/goal/subsystem]", "Help for a target type, goal or subsystem.") + print_cmd( + "help-advanced [goal/subsystem]", "Help for a goal or subsystem's advanced options." ) + print_cmd("help-all", "Print a JSON object containing all help info.") + + print("") + print(" [file] can be:") + print(f" {self.maybe_cyan('path/to/file.ext')}") + glob_str = self.maybe_cyan("'**/*.ext'") print( - f" {self._bin_name} help-advanced Get help for global advanced options." + f" A path glob, such as {glob_str}, in quotes to prevent premature shell expansion." ) + print("\n [target] can be:") + print(f" {self.maybe_cyan('path/to/dir:target_name')}.") print( - f" {self._bin_name} help-advanced [goal/subsystem] Get help for a goal's or subsystem's advanced options." + f" {self.maybe_cyan('path/to/dir')} for a target whose name is the same as the directory name." ) print( - f" {self._bin_name} help-all Get a JSON object containing all help info." + f" {self.maybe_cyan('path/to/dir:')} to include all targets in the specified directory." ) print( - f" {self._bin_name} goals List all installed goals." + f" {self.maybe_cyan('path/to/dir::')} to include all targets found recursively under the directory.\n" ) - print("") - print(" [file] can be:") - print(" A path to a file.") - print(" A path glob, such as '**/*.ext', in quotes to prevent shell expansion.") - print(" [target] accepts two special forms:") - print(" dir: to include all targets in the specified directory.") - print(" dir:: to include all targets found recursively under the directory.\n") - print("More documentation is available at https://www.pantsbuild.org") - - print(self._format_help(GLOBAL_SCOPE, advanced)) + print(f"Documentation at {self.maybe_magenta('https://www.pantsbuild.org')}") + pypi_url = f"https://pypi.org/pypi/pantsbuild.pants/{pants_version()}" + print(f"Download at {self.maybe_magenta(pypi_url)}") - def _format_help(self, scope: str, show_advanced_and_deprecated: bool) -> str: - """Return a human-readable help message for the options registered on this object. + def _print_options_help(self, scope: str, show_advanced_and_deprecated: bool) -> None: + """Prints a human-readable help message for the options registered on this object. Assumes that self._help_request is an instance of OptionsHelp. """ @@ -178,7 +191,7 @@ def _format_help(self, scope: str, show_advanced_and_deprecated: bool) -> str: ) oshi = self._all_help_info.scope_to_help_info.get(scope) if not oshi: - return "" + return formatted_lines = help_formatter.format_options(oshi) goal_info = self._all_help_info.name_to_goal_info.get(scope) if goal_info: @@ -187,7 +200,45 @@ def _format_help(self, scope: str, show_advanced_and_deprecated: bool) -> str: related_subsystems_label = self.maybe_green("Related subsystems:") formatted_lines.append(f"{related_subsystems_label} {', '.join(related_scopes)}") formatted_lines.append("") - return "\n".join(formatted_lines).rstrip() + for line in formatted_lines: + print(line) + + def _print_targets_help(self) -> None: + self._print_title("Target types") + + longest_target_alias = max( + len(alias) for alias in self._all_help_info.name_to_target_type_info.keys() + ) + chars_before_description = longest_target_alias + 2 + for alias, target_type_info in sorted( + self._all_help_info.name_to_target_type_info.items(), key=lambda x: x[0] + ): + alias_str = self.maybe_cyan(f"{alias}".ljust(chars_before_description)) + summary = target_type_info.summary or "" + print(f"{alias_str}{summary}\n") + specific_help_cmd = f"{self._bin_name} help $targettype" + print( + f"Use `{self.maybe_green(specific_help_cmd)}` to get help for a specific " + f"target type.\n" + ) + + def _print_target_help(self, target_alias: str) -> None: + self._print_title(target_alias) + tinfo = self._all_help_info.name_to_target_type_info[target_alias] + if tinfo.description: + print(tinfo.description) + print("\nValid fields:") + for field in sorted(tinfo.fields, key=lambda x: x.alias): + print() + print(self.maybe_magenta(field.alias)) + indent = " " + required_or_default = "required" if field.required else f"default: {field.default}" + print(self.maybe_cyan(f"{indent}type: {field.type_hint}")) + print(self.maybe_cyan(f"{indent}{required_or_default}")) + if field.description: + for line in textwrap.wrap(field.description, 80): + print(f"{indent}{line}") + print() def _get_help_json(self) -> str: """Return a JSON object containing all the help info we have, for every scope.""" diff --git a/src/python/pants/help/list_goals_integration_test.py b/src/python/pants/help/list_goals_integration_test.py deleted file mode 100644 index d7aa249c00c..00000000000 --- a/src/python/pants/help/list_goals_integration_test.py +++ /dev/null @@ -1,42 +0,0 @@ -# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -from pants.testutil.pants_integration_test import run_pants - - -def test_goals() -> None: - pants_run = run_pants(["goals"]) - pants_run.assert_success() - assert "to get help for a particular goal" in pants_run.stdout - # Spot check a few core goals. - for goal in ["filedeps", "list", "roots", "validate"]: - assert goal in pants_run.stdout - - -def test_only_show_implemented_goals() -> None: - # Some core goals, such as `./pants test`, require downstream implementations to work - # properly. We should only show those goals when an implementation is provided. - goals_that_need_implementation = ["binary", "fmt", "lint", "run", "test"] - command = ["--pants-config-files=[]", "goals"] - - not_implemented_run = run_pants(["--backend-packages=[]", *command]) - not_implemented_run.assert_success() - for goal in goals_that_need_implementation: - assert goal not in not_implemented_run.stdout - - implemented_run = run_pants( - [ - "--backend-packages=['pants.backend.python', 'pants.backend.python.lint.isort']", - *command, - ], - ) - implemented_run.assert_success() - for goal in goals_that_need_implementation: - assert goal in implemented_run.stdout - - -def test_ignored_args() -> None: - # Test that arguments (some of which used to be relevant) are ignored. - pants_run = run_pants(["goals", "--all", "--graphviz", "--llama"]) - pants_run.assert_success() - assert "to get help for a particular goal" in pants_run.stdout diff --git a/src/python/pants/init/BUILD b/src/python/pants/init/BUILD index 677254b8b1c..be01a1a166e 100644 --- a/src/python/pants/init/BUILD +++ b/src/python/pants/init/BUILD @@ -12,7 +12,6 @@ target( dependencies=[ 'src/python/pants/backend/awslambda/python', 'src/python/pants/backend/codegen/protobuf/python', - 'src/python/pants/backend/pants_info', 'src/python/pants/backend/project_info', 'src/python/pants/backend/python', 'src/python/pants/backend/python/lint/bandit', diff --git a/src/python/pants/init/extension_loader.py b/src/python/pants/init/extension_loader.py index d95f63586a6..a648f3c0e09 100644 --- a/src/python/pants/init/extension_loader.py +++ b/src/python/pants/init/extension_loader.py @@ -109,9 +109,7 @@ def load_build_configuration_from_source( :raises: :class:``pants.base.exceptions.BuildConfigurationError`` if there is a problem loading the build configuration. """ - backend_packages = FrozenOrderedSet( - ["pants.core", "pants.backend.pants_info", "pants.backend.project_info", *backends] - ) + backend_packages = FrozenOrderedSet(["pants.core", "pants.backend.project_info", *backends]) for backend_package in backend_packages: load_backend(build_configuration, backend_package) diff --git a/src/python/pants/init/load_backends_integration_test.py b/src/python/pants/init/load_backends_integration_test.py index 4a06bdc2fdb..dadc3e872e6 100644 --- a/src/python/pants/init/load_backends_integration_test.py +++ b/src/python/pants/init/load_backends_integration_test.py @@ -13,7 +13,7 @@ def discover_backends() -> List[str]: str(register_py.parent).replace("src/python/", "").replace("/", ".") for register_py in register_pys } - always_activated = {"pants.core", "pants.backend.project_info", "pants.backend.pants_info"} + always_activated = {"pants.core", "pants.backend.project_info"} return sorted(backends - always_activated) diff --git a/src/python/pants/option/arg_splitter.py b/src/python/pants/option/arg_splitter.py index 11e688ec7e8..53a3f5191ab 100644 --- a/src/python/pants/option/arg_splitter.py +++ b/src/python/pants/option/arg_splitter.py @@ -31,21 +31,17 @@ class HelpRequest(ABC): @dataclass(frozen=True) -class OptionsHelp(HelpRequest): - """The user requested help on which options they can set.""" +class ThingHelp(HelpRequest): + """The user requested help on one or more things: e.g., an options scope or a target type.""" advanced: bool = False - scopes: Tuple[str, ...] = () + things: Tuple[str, ...] = () class VersionHelp(HelpRequest): """The user asked for the version of this instance of pants.""" -class GoalsHelp(HelpRequest): - """The user requested help for installed Goals.""" - - class AllHelp(HelpRequest): """The user requested a dump of all help info.""" @@ -77,13 +73,11 @@ class ArgSplitter: _HELP_BASIC_ARGS = ("-h", "--help", "help") _HELP_ADVANCED_ARGS = ("--help-advanced", "help-advanced") _HELP_VERSION_ARGS = ("-v", "-V", "--version", "version") - _HELP_GOALS_ARGS = ("goals",) _HELP_ALL_SCOPES_ARGS = ("help-all",) _HELP_ARGS = ( *_HELP_BASIC_ARGS, *_HELP_ADVANCED_ARGS, *_HELP_VERSION_ARGS, - *_HELP_GOALS_ARGS, *_HELP_ALL_SCOPES_ARGS, ) @@ -92,7 +86,6 @@ def __init__(self, known_scope_infos: Iterable[ScopeInfo], buildroot: str) -> No self._known_scope_infos = known_scope_infos self._known_scopes = {si.scope for si in known_scope_infos} | { "version", - "goals", "help", "help-advanced", "help-all", @@ -128,16 +121,14 @@ def _check_for_help_request(self, arg: str) -> bool: return False if arg in self._HELP_VERSION_ARGS: self._help_request = VersionHelp() - elif arg in self._HELP_GOALS_ARGS: - self._help_request = GoalsHelp() elif arg in self._HELP_ALL_SCOPES_ARGS: self._help_request = AllHelp() else: # First ensure that we have a basic OptionsHelp. if not self._help_request: - self._help_request = OptionsHelp() + self._help_request = ThingHelp() # Now see if we need to enhance it. - if isinstance(self._help_request, OptionsHelp): + if isinstance(self._help_request, ThingHelp): advanced = self._help_request.advanced or arg in self._HELP_ADVANCED_ARGS self._help_request = dataclasses.replace(self._help_request, advanced=advanced) return True @@ -200,14 +191,16 @@ def assign_flag_to_scope(flg: str, default_scope: str) -> None: self._unconsumed_args.pop() passthru = list(reversed(self._unconsumed_args)) - if unknown_scopes: + if unknown_scopes and not self._help_request: self._help_request = UnknownGoalHelp(tuple(unknown_scopes)) if not goals and not self._help_request: self._help_request = NoGoalHelp() - if isinstance(self._help_request, OptionsHelp): - self._help_request = dataclasses.replace(self._help_request, scopes=tuple(goals)) + if isinstance(self._help_request, ThingHelp): + self._help_request = dataclasses.replace( + self._help_request, things=tuple(goals) + tuple(unknown_scopes) + ) return SplitArgs( goals=list(goals), scope_to_flags=scope_to_flags, diff --git a/src/python/pants/option/arg_splitter_test.py b/src/python/pants/option/arg_splitter_test.py index acd93bdf301..0b5b5544ea1 100644 --- a/src/python/pants/option/arg_splitter_test.py +++ b/src/python/pants/option/arg_splitter_test.py @@ -12,7 +12,7 @@ AllHelp, ArgSplitter, NoGoalHelp, - OptionsHelp, + ThingHelp, UnknownGoalHelp, VersionHelp, ) @@ -54,7 +54,7 @@ def assert_valid_split( assert expected_passthru == split_args.passthru assert expected_is_help == (splitter.help_request is not None) assert expected_help_advanced == ( - isinstance(splitter.help_request, OptionsHelp) and splitter.help_request.advanced + isinstance(splitter.help_request, ThingHelp) and splitter.help_request.advanced ) assert expected_help_all == isinstance(splitter.help_request, AllHelp) @@ -102,8 +102,8 @@ def test_is_spec(self) -> None: # With files/directories on disk to tiebreak. with temporary_dir() as tmpdir: splitter = ArgSplitter(ArgSplitterTest._known_scope_infos, tmpdir) - for dir in directories_vs_goals: - Path(tmpdir, dir).mkdir() + for directory in directories_vs_goals: + Path(tmpdir, directory).mkdir() for f in files_vs_subscopes: Path(tmpdir, f).touch() for spec in ambiguous_specs: diff --git a/src/python/pants/option/global_options.py b/src/python/pants/option/global_options.py index 8660dd65937..1b4f40d8db3 100644 --- a/src/python/pants/option/global_options.py +++ b/src/python/pants/option/global_options.py @@ -164,12 +164,7 @@ def register_bootstrap_options(cls, register): default_rel_distdir = f"/{default_distdir_name}/" register( - "-l", - "--level", - type=LogLevel, - default=LogLevel.INFO, - help="Set the logging level. The logging levels are one of: " - '"error", "warn", "info", "debug", "trace".', + "-l", "--level", type=LogLevel, default=LogLevel.INFO, help="Set the logging level." ) register( "--show-log-target", From 9bfd130b8064f8072371ba0aeabc213cef5d516f Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Thu, 22 Oct 2020 16:25:41 -0400 Subject: [PATCH 2/3] Fix tests # 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] --- build-support/bin/ci.py | 4 ++-- .../pants/option/options_integration_test.py | 2 +- .../pantsd/pantsd_integration_test.py | 20 +++++++++---------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/build-support/bin/ci.py b/build-support/bin/ci.py index c5d30c7ef9a..8efda21d6a3 100755 --- a/build-support/bin/ci.py +++ b/build-support/bin/ci.py @@ -295,10 +295,10 @@ def run_check(command: List[str]) -> None: die(f"Failed to execute `./pants {command}`.") checks = [ - ["goals"], + ["help", "goals"], ["list", "::"], ["roots"], - ["target-types"], + ["help", "targets"], ] with travis_section("SmokeTest", "Smoke testing bootstrapped Pants and repo BUILD files"): check_pants_pex_exists() diff --git a/src/python/pants/option/options_integration_test.py b/src/python/pants/option/options_integration_test.py index fcc11add379..257ad853612 100644 --- a/src/python/pants/option/options_integration_test.py +++ b/src/python/pants/option/options_integration_test.py @@ -157,7 +157,7 @@ def test_from_config_invalid_section(self) -> None: """ ) ) - pants_run = self.run_pants([f"--pants-config-files={config_path}", "goals"]) + pants_run = self.run_pants([f"--pants-config-files={config_path}", "roots"]) pants_run.assert_failure() self.assertIn("ERROR] Invalid scope [invalid_scope]", pants_run.stderr) self.assertIn("ERROR] Invalid scope [another_invalid_scope]", pants_run.stderr) diff --git a/tests/python/pants_test/pantsd/pantsd_integration_test.py b/tests/python/pants_test/pantsd/pantsd_integration_test.py index 147c7ea125f..6d2eb1fc4cf 100644 --- a/tests/python/pants_test/pantsd/pantsd_integration_test.py +++ b/tests/python/pants_test/pantsd/pantsd_integration_test.py @@ -140,7 +140,7 @@ def test_pantsd_aligned_output(self) -> None: # Set for pytest output display. self.maxDiff = None - cmds = [["goals"], ["target-types"], ["roots"]] + cmds = [["help", "goals"], ["help", "targets"], ["roots"]] non_daemon_runs = [self.run_pants(cmd) for cmd in cmds] @@ -149,12 +149,12 @@ def test_pantsd_aligned_output(self) -> None: ctx.checker.assert_started() for cmd, run in zip(cmds, daemon_runs): - print(f"(cmd, run) = ({cmd}, {run.stdout_data}, {run.stderr_data})") - self.assertNotEqual(run.stdout_data, "", f"Empty stdout for {cmd}") + print(f"(cmd, run) = ({cmd}, {run.stdout}, {run.stderr_data})") + self.assertNotEqual(run.stdout, "", f"Empty stdout for {cmd}") for run_pair in zip(non_daemon_runs, daemon_runs): non_daemon_stdout = run_pair[0].stdout - daemon_stdout = run_pair[1].stdout_data + daemon_stdout = run_pair[1].stdout for line_pair in zip(non_daemon_stdout.splitlines(), daemon_stdout.splitlines()): assert line_pair[0] == line_pair[1] @@ -198,7 +198,7 @@ def test_pantsd_client_env_var_is_inherited_by_pantsd_runner_children(self): ) ctx.checker.assert_running() - self.assertEqual(EXPECTED_VALUE, "".join(result.stdout_data).strip()) + self.assertEqual(EXPECTED_VALUE, "".join(result.stdout).strip()) def test_pantsd_launch_env_var_is_not_inherited_by_pantsd_runner_children(self): with self.pantsd_test_context() as (workdir, pantsd_config, checker): @@ -377,17 +377,17 @@ def test_pantsd_invalidation_stale_sources(self): result = ctx.runner(filedeps_cmd) ctx.checker.assert_running() - assert non_existent_file not in result.stdout_data + assert non_existent_file not in result.stdout safe_file_dump(test_build_file, "python_library(sources=['*.py'])") result = ctx.runner(filedeps_cmd) ctx.checker.assert_running() - assert non_existent_file not in result.stdout_data + assert non_existent_file not in result.stdout safe_file_dump(test_src_file, "print('hello')\n") result = ctx.runner(filedeps_cmd) ctx.checker.assert_running() - assert test_src_file in result.stdout_data + assert test_src_file in result.stdout finally: rm_rf(test_path) @@ -636,7 +636,7 @@ def list_and_verify(): ctx.checker.assert_started() result.assert_success() expected_targets = {f"{directory}:{target}" for target in ("A", "B")} - self.assertEqual(expected_targets, set(result.stdout_data.strip().split("\n"))) + self.assertEqual(expected_targets, set(result.stdout.strip().split("\n"))) with open(os.path.join(directory, "BUILD"), "w") as f: f.write(template.format(a_deps='dependencies = [":B"],', b_deps="")) @@ -651,7 +651,7 @@ def test_concurrent_overrides_pantsd(self): concurrent runs under pantsd.""" config = {"GLOBAL": {"concurrent": True, "pantsd": True}} with temporary_workdir() as workdir: - pants_run = self.run_pants_with_workdir(["goals"], workdir=workdir, config=config) + pants_run = self.run_pants_with_workdir(["help", "goals"], workdir=workdir, config=config) pants_run.assert_success() pantsd_log_location = os.path.join(workdir, "pantsd", "pantsd.log") self.assertFalse(os.path.exists(pantsd_log_location)) From 0e9e0f5a0bfdcf1e04c071acb233f257e35288ae Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Thu, 22 Oct 2020 17:03:20 -0400 Subject: [PATCH 3/3] lint # 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] --- tests/python/pants_test/pantsd/pantsd_integration_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/python/pants_test/pantsd/pantsd_integration_test.py b/tests/python/pants_test/pantsd/pantsd_integration_test.py index 6d2eb1fc4cf..6dd41c0d30b 100644 --- a/tests/python/pants_test/pantsd/pantsd_integration_test.py +++ b/tests/python/pants_test/pantsd/pantsd_integration_test.py @@ -651,7 +651,9 @@ def test_concurrent_overrides_pantsd(self): concurrent runs under pantsd.""" config = {"GLOBAL": {"concurrent": True, "pantsd": True}} with temporary_workdir() as workdir: - pants_run = self.run_pants_with_workdir(["help", "goals"], workdir=workdir, config=config) + pants_run = self.run_pants_with_workdir( + ["help", "goals"], workdir=workdir, config=config + ) pants_run.assert_success() pantsd_log_location = os.path.join(workdir, "pantsd", "pantsd.log") self.assertFalse(os.path.exists(pantsd_log_location))