Skip to content

Commit

Permalink
Deprecate the compatibility field in favor of `interpreter_constrai…
Browse files Browse the repository at this point in the history
…nts` (#11074)

This solves two problems:

1. `[python-setup].interpreter_constraints` did not align with `compatibility`.
2. `StringOrStringSequenceField` is confusing. It's better to always use `SequenceField`.*

*We can't use `StringField` as found in #9273 (comment).

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano authored Oct 29, 2020
1 parent c98ee36 commit 47b583e
Show file tree
Hide file tree
Showing 17 changed files with 184 additions and 213 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,27 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from pants.backend.codegen.protobuf.target_types import ProtobufLibrary
from pants.backend.python.target_types import PythonInterpreterCompatibility
from pants.backend.python.target_types import (
InterpreterConstraintsField,
PythonInterpreterCompatibility,
)
from pants.engine.target import StringField


class ProtobufPythonInterpreterConstraints(InterpreterConstraintsField):
alias = "python_interpreter_constraints"


class ProtobufPythonInterpreterCompatibility(PythonInterpreterCompatibility):
"""Deprecated in favor of the `python_interpreter_constraints` field."""

alias = "python_compatibility"
deprecated_removal_version = "2.2.0.dev0"
deprecated_removal_hint = (
"Use the field `python_interpreter_constraints`. The field does not work with bare strings "
"and expects a list of strings, so replace `python_compatibility='>3.6'` with "
"`python_interpreter_constraints=['>3.6']`."
)


class PythonSourceRootField(StringField):
Expand All @@ -21,6 +36,7 @@ class PythonSourceRootField(StringField):

def rules():
return [
ProtobufLibrary.register_plugin_field(ProtobufPythonInterpreterConstraints),
ProtobufLibrary.register_plugin_field(ProtobufPythonInterpreterCompatibility),
ProtobufLibrary.register_plugin_field(PythonSourceRootField),
]
10 changes: 2 additions & 8 deletions src/python/pants/backend/python/goals/pytest_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
)
from pants.backend.python.subsystems.pytest import PyTest
from pants.backend.python.target_types import (
PythonInterpreterCompatibility,
PythonRuntimeBinaryDependencies,
PythonRuntimePackageDependencies,
PythonTestsSources,
Expand Down Expand Up @@ -109,13 +108,8 @@ async def setup_pytest_for_target(
)
all_targets = transitive_targets.closure

interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields(
(
tgt[PythonInterpreterCompatibility]
for tgt in all_targets
if tgt.has_field(PythonInterpreterCompatibility)
),
python_setup,
interpreter_constraints = PexInterpreterConstraints.create_from_targets(
all_targets, python_setup
)

# Defaults to zip_safe=False.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def create_test_target(
python_tests(
name={repr(name)},
dependencies={dependencies or []},
compatibility={[interpreter_constraints] if interpreter_constraints else []},
interpreter_constraints={[interpreter_constraints] if interpreter_constraints else []},
)
"""
),
Expand Down
10 changes: 2 additions & 8 deletions src/python/pants/backend/python/goals/setup_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
from pants.backend.python.target_types import (
PexBinarySources,
PexEntryPointField,
PythonInterpreterCompatibility,
PythonProvidesField,
PythonRequirementsField,
PythonSources,
Expand Down Expand Up @@ -364,13 +363,8 @@ async def package_python_dist(
) -> BuiltPackage:
transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest([field_set.address]))
exported_target = ExportedTarget(transitive_targets.roots[0])
interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields(
(
tgt[PythonInterpreterCompatibility]
for tgt in transitive_targets.dependencies
if tgt.has_field(PythonInterpreterCompatibility)
),
python_setup,
interpreter_constraints = PexInterpreterConstraints.create_from_targets(
transitive_targets.closure, python_setup
)
chroot = await Get(
SetupPyChroot,
Expand Down
7 changes: 6 additions & 1 deletion src/python/pants/backend/python/lint/bandit/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
from typing import Optional, Tuple

from pants.backend.python.lint.bandit.subsystem import Bandit
from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonSources
from pants.backend.python.target_types import (
InterpreterConstraintsField,
PythonInterpreterCompatibility,
PythonSources,
)
from pants.backend.python.util_rules import pex
from pants.backend.python.util_rules.pex import (
Pex,
Expand Down Expand Up @@ -33,6 +37,7 @@ class BanditFieldSet(FieldSet):

sources: PythonSources
compatibility: PythonInterpreterCompatibility
interpreter_constraints: InterpreterConstraintsField


class BanditRequest(LintRequest):
Expand Down
18 changes: 15 additions & 3 deletions src/python/pants/backend/python/lint/black/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@

from pants.backend.python.lint.black.subsystem import Black
from pants.backend.python.lint.python_fmt import PythonFmtRequest
from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonSources
from pants.backend.python.target_types import (
InterpreterConstraintsField,
PythonInterpreterCompatibility,
PythonSources,
)
from pants.backend.python.util_rules import pex
from pants.backend.python.util_rules.pex import (
Pex,
Expand Down Expand Up @@ -36,7 +40,8 @@ class BlackFieldSet(FieldSet):
required_fields = (PythonSources,)

sources: PythonSources
interpreter_constraints: PythonInterpreterCompatibility
interpreter_constraints: InterpreterConstraintsField
deprecated_interpreter_constraints: PythonInterpreterCompatibility


class BlackRequest(PythonFmtRequest, LintRequest):
Expand Down Expand Up @@ -82,7 +87,14 @@ async def setup_black(
# `>=3.6` to result in requiring Python 3.8, which would error if 3.8 is not installed on the
# machine.
all_interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields(
(field_set.interpreter_constraints for field_set in setup_request.request.field_sets),
(
PexInterpreterConstraints.resolve_conflicting_fields(
field_set.deprecated_interpreter_constraints,
field_set.interpreter_constraints,
field_set.address,
)
for field_set in setup_request.request.field_sets
),
python_setup,
)
tool_interpreter_constraints = PexInterpreterConstraints(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,15 @@ def make_target(
for source_file in source_files:
rule_runner.create_file(source_file.path, source_file.content.decode())
rule_runner.add_to_build_file(
"", f"python_library(name='{name}', compatibility={repr(interpreter_constraints)})\n"
"",
dedent(
f"""\
python_library(
name='{name}',
interpreter_constraints={[interpreter_constraints] if interpreter_constraints else None},
)
"""
),
)
return rule_runner.get_target(Address("", target_name=name))

Expand Down
7 changes: 6 additions & 1 deletion src/python/pants/backend/python/lint/flake8/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
from typing import Optional, Tuple

from pants.backend.python.lint.flake8.subsystem import Flake8
from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonSources
from pants.backend.python.target_types import (
InterpreterConstraintsField,
PythonInterpreterCompatibility,
PythonSources,
)
from pants.backend.python.util_rules import pex
from pants.backend.python.util_rules.pex import (
Pex,
Expand Down Expand Up @@ -33,6 +37,7 @@ class Flake8FieldSet(FieldSet):

sources: PythonSources
compatibility: PythonInterpreterCompatibility
interpreter_constraints: InterpreterConstraintsField


class Flake8Request(LintRequest):
Expand Down
17 changes: 13 additions & 4 deletions src/python/pants/backend/python/lint/pylint/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from pants.backend.python.lint.pylint.subsystem import Pylint
from pants.backend.python.target_types import (
InterpreterConstraintsField,
PythonInterpreterCompatibility,
PythonRequirementsField,
PythonSources,
Expand Down Expand Up @@ -243,9 +244,13 @@ async def pylint_lint(
plugin_targets, linted_targets = await MultiGet(plugin_targets_request, linted_targets_request)

plugin_targets_compatibility_fields = tuple(
plugin_tgt[PythonInterpreterCompatibility]
PexInterpreterConstraints.resolve_conflicting_fields(
plugin_tgt[PythonInterpreterCompatibility],
plugin_tgt[InterpreterConstraintsField],
plugin_tgt.address,
)
for plugin_tgt in plugin_targets.closure
if plugin_tgt.has_field(PythonInterpreterCompatibility)
if plugin_tgt.has_fields([PythonInterpreterCompatibility, InterpreterConstraintsField])
)

# Pylint needs direct dependencies in the chroot to ensure that imports are valid. However, it
Expand All @@ -267,9 +272,13 @@ async def pylint_lint(
interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields(
(
*(
tgt[PythonInterpreterCompatibility]
PexInterpreterConstraints.resolve_conflicting_fields(
tgt[PythonInterpreterCompatibility],
tgt[InterpreterConstraintsField],
tgt.address,
)
for tgt in [tgt, *dependencies]
if tgt.has_field(PythonInterpreterCompatibility)
if tgt.has_fields([PythonInterpreterCompatibility, InterpreterConstraintsField])
),
*plugin_targets_compatibility_fields,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def make_target(
name={repr(name)},
sources={source_globs},
dependencies={[str(dep) for dep in dependencies or ()]},
compatibility={repr(interpreter_constraints)},
interpreter_constraints={[interpreter_constraints] if interpreter_constraints else None},
)
"""
),
Expand Down
40 changes: 33 additions & 7 deletions src/python/pants/backend/python/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,41 @@ class PythonSources(Sources):
expected_file_extensions = (".py", ".pyi")


class PythonInterpreterCompatibility(StringOrStringSequenceField):
"""A string for Python interpreter constraints on this target.
class InterpreterConstraintsField(StringSequenceField):
"""The Python interpreters this code is compatible with.
Each element should be written in pip-style format, e.g. 'CPython==2.7.*' or 'CPython>=3.6,<4'.
You can leave off `CPython` as a shorthand, e.g. '>=2.7' will be expanded to 'CPython>=2.7'.
This should be written in Requirement-style format, e.g. `CPython==2.7.*` or `CPython>=3.6,<4`.
As a shortcut, you can leave off `CPython`, e.g. `>=2.7` will be expanded to `CPython>=2.7`.
Specify more than one element to OR the constraints, e.g. `['PyPy==3.7.*', 'CPython==3.7.*']`
means either PyPy 3.7 _or_ CPython 3.7.
If this is left off, this will default to the option `interpreter_constraints` in the
[python-setup] scope.
If the field is not set, it will default to the option
`[python-setup].interpreter_constraints]`.
See https://www.pantsbuild.org/docs/python-interpreter-compatibility.
"""

alias = "interpreter_constraints"

def value_or_global_default(self, python_setup: PythonSetup) -> Tuple[str, ...]:
"""Return either the given `compatibility` field or the global interpreter constraints.
If interpreter constraints are supplied by the CLI flag, return those only.
"""
return python_setup.compatibility_or_constraints(self.value)


class PythonInterpreterCompatibility(StringOrStringSequenceField):
"""Deprecated in favor of the `interpreter_constraints` field."""

alias = "compatibility"
deprecated_removal_version = "2.2.0.dev0"
deprecated_removal_hint = (
"Use the field `interpreter_constraints`. The field does not work with bare strings "
"and expects a list of strings, so replace `compatibility='>3.6'` with "
"interpreter_constraints=['>3.6']`."
)

def value_or_global_default(self, python_setup: PythonSetup) -> Tuple[str, ...]:
"""Return either the given `compatibility` field or the global interpreter constraints.
Expand All @@ -76,7 +98,11 @@ def value_or_global_default(self, python_setup: PythonSetup) -> Tuple[str, ...]:
return python_setup.compatibility_or_constraints(self.value)


COMMON_PYTHON_FIELDS = (*COMMON_TARGET_FIELDS, PythonInterpreterCompatibility)
COMMON_PYTHON_FIELDS = (
*COMMON_TARGET_FIELDS,
InterpreterConstraintsField,
PythonInterpreterCompatibility,
)


# -----------------------------------------------------------------------------------------------
Expand Down
20 changes: 4 additions & 16 deletions src/python/pants/backend/python/typecheck/mypy/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,7 @@
from textwrap import dedent
from typing import Iterable, Optional, Tuple

from pants.backend.python.target_types import (
PythonInterpreterCompatibility,
PythonRequirementsField,
PythonSources,
)
from pants.backend.python.target_types import PythonRequirementsField, PythonSources
from pants.backend.python.typecheck.mypy.subsystem import MyPy
from pants.backend.python.util_rules import extract_pex, pex_from_targets
from pants.backend.python.util_rules.extract_pex import ExtractedPexDistributions
Expand Down Expand Up @@ -357,17 +353,9 @@ async def mypy_typecheck(

interpreter_constraints_to_transitive_targets = defaultdict(set)
for transitive_targets in transitive_targets_per_field_set:
interpreter_constraints = (
PexInterpreterConstraints.create_from_compatibility_fields(
(
tgt[PythonInterpreterCompatibility]
for tgt in transitive_targets.closure
if tgt.has_field(PythonInterpreterCompatibility)
),
python_setup,
)
or PexInterpreterConstraints(mypy.interpreter_constraints)
)
interpreter_constraints = PexInterpreterConstraints.create_from_targets(
transitive_targets.closure, python_setup
) or PexInterpreterConstraints(mypy.interpreter_constraints)
interpreter_constraints_to_transitive_targets[interpreter_constraints].add(
transitive_targets
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def make_target(
python_library(
name={repr(name)},
sources={source_globs},
compatibility={repr(interpreter_constraints)},
interpreter_constraints={[interpreter_constraints] if interpreter_constraints else None},
)
"""
),
Expand Down Expand Up @@ -447,7 +447,9 @@ def add(x, y):
"""
),
)
rule_runner.add_to_build_file(f"{PACKAGE}/py2", "python_library(compatibility='==2.7.*')")
rule_runner.add_to_build_file(
f"{PACKAGE}/py2", "python_library(interpreter_constraints=['==2.7.*'])"
)

rule_runner.create_file(f"{PACKAGE}/py3/__init__.py")
rule_runner.create_file(
Expand All @@ -459,7 +461,9 @@ def add(x: int, y: int) -> int:
"""
),
)
rule_runner.add_to_build_file(f"{PACKAGE}/py3", "python_library(compatibility='>=3.6')")
rule_runner.add_to_build_file(
f"{PACKAGE}/py3", "python_library(interpreter_constraints=['>=3.6'])"
)

# Our input files belong to the same target, which is compatible with both Py2 and Py3.
rule_runner.create_file(f"{PACKAGE}/__init__.py")
Expand All @@ -469,7 +473,9 @@ def add(x: int, y: int) -> int:
rule_runner.create_file(
f"{PACKAGE}/uses_py3.py", "from project.py3.lib import add\nassert add(2, 2) == 4\n"
)
rule_runner.add_to_build_file(PACKAGE, "python_library(compatibility=['==2.7.*', '>=3.6'])")
rule_runner.add_to_build_file(
PACKAGE, "python_library(interpreter_constraints=['==2.7.*', '>=3.6'])"
)
py2_target = rule_runner.get_target(Address(PACKAGE, relative_file_path="uses_py2.py"))
py3_target = rule_runner.get_target(Address(PACKAGE, relative_file_path="uses_py3.py"))

Expand Down
Loading

0 comments on commit 47b583e

Please sign in to comment.