-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
getpath problems with framework build #91046
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
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. |
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: In python3.11: |
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. |
This may be related to the getpath.py work Steve did. |
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. |
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. |
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. |
It shouldn't be, but site.py might do it. More likely it's in how getpath.py is handling the environment variable If there's a way to add a test for this case as well, that would be |
There's definitely a way to add a test now, because test_getpath uses |
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. |
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? |
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', 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. |
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. |
Ok, marking is at "deferred blocker" |
The getpath.py change is probably adding an OS check for this line: 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 |
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)? |
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) |
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 Note how the code checks for the standard library outside of the framework itself. |
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. |
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.
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. |
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:
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 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. |
Might be worth changing it then. I double/triple checked whether My assumption was that any higher-level directories in that tree would |
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: ``
before reconfig: config->prefix = (null)
|
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? |
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. |
Honestly, no idea. I just copied how it was designed before, and had At a guess, I think virtualenv did everything with a patched site.py, so |
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? |
venv requiring site isn't really a problem, although it is a bit weird that sys.prefix changes when using the -S flag. |
|
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 |
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: