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.

Sign up for GitHub

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

Jump to bottom

The pex cache (PEX_ROOT) is not always controllable. #926

Closed
jsirois opened this issue Mar 21, 2020 · 1 comment · Fixed by #929
Closed

The pex cache (PEX_ROOT) is not always controllable. #926

jsirois opened this issue Mar 21, 2020 · 1 comment · Fixed by #929
Assignees
@jsirois
Labels
bug

Comments

@jsirois
Copy link
Member

jsirois commented Mar 21, 2020 •
edited
Loading

In general, there is cruft here, including the confusing existence of --cache-dir and --pex-root at build-time, in addition to PEX_ROOT at both build-time and run-time, and, finally PexInfo.pex_root at run-time. The behavior of PEX with combinations of these settings is not currently well defined, but there is also at least one concrete case where the pex root cannot be controlled, in a Pex PEX. This case is of importance for Pants v2 which uses PEXed Pex to build PEXes.

On PEX startup, all PEX_ env vars are stripped before handing off control to the underlying application:
https://github.com/pantsbuild/pex/blob/73ce2f4b4fdc1fe3ad76779c06f0b9a25f443384/pex/pex.py#L416-L419
https://github.com/pantsbuild/pex/blob/73ce2f4b4fdc1fe3ad76779c06f0b9a25f443384/pex/pex.py#L45-L54

When that application is itself Pex, this takes away PEX_ROOT as a means to control the pex root for PEXed Pex build-time. Whether or not some affordance should be made for passing PEX_ env vars to Pex when PEXed is tracked in #925. As a result, any direct uses of PEX_ROOT instead of --pex-root or --cache-dir will use the default PEX_ROOT of ~/.pex. We have three of these:

  1. https://github.com/pantsbuild/pex/blob/128c03de866c3f0d667a97524aade5b05faad8a5/pex/interpreter.py#L337
  2. https://github.com/pantsbuild/pex/blob/bf66b72a9aa467b1bfd6ae6f07d1b679733aaaa5/pex/third_party/__init__.py#L362
  3. https://github.com/pantsbuild/pex/blob/91159b34a8c88f8788df41aba2d09cd2938a67f8/pex/pip.py#L56-L82

Item 3 is easy to fix to respect cache if passed - this appears to be a bug. Items 1 & 2 are a bit harder to fix:

  1. PythonInterpreter is used at both build-time and run-time and even in the public API.
  2. The isolated API is used by on-demand pex.third_party imports.

Fixing item 3 may be enough to solve the 1 known controllability issue, but it would be good to solve for all 3 cases by addressing #925 eventually.
@jsirois jsirois added the bug label Mar 21, 2020
@jsirois jsirois self-assigned this Mar 21, 2020
This was referenced Mar 21, 2020
Closed
Closed
Merged
@jsirois
Copy link
Member Author

jsirois commented Mar 23, 2020

The work by @yelly in #780 is relevant here as are the issues raised in #746 and #816.

@jsirois jsirois mentioned this issue Mar 23, 2020
Merged
jsirois added a commit that referenced this issue Mar 23, 2020
@yelly @jsirois
Adding --runtime-pex-root option. (#780)
4914a60 
Adding the option to set the PEX root in in built PEXes.

The option already existed for use from the PexInfo API but was not
exposed to the CLI.

Work towards #746
Work towards #926

Co-authored-by: John Sirois <[email protected]>
jsirois added a commit to jsirois/pex that referenced this issue Mar 24, 2020
@jsirois
TODO:
e6f31ac 
1. A good commit message is needed.
2. Tests.

Fixes pex-tool#746
Fixes pex-tool#816
Fixes pex-tool#926
jsirois added a commit to jsirois/pex that referenced this issue Mar 24, 2020
@jsirois
Fix PEX_* env stripping and allow turning off.
7a5c89f 
Previously, `PEX_*` environment variable stripping incorrectly stripped
`PEX_*` environment variables in host process when the `PEX.run` API was
used. This is fixed and an option to not strip pex environment variables
at all is added to `PexInfo` / the Pex CLI. This new option is used to
publish a Pex PEX that supports control via environment variables.

Fixes pex-tool#180
Fixes pex-tool#925
Work towards pex-tool#926
@jsirois jsirois mentioned this issue Mar 24, 2020
Merged
jsirois added a commit that referenced this issue Mar 24, 2020
@jsirois
Fix PEX_* env stripping and allow turning off. (#932)
1c6ae63 
* Fix `PEX_*` env stripping and allow turning off.

Previously, `PEX_*` environment variable stripping incorrectly stripped
`PEX_*` environment variables in host process when the `PEX.run` API was
used. This is fixed and an option to not strip pex environment variables
at all is added to `PexInfo` / the Pex CLI. This new option is used to
publish a Pex PEX that supports control via environment variables.

Fixes #180
Fixes #925
Work towards #926

* Fix Python versionisms.

* Explain --no-strip-pex-env more.
jsirois added a commit to jsirois/pex that referenced this issue Mar 25, 2020
@jsirois
Sanitize PEX_ROOT handling.
f2440b7 
In the past, portions of the pex cache could be controlled individually
but this was no longer the case in practice nor was it desirable. Unify
all cache handling under the PEX_ROOT and deprecate `--cache-dir` to
codify the new single pex cache as the PEX_ROOT.

Also add best-effort support for always finding a a viable PEX_ROOT even
when the configured value is not writeable. In conjunction with
`--runtime-pex-root` this provides for both a Pex CLI and created PEXes
that should always work wrt having a viable PEX_ROOT but still display a
warning if a suboptimal alternate had to be used.

Fixes pex-tool#746
Fixes pex-tool#816
Fixes pex-tool#926
jsirois added a commit that referenced this issue Mar 25, 2020
@jsirois
Sanitize PEX_ROOT handling. (#929)
7365e82 
In the past, portions of the pex cache could be controlled individually
but this was no longer the case in practice nor was it desirable. Unify
all cache handling under the PEX_ROOT and deprecate `--cache-dir` to
codify the new single pex cache as the PEX_ROOT.

Also add best-effort support for always finding a a viable PEX_ROOT even
when the configured value is not writeable. In conjunction with
`--runtime-pex-root` this provides for both a Pex CLI and created PEXes
that should always work wrt having a viable PEX_ROOT but still display a
warning if a suboptimal alternate had to be used.

Fixes #746
Fixes #816
Fixes #926
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jsirois jsirois

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Sanitize PEX_ROOT handling. jsirois/pex
1 participant
@jsirois