From 9d6147aa6e293201386f65512abb7f18cec480ea Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Fri, 30 Apr 2021 18:23:47 -0700 Subject: [PATCH 1/2] Remove experimental and unused `[python-setup].requirement_constraints_target` option and `_python_constraints` target # 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/register.py | 2 - .../pants/backend/python/target_types.py | 17 ----- .../pants/backend/python/target_types_test.py | 21 +++--- .../pants/backend/python/util_rules/pex.py | 69 ++++--------------- .../python/util_rules/pex_from_targets.py | 36 +++++----- .../util_rules/pex_from_targets_test.py | 5 +- .../backend/python/util_rules/pex_test.py | 40 +---------- src/python/pants/python/python_setup.py | 22 +----- 8 files changed, 42 insertions(+), 170 deletions(-) diff --git a/src/python/pants/backend/python/register.py b/src/python/pants/backend/python/register.py index e68cd461c04..ab155ba451d 100644 --- a/src/python/pants/backend/python/register.py +++ b/src/python/pants/backend/python/register.py @@ -26,7 +26,6 @@ PexBinary, PythonDistribution, PythonLibrary, - PythonRequirementConstraints, PythonRequirementLibrary, PythonRequirementsFile, PythonTests, @@ -81,6 +80,5 @@ def target_types(): PythonLibrary, PythonRequirementLibrary, PythonRequirementsFile, - PythonRequirementConstraints, PythonTests, ] diff --git a/src/python/pants/backend/python/target_types.py b/src/python/pants/backend/python/target_types.py index 15cc37af172..41adeacf84c 100644 --- a/src/python/pants/backend/python/target_types.py +++ b/src/python/pants/backend/python/target_types.py @@ -638,23 +638,6 @@ class PythonRequirementsFile(Target): help = "A private helper target type for requirements.txt files." -# ----------------------------------------------------------------------------------------------- -# `_python_constraints` target -# ----------------------------------------------------------------------------------------------- - - -class PythonRequirementConstraintsField(_RequirementSequenceField): - alias = "constraints" - required = True - help = "A list of pip-style requirement strings, e.g. `my_dist==4.2.1`." - - -class PythonRequirementConstraints(Target): - alias = "_python_constraints" - core_fields = (*COMMON_TARGET_FIELDS, PythonRequirementConstraintsField) - help = "A private helper target for inlined requirements constraints, used by macros." - - # ----------------------------------------------------------------------------------------------- # `python_distribution` target # ----------------------------------------------------------------------------------------------- diff --git a/src/python/pants/backend/python/target_types_test.py b/src/python/pants/backend/python/target_types_test.py index 73d6be3baa4..15a2cea9435 100644 --- a/src/python/pants/backend/python/target_types_test.py +++ b/src/python/pants/backend/python/target_types_test.py @@ -24,7 +24,6 @@ PythonDistribution, PythonDistributionDependencies, PythonLibrary, - PythonRequirementConstraintsField, PythonRequirementLibrary, PythonRequirementsField, PythonTestsTimeout, @@ -42,7 +41,6 @@ from pants.engine.addresses import Address from pants.engine.internals.scheduler import ExecutionError from pants.engine.target import ( - Field, InjectedDependencies, InvalidFieldException, InvalidFieldTypeException, @@ -285,8 +283,7 @@ def assert_injected(address: Address, *, expected: Optional[Address]) -> None: assert not caplog.records -@pytest.mark.parametrize("field", [PythonRequirementsField, PythonRequirementConstraintsField]) -def test_requirements_and_constraints_fields(field: type[Field]) -> None: +def test_requirements_field() -> None: raw_value = ( "argparse==1.2.1", "configparser ; python_version<'3'", @@ -294,28 +291,28 @@ def test_requirements_and_constraints_fields(field: type[Field]) -> None: ) parsed_value = tuple(Requirement.parse(v) for v in raw_value) - assert field(raw_value, Address("demo")).value == parsed_value + assert PythonRequirementsField(raw_value, Address("demo")).value == parsed_value # Macros can pass pre-parsed Requirement objects. - assert field(parsed_value, Address("demo")).value == parsed_value + assert PythonRequirementsField(parsed_value, Address("demo")).value == parsed_value # Reject invalid types. with pytest.raises(InvalidFieldTypeException): - field("sneaky_str", Address("demo")) + PythonRequirementsField("sneaky_str", Address("demo")) with pytest.raises(InvalidFieldTypeException): - field([1, 2], Address("demo")) + PythonRequirementsField([1, 2], Address("demo")) # Give a nice error message if the requirement can't be parsed. with pytest.raises(InvalidFieldException) as exc: - field(["not valid! === 3.1"], Address("demo")) + PythonRequirementsField(["not valid! === 3.1"], Address("demo")) assert ( - f"Invalid requirement 'not valid! === 3.1' in the '{field.alias}' field for the " - "target demo:" + f"Invalid requirement 'not valid! === 3.1' in the '{PythonRequirementsField.alias}' " + f"field for the target demo:" ) in str(exc.value) # Give a nice error message if it looks like they're trying to use pip VCS-style requirements. with pytest.raises(InvalidFieldException) as exc: - field(["git+https://github.com/pypa/pip.git#egg=pip"], Address("demo")) + PythonRequirementsField(["git+https://github.com/pypa/pip.git#egg=pip"], Address("demo")) assert "It looks like you're trying to use a pip VCS-style requirement?" in str(exc.value) diff --git a/src/python/pants/backend/python/util_rules/pex.py b/src/python/pants/backend/python/util_rules/pex.py index 2aab0f2b87d..a2d13209957 100644 --- a/src/python/pants/backend/python/util_rules/pex.py +++ b/src/python/pants/backend/python/util_rules/pex.py @@ -22,10 +22,7 @@ from pants.backend.python.target_types import InterpreterConstraintsField, MainSpecification from pants.backend.python.target_types import PexPlatformsField as PythonPlatformsField -from pants.backend.python.target_types import ( - PythonRequirementConstraintsField, - PythonRequirementsField, -) +from pants.backend.python.target_types import PythonRequirementsField from pants.backend.python.util_rules import pex_cli from pants.backend.python.util_rules.pex_cli import PexCliProcess, PexPEX from pants.backend.python.util_rules.pex_environment import ( @@ -33,7 +30,7 @@ PexRuntimeEnvironment, PythonExecutable, ) -from pants.engine.addresses import Address, UnparsedAddressInputs +from pants.engine.addresses import Address from pants.engine.collection import Collection, DeduplicatedCollection from pants.engine.engine_aware import EngineAwareParameter from pants.engine.fs import ( @@ -42,7 +39,6 @@ CreateDigest, Digest, FileContent, - GlobExpansionConjunction, GlobMatchErrorBehavior, MergeDigests, PathGlobs, @@ -56,7 +52,7 @@ ProcessResult, ) from pants.engine.rules import Get, collect_rules, rule -from pants.engine.target import Target, Targets +from pants.engine.target import Target from pants.python.python_repos import PythonRepos from pants.python.python_setup import PythonSetup from pants.util.frozendict import FrozenDict @@ -496,51 +492,6 @@ async def find_interpreter( return PythonExecutable(path=path, fingerprint=fingerprint) -@dataclass(frozen=True) -class MaybeConstraintsFile: - path: str | None - digest: Digest - - -@rule(desc="Resolve requirements constraints file") -async def resolve_requirements_constraints_file(python_setup: PythonSetup) -> MaybeConstraintsFile: - if python_setup.requirement_constraints: - digest = await Get( - Digest, - PathGlobs( - [python_setup.requirement_constraints], - glob_match_error_behavior=GlobMatchErrorBehavior.error, - conjunction=GlobExpansionConjunction.all_match, - description_of_origin="the option `[python-setup].requirement_constraints`", - ), - ) - return MaybeConstraintsFile(python_setup.requirement_constraints, digest) - - if python_setup.requirement_constraints_target: - targets = await Get( - Targets, - UnparsedAddressInputs( - [python_setup.requirement_constraints_target], owning_address=None - ), - ) - tgt = targets.expect_single() - if not tgt.has_field(PythonRequirementConstraintsField): - raise ValueError( - "Invalid target type for `[python-setup].requirement_constraints_target`. Please " - f"use a `_python_constraints` target instead of a `{tgt.alias}` target." - ) - formatted_constraints = "\n".join( - str(constraint) for constraint in tgt[PythonRequirementConstraintsField].value - ) - path = "constraints.generated.txt" - digest = await Get( - Digest, CreateDigest([FileContent(path, formatted_constraints.encode())]) - ) - return MaybeConstraintsFile(path, digest) - - return MaybeConstraintsFile(None, EMPTY_DIGEST) - - @dataclass(frozen=True) class BuildPexResult: result: ProcessResult @@ -559,7 +510,6 @@ async def build_pex( python_repos: PythonRepos, platform: Platform, pex_runtime_env: PexRuntimeEnvironment, - constraints_file: MaybeConstraintsFile, ) -> BuildPexResult: """Returns a PEX with the given settings.""" @@ -624,9 +574,16 @@ async def build_pex( argv.extend(request.requirements) constraint_file_digest = EMPTY_DIGEST - if request.apply_requirement_constraints and constraints_file.path is not None: - argv.extend(["--constraints", constraints_file.path]) - constraint_file_digest = constraints_file.digest + if request.apply_requirement_constraints and python_setup.requirement_constraints is not None: + argv.extend(["--constraints", python_setup.requirement_constraints]) + constraint_file_digest = await Get( + Digest, + PathGlobs( + [python_setup.requirement_constraints], + glob_match_error_behavior=GlobMatchErrorBehavior.error, + description_of_origin="the option `[python-setup].requirement-constraints`", + ), + ) sources_digest_as_subdir = await Get( Digest, AddPrefix(request.sources or EMPTY_DIGEST, source_dir_name) diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets.py b/src/python/pants/backend/python/util_rules/pex_from_targets.py index 14919df6665..7bf1aea8fa5 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets.py @@ -18,7 +18,6 @@ parse_requirements_file, ) from pants.backend.python.util_rules.pex import ( - MaybeConstraintsFile, Pex, PexInterpreterConstraints, PexPlatforms, @@ -33,7 +32,7 @@ ) from pants.backend.python.util_rules.python_sources import rules as python_sources_rules from pants.engine.addresses import Address, Addresses -from pants.engine.fs import Digest, DigestContents, MergeDigests +from pants.engine.fs import Digest, DigestContents, GlobMatchErrorBehavior, MergeDigests, PathGlobs from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import ( Dependencies, @@ -180,11 +179,7 @@ class TwoStepPexFromTargetsRequest: @rule(level=LogLevel.DEBUG) -async def pex_from_targets( - request: PexFromTargetsRequest, - python_setup: PythonSetup, - constraints_file: MaybeConstraintsFile, -) -> PexRequest: +async def pex_from_targets(request: PexFromTargetsRequest, python_setup: PythonSetup) -> PexRequest: if request.direct_deps_only: targets = await Get(Targets, Addresses(request.addresses)) direct_deps = await MultiGet( @@ -232,12 +227,19 @@ async def pex_from_targets( repository_pex: Pex | None = None description = request.description - if constraints_file.path: - constraints_file_contents = await Get(DigestContents, Digest, constraints_file.digest) + if python_setup.requirement_constraints: + constraints_file_contents = await Get( + DigestContents, + PathGlobs( + [python_setup.requirement_constraints], + glob_match_error_behavior=GlobMatchErrorBehavior.error, + description_of_origin="the option `[python-setup].requirement_constraints`", + ), + ) constraints_file_reqs = set( parse_requirements_file( constraints_file_contents[0].content.decode(), - rel_path=constraints_file.path, + rel_path=python_setup.requirement_constraints, ) ) @@ -264,14 +266,9 @@ async def pex_from_targets( # constrained by their very nature). See https://github.com/pypa/pip/issues/8210. unconstrained_projects = name_req_projects - constraint_file_projects if unconstrained_projects: - constraints_descr = ( - f"constraints file {constraints_file.path}" - if python_setup.requirement_constraints - else f"_python_constraints target {python_setup.requirement_constraints_target}" - ) logger.warning( - f"The {constraints_descr} does not contain entries for the following " - f"requirements: {', '.join(unconstrained_projects)}" + f"The constraints file {python_setup.requirement_constraints} does not contain " + f"entries for the following requirements: {', '.join(unconstrained_projects)}" ) if python_setup.resolve_all_constraints: @@ -309,9 +306,8 @@ async def pex_from_targets( and python_setup.resolve_all_constraints_was_set_explicitly() ): raise ValueError( - "[python-setup].resolve_all_constraints is enabled, so either " - "[python-setup].requirement_constraints or " - "[python-setup].requirement_constraints_target must also be provided." + "`[python-setup].resolve_all_constraints` is enabled, so " + "`[python-setup].requirement_constraints` must also be set." ) return PexRequest( diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py index e631c773358..6f72e9e9f6a 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py @@ -228,9 +228,8 @@ def get_pex_request( assert len(err.value.wrapped_exceptions) == 1 assert isinstance(err.value.wrapped_exceptions[0], ValueError) assert ( - "[python-setup].resolve_all_constraints is enabled, so " - "either [python-setup].requirement_constraints or " - "[python-setup].requirement_constraints_target must also be provided." + "`[python-setup].resolve_all_constraints` is enabled, so " + "`[python-setup].requirement_constraints` must also be provided." ) in str(err.value) # Shouldn't error, as we don't explicitly set --resolve-all-constraints. 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 1da7b7d7e2b..f29fec9b4b7 100644 --- a/src/python/pants/backend/python/util_rules/pex_test.py +++ b/src/python/pants/backend/python/util_rules/pex_test.py @@ -19,10 +19,8 @@ EntryPoint, InterpreterConstraintsField, MainSpecification, - PythonRequirementConstraints, ) from pants.backend.python.util_rules.pex import ( - MaybeConstraintsFile, Pex, PexDistributionInfo, PexInterpreterConstraints, @@ -33,14 +31,12 @@ PexResolveInfo, VenvPex, VenvPexProcess, - resolve_requirements_constraints_file, ) from pants.backend.python.util_rules.pex import rules as pex_rules from pants.backend.python.util_rules.pex_cli import PexPEX from pants.engine.addresses import Address -from pants.engine.fs import EMPTY_DIGEST, CreateDigest, Digest, FileContent +from pants.engine.fs import CreateDigest, Digest, FileContent from pants.engine.process import Process, ProcessResult -from pants.engine.rules import SubsystemRule from pants.engine.target import FieldSet from pants.python.python_setup import PythonSetup from pants.testutil.option_util import create_subsystem @@ -295,40 +291,6 @@ def test_group_field_sets_by_constraints_with_unsorted_inputs() -> None: ) -def test_maybe_constraints_file() -> None: - rule_runner = RuleRunner( - rules=[ - resolve_requirements_constraints_file, - SubsystemRule(PythonSetup), - QueryRule(MaybeConstraintsFile, []), - ], - target_types=[PythonRequirementConstraints], - ) - constraints = ["c1==1.1.1", "c2==2.2.2"] - constraints_file = "\n".join(constraints) - rule_runner.create_file("constraints.txt", constraints_file) - rule_runner.add_to_build_file( - "", f"_python_constraints(name='constraints', constraints={repr(constraints)})" - ) - - def get_constraints(arg: str | None) -> MaybeConstraintsFile: - if arg: - rule_runner.set_options([arg]) - return rule_runner.request(MaybeConstraintsFile, []) - - assert get_constraints(None) == MaybeConstraintsFile(None, EMPTY_DIGEST) - expected_digest = rule_runner.make_snapshot({"constraints.txt": constraints_file}).digest - assert get_constraints( - "--python-setup-requirement-constraints=constraints.txt" - ) == MaybeConstraintsFile("constraints.txt", expected_digest) - expected_digest = rule_runner.make_snapshot( - {"constraints.generated.txt": constraints_file} - ).digest - assert get_constraints( - "--python-setup-requirement-constraints-target=//:constraints" - ) == MaybeConstraintsFile("constraints.generated.txt", expected_digest) - - @dataclass(frozen=True) class ExactRequirement: project_name: str diff --git a/src/python/pants/python/python_setup.py b/src/python/pants/python/python_setup.py index 5f30a7cc026..b411a12daca 100644 --- a/src/python/pants/python/python_setup.py +++ b/src/python/pants/python/python_setup.py @@ -15,7 +15,7 @@ from pants.base.build_environment import get_buildroot from pants.base.deprecated import deprecated_conditional from pants.engine.environment import Environment -from pants.option.custom_types import file_option, target_option +from pants.option.custom_types import file_option from pants.option.errors import BooleanConversionError from pants.option.parser import Parser from pants.option.subsystem import Subsystem @@ -82,7 +82,6 @@ def register_options(cls, register): "--requirement-constraints", advanced=True, type=file_option, - mutually_exclusive_group="constraints", help=( "When resolving third-party requirements, use this " "constraints file to determine which versions to use.\n\nSee " @@ -91,20 +90,6 @@ def register_options(cls, register): "\n\nMutually exclusive with `--requirement-constraints-target`." ), ) - register( - "--requirement-constraints-target", - advanced=True, - type=target_option, - mutually_exclusive_group="constraints", - help=( - "When resolving third-party requirements, use this " - "_python_constraints target to determine which versions to use.\n\nThis is " - "primarily intended for macros (for now). Normally, use " - "`--requirement-constraints` instead with a constraints file.\n\nSee " - "https://pip.pypa.io/en/stable/user_guide/#constraints-files for more information " - "on the format of constraints and how constraints are applied in Pex and pip." - ), - ) register( "--resolve-all-constraints", advanced=True, @@ -173,11 +158,6 @@ def requirement_constraints(self) -> str | None: """Path to constraint file.""" return cast("str | None", self.options.requirement_constraints) - @property - def requirement_constraints_target(self) -> str | None: - """Address for a _python_constraints target.""" - return cast("str | None", self.options.requirement_constraints_target) - @property def resolve_all_constraints(self) -> bool: return cast(bool, self.options.resolve_all_constraints) From c4d44bf13d0036dd4c60b716e6b2aa24c0491b33 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Thu, 6 May 2021 10:04:08 -0700 Subject: [PATCH 2/2] Fix test expected error message --- .../pants/backend/python/util_rules/pex_from_targets_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py index 6f72e9e9f6a..0b0bd5a1849 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py @@ -229,7 +229,7 @@ def get_pex_request( assert isinstance(err.value.wrapped_exceptions[0], ValueError) assert ( "`[python-setup].resolve_all_constraints` is enabled, so " - "`[python-setup].requirement_constraints` must also be provided." + "`[python-setup].requirement_constraints` must also be set." ) in str(err.value) # Shouldn't error, as we don't explicitly set --resolve-all-constraints.