Skip to content

Commit 5f6c518

Browse files
authored
fix(venv): Use the interpreter to abspath (#590)
Identified in manual testing of v1.6.0-rc0. Because of overly conservative removals of `realpath` in #579, an issue is exposed where `$VIRTUAL_ENV` would be an un-resolved relative path both under Bazel and more significantly once a user activates a linked venv. This isn't so bad for binaries which usually don't `chdir`, but it's a problem for shells. Since MacOS doesn't ship a `realpath` which can be configured to ignore symlinks, we can't just `realpath` the runfiles dir or the virtualenv home. But once we've configured a Python interpreter what we can do is use `os.path.abspath`. Unlike `realpath`, `abspath` does not attempt to resolve symlink path segments. It just uses `normpath` to eliminate relative path segments. This allows us to compute an absolute path in a portable way, once we get an appropriate interpreter up and running. --- ### Changes are visible to end-users: no ### Test plan - Manual testing; please provide instructions so we can reproduce: ``` ❯ bazel run //examples/py_binary:py_binary.venv INFO: Analyzed target //examples/py_binary:py_binary.venv (0 packages loaded, 0 targets configured). INFO: Found 1 target... Target //examples/py_binary:py_binary.venv up-to-date: bazel-bin/examples/py_binary/py_binary.venv INFO: Elapsed time: 0.785s, Critical Path: 0.02s INFO: 1 process: 1 internal. INFO: Build completed successfully, 1 total action INFO: Running command line: bazel-bin/examples/py_binary/py_binary.venv Linking: /private/var/tmp/_bazel_arrdem/93bfea6cdc1153cc29a75400cd38823a/execroot/aspect_rules_py/bazel-out/darwin_arm64-fastbuild/bin/examples/py_binary/.py_binary.venv -> /Users/arrdem/Documents/work/aspect/rules_py/examples/py_binary/.py_binary.venv To activate the virtualenv run: source /Users/arrdem/Documents/work/aspect/rules_py/examples/py_binary/.py_binary.venv/bin/activate Link is up to date! ❯ source /Users/arrdem/Documents/work/aspect/rules_py/examples/py_binary/.py_binary.venv/bin/activate ❯ env | grep -e PYTHON -e RUNFILES -e VIRTUAL -e VENV | sort PYTHONEXECUTABLE=/Users/arrdem/Documents/work/aspect/rules_py/examples/py_binary/.py_binary.venv/bin/python PYTHONHOME=/private/var/tmp/_bazel_arrdem/93bfea6cdc1153cc29a75400cd38823a/execroot/aspect_rules_py/bazel-out/darwin_arm64-fastbuild/bin/examples/py_binary/py_binary.venv.runfiles/python_toolchain_aarch64-apple-darwin RUNFILES_DIR=/private/var/tmp/_bazel_arrdem/93bfea6cdc1153cc29a75400cd38823a/execroot/aspect_rules_py/bazel-out/darwin_arm64-fastbuild/bin/examples/py_binary/py_binary.venv.runfiles RUNFILES_MANIFEST_FILE=/private/var/tmp/_bazel_arrdem/93bfea6cdc1153cc29a75400cd38823a/execroot/aspect_rules_py/bazel-out/darwin_arm64-fastbuild/bin/examples/py_binary/py_binary.venv.runfiles/MANIFEST RUNFILES_REPO_MAPPING=/private/var/tmp/_bazel_arrdem/93bfea6cdc1153cc29a75400cd38823a/execroot/aspect_rules_py/bazel-out/darwin_arm64-fastbuild/bin/examples/py_binary/py_binary.venv.runfiles/_repo_mapping VIRTUAL_ENV_DISABLE_PROMPT=1 VIRTUAL_ENV=/Users/arrdem/Documents/work/aspect/rules_py/examples/py_binary/.py_binary.venv ```
1 parent 823c787 commit 5f6c518

File tree

5 files changed

+24
-6
lines changed

5 files changed

+24
-6
lines changed

py/tests/py_venv_image_layer/my_app_amd64_layers_listing.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2510,7 +2510,7 @@ files:
25102510
- drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/
25112511
- drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/
25122512
- drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/bin/
2513-
- -rwxr-xr-x 0 0 0 8099 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/bin/activate
2513+
- -rwxr-xr-x 0 0 0 8620 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/bin/activate
25142514
- -rwxr-xr-x 0 0 0 817416 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/bin/python
25152515
- -rwxr-xr-x 0 0 0 817416 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/bin/python3
25162516
- -rwxr-xr-x 0 0 0 817416 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/bin/python3.9

py/tests/py_venv_image_layer/my_app_arm64_layers_listing.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2491,7 +2491,7 @@ files:
24912491
- drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/
24922492
- drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/
24932493
- drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/bin/
2494-
- -rwxr-xr-x 0 0 0 8100 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/bin/activate
2494+
- -rwxr-xr-x 0 0 0 8621 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/bin/activate
24952495
- -rwxr-xr-x 0 0 0 698064 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/bin/python
24962496
- -rwxr-xr-x 0 0 0 698064 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/bin/python3
24972497
- -rwxr-xr-x 0 0 0 698064 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/bin/python3.9

py/tools/py/src/activate.tmpl

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export PYTHONEXECUTABLE
7070
# unset PYTHONHOME if set
7171
# this will fail if PYTHONHOME is set to the empty string (which is bad anyway)
7272
# could use `if (set -u; : $PYTHONHOME) ;` in bash.
73-
_OLD_VIRTUAL_PYTHONHOME="${PYTHONHOME:-}"
73+
_OLD_VIRTUAL_PYTHONHOME="${PYTHONHOME:-_activate_undef}"
7474
unset PYTHONHOME
7575

7676
_OLD_VIRTUAL_PATH="$PATH"
@@ -85,6 +85,24 @@ _OLD_VIRTUAL_PATH="$PATH"
8585
# can provide some fallback handling around runfiles too.
8686
{{RUNFILES_INTERPRETER}}
8787

88+
_abspath() {
89+
"${PYTHONEXECUTABLE}" -c 'import os, sys; print(os.path.abspath(sys.argv[1]))' "$@"
90+
}
91+
92+
# Re-export abspath'd vars
93+
# This allows us to avoid relative path issues without incurring sandbox escapes
94+
VIRTUAL_ENV="$(_abspath "${VIRTUAL_ENV}")"
95+
export VIRTUAL_ENV
96+
97+
PYTHONEXECUTABLE="$(_abspath "${PYTHONEXECUTABLE}")"
98+
export PYTHONEXECUTABLE
99+
100+
if [ -n "${PYTHONHOME:-}" ]; then
101+
PYTHONHOME="$(_abspath "${PYTHONHOME}")"
102+
export PYTHONHOME
103+
fi
104+
105+
# Now we can put the venv's absolute bin on the path
88106
PATH="$VIRTUAL_ENV/bin:$PATH"
89107
export PATH
90108

py/tools/py/src/runfiles_interpreter.tmpl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# --- Runfiles-based interpreter setup --
1+
# --- Runfiles-based interpreter setup ---
22

33
# If the runfiles dir is unset AND we will fail to find a runfiles manifest
44
# based on inspecting $0, we need to try something different.
@@ -22,7 +22,7 @@ _activate_find_runfiles() {
2222
# find the manifest file and runfiles tree relative to the execroot.
2323

2424
# HACK: We can't lazy-match to the first /bin/, so we have to manually count four groups
25-
EXECROOT="$(echo "${1}" | sed 's/\(execroot\/[^\/]*\/[^\/]*\/[^\/]*\/[^\/]*\/\).*$/\1/' )"
25+
EXECROOT="$(echo "${1}" | sed 's/\(execroot\/[^\/]*\/[^\/]*\/[^\/]*\/[^\/]*\).*$/\1/' )"
2626
export RUNFILES_DIR="${EXECROOT}/${RUNFILES_PATH}"
2727
elif [[ "${1}" == *.runfiles/* ]]; then
2828
# Examples:

py/tools/venv_shim/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ fn main() -> miette::Result<()> {
170170
{
171171
eprintln!("[aspect] Found potential Python interpreters in PATH with matching version:");
172172
for exe in &python_executables {
173-
println!("[aspect] - {:?}", exe);
173+
eprintln!("[aspect] - {:?}", exe);
174174
}
175175
}
176176

0 commit comments

Comments
 (0)