Skip to content

Commit

Permalink
Avoid resolving symbolic links when querying Python interpreters
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Jan 29, 2025
1 parent d7825c4 commit f83c9f9
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 26 deletions.
69 changes: 53 additions & 16 deletions crates/uv-python/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::env;
use std::fmt;
use std::path::{Path, PathBuf};
use std::sync::Arc;
use tracing::debug;
use uv_cache::Cache;
use uv_cache_key::cache_digest;
use uv_fs::{LockedFile, Simplified};
Expand Down Expand Up @@ -161,8 +162,21 @@ impl PythonEnvironment {
///
/// N.B. This function also works for system Python environments and users depend on this.
pub fn from_root(root: impl AsRef<Path>, cache: &Cache) -> Result<Self, Error> {
let venv = match fs_err::canonicalize(root.as_ref()) {
Ok(venv) => venv,
debug!(
"Checking for Python environment at `{}`",
root.as_ref().user_display()
);
let canonical_root = match fs_err::canonicalize(root.as_ref()) {
Ok(canonical_root) => {
if root.as_ref() != &canonical_root {
debug!(
"Environment path `{}` canonicalized to `{}`",
root.as_ref().user_display(),
canonical_root.user_display()
);
}
canonical_root
}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
return Err(Error::MissingEnvironment(EnvironmentNotFound {
preference: EnvironmentPreference::Any,
Expand All @@ -172,33 +186,56 @@ impl PythonEnvironment {
Err(err) => return Err(Error::Discovery(err.into())),
};

if venv.is_file() {
if canonical_root.is_file() {
return Err(InvalidEnvironment {
path: venv,
path: canonical_root,
kind: InvalidEnvironmentKind::NotDirectory,
}
.into());
}

if venv.read_dir().is_ok_and(|mut dir| dir.next().is_none()) {
if canonical_root
.read_dir()
.is_ok_and(|mut dir| dir.next().is_none())
{
return Err(InvalidEnvironment {
path: venv,
path: canonical_root,
kind: InvalidEnvironmentKind::Empty,
}
.into());
}

let executable = virtualenv_python_executable(&venv);
// Check if the executable exists at the non-canonicalized directory first. This is
// important because the path the interpreter is invoked at can determine the value of
// `sys.executable`.
//
// We intentionally don't require a resolved link to exist here, we're just trying to tell
// if this _looks_ like a Python environment.
let executable = virtualenv_python_executable(&root);
let executable = if executable.is_symlink() || executable.is_file() {
debug!(
"Found possible interpreter at {}",
executable.user_display()
);
executable
} else {
let canonical_executable = virtualenv_python_executable(&canonical_root);
debug!(
"No interpreter at `{}`, checking `{}`",
executable.user_display(),
canonical_executable.user_display()
);

// If we can't find an executable, exit before querying to provide a better error.
if !(canonical_executable.is_symlink() || canonical_executable.is_file()) {
return Err(InvalidEnvironment {
path: canonical_root,
kind: InvalidEnvironmentKind::MissingExecutable(canonical_executable.clone()),
}
.into());
};

// Check if the executable exists before querying so we can provide a more specific error
// Note we intentionally don't require a resolved link to exist here, we're just trying to
// tell if this _looks_ like a Python environment.
if !(executable.is_symlink() || executable.is_file()) {
return Err(InvalidEnvironment {
path: venv,
kind: InvalidEnvironmentKind::MissingExecutable(executable.clone()),
}
.into());
canonical_executable
};

let interpreter = Interpreter::query(executable, cache)?;
Expand Down
49 changes: 39 additions & 10 deletions crates/uv/tests/it/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3352,6 +3352,8 @@ fn run_gui_script_explicit_unix() -> Result<()> {
#[test]
#[cfg(unix)]
fn run_linked_environment_path() -> Result<()> {
use anyhow::Ok;

let context = TestContext::new("3.12")
.with_filtered_virtualenv_bin()
.with_filtered_python_names();
Expand All @@ -3363,7 +3365,7 @@ fn run_linked_environment_path() -> Result<()> {
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["iniconfig"]
dependencies = ["black"]
"#,
)?;

Expand All @@ -3378,27 +3380,54 @@ fn run_linked_environment_path() -> Result<()> {
----- stdout -----
----- stderr -----
Resolved 2 packages in [TIME]
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ iniconfig==2.0.0
Resolved 8 packages in [TIME]
Prepared 6 packages in [TIME]
Installed 6 packages in [TIME]
+ black==24.3.0
+ click==8.1.7
+ mypy-extensions==1.0.0
+ packaging==24.0
+ pathspec==0.12.1
+ platformdirs==4.2.0
"###);

// Using `uv run` should
// `sys.prefix` and `sys.executable` should be from the `target` directory
uv_snapshot!(context.filters(), context.run()
.env_remove("VIRTUAL_ENV") // Ignore the test context virtual environment activation
.env_remove("VIRTUAL_ENV") // Ignore the test context's active virtual environment
.env(EnvVars::UV_PROJECT_ENVIRONMENT, "target")
.arg("python").arg("-c").arg("import sys; print(sys.executable)"), @r###"
.arg("python").arg("-c").arg("import sys; print(sys.prefix); print(sys.executable)"), @r###"
success: true
exit_code: 0
----- stdout -----
[VENV]/
[VENV]/[BIN]/python
----- stderr -----
Resolved 2 packages in [TIME]
Audited 1 package in [TIME]
Resolved 8 packages in [TIME]
Audited 6 packages in [TIME]
"###);

// And, similarly, the entrypoint should use `target`
let black_entrypoint = context.read("target/bin/black");
insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
black_entrypoint, @r###"
#![VENV]/[BIN]/python
# -*- coding: utf-8 -*-
import sys
from black import patched_main
if __name__ == "__main__":
if sys.argv[0].endswith("-script.pyw"):
sys.argv[0] = sys.argv[0][:-11]
elif sys.argv[0].endswith(".exe"):
sys.argv[0] = sys.argv[0][:-4]
sys.exit(patched_main())
"###
);
});

Ok(())
}

Expand Down

0 comments on commit f83c9f9

Please sign in to comment.