Skip to content

Conversation

@mattem
Copy link
Collaborator

@mattem mattem commented Jun 2, 2022

On MacOS (or at least what I'm running), it has some odd semantics around how /usr/bin/python behaves which appear to depend on the name of the binary itself 🙄

If the autodetecting toolchain links /usr/bin/python, then the shim will call python_real, which in turn gets called by whatever the binary at /usr/bin/python is, and as python_real isn't a binary it can call, it decides it needs to install Xcodes command line utilities. This of course doesn't help, and the cycle continues.

Instead, link to python3, and only link in python into the venv/bin as the others were kinda extraneous anyway.

@mattem mattem requested a review from alexeagle June 2, 2022 02:05
exec "${PYTHON_BIN}" "$@"
else
PYTHON_REAL="${VIRTUAL_ENV}/bin/python_real"
PYTHON_REAL="${VIRTUAL_ENV}/bin/python3"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some comments or test cases so we know why it has to be this way? (similar to the commit description?)

@jvolkman
Copy link

jvolkman commented Jun 2, 2022

It's hard to tell from the description, but this might be what the PYTHONEXECUTABLE env var helps with. It's a macOS-specific thing that I've had to use in the past when dealing with Python wrappers.

For lack of a better thing to link to: https://github.com/python/cpython/blob/8aa9d40b00741213c5a53b1ae15509998893ae31/Misc/python.man#L578-L582

which says

PYTHONEXECUTABLE
If this environment variable is set, sys.argv[0] will be set to its value instead of the value got through the C runtime. Only works on Mac OS X.

@mattem
Copy link
Collaborator Author

mattem commented Jun 3, 2022

Hmm, I'm not sure about PYTHONEXECUTABLE, it just seems to be setting sys.arv[0], which isn't really what I want (also as we run in isolated mode, we can't pick up this env var).
I'll give it a go though, perhaps I'm misunderstanding the doc.

@mattem
Copy link
Collaborator Author

mattem commented Jun 3, 2022

This is what I get on my local Mac:

$ which python
/usr/bin/python

$ ln -snf /usr/bin/python3 ./python

$ ./python --version
Python 2.7.16

$ ln -snf /usr/bin/python3 ./python3

$ ./python3 --version
Python 3.8.9

$ export PYTHONEXECUTABLE=/usr/bin/python3; ./python --version
Python 2.7.16

$ export PYTHONEXECUTABLE=python3; ./python --version
Python 2.7.16

$ ln -snf /usr/bin/python3 ./python_real
$ ./python_real
xcode-select: Failed to locate 'python_real', requesting installation of command line developer tools.

@jvolkman
Copy link

jvolkman commented Jun 3, 2022

Hmm yeah, this is different. /usr/bin/python3 is some launcher that actually invokes /Library/Developer/CommandLineTools/usr/bin/python3. When I do e.g. /usr/bin/python3 -m venv /tmp/env, /tmp/env/bin/python3 links to that executable under /Library/...

So I guess you could also set the symlink target to the output of e.g. $(/usr/bin/python3 -c 'import sys; print(sys.executable)')

@mattem
Copy link
Collaborator Author

mattem commented Jun 3, 2022

Hm yeah good point. That might help in breaking out into the wrong site-packages folder too. Let me give that a try.

@mattem mattem force-pushed the fix/python_vs_python3 branch from 9122b3c to 3241d2c Compare June 4, 2022 11:31
@mattem
Copy link
Collaborator Author

mattem commented Jun 4, 2022

I like this fix better as it also resolves the issue where the autodetecting toolchain could in some cases pick up the hosts site-packages folder, even with the symlink trickery. Now we resolve back to the actual Python interpreter binary at toolchain resolution time (I mean, it's still kinda wrong, but as host toolchains go, it's fine) and the symlink trickery works in all cases as the interpreter itself is not a wrapper or another symlink, so the base prefix is always set correctly.

@mattem mattem merged commit 6c90da2 into main Jun 4, 2022
@mattem mattem deleted the fix/python_vs_python3 branch June 4, 2022 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants