Skip to content

Commit

Permalink
Improve cache reuse for ./pants package when using a constraints fi…
Browse files Browse the repository at this point in the history
…le or lockfile (pantsbuild#12807)

Improves upon the solution from pantsbuild#12076. There are some args like `--manylinux` that we need to use both with the lockfile.pex and the final `PexFromTargets`. But many others like `--strip-pex-env` are irrelevant for the lockfile.pex, and we only need to set on the final PEX.

WIth this diff:

```diff
diff --git a/build-support/bin/BUILD b/build-support/bin/BUILD
index d9751179b..2a6e76478 100644
--- a/build-support/bin/BUILD
+++ b/build-support/bin/BUILD
@@ -17,4 +17,4 @@ pex_binary(name="generate_docs", entry_point="generate_docs.py", dependencies=["
 pex_binary(name="generate_github_workflows", entry_point="generate_github_workflows.py")
 pex_binary(name="generate_user_list", entry_point="generate_user_list.py", dependencies=[":user_list_templates"])
 pex_binary(name="release_helper", entry_point="_release_helper.py")
-pex_binary(name="reversion", entry_point="reversion.py")
+pex_binary(name="reversion", entry_point="reversion.py", always_write_cache=True)
```

Before:

```
❯ ./pants --no-process-execution-local-cache --no-remote-cache-read --no-pantsd package build-support/bin:
...
23:50:18.32 [INFO] Completed: Installing 3rdparty/python/lockfiles/user_reqs.txt
23:50:18.32 [INFO] Completed: Installing 3rdparty/python/lockfiles/user_reqs.txt
```

After:

```
❯ ./pants --no-process-execution-local-cache --no-remote-cache-read --no-pantsd package build-support/bin:
...
23:49:14.07 [INFO] Completed: Installing 3rdparty/python/lockfiles/user_reqs.txt
...
```

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano committed Sep 9, 2021
1 parent 381db29 commit 42f4d0a
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 21 deletions.
17 changes: 9 additions & 8 deletions src/python/pants/backend/awslambda/python/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,20 @@ async def package_python_awslambda(
if (py_major, py_minor) == (2, 7):
platform += "u"

additional_pex_args = (
# Ensure we can resolve manylinux wheels in addition to any AMI-specific wheels.
"--manylinux=manylinux2014",
# When we're executing Pex on Linux, allow a local interpreter to be resolved if
# available and matching the AMI platform.
"--resolve-local-platforms",
)
pex_request = PexFromTargetsRequest(
addresses=[field_set.address],
internal_only=False,
main=None,
output_filename=output_filename,
platforms=PexPlatforms([platform]),
additional_args=[
# Ensure we can resolve manylinux wheels in addition to any AMI-specific wheels.
"--manylinux=manylinux2014",
# When we're executing Pex on Linux, allow a local interpreter to be resolved if
# available and matching the AMI platform.
"--resolve-local-platforms",
],
additional_args=additional_pex_args,
additional_lockfile_args=additional_pex_args,
)

lambdex_request = PexRequest(
Expand Down
15 changes: 11 additions & 4 deletions src/python/pants/backend/python/util_rules/pex_from_targets.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class PexFromTargetsRequest:
main: MainSpecification | None
platforms: PexPlatforms
additional_args: Tuple[str, ...]
additional_lockfile_args: Tuple[str, ...]
additional_requirements: Tuple[str, ...]
include_source_files: bool
additional_sources: Digest | None
Expand All @@ -77,6 +78,7 @@ def __init__(
main: MainSpecification | None = None,
platforms: PexPlatforms = PexPlatforms(),
additional_args: Iterable[str] = (),
additional_lockfile_args: Iterable[str] = (),
additional_requirements: Iterable[str] = (),
include_source_files: bool = True,
additional_sources: Digest | None = None,
Expand All @@ -101,6 +103,10 @@ def __init__(
interpreter constraints to not be used because platforms already constrain the valid
Python versions, e.g. by including `cp36m` in the platform string.
:param additional_args: Any additional Pex flags.
:param additional_lockfile_args: Any additional Pex flags that should be used with the lockfile.pex.
Many Pex args like `--emit-warnings` do not impact the lockfile, and setting them
would reduce reuse with other call sites. Generally, this should only be flags that
impact lockfile resolution like `--manylinux`.
:param additional_requirements: Additional requirements to install, in addition to any
requirements used by the transitive closure of the given addresses.
:param include_source_files: Whether to include source files in the built Pex or not.
Expand All @@ -123,6 +129,7 @@ def __init__(
self.main = main
self.platforms = platforms
self.additional_args = tuple(additional_args)
self.additional_lockfile_args = tuple(additional_lockfile_args)
self.additional_requirements = tuple(additional_requirements)
self.include_source_files = include_source_files
self.additional_sources = additional_sources
Expand Down Expand Up @@ -174,7 +181,7 @@ class _ConstraintsResolvedDistributionsRequest:
platforms: PexPlatforms
interpreter_constraints: InterpreterConstraints
internal_only: bool
additional_args: tuple[str, ...]
additional_lockfile_args: tuple[str, ...]


@rule(level=LogLevel.DEBUG)
Expand Down Expand Up @@ -234,7 +241,7 @@ async def pex_from_targets(request: PexFromTargetsRequest, python_setup: PythonS
request.platforms,
interpreter_constraints,
request.internal_only,
request.additional_args,
request.additional_lockfile_args,
),
)
resolved_dists = constraints_resolved_dists.maybe_resolved_dists
Expand Down Expand Up @@ -264,7 +271,7 @@ async def pex_from_targets(request: PexFromTargetsRequest, python_setup: PythonS
),
interpreter_constraints=interpreter_constraints,
platforms=request.platforms,
additional_args=request.additional_args,
additional_args=request.additional_lockfile_args,
),
)
requirements = dataclasses.replace(requirements, resolved_dists=resolved_dists)
Expand Down Expand Up @@ -356,7 +363,7 @@ async def _setup_constraints_repository_pex(
requirements=PexRequirements(all_constraints, apply_constraints=True),
interpreter_constraints=request.interpreter_constraints,
platforms=request.platforms,
additional_args=request.additional_args,
additional_args=request.additional_lockfile_args,
),
)
return _ConstraintsResolvedDistributions(resolved_dists)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ def get_pex_request(
*,
direct_deps_only: bool = False,
additional_args: Iterable[str] = (),
additional_lockfile_args: Iterable[str] = (),
) -> PexRequest:
args = ["--backend-packages=pants.backend.python"]
request = PexFromTargetsRequest(
Expand All @@ -193,6 +194,7 @@ def get_pex_request(
internal_only=True,
direct_deps_only=direct_deps_only,
additional_args=additional_args,
additional_lockfile_args=additional_lockfile_args,
)
if resolve_all_constraints is not None:
args.append(f"--python-setup-resolve-all-constraints={resolve_all_constraints!r}")
Expand All @@ -205,13 +207,10 @@ def get_pex_request(
assert OrderedSet(additional_args).issubset(OrderedSet(pex_request.additional_args))
return pex_request

additional_args = ["--no-strip-pex-env"]
additional_args = ["--strip-pex-env"]
additional_lockfile_args = ["--no-strip-pex-env"]

pex_req1 = get_pex_request(
"constraints1.txt",
resolve_all_constraints=False,
additional_args=additional_args,
)
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], apply_constraints=True
)
Expand All @@ -225,6 +224,7 @@ def get_pex_request(
"constraints1.txt",
resolve_all_constraints=True,
additional_args=additional_args,
additional_lockfile_args=additional_lockfile_args,
)
pex_req2_reqs = pex_req2.requirements
assert isinstance(pex_req2_reqs, PexRequirements)
Expand All @@ -241,6 +241,7 @@ def get_pex_request(
resolve_all_constraints=True,
direct_deps_only=True,
additional_args=additional_args,
additional_lockfile_args=additional_lockfile_args,
)
pex_req2_reqs = pex_req2_direct.requirements
assert isinstance(pex_req2_reqs, PexRequirements)
Expand All @@ -249,9 +250,7 @@ def get_pex_request(
assert not info(rule_runner, pex_req2_reqs.resolved_dists.pex)["strip_pex_env"]

pex_req3_direct = get_pex_request(
"constraints1.txt",
resolve_all_constraints=True,
direct_deps_only=True,
"constraints1.txt", resolve_all_constraints=True, direct_deps_only=True
)
pex_req3_reqs = pex_req3_direct.requirements
assert isinstance(pex_req3_reqs, PexRequirements)
Expand Down

0 comments on commit 42f4d0a

Please sign in to comment.