Skip to content

Commit

Permalink
Fix default layout and use of PEX-repositories for legacy lockfiles (…
Browse files Browse the repository at this point in the history
…cherrypick of #15014) (#15032)

As reported in #14980 and #14991: 1) the default layout of non-internal PEXes changed to `PACKED`, and 2) the PEX-repository subsetting optimization was disabled for manual or poetry lockfiles.

This change fixes those issues, and adds a test of the `pex_from_targets` codepath which decides which resolution strategy to use. But it also fairly significantly refactors the `PexRequest` consumption code to improve typesafety. In particular:
* Replaces the `is_all_constraints_resolve` special case with explicitly specified `PexLayout`s for those `PexRequests` (which would have been harder at the time when the special case was originally added, since `PexRequest.layout` didn't exist yet).
* Moves lockfile header parsing and PEX-native-detection into a `@rule` which produces a `LoadedLockfile`.
* Moves "subsetting of a lockfile or a repository PEX" to the `PexRequirements.from_subset` field, which takes a `LoadedLockfile` and asserts (not worth introducing another type yet probably) that the lockfile is PEX-native.
* Move the requirements which used to be attached to the `Lockfile`/`LockfileContents` off onto an `PexRequest.requirements = EntireLockfile` case, to assert that they are optional, and only used for metadata validation (rather than subsetting).

Fixes #14980 and fixes #14991.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
stuhood authored Apr 7, 2022
1 parent 0b411e1 commit 232293c
Show file tree
Hide file tree
Showing 9 changed files with 880 additions and 638 deletions.
27 changes: 14 additions & 13 deletions src/python/pants/backend/python/subsystems/python_tool_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
from pants.backend.python.util_rules.pex import PexRequest
from pants.backend.python.util_rules.pex_requirements import (
EntireLockfile,
PexRequirements,
ToolCustomLockfile,
ToolDefaultLockfile,
Expand All @@ -21,7 +22,6 @@
from pants.option.option_types import StrListOption, StrOption
from pants.option.subsystem import Subsystem
from pants.util.docutil import bin_name, doc_url
from pants.util.ordered_set import FrozenOrderedSet


class PythonToolRequirementsBase(Subsystem):
Expand Down Expand Up @@ -125,7 +125,7 @@ def pex_requirements(
self,
*,
extra_requirements: Iterable[str] = (),
) -> PexRequirements | ToolDefaultLockfile | ToolCustomLockfile:
) -> PexRequirements | EntireLockfile:
"""The requirements to be used when installing the tool.
If the tool supports lockfiles, the returned type will install from the lockfile rather than
Expand All @@ -139,28 +139,29 @@ def pex_requirements(

hex_digest = calculate_invalidation_digest(requirements)

lockfile: ToolDefaultLockfile | ToolCustomLockfile
if self.lockfile == DEFAULT_TOOL_LOCKFILE:
assert self.default_lockfile_resource is not None
return ToolDefaultLockfile(
lockfile = ToolDefaultLockfile(
file_content=FileContent(
f"{self.options_scope}_default.lock",
importlib.resources.read_binary(*self.default_lockfile_resource),
),
lockfile_hex_digest=hex_digest,
req_strings=FrozenOrderedSet(requirements),
resolve_name=self.options_scope,
uses_project_interpreter_constraints=(not self.register_interpreter_constraints),
uses_source_plugins=self.uses_requirements_from_source_plugins,
)
return ToolCustomLockfile(
file_path=self.lockfile,
file_path_description_of_origin=f"the option `[{self.options_scope}].lockfile`",
lockfile_hex_digest=hex_digest,
req_strings=FrozenOrderedSet(requirements),
resolve_name=self.options_scope,
uses_project_interpreter_constraints=(not self.register_interpreter_constraints),
uses_source_plugins=self.uses_requirements_from_source_plugins,
)
else:
lockfile = ToolCustomLockfile(
file_path=self.lockfile,
file_path_description_of_origin=f"the option `[{self.options_scope}].lockfile`",
lockfile_hex_digest=hex_digest,
resolve_name=self.options_scope,
uses_project_interpreter_constraints=(not self.register_interpreter_constraints),
uses_source_plugins=self.uses_requirements_from_source_plugins,
)
return EntireLockfile(lockfile, complete_req_strings=tuple(requirements))

@property
def lockfile(self) -> str:
Expand Down
8 changes: 2 additions & 6 deletions src/python/pants/backend/python/util_rules/dists.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess
from pants.backend.python.util_rules.pex import rules as pex_rules
from pants.backend.python.util_rules.pex_requirements import (
Lockfile,
LockfileContent,
PexRequirements,
)
from pants.backend.python.util_rules.pex_requirements import EntireLockfile, PexRequirements
from pants.base.glob_match_error_behavior import GlobMatchErrorBehavior
from pants.engine.fs import (
CreateDigest,
Expand Down Expand Up @@ -62,7 +58,7 @@ class BuildSystemRequest:
class BuildSystem:
"""A PEP 517/518 build system configuration."""

requires: PexRequirements | Lockfile | LockfileContent
requires: PexRequirements | EntireLockfile
build_backend: str

@classmethod
Expand Down
Loading

0 comments on commit 232293c

Please sign in to comment.