Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove experimental and unused [python-setup].requirement_constraints_target option and _python_constraints target #11998

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/python/pants/backend/python/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
PexBinary,
PythonDistribution,
PythonLibrary,
PythonRequirementConstraints,
PythonRequirementLibrary,
PythonRequirementsFile,
PythonTests,
Expand Down Expand Up @@ -81,6 +80,5 @@ def target_types():
PythonLibrary,
PythonRequirementLibrary,
PythonRequirementsFile,
PythonRequirementConstraints,
PythonTests,
]
17 changes: 0 additions & 17 deletions src/python/pants/backend/python/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -634,23 +634,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
# -----------------------------------------------------------------------------------------------
Expand Down
21 changes: 9 additions & 12 deletions src/python/pants/backend/python/target_types_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
PythonDistribution,
PythonDistributionDependencies,
PythonLibrary,
PythonRequirementConstraintsField,
PythonRequirementLibrary,
PythonRequirementsField,
PythonTestsTimeout,
Expand All @@ -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,
Expand Down Expand Up @@ -285,37 +283,36 @@ 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'",
"pip@ git+https://github.com/pypa/pip.git",
)
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)


Expand Down
69 changes: 13 additions & 56 deletions src/python/pants/backend/python/util_rules/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,15 @@

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 (
PexEnvironment,
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 (
Expand All @@ -42,7 +39,6 @@
CreateDigest,
Digest,
FileContent,
GlobExpansionConjunction,
GlobMatchErrorBehavior,
MergeDigests,
PathGlobs,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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."""

Expand Down Expand Up @@ -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)
Expand Down
36 changes: 16 additions & 20 deletions src/python/pants/backend/python/util_rules/pex_from_targets.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
parse_requirements_file,
)
from pants.backend.python.util_rules.pex import (
MaybeConstraintsFile,
Pex,
PexInterpreterConstraints,
PexPlatforms,
Expand All @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
)
)

Expand All @@ -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:
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 set."
) in str(err.value)

# Shouldn't error, as we don't explicitly set --resolve-all-constraints.
Expand Down
40 changes: 1 addition & 39 deletions src/python/pants/backend/python/util_rules/pex_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@
EntryPoint,
InterpreterConstraintsField,
MainSpecification,
PythonRequirementConstraints,
)
from pants.backend.python.util_rules.pex import (
MaybeConstraintsFile,
Pex,
PexDistributionInfo,
PexInterpreterConstraints,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading