From c78cba0b771818d0d6ce3acd56b2b404fcf1d722 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Tue, 8 Sep 2020 18:50:00 -0700 Subject: [PATCH 1/4] Fix Black and MyPy to work by default with Python 3.8+ code [ci skip-rust] [ci skip-build-wheels] --- pants.toml | 3 - .../pants/backend/python/goals/setup_py.py | 33 ++-- .../backend/python/goals/setup_py_test.py | 30 ---- .../pants/backend/python/lint/black/rules.py | 35 ++++- .../lint/black/rules_integration_test.py | 63 ++++++-- .../backend/python/typecheck/mypy/rules.py | 43 +++++- .../typecheck/mypy/rules_integration_test.py | 27 +++- .../pants/backend/python/util_rules/pex.py | 143 ++++++++++++------ .../backend/python/util_rules/pex_test.py | 68 ++++++++- .../testutil/python_interpreter_selection.py | 11 ++ 10 files changed, 324 insertions(+), 132 deletions(-) diff --git a/pants.toml b/pants.toml index edf8f5345f9a..bfb574ed2a8e 100644 --- a/pants.toml +++ b/pants.toml @@ -75,9 +75,6 @@ config = ["pyproject.toml", "examples/.isort.cfg"] [mypy] config = "build-support/mypy/mypy.ini" -# TODO(John Sirois): Remove once proper interpreter selection is performed automatically as part of -# https://github.com/pantsbuild/pants/issues/10131 -interpreter_constraints=["CPython>=3.6"] [pants-releases] release_notes = """ diff --git a/src/python/pants/backend/python/goals/setup_py.py b/src/python/pants/backend/python/goals/setup_py.py index df881f8a878e..1a3d886d73fc 100644 --- a/src/python/pants/backend/python/goals/setup_py.py +++ b/src/python/pants/backend/python/goals/setup_py.py @@ -9,7 +9,7 @@ from abc import ABC, abstractmethod from collections import abc, defaultdict from dataclasses import dataclass -from typing import Any, Dict, Iterable, List, Mapping, Set, Tuple, cast +from typing import Any, Dict, List, Mapping, Set, Tuple, cast from pants.backend.python.macros.python_artifact import PythonArtifact from pants.backend.python.subsystems.setuptools import Setuptools @@ -26,7 +26,6 @@ PexProcess, PexRequest, PexRequirements, - parse_interpreter_constraint, ) from pants.backend.python.util_rules.python_sources import ( PythonSourceFilesRequest, @@ -405,14 +404,19 @@ async def run_setup_pys( ) exported_targets = list(FrozenOrderedSet(owners)) - py2 = is_python2( - python_setup.compatibilities_or_constraints( - target_with_origin.target.get(PythonInterpreterCompatibility).value + interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields( + ( + target_with_origin.target[PythonInterpreterCompatibility] for target_with_origin in targets_with_origins - ) + if target_with_origin.target.has_field(PythonInterpreterCompatibility) + ), + python_setup, ) chroots = await MultiGet( - Get(SetupPyChroot, SetupPyChrootRequest(exported_target, py2)) + Get( + SetupPyChroot, + SetupPyChrootRequest(exported_target, py2=interpreter_constraints.includes_python2()), + ) for exported_target in exported_targets ) @@ -958,21 +962,6 @@ def has_args(call_node: ast.Call, required_arg_ids: Tuple[str, ...]) -> bool: return False -def is_python2(compatibilities_or_constraints: Iterable[str]) -> bool: - """Checks if we should assume python2 code.""" - - def iter_reqs(): - for constraint in compatibilities_or_constraints: - yield parse_interpreter_constraint(constraint) - - for req in iter_reqs(): - for python_27_ver in range(0, 18): # The last python 2.7 version was 2.7.18. - if req.specifier.contains(f"2.7.{python_27_ver}"): - # At least one constraint limits us to Python 2, so assume that. - return True - return False - - def rules(): return [ *python_sources_rules(), diff --git a/src/python/pants/backend/python/goals/setup_py_test.py b/src/python/pants/backend/python/goals/setup_py_test.py index bd7b4f0b095d..86aa2b94a0f5 100644 --- a/src/python/pants/backend/python/goals/setup_py_test.py +++ b/src/python/pants/backend/python/goals/setup_py_test.py @@ -30,7 +30,6 @@ get_owned_dependencies, get_requirements, get_sources, - is_python2, validate_args, ) from pants.backend.python.macros.python_artifact import PythonArtifact @@ -808,32 +807,3 @@ def test_declares_pkg_resources_namespace_package(python_src: str) -> None: ) def test_does_not_declare_pkg_resources_namespace_package(python_src: str) -> None: assert not declares_pkg_resources_namespace_package(python_src) - - -@pytest.mark.parametrize( - "constraints", - [ - ["CPython>=2.7,<3"], - ["CPython>=2.7,<3", "CPython>=3.6"], - ["CPython>=2.7.13"], - ["CPython>=2.7.13,<2.7.16"], - ["CPython>=2.7.13,!=2.7.16"], - ["PyPy>=2.7,<3"], - ], -) -def test_is_python2(constraints): - assert is_python2(constraints) - - -@pytest.mark.parametrize( - "constraints", - [ - ["CPython>=3.6"], - ["CPython>=3.7"], - ["CPython>=3.6", "CPython>=3.8"], - ["CPython!=2.7.*"], - ["PyPy>=3.6"], - ], -) -def test_is_not_python2(constraints): - assert not is_python2(constraints) diff --git a/src/python/pants/backend/python/lint/black/rules.py b/src/python/pants/backend/python/lint/black/rules.py index 75f4e87ee7a9..acd2460a5eba 100644 --- a/src/python/pants/backend/python/lint/black/rules.py +++ b/src/python/pants/backend/python/lint/black/rules.py @@ -8,7 +8,7 @@ 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 PythonSources +from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonSources from pants.backend.python.util_rules import pex from pants.backend.python.util_rules.pex import ( Pex, @@ -26,6 +26,7 @@ from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import FieldSet from pants.engine.unions import UnionRule +from pants.python.python_setup import PythonSetup from pants.util.logging import LogLevel from pants.util.strutil import pluralize @@ -35,6 +36,7 @@ class BlackFieldSet(FieldSet): required_fields = (PythonSources,) sources: PythonSources + interpreter_constraints: PythonInterpreterCompatibility class BlackRequest(PythonFmtRequest, LintRequest): @@ -71,14 +73,31 @@ def generate_args(*, source_files: SourceFiles, black: Black, check_only: bool) @rule(level=LogLevel.DEBUG) -async def setup_black(setup_request: SetupRequest, black: Black) -> Setup: - requirements_pex_request = Get( +async def setup_black( + setup_request: SetupRequest, black: Black, python_setup: PythonSetup +) -> Setup: + # Black requires 3.6+ but uses the typed-ast library to work with 2.7, 3.4, 3.5, 3.6, and 3.7. + # However, typed-ast does not understand 3.8, so instead we must run Black with Python 3.8 when + # relevant. We only do this if if <3.8 can't be used, as we don't want a loose requirement like + # `>=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), + python_setup, + ) + tool_interpreter_constraints = PexInterpreterConstraints( + black.interpreter_constraints + if not all_interpreter_constraints.requires_python38_or_newer() + else ("CPython>=3.8",) + ) + + black_pex_request = Get( Pex, PexRequest( output_filename="black.pex", internal_only=True, requirements=PexRequirements(black.all_requirements), - interpreter_constraints=PexInterpreterConstraints(black.interpreter_constraints), + interpreter_constraints=tool_interpreter_constraints, entry_point=black.entry_point, ), ) @@ -97,8 +116,8 @@ async def setup_black(setup_request: SetupRequest, black: Black) -> Setup: SourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets), ) - source_files, requirements_pex, config_digest = await MultiGet( - source_files_request, requirements_pex_request, config_digest_request + source_files, black_pex, config_digest = await MultiGet( + source_files_request, black_pex_request, config_digest_request ) source_files_snapshot = ( source_files.snapshot @@ -108,13 +127,13 @@ async def setup_black(setup_request: SetupRequest, black: Black) -> Setup: input_digest = await Get( Digest, - MergeDigests((source_files_snapshot.digest, requirements_pex.digest, config_digest)), + MergeDigests((source_files_snapshot.digest, black_pex.digest, config_digest)), ) process = await Get( Process, PexProcess( - requirements_pex, + black_pex, argv=generate_args( source_files=source_files, black=black, check_only=setup_request.check_only ), diff --git a/src/python/pants/backend/python/lint/black/rules_integration_test.py b/src/python/pants/backend/python/lint/black/rules_integration_test.py index 1af183a8e51c..36c2731a6828 100644 --- a/src/python/pants/backend/python/lint/black/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/black/rules_integration_test.py @@ -1,6 +1,7 @@ # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +from textwrap import dedent from typing import List, Optional, Sequence, Tuple import pytest @@ -17,6 +18,7 @@ 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.rule_runner import RuleRunner @@ -28,22 +30,32 @@ def rule_runner() -> RuleRunner: QueryRule(LintResults, (BlackRequest, OptionsBootstrapper)), QueryRule(FmtResult, (BlackRequest, OptionsBootstrapper)), QueryRule(SourceFiles, (SourceFilesRequest, OptionsBootstrapper)), - ] + ], + target_types=[PythonLibrary], ) -GOOD_SOURCE = FileContent(path="good.py", content=b'animal = "Koala"\n') -BAD_SOURCE = FileContent(path="bad.py", content=b'name= "Anakin"\n') -FIXED_BAD_SOURCE = FileContent(path="bad.py", content=b'name = "Anakin"\n') +GOOD_SOURCE = FileContent("good.py", b'animal = "Koala"\n') +BAD_SOURCE = FileContent("bad.py", b'name= "Anakin"\n') +FIXED_BAD_SOURCE = FileContent("bad.py", b'name = "Anakin"\n') # Note the single quotes, which Black does not like by default. To get Black to pass, it will # need to successfully read our config/CLI args. -NEEDS_CONFIG_SOURCE = FileContent(path="needs_config.py", content=b"animal = 'Koala'\n") +NEEDS_CONFIG_SOURCE = FileContent("needs_config.py", b"animal = 'Koala'\n") -def make_target(rule_runner: RuleRunner, source_files: List[FileContent]) -> Target: +def make_target( + rule_runner: RuleRunner, + source_files: List[FileContent], + *, + name: str = "target", + interpreter_constraints: Optional[str] = None, +) -> Target: for source_file in source_files: - rule_runner.create_file(f"{source_file.path}", source_file.content.decode()) - return PythonLibrary({}, address=Address.parse(":target")) + 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" + ) + return rule_runner.get_target(Address("", target_name=name)) def run_black( @@ -122,7 +134,10 @@ def test_mixed_sources(rule_runner: RuleRunner) -> None: def test_multiple_targets(rule_runner: RuleRunner) -> None: - targets = [make_target(rule_runner, [GOOD_SOURCE]), make_target(rule_runner, [BAD_SOURCE])] + targets = [ + make_target(rule_runner, [GOOD_SOURCE], name="good"), + make_target(rule_runner, [BAD_SOURCE], name="bad"), + ] lint_results, fmt_result = run_black(rule_runner, targets) assert len(lint_results) == 1 assert lint_results[0].exit_code == 1 @@ -164,3 +179,33 @@ def test_skip(rule_runner: RuleRunner) -> None: assert not lint_results assert fmt_result.skipped is True assert fmt_result.did_change is False + + +@skip_unless_python38_present +def test_works_with_python38(rule_runner: RuleRunner) -> None: + """Black's typed-ast dependency does not understand Python 3.8, so we must instead run Black + with Python 3.8 when relevant.""" + py38_sources = FileContent( + "py38.py", + dedent( + """\ + import datetime + + x = True + if y := x: + print("x is truthy and now assigned to y") + + + class Foo: + pass + """ + ).encode(), + ) + target = make_target(rule_runner, [py38_sources], interpreter_constraints=">=3.8") + lint_results, fmt_result = run_black(rule_runner, [target]) + assert len(lint_results) == 1 + assert lint_results[0].exit_code == 0 + assert "1 file would be left unchanged" in lint_results[0].stderr + assert "1 file left unchanged" in fmt_result.stderr + assert fmt_result.output == get_digest(rule_runner, [py38_sources]) + assert fmt_result.did_change is False diff --git a/src/python/pants/backend/python/typecheck/mypy/rules.py b/src/python/pants/backend/python/typecheck/mypy/rules.py index e60eaddb4f1c..2afed7692974 100644 --- a/src/python/pants/backend/python/typecheck/mypy/rules.py +++ b/src/python/pants/backend/python/typecheck/mypy/rules.py @@ -1,10 +1,11 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +import logging from dataclasses import dataclass from typing import Tuple -from pants.backend.python.target_types import PythonSources +from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonSources from pants.backend.python.typecheck.mypy.subsystem import MyPy from pants.backend.python.util_rules.pex import ( Pex, @@ -34,9 +35,12 @@ from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import FieldSet, TransitiveTargets from pants.engine.unions import UnionRule +from pants.python.python_setup import PythonSetup from pants.util.logging import LogLevel from pants.util.strutil import pluralize +logger = logging.getLogger(__name__) + @dataclass(frozen=True) class MyPyFieldSet(FieldSet): @@ -60,8 +64,11 @@ def generate_args(mypy: MyPy, *, file_list_path: str) -> Tuple[str, ...]: # TODO(#10131): Improve performance, e.g. by leveraging the MyPy cache. # TODO(#10131): Support plugins and type stubs. +# TODO(#10131): Support third-party requirements. @rule(desc="Typecheck using MyPy", level=LogLevel.DEBUG) -async def mypy_typecheck(request: MyPyRequest, mypy: MyPy) -> TypecheckResults: +async def mypy_typecheck( + request: MyPyRequest, mypy: MyPy, python_setup: PythonSetup +) -> TypecheckResults: if mypy.skip: return TypecheckResults([], typechecker_name="MyPy") @@ -69,6 +76,31 @@ async def mypy_typecheck(request: MyPyRequest, mypy: MyPy) -> TypecheckResults: TransitiveTargets, Addresses(fs.address for fs in request.field_sets) ) + # Interpreter constraints are tricky with MyPy: + # * MyPy requires running with Python 3.5+. If run with Python 3.5 - 3.7, MyPy can understand + # Python 2.7 and 3.4-3.7 thanks to the typed-ast library, but it can't understand 3.8+ If + # run with Python 3.8, it can understand 2.7 and 3.4-3.8. So, we need to check if the user + # has code that requires Python 3.8+, and if so, use a tighter requirement. We only do this + # if <3.8 can't be used, as we don't want a loose requirement like `>=3.6` to result in + # requiring Python 3.8, which would error if 3.8 is not installed on the machine. + # * We must resolve third-party dependencies. This should use whatever the actual code's + # constraints are, as the constraints for the tool can be different than for the + # requirements. + # * The runner Pex should use the same constraints as the tool Pex. + all_interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields( + ( + tgt[PythonInterpreterCompatibility] + for tgt in transitive_targets.closure + if tgt.has_field(PythonInterpreterCompatibility) + ), + python_setup, + ) + tool_interpreter_constraints = PexInterpreterConstraints( + mypy.interpreter_constraints + if not all_interpreter_constraints.requires_python38_or_newer() + else ("CPython>=3.8",) + ) + prepared_sources_request = Get( PythonSourceFiles, PythonSourceFilesRequest(transitive_targets.closure), @@ -79,12 +111,7 @@ async def mypy_typecheck(request: MyPyRequest, mypy: MyPy) -> TypecheckResults: output_filename="mypy.pex", internal_only=True, requirements=PexRequirements(mypy.all_requirements), - # NB: This only determines what MyPy is run with. The user can specify what version - # their code is with `--python-version`. See - # https://mypy.readthedocs.io/en/stable/config_file.html#platform-configuration. We do - # not auto-configure this for simplicity and to avoid Pants magically setting values for - # users. - interpreter_constraints=PexInterpreterConstraints(mypy.interpreter_constraints), + interpreter_constraints=tool_interpreter_constraints, entry_point=mypy.entry_point, ), ) diff --git a/src/python/pants/backend/python/typecheck/mypy/rules_integration_test.py b/src/python/pants/backend/python/typecheck/mypy/rules_integration_test.py index fdbde0b26545..d25172d9a465 100644 --- a/src/python/pants/backend/python/typecheck/mypy/rules_integration_test.py +++ b/src/python/pants/backend/python/typecheck/mypy/rules_integration_test.py @@ -18,6 +18,7 @@ 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.rule_runner import RuleRunner @@ -26,7 +27,7 @@ def rule_runner() -> RuleRunner: return RuleRunner( rules=[ *mypy_rules(), - *dependency_inference_rules.rules(), # Used for `__init__.py` inference. + *dependency_inference_rules.rules(), # Used for import inference. QueryRule(TypecheckResults, (MyPyRequest, OptionsBootstrapper)), ], target_types=[PythonLibrary], @@ -81,6 +82,7 @@ def make_target( *, package: Optional[str] = None, name: str = "target", + interpreter_constraints: Optional[str] = None, ) -> Target: if not package: package = PACKAGE @@ -94,6 +96,7 @@ def make_target( python_library( name={repr(name)}, sources={source_globs}, + compatibility={repr(interpreter_constraints)}, ) """ ), @@ -242,3 +245,25 @@ def add(x: int, y: int) -> str: assert len(result) == 1 assert result[0].exit_code == 1 assert f"{PACKAGE}/math/add.py:5" 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 + Python 3.8 when relevant.""" + rule_runner.create_file(f"{PACKAGE}/__init__.py") + py38_sources = FileContent( + f"{PACKAGE}/py38.py", + dedent( + """\ + x = 0 + if y := x: + print("x is truthy and now assigned to y") + """ + ).encode(), + ) + target = make_target(rule_runner, [py38_sources], interpreter_constraints=">=3.8") + result = run_mypy(rule_runner, [target]) + assert len(result) == 1 + assert result[0].exit_code == 0 + assert "Success: no issues found" in result[0].stdout.strip() diff --git a/src/python/pants/backend/python/util_rules/pex.py b/src/python/pants/backend/python/util_rules/pex.py index d852011e4cc8..7975168b2d4c 100644 --- a/src/python/pants/backend/python/util_rules/pex.py +++ b/src/python/pants/backend/python/util_rules/pex.py @@ -2,6 +2,7 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). import dataclasses +import functools import itertools import logging from dataclasses import dataclass @@ -10,12 +11,12 @@ Iterable, List, Mapping, - NamedTuple, Optional, Sequence, Set, Tuple, TypeVar, + Union, ) from pkg_resources import Requirement @@ -46,6 +47,7 @@ from pants.util.frozendict import FrozenDict from pants.util.logging import LogLevel from pants.util.meta import frozen_after_init +from pants.util.ordered_set import FrozenOrderedSet from pants.util.strutil import pluralize @@ -63,33 +65,6 @@ def create_from_requirement_fields( return PexRequirements({*field_requirements, *additional_requirements}) -def parse_interpreter_constraint(constraint: str) -> Requirement: - """Parse an interpreter constraint, e.g., CPython>=2.7,<3. - - We allow shorthand such as `>=3.7`, which gets expanded to `CPython>=3.7`. See Pex's - interpreter.py's `parse_requirement()`. - """ - try: - parsed_requirement = Requirement.parse(constraint) - except ValueError: - parsed_requirement = Requirement.parse(f"CPython{constraint}") - return parsed_requirement - - -Spec = Tuple[str, str] # e.g. (">=", "3.6") - - -class ParsedConstraint(NamedTuple): - interpreter: str - specs: FrozenSet[Spec] - - def __str__(self) -> str: - specs = ",".join( - f"{op}{version}" for op, version in sorted(self.specs, key=lambda spec: spec[1]) - ) - return f"{self.interpreter}{specs}" - - # This protocol allows us to work with any arbitrary FieldSet. See # https://mypy.readthedocs.io/en/stable/protocols.html. class FieldSetWithCompatibility(Protocol): @@ -105,11 +80,29 @@ def compatibility(self) -> PythonInterpreterCompatibility: _FS = TypeVar("_FS", bound=FieldSetWithCompatibility) -class PexInterpreterConstraints(DeduplicatedCollection[str]): - sort_input = True +# Normally we would subclass `DeduplicatedCollection`, but we want a custom constructor. +class PexInterpreterConstraints(FrozenOrderedSet[Requirement]): + def __init__(self, constraints: Iterable[Union[str, Requirement]] = ()) -> None: + super().__init__( + v if isinstance(v, Requirement) else self.parse_constraint(v) + for v in sorted(constraints) + ) @staticmethod - def merge_constraint_sets(constraint_sets: Iterable[Iterable[str]]) -> List[str]: + def parse_constraint(constraint: str) -> Requirement: + """Parse an interpreter constraint, e.g., CPython>=2.7,<3. + + We allow shorthand such as `>=3.7`, which gets expanded to `CPython>=3.7`. See Pex's + interpreter.py's `parse_requirement()`. + """ + try: + parsed_requirement = Requirement.parse(constraint) + except ValueError: + parsed_requirement = Requirement.parse(f"CPython{constraint}") + return parsed_requirement + + @classmethod + def merge_constraint_sets(cls, constraint_sets: Iterable[Iterable[str]]) -> List[Requirement]: """Given a collection of constraints sets, merge by ORing within each individual constraint set and ANDing across each distinct constraint set. @@ -120,29 +113,26 @@ def merge_constraint_sets(constraint_sets: Iterable[Iterable[str]]) -> List[str] # identical top-level parsed constraint sets. if not constraint_sets: return [] - parsed_constraint_sets: Set[FrozenSet[ParsedConstraint]] = set() + parsed_constraint_sets: Set[FrozenSet[Requirement]] = set() for constraint_set in constraint_sets: # Each element (a ParsedConstraint) will get ORed. - parsed_constraint_set: Set[ParsedConstraint] = set() - for constraint in constraint_set: - parsed_requirement = parse_interpreter_constraint(constraint) - interpreter = parsed_requirement.project_name - specs = frozenset(parsed_requirement.specs) - parsed_constraint_set.add(ParsedConstraint(interpreter, specs)) - parsed_constraint_sets.add(frozenset(parsed_constraint_set)) - - def and_constraints(parsed_constraints: Sequence[ParsedConstraint]) -> ParsedConstraint: - merged_specs: Set[Spec] = set() - expected_interpreter = parsed_constraints[0][0] + parsed_constraint_set = frozenset( + cls.parse_constraint(constraint) for constraint in constraint_set + ) + parsed_constraint_sets.add(parsed_constraint_set) + + def and_constraints(parsed_constraints: Sequence[Requirement]) -> Requirement: + merged_specs: Set[Tuple[str, str]] = set() + expected_interpreter = parsed_constraints[0].project_name for parsed_constraint in parsed_constraints: - if parsed_constraint.interpreter != expected_interpreter: + if parsed_constraint.project_name != expected_interpreter: attempted_interpreters = { interp: sorted( str(parsed_constraint) for parsed_constraint in parsed_constraints ) for interp, parsed_constraints in itertools.groupby( parsed_constraints, - key=lambda pc: pc.interpreter, + key=lambda pc: pc.project_name, ) } raise ValueError( @@ -151,13 +141,23 @@ def and_constraints(parsed_constraints: Sequence[ParsedConstraint]) -> ParsedCon f"{attempted_interpreters}." ) merged_specs.update(parsed_constraint.specs) - return ParsedConstraint(expected_interpreter, frozenset(merged_specs)) + + formatted_specs = ",".join(f"{op}{version}" for op, version in merged_specs) + return Requirement.parse(f"{expected_interpreter}{formatted_specs}") + + def cmp_constraints(req1: Requirement, req2: Requirement) -> int: + if req1.project_name != req2.project_name: + return -1 if req1.project_name < req2.project_name else 1 + if req1.specs == req2.specs: + return 0 + return -1 if req1.specs < req2.specs else 1 return sorted( { - str(and_constraints(constraints_product)) + and_constraints(constraints_product) for constraints_product in itertools.product(*parsed_constraint_sets) - } + }, + key=functools.cmp_to_key(cmp_constraints), ) @classmethod @@ -187,9 +187,54 @@ def group_field_sets_by_constraints( def generate_pex_arg_list(self) -> List[str]: args = [] for constraint in self: - args.extend(["--interpreter-constraint", constraint]) + args.extend(["--interpreter-constraint", str(constraint)]) return args + def includes_python2(self) -> bool: + """Checks if any of the constraints include Python 2. + + This will return True even if the code works with Python 3 too, so long as at least one of + the constraints works with Python 2. + """ + py27_patch_versions = list( + reversed(range(0, 18)) + ) # The last python 2.7 version was 2.7.18. + for req in self: + if any( + req.specifier.contains(f"2.7.{p}") for p in py27_patch_versions # type: ignore[attr-defined] + ): + return True + return False + + def requires_python38_or_newer(self) -> bool: + """Checks if the constraints are all for Python 3.8+. + + This will return False if Python 3.8 is allowed, but prior versions like 3.7 are also + allowed. + """ + # Assume any 3.x release has no more than 13 releases. The max is currently 10. + patch_versions = list(reversed(range(0, 13))) + # We only need to look at Python 3.7. If using something like `>=3.5`, Py37 will be + # included. `==3.6.*,!=3.7.*,==3.8.*` is extremely unlikely, and even that will work + # correctly as it's an invalid constraint so setuptools returns False always. + # `['==2.7.*', '==3.8.*']` will fail because not every single constraint is exclusively 3.8. + py37_versions = [f"3.7.{v}" for v in patch_versions] + py38_versions = [f"3.8.{v}" for v in patch_versions] + py39_versions = [f"3.9.{v}" for v in patch_versions] + for req in self: + if any( + req.specifier.contains(py37_version) for py37_version in py37_versions # type: ignore[attr-defined] + ): + return False + works_with_py38_or_py39 = any( + req.specifier.contains(py38_version) for py38_version in py38_versions # type: ignore[attr-defined] + ) or any( + req.specifier.contains(py39_version) for py39_version in py39_versions # type: ignore[attr-defined] + ) + if not works_with_py38_or_py39: + return False + return True + class PexPlatforms(DeduplicatedCollection[str]): sort_input = True diff --git a/src/python/pants/backend/python/util_rules/pex_test.py b/src/python/pants/backend/python/util_rules/pex_test.py index 0af8b16cda29..12f3809dcf55 100644 --- a/src/python/pants/backend/python/util_rules/pex_test.py +++ b/src/python/pants/backend/python/util_rules/pex_test.py @@ -34,7 +34,11 @@ def test_merge_interpreter_constraints() -> None: def assert_merged(*, inp: List[List[str]], expected: List[str]) -> None: - assert PexInterpreterConstraints.merge_constraint_sets(inp) == expected + result = sorted(str(req) for req in PexInterpreterConstraints.merge_constraint_sets(inp)) + # Requirement.parse() sorts specs differently than we'd like, so we convert each str to a + # Requirement. + normalized_expected = sorted(str(Requirement.parse(v)) for v in expected) + assert result == normalized_expected # Multiple constraint sets get merged so that they are ANDed. # A & B => A & B @@ -126,6 +130,66 @@ def assert_merged(*, inp: List[List[str]], expected: List[str]) -> None: assert_merged(inp=[], expected=[]) +@pytest.mark.parametrize( + "constraints", + [ + ["CPython>=2.7,<3"], + ["CPython>=2.7,<3", "CPython>=3.6"], + ["CPython>=2.7.13"], + ["CPython>=2.7.13,<2.7.16"], + ["CPython>=2.7.13,!=2.7.16"], + ["PyPy>=2.7,<3"], + ], +) +def test_interpreter_constraints_includes_python2(constraints) -> None: + assert PexInterpreterConstraints(constraints).includes_python2() is True + + +@pytest.mark.parametrize( + "constraints", + [ + ["CPython>=3.6"], + ["CPython>=3.7"], + ["CPython>=3.6", "CPython>=3.8"], + ["CPython!=2.7.*"], + ["PyPy>=3.6"], + ], +) +def test_interpreter_constraints_do_not_include_python2(constraints): + assert PexInterpreterConstraints(constraints).includes_python2() is False + + +@pytest.mark.parametrize( + "constraints", + [ + ["CPython==3.8.*"], + ["CPython==3.8.1"], + ["CPython==3.9.1"], + ["CPython>=3.8"], + ["CPython>=3.9"], + ["CPython==3.8.*", "CPython==3.9.*"], + ["PyPy>=3.8"], + ], +) +def test_interpreter_constraints_require_python38(constraints) -> None: + assert PexInterpreterConstraints(constraints).requires_python38_or_newer() is True + + +@pytest.mark.parametrize( + "constraints", + [ + ["CPython==3.7.*"], + ["CPython==3.7.3"], + ["CPython>=3.6"], + ["CPython==3.7.*", "CPython==3.8.*"], + ["CPython==3.5.3", "CPython==3.8.3"], + ["PyPy>=3.6"], + ], +) +def test_interpreter_constraints_do_not_require_python38(constraints): + assert PexInterpreterConstraints(constraints).requires_python38_or_newer() is False + + @dataclass(frozen=True) class MockFieldSet(FieldSet): compatibility: PythonInterpreterCompatibility @@ -336,7 +400,7 @@ def test_entry_point(rule_runner: RuleRunner) -> None: def test_interpreter_constraints(rule_runner: RuleRunner) -> None: constraints = PexInterpreterConstraints(["CPython>=2.7,<3", "CPython>=3.6"]) pex_info = create_pex_and_get_pex_info(rule_runner, interpreter_constraints=constraints) - assert set(pex_info["interpreter_constraints"]) == set(constraints) + assert set(pex_info["interpreter_constraints"]) == {str(c) for c in constraints} def test_additional_args(rule_runner: RuleRunner) -> None: diff --git a/src/python/pants/testutil/python_interpreter_selection.py b/src/python/pants/testutil/python_interpreter_selection.py index b655a9273445..f841641fbe89 100644 --- a/src/python/pants/testutil/python_interpreter_selection.py +++ b/src/python/pants/testutil/python_interpreter_selection.py @@ -11,6 +11,7 @@ PY_27 = "2.7" PY_36 = "3.6" PY_37 = "3.7" +PY_38 = "3.8" def has_python_version(version): @@ -73,6 +74,16 @@ def skip_unless_python36_present(func): return skip_unless_all_pythons_present(PY_36)(func) +def skip_unless_python37_present(func): + """A test skip decorator that only runs a test method if python3.7 is present.""" + return skip_unless_all_pythons_present(PY_37)(func) + + +def skip_unless_python38_present(func): + """A test skip decorator that only runs a test method if python3.8 is present.""" + return skip_unless_all_pythons_present(PY_38)(func) + + def skip_unless_python27_and_python3_present(func): """A test skip decorator that only runs a test method if python2.7 and python3 are present.""" return skip_unless_all_pythons_present(PY_27, PY_3)(func) From b80c3bbc022fea8d261f74e05d760ddb168c902f Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Tue, 8 Sep 2020 20:17:11 -0700 Subject: [PATCH 2/4] Fix MyPy using the wrong AST for Py36-only and Py37-only codebases Also fix both Black and MyPy to respect the --interpreter-constraints option if it was configured. # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../pants/backend/python/lint/black/rules.py | 9 ++- .../backend/python/typecheck/mypy/rules.py | 32 ++++++--- .../python/typecheck/mypy/subsystem.py | 1 + .../pants/backend/python/util_rules/pex.py | 66 +++++++++++------ .../backend/python/util_rules/pex_test.py | 72 ++++++++++++++++++- 5 files changed, 146 insertions(+), 34 deletions(-) diff --git a/src/python/pants/backend/python/lint/black/rules.py b/src/python/pants/backend/python/lint/black/rules.py index acd2460a5eba..607c4ddc5304 100644 --- a/src/python/pants/backend/python/lint/black/rules.py +++ b/src/python/pants/backend/python/lint/black/rules.py @@ -86,9 +86,12 @@ async def setup_black( python_setup, ) tool_interpreter_constraints = PexInterpreterConstraints( - black.interpreter_constraints - if not all_interpreter_constraints.requires_python38_or_newer() - else ("CPython>=3.8",) + ("CPython>=3.8",) + if ( + all_interpreter_constraints.requires_python38_or_newer() + and black.options.is_default("interpreter_constraints") + ) + else black.interpreter_constraints ) black_pex_request = Get( diff --git a/src/python/pants/backend/python/typecheck/mypy/rules.py b/src/python/pants/backend/python/typecheck/mypy/rules.py index 2afed7692974..edba059eeef0 100644 --- a/src/python/pants/backend/python/typecheck/mypy/rules.py +++ b/src/python/pants/backend/python/typecheck/mypy/rules.py @@ -80,9 +80,18 @@ async def mypy_typecheck( # * MyPy requires running with Python 3.5+. If run with Python 3.5 - 3.7, MyPy can understand # Python 2.7 and 3.4-3.7 thanks to the typed-ast library, but it can't understand 3.8+ If # run with Python 3.8, it can understand 2.7 and 3.4-3.8. So, we need to check if the user - # has code that requires Python 3.8+, and if so, use a tighter requirement. We only do this - # if <3.8 can't be used, as we don't want a loose requirement like `>=3.6` to result in - # requiring Python 3.8, which would error if 3.8 is not installed on the machine. + # has code that requires Python 3.8+, and if so, use a tighter requirement. + # + # On top of this, MyPy parses the AST using the value from `python_version`. If this is not + # configured, it defaults to the interpreter being used. This means that running the + # interpreter with Py35 would choke on f-strings in Python 3.6, unless the user set + # `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 are know that there are no Py35 or Py27 + # constraints, as we would assume that their code is compatible with those ASTs, and we + # want looser constraints to make it more flexible to install MyPy. # * We must resolve third-party dependencies. This should use whatever the actual code's # constraints are, as the constraints for the tool can be different than for the # requirements. @@ -95,11 +104,16 @@ async def mypy_typecheck( ), python_setup, ) - tool_interpreter_constraints = PexInterpreterConstraints( - mypy.interpreter_constraints - if not all_interpreter_constraints.requires_python38_or_newer() - else ("CPython>=3.8",) - ) + 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 prepared_sources_request = Get( PythonSourceFiles, @@ -111,7 +125,7 @@ async def mypy_typecheck( output_filename="mypy.pex", internal_only=True, requirements=PexRequirements(mypy.all_requirements), - interpreter_constraints=tool_interpreter_constraints, + interpreter_constraints=PexInterpreterConstraints(tool_interpreter_constraints), entry_point=mypy.entry_point, ), ) diff --git a/src/python/pants/backend/python/typecheck/mypy/subsystem.py b/src/python/pants/backend/python/typecheck/mypy/subsystem.py index 5d1cb44b5dee..53e9ed2564ea 100644 --- a/src/python/pants/backend/python/typecheck/mypy/subsystem.py +++ b/src/python/pants/backend/python/typecheck/mypy/subsystem.py @@ -13,6 +13,7 @@ class MyPy(PythonToolBase): options_scope = "mypy" default_version = "mypy==0.782" default_entry_point = "mypy" + # See `mypy/rules.py`. We only use these default constraints in some situations. default_interpreter_constraints = ["CPython>=3.5"] @classmethod diff --git a/src/python/pants/backend/python/util_rules/pex.py b/src/python/pants/backend/python/util_rules/pex.py index 7975168b2d4c..4fbc22aaf513 100644 --- a/src/python/pants/backend/python/util_rules/pex.py +++ b/src/python/pants/backend/python/util_rules/pex.py @@ -206,35 +206,61 @@ def includes_python2(self) -> bool: return True return False - def requires_python38_or_newer(self) -> bool: - """Checks if the constraints are all for Python 3.8+. - - This will return False if Python 3.8 is allowed, but prior versions like 3.7 are also - allowed. - """ + def _requires_python3_version_or_newer( + self, *, allowed_versions: Iterable[str], prior_version: str + ) -> bool: # Assume any 3.x release has no more than 13 releases. The max is currently 10. patch_versions = list(reversed(range(0, 13))) - # We only need to look at Python 3.7. If using something like `>=3.5`, Py37 will be - # included. `==3.6.*,!=3.7.*,==3.8.*` is extremely unlikely, and even that will work - # correctly as it's an invalid constraint so setuptools returns False always. - # `['==2.7.*', '==3.8.*']` will fail because not every single constraint is exclusively 3.8. - py37_versions = [f"3.7.{v}" for v in patch_versions] - py38_versions = [f"3.8.{v}" for v in patch_versions] - py39_versions = [f"3.9.{v}" for v in patch_versions] + # We only need to look at the prior Python release. For example, consider Python 3.8+ + # looking at 3.7. If using something like `>=3.5`, Py37 will be included. + # `==3.6.*,!=3.7.*,==3.8.*` is extremely unlikely, and even that will work correctly as + # it's an invalid constraint so setuptools returns False always. `['==2.7.*', '==3.8.*']` + # will fail because not every single constraint is exclusively 3.8. + prior_versions = [f"{prior_version}.{p}" for p in patch_versions] + allowed_versions = [ + f"{major_minor}.{p}" for major_minor in allowed_versions for p in patch_versions + ] for req in self: if any( - req.specifier.contains(py37_version) for py37_version in py37_versions # type: ignore[attr-defined] + req.specifier.contains(prior) for prior in prior_versions # type: ignore[attr-defined] ): return False - works_with_py38_or_py39 = any( - req.specifier.contains(py38_version) for py38_version in py38_versions # type: ignore[attr-defined] - ) or any( - req.specifier.contains(py39_version) for py39_version in py39_versions # type: ignore[attr-defined] - ) - if not works_with_py38_or_py39: + if not any( + req.specifier.contains(allowed) for allowed in allowed_versions # type: ignore[attr-defined] + ): 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+. + + This will return False if Python 3.8 is allowed, but prior versions like 3.7 are also + allowed. + """ + return self._requires_python3_version_or_newer( + allowed_versions=["3.8", "3.9"], prior_version="3.7" + ) + class PexPlatforms(DeduplicatedCollection[str]): sort_input = True diff --git a/src/python/pants/backend/python/util_rules/pex_test.py b/src/python/pants/backend/python/util_rules/pex_test.py index 12f3809dcf55..c56084e2c528 100644 --- a/src/python/pants/backend/python/util_rules/pex_test.py +++ b/src/python/pants/backend/python/util_rules/pex_test.py @@ -159,6 +159,72 @@ def test_interpreter_constraints_do_not_include_python2(constraints): assert PexInterpreterConstraints(constraints).includes_python2() is False +@pytest.mark.parametrize( + "constraints", + [ + ["CPython==3.6.*"], + ["CPython==3.6.1"], + ["CPython==3.7.1"], + ["CPython==3.8.2"], + ["CPython==3.9.1"], + ["CPython>=3.6"], + ["CPython>=3.7"], + ["CPython==3.6.*", "CPython==3.7.*"], + ["PyPy>=3.6"], + ], +) +def test_interpreter_constraints_require_python36(constraints) -> None: + assert PexInterpreterConstraints(constraints).requires_python36_or_newer() is True + + +@pytest.mark.parametrize( + "constraints", + [ + ["CPython==3.5.*"], + ["CPython==3.5.3"], + ["CPython>=3.5"], + ["CPython==3.5.*", "CPython==3.6.*"], + ["CPython==3.5.3", "CPython==3.6.3"], + ["PyPy>=3.5"], + ], +) +def test_interpreter_constraints_do_not_require_python36(constraints): + assert PexInterpreterConstraints(constraints).requires_python36_or_newer() is False + + +@pytest.mark.parametrize( + "constraints", + [ + ["CPython==3.7.*"], + ["CPython==3.7.1"], + ["CPython==3.8.1"], + ["CPython==3.9.1"], + ["CPython>=3.7"], + ["CPython>=3.8"], + ["CPython==3.7.*", "CPython==3.8.*"], + ["PyPy>=3.7"], + ], +) +def test_interpreter_constraints_require_python37(constraints) -> None: + assert PexInterpreterConstraints(constraints).requires_python37_or_newer() is True + + +@pytest.mark.parametrize( + "constraints", + [ + ["CPython==3.5.*"], + ["CPython==3.6.*"], + ["CPython==3.6.3"], + ["CPython>=3.6"], + ["CPython==3.6.*", "CPython==3.7.*"], + ["CPython==3.6.3", "CPython==3.7.3"], + ["PyPy>=3.6"], + ], +) +def test_interpreter_constraints_do_not_require_python37(constraints): + assert PexInterpreterConstraints(constraints).requires_python37_or_newer() is False + + @pytest.mark.parametrize( "constraints", [ @@ -178,12 +244,14 @@ def test_interpreter_constraints_require_python38(constraints) -> None: @pytest.mark.parametrize( "constraints", [ + ["CPython==3.5.*"], + ["CPython==3.6.*"], ["CPython==3.7.*"], ["CPython==3.7.3"], - ["CPython>=3.6"], + ["CPython>=3.7"], ["CPython==3.7.*", "CPython==3.8.*"], ["CPython==3.5.3", "CPython==3.8.3"], - ["PyPy>=3.6"], + ["PyPy>=3.7"], ], ) def test_interpreter_constraints_do_not_require_python38(constraints): From 682e7262d192966fcf549048adacbde2eb608fa5 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Tue, 8 Sep 2020 20:27:15 -0700 Subject: [PATCH 3/4] Fix unused logger and some comments # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../pants/backend/python/typecheck/mypy/rules.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/python/pants/backend/python/typecheck/mypy/rules.py b/src/python/pants/backend/python/typecheck/mypy/rules.py index edba059eeef0..ffcb01c6b0da 100644 --- a/src/python/pants/backend/python/typecheck/mypy/rules.py +++ b/src/python/pants/backend/python/typecheck/mypy/rules.py @@ -1,7 +1,6 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -import logging from dataclasses import dataclass from typing import Tuple @@ -39,8 +38,6 @@ from pants.util.logging import LogLevel from pants.util.strutil import pluralize -logger = logging.getLogger(__name__) - @dataclass(frozen=True) class MyPyFieldSet(FieldSet): @@ -77,7 +74,7 @@ async def mypy_typecheck( ) # Interpreter constraints are tricky with MyPy: - # * MyPy requires running with Python 3.5+. If run with Python 3.5 - 3.7, MyPy can understand + # * MyPy requires running with Python 3.5+. If run with Python 3.5-3.7, MyPy can understand # Python 2.7 and 3.4-3.7 thanks to the typed-ast library, but it can't understand 3.8+ If # run with Python 3.8, it can understand 2.7 and 3.4-3.8. So, we need to check if the user # has code that requires Python 3.8+, and if so, use a tighter requirement. @@ -90,11 +87,11 @@ async def mypy_typecheck( # # We only apply these optimizations if the user did not configure # `--mypy-interpreter-constraints`, and if we are know that there are no Py35 or Py27 - # constraints, as we would assume that their code is compatible with those ASTs, and we - # want looser constraints to make it more flexible to install MyPy. + # 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, as the constraints for the tool can be different than for the - # requirements. + # constraints are. The constraints for the tool can be different than for the requirements. # * The runner Pex should use the same constraints as the tool Pex. all_interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields( ( From 2c24ecb88887313c640098614e25050bbc260334 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Tue, 8 Sep 2020 21:55:18 -0700 Subject: [PATCH 4/4] Fix linter partition descriptions # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- src/python/pants/backend/python/lint/bandit/rules.py | 4 +++- src/python/pants/backend/python/lint/flake8/rules.py | 4 +++- src/python/pants/backend/python/lint/pylint/rules.py | 2 +- .../backend/python/lint/pylint/rules_integration_test.py | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/python/pants/backend/python/lint/bandit/rules.py b/src/python/pants/backend/python/lint/bandit/rules.py index 822bdc0dc6be..b9ee405be932 100644 --- a/src/python/pants/backend/python/lint/bandit/rules.py +++ b/src/python/pants/backend/python/lint/bandit/rules.py @@ -129,7 +129,9 @@ async def bandit_lint_partition( report = LintReport(report_file_name, report_digest) return LintResult.from_fallible_process_result( - result, partition_description=str(sorted(partition.interpreter_constraints)), report=report + result, + partition_description=str(sorted(str(c) for c in partition.interpreter_constraints)), + report=report, ) diff --git a/src/python/pants/backend/python/lint/flake8/rules.py b/src/python/pants/backend/python/lint/flake8/rules.py index 509d141520b1..c239cce0562d 100644 --- a/src/python/pants/backend/python/lint/flake8/rules.py +++ b/src/python/pants/backend/python/lint/flake8/rules.py @@ -130,7 +130,9 @@ async def flake8_lint_partition( report = LintReport(report_file_name, report_digest) return LintResult.from_fallible_process_result( - result, partition_description=str(sorted(partition.interpreter_constraints)), report=report + result, + partition_description=str(sorted(str(c) for c in partition.interpreter_constraints)), + report=report, ) diff --git a/src/python/pants/backend/python/lint/pylint/rules.py b/src/python/pants/backend/python/lint/pylint/rules.py index f3804dbe3bb0..5b3a51c24157 100644 --- a/src/python/pants/backend/python/lint/pylint/rules.py +++ b/src/python/pants/backend/python/lint/pylint/rules.py @@ -232,7 +232,7 @@ async def pylint_lint_partition(partition: PylintPartition, pylint: Pylint) -> L ), ) return LintResult.from_fallible_process_result( - result, partition_description=str(sorted(partition.interpreter_constraints)) + result, partition_description=str(sorted(str(c) for c in partition.interpreter_constraints)) ) diff --git a/src/python/pants/backend/python/lint/pylint/rules_integration_test.py b/src/python/pants/backend/python/lint/pylint/rules_integration_test.py index 901ec7ca18c9..f6aefb7e69b4 100644 --- a/src/python/pants/backend/python/lint/pylint/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/pylint/rules_integration_test.py @@ -180,7 +180,7 @@ def test_uses_correct_python_version(rule_runner: RuleRunner) -> None: assert "invalid syntax (, line 2) (syntax-error)" in batched_py2_result.stdout assert batched_py3_result.exit_code == 0 - assert batched_py3_result.partition_description == "['CPython>=3.6,<3.8']" + assert batched_py3_result.partition_description == "['CPython<3.8,>=3.6']" assert "Your code has been rated at 10.00/10" in batched_py3_result.stdout.strip()