Skip to content

Commit

Permalink
Avoid passing PEX_MAX_INSTALL_JOBS incorrectly to PEX processes (#20795)
Browse files Browse the repository at this point in the history
This fixes/removes the use of `PEX_MAX_INSTALL_JOBS` when running a PEX.
This was added in #20670 in an attempt to sync with
`--no-pre-install-wheels` and do more work in parallel when booting
"internal" PEXes... but it was implemented incorrectly. This would need
to be set to `PEX_MAX_INSTALL_JOBS={pants_concurrency}` or similar, and:

- that substitution isn't currently supported (only argv substitutions)
- it's somewhat unclear if we even want to do that at all, as it'll
result in more cache misses

Noticed in
#20670 (comment)
  • Loading branch information
huonw authored Apr 17, 2024
1 parent 3d7160a commit 76bcd4e
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 12 deletions.
3 changes: 1 addition & 2 deletions src/python/pants/backend/python/goals/publish_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ def test_twine_upload(rule_runner, packages) -> None:
{
"TWINE_USERNAME": "whoareyou",
"TWINE_PASSWORD": "secret",
"PEX_MAX_INSTALL_JOBS": "0",
}
),
),
Expand All @@ -162,7 +161,7 @@ def test_twine_upload(rule_runner, packages) -> None:
"my-package-0.1.0.tar.gz",
"my_package-0.1.0-py3-none-any.whl",
),
env=FrozenDict({"TWINE_USERNAME": "whoami", "PEX_MAX_INSTALL_JOBS": "0"}),
env=FrozenDict({"TWINE_USERNAME": "whoami"}),
),
)

Expand Down
11 changes: 1 addition & 10 deletions src/python/pants/backend/python/util_rules/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -1163,9 +1163,6 @@ async def setup_pex_process(request: PexProcess, pex_environment: PexEnvironment
complete_pex_env = pex_environment.in_sandbox(working_directory=request.working_directory)
argv = complete_pex_env.create_argv(pex.name, *request.argv)
env = {
# Set this in case this PEX was built with --no-pre-install-wheels, and thus parallelising
# the install on cold boot is handy.
"PEX_MAX_INSTALL_JOBS": str(request.concurrency_available),
**complete_pex_env.environment_dict(python=pex.python),
**request.extra_env,
}
Expand Down Expand Up @@ -1261,12 +1258,6 @@ async def setup_venv_pex_process(
else venv_pex.pex.argv0
)
argv = (pex_bin, *request.argv)
env = {
# Set this in case this PEX was built with --no-pre-install-wheels, and thus parallelising
# the install on cold boot is handy.
"PEX_MAX_INSTALL_JOBS": str(request.concurrency_available),
**request.extra_env,
}
input_digest = (
await Get(Digest, MergeDigests((venv_pex.digest, request.input_digest)))
if request.input_digest
Expand All @@ -1285,7 +1276,7 @@ async def setup_venv_pex_process(
level=request.level,
input_digest=input_digest,
working_directory=request.working_directory,
env=env,
env=request.extra_env,
output_files=request.output_files,
output_directories=request.output_directories,
append_only_caches=append_only_caches,
Expand Down

0 comments on commit 76bcd4e

Please # to comment.