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

duplicate path in python-virtualenv #332

Open
dotysan opened this issue May 8, 2023 · 1 comment
Open

duplicate path in python-virtualenv #332

dotysan opened this issue May 8, 2023 · 1 comment

Comments

@dotysan
Copy link

dotysan commented May 8, 2023

When installed into a running python3 venv, that path is duplicated. Fairly easy to reproduce.

python3 -m venv .venv
source .venv/bin/activate
tr : '\n' <<<$PATH

Notice that PATH is already prepended with the .venv/bin.

/HERE/.venv/bin
/usr/local/bin
/usr/bin
/usr/local/sbin
/usr/sbin
/home/bozo/.local/bin
/home/bozo/bin

Then install nodeenv inside that active venv.

pip install nodeenv
nodeenv --python-virtualenv --node=lts

The next time you re-activate...

source .venv/bin/activate
tr : '\n' <<<$PATH

...that path is duplicated.

/HERE/.venv/lib/node_modules/.bin
/HERE/.venv/bin
/HERE/.venv/bin    <== whoops!
/usr/local/bin
/usr/bin
/usr/local/sbin
/usr/sbin
/home/bozo/.local/bin
/home/bozo/bin
@dotysan
Copy link
Author

dotysan commented May 9, 2023

I'm not familiar enough with this project to know if this is a bug or a regression.

I can't think of a reason that NODE_VIRTUAL_ENV isn't already in the PATH. So the quick and brutal fix is this.

diff --git a/nodeenv.py b/nodeenv.py
index 5c8aa2e..70bd577 100644
--- a/nodeenv.py
+++ b/nodeenv.py
@@ -1338,7 +1338,7 @@ fi
 export NODE_VIRTUAL_ENV
 
 _OLD_NODE_VIRTUAL_PATH="$PATH"
-PATH="$NODE_VIRTUAL_ENV/lib/node_modules/.bin:$NODE_VIRTUAL_ENV/__BIN_NAME__:$PATH"
+PATH="$NODE_VIRTUAL_ENV/lib/node_modules/.bin:$PATH"
 export PATH
 
 _OLD_NODE_PATH="$NODE_PATH"

However, out of the abundance of caution, if there ever is a reason that the NODE_VIRTUAL_ENV would be missing from the PATH, then here's a more spaghettified patch.

diff --git a/nodeenv.py b/nodeenv.py
index 5c8aa2e..23ed41f 100644
--- a/nodeenv.py
+++ b/nodeenv.py
@@ -1338,7 +1338,10 @@ fi
 export NODE_VIRTUAL_ENV
 
 _OLD_NODE_VIRTUAL_PATH="$PATH"
-PATH="$NODE_VIRTUAL_ENV/lib/node_modules/.bin:$NODE_VIRTUAL_ENV/__BIN_NAME__:$PATH"
+if [[ $PATH != *$NODE_VIRTUAL_ENV/__BIN_NAME__* ]]
+then PATH="$NODE_VIRTUAL_ENV/__BIN_NAME__:$PATH"
+fi
+PATH="$NODE_VIRTUAL_ENV/lib/node_modules/.bin:$PATH"
 export PATH
 
 _OLD_NODE_PATH="$NODE_PATH"

Or this variant:

diff --git a/nodeenv.py b/nodeenv.py
index 5c8aa2e..229efc2 100644
--- a/nodeenv.py
+++ b/nodeenv.py
@@ -1338,7 +1338,9 @@ fi
 export NODE_VIRTUAL_ENV
 
 _OLD_NODE_VIRTUAL_PATH="$PATH"
-PATH="$NODE_VIRTUAL_ENV/lib/node_modules/.bin:$NODE_VIRTUAL_ENV/__BIN_NAME__:$PATH"
+[[ $PATH = *$NODE_VIRTUAL_ENV/__BIN_NAME__* ]] \
+  || PATH="$NODE_VIRTUAL_ENV/__BIN_NAME__:$PATH"
+PATH="$NODE_VIRTUAL_ENV/lib/node_modules/.bin:$PATH"
 export PATH
 
 _OLD_NODE_PATH="$NODE_PATH"

Thoughts?

This is bash only. I still haven't looked at csh/fish/ps implementations of activate. But if logic is sound, happy to generate a PR and start working on those too.

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

No branches or pull requests

1 participant