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.

Sign up for GitHub

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

Add mechanism to deprecate target types and fields #10966

Merged
merged 3 commits into from
Oct 15, 2020
Merged

Add mechanism to deprecate target types and fields #10966

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
99a02cf
Add mechanism to deprecate target types and fields
Eric-Arellano Oct 15, 2020
73eb739
Only warn on original targets, not generated subtargets
Eric-Arellano Oct 15, 2020
dc288f7
Remove unused imports
Eric-Arellano Oct 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/python/pants/backend/pants_info/list_target_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ def create(
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
],
)

Expand All @@ -208,11 +209,7 @@ def format_for_cli(self, console: Console) -> str:
output.extend(
[
"Valid fields:\n",
*sorted(
f"{field.format_for_cli(console)}\n"
for field in self.fields
if not field.alias.startswith("_")
),
*sorted(f"{field.format_for_cli(console)}\n" for field in self.fields),
]
)
return "\n".join(output).rstrip()
Expand All @@ -239,7 +236,7 @@ def list_target_types(
target_type, union_membership=union_membership
).as_dict()
for alias, target_type in registered_target_types.aliases_to_types.items()
if not alias.startswith("_")
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:
Expand All @@ -261,6 +258,10 @@ def list_target_types(
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
Expand All @@ -276,7 +277,6 @@ def list_target_types(
*(
target_info.format_for_cli(console, longest_target_alias=longest_target_alias)
for target_info in target_infos
if not target_info.alias.startswith("_")
),
]
print_stdout("\n".join(lines).rstrip())
Expand Down
44 changes: 10 additions & 34 deletions src/python/pants/backend/python/target_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import logging
import os.path
from textwrap import dedent
from typing import Any, Dict, Iterable, Optional, Tuple, Union, cast
from typing import Iterable, Optional, Tuple, Union, cast

from pkg_resources import Requirement

Expand Down Expand Up @@ -36,7 +36,6 @@
Target,
WrappedTarget,
)
from pants.engine.unions import UnionMembership
from pants.option.subsystem import Subsystem
from pants.python.python_requirement import PythonRequirement
from pants.python.python_setup import PythonSetup
Expand Down Expand Up @@ -281,26 +280,13 @@ class PythonBinary(PexBinary):
"""

alias = "python_binary"

def __init__( # type: ignore[misc] # This is `@final`, but it's fine to override here.
self,
unhydrated_values: Dict[str, Any],
*,
address: Address,
union_membership: Optional[UnionMembership] = None,
) -> None:
warn_or_error(
removal_version="2.1.0.dev0",
deprecated_entity_description="the `python_binary` target",
hint=(
f"Use the target type `pex_binary`, rather than `python_binary` for {address}. "
"The behavior is identical.\n\nTo fix this globally, run the below command:\n\t"
"macOS: for f in $(find . -name BUILD); do sed -i '' -Ee "
"'s#^python_binary\\(#pex_binary(#g' $f ; done\n\tLinux: for f in $(find . "
"-name BUILD); do sed -i -Ee 's#^python_binary\\(#pex_binary(#g' $f ; done\n"
),
)
super().__init__(unhydrated_values, address=address, union_membership=union_membership)
deprecated_removal_version = "2.1.0.dev0"
deprecated_removal_hint = (
"Use `pex_binary` instead, which behaves identically.\n\nTo fix this globally, run the "
"below command:\n\tmacOS: for f in $(find . -name BUILD); do sed -i '' -Ee "
"'s#^python_binary\\(#pex_binary(#g' $f ; done\n\tLinux: for f in $(find . -name BUILD); "
"do sed -i -Ee 's#^python_binary\\(#pex_binary(#g' $f ; done"
)


# -----------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -344,18 +330,8 @@ class PythonRuntimeBinaryDependencies(SpecialCasedDependencies):
types like `archive` and `python_awslambda`."""

alias = "runtime_binary_dependencies"

@classmethod
def sanitize_raw_value(
cls, raw_value: Optional[Iterable[str]], *, address: Address
) -> Optional[Tuple[str, ...]]:
if raw_value is not None:
logger.warning(
f"Using the `runtime_binary_dependencies` field in the target {address}. This "
"field is now deprecated in favor of the more flexible "
"`runtime_package_dependencies` field, and it will be removed in 2.1.0.dev0."
)
return super().sanitize_raw_value(raw_value, address=address)
deprecated_removal_version = "2.1.0dev0"
deprecated_removal_hint = "Use the more flexible `runtime_package_dependencies` field instead."


class PythonTestsTimeout(IntField):
Expand Down
51 changes: 41 additions & 10 deletions src/python/pants/engine/target.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import dataclasses
import itertools
import os.path
from abc import ABC, ABCMeta, abstractmethod
from abc import ABC, ABCMeta
from dataclasses import dataclass
from enum import Enum
from pathlib import PurePath
Expand All @@ -28,6 +28,7 @@

from typing_extensions import final

from pants.base.deprecated import warn_or_error
from pants.base.specs import Spec
from pants.engine.addresses import Address, UnparsedAddressInputs, assert_single_address
from pants.engine.collection import Collection, DeduplicatedCollection
Expand Down Expand Up @@ -58,17 +59,26 @@ class Field(ABC):
default: ClassVar[ImmutableValue]
# Subclasses may define these.
required: ClassVar[bool] = False
deprecated_removal_version: ClassVar[Optional[str]] = None
deprecated_removal_hint: ClassVar[Optional[str]] = None

# This is a little weird to have an abstract __init__(). We do this to ensure that all
# subclasses have this exact type signature for their constructor.
#
# Normally, with dataclasses, each constructor parameter would instead be specified via a
# dataclass field declaration. But, we don't want to declare either `address` or `raw_value` as
# attributes because we make no assumptions whether the subclasses actually store those values
# on each instance. All that we care about is a common constructor interface.
@abstractmethod
# NB: We still expect `PrimitiveField` and `AsyncField` to define their own constructors. This
# is only implemented so that we have a common deprecation mechanism.
def __init__(self, raw_value: Optional[Any], *, address: Address) -> None:
pass
if self.deprecated_removal_version and address.is_base_target and raw_value is not None:
if not self.deprecated_removal_hint:
raise ValueError(
f"You specified `deprecated_removal_version` for {self.__class__}, but not "
"the class property `deprecated_removal_hint`."
)
warn_or_error(
removal_version=self.deprecated_removal_version,
deprecated_entity_description=f"the {repr(self.alias)} field",
hint=(
f"Using the `{self.alias}` field in the target {address}. "
f"{self.deprecated_removal_hint}"
),
)


@frozen_after_init
Expand Down Expand Up @@ -123,6 +133,7 @@ def __init__(self, raw_value: Optional[Any], *, address: Address) -> None:
# this Field could not be passed around in the engine.
# * Don't store `address` to avoid the cost in memory of storing `Address` on every single
# field encountered by Pants in a run.
super().__init__(raw_value, address=address)
self.value = self.compute_value(raw_value, address=address)

@classmethod
Expand Down Expand Up @@ -214,6 +225,7 @@ def rules():

@final
def __init__(self, raw_value: Optional[Any], *, address: Address) -> None:
super().__init__(raw_value, address=address)
self.address = address
self.sanitized_raw_value = self.sanitize_raw_value(raw_value, address=address)

Expand Down Expand Up @@ -260,6 +272,10 @@ class Target(ABC):
alias: ClassVar[str]
core_fields: ClassVar[Tuple[Type[Field], ...]]

# Subclasses may define these.
deprecated_removal_version: ClassVar[Optional[str]] = None
deprecated_removal_hint: ClassVar[Optional[str]] = None

# These get calculated in the constructor
address: Address
plugin_fields: Tuple[Type[Field], ...]
Expand All @@ -276,6 +292,21 @@ def __init__(
# rarely directly instantiate Targets and should instead use the engine to request them.
union_membership: Optional[UnionMembership] = None,
) -> None:
if self.deprecated_removal_version and address.is_base_target:
if not self.deprecated_removal_hint:
raise ValueError(
f"You specified `deprecated_removal_version` for {self.__class__}, but not "
"the class property `deprecated_removal_hint`."
)
warn_or_error(
removal_version=self.deprecated_removal_version,
deprecated_entity_description=f"the {repr(self.alias)} target type",
hint=(
f"Using the `{self.alias}` target type for {address}. "
f"{self.deprecated_removal_hint}"
),
)

self.address = address
self.plugin_fields = self._find_plugin_fields(union_membership or UnionMembership({}))

Expand Down