Skip to content

Commit fb83c4b

Browse files
authored
fix(py_venv): Carefully resolve executable name to identify runfiles (#654)
Fixes #649. > ``` > thread 'main' panicked at py/tools/venv_shim/src/main.rs:199:40: > called `Result::unwrap()` on an `Err` value: RunfilesDirIoError(Os { code: 2, kind: NotFound, message: "No such file or directory" }) > note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace > ``` There are two issues which combine to produce this problem. ### Defect 1 -- `venv_shim` resolves not enough links <details> If a user builds a static venv and then creates a link to it, on some sandbox implementations the `realpath` of the `./.venv/bin/python3` interpreter will resolve no links (current behavior) or (previously) resolve all symlinks and produce a file name somewhere in the Bazel execroot which has escaped from the `.runfiles/` tree which bundles together both the venv and the interpreter which it requires to operate. This is a problem because the runfiles implementations all variously rely on inspecting `argv[0]` (the current executable name) in order to configure the runfiles tree. When entering the `python3` interpreter within the venv within the runfiles tree, `argv[0]` _must_ contain a `*.runfiles/` parent directory in order to configure properly. Since resolving no links is wrong and resolving all links is also wrong we have to carefully resolve just enough links in the path of the interpreter to identify the `.runfiles/` dir. If there's no intermediate symlink state which contains the `.runfiles` path segment, then the link as originally created was incorrect and cannot be resolved correctly. If there is such a state, this logic will find it. </details> ### Defect 2 -- `link.py` resolves too many links <details> By calling `realpath` not `normpath`, the `link.py` script makes the same mistake as above and will create a venv symlink which likely points into the Bazel execroot beyond the limits of the `.runfiles` tree which should serve as a logical sandbox. Switch to `normpath` which canonicalizes but does not resolve links. This preserves the observed `.runfiles/` child location of the virtualenv as the desired destination of the user's link, which allows runfiles-local interpreter identification to operate. </details> ### Changes are visible to end-users: yes/no <!-- If no, please delete this section. --> - Searched for relevant documentation and updated as needed: yes/no - Breaking change (forces users to change their own code or config): yes/no - Suggested release notes appear below: yes/no ### Test plan - Manual testing; please provide instructions so we can reproduce: On MacOS; ``` ❯ bazel run //examples/py_binary:py_binary.venv ❯ source .examples+py_binary+py_binary.venv/bin/activate ❯ python3 Python 3.9.18 (main, Oct 2 2023, 20:16:14) [Clang 16.0.3 ] on darwin Type "help", "copyright", "credits" or "license" for more information. Could not open PYTHONSTARTUP FileNotFoundError: [Errno 2] No such file or directory: '/Users/arrdem/.pythonrc' >>> import site >>> site.PREFIXES ['/Users/arrdem/Documents/work/aspect/rules_py/.examples+py_binary+py_binary.venv'] >>> import sys >>> sys.executable '/Users/arrdem/Documents/work/aspect/rules_py/.examples+py_binary+py_binary.venv/bin/python3' ``` Also hand-tested on Linux to the same effect, which is the source of the manual `current_executable()` behavior.
1 parent 6439c64 commit fb83c4b

File tree

10 files changed

+1269
-333
lines changed

10 files changed

+1269
-333
lines changed

.pre-commit-config.yaml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,17 @@ repos:
5353
entry: /bin/sh -c '/usr/bin/env bazel run //:gazelle_python_manifest.update'
5454
language: script
5555
require_serial: true
56+
57+
- id: check-amd64-image-lists
58+
name: Update amd64 image lists
59+
# Note that we use a nested shell to discard $@, which is the file list
60+
entry: /bin/sh -c '/usr/bin/env bazel run //py/tests/py_venv_image_layer:my_app_amd64_layers'
61+
language: script
62+
require_serial: true
63+
64+
- id: check-arm64-image-lists
65+
name: Update arm64 image lists
66+
# Note that we use a nested shell to discard $@, which is the file list
67+
entry: /bin/sh -c '/usr/bin/env bazel run //py/tests/py_venv_image_layer:my_app_arm64_layers'
68+
language: script
69+
require_serial: true

0 commit comments

Comments
 (0)