Skip to content

Commit

Permalink
Merge cd3d1c1 into fe2d238
Browse files Browse the repository at this point in the history
  • Loading branch information
Eric-Arellano committed Sep 22, 2020
2 parents fe2d238 + cd3d1c1 commit d516fe3
Show file tree
Hide file tree
Showing 15 changed files with 160 additions and 152 deletions.
5 changes: 1 addition & 4 deletions src/python/pants/backend/python/lint/bandit/rules.py
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
2 changes: 1 addition & 1 deletion src/python/pants/backend/python/lint/bandit/subsystem.py
Expand Up @@ -15,7 +15,7 @@ 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"]
register_interpreter_constraints = False

@classmethod
def register_options(cls, register):
Expand Down
Expand Up @@ -13,6 +13,7 @@ class Docformatter(PythonToolBase):
options_scope = "docformatter"
default_version = "docformatter>=1.3.1,<1.4"
default_entry_point = "docformatter:main"
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
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
2 changes: 1 addition & 1 deletion src/python/pants/backend/python/lint/flake8/subsystem.py
Expand Up @@ -14,7 +14,7 @@ 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"]
register_interpreter_constraints = False

@classmethod
def register_options(cls, register):
Expand Down
23 changes: 15 additions & 8 deletions src/python/pants/backend/python/lint/pylint/rules.py
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
2 changes: 1 addition & 1 deletion src/python/pants/backend/python/lint/pylint/subsystem.py
Expand Up @@ -14,7 +14,7 @@ class Pylint(PythonToolBase):
options_scope = "pylint"
default_version = "pylint>=2.4.4,<2.5"
default_entry_point = "pylint"
default_interpreter_constraints = ["CPython>=3.5"]
register_interpreter_constraints = False

@classmethod
def register_options(cls, register):
Expand Down
5 changes: 1 addition & 4 deletions src/python/pants/backend/python/subsystems/ipython.py
@@ -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,8 @@ 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"]
register_interpreter_constraints = False

@classmethod
def register_options(cls, register):
Expand Down
46 changes: 33 additions & 13 deletions src/python/pants/backend/python/subsystems/python_tool_base.py
@@ -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] = True

@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,33 @@ def register_options(cls, register):
"library, invoked by a wrapper script.",
)

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
1 change: 1 addition & 0 deletions src/python/pants/backend/python/subsystems/setuptools.py
Expand Up @@ -12,3 +12,4 @@ class Setuptools(PythonToolBase):
options_scope = "setuptools"
default_version = "setuptools>=50.3.0,<50.4"
default_extra_requirements = ["wheel==0.35.1"]
default_interpreter_constraints = ["CPython>=3.5"]
51 changes: 29 additions & 22 deletions src/python/pants/backend/python/typecheck/mypy/rules.py
Expand Up @@ -149,34 +149,33 @@ async def mypy_typecheck(
# `python_version`. We don't want to make the user set this up. (If they do, MyPy will use
# `python_version`, rather than defaulting to the executing interpreter).
#
# We only apply these optimizations if the user did not configure
# `--mypy-interpreter-constraints`, and if we know that there are no Py35 or Py27
# constraints. If they use Py27 or Py35, this implies that they're not using Py36+ syntax,
# so it's fine to use the Py35 parser. We want the loosest constraints possible to make it
# more flexible to install MyPy.
# * We must resolve third-party dependencies. This should use whatever the actual code's
# constraints are. The constraints for the tool can be different than for the requirements.
# constraints are. However, PEX will error if they are not compatible
# with the interpreter being used to run MyPy. So, we should generally align the tool PEX
# constraints with the requirements constraints.
#
# However, it's possible for the requirements' constraints to include Python 2 and not be
# compatible with MyPy's >=3.5 requirement. If any of the requirements only have
# Python 2 wheels and they are not compatible with Python 3, then Pex will error about
# missing wheels. So, in this case, we set `PEX_IGNORE_ERRORS`, which will avoid erroring,
# but may result in MyPy complaining that it cannot find certain distributions.
#
# * The runner Pex should use the same constraints as the tool Pex.
all_interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields(
code_interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields(
(
tgt[PythonInterpreterCompatibility]
for tgt in itertools.chain(
typechecked_transitive_targets.closure, plugin_transitive_targets.closure
)
for tgt in typechecked_transitive_targets.closure
if tgt.has_field(PythonInterpreterCompatibility)
),
python_setup,
)
if not mypy.options.is_default("interpreter_constraints"):
tool_interpreter_constraints = mypy.interpreter_constraints
elif all_interpreter_constraints.requires_python38_or_newer():
tool_interpreter_constraints = ("CPython>=3.8",)
elif all_interpreter_constraints.requires_python37_or_newer():
tool_interpreter_constraints = ("CPython>=3.7",)
elif all_interpreter_constraints.requires_python36_or_newer():
tool_interpreter_constraints = ("CPython>=3.6",)
else:
tool_interpreter_constraints = mypy.interpreter_constraints
use_subsystem_constraints = (
not mypy.options.is_default("interpreter_constraints")
or code_interpreter_constraints.includes_python2()
)
tool_interpreter_constraints = (
mypy.interpreter_constraints if use_subsystem_constraints else code_interpreter_constraints
)

plugin_sources_request = Get(
PythonSourceFiles, PythonSourceFilesRequest(plugin_transitive_targets.closure)
Expand All @@ -201,7 +200,9 @@ async def mypy_typecheck(
Pex,
PexFromTargetsRequest,
PexFromTargetsRequest.for_requirements(
(field_set.address for field_set in request.field_sets), internal_only=True
(field_set.address for field_set in request.field_sets),
hardcoded_interpreter_constraints=code_interpreter_constraints,
internal_only=True,
),
)
runner_pex_request = Get(
Expand Down Expand Up @@ -277,13 +278,19 @@ async def mypy_typecheck(
all_used_source_roots = sorted(
set(itertools.chain(plugin_sources.source_roots, typechecked_sources.source_roots))
)
extra_env = {"PEX_EXTRA_SYS_PATH": ":".join(all_used_source_roots)}
# If the constraints are different for the tool than for the requirements, we must tell Pex to
# ignore errors. Otherwise, we risk runtime errors about missing dependencies.
if code_interpreter_constraints != tool_interpreter_constraints:
extra_env["PEX_IGNORE_ERRORS"] = "true"

result = await Get(
FallibleProcessResult,
PexProcess(
runner_pex,
argv=generate_args(mypy, file_list_path=file_list_path),
input_digest=merged_input_files,
extra_env={"PEX_EXTRA_SYS_PATH": ":".join(all_used_source_roots)},
extra_env=extra_env,
description=f"Run MyPy on {pluralize(len(typechecked_srcs_snapshot.files), 'file')}.",
level=LogLevel.DEBUG,
),
Expand Down
Expand Up @@ -20,7 +20,10 @@
from pants.engine.target import Target
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.python_interpreter_selection import skip_unless_python38_present
from pants.testutil.python_interpreter_selection import (
skip_unless_python27_present,
skip_unless_python38_present,
)
from pants.testutil.rule_runner import RuleRunner


Expand Down Expand Up @@ -347,6 +350,59 @@ def add(x: int, y: int) -> str:
assert f"{PACKAGE}/math/add.py:5" in result[0].stdout


@skip_unless_python27_present
def test_works_with_python27(rule_runner: RuleRunner) -> None:
"""A regression test that we can properly handle Python 2-only third-party dependencies.
There was a bug that this would cause the runner PEX to fail to execute because it did not have
Python 3 distributions of the requirements.
TODO(#10819): This support is not as robust as we'd like. We'll only use third-party
distributions if its wheel is also compatible with the Python 3 interpreter being used to run
MyPy. Is it possible to fix this to always include the Python 2 wheels and have MyPy respect
them?
"""
rule_runner.add_to_build_file(
"",
dedent(
"""\
# Both requirements are a) typed and b) compatible with Py2 and Py3. However, `x690`
# has a distinct wheel for Py2 vs. Py3, whereas libumi has a universal wheel. We only
# expect libumi to be usable by MyPy.
python_requirement_library(
name="libumi",
requirements=["libumi==0.0.2"],
)
python_requirement_library(
name="x690",
requirements=["x690==0.2.0"],
)
"""
),
)
source_file = FileContent(
f"{PACKAGE}/py2.py",
dedent(
"""\
from libumi import hello_world
from x690 import types
print "Blast from the past!"
print hello_world() - 21 # MyPy should fail. You can't subtract an `int` from `bytes`.
"""
).encode(),
)
target = make_target(rule_runner, [source_file], interpreter_constraints="==2.7.*")
result = run_mypy(rule_runner, [target], passthrough_args="--py2")
assert len(result) == 1
assert result[0].exit_code == 1
assert "Failed to execute PEX file" not in result[0].stderr
assert "Cannot find implementation or library stub for module named 'x690'" in result[0].stdout
assert f"{PACKAGE}/py2.py:5: error: Unsupported operand types" in result[0].stdout


@skip_unless_python38_present
def test_works_with_python38(rule_runner: RuleRunner) -> None:
"""MyPy's typed-ast dependency does not understand Python 3.8, so we must instead run MyPy with
Expand Down
20 changes: 0 additions & 20 deletions src/python/pants/backend/python/util_rules/pex.py
Expand Up @@ -242,26 +242,6 @@ def _requires_python3_version_or_newer(
return False
return True

def requires_python36_or_newer(self) -> bool:
"""Checks if the constraints are all for Python 3.6+.
This will return False if Python 3.6 is allowed, but prior versions like 3.5 are also
allowed.
"""
return self._requires_python3_version_or_newer(
allowed_versions=["3.6", "3.7", "3.8", "3.9"], prior_version="3.5"
)

def requires_python37_or_newer(self) -> bool:
"""Checks if the constraints are all for Python 3.7+.
This will return False if Python 3.7 is allowed, but prior versions like 3.6 are also
allowed.
"""
return self._requires_python3_version_or_newer(
allowed_versions=["3.7", "3.8", "3.9"], prior_version="3.6"
)

def requires_python38_or_newer(self) -> bool:
"""Checks if the constraints are all for Python 3.8+.
Expand Down

0 comments on commit d516fe3

Please sign in to comment.