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.

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

Fix --venv PEX PEX_EXTRA_SYS_PATH propagation. #1837

Merged
merged 1 commit into from
Jul 12, 2022

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Jul 11, 2022

Previously this used PYTHONPATH for propagation, which could lead to
issues executing foreign Pythons in subprocesses. Switch to using a
.pth file. Although the problem of polluting foreign Pythons in
subprocesses remains for zipapp PEXes, it is the case that --venv is
Pex's maximum compatibility mode and this can be viewed as yet another
case where a Pex feature breaking a Python app should be solved by using
--venv mode.

Fixes #1836

Previously this used `PYTHONPATH` for propagation, which could lead to
issues executing foreign Pythons in subprocesses. Switch to using a
`.pth` file. Although the problem of polluting foreign Pythons in
subprocesses remains for zipapp PEXes, it is the case that `--venv` is
Pex's maximum compatibility mode and this can be viewed as yet another
case where a Pex feature breaking a Python app should be solved by using
`--venv` mode.

Fixes pex-tool#1836
@jsirois jsirois requested review from benjyw and thejcannon July 11, 2022 17:21
@jsirois
Copy link
Member Author

jsirois commented Jul 11, 2022

N.B.: The pre-existing tests from #989 and #1832 vet this. In particular, the following test added in #1832 brought to light the issue with PEX* scrubbing / led to the use of __PEX_EXTRA_SYS_PATH__ in that case:
https://github.com/pantsbuild/pex/blob/4aecc86263ccf8d27630ef761335591ab5100759/tests/integration/test_issue_1825.py#L114-L133

This test failed in assert_inherited in VENV mode.

@jsirois
Copy link
Member Author

jsirois commented Jul 11, 2022

I did verify that this Pex causes ./build-support/bin/release.sh build-wheels to go green over in https://github.com/pantsbuild/pants.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PEX_EXTRA_SYS_PATH propagation can break subprocesses run against other venvs.
2 participants