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

getpath problems with framework build #91046

Closed
ronaldoussoren opened this issue Mar 1, 2022 · 31 comments
Closed

getpath problems with framework build #91046

ronaldoussoren opened this issue Mar 1, 2022 · 31 comments
Labels
3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-mac release-blocker stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ronaldoussoren
Copy link
Contributor

BPO 46890
Nosy @vsajip, @ronaldoussoren, @ned-deily, @ericsnowcurrently, @eryksun, @zooba, @pablogsal
PRs
  • bpo-46890: Fix setting of sys._base_executable with framework builds on macOS #31958
  • Files
  • issue-46890.txt
  • issue-46890-v2.txt
  • issue-46890-v3.txt
  • 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 = None
    closed_at = None
    created_at = <Date 2022-03-01.16:25:46.271>
    labels = ['OS-mac', 'interpreter-core', 'deferred-blocker', 'type-bug', '3.11', 'library']
    title = 'getpath problems with framework build'
    updated_at = <Date 2022-04-05.06:05:44.693>
    user = 'https://github.com/ronaldoussoren'

    bugs.python.org fields:

    activity = <Date 2022-04-05.06:05:44.693>
    actor = 'ned.deily'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Interpreter Core', 'Library (Lib)', 'macOS']
    creation = <Date 2022-03-01.16:25:46.271>
    creator = 'ronaldoussoren'
    dependencies = []
    files = ['50660', '50662', '50669']
    hgrepos = []
    issue_num = 46890
    keywords = ['patch', '3.11regression']
    message_count = 29.0
    messages = ['414278', '414281', '414282', '414285', '414288', '414290', '414291', '414336', '414337', '414341', '414348', '414354', '414355', '414365', '414373', '414659', '414665', '414667', '414669', '414676', '414699', '414701', '415033', '415175', '415334', '415335', '415343', '415346', '416755']
    nosy_count = 7.0
    nosy_names = ['vinay.sajip', 'ronaldoussoren', 'ned.deily', 'eric.snow', 'eryksun', 'steve.dower', 'pablogsal']
    pr_nums = ['31958']
    priority = 'deferred blocker'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue46890'
    versions = ['Python 3.11']

    @ronaldoussoren
    Copy link
    Contributor Author

    In python3.10 and earlier "python3 -m venv something" creates "something/bin/python" as a symlink to the interpreter itself.

    In python3.11 (a5) the same command no longer creates "something/bin/python", but only the ".../python3" symlink.

    With this change the "python" command no longer refers to the interpreter in an activated virtualenv, but to a different binary on $PATH (if any).

    I tested using the Python 3.11a5 installer for macOS on python.org.

    @ronaldoussoren ronaldoussoren added 3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Mar 1, 2022
    @zware zware changed the title vent does not create "python" link in python 3.11 venv does not create "python" link in python 3.11 Mar 1, 2022
    @zware zware changed the title vent does not create "python" link in python 3.11 venv does not create "python" link in python 3.11 Mar 1, 2022
    @ronaldoussoren
    Copy link
    Contributor Author

    This is be related to how the virtual environment is populated. In 3.10 the "python3.10" name in the environment is a symlink to sys.executable. In 3.10 "Python" (not the capital) is a symlink to the binary in Python.app and "python3.11" is a symlink to "Python". Given that the filesystem is case preserving there is no "python" name.

    The behaviour in 3.11 is clearly a bug: the virtual environment is no longer using the launcher binary in "{sys.prefix]/bin" but directly uses the "real" binary (in a framework build), and because of that scripts cannot use system APIs that expect to run in an application bundle.

    Listing "env/bin" in Python 3.10:
    -rw-rw-r-- 1 ronald staff 9033 Mar 1 18:11 Activate.ps1
    -rw-rw-r-- 1 ronald staff 2018 Mar 1 18:11 activate
    -rw-rw-r-- 1 ronald staff 944 Mar 1 18:11 activate.csh
    -rw-rw-r-- 1 ronald staff 2086 Mar 1 18:11 activate.fish
    -rwxr-xr-x 1 ronald staff 269 Mar 1 18:11 pip
    -rwxr-xr-x 1 ronald staff 269 Mar 1 18:11 pip3
    -rwxr-xr-x 1 ronald staff 269 Mar 1 18:11 pip3.10
    lrwxr-xr-x 1 ronald staff 10 Mar 1 18:11 python -> python3.10
    lrwxr-xr-x 1 ronald staff 10 Mar 1 18:11 python3 -> python3.10
    lrwxr-xr-x 1 ronald staff 65 Mar 1 18:11 python3.10 -> /Library/Frameworks/Python.framework/Versions/3.10/bin/python3.10

    In python3.11:
    total 72
    -rw-rw-r-- 1 ronald staff 9033 Mar 1 17:23 Activate.ps1
    lrwxr-xr-x 1 ronald staff 93 Mar 1 17:23 Python -> /Library/Frameworks/Python.framework/Versions/3.11/Resources/Python.app/Contents/MacOS/Python
    -rw-rw-r-- 1 ronald staff 2018 Mar 1 17:23 activate
    -rw-rw-r-- 1 ronald staff 944 Mar 1 17:23 activate.csh
    -rw-rw-r-- 1 ronald staff 2086 Mar 1 17:23 activate.fish
    -rwxr-xr-x 1 ronald staff 265 Mar 1 17:23 pip
    -rwxr-xr-x 1 ronald staff 265 Mar 1 17:23 pip3
    -rwxr-xr-x 1 ronald staff 265 Mar 1 17:23 pip3.11
    lrwxr-xr-x 1 ronald staff 6 Mar 1 17:23 python3 -> Python
    lrwxr-xr-x 1 ronald staff 6 Mar 1 17:23 python3.11 -> Python

    @ronaldoussoren
    Copy link
    Contributor Author

    The root cause likely is the calculation of sys._base_executable. In 3.10 this is {sys.prefix}/bin/python3.10, while in 3.11 this is {sys.prefix}/Resources/Python.app/Contents/MacOS/Python.

    The venv library uses the incorrect value and therefore creates an incorrect virtual environment.

    @ronaldoussoren ronaldoussoren added interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Mar 1, 2022
    @ericsnowcurrently
    Copy link
    Member

    This may be related to the getpath.py work Steve did.

    @zooba
    Copy link
    Member

    zooba commented Mar 1, 2022

    The _base_executable change might be, though it should still be preferring the environment variables here, but I don't think I touched anything that would affect which symlinks are created by venv.

    @ned-deily
    Copy link
    Member

    As Ronald notes, the issue isn't in venv, it's that the value of sys._base_executable has changed between 3.10 and 3.11 for macOS builds.

    $ /usr/local/bin/python3.10
    Python 3.10.2 (v3.10.2:a58ebcc701, Jan 13 2022, 14:50:16) [Clang 13.0.0 (clang-1300.0.29.30)] on darwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import sys
    >>> sys._base_executable
    '/usr/local/bin/python3.10'
    >>> ^D
    nad@vana:~$ /usr/local/bin/python3.11
    Python 3.11.0a5 (v3.11.0a5:c4e4b91557, Feb  3 2022, 14:54:01) [Clang 13.0.0 (clang-1300.0.29.30)] on darwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import sys
    >>> sys._base_executable
    '/Library/Frameworks/Python.framework/Versions/3.11.0a5_11/Resources/Python.app/Contents/MacOS/Python'
    >>>

    The 3.11 value is incorrect for the reasons Ronald noted.

    @ronaldoussoren
    Copy link
    Contributor Author

    The change to _base_executable is the real problem. Venv creates the correct directory structure if I add a sitecustomize.py to the python path that sets _base_executable to the correct value.

    # sitecustomize
    import sys
    sys._base_executable = sys.executable
    # EOF

    Is sys._base_executable updated after running getpath.py? On first glance getpath.py does update config['base_executable'] and _PyConfig_FromDict reads that value back, but that's based on a quick scan through the code. I haven't tried debugging this yet.

    @zooba
    Copy link
    Member

    zooba commented Mar 2, 2022

    Is sys._base_executable updated after running getpath.py?

    It shouldn't be, but site.py might do it.

    More likely it's in how getpath.py is handling the environment variable
    overrides. It should be pretty easy to track down and change - the old
    "logic" was certainly not.

    If there's a way to add a test for this case as well, that would be
    handy. Way too little documentation here.

    @zooba
    Copy link
    Member

    zooba commented Mar 2, 2022

    If there's a way to add a test for this case as well, that would be
    handy. Way too little documentation here.

    There's definitely a way to add a test now, because test_getpath uses
    completely fake initial conditions to verify that getpath.py does its
    thing. So it's just a matter of understanding exactly what conditions
    lead to this point.

    @ned-deily
    Copy link
    Member

    To reiterate, the example I gave had nothing to do with using a venv. The value of sys._base_executable is now always wrong whether or not using a venv. The consequences are more obvious when using a venv.

    @pablogsal
    Copy link
    Member

    This is marked as a release blocker so I am holding the alpha release on this. Is there anything we can do to unblock this issue?

    @ronaldoussoren
    Copy link
    Contributor Author

    Again without diving deep I've constructed a test case that is probably relevant. I've pasted the diff below because I'm not yet convinced that it is correct (in particular the value for "argv0". This would also require a second test case that does something similar for a venv when using a framework build (the test_venv_macos case seems to be for a regular install and not a framework install)

    With this tests case I get a test failure that matches my observations:

    test test_getpath failed -- Traceback (most recent call last):
      File "/Users/ronald/Projects/Forks/cpython/Lib/test/test_getpath.py", line 444, in test_framework_python
        self.assertEqual(expected, actual)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    AssertionError: {'exe[273 chars]/9.8/bin/python9.8', 'base_prefix': '/Library/[381 chars]ad']} != {'exe[273 chars]/9.8/Resources/Python.app/Contents/MacOS/Pytho[410 chars]ad']}
      {'base_exec_prefix': '/Library/Frameworks/Python.framework/Versions/9.8',
    -  'base_executable': '/Library/Frameworks/Python.framework/Versions/9.8/bin/python9.8',
    ?                                                                        ^^^ ^     - ^

    + 'base_executable': '/Library/Frameworks/Python.framework/Versions/9.8/Resources/Python.app/Contents/MacOS/Python',
    ? ^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^

    'base_prefix': '/Library/Frameworks/Python.framework/Versions/9.8',
    'exec_prefix': '/Library/Frameworks/Python.framework/Versions/9.8',
    'executable': '/Library/Frameworks/Python.framework/Versions/9.8/bin/python9.8',
    'module_search_paths': ['/Library/Frameworks/Python.framework/Versions/9.8/lib/python98.zip',
    '/Library/Frameworks/Python.framework/Versions/9.8/lib/python9.8',
    '/Library/Frameworks/Python.framework/Versions/9.8/lib/python9.8/lib-dynload'],
    'module_search_paths_set': 1,
    'prefix': '/Library/Frameworks/Python.framework/Versions/9.8'}

    test_getpath failed (1 failure)

    The inline diff (and as mentioned before, I'm not sure if the value for "argv0" is correct):

    %  git diff ../Lib/test/test_getpath.py                                                                                                                     (main)cpython
    diff --git a/Lib/test/test_getpath.py b/Lib/test/test_getpath.py
    index 3fb1b28003..69b469f179 100644
    --- a/Lib/test/test_getpath.py
    +++ b/Lib/test/test_getpath.py
    @@ -414,6 +414,36 @@ def test_custom_platlibdir_posix(self):
             actual = getpath(ns, expected)
             self.assertEqual(expected, actual)
     
    +    def test_framework_python(self):
    +        """ Test framework layout on macOS """
    +        ns = MockPosixNamespace(
    +            os_name="darwin",
    +            argv0="/Library/Frameworks/Python.framework/Versions/9.8/Resources/Python.app/Contents/MacOS/Python",
    +            PREFIX="/Library/Frameworks/Python.framework/Versions/9.8",
    +            ENV___PYVENV_LAUNCHER__="/Library/Frameworks/Python.framework/Versions/9.8/bin/python9.8",
    +            real_executable="/Library/Frameworks/Python.framework/Versions/9.8/Resources/Python.app/Contents/MacOS/Python",
    +        )
    +        ns.add_known_xfile("/Library/Frameworks/Python.framework/Versions/9.8/Resources/Python.app/Contents/MacOS/Python")
    +        ns.add_known_xfile("/Library/Frameworks/Python.framework/Versions/9.8/bin/python9.8")
    +        ns.add_known_xfile("/Library/Frameworks/Python.framework/Versions/9.8/lib/python9.8/lib-dynload")
    +        expected = dict(
    +            executable="/Library/Frameworks/Python.framework/Versions/9.8/bin/python9.8",
    +            prefix="/Library/Frameworks/Python.framework/Versions/9.8",
    +            exec_prefix="/Library/Frameworks/Python.framework/Versions/9.8",
    +            base_executable="/Library/Frameworks/Python.framework/Versions/9.8/bin/python9.8",
    +            base_prefix="/Library/Frameworks/Python.framework/Versions/9.8",
    +            base_exec_prefix="/Library/Frameworks/Python.framework/Versions/9.8",
    +            module_search_paths_set=1,
    +            module_search_paths=[
    +                "/Library/Frameworks/Python.framework/Versions/9.8/lib/python98.zip",
    +                "/Library/Frameworks/Python.framework/Versions/9.8/lib/python9.8",
    +                "/Library/Frameworks/Python.framework/Versions/9.8/lib/python9.8/lib-dynload",
    +            ],
    +        )
    +        actual = getpath(ns, expected)
    +        self.assertEqual(expected, actual)
    +
    +

    I may work on a PR later this week, but can't promise anything. My energy level is fairly low at the moment (for various reasons).

    BTW. The code in getpath.py, and tests, also don't seem to handle different values for PYTHONFRAMEWORK, I sometimes use that to have a debug framework install next to a regular install. I'll file a separate bug for that when I get around to testing this.

    @ronaldoussoren
    Copy link
    Contributor Author

    This is marked as a release blocker so I am holding the alpha release on this. Is there anything we can do to unblock this issue?

    I marked the issue as a release blocker because it must not end up in a beta release, its probably not worth it to hold up an alpha release for this.

    @pablogsal
    Copy link
    Member

    Ok, marking is at "deferred blocker"

    @zooba
    Copy link
    Member

    zooba commented Mar 2, 2022

    I've pasted the diff below because I'm not yet convinced that it is correct (in particular the value for "argv0".)
    argv0 is literally what C sees in argv[0], which in the framework case I
    believe is calculated by a launcher?

    The getpath.py change is probably adding an OS check for this line:
    https://github.com/python/cpython/blob/main/Modules/getpath.py#L304

    if ENV_PYTHONEXECUTABLE or ENV___PYVENV_LAUNCHER__:
         # If set, these variables imply that we should be using them as
         # sys.executable and when searching for venvs. However, we should
         # use the argv0 path for prefix calculation
         if os_name != 'darwin':
             base_executable = executable
         if not real_executable:
             real_executable = executable
         executable = ENV_PYTHONEXECUTABLE or ENV___PYVENV_LAUNCHER__
         executable_dir = dirname(executable)

    I think the comment "we should use the argv0 path for prefix
    calculation" isn't correct (anymore?) either, since updating
    executable_dir is going to do the opposite of that. But I guess there
    were tests proving otherwise. It won't affect this case, since both
    argv0 and ENV___PYVENV_LAUNCHER__ appear to be under the same prefix (at
    least in Ronald's example).

    @ronaldoussoren
    Copy link
    Contributor Author

    I have a patch that seems to do the right thing. It required adding WITH_NEXT_FRAMEWORK to the globals when evaluating getpath.py to detect this scenario.

    There probably should be more tests, in particular a test for a virtual environment using a framework install as I had to adjust my patch after manually testing a virtual environment.

    I'm not entirely happy with the patch yet, the change to getpath.py is basically the minimal change I could get away with without grokking the entire file.

    There's no PR yet as I've broken my fork of cpython, fixing that is next up on the list.

    BTW. Is there a way to find the values that get put into the globals dict for getpath.py without using a debugger (either a real one or the printf variant)?

    @ronaldoussoren
    Copy link
    Contributor Author

    I'm now in the fun position where the code works, but the test test_getpath.py fails.

    The test case below tries to test a venv in a framework build, but somehow fails to calculate prefix and exec_prefix correctly. The calculation does work when using a framework build with my patch...

        def test_venv_framework_macos(self):
            """Test a venv layout on macOS using a framework build
            """
            venv_path = "/tmp/workdir/venv"
            ns = MockPosixNamespace(
                os_name="darwin",
                argv0="/Library/Frameworks/Python.framework/Versions/9.8/Resources/Python.app/Contents/MacOS/Python",
                WITH_NEXT_FRAMEWORK=1,
                PREFIX="/Library/Frameworks/Python.framework/Versions/9.8",
                EXEC_PREFIX="/Library/Frameworks/Python.framework/Versions/9.8",
                ENV___PYVENV_LAUNCHER__=f"{venv_path}/bin/python",
                real_executable="/Library/Frameworks/Python.framework/Versions/9.8/Resources/Python.app/Contents/MacOS/Python",
                library="/Library/Frameworks/Python.framework/Versions/9.8/Python",
            )
            ns.add_known_dir(venv_path)
            ns.add_known_xfile(f"{venv_path}/bin/python")
            ns.add_known_dir(f"{venv_path}/lib/python9.8")
            ns.add_known_xfile("/Library/Frameworks/Python.framework/Versions/9.8/Resources/Python.app/Contents/MacOS/Python")
            ns.add_known_xfile("/Library/Frameworks/Python.framework/Versions/9.8/bin/python9.8")
            ns.add_known_dir("/Library/Frameworks/Python.framework/Versions/9.8/lib/python9.8/lib-dynload")
            ns.add_known_xfile("/Library/Frameworks/Python.framework/Versions/9.8/lib/python9.8/os.py")
            ns.add_known_file(f"{venv_path}/pyvenv.cfg", [
                "home = /Library/Frameworks/Python.framework/Versions/9.8/bin"
            ])
            expected = dict(
                executable=f"{venv_path}/bin/python",
                prefix=venv_path,
                exec_prefix=venv_path,
                base_executable="/Library/Frameworks/Python.framework/Versions/9.8/bin/python9.8",
                base_prefix="/Library/Frameworks/Python.framework/Versions/9.8",
                base_exec_prefix="/Library/Frameworks/Python.framework/Versions/9.8",
                module_search_paths_set=1,
                module_search_paths=[
                    "/Library/Frameworks/Python.framework/Versions/9.8/lib/python98.zip",
                    "/Library/Frameworks/Python.framework/Versions/9.8/lib/python9.8",
                    "/Library/Frameworks/Python.framework/Versions/9.8/lib/python9.8/lib-dynload",
                ],
            )
            actual = getpath(ns, expected)
            self.assertEqual(expected, actual)

    @ronaldoussoren
    Copy link
    Contributor Author

    This is also dodgy:

    test_framework_macos (test.test_getpath.MockGetPathTests) ... Read link from /Library/Frameworks/Python.framework/Versions/9.8/Resources/Python.app/Contents/MacOS/Python
    Check if /Library/Frameworks/Python.framework/Versions/9.8/Resources/Python.app/Contents/MacOS/Modules/Setup.local is a file
    Check if /Library/Frameworks/Python.framework/Versions/9.8/lib/python98.zip is a file
    Check if /Library/Frameworks/Python.framework/Versions/lib/python98.zip is a file
    Check if /Library/Frameworks/Python.framework/lib/python98.zip is a file
    Check if /Library/Frameworks/lib/python98.zip is a file
    Check if /Library/lib/python98.zip is a file
    Check if /Library/Frameworks/Python.framework/Versions/9.8/lib/python9.8/os.py is a file
    Check if /Library/Frameworks/Python.framework/Versions/9.8/bin/lib/python9.8/lib-dynload is a dir
    Check if /Library/Frameworks/Python.framework/Versions/9.8/lib/python9.8/lib-dynload is a dir

    Note how the code checks for the standard library outside of the framework itself.

    @ronaldoussoren
    Copy link
    Contributor Author

    I've attached a new patch file with some more tweaks, including the additional test case from msg414665.

    I've pretty sure that the changes to getpath.py are slightly worse in the new version, but aren't fully correct in the initial version as well (see my previous message about searching outside of the framework).

    I may return to this later when I have time to try to really understand what's going on and compare the logic with the previous C code (especially for framework builds). But for now I'm giving up.

    @zooba
    Copy link
    Member

    zooba commented Mar 7, 2022

    I have a patch that seems to do the right thing. It required adding WITH_NEXT_FRAMEWORK to the globals when evaluating getpath.py to detect this scenario.

    I haven't had a chance to go through all your changes, and I'm only very vaguely familiar with this space anyway, but isn't WITH_NEXT_FRAMEWORK a compile-time option? I'm sure that's how it was used in the old getpath.

    Is there a way to find the values that get put into the globals dict for getpath.py without using a debugger

    warn(str(globals())) or some variant at the top of getpath.py should do it. There's also an "#if 0" section in getpath.c that will print it out (unless I deleted it before merging). There's nothing permanently built in to do this, as I suspect the security concerns (information leakage) would come up more often than the debugging concerns.

    @ronaldoussoren
    Copy link
    Contributor Author

    WITH_NEXT_FRAMEWORK is a compile time option, I've added it to globals in values like PREFIX are added. That way the python code can behave differently for framework builds (which appears to be needed).

    There are two big problems with my patches:

    • Calculation of sys.prefix doesn't work in test_venv_framework_macos, but somehow works with a real installation. I haven't managed to track down the difference yet.
    • Calculation for test_framework_macos appears to be ok on first glance, but adding "/Library/lib/python9.8.zip" as a known file shows that the code to look for the stdlib is misbehaving.

    The latter appears to be a wider problem, if I add a test case based on test_normal_posix with PREFIX=/opt/python9.8 the getpath code looks for /opt/lib/python98.zip and uses that when found.

    Test case for this (test passed when ns.add_known_file("/opt/lib/python98.zip") is commented out:

    def test_normal_posix_in_opt(self):
              """Test a 'standard' install layout on *nix
              
              This uses '/opt/python9.8' as PREFIX
              """
              ns = MockPosixNamespace(
                  PREFIX="/opt/python9.8",
                  argv0="python",
                  ENV_PATH="/usr/bin:/opt/python9.8/bin",
              )
              ns.add_known_xfile("/opt/python9.8/bin/python")
              ns.add_known_file("/opt/python9.8/lib/python9.8/os.py")
              ns.add_known_dir("/opt/python9.8/lib/python9.8/lib-dynload")
      
              # This shouldn't matter:
              ns.add_known_file("/opt/lib/python98.zip")
      
              expected = dict(
                  executable="/opt/python9.8/bin/python",
                  base_executable="/opt/python9.8/bin/python",
                  prefix="/opt/python9.8",
                  exec_prefix="/opt/python9.8",
                  module_search_paths_set=1,
                  module_search_paths=[
                      "/opt/python9.8/lib/python98.zip",
                      "/opt/python9.8/lib/python9.8",
                      "/opt/python9.8/lib/python9.8/lib-dynload",
                  ],
              )
              actual = getpath(ns, expected)
              self.assertEqual(expected, actual)

    This could be problematic, adding a suitably named file outside of $PREFIX breaks the python installation. I haven't checked this with an unchanged getpath.py yet, but I shouldn't have made any changes that affect a non-framework install.

    @zooba
    Copy link
    Member

    zooba commented Mar 7, 2022

    This could be problematic, adding a suitably named file outside of $PREFIX breaks the python installation.

    Might be worth changing it then. I double/triple checked whether
    searching up for the zip file was the old behaviour, and it sure seemed
    to be (it wasn't on Windows). Will only be a little tweak to change,
    since both codepaths are already there.

    My assumption was that any higher-level directories in that tree would
    be at least as restricted as where Python is installed, so anyone who
    could hijack it there could also have modified it closer to the actual file.

    @ronaldoussoren
    Copy link
    Contributor Author

    I've updated the title to better reflect the actual problem.

    An update on the current state of this issue:

    I haven't looked at the code for a couple of days because because I got stuck. With a fresh mind I've continued debugging and noticed that I'm looking in the wrong place...

    I've added some warn() calls to the end of getpath.py to print the updated variables:

    ``
    warn(f"END prefix: {config['prefix']}")
    warn(f"END exec_prefix: {config['exec_prefix']}")
    warn(f"END base_prefix: {config['base_prefix']}")
    warn(f"END base_exec_prefix: {config['base_exec_prefix']}")

    
    When I use this build with a framework build with and alternate name (``--enable-framework --with-framework-name=DebugPython``) and then create a venv in ``X/workenv`` I get expected output when I use the python in that venv:
    
    ``````sh
    $ X/workenv/bin/python -c 'import sys; print(f"ACTUAL prefix: {sys.prefix}\nACTUAL base_prefix: {sys.base_prefix}")' 
    END prefix: /Library/Frameworks/DebugPython.framework/Versions/3.11
    END exec_prefix: /Library/Frameworks/DebugPython.framework/Versions/3.11
    END base_prefix: /Library/Frameworks/DebugPython.framework/Versions/3.11
    END base_exec_prefix: /Library/Frameworks/DebugPython.framework/Versions/3.11
    ACTUAL prefix: /Users/ronald/Projects/Forks/cpython/build/X/workenv
    ACTUAL base_prefix: /Library/Frameworks/DebugPython.framework/Versions/3.11
    
    
    
    Note how "ACTUAL prefix" is different from "END prefix". 
    
    The weird bit is that the only reference to 'workenv' (the name of the venv) is in the value of config["executable"].
    
    I'm now convinced that sys.prefix is set after ``_PyConfig_InitPathConfig``, I've added some more debug prints around the call to ``_PyConfig_FromDict`` in ``_PyConfig_InitPathConfig`` and that prints:
    
    

    before reconfig: config->prefix = (null)
    before reconfig: sys.prefix = (not set)
    after reconfig: config->prefix = /Library/Frameworks/DebugPython.framework/Versions/3.11
    after reconfig: sys.prefix = (not set)

    
    I have no idea where sys.prefix get's initialised though, the configuration initialisation code could use some documentation. 
    
    I've attached a new version of my patch, still work in progress and including debug code. Definitely not ready for merging.
    
    
    In short: my patch (v3) seems to work, but I have no idea why.
    

    @ronaldoussoren ronaldoussoren changed the title venv does not create "python" link in python 3.11 getpath problems with framework build Mar 13, 2022
    @ronaldoussoren ronaldoussoren changed the title venv does not create "python" link in python 3.11 getpath problems with framework build Mar 13, 2022
    @zooba
    Copy link
    Member

    zooba commented Mar 14, 2022

    The sys module gets initialised in _PySys_UpdateConfig() in Python/sysmodule.c. It gets called later in pylifecycle.c. But it ought to just copy directly from the config.

    However, it's the site.py module that actually updates sys.prefix for the venv. So you may just be inspecting at the wrong point? Or possibly it's in a codepath that doesn't run on macOS because *previously* it was being set correctly in getpath instead of being deferred until later?

    @ronaldoussoren
    Copy link
    Contributor Author

    However, it's the site.py module that actually updates sys.prefix for the venv. So you may just be inspecting at the wrong point? Or possibly it's in a codepath that doesn't run on macOS because *previously* it was being set correctly in getpath instead of being deferred until later?

    I just noticed the same. Is this intentional? This means that "python -S" doesn't work with a virtual environment. I also noticed that site.py's venv support contains some special code for framework builds that shouldn't be necessary. But I guess that's something for some other tickets.

    I'm pretty sure I now just have to clean up my patch and create a PR from it.

    @zooba
    Copy link
    Member

    zooba commented Mar 16, 2022

    I just noticed the same. Is this intentional?

    Honestly, no idea. I just copied how it was designed before, and had
    nothing to do with the original venv implementation.

    At a guess, I think virtualenv did everything with a patched site.py, so
    when it was absorbed as venv it was retained. Changing this to calculate
    more in getpath.py was a goal of the rewrite, it just wasn't one that I
    wanted to do at the same time as the conversion, and am not particularly
    motivated to do myself anyway (when I hack install layouts, I don't use
    venvs at all ;) )

    @eryksun
    Copy link
    Contributor

    eryksun commented Mar 16, 2022

    This means that "python -S" doesn't work with a virtual environment.

    Does that matter? I've only used a virtual environment to override or extend the system site packages. Is there a reason to use one without site packages?

    @ronaldoussoren
    Copy link
    Contributor Author

    venv requiring site isn't really a problem, although it is a bit weird that sys.prefix changes when using the -S flag.

    @ned-deily
    Copy link
    Member

    New changeset 6aaf4cd by Ronald Oussoren in branch 'main':
    bpo-46890: Fix setting of sys._base_executable with framework builds on macOS (GH-31958)
    6aaf4cd

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @pablogsal
    Copy link
    Member

    ⚠️ This issue has been updated from 'deferred-blocker' to 'release blocker' as we are past beta1. This issue will block the next release (Python 3.11.0 beta 2). ⚠️

    @ronaldoussoren
    Copy link
    Contributor Author

    This issue itself has been fixed, one thing I still want to look into is one of my comments about looking for python311.zip outside of sys.prefix, but that should be a different issue (if I find anything).

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-mac release-blocker stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    Development

    No branches or pull requests

    6 participants