Skip to content

Commit

Permalink
Use --layout=packed for all monolithic resolves. (cherrypick of #13400
Browse files Browse the repository at this point in the history
) (#13403)

As described in #13398, we should use `--layout=packed` for all monolithic resolves (lockfiles, all_constraints, etc), in addition to for `internal_only` resolves.

Note that this is _not_ sufficient to allow for cache reuse across internal PEXes (i.e. those created for tests) and external PEXes (`pex_binary`), because the `repository.pex` is constructed with other differing arguments. 

Fixes #13398.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
stuhood authored Oct 28, 2021
1 parent 35f2c8f commit 1179b6b
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 2 deletions.
12 changes: 11 additions & 1 deletion src/python/pants/backend/python/util_rules/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,19 @@ class ToolCustomLockfile(Lockfile, _ToolLockfileMixin):
class PexRequirements:
req_strings: FrozenOrderedSet[str]
apply_constraints: bool
# TODO: The constraints.txt resolve for `resolve_all_constraints` will be removed as part of
# #12314, but in the meantime, it "acts like" a lockfile, but isn't actually typed as a Lockfile
# because the constraints are modified in memory first. This flag marks a `PexRequirements`
# resolve as being a request for the entire constraints file.
is_all_constraints_resolve: bool
repository_pex: Pex | None

def __init__(
self,
req_strings: Iterable[str] = (),
*,
apply_constraints: bool = False,
is_all_constraints_resolve: bool = False,
repository_pex: Pex | None = None,
) -> None:
"""
Expand All @@ -121,6 +127,7 @@ def __init__(
"""
self.req_strings = FrozenOrderedSet(sorted(req_strings))
self.apply_constraints = apply_constraints
self.is_all_constraints_resolve = is_all_constraints_resolve
self.repository_pex = repository_pex

@classmethod
Expand Down Expand Up @@ -423,6 +430,7 @@ async def build_pex(
)

if isinstance(request.requirements, Lockfile):
is_monolithic_resolve = True
argv.extend(["--requirement", request.requirements.file_path])
argv.append("--no-transitive")
globs = PathGlobs(
Expand All @@ -444,6 +452,7 @@ async def build_pex(
requirements_file_digest = await Get(Digest, PathGlobs, globs)

elif isinstance(request.requirements, LockfileContent):
is_monolithic_resolve = True
file_content = request.requirements.file_content
argv.extend(["--requirement", file_content.path])
argv.append("--no-transitive")
Expand All @@ -458,6 +467,7 @@ async def build_pex(
requirements_file_digest = await Get(Digest, CreateDigest([file_content]))
else:
assert isinstance(request.requirements, PexRequirements)
is_monolithic_resolve = request.requirements.is_all_constraints_resolve

if (
request.requirements.apply_constraints
Expand Down Expand Up @@ -491,7 +501,7 @@ async def build_pex(

output_files: Iterable[str] | None = None
output_directories: Iterable[str] | None = None
if request.internal_only:
if request.internal_only or is_monolithic_resolve:
# This is a much friendlier layout for the CAS than the default zipapp.
argv.extend(["--layout", "packed"])
output_directories = [request.output_filename]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,12 @@ async def _setup_constraints_repository_pex(
description=f"Resolving {constraints_path}",
output_filename="repository.pex",
internal_only=request.internal_only,
requirements=PexRequirements(all_constraints, apply_constraints=True),
requirements=PexRequirements(
all_constraints,
apply_constraints=True,
# TODO: See PexRequirements docs.
is_all_constraints_resolve=True,
),
interpreter_constraints=request.interpreter_constraints,
platforms=request.platforms,
additional_args=request.additional_lockfile_args,
Expand Down
16 changes: 16 additions & 0 deletions src/python/pants/backend/python/util_rules/pex_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,22 @@ def test_resolves_dependencies(rule_runner: RuleRunner) -> None:
)


@pytest.mark.parametrize("is_all_constraints_resolve", [True, False])
@pytest.mark.parametrize("internal_only", [True, False])
def test_use_packed_pex_requirements(
rule_runner: RuleRunner, is_all_constraints_resolve: bool, internal_only: bool
) -> None:
requirements = PexRequirements(
["six==1.12.0"], is_all_constraints_resolve=is_all_constraints_resolve
)
pex_data = create_pex_and_get_all_data(
rule_runner, requirements=requirements, internal_only=internal_only
)
# If this is either internal_only, or an all_constraints resolve, we should use packed.
should_use_packed = is_all_constraints_resolve or internal_only
assert (not pex_data.is_zipapp) == should_use_packed


def test_requirement_constraints(rule_runner: RuleRunner) -> None:
direct_deps = ["requests>=1.0.0,<=2.23.0"]

Expand Down

0 comments on commit 1179b6b

Please sign in to comment.