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

bpo-22490: Remove __PYVENV_LAUNCHER__ from environment during launch #9516

Merged
merged 5 commits into from Mar 22, 2020

Conversation

Projects
None yet
10 participants
@ronaldoussoren
Copy link
Contributor

@ronaldoussoren ronaldoussoren commented Sep 23, 2018

Alternative patch for bpo-22490 that removes the shell environment variable PYVENV_LAUNCHER during interpreter launch because it is only used to communicate between the stub executable in Mac/Tools/pythonw.c and the real interpreter.

This avoids problems when launching the "real" interpreter without going through the stub executable.

https://bugs.python.org/issue22490

… macOS

This changeset removes the environment varialbe "__PYVENV_LAUNCHER__"
during interpreter launch as it is only needed to communicate between
the stub executable in framework installs and the actual interpreter.

Leaving the environment variable present may lead to misbehaviour when
launching other scripts.
jaraco
jaraco approved these changes Sep 23, 2018
Copy link
Member

@jaraco jaraco left a comment

Looks good to me. I think it would be nice if, since #9498 didn't fail any tests, there were a documented way (even if just in the ticket), one could verify that the behavior is working as intended (a test that would fail with #9498).

@gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Sep 23, 2018

I would love to know how to repro the original bug so I can demonstrate this actually fixes it.

Here's what I think should be the repro:

/Library/Frameworks/Python.framework/Versions/3.8/bin/python3  -c 'import subprocess; p = subprocess.Popen([".mypy/venv/bin/python3","-m","mypy.dmypy","-h"]).communicate()'

The setup is that .mypy/venv is a virtualenv that has mypy.dmypy installed. This is what failed for the user who reported this.

Unfortunately this does not fail for me (with a framework build from master installed). Would I have to do the install via brew? (I would have to learn about locally testing modified brew recipes, I suppose.)

Copy link
Member

@gvanrossum gvanrossum left a comment

The patch LG from a code review POV.

@ned-deily
Copy link
Member

@ned-deily ned-deily commented Mar 24, 2019

xeonchen pushed a commit to xeonchen/gecko that referenced this issue Jan 15, 2020
…ld-system-reviewers,rstewart

The __PYVENV_LAUNCHER__ is set on macos to indicate which
python to use. Sadly, keeping this set confuses pip in
virtualenvs.
This corresponds to python/cpython#9516.

Differential Revision: https://phabricator.services.mozilla.com/D59853
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jan 15, 2020
…ld-system-reviewers,rstewart

The __PYVENV_LAUNCHER__ is set on macos to indicate which
python to use. Sadly, keeping this set confuses pip in
virtualenvs.
This corresponds to python/cpython#9516.

Differential Revision: https://phabricator.services.mozilla.com/D59853

--HG--
extra : moz-landing-system : lando
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Jan 16, 2020
…ld-system-reviewers,rstewart

The __PYVENV_LAUNCHER__ is set on macos to indicate which
python to use. Sadly, keeping this set confuses pip in
virtualenvs.
This corresponds to python/cpython#9516.

Differential Revision: https://phabricator.services.mozilla.com/D59853

UltraBlame original commit: ed91bca1f4a669454801390a1e5432fa2cd9561e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Jan 16, 2020
…ld-system-reviewers,rstewart

The __PYVENV_LAUNCHER__ is set on macos to indicate which
python to use. Sadly, keeping this set confuses pip in
virtualenvs.
This corresponds to python/cpython#9516.

Differential Revision: https://phabricator.services.mozilla.com/D59853

UltraBlame original commit: ed91bca1f4a669454801390a1e5432fa2cd9561e
@mattip
Copy link
Contributor

@mattip mattip commented Feb 27, 2020

We think this is borking creation of a PyPy virtualenv using cpython host on macOS and is was very difficult to detect, so it would be nice to clean this up. For a test, you could try to install pypy 7.3.0 using cpython, see https://foss.heptapod.net/pypy/pypy/issues/3175. Although now that we know we will probably fix it for PyPy's next release.

Mac/Tools/pythonw.c Outdated Show resolved Hide resolved
Co-Authored-By: Nicola Soranzo <[email protected]>
@jaraco jaraco requested review from vsajip and as code owners Mar 22, 2020
@jaraco
Copy link
Member

@jaraco jaraco commented Mar 22, 2020

The test failure is also happening in master, so unrelated to this change.

@jaraco jaraco merged commit 044cf94 into python:master Mar 22, 2020
8 of 9 checks passed
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Mar 22, 2020

Thanks @ronaldoussoren for the PR, and @jaraco for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒🤖

@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Mar 22, 2020

GH-19110 is a backport of this pull request to the 3.8 branch.

@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Mar 22, 2020

Sorry, @ronaldoussoren and @jaraco, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 044cf94f610e831464a69a8e713dad89878824ce 3.7

miss-islington added a commit to miss-islington/cpython that referenced this issue Mar 22, 2020
…ythonGH-9516)

* bpo-22490: Remove "__PYVENV_LAUNCHER__" from the shell environment on macOS

This changeset removes the environment varialbe "__PYVENV_LAUNCHER__"
during interpreter launch as it is only needed to communicate between
the stub executable in framework installs and the actual interpreter.

Leaving the environment variable present may lead to misbehaviour when
launching other scripts.

* Actually commit the changes for issue 22490...

* Correct typo

Co-Authored-By: Nicola Soranzo <[email protected]>

* Run make patchcheck

Co-authored-by: Jason R. Coombs <[email protected]>
Co-authored-by: Nicola Soranzo <[email protected]>
(cherry picked from commit 044cf94)

Co-authored-by: Ronald Oussoren <[email protected]>
jaraco added a commit to jaraco/cpython that referenced this issue Mar 22, 2020
…aunch (pythonGH-9516)

* bpo-22490: Remove "__PYVENV_LAUNCHER__" from the shell environment on macOS

This changeset removes the environment varialbe "__PYVENV_LAUNCHER__"
during interpreter launch as it is only needed to communicate between
the stub executable in framework installs and the actual interpreter.

Leaving the environment variable present may lead to misbehaviour when
launching other scripts.

* Actually commit the changes for issue 22490...

* Correct typo

Co-Authored-By: Nicola Soranzo <[email protected]>

* Run make patchcheck

Co-authored-by: Jason R. Coombs <[email protected]>
Co-authored-by: Nicola Soranzo <[email protected]>.
(cherry picked from commit 044cf94)

Co-authored-by: Ronald Oussoren <[email protected]>
jaraco added a commit to jaraco/cpython that referenced this issue Mar 22, 2020
…aunch (pythonGH-9516)

* bpo-22490: Remove "__PYVENV_LAUNCHER__" from the shell environment on macOS

This changeset removes the environment varialbe "__PYVENV_LAUNCHER__"
during interpreter launch as it is only needed to communicate between
the stub executable in framework installs and the actual interpreter.

Leaving the environment variable present may lead to misbehaviour when
launching other scripts.

* Actually commit the changes for issue 22490...

* Correct typo

Co-Authored-By: Nicola Soranzo <[email protected]>

* Run make patchcheck

Co-authored-by: Jason R. Coombs <[email protected]>
Co-authored-by: Nicola Soranzo <[email protected]>.
(cherry picked from commit 044cf94)

Co-authored-by: Ronald Oussoren <[email protected]>
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Mar 22, 2020

GH-19111 is a backport of this pull request to the 3.7 branch.

jaraco pushed a commit that referenced this issue Mar 22, 2020
…H-9516) (GH-19110)

* bpo-22490: Remove "__PYVENV_LAUNCHER__" from the shell environment on macOS

This changeset removes the environment varialbe "__PYVENV_LAUNCHER__"
during interpreter launch as it is only needed to communicate between
the stub executable in framework installs and the actual interpreter.

Leaving the environment variable present may lead to misbehaviour when
launching other scripts.

* Actually commit the changes for issue 22490...

* Correct typo

Co-Authored-By: Nicola Soranzo <[email protected]>

* Run make patchcheck

Co-authored-by: Jason R. Coombs <[email protected]>
Co-authored-by: Nicola Soranzo <[email protected]>
(cherry picked from commit 044cf94)

Co-authored-by: Ronald Oussoren <[email protected]>

Co-authored-by: Ronald Oussoren <[email protected]>
jaraco added a commit that referenced this issue Mar 22, 2020
…aunch (GH-9516) (GH-19111)

* bpo-22490: Remove "__PYVENV_LAUNCHER__" from the shell environment on macOS

This changeset removes the environment varialbe "__PYVENV_LAUNCHER__"
during interpreter launch as it is only needed to communicate between
the stub executable in framework installs and the actual interpreter.

Leaving the environment variable present may lead to misbehaviour when
launching other scripts.

* Actually commit the changes for issue 22490...

* Correct typo

Co-Authored-By: Nicola Soranzo <[email protected]>

* Run make patchcheck

Co-authored-by: Jason R. Coombs <[email protected]>
Co-authored-by: Nicola Soranzo <[email protected]>.
(cherry picked from commit 044cf94)

Co-authored-by: Ronald Oussoren <[email protected]>

Co-authored-by: Ronald Oussoren <[email protected]>
vegerot added a commit to vegerot/cpython that referenced this issue Jun 10, 2020
…ythonGH-9516)

* bpo-22490: Remove "__PYVENV_LAUNCHER__" from the shell environment on macOS

This changeset removes the environment varialbe "__PYVENV_LAUNCHER__"
during interpreter launch as it is only needed to communicate between
the stub executable in framework installs and the actual interpreter.

Leaving the environment variable present may lead to misbehaviour when
launching other scripts.

* Actually commit the changes for issue 22490...

* Correct typo

Co-Authored-By: Nicola Soranzo <[email protected]>

* Run make patchcheck

Co-authored-by: Jason R. Coombs <[email protected]>
Co-authored-by: Nicola Soranzo <[email protected]>
vegerot added a commit to vegerot/cpython that referenced this issue Jun 10, 2020
…ythonGH-9516)

* bpo-22490: Remove "__PYVENV_LAUNCHER__" from the shell environment on macOS

This changeset removes the environment varialbe "__PYVENV_LAUNCHER__"
during interpreter launch as it is only needed to communicate between
the stub executable in framework installs and the actual interpreter.

Leaving the environment variable present may lead to misbehaviour when
launching other scripts.

* Actually commit the changes for issue 22490...

* Correct typo

Co-Authored-By: Nicola Soranzo <[email protected]>

* Run make patchcheck

Co-authored-by: Jason R. Coombs <[email protected]>
Co-authored-by: Nicola Soranzo <[email protected]>
gmelikov added a commit to gmelikov/cpython that referenced this issue Aug 22, 2020
…ythonGH-9516)

* bpo-22490: Remove "__PYVENV_LAUNCHER__" from the shell environment on macOS

This changeset removes the environment varialbe "__PYVENV_LAUNCHER__"
during interpreter launch as it is only needed to communicate between
the stub executable in framework installs and the actual interpreter.

Leaving the environment variable present may lead to misbehaviour when
launching other scripts.

* Actually commit the changes for issue 22490...

* Correct typo

Co-Authored-By: Nicola Soranzo <[email protected]>

* Run make patchcheck

Co-authored-by: Jason R. Coombs <[email protected]>
Co-authored-by: Nicola Soranzo <[email protected]>
@jsirois jsirois mentioned this pull request Sep 28, 2020
4 tasks
chrisburr added a commit to chrisburr/cpython that referenced this issue Dec 9, 2020
…ythonGH-9516)

* bpo-22490: Remove "__PYVENV_LAUNCHER__" from the shell environment on macOS

This changeset removes the environment varialbe "__PYVENV_LAUNCHER__"
during interpreter launch as it is only needed to communicate between
the stub executable in framework installs and the actual interpreter.

Leaving the environment variable present may lead to misbehaviour when
launching other scripts.

* Actually commit the changes for issue 22490...

* Correct typo

Co-Authored-By: Nicola Soranzo <[email protected]>

* Run make patchcheck

Co-authored-by: Jason R. Coombs <[email protected]>
Co-authored-by: Nicola Soranzo <[email protected]>
foolip added a commit to web-platform-tests/wpt that referenced this issue Apr 23, 2021
Exactly how this all works isn't clear, but since the fix in
python/cpython#9516 was to simply hide the
environment variable from the interpreter it seems reasonably safe.

Fixes #27377.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment