Skip to content

Commit

Permalink
Remove experimental and unused `[python-setup].requirement_constraint…
Browse files Browse the repository at this point in the history
…s_target` option and `_python_constraints` target (#11998)

This was added in #11724 to support a macro for Poetry for constraints files. But we recently decided that the macro will not support automatically reading `Poetry.lock` because it is not very much to ask users to do `poetry --export`. The macro will only operate on `pyproject.toml`. As suggested in https://docs.google.com/document/d/1bCYb0UQZx9a-9tAagydCN_z3826QRvz_3aVnXKSNTJw/edit#heading=h.11w5a92c1zo8, (for now) the lockfile format will look like requirements.txt

This PR's reverted code diverges from the tool lockfile proposal and it's getting in the way.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano authored May 6, 2021
1 parent 51187e9 commit 311419b
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 170 deletions.
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

0 comments on commit 311419b

Please sign in to comment.