-
-
Notifications
You must be signed in to change notification settings - Fork 289
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
Cleanup bootstrap dependencies before handoff. #542
Conversation
Now with full test coverage for all for terminal entry points to executing user code. It's probably worth noting the new PEX_VERBOSE=1 output:
|
pex/pex.py
Outdated
@@ -415,6 +410,40 @@ def _execute(self): | |||
TRACER.log('No entry point specified, dropping into interpreter') | |||
return self.execute_interpreter() | |||
|
|||
@classmethod | |||
@contextmanager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any point to keeping this as a contextmanager
if it doesn't snapshot and restore prior sys.path
state? as a caller, I'd expect the contextmgr to undo any mutations vs purely apply them and do nothing upon context block exit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. I did start off re-jiggering in finally until I realized it was not needed. I'll fix. The patch_sys
has this problem too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth another look - I stripped the contextmanagers from this class. Only one tried to restore anything (patch_pkg_resources
) and the restoration was not needed for the pex handling of teardown.
OK - rebased past conflicts in master. PTAL at 457bc90. |
Previously we polluted the third party import space with any third party dependencies of the pex bootstrap code (currently a subset of setuptools). We now unimport all third_party code in bootstrap and place bootstrap at the end of `sys.path` before handing off to user code to allow their custom versions of overlapping third party code to be imported. Added a failing test that the demoted_bootstrap code now fixes. Fixes pex-tool#437.
We had a few of this operating in the leadup to hand-off to user code.
Another rebase past conflicts in master @ a972cb2. I'll submit by end of day unless there are objections. |
Previously we polluted the third party import space with any third
party dependencies of the pex bootstrap code (currently a subset of
setuptools). We now unimport all third_party code in bootstrap and
place bootstrap at the end of
sys.path
before handing off to usercode to allow their custom versions of overlapping third party code
to be imported.
Added a failing test that the demoted_bootstrap code now fixes.
Fixes #437.