Skip to content

Commit

Permalink
Fix MyPy with Python 2-only third-party requirements (#10820)
Browse files Browse the repository at this point in the history
Closes #10819.

Turns out that when using `--pex-path` with PEX, it will expect for every requirement to have a wheel that is compatible with the current interpreter, unless you set `PEX_IGNORE_ERRORS`. This means that it is naively not safe to resolve requirements with Python 2, but run MyPy with Python 3.5+.

Likewise, it is not safe to resolve requirements with Python 3.6 but run MyPy with Python 3.7.

Instead, we generally use the same interpreter constraints for the requirements when creating the tool pex. However, if the requirements have any Python 2 code—or the user set `--mypy-interpreter-constraints`—then we will let the two sets diverge, and we'll set `PEX_IGNORE_ERRORS`.
  • Loading branch information
Eric-Arellano authored Sep 23, 2020
1 parent 9aec06f commit f8a0b0f
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 25 deletions.
57 changes: 33 additions & 24 deletions src/python/pants/backend/python/typecheck/mypy/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,34 +149,35 @@ 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 = (
PexInterpreterConstraints(mypy.interpreter_constraints)
if use_subsystem_constraints
else code_interpreter_constraints
)

plugin_sources_request = Get(
PythonSourceFiles, PythonSourceFilesRequest(plugin_transitive_targets.closure)
Expand All @@ -193,15 +194,17 @@ async def mypy_typecheck(
requirements=PexRequirements(
itertools.chain(mypy.all_requirements, plugin_requirements)
),
interpreter_constraints=PexInterpreterConstraints(tool_interpreter_constraints),
interpreter_constraints=tool_interpreter_constraints,
entry_point=mypy.entry_point,
),
)
requirements_pex_request = Get(
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 All @@ -210,7 +213,7 @@ async def mypy_typecheck(
output_filename="mypy_runner.pex",
internal_only=True,
sources=launcher_script,
interpreter_constraints=PexInterpreterConstraints(tool_interpreter_constraints),
interpreter_constraints=tool_interpreter_constraints,
entry_point=PurePath(LAUNCHER_FILE.path).stem,
additional_args=(
"--pex-path",
Expand Down Expand Up @@ -277,13 +280,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
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@
from pants.engine.rules import QueryRule
from pants.engine.target import Target
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 @@ -346,6 +349,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

0 comments on commit f8a0b0f

Please sign in to comment.