Skip to content

Commit

Permalink
Add mechanism to deprecate target types and fields (#10966)
Browse files Browse the repository at this point in the history 
We leave off deprecated target types and fields from `./pants target-types` for now. In a followup, we may want to do something like `./pants help` where it shows the values, but in red.

We only log on the original targets in BUILD files, rather than on generated subtargets. This avoids unnecessary noise, where one target owning 7 source files would have had resulted in 8 different warnings.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
@Eric-Arellano
Eric-Arellano authored Oct 15, 2020
1 parent 02b75b1 commit b32f1d1
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 51 deletions.
14 changes: 7 additions & 7 deletions src/python/pants/backend/pants_info/list_target_types.py
View file
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
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
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

0 comments on commit b32f1d1

Please sign in to comment.