Skip to content

Commit

Permalink
Deprecate --interpreter-constraints option where it no-ops (#10833)
Browse files Browse the repository at this point in the history
There are several places now where `--interpreter-constraints` is never used, or it's exceedingly rare to be used so it's misleading to have as an option.

This also fixes Pylint to ensure that no matter what, the tool Pex uses the same constraints as the requirements Pex. It looks like this was happening in practice, but it's important to enforce to avoid a Pex runtime error about "Missing Wheels".

Finally, this fixes `setuptools` to have default constraints, which it needs.

[ci skip-build-wheels]
[ci skip-rust]
  • Loading branch information
Eric-Arellano authored Sep 23, 2020
1 parent f4763dd commit 9aec06f
Show file tree
Hide file tree
Showing 17 changed files with 85 additions and 45 deletions.
1 change: 1 addition & 0 deletions src/python/pants/backend/awslambda/python/lambdex.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ class Lambdex(PythonToolBase):
# TODO(John Sirois): Remove when we can upgrade to a version of lambdex with
# https://github.com/wickman/lambdex/issues/6 fixed.
default_extra_requirements = ["setuptools>=50.3.0,<50.4"]
register_interpreter_constraints = True
default_interpreter_constraints = ["CPython>=3.5"]
default_entry_point = "lambdex.bin.lambdex"
1 change: 1 addition & 0 deletions src/python/pants/backend/python/goals/coverage_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ class CoverageSubsystem(PythonToolBase):
options_scope = "coverage-py"
default_version = "coverage>=5.0.3,<5.1"
default_entry_point = "coverage"
register_interpreter_constraints = True
default_interpreter_constraints = ["CPython>=3.6"]

@classmethod
Expand Down
5 changes: 1 addition & 4 deletions src/python/pants/backend/python/lint/bandit/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ async def bandit_lint_partition(
output_filename="bandit.pex",
internal_only=True,
requirements=PexRequirements(bandit.all_requirements),
interpreter_constraints=(
partition.interpreter_constraints
or PexInterpreterConstraints(bandit.interpreter_constraints)
),
interpreter_constraints=partition.interpreter_constraints,
entry_point=bandit.entry_point,
),
)
Expand Down
1 change: 0 additions & 1 deletion src/python/pants/backend/python/lint/bandit/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ class Bandit(PythonToolBase):
# `setuptools<45` is for Python 2 support. `stevedore` is because the 3.0 release breaks Bandit.
default_extra_requirements = ["setuptools<45", "stevedore<3"]
default_entry_point = "bandit"
default_interpreter_constraints = ["CPython>=2.7,<3", "CPython>=3.5"]

@classmethod
def register_options(cls, register):
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/python/lint/black/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class Black(PythonToolBase):
default_version = "black==20.8b1"
default_extra_requirements = ["setuptools"]
default_entry_point = "black:patched_main"
register_interpreter_constraints = True
default_interpreter_constraints = ["CPython>=3.6"]

@classmethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ class Docformatter(PythonToolBase):
options_scope = "docformatter"
default_version = "docformatter>=1.3.1,<1.4"
default_entry_point = "docformatter:main"
register_interpreter_constraints = True
default_interpreter_constraints = ["CPython==2.7.*", "CPython>=3.4,<3.9"]

@classmethod
def register_options(cls, register):
Expand Down
5 changes: 1 addition & 4 deletions src/python/pants/backend/python/lint/flake8/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ async def flake8_lint_partition(
output_filename="flake8.pex",
internal_only=True,
requirements=PexRequirements(flake8.all_requirements),
interpreter_constraints=(
partition.interpreter_constraints
or PexInterpreterConstraints(flake8.interpreter_constraints)
),
interpreter_constraints=partition.interpreter_constraints,
entry_point=flake8.entry_point,
),
)
Expand Down
1 change: 0 additions & 1 deletion src/python/pants/backend/python/lint/flake8/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ class Flake8(PythonToolBase):
default_version = "flake8>=3.7.9,<3.9"
default_extra_requirements = ["setuptools<45"] # NB: `<45` is for Python 2 support
default_entry_point = "flake8"
default_interpreter_constraints = ["CPython>=2.7,<3", "CPython>=3.4"]

@classmethod
def register_options(cls, register):
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/python/lint/isort/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class Isort(PythonToolBase):
options_scope = "isort"
default_version = "isort[pyproject]>=5.5.1,<5.6"
default_extra_requirements = ["setuptools"]
register_interpreter_constraints = True
default_interpreter_constraints = ["CPython>=3.6"]
default_entry_point = "isort.main"

Expand Down
23 changes: 15 additions & 8 deletions src/python/pants/backend/python/lint/pylint/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ async def pylint_lint_partition(partition: PylintPartition, pylint: Pylint) -> L
PexFromTargetsRequest,
PexFromTargetsRequest.for_requirements(
(field_set.address for field_set in partition.field_sets),
# NB: These constraints must be identical to the other PEXes. Otherwise, we risk using
# a different version for the requirements than the other two PEXes, which can result
# in a PEX runtime error about missing dependencies.
hardcoded_interpreter_constraints=partition.interpreter_constraints,
internal_only=True,
direct_deps_only=True,
),
Expand Down Expand Up @@ -267,20 +271,23 @@ async def pylint_lint(

# We batch targets by their interpreter constraints to ensure, for example, that all Python 2
# targets run together and all Python 3 targets run together.
# Note that Pylint uses the AST of the interpreter that runs it. So, we include any plugin
# targets in this interpreter constraints calculation.
interpreter_constraints_to_target_setup = defaultdict(set)
for field_set, tgt, dependencies in zip(
request.field_sets, linted_targets, per_target_dependencies
):
target_setup = PylintTargetSetup(field_set, Targets([tgt, *dependencies]))
interpreter_constraints = (
PexInterpreterConstraints.create_from_compatibility_fields(
(
*(tgt.get(PythonInterpreterCompatibility) for tgt in [tgt, *dependencies]),
*plugin_targets_compatibility_fields,
interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields(
(
*(
tgt[PythonInterpreterCompatibility]
for tgt in [tgt, *dependencies]
if tgt.has_field(PythonInterpreterCompatibility)
),
python_setup,
)
or PexInterpreterConstraints(pylint.interpreter_constraints)
*plugin_targets_compatibility_fields,
),
python_setup,
)
interpreter_constraints_to_target_setup[interpreter_constraints].add(target_setup)

Expand Down
1 change: 0 additions & 1 deletion src/python/pants/backend/python/lint/pylint/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ class Pylint(PythonToolBase):
options_scope = "pylint"
default_version = "pylint>=2.4.4,<2.5"
default_entry_point = "pylint"
default_interpreter_constraints = ["CPython>=3.5"]

@classmethod
def register_options(cls, register):
Expand Down
4 changes: 0 additions & 4 deletions src/python/pants/backend/python/subsystems/ipython.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from typing import List

from pants.backend.python.subsystems.python_tool_base import PythonToolBase


Expand All @@ -11,9 +9,7 @@ class IPython(PythonToolBase):

options_scope = "ipython"
default_version = "ipython==7.16.1" # The last version to support Python 3.6.
default_extra_requirements: List[str] = []
default_entry_point = "IPython:start_ipython"
default_interpreter_constraints = ["CPython>=3.6"]

@classmethod
def register_options(cls, register):
Expand Down
53 changes: 40 additions & 13 deletions src/python/pants/backend/python/subsystems/python_tool_base.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from typing import Optional, Sequence, Tuple, cast
from typing import ClassVar, Optional, Sequence, Tuple, cast

from pants.option.subsystem import Subsystem

Expand All @@ -10,11 +10,12 @@ class PythonToolBase(Subsystem):
"""Base class for subsystems that configure a python tool to be invoked out-of-process."""

# Subclasses must set.
default_version: Optional[str] = None
default_entry_point: Optional[str] = None
default_version: ClassVar[Optional[str]] = None
default_entry_point: ClassVar[Optional[str]] = None
# Subclasses do not need to override.
default_extra_requirements: Sequence[str] = []
default_interpreter_constraints: Sequence[str] = []
default_extra_requirements: ClassVar[Sequence[str]] = []
default_interpreter_constraints: ClassVar[Sequence[str]] = []
register_interpreter_constraints: ClassVar[bool] = False

@classmethod
def register_options(cls, register):
Expand All @@ -36,14 +37,6 @@ def register_options(cls, register):
"tool allows you to install plugins or if you need to constrain a dependency to "
"a certain version.",
)
register(
"--interpreter-constraints",
type=list,
advanced=True,
default=cls.default_interpreter_constraints,
help="Python interpreter constraints for this tool. An empty list uses the default "
"interpreter constraints for the repo.",
)
register(
"--entry-point",
type=str,
Expand All @@ -54,6 +47,40 @@ def register_options(cls, register):
"library, invoked by a wrapper script.",
)

if cls.default_interpreter_constraints and not cls.register_interpreter_constraints:
raise ValueError(
f"`default_interpreter_constraints` are configured for `{cls.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?"
)
interpreter_constraints_help = (
"Python interpreter constraints for this tool. An empty list uses the default "
"interpreter constraints for the repo."
)
if cls.register_interpreter_constraints:
register(
"--interpreter-constraints",
type=list,
advanced=True,
default=cls.default_interpreter_constraints,
help=interpreter_constraints_help,
)
else:
register(
"--interpreter-constraints",
type=list,
advanced=True,
default=[],
help=interpreter_constraints_help,
removal_version="2.1.0.dev0",
removal_hint=(
"This option no longer does anything, as Pants auto-configures the interpreter "
f"constraints for {cls.options_scope} based on your code's interpreter "
"constraints."
),
)

@property
def version(self) -> Optional[str]:
return cast(Optional[str], self.options.version)
Expand Down
2 changes: 2 additions & 0 deletions src/python/pants/backend/python/subsystems/setuptools.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ class Setuptools(PythonToolBase):
options_scope = "setuptools"
default_version = "setuptools>=50.3.0,<50.4"
default_extra_requirements = ["wheel==0.35.1"]
register_interpreter_constraints = True
default_interpreter_constraints = ["CPython>=3.5"]
3 changes: 1 addition & 2 deletions src/python/pants/backend/python/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,7 @@ class PythonBinary(Target):
"""A Python target that can be converted into an executable PEX file.
PEX files are self-contained executable files that contain a complete Python environment capable
of running the target. For more information about PEX files, see
https://www.pantsbuild.org/docs/pex-files.
of running the target. For more information, see https://www.pantsbuild.org/docs/pex-files.
"""

alias = "python_binary"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class MyPy(PythonToolBase):
default_version = "mypy==0.782"
default_entry_point = "mypy"
# See `mypy/rules.py`. We only use these default constraints in some situations.
register_interpreter_constraints = True
default_interpreter_constraints = ["CPython>=3.5"]

@classmethod
Expand Down
25 changes: 18 additions & 7 deletions src/python/pants/backend/python/util_rules/pex_from_targets.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class PexFromTargetsRequest:
include_source_files: bool
additional_sources: Optional[Digest]
additional_inputs: Optional[Digest]
hardcoded_interpreter_constraints: Optional[PexInterpreterConstraints]
direct_deps_only: bool
# This field doesn't participate in comparison (and therefore hashing), as it doesn't affect
# the result.
Expand All @@ -77,6 +78,7 @@ def __init__(
include_source_files: bool = True,
additional_sources: Optional[Digest] = None,
additional_inputs: Optional[Digest] = None,
hardcoded_interpreter_constraints: Optional[PexInterpreterConstraints] = None,
direct_deps_only: bool = False,
description: Optional[str] = None,
) -> None:
Expand Down Expand Up @@ -105,6 +107,8 @@ def __init__(
:param additional_sources: Any additional source files to include in the built Pex.
:param additional_inputs: Any inputs that are not source files and should not be included
directly in the Pex, but should be present in the environment when building the Pex.
:param hardcoded_interpreter_constraints: Use these constraints rather than resolving the
constraints from the input.
:param direct_deps_only: Only consider the input addresses and their direct dependencies,
rather than the transitive closure.
:param description: A human-readable description to render in the dynamic UI when building
Expand All @@ -120,6 +124,7 @@ def __init__(
self.include_source_files = include_source_files
self.additional_sources = additional_sources
self.additional_inputs = additional_inputs
self.hardcoded_interpreter_constraints = hardcoded_interpreter_constraints
self.direct_deps_only = direct_deps_only
self.description = description

Expand All @@ -129,6 +134,7 @@ def for_requirements(
addresses: Iterable[Address],
*,
internal_only: bool,
hardcoded_interpreter_constraints: Optional[PexInterpreterConstraints] = None,
zip_safe: bool = False,
direct_deps_only: bool = False,
) -> "PexFromTargetsRequest":
Expand All @@ -148,6 +154,7 @@ def for_requirements(
output_filename="requirements.pex",
include_source_files=False,
additional_args=() if zip_safe else ("--not-zip-safe",),
hardcoded_interpreter_constraints=hardcoded_interpreter_constraints,
internal_only=internal_only,
direct_deps_only=direct_deps_only,
)
Expand Down Expand Up @@ -190,13 +197,17 @@ async def pex_from_targets(request: PexFromTargetsRequest, python_setup: PythonS
input_digests.append(prepared_sources.stripped_source_files.snapshot.digest)
merged_input_digest = await Get(Digest, MergeDigests(input_digests))

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

exact_reqs = PexRequirements.create_from_requirement_fields(
Expand Down

0 comments on commit 9aec06f

Please sign in to comment.