From a5584f2aa86a6aca67acc9b307e6f95faee07027 Mon Sep 17 00:00:00 2001 From: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Date: Wed, 14 Jul 2021 17:13:40 -0700 Subject: [PATCH] [Internal] Refactor how `PythonToolBase` exposes requirements and interpreter constraints (#12356) In a followup, the new `PythonToolBase.pex_requirements` will have the logic for how to load a lockfile: https://github.com/pantsbuild/pants/blob/bc46347cacea87618eeb3c65e1a7d0e5b01a0de7/src/python/pants/backend/python/lint/docformatter/rules.py#L62-L77 This will allow us to DRY setup of lockfiles. [ci skip-rust] [ci skip-build-wheels] --- .../pants/backend/awslambda/python/rules.py | 6 +-- .../backend/codegen/protobuf/python/rules.py | 15 ++------ .../backend/experimental/python/lockfile.py | 12 +++--- .../pants/backend/python/goals/coverage_py.py | 7 ++-- src/python/pants/backend/python/goals/repl.py | 4 +- .../pants/backend/python/goals/setup_py.py | 2 +- .../pants/backend/python/lint/bandit/rules.py | 4 +- .../pants/backend/python/lint/black/rules.py | 6 +-- .../lint/black/rules_integration_test.py | 10 +---- .../backend/python/lint/black/subsystem.py | 1 - .../backend/python/lint/docformatter/rules.py | 8 ++-- .../pants/backend/python/lint/flake8/rules.py | 4 +- .../pants/backend/python/lint/isort/rules.py | 13 ++----- .../python/subsystems/python_tool_base.py | 37 +++++++++++++++---- .../backend/python/typecheck/mypy/rules.py | 9 +++-- 15 files changed, 66 insertions(+), 72 deletions(-) diff --git a/src/python/pants/backend/awslambda/python/rules.py b/src/python/pants/backend/awslambda/python/rules.py index 0acd5ee8921..cef75ab1538 100644 --- a/src/python/pants/backend/awslambda/python/rules.py +++ b/src/python/pants/backend/awslambda/python/rules.py @@ -12,12 +12,10 @@ ResolvePythonAwsHandlerRequest, ) from pants.backend.python.util_rules import pex_from_targets -from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints from pants.backend.python.util_rules.pex import ( Pex, PexPlatforms, PexRequest, - PexRequirements, VenvPex, VenvPexProcess, ) @@ -91,8 +89,8 @@ async def package_python_awslambda( lambdex_request = PexRequest( output_filename="lambdex.pex", internal_only=True, - requirements=PexRequirements(lambdex.all_requirements), - interpreter_constraints=InterpreterConstraints(lambdex.interpreter_constraints), + requirements=lambdex.pex_requirements, + interpreter_constraints=lambdex.interpreter_constraints, main=lambdex.main, ) diff --git a/src/python/pants/backend/codegen/protobuf/python/rules.py b/src/python/pants/backend/codegen/protobuf/python/rules.py index 96287681492..5aa1381b860 100644 --- a/src/python/pants/backend/codegen/protobuf/python/rules.py +++ b/src/python/pants/backend/codegen/protobuf/python/rules.py @@ -13,14 +13,7 @@ from pants.backend.codegen.protobuf.target_types import ProtobufGrpcToggle, ProtobufSources from pants.backend.python.target_types import PythonSources from pants.backend.python.util_rules import pex -from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints -from pants.backend.python.util_rules.pex import ( - PexRequest, - PexRequirements, - PexResolveInfo, - VenvPex, - VenvPexRequest, -) +from pants.backend.python.util_rules.pex import PexRequest, PexResolveInfo, VenvPex, VenvPexRequest from pants.backend.python.util_rules.pex_environment import SandboxPexEnvironment from pants.core.util_rules.external_tool import DownloadedExternalTool, ExternalToolRequest from pants.core.util_rules.source_files import SourceFilesRequest @@ -108,10 +101,8 @@ async def generate_python_from_protobuf( mypy_request = PexRequest( output_filename="mypy_protobuf.pex", internal_only=True, - requirements=PexRequirements([python_protobuf_mypy_plugin.requirement]), - interpreter_constraints=InterpreterConstraints( - python_protobuf_mypy_plugin.interpreter_constraints - ), + requirements=python_protobuf_mypy_plugin.pex_requirements, + interpreter_constraints=python_protobuf_mypy_plugin.interpreter_constraints, ) if python_protobuf_subsystem.mypy_plugin: diff --git a/src/python/pants/backend/experimental/python/lockfile.py b/src/python/pants/backend/experimental/python/lockfile.py index 1cbf7fb27d8..fecdee49454 100644 --- a/src/python/pants/backend/experimental/python/lockfile.py +++ b/src/python/pants/backend/experimental/python/lockfile.py @@ -124,7 +124,7 @@ async def generate_lockfile( PexRequest( output_filename="pip_compile.pex", internal_only=True, - requirements=PexRequirements(pip_tools_subsystem.all_requirements), + requirements=pip_tools_subsystem.pex_requirements, interpreter_constraints=interpreter_constraints, main=pip_tools_subsystem.main, description=( @@ -186,7 +186,7 @@ class PythonToolLockfileRequest: tool_name: str lockfile_path: str requirements: tuple[str, ...] - interpreter_constraints: tuple[str, ...] + interpreter_constraints: InterpreterConstraints # TODO(#12314): Unify this goal with `lock` once we figure out how to unify the semantics, @@ -215,17 +215,17 @@ async def generate_tool_lockfile( ] ), ) - interpreter_constraints = InterpreterConstraints(request.interpreter_constraints) pip_compile_get = Get( VenvPex, PexRequest( output_filename="pip_compile.pex", internal_only=True, - requirements=PexRequirements(pip_tools_subsystem.all_requirements), - interpreter_constraints=interpreter_constraints, + requirements=pip_tools_subsystem.pex_requirements, + interpreter_constraints=request.interpreter_constraints, main=pip_tools_subsystem.main, description=( - f"Building pip_compile.pex with interpreter constraints: {interpreter_constraints}" + "Building pip_compile.pex with interpreter constraints: " + f"{request.interpreter_constraints}" ), ), ) diff --git a/src/python/pants/backend/python/goals/coverage_py.py b/src/python/pants/backend/python/goals/coverage_py.py index 633635c8ea6..e3c63c5f2b9 100644 --- a/src/python/pants/backend/python/goals/coverage_py.py +++ b/src/python/pants/backend/python/goals/coverage_py.py @@ -14,8 +14,7 @@ from pants.backend.python.subsystems.python_tool_base import PythonToolBase from pants.backend.python.target_types import ConsoleScript -from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints -from pants.backend.python.util_rules.pex import PexRequest, PexRequirements, VenvPex, VenvPexProcess +from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess from pants.backend.python.util_rules.python_sources import ( PythonSourceFiles, PythonSourceFilesRequest, @@ -317,8 +316,8 @@ async def setup_coverage(coverage: CoverageSubsystem) -> CoverageSetup: PexRequest( output_filename="coverage.pex", internal_only=True, - requirements=PexRequirements(coverage.all_requirements), - interpreter_constraints=InterpreterConstraints(coverage.interpreter_constraints), + requirements=coverage.pex_requirements, + interpreter_constraints=coverage.interpreter_constraints, main=coverage.main, ), ) diff --git a/src/python/pants/backend/python/goals/repl.py b/src/python/pants/backend/python/goals/repl.py index d3fe859ba7c..3ab34d67007 100644 --- a/src/python/pants/backend/python/goals/repl.py +++ b/src/python/pants/backend/python/goals/repl.py @@ -2,7 +2,7 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from pants.backend.python.subsystems.ipython import IPython -from pants.backend.python.util_rules.pex import Pex, PexRequest, PexRequirements +from pants.backend.python.util_rules.pex import Pex, PexRequest from pants.backend.python.util_rules.pex_environment import WorkspacePexEnvironment from pants.backend.python.util_rules.pex_from_targets import PexFromTargetsRequest from pants.backend.python.util_rules.python_sources import ( @@ -81,7 +81,7 @@ async def create_ipython_repl_request( PexRequest( output_filename="ipython.pex", main=ipython.main, - requirements=PexRequirements(ipython.all_requirements), + requirements=ipython.pex_requirements, interpreter_constraints=requirements_pex_request.interpreter_constraints, internal_only=True, ), diff --git a/src/python/pants/backend/python/goals/setup_py.py b/src/python/pants/backend/python/goals/setup_py.py index 78662fbfd09..bc593fb7944 100644 --- a/src/python/pants/backend/python/goals/setup_py.py +++ b/src/python/pants/backend/python/goals/setup_py.py @@ -429,7 +429,7 @@ async def run_setup_py(req: RunSetupPyRequest, setuptools: Setuptools) -> RunSet PexRequest( output_filename="setuptools.pex", internal_only=True, - requirements=PexRequirements(setuptools.all_requirements), + requirements=setuptools.pex_requirements, interpreter_constraints=req.interpreter_constraints, ), ) diff --git a/src/python/pants/backend/python/lint/bandit/rules.py b/src/python/pants/backend/python/lint/bandit/rules.py index bb11e0b4e67..2ac042129a1 100644 --- a/src/python/pants/backend/python/lint/bandit/rules.py +++ b/src/python/pants/backend/python/lint/bandit/rules.py @@ -9,7 +9,7 @@ from pants.backend.python.target_types import InterpreterConstraintsField, PythonSources from pants.backend.python.util_rules import pex from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints -from pants.backend.python.util_rules.pex import PexRequest, PexRequirements, VenvPex, VenvPexProcess +from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess from pants.core.goals.lint import REPORT_DIR, LintRequest, LintResult, LintResults from pants.core.util_rules.config_files import ConfigFiles, ConfigFilesRequest from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest @@ -61,7 +61,7 @@ async def bandit_lint_partition(partition: BanditPartition, bandit: Bandit) -> L PexRequest( output_filename="bandit.pex", internal_only=True, - requirements=PexRequirements(bandit.all_requirements), + requirements=bandit.pex_requirements, interpreter_constraints=partition.interpreter_constraints, main=bandit.main, ), diff --git a/src/python/pants/backend/python/lint/black/rules.py b/src/python/pants/backend/python/lint/black/rules.py index a6966870aba..e482471f39c 100644 --- a/src/python/pants/backend/python/lint/black/rules.py +++ b/src/python/pants/backend/python/lint/black/rules.py @@ -10,7 +10,7 @@ from pants.backend.python.target_types import InterpreterConstraintsField, PythonSources from pants.backend.python.util_rules import pex from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints -from pants.backend.python.util_rules.pex import PexRequest, PexRequirements, VenvPex, VenvPexProcess +from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess from pants.core.goals.fmt import FmtResult from pants.core.goals.lint import LintRequest, LintResult, LintResults from pants.core.util_rules.config_files import ConfigFiles, ConfigFilesRequest @@ -83,7 +83,7 @@ async def setup_black( all_interpreter_constraints.requires_python38_or_newer() and black.options.is_default("interpreter_constraints") ) - else InterpreterConstraints(black.interpreter_constraints) + else black.interpreter_constraints ) black_pex_get = Get( @@ -91,7 +91,7 @@ async def setup_black( PexRequest( output_filename="black.pex", internal_only=True, - requirements=PexRequirements(black.all_requirements), + requirements=black.pex_requirements, interpreter_constraints=tool_interpreter_constraints, main=black.main, ), 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 d281c1f6420..fcec9db2e77 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 @@ -220,15 +220,7 @@ def replaced(x: bool) -> str: {"f.py": content, "BUILD": "python_library(name='t', interpreter_constraints=['>=3.9'])"} ) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_black( - rule_runner, - [tgt], - # TODO: remove this and go back to using the default version once the new Black release - # comes out. - extra_args=[ - "--black-version=Black@ git+https://github.com/psf/black.git@aebd3c37b28bbc0183a58d13b80e7595db3c09bb" - ], - ) + lint_results, fmt_result = run_black(rule_runner, [tgt]) assert len(lint_results) == 1 assert lint_results[0].exit_code == 0 assert "1 file would be left unchanged" in lint_results[0].stderr diff --git a/src/python/pants/backend/python/lint/black/subsystem.py b/src/python/pants/backend/python/lint/black/subsystem.py index 14127946ea3..e26b460d6b6 100644 --- a/src/python/pants/backend/python/lint/black/subsystem.py +++ b/src/python/pants/backend/python/lint/black/subsystem.py @@ -16,7 +16,6 @@ class Black(PythonToolBase): options_scope = "black" help = "The Black Python code formatter (https://black.readthedocs.io/)." - # TODO: simplify `test_works_with_python39()` to stop using a VCS version. default_version = "black==21.5b2" default_extra_requirements = ["setuptools"] default_main = ConsoleScript("black") diff --git a/src/python/pants/backend/python/lint/docformatter/rules.py b/src/python/pants/backend/python/lint/docformatter/rules.py index e0af1c58ad8..53a472d7893 100644 --- a/src/python/pants/backend/python/lint/docformatter/rules.py +++ b/src/python/pants/backend/python/lint/docformatter/rules.py @@ -10,7 +10,6 @@ from pants.backend.python.lint.python_fmt import PythonFmtRequest from pants.backend.python.target_types import PythonSources from pants.backend.python.util_rules import pex -from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints from pants.backend.python.util_rules.pex import PexRequest, PexRequirements, VenvPex, VenvPexProcess from pants.core.goals.fmt import FmtResult from pants.core.goals.lint import LintRequest, LintResult, LintResults @@ -60,7 +59,7 @@ def generate_args( @rule(level=LogLevel.DEBUG) async def setup_docformatter(setup_request: SetupRequest, docformatter: Docformatter) -> Setup: if docformatter.lockfile == "": - requirements = PexRequirements(docformatter.all_requirements) + requirements = docformatter.pex_requirements elif docformatter.lockfile == "": requirements = PexRequirements( file_content=FileContent( @@ -81,10 +80,9 @@ async def setup_docformatter(setup_request: SetupRequest, docformatter: Docforma PexRequest( output_filename="docformatter.pex", internal_only=True, - interpreter_constraints=InterpreterConstraints(docformatter.interpreter_constraints), - main=docformatter.main, - description="Build docformatter.pex", requirements=requirements, + interpreter_constraints=docformatter.interpreter_constraints, + main=docformatter.main, is_lockfile=docformatter.lockfile != "", ), ) diff --git a/src/python/pants/backend/python/lint/flake8/rules.py b/src/python/pants/backend/python/lint/flake8/rules.py index b05c4db877e..8a2644ea109 100644 --- a/src/python/pants/backend/python/lint/flake8/rules.py +++ b/src/python/pants/backend/python/lint/flake8/rules.py @@ -9,7 +9,7 @@ from pants.backend.python.target_types import InterpreterConstraintsField, PythonSources from pants.backend.python.util_rules import pex from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints -from pants.backend.python.util_rules.pex import PexRequest, PexRequirements, VenvPex, VenvPexProcess +from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess from pants.core.goals.lint import REPORT_DIR, LintRequest, LintResult, LintResults from pants.core.util_rules.config_files import ConfigFiles, ConfigFilesRequest from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest @@ -61,7 +61,7 @@ async def flake8_lint_partition(partition: Flake8Partition, flake8: Flake8) -> L PexRequest( output_filename="flake8.pex", internal_only=True, - requirements=PexRequirements(flake8.all_requirements), + requirements=flake8.pex_requirements, interpreter_constraints=partition.interpreter_constraints, main=flake8.main, ), diff --git a/src/python/pants/backend/python/lint/isort/rules.py b/src/python/pants/backend/python/lint/isort/rules.py index 2fd12feed63..f973c38c973 100644 --- a/src/python/pants/backend/python/lint/isort/rules.py +++ b/src/python/pants/backend/python/lint/isort/rules.py @@ -9,14 +9,7 @@ from pants.backend.python.lint.python_fmt import PythonFmtRequest from pants.backend.python.target_types import PythonSources from pants.backend.python.util_rules import pex -from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints -from pants.backend.python.util_rules.pex import ( - PexRequest, - PexRequirements, - PexResolveInfo, - VenvPex, - VenvPexProcess, -) +from pants.backend.python.util_rules.pex import PexRequest, PexResolveInfo, VenvPex, VenvPexProcess from pants.core.goals.fmt import FmtResult from pants.core.goals.lint import LintRequest, LintResult, LintResults from pants.core.util_rules.config_files import ConfigFiles, ConfigFilesRequest @@ -89,8 +82,8 @@ async def setup_isort(setup_request: SetupRequest, isort: Isort) -> Setup: PexRequest( output_filename="isort.pex", internal_only=True, - requirements=PexRequirements(isort.all_requirements), - interpreter_constraints=InterpreterConstraints(isort.interpreter_constraints), + requirements=isort.pex_requirements, + interpreter_constraints=isort.interpreter_constraints, main=isort.main, ), ) diff --git a/src/python/pants/backend/python/subsystems/python_tool_base.py b/src/python/pants/backend/python/subsystems/python_tool_base.py index efc5eea480c..895b012a3e2 100644 --- a/src/python/pants/backend/python/subsystems/python_tool_base.py +++ b/src/python/pants/backend/python/subsystems/python_tool_base.py @@ -1,9 +1,13 @@ # Copyright 2018 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -from typing import ClassVar, Sequence, Tuple, cast +from __future__ import annotations + +from typing import ClassVar, Sequence, cast from pants.backend.python.target_types import ConsoleScript, EntryPoint, MainSpecification +from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints +from pants.backend.python.util_rules.pex import PexRequirements from pants.option.errors import OptionsError from pants.option.subsystem import Subsystem @@ -56,20 +60,39 @@ def register_options(cls, register): ) @property - def requirement(self) -> str: + def version(self) -> str: return cast(str, self.options.version) @property - def extra_requirements(self) -> Tuple[str, ...]: + def extra_requirements(self) -> tuple[str, ...]: return tuple(self.options.extra_requirements) @property - def all_requirements(self) -> Tuple[str, ...]: - return (self.requirement, *self.extra_requirements) + def all_requirements(self) -> tuple[str, ...]: + """All the raw requirement strings to install the tool. + + This may not include transitive dependencies: these are top-level requirements. + """ + return (self.version, *self.extra_requirements) + + @property + def pex_requirements(self) -> PexRequirements: + """The requirements to be used when installing the tool. + + If the tool supports lockfiles, the returned type will install from the lockfile rather than + `all_requirements`. + """ + # TODO(#11898): move lockfile option registration into PythonToolBase, and update this to + # set the PexRequirements appropriately. + return PexRequirements(self.all_requirements) @property - def interpreter_constraints(self) -> Tuple[str, ...]: - return tuple(self.options.interpreter_constraints) + def interpreter_constraints(self) -> InterpreterConstraints: + """The interpreter constraints to use when installing and running the tool. + + This assumes you have set the class property `register_interpreter_constraints = True`. + """ + return InterpreterConstraints(self.options.interpreter_constraints) class PythonToolBase(PythonToolRequirementsBase): diff --git a/src/python/pants/backend/python/typecheck/mypy/rules.py b/src/python/pants/backend/python/typecheck/mypy/rules.py index 26aa07d4a02..ac8cfaab14d 100644 --- a/src/python/pants/backend/python/typecheck/mypy/rules.py +++ b/src/python/pants/backend/python/typecheck/mypy/rules.py @@ -170,7 +170,7 @@ async def mypy_typecheck_partition(partition: MyPyPartition, mypy: MyPy) -> Type mypy.options.is_default("interpreter_constraints") and partition.interpreter_constraints.requires_python38_or_newer() ) - else InterpreterConstraints(mypy.interpreter_constraints) + else mypy.interpreter_constraints ) plugin_sources_get = Get( @@ -331,9 +331,10 @@ async def mypy_typecheck( interpreter_constraints_to_transitive_targets = defaultdict(set) for transitive_targets in transitive_targets_per_field_set: - interpreter_constraints = InterpreterConstraints.create_from_targets( - transitive_targets.closure, python_setup - ) or InterpreterConstraints(mypy.interpreter_constraints) + interpreter_constraints = ( + InterpreterConstraints.create_from_targets(transitive_targets.closure, python_setup) + or mypy.interpreter_constraints + ) interpreter_constraints_to_transitive_targets[interpreter_constraints].add( transitive_targets )