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

Do not use a repository-PEX if a PEX has platforms specified (cherrypick of #15031) #15034

Merged
merged 1 commit into from
Apr 7, 2022
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
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