Skip to content

Commit 3512cbe

Browse files
authored
fix(venv): Don't use dirs as the interpreter (#665)
Fixes #664 On Linux and MacOS, the `current_exe()` path has likely been realpath-ed by the system or the libc somewhere along the line. This means that if the user types `.venv/bin/python3`, the entire symlink chain that may point into will be chased and we'll get a path somewhere inside the Bazel `execroot/` output tree. Critically on Linux this symlink tree goes past the `.runfiles/` directory entry the user intended to create a symlink to, which prevents our interpreter shim from delegating to a `.runfiles/` packaged Python like the user desired. In order for us to resolve "just enough" symlinks to identify the `.runfiles/` tree, we need to manually resolve the path of the current executable. Previously doing so would create a false positive if there was a `./python` or `./python3` directory, and the interpreter shim was invoked as `python3` which is ambiguous as to whether the user meant `./python3` or `python3 from the $PATH`. The fix here centers on two insights: - The "real" interpreter can never be a directory (directly fixes the bug) - The "real" interpreter's absolute path _must_ have the _same_ absolute path as whatever `current_exe()` is ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes Fixed a bug which would cause the interpreter shims to identify `./python3/` or `./python/` (directories) as the real path of the interpreter "python3" rather than falling back to `which()` behavior. ### Test plan - Manual testing
1 parent 194a388 commit 3512cbe

File tree

1 file changed

+8
-3
lines changed

1 file changed

+8
-3
lines changed

py/tools/venv_shim/src/main.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
use miette::IntoDiagnostic;
1+
use miette::{miette, IntoDiagnostic};
22
use runfiles::Runfiles;
33
use which::which;
44
// Depended on out of rules_rust
5-
use std::env;
5+
use std::env::{self, current_exe};
66
use std::ffi::OsStr;
77
use std::fs;
88
use std::os::unix::process::CommandExt;
@@ -245,14 +245,19 @@ fn main() -> miette::Result<()> {
245245
let cwd = std::env::current_dir().into_diagnostic()?;
246246
if !executable.is_absolute() {
247247
let candidate = cwd.join(&executable);
248-
if candidate.exists() {
248+
if candidate.exists()
249+
&& !candidate.is_dir()
250+
&& candidate.canonicalize().unwrap() == current_exe().unwrap().canonicalize().unwrap()
251+
{
249252
executable = candidate;
250253
#[cfg(feature = "debug")]
251254
eprintln!(" {:?}", executable);
252255
} else if let Ok(exe) = which(&exec_name) {
253256
executable = exe;
254257
#[cfg(feature = "debug")]
255258
eprintln!(" {:?}", executable);
259+
} else {
260+
return Err(miette!("Unable to identify the real interpreter path!"));
256261
}
257262
}
258263

0 commit comments

Comments
 (0)