Skip to content

Commit

Permalink
[internal] Cleanup register_if type and use it for python_tool_base (…
Browse files Browse the repository at this point in the history 
…#14712)

This is the last subsystem to be retrofitted except for the bootstrap option tada

[ci skip-rust]
  • Loading branch information
@thejcannon
thejcannon authored Mar 6, 2022
1 parent f1e10ca commit 94921a8
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 115 deletions.
192 changes: 91 additions & 101 deletions src/python/pants/backend/python/subsystems/python_tool_base.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from __future__ import annotations

import importlib.resources
from typing import ClassVar, Iterable, Sequence, cast
from typing import ClassVar, Iterable, Sequence

from pants.backend.python.target_types import ConsoleScript, EntryPoint, MainSpecification
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
Expand All @@ -18,6 +18,7 @@
from pants.core.util_rules.lockfile_metadata import calculate_invalidation_digest
from pants.engine.fs import Digest, FileContent
from pants.option.errors import OptionsError
from pants.option.option_types import StrListOption, StrOption
from pants.option.subsystem import Subsystem
from pants.util.docutil import bin_name
from pants.util.ordered_set import FrozenOrderedSet
Expand Down Expand Up @@ -45,85 +46,73 @@ class PythonToolRequirementsBase(Subsystem):
default_lockfile_url: ClassVar[str | None] = None
uses_requirements_from_source_plugins: ClassVar[bool] = False

@classmethod
def register_options(cls, register):
super().register_options(register)
register(
"--version",
type=str,
advanced=True,
default=cls.default_version,
help="Requirement string for the tool.",
)
register(
"--extra-requirements",
type=list,
member_type=str,
advanced=True,
default=cls.default_extra_requirements,
help="Any additional requirement strings to use with the tool. This is useful if the "
"tool allows you to install plugins or if you need to constrain a dependency to "
"a certain version.",
)

if cls.default_interpreter_constraints and not cls.register_interpreter_constraints:
version = StrOption(
"--version",
advanced=True,
default=lambda cls: cls.default_version,
help="Requirement string for the tool.",
)
extra_requirements = StrListOption(
"--extra-requirements",
advanced=True,
default=lambda cls: cls.default_extra_requirements,
help="Any additional requirement strings to use with the tool. This is useful if the "
"tool allows you to install plugins or if you need to constrain a dependency to "
"a certain version.",
)
_interpreter_constraints = StrListOption(
"--interpreter-constraints",
register_if=lambda cls: cls.register_interpreter_constraints,
advanced=True,
default=lambda cls: cls.default_interpreter_constraints,
help="Python interpreter constraints for this tool.",
)

_lockfile = StrOption(
"--lockfile",
register_if=lambda cls: cls.register_lockfile,
default=DEFAULT_TOOL_LOCKFILE,
advanced=True,
help=lambda cls: (
"Path to a lockfile used for installing the tool.\n\n"
f"Set to the string `{DEFAULT_TOOL_LOCKFILE}` to use a lockfile provided by "
"Pants, so long as you have not changed the `--version` and "
"`--extra-requirements` options, and the tool's interpreter constraints are "
"compatible with the default. Pants will error or warn if the lockfile is not "
"compatible (controlled by `[python].invalid_lockfile_behavior`). See "
f"{cls.default_lockfile_url} for the default lockfile contents.\n\n"
f"Set to the string `{NO_TOOL_LOCKFILE}` to opt out of using a lockfile. We "
f"do not recommend this, though, as lockfiles are essential for reproducible "
f"builds.\n\n"
"To use a custom lockfile, set this option to a file path relative to the "
f"build root, then run `{bin_name()} generate-lockfiles "
f"--resolve={cls.options_scope}`.\n\n"
"Lockfile generation currently does not wire up the `[python-repos]` options. "
"If lockfile generation fails, you can manually generate a lockfile, such as "
"by using pip-compile or `pip freeze`. Set this option to the path to your "
"manually generated lockfile. When manually maintaining lockfiles, set "
"`[python].invalid_lockfile_behavior = 'ignore'`."
),
)

def __init__(self, *args, **kwargs):
if self.default_interpreter_constraints and not self.register_interpreter_constraints:
raise ValueError(
f"`default_interpreter_constraints` are configured for `{cls.options_scope}`, but "
f"`default_interpreter_constraints` are configured for `{self.options_scope}`, but "
"`register_interpreter_constraints` is not set to `True`, so the "
"`--interpreter-constraints` option will not be registered. Did you mean to set "
"this?"
)
if cls.register_interpreter_constraints:
register(
"--interpreter-constraints",
type=list,
advanced=True,
default=cls.default_interpreter_constraints,
help="Python interpreter constraints for this tool.",
)

if cls.register_lockfile and (
not cls.default_lockfile_resource or not cls.default_lockfile_url
if self.register_lockfile and (
not self.default_lockfile_resource or not self.default_lockfile_url
):
raise ValueError(
"The class property `default_lockfile_resource` and `default_lockfile_url` "
f"must be set if `register_lockfile` is set. See `{cls.options_scope}`."
)
if cls.register_lockfile:
register(
"--lockfile",
type=str,
default=DEFAULT_TOOL_LOCKFILE,
advanced=True,
help=(
"Path to a lockfile used for installing the tool.\n\n"
f"Set to the string `{DEFAULT_TOOL_LOCKFILE}` to use a lockfile provided by "
"Pants, so long as you have not changed the `--version` and "
"`--extra-requirements` options, and the tool's interpreter constraints are "
"compatible with the default. Pants will error or warn if the lockfile is not "
"compatible (controlled by `[python].invalid_lockfile_behavior`). See "
f"{cls.default_lockfile_url} for the default lockfile contents.\n\n"
f"Set to the string `{NO_TOOL_LOCKFILE}` to opt out of using a lockfile. We "
f"do not recommend this, though, as lockfiles are essential for reproducible "
f"builds.\n\n"
"To use a custom lockfile, set this option to a file path relative to the "
f"build root, then run `{bin_name()} generate-lockfiles "
f"--resolve={cls.options_scope}`.\n\n"
"Lockfile generation currently does not wire up the `[python-repos]` options. "
"If lockfile generation fails, you can manually generate a lockfile, such as "
"by using pip-compile or `pip freeze`. Set this option to the path to your "
"manually generated lockfile. When manually maintaining lockfiles, set "
"`[python].invalid_lockfile_behavior = 'ignore'`."
),
f"must be set if `register_lockfile` is set. See `{self.options_scope}`."
)

@property
def version(self) -> str:
return cast(str, self.options.version)

@property
def extra_requirements(self) -> tuple[str, ...]:
return tuple(self.options.extra_requirements)
super().__init__(*args, **kwargs)

@property
def all_requirements(self) -> tuple[str, ...]:
Expand Down Expand Up @@ -180,7 +169,7 @@ def lockfile(self) -> str:
This assumes you have set the class property `register_lockfile = True`.
"""
return cast(str, self.options.lockfile)
return self._lockfile

@property
def uses_lockfile(self) -> bool:
Expand All @@ -192,7 +181,7 @@ def interpreter_constraints(self) -> InterpreterConstraints:
This assumes you have set the class property `register_interpreter_constraints = True`.
"""
return InterpreterConstraints(self.options.interpreter_constraints)
return InterpreterConstraints(self._interpreter_constraints)

def to_pex_request(
self,
Expand All @@ -218,47 +207,48 @@ class PythonToolBase(PythonToolRequirementsBase):
# Subclasses must set.
default_main: ClassVar[MainSpecification]

@classmethod
def register_options(cls, register):
super().register_options(register)
register(
"--console-script",
type=str,
advanced=True,
default=cls.default_main.spec if isinstance(cls.default_main, ConsoleScript) else None,
help=(
"The console script for the tool. Using this option is generally preferable to "
"(and mutually exclusive with) specifying an --entry-point since console script "
"names have a higher expectation of staying stable across releases of the tool. "
"Usually, you will not want to change this from the default."
),
)
register(
"--entry-point",
type=str,
advanced=True,
default=cls.default_main.spec if isinstance(cls.default_main, EntryPoint) else None,
help=(
"The entry point for the tool. Generally you only want to use this option if the "
"tool does not offer a --console-script (which this option is mutually exclusive "
"with). Usually, you will not want to change this from the default."
),
)
console_script = StrOption(
"--console-script",
advanced=True,
default=lambda cls: (
cls.default_main.spec if isinstance(cls.default_main, ConsoleScript) else None
),
help=(
"The console script for the tool. Using this option is generally preferable to "
"(and mutually exclusive with) specifying an --entry-point since console script "
"names have a higher expectation of staying stable across releases of the tool. "
"Usually, you will not want to change this from the default."
),
)
entry_point = StrOption(
"--entry-point",
advanced=True,
default=lambda cls: (
cls.default_main.spec if isinstance(cls.default_main, EntryPoint) else None
),
help=(
"The entry point for the tool. Generally you only want to use this option if the "
"tool does not offer a --console-script (which this option is mutually exclusive "
"with). Usually, you will not want to change this from the default."
),
)

@property
def main(self) -> MainSpecification:
is_default_console_script = self.options.is_default("console_script")
is_default_entry_point = self.options.is_default("entry_point")
if not is_default_console_script and not is_default_entry_point:
raise OptionsError(
f"Both [{self.options_scope}].console-script={self.options.console_script} and "
f"[{self.options_scope}].entry-point={self.options.entry_point} are configured "
f"Both [{self.options_scope}].console-script={self.console_script} and "
f"[{self.options_scope}].entry-point={self.entry_point} are configured "
f"but these options are mutually exclusive. Please pick one."
)
if not is_default_console_script:
return ConsoleScript(cast(str, self.options.console_script))
assert self.console_script is not None
return ConsoleScript(self.console_script)
if not is_default_entry_point:
return EntryPoint.parse(cast(str, self.options.entry_point))
assert self.entry_point is not None
return EntryPoint.parse(self.entry_point)
return self.default_main

def to_pex_request(
Expand Down
28 changes: 14 additions & 14 deletions src/python/pants/option/option_types.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,11 @@
import inspect
from dataclasses import dataclass
from enum import Enum
from typing import TYPE_CHECKING, Any, Callable, Generic, TypeVar, Union, cast, overload
from typing import Any, Callable, Generic, TypeVar, Union, cast, overload

from pants.option import custom_types
from pants.util.docutil import bin_name

if TYPE_CHECKING:
pass


@dataclass(frozen=True)
class OptionsInfo:
Expand All @@ -40,6 +37,9 @@ class OptionsInfo:
_MaybeDynamicT = Union[_DynamicDefaultT, _DefaultT]
# The type of the `help` parameter for each option.
_HelpT = _MaybeDynamicT[str]
# NB: Ideally this would be `_RegisterIfFuncT`, however where this type is used is
# generally provided untyped lambdas.
_RegisterIfFuncT = Callable[[_SubsystemType], Any]


def _eval_maybe_dynamic(val: _MaybeDynamicT[_DefaultT], subsystem_cls: _SubsystemType) -> _DefaultT:
Expand All @@ -59,7 +59,7 @@ class _OptionBase(Generic[_OptT, _DefaultT]):
_flag_names: tuple[str, ...]
_default: _MaybeDynamicT[_DefaultT]
_help: _HelpT
_register_if: Callable[[_SubsystemType], bool]
_register_if: _RegisterIfFuncT
_extra_kwargs: dict[str, Any]

# NB: We define `__new__` rather than `__init__` because some subclasses need to define
Expand All @@ -69,7 +69,7 @@ def __new__(
*flag_names: str,
default: _MaybeDynamicT[_DefaultT],
help: _HelpT,
register_if: Callable[[_SubsystemType], bool] | None = None,
register_if: _RegisterIfFuncT | None = None,
# Additional bells/whistles
advanced: bool | None = None,
daemon: bool | None = None,
Expand Down Expand Up @@ -163,7 +163,7 @@ def __new__(
*flag_names: str,
default: _MaybeDynamicT[list[_ListMemberT]] = [],
help: _HelpT,
register_if: Callable[[_SubsystemType], bool] | None = None,
register_if: _RegisterIfFuncT | None = None,
# Additional bells/whistles
advanced: bool | None = None,
daemon: bool | None = None,
Expand Down Expand Up @@ -376,7 +376,7 @@ def __new__(
*flag_names: str,
default: _EnumT,
help: _HelpT,
register_if: Callable[[_SubsystemType], bool] | None = None,
register_if: _RegisterIfFuncT | None = None,
# Additional bells/whistles
advanced: bool | None = None,
daemon: bool | None = None,
Expand All @@ -398,7 +398,7 @@ def __new__(
enum_type: type[_EnumT],
default: _DynamicDefaultT,
help: _HelpT,
register_if: Callable[[_SubsystemType], bool] | None = None,
register_if: _RegisterIfFuncT | None = None,
# Additional bells/whistles
advanced: bool | None = None,
daemon: bool | None = None,
Expand All @@ -420,7 +420,7 @@ def __new__(
enum_type: type[_EnumT],
default: None,
help: _HelpT,
register_if: Callable[[_SubsystemType], bool] | None = None,
register_if: _RegisterIfFuncT | None = None,
# Additional bells/whistles
advanced: bool | None = None,
daemon: bool | None = None,
Expand Down Expand Up @@ -506,7 +506,7 @@ def __new__(
*flag_names: str,
default: list[_EnumT],
help: _HelpT,
register_if: Callable[[_SubsystemType], bool] | None = None,
register_if: _RegisterIfFuncT | None = None,
# Additional bells/whistles
advanced: bool | None = ...,
daemon: bool | None = ...,
Expand All @@ -528,7 +528,7 @@ def __new__(
enum_type: type[_EnumT],
default: _DynamicDefaultT,
help: _HelpT,
register_if: Callable[[_SubsystemType], bool] | None = None,
register_if: _RegisterIfFuncT | None = None,
# Additional bells/whistles
advanced: bool | None = ...,
daemon: bool | None = ...,
Expand All @@ -549,7 +549,7 @@ def __new__(
*flag_names: str,
enum_type: type[_EnumT],
help: _HelpT,
register_if: Callable[[_SubsystemType], bool] | None = None,
register_if: _RegisterIfFuncT | None = None,
# Additional bells/whistles
advanced: bool | None = ...,
daemon: bool | None = ...,
Expand Down Expand Up @@ -648,7 +648,7 @@ def __new__(
*flag_names,
default: _MaybeDynamicT[dict[str, _ValueT]] = {},
help,
register_if: Callable[[_SubsystemType], bool] | None = None,
register_if: _RegisterIfFuncT | None = None,
advanced: bool | None = None,
daemon: bool | None = None,
default_help_repr: str | None = None,
Expand Down

0 comments on commit 94921a8

Please sign in to comment.