Cleanup sys.path after __pex__ is imported
#1959
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsirois
approved these changes
Oct 19, 2022
pex/pex_bootstrapper.py
Outdated
| VendorImporter.install( | ||
| uninstallable=False, prefix="__pex__", path_items=["."], root=location | ||
| ) | ||
| uninstall() |
There was a problem hiding this comment.
It would be nice if pex.pex.PEX and this bit here shared a utility function, but I can also circle back and correct that if I'm motivated enough to put my money where my mouth is.
Either way, the activate, import, install, uninstall+demote is a finely ordered thing IIUC. In particular that Bootstrap and uninstall being imported when they are. If so, and you can shed light in a comment - great. If not, that's fine. This is hopefully pretty obviously fragile code.
There was a problem hiding this comment.
Unfortunately I cannot shed light as I have a very poor understanding of the underlying code. Feel free to edit the PR with a comment that you think is best.
There was a problem hiding this comment.
Yeah, I don't even know. I'll need to take time and experiment to come up with an explanation. I can do that after this merges.
This fixes pex-tool#1954 by ensuring all vendored code is uninstalled from the path and bootstrap code is demoted to the end of `sys.path` after `__pex__` is imported. Fixes: pex-tool#1954
zmanji
force-pushed
the
investigate-pex-bootstrap-leak
branch
from
d97a39a
to
90dfb25
Compare
This gets ahead of the deprecation announced here: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/
…tool#1961) Change `atomic_directory` to always grab an exclusive lock and use a stable work directory per target directory to surface multiple lock owners as up-front warnings instead of possibly slow corruptions.
This allows experimenting with making all Pex file locks use BSD style locks to help debug file locking issues. This environment variable knob can be removed at any time Pex sees fit.
|
@zmanji I moved this back a release and I'll just keep bumping until you have time to circle back and get CI green here. |
I'm stuck as to why there are failures on Python 2.7. A bunch of attempts locally have resulted in failure. I don't need this so this PR might stagnate. |
This was broken by pex-tool#1961 which did not account for the two unlocked uses of AtomicDirectory by the pex.resolver for wheel building and installing. Although those directories deserve a second look as candidates for exclusive locking, we have seen no known issues with those directories since introduction of AtomicDirectory up until pex-tool#1961. As such, this change just restores the prior behavior for those two cases. Fixes pex-tool#1968
…sh. (pex-tool#1972) * map pex python path interpreter to realpath when checking if the venv_dir hash should include an intepreter key * Update other vendor files. replace logic to clean _PIP in memory cache during test.
This was all accounted for with `--no-build-isolation` and tox venv pinning with an extensive comment about all this to boot. For some reason unclear to me, I undid `--no-build-isolation` in pex-tool#1661 though. This just re-instates that flag and re-runs vendoring to get back to stable wheel versions.
The googleapis-common-protos 1.57.0 release on 11/15 breaks the test_issue_729 test_undeclared_setuptools_import_on_pex_path test. Pin behind that release to fix CI on main.
…ter mode (pex-tool#1984) Fixes pex-tool#1983. Note that this PR doesn't revert the code to use the prior code (`sys.path.extend(os.environ.get("PEX_EXTRA_SYS_PATH", "").split(os.pathsep))`) as that might still add `""` when `PEX_EXTRA_SYS_PATH` is not in the environment.
…ex-tool#1991) This makes --resolve-local-platforms work better with --complete-platforms: if there's an external complete platform specified for a platform that matches a local interpreter, the local interpreter's complete platform is checked against that external complete platform. If the local interpreter supports any additional tags, the local interpreter is not used. If the local interpreter/environment is newer (supports more tags), the old behaviour would risk including wheels that aren't compatible with the external complete platform. That is, execution will fail, see pantsbuild/pants#17621 for an example. Fixes pex-tool#1899
Several issues are fixed: + IPython can inject ~PS1-style prefixes depending on the IPython version and Python version in-play. Avoid having to worry about this with a dedicated test communication channel separate from stdio. + The PyPy 7.3.12 release includes HPy in the stdlib (including its dist-info) which highlights a bug in `Virtualenv` when iterating venv distributions; so we stick to iterating distributions found in the site-packages directories. + The setup-python action no longer supports Python 2.7; so we switch to pyenv for that. + A test involving `jaraco-collections==3.5.1` is broken by the recent pydantic 2.x release; so we bound low.
Previously namespace packages were handled only amongst 3rd-party deps. Any ns-package also split across PEX user sources would lead to one of the 3rd-party dep members of the namespace package getting its cached wheel install contaminated with the PEX user source that was a member of the ns-package. Fixes pex-tool#2160
This re-uses the scie-pants & a-scie infra and converts CHANGES.rst to CHANGES.md so that infra can be used as-is.
This also expands scope to cover all deprecated (or soon-to-be) interpreters we use since GitHub has now proved they do yank support eventually.
The crux here is supporting a version of Pip that works in 3.12. There is no such released version yet; so this change adds an unreleased Pip version but goes to some length to hide this version from users and make it only activatable by those in the know / CI. What follows is fixing or adjusting many tests. The result is Pex known to work with Python 3.12 ahead of its release by several months and the spectre of Pex 3 / a Pex branch split, etc., being forced by Python 3.12 support dispelled. It turns out Pex can still ship supporting Python 2.7, 3.5, etc. along side supporting 3.12. The main trick here is to use `python3.12 -mvenv` to spirit up a bootstrap Pip that works at least enough to install the unreleased Pip that truly works with Python 3.12. Previously all Pip version bootstrapping was handled exclusively by the vendored Pip.
A recent Pip commit has broken `pip --use-deprecated legacy-resolver download ...` which is our backwards-compatible default use case. Whether or not Pip decides to fix, the legacy resolver will be yanked at some point and it seems reasonable to just require any Pex user on Python 3.12 and thus Pip 23.2 to only use the Pip 2020 resolver. See: pypa/pip#12138
Pip 23.2 is both the latest release and the only release that supports Python 3.12. The Pip 23.2 release notes are here: https://pip.pypa.io/en/stable/news/#v23-2 Fixes pex-tool#2173
This was an oversight in the commits that landed Pip 23.2 / Python 3.12 support.
CI uses `tox -e py312-pip23_2-integration`, but for local dev runs `tox -e py312-integration` is more typical. Get this green by ensuring the reproducible build tests use a consistent Pip version since `pip wheel` has varying output for certain metadata files (line order and whitespace vary) across Pip / Setuptools / Wheel versions.
Previously the PEX bootstrap code and third party vendored code were not demoted and uninstalled respectively like they are for normal PEX execution. Fixes pex-tool#2183
This complements the existing `-D` / `--sources-directory` support for adding local sources and resources with finer-grained control over what files are included in the PEX file. Notably, this allows cleanly packaging projects with no `setup.py` / `pyproject.toml` based build when the projects have their Python code at the top level mixed with other files that should not be included in the PEX (e.g.: build scripts, CI configuration, documentation, etc.). Fixes pex-tool#2134
|
Ok, I finally circled back to this since the bootstrap demotion just got centralized in #2184. The CPython 2.7 failure took a while to pinpoint a cause for. I still don't understand why only CPython 2.7 has an issue with un-importing |
|
Hrm, that's ugly. Let me try something else... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes #1954 by ensuring all vendored code is uninstalled from the path and bootstrap code is demoted to the end of
sys.pathafter__pex__is imported.Fixes: #1954