-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
The activate script in Windows is not correct for venvs created in git-bash #82764
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
Comments
When creating a virtual environment on Windows from git-bash (using python -m venv), VIRTUAL_ENV in the activate script is set using a windows style path (C:\some\path) instead of the bash style (/c/some/path). This means the system python and pip get used, despite the user thinking they are working in a venv after sourcing activate. As activate is a bash script, the paths in it should always be in the bash style, regardless of platform. This is described in a stack overflow issue here: https://stackoverflow.com/questions/57758841/windows-virtualenv-created-via-gitbash-using-system-python I have confirmed the behaviour in 3.7.3, 3.7.4, 3.7.5 and 3.8.0. |
The issue comes as a result of abspath on line 59 of venv/init.py: This returns a Windows-style path, and os.path.abspath returning in this way is *probably* correct, as the OS is Windows, despite trying to forget that by using bash. It is still my view that the activate script is a bash script, and therefore should only contain paths in that style, but the simple solution to this issue is to change the double quotes around the definition of $VIRTUAL_ENV in the activate script to single quotes. It works. The output of "which python" is a bit odd, but this is clearly a quirk beyond Python's control. |
I agree this would be better, though it may not be that easy to do. If you're running Windows Python from WSL, then your "bash style" path should be "/mnt/c/some/path", not "/c/some/path". So the answer isn't as simple as .replace('\\', '/'). I did some quick testing with posixpath and pathlib and neither has an obvious conversion from a Windows path to a POSIX path, though pathlib.PurePosixPath(pathlib.PureWindowsPath(p)) comes closest. Perhaps the best approach here is to improve |
I had also tested with pathlib and posixpath and come to the same conclusion. As suggested by you, I looked into pushd . > /dev/null
SCRIPT_PATH="${BASH_SOURCE[0]}"
if ([ -h "${SCRIPT_PATH}" ]); then
while([ -h "${SCRIPT_PATH}" ]); do cd `dirname "$SCRIPT_PATH"`;
SCRIPT_PATH=`readlink "${SCRIPT_PATH}"`; done
fi
cd `dirname ${SCRIPT_PATH}` > /dev/null
cd .. > /dev/null
SCRIPT_PATH=`pwd`;
popd > /dev/null VIRTUAL_ENV="$SCRIPT_PATH" |
Would you consider just handling activate for windows directly in the lib/venv/init.py method "install_scripts(self, context, path)" Line 382 in 4f98f46
if not srcfile.endswith(('.exe', '.pdb')):
# handle activate for Windows (ignore WSL)
if srcfile == "activate":
# from docs: on unix drive is always empty
d, p = os.path.splitdrive(context.env_dir)
d = d.replace(':', '')
p = p.replace('\\', '/')
if d:
p = '/' + d + p
data = data.decode('utf-8')
data = data.replace('__VENV_DIR__', p)
data = data.encode('utf-8')
IMHO I don't think the use of windows python in WSL is a realistic use-case, my preference would be to just make this fail. In fact I tried to use it, and I could not make it work.
I guess I don't really understand why it would be useful to use the windows python in WSL, and if that's the only thing holding a quick fix for this, I guess, I would prefer to just handle windows python in windows in git-bash, and ignore WSL. Would you be open to that? If so, I'm happy to submit a PR thanks! |
probably this is the only option VIRTUAL_ENV='C:\Users\xyz\Projects\abc\.venvwin'
if ([ "$OSTYPE" = "cygwin" ] || [ "$OSTYPE" = "msys" ]) && $(command -v cygpath &> /dev/null) ; then
VIRTUAL_ENV=$(cygpath -u "$VIRTUAL_ENV")
fi
export VIRTUAL_ENV |
I was looking into a different issue with activate script from Windows' Python in a cygwin terminal and came across this Github issue. I have 3 pieces of feedback on this issue.
|
Indeed! That code change addresses my point 2. I think we can close this issue. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: