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

Second Try: Environment: improve/fix getting sys.prefix #1176

Merged
merged 2 commits into from
Jul 15, 2018
Merged

Conversation

davidhalter
Copy link
Owner

@davidhalter davidhalter commented Jul 15, 2018

Instead of #1108 I propose this change. It involves pretty much all of what @blueyed did there, but with a different approach about creating subprocesses. I think I cleaned up a mistake we made.

The issue was that if something changed about the environment (e.g. version
switch) or sys.path change, re-creating the environment was possible, but did
not involve the change. The environments have now a __del__ function that
deletes the subprocess after every time an Environment is garbage collected.

blueyed and others added 2 commits July 10, 2018 12:46
It only takes `executable` and gets all the information from the
subprocess directly.

Fixes #1107.
The issue was that if something changed about the environment (e.g. version
switch) or sys.path change, re-creating the environment was possible, but did
not involve the change. The environments have now a __del__ function that
deletes the subprocess after every time an Environment is garbage collected.
@davidhalter davidhalter changed the title Env Second Try: Environment: improve/fix getting sys.prefix Jul 15, 2018
@blueyed
Copy link
Contributor

blueyed commented Jul 15, 2018

LGTM, rebasing on master should fix AppVeyor then (py33), no?

@davidhalter
Copy link
Owner Author

The PR passed, it's only the branch that failed, so I guess it was fixed by rebasing it.

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.

2 participants