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

Virtualenvs created from links are broken #380

Open
konstin opened this issue Oct 23, 2024 · 14 comments
Open

Virtualenvs created from links are broken #380

konstin opened this issue Oct 23, 2024 · 14 comments
Labels
bug Something isn't working

Comments

@konstin
Copy link
Member

konstin commented Oct 23, 2024

Unlike a system interpreter or a pyenv installation, pbs can't create venv from a symlink to the pbs installation location. PYTHONHOME is in the wrong location (that of the link, i think), which causes the encoding error.

I've only tested linux, i expect windows isn't affected (different venv mechanism, no symlinks by default), but i expect macos is affected, too.

FROM ubuntu

RUN apt update && apt install -yy wget curl tar python3

# Install pbs in one dir
RUN mkdir /root/a
WORKDIR /root/a
RUN wget https://github.com/indygreg/python-build-standalone/releases/download/20241016/cpython-3.12.7+20241016-x86_64_v3-unknown-linux-gnu-install_only_stripped.tar.gz
RUN tar xf cpython-3.12.7+20241016-x86_64_v3-unknown-linux-gnu-install_only_stripped.tar.gz

# Go to another dir
RUN mkdir /root/b
WORKDIR /root/b

# Symlink the system interpreter for a level of indirection
RUN ln -s /usr/bin/python3.12 python3.12
RUN /usr/bin/python3.12 -m venv --without-pip python3.12-venv-direct
RUN ./python3.12 -m venv --without-pip python3.12-venv-link

# Symlink the pds interpreter for a level of indirection
RUN ln -s /root/a/python/bin/python3 pbs3.12
RUN /root/a/python/bin/python -m venv --without-pip pbs3.12-venv-direct
RUN ./pbs3.12 -m venv --without-pip pbs3.12-venv-link

CMD ["bash", "-c", "./python3.12-venv-direct/bin/python -c 'import sys; print(1, sys._base_executable)' \
    && ./python3.12-venv-link/bin/python -c 'import sys; print(2, sys._base_executable)' \
    && ./pbs3.12-venv-direct/bin/python -c 'import sys; print(3, sys._base_executable)' \
    && ./pbs3.12-venv-link/bin/python -c 'import sys; print(4, sys._base_executable)'"]
1 /usr/bin/python3.12
2 /usr/bin/python3.12
3 /root/a/python/bin/python3.12
Could not find platform independent libraries <prefix>
Could not find platform dependent libraries <exec_prefix>
Python path configuration:
  PYTHONHOME = (not set)
  PYTHONPATH = (not set)
  program name = './pbs3.12-venv-link/bin/python'
  isolated = 0
  environment = 1
  user site = 1
  safe_path = 0
  import site = 1
  is in build tree = 0
  stdlib dir = '/install/lib/python3.12'
  sys._base_executable = '/root/a/python/bin/python3.12'
  sys.base_prefix = '/install'
  sys.base_exec_prefix = '/install'
  sys.platlibdir = 'lib'
  sys.executable = '/root/b/pbs3.12-venv-link/bin/python'
  sys.prefix = '/install'
  sys.exec_prefix = '/install'
  sys.path = [
    '/install/lib/python312.zip',
    '/install/lib/python3.12',
    '/install/lib/python3.12/lib-dynload',
  ]
Fatal Python error: init_fs_encoding: failed to get the Python codec of the filesystem encoding
Python runtime state: core initialized
ModuleNotFoundError: No module named 'encodings'

Current thread 0x00007dce45bbb740 (most recent call first):
  <no Python frame>
@zanieb
Copy link
Member

zanieb commented Oct 23, 2024

Do you think this is a case of astral-sh/uv#8429 or are you suggesting we can fix this at build time?

@konstin
Copy link
Member Author

konstin commented Oct 23, 2024

I expect a sys variable is resolved incorrectly during interpreter startup, causing the wrong home in pyvenv.cfg. Ideally, we fix this in the pbs interpreter startup sequence itself, so that the venv module gets the right paths:

$ for file in *venv*/pyvenv.cfg; do echo && echo $file && cat $file; done

pbs3.12-venv-direct/pyvenv.cfg
home = /root/a/python/bin
include-system-site-packages = false
version = 3.12.7
executable = /root/a/python/bin/python3.12
command = /root/a/python/bin/python -m venv --without-pip /root/b/pbs3.12-venv-direct

pbs3.12-venv-link/pyvenv.cfg
home = /root/b
include-system-site-packages = false
version = 3.12.7
executable = /root/a/python/bin/python3.12
command = /root/b/pbs3.12 -m venv --without-pip /root/b/pbs3.12-venv-link

python3.12-venv-direct/pyvenv.cfg
home = /usr/bin
include-system-site-packages = false
version = 3.12.3
executable = /usr/bin/python3.12
command = /usr/bin/python3.12 -m venv --without-pip /root/b/python3.12-venv-direct

python3.12-venv-link/pyvenv.cfg
home = /root/b
include-system-site-packages = false
version = 3.12.3
executable = /usr/bin/python3.12
command = /root/b/python3.12 -m venv --without-pip /root/b/python3.12-venv-link```

@zanieb
Copy link
Member

zanieb commented Oct 23, 2024

I need a little more exposition alongside these examples, I don't follow what you're demonstrating there.

I would be surprised if this wasn't a sysconfig issue, but of course it'd be great to fix here if we can.

@konstin
Copy link
Member Author

konstin commented Oct 23, 2024

As shown, pbs creates broken venv when the base interpreter is a symlink. This is a bug, since it should be able to do this, as system interpreter and pyenv succeed at this.

The crash happens because home in pyvenv.cfg is wrong, which is derived from sys._base_executable.

https://github.com/python/cpython/blob/d48cc82ed25e26b02eb97c6263d95dcaa1e9111b/Lib/venv/__init__.py#L162-L175

https://github.com/python/cpython/blob/d48cc82ed25e26b02eb97c6263d95dcaa1e9111b/Lib/venv/__init__.py#L222

This bug is the most likely candidate to crash the test suite in astral-sh/uv#8481 (comment).

We need to figure out how sys._base_executable is set and at some level, patch it to the right value. What exactly is going on inside the sys module and why it only affects pbs is unclear at this point.

@charliermarsh
Copy link
Member

Interestingly, after symlinking, some of the sysconfig.get_config_vars() still point to the symlink location:

>>> sysconfig.get_config_vars()["base"]
'/Users/crmarsh/.local/share/uv/python/cpython-3.12.6-macos-aarch64-none'
>>> sysconfig.get_config_vars()["projectbase"]
'/Users/crmarsh/.local/share/uv/python/cpython-3.12.6-macos-aarch64-none/bin'

@ofek
Copy link

ofek commented Oct 24, 2024

That would definitely affect compilations of extension modules.

@charliermarsh
Copy link
Member

Ohh interesting, it's because Python does:

_PROJECT_BASE = os.path.dirname(_safe_realpath(sys.executable))

@indygreg
Copy link
Collaborator

I'm pretty sure I had to do something in PyOxidizer to work around this. Essentially mucking with the Python interpreter config to get it to resolve paths correctly because the default logic was insufficient.

We should consider involving a CPython maintainer on this one as the behavior is a smell. ISTR Windows having special logic for path resolution because portable installs are a thing on Windows. But since most UNIX-like installs are to a fixed path, CPython doesn't support dynamic paths as well as we'd like.

If CPython doesn't recognize the issue, we may have to patch our CPython build to handle dynamic install paths/symlibks correctly. This could be done with a custom site customize file. But since this included file can be disabled, we may need to patch C code.

@ncoghlan
Copy link

ncoghlan commented Dec 6, 2024

I haven't looked closely at this specific case, but we've definitely removed realpath calls from CPython before because resolving the symlink turned out to be the wrong thing to do (or added them, when things needed to be located relative to the underlying link target).

(Tangentially related, python/cpython#124825 is a proposal I filed to revisit adding lexical relative path resolution to pathlib, since I needed it to get venvstacks to work reliably on macOS. That's another project that uses python-build-standalone for its base runtime layers, so this could have been the underlying issue)

@charliermarsh
Copy link
Member

I'm looking into this a bit.

@charliermarsh
Copy link
Member

Ok, so for reference, the way prefix gets set in these builds is here.

If the executable is at /Users/crmarsh/python/install/bin/python, then executable_dir is /Users/crmarsh/python/install/bin and STDLIB_LANDMARKS is ['lib/python3.13/os.py', 'lib/python3.13/os.pyc']. So we look up the path and find /Users/crmarsh/python/install/lib/python3.13/os.py, and prefix gets set to /Users/crmarsh/python/install.

@charliermarsh
Copy link
Member

Ok... If you symlink an interpreter (but don't create a virtualenv), then executable_dir is still the realpath of the containing directory.

So, above, it would still be /Users/crmarsh/python/install/bin even if you symlinked /Users/crmarsh/python/install/bin/python to /Users/crmarsh/foo. That's why symlinked interpreters still work.

Specifically, when you're not in a virtualenv, the executable_dir gets set here.

However... if you create a virtualenv from a symlinked interpreter, then home gets set to the directory containing the symlink. Above, that would be home = /Users/crmarsh. In getpath.py, we'd then set executable_dir based on home here instead of reading from the realpath or similar.

So we'd then fail to find lib/python3.13/os.py when searching up the path, hence the failure.

@charliermarsh
Copy link
Member

charliermarsh commented Dec 9, 2024

I don't know if we should be setting home differently (in uv), or if we should be changing getpath.py to handle home differently.

@charliermarsh
Copy link
Member

charliermarsh commented Dec 9, 2024

One thing to note in the "symlink to ./bin/python" case: base_executable does not resolve the symlink, but executable_dir (and the resulting prefix) does.


Sorry, my own scratch notes...

When I symlink ./install/bin/python to ./foo:

  • base_executable: /Users/crmarsh/workspace/python-build-standalone/dist/foo
  • real_executable: /Users/crmarsh/workspace/python-build-standalone/dist/python/install/bin/python3.13
  • executable_dir: /Users/crmarsh/workspace/python-build-standalone/dist/python/install/bin
  • prefix: /Users/crmarsh/workspace/python-build-standalone/dist/python/install

When I symlink ./install to ./bar:

  • base_executable: /Users/crmarsh/workspace/python-build-standalone/dist/bar/bin/python
  • real_executable: /Users/crmarsh/workspace/python-build-standalone/dist/bar/bin/python3.13
  • executable_dir: /Users/crmarsh/workspace/python-build-standalone/dist/bar/bin
  • prefix: /Users/crmarsh/workspace/python-build-standalone/dist/bar

Ahhh, ok. The reason for this, I think, is that the realpath used in getpath.py only resolves a symlinked file, and not any path segments. That explains why the real_executable looks like this.


When we launch from a virtualenv, we take whatever is in home, and:

  • home becomes executable_dir (so we search for prefix from there).
  • base_executable is typically computed via realpath(executable). But if executable is not a symlink, then it's joinpath(executable_dir, basename(executable)), so home does have an impact in that latter case.

The combination of these behaviors means that whatever we put in home needs to be a valid search path for STDLIB_LANDMARKS.

We can solve this in two ways:

  1. virtualenv creators (like uv) could recognize that the base_executable is going to lead to an invalid home, then try real_executable instead (or, like, iteratively resolve the symlink and try to find the first valid home).
  2. getpath.py could be changed to ensure that we do resolve base_executable in the first case described above (symlink to a Python executable directly).

(1) seems easier to me, though (2) might be the "better" fix? I'm just not confident on how we would implement it or how it would affect other, non-PBS Pythons.

charliermarsh added a commit to astral-sh/uv that referenced this issue Dec 10, 2024
## Summary

This PR improves our "don't fully resolve symlinks" behavior for
`python-build-standalone` builds based on learnings from
astral-sh/python-build-standalone#380 (comment).

Specifically, we can now robustly detect whether a target executable
will lead to a valid `prefix` or not, and iteratively resolve symlinks
until we find a valid target executable.

## Test Plan

### Direct symlink to `python`

Correctly resolves to the symlink target, rather than the symlink
itself.

```
❯ ln -s /Users/crmarsh/.local/share/uv/python/cpython-3.12.6-macos-aarch64-none/bin/python foo
❯ cargo run venv --python ./foo
❯ cat .venv/pyvenv.cfg
home = /Users/crmarsh/.local/share/uv/python/cpython-3.12.6-macos-aarch64-none/bin
implementation = CPython
uv = 0.5.7
version_info = 3.12.6
include-system-site-packages = false
prompt = uv
❯ .venv/bin/python -c "import sys"
```

### Symlink to the Python installation

Correctly does _not_ resolve the symlink.

```
❯ ln -s /Users/crmarsh/.local/share/uv/python/cpython-3.12.6-macos-aarch64-none bar
❯ cargo run venv --python ./bar
❯ cat .venv/pyvenv.cfg
home = /Users/crmarsh/workspace/uv/bar/bin
implementation = CPython
uv = 0.5.7
version_info = 3.12.6
include-system-site-packages = false
prompt = uv
❯ .venv/bin/python -c "import sys"
```

### Direct symlink to `python` in a symlinked Python installation

Correctly resolves the direct symlink, but not the symlink of the Python
installation.

```
❯ ln -s bar/bin/python baz
❯ cargo run venv --python ./baz
❯ cat .venv/pyvenv.cfg
home = /Users/crmarsh/workspace/uv/bar/bin
implementation = CPython
uv = 0.5.7
version_info = 3.12.6
include-system-site-packages = false
prompt = uv
❯ .venv/bin/python -c "import sys"
```
@charliermarsh charliermarsh added the bug Something isn't working label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants