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

Make VIRTUAL_ENV not end with "/" as same as normal virtualenv activation #73

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

flying-foozy
Copy link

This commit improves compatibility of VIRTUAL_ENV value against normal virtualenv activation.

…tion

If a virtualenv is normally activated, environment variable
VIRTUAL_ENV does not end with "/", and its value is equal to
sys.prefix in the python process.

On the other hand, virtualenvwrapper.el before this commit makes
VIRTUAL_ENV end with "/". Therefore, its value is different from
sys.prefix in the python process spawned from Emacs.

VIRTUAL_ENV ending with "/" might cause issues. For example, Jedi
https://github.com/davidhalter/jedi stops working, in such case.

Jedi creates PIPE-ed child process at each completion requests or so,
and keeps them opened, if VIRTUAL_ENV is not equal to sys.prefix in
child process. Finally, it stops working soon, because of EMFILE ("Too
many open files").

Though child process management of Jedi may have to be more efficient,
virtualenvwrapper.el also have to make VIRTUAL_ENV not end with "/",
IMHO, because normal virtualenv activation does so, and the format of
environment variable is a kind of API for inter process co-operation.

According to commit f7c97e2 ("ensure venv-location has slash on end")
for issue porterjamesj#51, there are some code paths, which expect
venv-current-dir to end with "/". Therefore, this commit eliminates
tail "/" of venv-current-dir at each assigning non-nil value to
VIRTUAL_ENV, instead of making venv-current-dir not end with "/".
@flying-foozy
Copy link
Author

Oops, I overlooked behinding from recent master head, sorry.
I re-pushed rebased commit.

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.

1 participant