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

Using realpath for __PYVENV_LAUNCHER__ makes Homebrew installs fragile #66680

Closed
tdsmith mannequin opened this issue Sep 25, 2014 · 41 comments
Closed

Using realpath for __PYVENV_LAUNCHER__ makes Homebrew installs fragile #66680

tdsmith mannequin opened this issue Sep 25, 2014 · 41 comments
Assignees
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes OS-mac type-bug An unexpected behavior, bug, or error

Comments

@tdsmith
Copy link
Mannequin

tdsmith mannequin commented Sep 25, 2014

BPO 22490
Nosy @gvanrossum, @vsajip, @ronaldoussoren, @jaraco, @ned-deily, @boxed, @tdsmith, @csabella, @miss-islington
PRs
  • WIP: bpo-22490: Remove __PYVENV_LAUNCHER__ from the codebase. #9498
  • bpo-22490: Remove __PYVENV_LAUNCHER__ from environment during launch #9516
  • [3.8] bpo-22490: Remove __PYVENV_LAUNCHER__ from environment during launch (GH-9516) #19110
  • [3.7] bpo-22490: Remove __PYVENV_LAUNCHER__ from environment during launch (GH-9516) #19111
  • Files
  • dont-realpath-venv-dirname.diff
  • dont-realpath-venv-dirname.diff-1
  • delete-venev-launcher.diff
  • 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:

    assignee = 'https://github.com/ronaldoussoren'
    closed_at = <Date 2020-03-22.19:33:29.689>
    created_at = <Date 2014-09-25.05:30:45.446>
    labels = ['OS-mac', '3.8', 'type-bug', '3.7', '3.9']
    title = 'Using realpath for __PYVENV_LAUNCHER__ makes Homebrew installs fragile'
    updated_at = <Date 2020-03-22.21:40:46.615>
    user = 'https://github.com/tdsmith'

    bugs.python.org fields:

    activity = <Date 2020-03-22.21:40:46.615>
    actor = 'jaraco'
    assignee = 'ronaldoussoren'
    closed = True
    closed_date = <Date 2020-03-22.19:33:29.689>
    closer = 'jaraco'
    components = ['macOS']
    creation = <Date 2014-09-25.05:30:45.446>
    creator = 'tdsmith'
    dependencies = []
    files = ['36718', '36885', '46004']
    hgrepos = []
    issue_num = 22490
    keywords = ['patch']
    message_count = 41.0
    messages = ['227505', '227515', '227517', '227561', '227577', '227579', '229114', '229115', '229119', '229126', '229129', '229682', '230902', '230947', '234301', '258730', '283859', '283865', '284106', '284116', '284117', '284124', '284385', '326061', '326088', '326099', '326149', '326157', '326169', '326174', '326176', '326177', '326182', '326202', '326263', '338600', '342775', '364813', '364815', '364817', '364825']
    nosy_count = 9.0
    nosy_names = ['gvanrossum', 'vinay.sajip', 'ronaldoussoren', 'jaraco', 'ned.deily', 'Anders.Hovm\xc3\xb6ller', 'tdsmith', 'cheryl.sabella', 'miss-islington']
    pr_nums = ['9498', '9516', '19110', '19111']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue22490'
    versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

    @tdsmith
    Copy link
    Mannequin Author

    tdsmith mannequin commented Sep 25, 2014

    Homebrew, the OS X package manager, distributes python3 as a framework build. We like to be able to control the shebang that gets written to scripts installed with pip. [1]

    The path we prefer for invoking the python3 interpreter is like /usr/local/opt/python3/bin/python3.4, which is symlinked to the framework stub launcher at /usr/local/Cellar/python3/3.4.1_1/Frameworks/Python.framework/Versions/3.4/bin/python3.4. For Python 2.x, we discovered that assigning "/usr/local/opt/python/bin/python2.7" to sys.executable in sitecustomize.py resulted in correct shebangs for scripts installed by pip. The same approach doesn't work with Python 3.

    A very helpful conversation with Vinay Sajip [2] led us to consider how the python3 stub launcher sets __PYVENV_LAUNCHER__, which distlib uses in preference to sys.executable to discover the intended interpreter when pip writes shebangs.

    Roughly, __PYVENV_LAUNCHER__ is set from argv[0], so it mimics the invocation of the stub, except that symlinks in the directory component of the path to the identified interpreter are resolved to a "real" path. For us, this means that __PYVENV_LAUNCHER__ (and therefore the shebangs of installed scripts) always points to the Cellar path, not the preferred opt path, even when python is invoked via the opt path.

    Avoiding this symlink resolution would allow us to control pip's shebang (which sets the shebangs of all pip-installed scripts) by controlling the way we invoke python3 when we use ensurepip during installation.

    Building python3 with the attached diff removes the symlink resolution.

    [1] This is important to Homebrew because packages are physically installed to versioned prefixes, like /usr/local/Cellar/python3/3.4.1_1/. References to these real paths are fragile and break when the version number changes or when the revision number ("_1") changes, when can happen when e.g. openssl is released and Python needs to be recompiled against the new library. To avoid this breakage, Homebrew maintains a version-independent symlink to each package, like /usr/local/opt/python3, which points to the .../Cellar/python3/3.4.1_1/ location.

    [2] pypa/pip#2031

    @tdsmith tdsmith mannequin assigned ronaldoussoren Sep 25, 2014
    @tdsmith tdsmith mannequin added OS-mac type-bug An unexpected behavior, bug, or error labels Sep 25, 2014
    @ned-deily
    Copy link
    Member

    From the initial description of the problem, it's not clear to me that there is a problem here needing resolution in the stub launcher. I've asked for clarification on the pip issue tracker.

    @ned-deily
    Copy link
    Member

    Also, the patch causes a test failure with a framework build:

    ======================================================================
    FAIL: test_defaults (test.test_venv.BasicTest)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/py/dev/3x/root/fwd/Library/Frameworks/pytest_10.9.framework/Versions/3.5/lib/python3.5/test/test_venv.py", line 106, in test_defaults
        self.assertIn('home = %s' % path, data)
    AssertionError: 'home = /py/dev/3x/root/fwd/./bin' not found in 'home = /py/dev/3x/root/fwd/bin\ninclude-system-site-packages = false\nversion = 3.5.0\n'

    @ronaldoussoren
    Copy link
    Contributor

    I'm not convinced that this is a bug in python. __PYVENV_LAUNCHER__ is an implementation detail of CPython used in the implementation of the pyvenv functionality.

    I consider using this undocumented environment variable in distlib as a bug in distlib, which should be fixed there.

    It would be nice to know why distlib uses __PYVENV_LAUNCHER__ as that could lead us to a real bug or missing feature.

    BTW. I haven't read the discussion in [2] yet.

    @ronaldoussoren
    Copy link
    Contributor

    On 25 sep. 2014, at 19:58, Ronald Oussoren <report@bugs.python.org> wrote:

    Ronald Oussoren added the comment:

    I consider using this undocumented environment variable in distlib as a bug in distlib, which should be fixed

    Speaking of which... That environment variable shouldn't leak into Python code in the first place, launching a regular Python interpreter from a venv could currently result in unwanted behavior. I'll try to provide an example of that.

    Ronald

    ----------


    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue22490\>


    @vsajip
    Copy link
    Member

    vsajip commented Sep 25, 2014

    I consider using this undocumented environment variable in distlib
    as a bug in distlib, which should be fixed there.

    Well, let me explain why it's used: when Python >= 3.3 starts up, it looks for a pyvenv.cfg file proximate to sys.executable. If found, that means we're in a venv, and sys.path is set up accordingly. On OS X, the stub launcher is copied/symlinked to the venv when a new venv is created - the real interpreter is not copied. So, shebangs written by distlib into scripts installed into a venv must be of the form #!/path/to/venv/bin/python, and this cannot be obtained from sys.executable because that is pointing to a framework executable. There would be no pyvenv.cfg anywhere near that location.

    This is why the __PYVENV_LAUNCHER__ variable was created, and distlib uses it because it needs to conform to how venvs work. In this respect distlib is a bit like setuptools - it needs to understand some low-level details which other libraries don't need to worry about.

    Scripts installed in venvs work as expected (AFAICT) when used with stock Python framework builds on OS X. With HomeBrew, the complication appears to be caused by two levels of symlink: the executable in /usr/local/ points to one in /usr/local/Cellar/..., which in turn points to the framework executable.

    The failing test (test_defaults) could be fixed by looking for equivalence in the "home =" directories in the test, rather than a string-contains-value test as at present.

    @tdsmith
    Copy link
    Mannequin Author

    tdsmith mannequin commented Oct 12, 2014

    I'm attaching an updated patch; it passes tests for me locally with a framework build.

    @tdsmith
    Copy link
    Mannequin Author

    tdsmith mannequin commented Oct 12, 2014

    Er, because the test has been modified by taking Vinay's suggestion to test that the directories are physically identical instead of doing a string comparison.

    @ned-deily
    Copy link
    Member

    A comment on Vinay's comment: "and this cannot be obtained from sys.executable because that is pointing to a framework executable". That *was* true prior to 3.3 and is still true at the moment for 2.7.x but it is not true for 3.3+. Ronald's change (b79d276041a8) in bpo-15307 for venv framework support changed the stub launcher to no longer do a realpath() on the executable name which means that sys.executable contains the path to the Python stub launcher whether or not in a venv:

    $ ls -l /usr/local/bin/python2.7
    lrwxr-xr-x  1 root  admin  71 Jul  3 01:21 /usr/local/bin/python2.7 -> ../../../Library/Frameworks/Python.framework/Versions/2.7/bin/python2.7
    $ /usr/local/bin/python2.7 -c 'import sys;print(sys.executable)'
    /Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python
    
    $ ls -l /usr/local/bin/python3.4
    lrwxr-xr-x  1 root  wheel  71 Oct  6 16:53 /usr/local/bin/python3.4 -> ../../../Library/Frameworks/Python.framework/Versions/3.4/bin/python3.4
    $ /usr/local/bin/python3.4 -c 'import sys;print(sys.executable)'
    /usr/local/bin/python3.4

    @vsajip
    Copy link
    Member

    vsajip commented Oct 12, 2014

    Ronald's change ... changed the stub launcher to no longer do a realpath() on the executable name
    which means that sys.executable contains the path to the Python stub launcher whether or not in a venv

    Ronald's change doesn't do a realpath() on the executable name itself, but it *does* do a realpath() on the executable's directory. This seems to be what's causing the problem in Homebrew builds: there are two levels of symlink at play.

    /usr/local/bin/python3.4 -> /usr/local/opt/python3/bin/python3.4 -> /usr/local/Cellar/python3/3.4.1_1/Frameworks/Python.framework/Versions/3.4/bin/python3.4

    Because of the realpath(), the "wrong" stub launcher (from Homebrew's perspective) gets pointed to by the environment variable. At least, if my understanding is correct.

    pypa/pip#2031 (comment)

    @tdsmith
    Copy link
    Mannequin Author

    tdsmith mannequin commented Oct 12, 2014

    We would like to refer to python3 as /usr/local/opt/python3/bin/python3, where /usr/local/opt/python3 is a symlink to ../Cellar/python3/3.4.2, and /usr/local/Cellar/python3/3.4.2/bin/python3 is a symlink to /usr/local/Cellar/python3/3.4.2/Frameworks/Python.framework/Versions/3.4/bin/python3, which is the stub launcher for /usr/local/Cellar/python3/3.4.2/Frameworks/Python.framework/Versions/3.4/Python.

    @ronaldoussoren
    Copy link
    Contributor

    First of all, sorry about the slow response.

    Vinay: I don't quite understand why you use __PYVENV_LAUNCHER__: When I create a pyvenv using python 3.3 (with a oldish build from the 3.3 branch) sys.executable point to a binary in the venv without mucking with environment variables:

    $ pyvenv-3.3 testenv
    $ testenv/bin/python
    Python 3.3.5+ (3.3:a36d469f31c1, Aug 13 2014, 09:04:41) 
    [GCC 4.2.1 Compatible Apple LLVM 5.1 (clang-503.0.40)] on darwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import sys
    >>> sys.executable
    '/Users/ronald/Projects/pyobjc-hg/pyobjc/x/testenv/bin/python'

    If older version of Python 3.x don't do this that's a bug in those versions and it would be acceptable to use the environment variable with those versions to work around this bug.

    I still not convinced that __PYVENV_LAUNCHER__ needs to be changed. It's highly likely that any use of __PYVENV_LAUNCHER__ outside of CPython itself is an indication of a bug in CPython that needs to be fixed there.

    @vsajip
    Copy link
    Member

    vsajip commented Nov 9, 2014

    I don't quite understand why you use __PYVENV_LAUNCHER__

    When I first developed the venv functionality it was definitely needed, but it looks as if it is not needed now. Perhaps to fix this completely, the following needs to be done, on the assumption that __PYVENV_LAUNCHER__ is no longer needed:

    1. Remove the reference to it in distlib, and use sys.executable instead. Once pip incorporates this fix, the Homebrew problem should go away. (I have already made the change in the distlib repo, but this needs to be released in order for pip to consider vendoring it, and then pip needs to be released before Python can incorporate it).

    2. Remove references to the environment variable in Python itself, using sys.executable instead.

    As the env var was an implementation detail, ISTM it could be removed in a 3.4 point release - would you agree?

    @ronaldoussoren
    Copy link
    Contributor

    The environment variable itself cannot be removed from CPython, it is necessary to implement the correct behavior of pyvenv with framework builds of Python.

    That said, I do think that the environment variable should be unset as soon as possible in the CPython startup code to avoid accidentally affecting other interpreters.

    @tdsmith
    Copy link
    Mannequin Author

    tdsmith mannequin commented Jan 19, 2015

    Homebrew's interest in this ticket was resolved by the release of pip 6, which includes Vinay's change to distlib to use sys.executable instead of __PYVENV_LAUNCHER__. Many thanks!

    I'm not marking this fixed in case it is useful to leave this open to discuss unsetting __PYVENV_LAUNCHER__.

    @jaraco
    Copy link
    Member

    jaraco commented Jan 21, 2016

    I believe this behavior (presence of the __PYVENV_LAUNCHER__ and inheritance in child processes) continues to cause problems. See pypa/virtualenv#845 for a simple reproducing of the issue, even on pip 6+.

    @tdsmith
    Copy link
    Mannequin Author

    tdsmith mannequin commented Dec 22, 2016

    I spoke prematurely; I recently rediscovered that the persistence of __PYVENV_LAUNCHER__ poisons the sys.executable of virtualenv interpreters launched as a subprocess of another Python interpreter:

    $ virtualenv -p python3 test
    $ test/bin/python3 -c 'import sys; print(sys.executable)'
    /Users/tim/test/bin/python3
    
    $ /usr/local/bin/python3 -c 'import subprocess; subprocess.call(["/Users/tim/test/bin/python3", "-c", "import sys; print(sys.executable)"])'
    /usr/local/bin/python3
    
    $ /usr/local/bin/python3 -c 'import subprocess, os; del os.environ["__PYVENV_LAUNCHER__"]; subprocess.call(["/Users/tim/test/bin/python3", "-c", "import sys; print(sys.executable)"])'
    /Users/tim/test/bin/python3

    If __PYVENV_LAUNCHER__ can be unset before script execution begins, that seems ideal.

    @tdsmith
    Copy link
    Mannequin Author

    tdsmith mannequin commented Dec 22, 2016

    Since __PYVENV_LAUNCHER__ is consulted in site.py, it seems likely that the latest it can be deleted is in site.py. The attached patch does that.

    @vsajip
    Copy link
    Member

    vsajip commented Dec 27, 2016

    I'm not sure if msg230947 is still correct, and it seems neater to remove __PYVENV_LAUNCHER__ from where it's defined in the first place (the stub launcher C file) if it was just a shim needed around the time the functionality was developed (pre the 3.3 release).

    It would be helpful if a Mac person could try removing it and seeing if the resulting framework builds of Python on OS X are adversely affected.

    @jaraco
    Copy link
    Member

    jaraco commented Dec 27, 2016

    On Python 3.6, I made the edit. I actually used the one-liner os.environ.pop('__PYVENV_LAUNCHER__', None). I then created a venv and installed a package and successfully ran a module in that package:

    $ python -m venv ~/.envs/issue22490-test
    $ ~/.envs/issue22490-test/bin/python -m pip install cherrypy
    Collecting cherrypy
      Downloading CherryPy-8.5.0-py2.py3-none-any.whl (463kB)
        100% |████████████████████████████████| 471kB 2.2MB/s 
    Collecting six (from cherrypy)
      Using cached six-1.10.0-py2.py3-none-any.whl
    Installing collected packages: six, cherrypy
    Successfully installed cherrypy-8.5.0 six-1.10.0
    $ ~/.envs/issue22490-test/bin/python -m cherrypy            
    ...
    [27/Dec/2016:11:08:52] ENGINE Serving on http://127.0.0.1:8080
    [27/Dec/2016:11:08:52] ENGINE Bus STARTED

    So pyvenv seems to be working.

    Additionally, it's corrected the undesirable behavior that Tim demonstrated:

    $ rm -R ~/.envs/issue22490-test 
    $ python -m virtualenv ~/.envs/issue22490-test
    Using base prefix '/Library/Frameworks/Python.framework/Versions/3.6'
    New python executable in /Users/jaraco/.envs/issue22490-test/bin/python3
    Also creating executable in /Users/jaraco/.envs/issue22490-test/bin/python
    Installing setuptools, pip, wheel...done.
    $ python -c 'import subprocess; subprocess.call(["/Users/jaraco/.envs/issue22490-test/bin/python", "-c", "import sys; print(sys.executable)"])'
    /Users/jaraco/.envs/issue22490-test/bin/python

    I tried making a Framework build of Python with the environment variable removed entirely, but I'm not experienced enough with builds on a Mac to know what I need to do to create a proper build. I build Python from source and tried to create a venv, but it crashed with a nondescript error:

    $ ./python.exe -VV
    Python 3.6.0+ (3.6:86a412584c02+, Dec 27 2016, 11:24:01) 
    [GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.42.1)]
    $ ./python.exe -m venv ~/.envs/issue22490
    Error: Command '['/Users/jaraco/.envs/issue22490/bin/python.exe', '-Im', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 1.

    Aah. The issue is probably no zlib.

    $ curl https://bootstrap.pypa.io/get-pip.py -o - | ~/.envs/issue22490/bin/python
      % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                     Dload  Upload   Total   Spent    Left  Speed
    100 1558k  100 1558k    0     0  5221k      0 --:--:-- --:--:-- --:--:-- 5210k
    Traceback (most recent call last):
      File "<stdin>", line 20061, in <module>
      File "<stdin>", line 194, in main
      File "<stdin>", line 82, in bootstrap
    zipimport.ZipImportError: can't decompress data; zlib not available

    After making sure I had zlib installed and available, by re-running xcode-select --install, I was able to build with zlib, create a venv, and install packages to it:

    $ ./python.exe -m venv ~/.envs/issue22490 
    $ ~/.envs/issue22490/bin/python -c "import os; os.environ['__PYVENV_LAUNCHER__']"
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/Users/jaraco/Dropbox/code/public/cpython/Lib/os.py", line 669, in __getitem__
        raise KeyError(key) from None
    KeyError: '__PYVENV_LAUNCHER__'
    $ ~/.envs/issue22490/bin/pip install cherrypy
    Collecting cherrypy
      Using cached CherryPy-8.5.0-py2.py3-none-any.whl
    Collecting six (from cherrypy)
      Using cached six-1.10.0-py2.py3-none-any.whl
    Installing collected packages: six, cherrypy
    Successfully installed cherrypy-8.5.0 six-1.10.0
    $ ~/.envs/issue22490/bin/python -m cherrypy
    ...
    [27/Dec/2016:11:58:09] ENGINE Serving on http://127.0.0.1:8080
    [27/Dec/2016:11:58:09] ENGINE Bus STARTED

    What else would I need to test? What might one expect to fail due to the lack of that environment variable?

    @vsajip
    Copy link
    Member

    vsajip commented Dec 27, 2016

    Thanks. It's worth checking what shebang was written to a script that was installed into the venv - e.g. cherryd.

    @jaraco
    Copy link
    Member

    jaraco commented Dec 27, 2016

    That also looks good.

    $ head -n1 ~/.envs/issue22490/bin/cherryd
    #!/Users/jaraco/.envs/issue22490/bin/python

    @vsajip
    Copy link
    Member

    vsajip commented Dec 31, 2016

    I tried making a Framework build of Python with the environment variable removed entirely, but I'm not experienced enough with builds on a Mac to know what I need to do to create a proper build.

    Given Jason's comment above, it's probably best for one of the Mac-expert committers to remove the environment variable altogether from Mac/Tools/pythonw.c:main(), and confirm everything still builds and runs OK, and then for Tim to confirm that Homebrew isn't adversely impacted by the change.

    @gvanrossum
    Copy link
    Member

    I was pointed here after we found some erroneous behavior.

    We have a script run by the system python that invokes a specific venv's python3 with a -m flag requesting a module that is installed (only) in that venv. A user complained that this failed, with the venv's python3 claiming the module was not installed (but it was, as proved by manually running it with the same -m flag).

    Eventually someone realized that this was because the system python was a python3 installed by homebrew -- somehow this caused the system python (being python3) set __PYVENV_LAUNCHER__, and that made the venv's python3 ignore the venv's site-packages and instead look in the system python's site-packages.

    This feels very wrong.

    Maybe this is just a clearer description of https://bugs.python.org/issue31363? But that gained no traction while here there is at least some discussion.

    @vsajip
    Copy link
    Member

    vsajip commented Sep 22, 2018

    As I said in msg284106, it seems that the __PYVENV_LAUNCHER__ should be removed from the stub launcher and then tests should confirm that framework builds aren't adversely affected. Unfortunately, I can't do this testing as I don't have a Mac, and have been hoping that one of the Mac platform experts (or indeed anyone who can build and test framework builds) can look at this.

    @jaraco
    Copy link
    Member

    jaraco commented Sep 22, 2018

    I went naively through the codebase and removed every reference to PYVENV_LAUNCHER and submitted as PR 9498. Does the CI runner at https://python.visualstudio.com/cpython/_build/results?buildId=31019&view=logs use a framework build such that it's an adequate test of the change?

    @ronaldoussoren
    Copy link
    Contributor

    The pull request is wrong, the use of __PYVENV_LAUNCHER__ in pythonw.c is correct and should not be removed. Where the current code goes wrong is that it doesn't clear the environment as soon as possible.

    This patch should basically do the trick (but requires testing and probably adjustments to some other code):

    diff --git a/Modules/main.c b/Modules/main.c
    index 3a6cf31fc8..c673d06c56 100644
    --- a/Modules/main.c
    +++ b/Modules/main.c
    @@ -1360,6 +1360,8 @@ config_init_program_name(_PyCoreConfig *config)
                                              "variable", (Py_ssize_t)len);
                 }
                 config->program_name = program_name;
    +
    +           unsetenv("__PYVENV_LAUNCHER__");
                 return _Py_INIT_OK();
             }
         }

    Some background information on why and environment variable is used on macOS when using a framework install: To use system GUI frameworks on macOS the executable should be part of an application bundle (".app"), otherwise a number of APIs just don't work (which would affect the use of for example Tkinter in command-line scripts). To work around this platform limitation a framework install of Python has the actual interpreter in a Python.app bundle stored in the framework, and "{sys.prefix}/bin/python" is a small stub executable that launches the real interpreter.

    The environment variable is used to pass the path to the stub executable to the real interpreter, to enable it to behave correctly (look at venv configuration files, have sys.executable be correct, ...).

    That said, looking at the code it might be possible to do away with the environment variable after call because the code in pythonw.c suggests the the environment variable is only necessary for OSX 10.5 or earlier.

    I'm working on an alternative pull request that implements the patch I included inline in this message, with some additional documentation.

    @ronaldoussoren
    Copy link
    Contributor

    I've added PR 9516 that just clears __PYVENV_LAUNCHER__ from the environment as soon as it is no longer needed.

    A more involved change would be to change both the interpreter and the stub executable to avoid the need to use an environment variable in the first place:

    • Add and "-X" flag to the interpreter to pass the information that's currently passed using an environment variable
    • Change pythonw.c to add this new option to the argv vector.

    This would be slightly cleaner, at the cost of having more complicated code (and is a change that would IMHO not qualify for a back port, while my current PR is minimal enough for a back port)

    Note that both with and without this PR sys.executable points to the python executable inside the venv (when using a env).

    @jaraco
    Copy link
    Member

    jaraco commented Sep 23, 2018

    Is it documented anywhere how to do a framework build of Python? When I try to do a framework build by running ./configure --enable-framework then make, ./python.exe emits the following:

    dyld: Library not loaded: /Library/Frameworks/Python.framework/Versions/3.8/Python
    Referenced from: /Users/jaraco/code/public/cpython/./python.exe
    Reason: image not found
    Aborted (core dumped)

    @ronaldoussoren
    Copy link
    Contributor

    To use the framework build you either have to install, use or set "DYLD_FRAMEWORK_PATH" in the environment (See the definition of RUNFORSHARED in Makefile for the value to use).

    To properly test the this issue you have to install (or trick the system into thinking there is an install: I created a symlink in /Library/Frameworks/Python.framework/Versions: /Library/Frameworks/Python.framework/Versions/3.8 -> /Users/ronald/Projects/python/github/cpython-ronald/build/Python.framework/Versions/3.8).

    @gvanrossum
    Copy link
    Member

    Jason if you could test this we would be grateful!

    Ronald what do you think of marking this as backportable to 3.7, 3.6 and
    3.5?

    @ronaldoussoren
    Copy link
    Contributor

    I've added back port labels for 3.6 and 3.7.

    AFAIK 3.5 is closed for bugfixes at this point (except for security fixes).

    @gvanrossum
    Copy link
    Member

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

    @ronaldoussoren
    Copy link
    Contributor

    I had the some problem when using mypy. What does reproduce the issue is the following:

    • Create *two* venv environments
    • Install mypy in one of them
    • Activate the other
    • Run the subprocess call in the activated venv

    I then get a message about not being able to find mypy:

    .../some-env/bin/python3: No module named mypy.dmypy

    I don't get the message when I call subprocess outside of a venv.

    I haven't checked yet if my patch fixes this issue, that will have to wait for later this week.

    @gvanrossum
    Copy link
    Member

    Thanks, I have confirmed that this reproduces our issue and that your PR fixes it. I guess at this point we need someone to test with Brew.

    @boxed
    Copy link
    Mannequin

    boxed mannequin commented Mar 22, 2019

    I just discovered this ticket again and see that it's stuck!

    I have read through the thread but it's still a bit unclear what would be required to test this with homebrew like Guido says is needed for this to go forward. Is there anyone who can explain or better yet test?

    @csabella
    Copy link
    Contributor

    It looks like @ronaldoussoren's pull request was ready to go pending a test with Brew. The PR itself needs a rebase, but is anyone able to finish the testing on this for it to be merged? Thanks!

    @csabella csabella added 3.7 (EOL) end of life 3.8 only security fixes labels May 17, 2019
    @jaraco
    Copy link
    Member

    jaraco commented Mar 22, 2020

    New changeset 044cf94 by Ronald Oussoren in branch 'master':
    bpo-22490: Remove __PYVENV_LAUNCHER__ from environment during launch (GH-9516)
    044cf94

    @jaraco
    Copy link
    Member

    jaraco commented Mar 22, 2020

    New changeset c959fa9 by Miss Islington (bot) in branch '3.8':
    bpo-22490: Remove __PYVENV_LAUNCHER__ from environment during launch (GH-9516) (GH-19110)
    c959fa9

    @jaraco
    Copy link
    Member

    jaraco commented Mar 22, 2020

    New changeset 5765aca by Jason R. Coombs in branch '3.7':
    [3.7] bpo-22490: Remove __PYVENV_LAUNCHER__ from environment during launch (GH-9516) (GH-19111)
    5765aca

    @jaraco jaraco closed this as completed Mar 22, 2020
    @jaraco jaraco added the 3.9 only security fixes label Mar 22, 2020
    @jaraco
    Copy link
    Member

    jaraco commented Mar 22, 2020

    I've filed Homebrew/brew#7204 to track the testing/validation of this change against Homebrew.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes OS-mac type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants