Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/uv-python/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ reqwest = { workspace = true }
reqwest-middleware = { workspace = true }
reqwest-retry = { workspace = true }
rmp-serde = { workspace = true }
rustc-hash = { workspace = true }
same-file = { workspace = true }
schemars = { workspace = true, optional = true }
serde = { workspace = true, features = ["derive"] }
Expand Down
63 changes: 41 additions & 22 deletions crates/uv-python/src/discovery.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use itertools::{Either, Itertools};
use regex::Regex;
use rustc_hash::{FxBuildHasher, FxHashSet};
use same_file::is_same_file;
use std::env::consts::EXE_SUFFIX;
use std::fmt::{self, Debug, Formatter};
Expand Down Expand Up @@ -496,6 +497,7 @@ fn python_executables_from_search_path<'a>(
// check multiple names per directory while respecting the search path order and python names
// precedence.
let search_dirs: Vec<_> = env::split_paths(&search_path).collect();
let mut seen_dirs = FxHashSet::with_capacity_and_hasher(search_dirs.len(), FxBuildHasher);
search_dirs
.into_iter()
.filter(|dir| dir.is_dir())
Expand All @@ -506,33 +508,50 @@ fn python_executables_from_search_path<'a>(
"Checking `PATH` directory for interpreters: {}",
dir.display()
);
possible_names
.clone()
.into_iter()
.flat_map(move |name| {
// Since we're just working with a single directory at a time, we collect to simplify ownership
which::which_in_global(&*name, Some(&dir))
.into_iter()
.flatten()
// We have to collect since `which` requires that the regex outlives its
// parameters, and the dir is local while we return the iterator.
.collect::<Vec<_>>()
same_file::Handle::from_path(&dir)
// Skip directories we've already seen, to avoid inspecting interpreters multiple
// times when directories are repeated or symlinked in the `PATH`
.map(|handle| seen_dirs.insert(handle))
.inspect(|fresh_dir| {
if !fresh_dir {
trace!("Skipping already seen directory: {}", dir.display());
}
})
.chain(find_all_minor(implementation, version, &dir_clone))
.filter(|path| !is_windows_store_shim(path))
.inspect(|path| trace!("Found possible Python executable: {}", path.display()))
.chain(
// TODO(zanieb): Consider moving `python.bat` into `possible_names` to avoid a chain
cfg!(windows)
.then(move || {
which::which_in_global("python.bat", Some(&dir_clone))
// If we cannot determine if the directory is unique, we'll assume it is
.unwrap_or(true)
.then(|| {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possible_names
.clone()
.into_iter()
.flat_map(move |name| {
// Since we're just working with a single directory at a time, we collect to simplify ownership
which::which_in_global(&*name, Some(&dir))
.into_iter()
.flatten()
// We have to collect since `which` requires that the regex outlives its
// parameters, and the dir is local while we return the iterator.
.collect::<Vec<_>>()
})
.into_iter()
.flatten(),
)
.chain(find_all_minor(implementation, version, &dir_clone))
.filter(|path| !is_windows_store_shim(path))
.inspect(|path| {
trace!("Found possible Python executable: {}", path.display());
})
.chain(
// TODO(zanieb): Consider moving `python.bat` into `possible_names` to avoid a chain
cfg!(windows)
.then(move || {
which::which_in_global("python.bat", Some(&dir_clone))
.into_iter()
.flatten()
.collect::<Vec<_>>()
})
.into_iter()
.flatten(),
)
})
.into_iter()
.flatten()
})
}

Expand Down
68 changes: 68 additions & 0 deletions crates/uv/tests/it/python_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,3 +280,71 @@ fn python_list_unsupported_version() {
error: Invalid version request: Python <3.13 does not support free-threading but 3.12t was requested.
");
}

#[test]
fn python_list_duplicate_path_entries() {
let context: TestContext = TestContext::new_with_versions(&["3.11", "3.12"])
.with_filtered_python_symlinks()
.with_filtered_python_keys();

// Construct a `PATH` with all entries duplicated
let path = std::env::join_paths(
std::env::split_paths(&context.python_path())
.chain(std::env::split_paths(&context.python_path())),
)
.unwrap();

uv_snapshot!(context.filters(), context.python_list().env(EnvVars::UV_TEST_PYTHON_PATH, &path), @r"
success: true
exit_code: 0
----- stdout -----
cpython-3.12.[X]-[PLATFORM] [PYTHON-3.12]
cpython-3.11.[X]-[PLATFORM] [PYTHON-3.11]

----- stderr -----
");

#[cfg(unix)]
{
// Construct a `PATH` with symlinks
let path = std::env::join_paths(std::env::split_paths(&context.python_path()).chain(
std::env::split_paths(&context.python_path()).map(|path| {
let dst = format!("{}-link", path.display());
fs_err::os::unix::fs::symlink(&path, &dst).unwrap();
std::path::PathBuf::from(dst)
}),
))
.unwrap();

uv_snapshot!(context.filters(), context.python_list().env(EnvVars::UV_TEST_PYTHON_PATH, &path), @r"
success: true
exit_code: 0
----- stdout -----
cpython-3.12.[X]-[PLATFORM] [PYTHON-3.12]
cpython-3.11.[X]-[PLATFORM] [PYTHON-3.11]

----- stderr -----
");

// Reverse the order so the symlinks are first
let path = std::env::join_paths(
{
let mut paths = std::env::split_paths(&path).collect::<Vec<_>>();
paths.reverse();
paths
}
.iter(),
)
.unwrap();

uv_snapshot!(context.filters(), context.python_list().env(EnvVars::UV_TEST_PYTHON_PATH, &path), @r"
success: true
exit_code: 0
----- stdout -----
cpython-3.12.[X]-[PLATFORM] [PYTHON-3.12]-link/python3
cpython-3.11.[X]-[PLATFORM] [PYTHON-3.11]-link/python3

----- stderr -----
");
}
}
Loading