[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[Steve Dower]

Steve Dower  added the comment:

Fixed via issue39439

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[Steve Dower]

Steve Dower  added the comment:


New changeset 6990d1b6131873c7f0913908162e4c723d00ea19 by Steve Dower (Adam 
Meily) in branch '3.7':
bpo-38092: Reduce overhead when using multiprocessing in a Windows virtual 
environment (GH-16098)
https://github.com/python/cpython/commit/6990d1b6131873c7f0913908162e4c723d00ea19


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[Adam Meily]

Change by Adam Meily :


--
pull_requests: +17543
stage: needs patch -> patch review
pull_request: https://github.com/python/cpython/pull/18157

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[Steve Dower]

Steve Dower  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[Eryk Sun]

Eryk Sun  added the comment:

This should revert to setting `_python_exe = sys.executable` in 
Lib/multiprocessing/spawn.py. Then the code in 
Lib/multiprocessing/popen_spawn_win32.py will set __PYVENV_LAUNCHER__ in the 
spawned process to the virtual environment's sys.executable. Otherwise the 
worker process has no connection to the virtual environment, other than how 
sys.path gets manually copied from the parent process. In particular, without 
setting __PYVENV_LAUNCHER__, sys.executable is not the virtual-environment 
executable and sys.prefix is not the virtual-environment directory.

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.

--
nosy: +eryksun
resolution: fixed -> 
stage: resolved -> needs patch
status: closed -> open
versions: +Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[miss-islington]

miss-islington  added the comment:


New changeset 436b429ade87b10879b3f944e99a515478e86e5e by Miss Islington (bot) 
in branch '3.8':
bpo-38092: Reduce overhead when using multiprocessing in a Windows virtual 
environment (GH-16098)
https://github.com/python/cpython/commit/436b429ade87b10879b3f944e99a515478e86e5e


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[Steve Dower]

Steve Dower  added the comment:

Thanks for the report and partial patch!

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[miss-islington]

miss-islington  added the comment:


New changeset f2b7556ef851ac85e7cbf189d1b29fdeb9539b88 by Miss Islington (bot) 
(Steve Dower) in branch 'master':
bpo-38092: Reduce overhead when using multiprocessing in a Windows virtual 
environment (GH-16098)
https://github.com/python/cpython/commit/f2b7556ef851ac85e7cbf189d1b29fdeb9539b88


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[miss-islington]

Change by miss-islington :


--
pull_requests: +15731
pull_request: https://github.com/python/cpython/pull/16116

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[Steve Dower]

Steve Dower  added the comment:

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.

--
versions:  -Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[Steve Dower]

Change by Steve Dower :


--
pull_requests: +15718
pull_request: https://github.com/python/cpython/pull/16098

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[Steve Dower]

Steve Dower  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[Pierre Glaser]

Change by Pierre Glaser :


--
keywords: +patch
pull_requests: +15524
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/15883

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[Pierre Glaser]

Pierre Glaser  added the comment:

> 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)

In this case, spawn.get_executable() will return (sys._base_executable), and 
`env` will be set to None anyways no? (see these lines: 
https://github.com/python/cpython/blob/9008be303a89bfab8c3314c6a42330b5523adc8b/Lib/multiprocessing/popen_spawn_win32.py#L59-L68)

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.

> And maybe submit a PR with the fix?

Will do.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[Steve Dower]

Steve Dower  added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38092] environment variables not passed correctly using new virtualenv launching in windows and python3.7+

[Pierre Glaser]

New submission from Pierre Glaser :

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
(see 
https://github.com/python/cpython/blob/9008be303a89bfab8c3314c6a42330b5523adc8b/Lib/multiprocessing/popen_spawn_win32.py#L73-L75)

However, if I am not mistaken `env` is not passed at the right position 
(https://github.com/python/cpython/blob/9008be303a89bfab8c3314c6a42330b5523adc8b/Modules/_winapi.c#L1062-L1068).
 These lines were part of a bugfix patch (see 
https://bugs.python.org/issue35797), solving an issue for multiprocessing-based 
packages. We ended trying to backport to loky 
(https://github.com/tomMoral/loky, a multiprocessing-based package) but the bug 
was not fixed. Passing 'env' correctly fixed the bug.

Two things:
- It is hard to provide a reproducer for this issue as it requires patching the 
CPython source code.
- I don't understand why env being not passed correctly does not manifest 
itself in the test-suite. What is even more troubling is that even with this 
bug, the virtualenv launcher seems to get that a virtualenv is used when 
creating processes in multiprocessing (at least, sys.path includes the 
path/to/venv/lib/python3.x/site-packages). However, I am not familiar with the 
virtualenv launcher for python3.7+ and windows.

--
components: Windows
messages: 351650
nosy: paul.moore, pierreglaser, pitrou, steve.dower, tim.golden, zach.ware
priority: normal
severity: normal
status: open
title: environment variables not passed correctly using new virtualenv 
launching in windows and python3.7+
type: crash
versions: Python 3.7, Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com