-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
environment variables not passed correctly using new virtualenv launching in windows and python3.7+ #82273
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
If I am not mistaken, when creating a new process on Python3.7 and later on Windows, if using a virtualenv, Python now uses a launcher. The launcher is being notified that it must create a virtual-environment Python (and not a system Python) program using the __PYVENV_LAUNCHER__ environment variable, passed as part of the environment of launcher process created using in _winapi.CreateProcess cpython/Lib/multiprocessing/popen_spawn_win32.py Lines 73 to 75 in 9008be3
However, if I am not mistaken Lines 1062 to 1068 in 9008be3
Two things:
|
Yeah, very strange that. I can only assume that it's launching the venv redirector directly, rather than the real sys.executable, and we aren't ever calling set_executable() with the real one anymore. Dropping this into Lib/multiprocessing/spawn.py should cause a repro: if WINSERVICE:
_python_exe = os.path.join(sys.exec_prefix, 'python.exe')
else:
_python_exe = getattr(sys, '_base_executable', sys.executable) And as you point out, fixing the CreateProcess call should provide a fix. Could you try that? And maybe submit a PR with the fix? |
if WINSERVICE:
_python_exe = os.path.join(sys.exec_prefix, 'python.exe')
else:
_python_exe = getattr(sys, '_base_executable', sys.executable) In this case, spawn.get_executable() will return (sys._base_executable), and cpython/Lib/multiprocessing/popen_spawn_win32.py Lines 59 to 68 in 9008be3
We need to trigger the if clause of these lines instead, which happens by default in a virtual env -- this is why it is so troubling: even though a very simple case (launching a new process from within a virtualenv) should trigger a bug, it does not.
Will do. |
The difference is that launching sys._base_executable *without* __PYVENV_LAUNCHER__ set (because env is not being passed) should lose you access to anything installed into the venv. You may also need to import something from the venv in order to see the issue. Launching sys.executable will hit the launcher that sets the environment variable. Setting the environment correctly and launching sys._base_executable will also load correctly. The latter is theoretically required for correct handle sharing, but that may depend on which Windows version you're running. I'd like to see both changes in the PR. Just setting the environment variable doesn't really improve the situation at all. |
I posted a second PR with the rest of the change, as it'd be good to get this in before the next 3.8 release. |
Thanks for the report and partial patch! |
This should revert to setting Also, the fix for the parameters that are passed to _winapi.CreateProcess needs to be backported to 3.7. Else __PYVENV_LAUNCHER__ won't actually be set in the child. |
You're right, the logic to actually launch _base_executable is in there twice now, and the second one (that never gets used) is more important. |
Fixed via bpo-39439 |
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: