Skip to content

Commit

Permalink
Make VIRTUAL_ENV not end with "/" as same as normal virtualenv activa…
Browse files Browse the repository at this point in the history
…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 #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 "/".
  • Loading branch information
flying-foozy committed Nov 13, 2019
1 parent c3da41a commit 3c67b10
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
2 changes: 2 additions & 0 deletions test/virtualenvwrapper-test.el
Expand Up @@ -50,6 +50,8 @@
(should (s-contains? venv-tmp-env (getenv "PATH")))
;; we set VIRTUAL_ENV for jedi and whoever else needs it
(should (s-contains? venv-tmp-env (getenv "VIRTUAL_ENV")))
;; VIRTUAL_ENV does not end with "/"
(should (not (s-ends-with? "/" (getenv "VIRTUAL_ENV"))))
;; we add our dir to exec-path
(should (s-contains? venv-tmp-env (car exec-path))))

Expand Down
4 changes: 2 additions & 2 deletions virtualenvwrapper.el
Expand Up @@ -242,7 +242,7 @@ prompting the user with the string PROMPT"
(setenv "PATH" path)
;; keep eshell path in sync
(setq eshell-path-env path))
(setenv "VIRTUAL_ENV" venv-current-dir)
(setenv "VIRTUAL_ENV" (s-chop-suffix "/" venv-current-dir))
(if venv-workon-cd
(venv--switch-to-project-dir))
(venv--set-venv-gud-pdb-command-name)
Expand Down Expand Up @@ -537,7 +537,7 @@ virtualenvwrapper.el."
ad-do-it
(venv-shell-init buffer-name)
(setenv "PATH" (concat venv-current-dir venv-executables-dir path-separator (getenv "PATH")))
(setenv "VIRTUAL_ENV" venv-current-dir)))))
(setenv "VIRTUAL_ENV" (s-chop-suffix "/" venv-current-dir))))))
(ad-activate 'shell))


Expand Down

0 comments on commit 3c67b10

Please sign in to comment.