Skip to content

Commit

Permalink
Do not use a repository-PEX if a PEX has platforms specified (cherryp…
Browse files Browse the repository at this point in the history
…ick of #15031) (#15034)

#14995 shows that #12222 re-broke for the `enable_resolves` case, since the fix for the issue was only checking the `resolve_all_constraints` field. Fixes #14995.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
stuhood authored Apr 7, 2022
1 parent 232293c commit f51f504
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 19 deletions.
5 changes: 3 additions & 2 deletions src/python/pants/backend/python/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1025,8 +1025,9 @@ def parse_requirements_file(content: str, *, rel_path: str) -> Iterator[PipRequi
VCS requirements will fail, with a helpful error message describing how to use PEP 440.
"""
for i, line in enumerate(content.splitlines()):
line = line.strip()
if not line or line.startswith("#") or line.startswith("-"):
line, _, _ = line.partition("--")
line = line.strip().rstrip("\\")
if not line or line.startswith("#"):
continue
try:
yield PipRequirement.parse(line)
Expand Down
8 changes: 7 additions & 1 deletion src/python/pants/backend/python/target_types_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,20 +358,26 @@ def test_requirements_field() -> None:

def test_parse_requirements_file() -> None:
content = dedent(
"""\
r"""\
# Comment.
--find-links=https://duckduckgo.com
ansicolors>=1.18.0
Django==3.2 ; python_version>'3'
Un-Normalized-PROJECT # Inline comment.
pip@ git+https://github.com/pypa/pip.git
setuptools==54.1.2; python_version >= "3.6" \
--hash=sha256:dd20743f36b93cbb8724f4d2ccd970dce8b6e6e823a13aa7e5751bb4e674c20b \
--hash=sha256:ebd0148faf627b569c8d2a1b20f5d3b09c873f12739d71c7ee88f037d5be82ff
wheel==1.2.3 --hash=sha256:dd20743f36b93cbb8724f4d2ccd970dce8b6e6e823a13aa7e5751bb4e674c20b
"""
)
assert set(parse_requirements_file(content, rel_path="foo.txt")) == {
PipRequirement.parse("ansicolors>=1.18.0"),
PipRequirement.parse("Django==3.2 ; python_version>'3'"),
PipRequirement.parse("Un-Normalized-PROJECT"),
PipRequirement.parse("pip@ git+https://github.com/pypa/pip.git"),
PipRequirement.parse("setuptools==54.1.2; python_version >= '3.6'"),
PipRequirement.parse("wheel==1.2.3"),
}


Expand Down
15 changes: 12 additions & 3 deletions src/python/pants/backend/python/util_rules/pex_from_targets.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,13 +393,20 @@ async def create_pex_from_targets(

pex_native_subsetting_supported = False
if python_setup.enable_resolves:
# TODO: Once `requirement_constraints` is removed in favor of `enable_resolves`,
# `ChosenPythonResolveRequest` and `_PexRequirementsRequest` should merge and
# do a single transitive walk to replace this method.
chosen_resolve = await Get(
ChosenPythonResolve, ChosenPythonResolveRequest(request.addresses)
)
loaded_lockfile = await Get(
LoadedLockfile, LoadedLockfileRequest(chosen_resolve.lockfile)
)
pex_native_subsetting_supported = loaded_lockfile.is_pex_native
if loaded_lockfile.constraints_strings:
requirements = dataclasses.replace(
requirements, constraints_strings=loaded_lockfile.constraints_strings
)

should_return_entire_lockfile = (
python_setup.run_against_entire_lockfile and request.internal_only
Expand Down Expand Up @@ -520,6 +527,10 @@ async def create_pex_from_targets(
async def get_repository_pex(
request: _RepositoryPexRequest, python_setup: PythonSetup
) -> OptionalPexRequest:
# NB: It isn't safe to resolve against an entire lockfile or constraints file if
# platforms are in use. See https://github.com/pantsbuild/pants/issues/12222.
if request.platforms or request.complete_platforms:
return OptionalPexRequest(None)

interpreter_constraints = await Get(
InterpreterConstraints,
Expand Down Expand Up @@ -570,9 +581,7 @@ async def _setup_constraints_repository_pex(
global_requirement_constraints: GlobalRequirementConstraints,
) -> OptionalPexRequest:
request = constraints_request.repository_pex_request
# NB: it isn't safe to resolve against the whole constraints file if
# platforms are in use. See https://github.com/pantsbuild/pants/issues/12222.
if not python_setup.resolve_all_constraints or request.platforms or request.complete_platforms:
if not python_setup.resolve_all_constraints:
return OptionalPexRequest(None)

constraints_path = python_setup.requirement_constraints
Expand Down
32 changes: 22 additions & 10 deletions src/python/pants/backend/python/util_rules/pex_from_targets_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ class Project:
build_deps = ["setuptools==54.1.2", "wheel==0.36.2"]


setuptools_poetry_lockfile = """
setuptools_poetry_lockfile = r"""
# This lockfile was autogenerated by Pants. To regenerate, run:
#
# ./pants generate-lockfiles --resolve=setuptools
Expand Down Expand Up @@ -430,11 +430,16 @@ def test_exclude_requirements(
assert len(pex_request.requirements.req_strings) == (1 if include_requirements else 0)


def test_issue_12222(rule_runner: RuleRunner) -> None:
@pytest.mark.parametrize("enable_resolves", [False, True])
def test_cross_platform_pex_disables_subsetting(
rule_runner: RuleRunner, enable_resolves: bool
) -> None:
# See https://github.com/pantsbuild/pants/issues/12222.
lockfile = "3rdparty/python/default.lock"
constraints = ["foo==1.0", "bar==1.0"]
rule_runner.write_files(
{
"constraints.txt": "\n".join(constraints),
lockfile: "\n".join(constraints),
"a.py": "",
"BUILD": dedent(
"""
Expand All @@ -445,19 +450,26 @@ def test_issue_12222(rule_runner: RuleRunner) -> None:
),
}
)

if enable_resolves:
options = [
"--python-enable-resolves",
# NB: Because this is a synthetic lockfile without a header.
"--python-invalid-lockfile-behavior=ignore",
]
else:
options = [
f"--python-requirement-constraints={lockfile}",
"--python-resolve-all-constraints",
]
rule_runner.set_options(options, env_inherit={"PATH"})

request = PexFromTargetsRequest(
[Address("", target_name="lib")],
output_filename="demo.pex",
internal_only=False,
platforms=PexPlatforms(["some-platform-x86_64"]),
)
rule_runner.set_options(
[
"--python-requirement-constraints=constraints.txt",
"--python-resolve-all-constraints",
],
env_inherit={"PATH"},
)
result = rule_runner.request(PexRequest, [request])

assert result.requirements == PexRequirements(["foo"], constraints_strings=constraints)
Expand Down
15 changes: 12 additions & 3 deletions src/python/pants/backend/python/util_rules/pex_requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

from pants.backend.python.pip_requirement import PipRequirement
from pants.backend.python.subsystems.setup import InvalidLockfileBehavior, PythonSetup
from pants.backend.python.target_types import PythonRequirementsField
from pants.backend.python.target_types import PythonRequirementsField, parse_requirements_file
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
from pants.backend.python.util_rules.lockfile_metadata import (
InvalidPythonLockfileReason,
Expand Down Expand Up @@ -87,8 +87,11 @@ class LoadedLockfile:
requirement_estimate: int
# True if the loaded lockfile is in PEX's native format.
is_pex_native: bool
# If !is_pex_native, the lockfile parsed as constraints strings, for use when the lockfile
# needs to be subsetted (see #15031, ##12222).
constraints_strings: FrozenOrderedSet[str] | None
# The original file or file content (which may not have identical content to the output
# `lockfile_digest).
# `lockfile_digest`).
original_lockfile: Lockfile | LockfileContent


Expand Down Expand Up @@ -167,11 +170,16 @@ async def load_lockfile(
),
)
requirement_estimate = _pex_lockfile_requirement_count(lock_bytes)
constraints_strings = None
else:
header_delimiter = "#"
lock_string = lock_bytes.decode()
# Note: this is a very naive heuristic. It will overcount because entries often
# have >1 line due to `--hash`.
requirement_estimate = len(lock_bytes.decode().splitlines())
requirement_estimate = len(lock_string.splitlines())
constraints_strings = FrozenOrderedSet(
str(req) for req in parse_requirements_file(lock_string, rel_path=lockfile_path)
)

metadata: PythonLockfileMetadata | None = None
if should_validate_metadata(lockfile, python_setup):
Expand All @@ -188,6 +196,7 @@ async def load_lockfile(
metadata,
requirement_estimate,
is_pex_native,
constraints_strings,
original_lockfile=lockfile,
)

Expand Down

0 comments on commit f51f504

Please sign in to comment.