Skip to content

Commit

Permalink
Restore refactor of apply_requirement_constraints in #12675. (#12883)
Browse files Browse the repository at this point in the history
The refactor was a step forward, it was just all the remaining code in the PR
that was target of the revert.

Follow-up to #12808.
  • Loading branch information
jsirois authored Sep 14, 2021
1 parent fd71177 commit 1dacea9
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 39 deletions.
56 changes: 31 additions & 25 deletions src/python/pants/backend/python/util_rules/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,17 +99,25 @@ class ToolCustomLockfile(Lockfile, _ToolLockfileMixin):
@dataclass(unsafe_hash=True)
class PexRequirements:
req_strings: FrozenOrderedSet[str]
apply_constraints: bool
repository_pex: Pex | None

def __init__(
self, req_strings: Iterable[str] = (), *, repository_pex: Pex | None = None
self,
req_strings: Iterable[str] = (),
*,
apply_constraints: bool = False,
repository_pex: Pex | None = None,
) -> None:
"""
:param req_strings: The requirement strings to resolve.
:param apply_constraints: Whether to apply any configured requirement_constraints while
building this PEX.
:param repository_pex: An optional PEX to resolve requirements from via the Pex CLI
`--pex-repository` option.
"""
self.req_strings = FrozenOrderedSet(sorted(req_strings))
self.apply_constraints = apply_constraints
self.repository_pex = repository_pex

@classmethod
Expand All @@ -118,9 +126,12 @@ def create_from_requirement_fields(
fields: Iterable[PythonRequirementsField],
*,
additional_requirements: Iterable[str] = (),
apply_constraints: bool = True,
) -> PexRequirements:
field_requirements = {str(python_req) for field in fields for python_req in field.value}
return PexRequirements({*field_requirements, *additional_requirements})
return PexRequirements(
{*field_requirements, *additional_requirements}, apply_constraints=apply_constraints
)

def __bool__(self) -> bool:
return bool(self.req_strings)
Expand Down Expand Up @@ -154,7 +165,6 @@ class PexRequest(EngineAwareParameter):
main: MainSpecification | None
additional_args: Tuple[str, ...]
pex_path: Tuple[Pex, ...]
apply_requirement_constraints: bool
description: str | None = dataclasses.field(compare=False)

def __init__(
Expand All @@ -171,7 +181,6 @@ def __init__(
main: MainSpecification | None = None,
additional_args: Iterable[str] = (),
pex_path: Iterable[Pex] = (),
apply_requirement_constraints: bool = False,
description: str | None = None,
) -> None:
"""A request to create a PEX from its inputs.
Expand All @@ -196,8 +205,6 @@ def __init__(
left off, the Pex will open up as a REPL.
:param additional_args: Any additional Pex flags.
:param pex_path: Pex files to add to the PEX_PATH.
:param apply_requirement_constraints: Whether to apply any configured
requirement_constraints while building this PEX.
:param description: A human-readable description to render in the dynamic UI when building
the Pex.
"""
Expand All @@ -212,7 +219,6 @@ def __init__(
self.main = main
self.additional_args = tuple(additional_args)
self.pex_path = tuple(pex_path)
self.apply_requirement_constraints = apply_requirement_constraints
self.description = description
self.__post_init__()

Expand Down Expand Up @@ -350,10 +356,6 @@ async def build_pex(
]
)

is_lockfile = isinstance(request.requirements, (Lockfile, LockfileContent))
if is_lockfile:
argv.append("--no-transitive")

python: PythonExecutable | None = None

# NB: If `--platform` is specified, this signals that the PEX should not be built locally.
Expand Down Expand Up @@ -407,21 +409,7 @@ async def build_pex(

additional_inputs_digest = request.additional_inputs or EMPTY_DIGEST
repository_pex_digest = repository_pex.digest if repository_pex else EMPTY_DIGEST

constraint_file_digest = EMPTY_DIGEST
if (
not is_lockfile and 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`",
),
)

requirements_file_digest = EMPTY_DIGEST

# TODO(#12314): Capture the resolve name for multiple user lockfiles.
Expand All @@ -433,6 +421,7 @@ async def build_pex(

if isinstance(request.requirements, Lockfile):
argv.extend(["--requirement", request.requirements.file_path])
argv.append("--no-transitive")

globs = PathGlobs(
[request.requirements.file_path],
Expand All @@ -454,12 +443,29 @@ async def build_pex(
elif isinstance(request.requirements, LockfileContent):
file_content = request.requirements.file_content
argv.extend(["--requirement", file_content.path])
argv.append("--no-transitive")

metadata = LockfileMetadata.from_lockfile(file_content.content, resolve_name=resolve_name)
_validate_metadata(metadata, request, request.requirements, python_setup)

requirements_file_digest = await Get(Digest, CreateDigest([file_content]))
else:
assert isinstance(request.requirements, PexRequirements)

if (
request.requirements.apply_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`",
),
)

argv.extend(request.requirements.req_strings)

merged_digest = await Get(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ async def pex_from_targets(request: PexFromTargetsRequest, python_setup: PythonS
if tgt.has_field(PythonRequirementsField)
),
additional_requirements=request.additional_requirements,
apply_constraints=True,
)

description = request.description
Expand Down Expand Up @@ -309,7 +310,6 @@ async def pex_from_targets(request: PexFromTargetsRequest, python_setup: PythonS
additional_inputs=request.additional_inputs,
additional_args=request.additional_args,
description=description,
apply_requirement_constraints=True,
)


Expand Down Expand Up @@ -383,11 +383,10 @@ async def _setup_constraints_repository_pex(
description=f"Resolving {constraints_path}",
output_filename="repository.pex",
internal_only=request.internal_only,
requirements=PexRequirements(all_constraints),
requirements=PexRequirements(all_constraints, apply_constraints=True),
interpreter_constraints=request.interpreter_constraints,
platforms=request.platforms,
additional_args=request.additional_lockfile_args,
apply_requirement_constraints=True,
),
)
return _ConstraintsRepositoryPex(repository_pex)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,13 @@ def get_pex_request(

pex_req1 = get_pex_request("constraints1.txt", resolve_all_constraints=False)
assert pex_req1.requirements == PexRequirements(
["foo-bar>=0.1.2", "bar==5.5.5", "baz", url_req]
["foo-bar>=0.1.2", "bar==5.5.5", "baz", url_req], apply_constraints=True
)

pex_req1_direct = get_pex_request(
"constraints1.txt", resolve_all_constraints=False, direct_deps_only=True
)
assert pex_req1_direct.requirements == PexRequirements(["baz", url_req])
assert pex_req1_direct.requirements == PexRequirements(["baz", url_req], apply_constraints=True)

pex_req2 = get_pex_request(
"constraints1.txt",
Expand Down Expand Up @@ -298,4 +298,4 @@ def test_issue_12222(rule_runner: RuleRunner) -> None:
)
result = rule_runner.request(PexRequest, [request])

assert result.requirements == PexRequirements(["foo"])
assert result.requirements == PexRequirements(["foo"], apply_constraints=True)
7 changes: 3 additions & 4 deletions src/python/pants/backend/python/util_rules/pex_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ def create_pex_and_get_all_data(
sources=sources,
additional_inputs=additional_inputs,
additional_args=additional_pex_args,
apply_requirement_constraints=True,
)
rule_runner.set_options(
["--backend-packages=pants.backend.python", *additional_pants_args],
Expand Down Expand Up @@ -423,7 +422,7 @@ def assert_direct_requirements(pex_info):
# Unconstrained, we should always pick the top of the range (requests 2.23.0) since the top of
# the range is a transitive closure over universal wheels.
direct_pex_info = create_pex_and_get_pex_info(
rule_runner, requirements=PexRequirements(direct_deps)
rule_runner, requirements=PexRequirements(direct_deps, apply_constraints=False)
)
assert_direct_requirements(direct_pex_info)
assert "requests-2.23.0-py2.py3-none-any.whl" in set(direct_pex_info["distributions"].keys())
Expand All @@ -438,7 +437,7 @@ def assert_direct_requirements(pex_info):
rule_runner.create_file("constraints.txt", "\n".join(constraints))
constrained_pex_info = create_pex_and_get_pex_info(
rule_runner,
requirements=PexRequirements(direct_deps),
requirements=PexRequirements(direct_deps, apply_constraints=True),
additional_pants_args=("--python-setup-requirement-constraints=constraints.txt",),
)
assert_direct_requirements(constrained_pex_info)
Expand Down Expand Up @@ -541,7 +540,7 @@ def test_venv_pex_resolve_info(rule_runner: RuleRunner, pex_type: type[Pex | Ven
pex = create_pex_and_get_all_data(
rule_runner,
pex_type=pex_type,
requirements=PexRequirements(["requests==2.23.0"]),
requirements=PexRequirements(["requests==2.23.0"], apply_constraints=True),
additional_pants_args=("--python-setup-requirement-constraints=constraints.txt",),
).pex
dists = rule_runner.request(PexResolveInfo, [pex])
Expand Down
7 changes: 3 additions & 4 deletions src/python/pants/init/plugin_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ async def resolve_plugins(
`named_caches` directory), but consequently needs to disable the process cache: see the
ProcessCacheScope reference in the body.
"""
requirements = PexRequirements(sorted(global_options.options.plugins))
# The repository's constraints are not relevant here, because this resolve is mixed
# into the Pants' process' path, and never into user code.
requirements = PexRequirements(sorted(global_options.options.plugins), apply_constraints=False)
if not requirements:
return ResolvedPluginDistributions()

Expand All @@ -72,9 +74,6 @@ async def resolve_plugins(
python=python,
requirements=requirements,
interpreter_constraints=request.interpreter_constraints,
# The repository's constraints are not relevant here, because this resolve is mixed
# into the Pants' process' path, and never into user code.
apply_requirement_constraints=False,
description=f"Resolving plugins: {', '.join(requirements.req_strings)}",
),
)
Expand Down

0 comments on commit 1dacea9

Please sign in to comment.