Skip to content

Commit

Permalink
Search for all python3.x in PATH (#5148)
Browse files Browse the repository at this point in the history
Search for all `python3.x` minor versions in PATH, skipping those we
already know we can use.

For example, let's say `python` and `python3` are Python 3.10. When a
user requests `>= 3.11`, we still need to find a `python3.12` in PATH.
We do so with a regex matcher.

Fixes #4709
  • Loading branch information
konstin authored Jul 18, 2024
1 parent 36a0ee9 commit 7beae77
Show file tree
Hide file tree
Showing 6 changed files with 165 additions and 13 deletions.
11 changes: 10 additions & 1 deletion Cargo.lock

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

4 changes: 3 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ rkyv = { version = "0.7.43", features = ["strict", "validation"] }
rmp-serde = { version = "1.1.2" }
rust-netrc = { version = "0.1.1" }
rustc-hash = { version = "2.0.0" }
rustix = { version = "0.38.34", default-features = false, features = ["fs", "std"] }
same-file = { version = "1.0.6" }
schemars = { version = "0.8.16", features = ["url"] }
seahash = { version = "4.1.0" }
Expand Down Expand Up @@ -151,9 +152,10 @@ unscanny = { version = "0.1.0" }
url = { version = "2.5.0" }
urlencoding = { version = "2.1.3" }
walkdir = { version = "2.5.0" }
which = { version = "6.0.0" }
which = { version = "6.0.0", features = ["regex"] }
winapi = { version = "0.3.9", features = ["fileapi", "handleapi", "ioapiset", "winbase", "winioctl", "winnt"] }
winreg = { version = "0.52.0" }
winsafe = { version = "0.0.21", features = ["kernel"] }
wiremock = { version = "0.6.0" }
zip = { version = "0.6.6", default-features = false, features = ["deflate"] }

Expand Down
4 changes: 4 additions & 0 deletions crates/uv-python/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,12 @@ tracing = { workspace = true }
url = { workspace = true }
which = { workspace = true }

[target.'cfg(any(unix, target_os = "wasi", target_os = "redox"))'.dependencies]
rustix = { workspace = true }

[target.'cfg(target_os = "windows")'.dependencies]
winapi = { workspace = true }
winsafe = { workspace = true }

[dev-dependencies]
anyhow = { version = "1.0.80" }
Expand Down
92 changes: 81 additions & 11 deletions crates/uv-python/src/discovery.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
use itertools::{Either, Itertools};
use regex::Regex;
use same_file::is_same_file;
use std::borrow::Cow;
use std::env::consts::EXE_SUFFIX;
use std::fmt::{self, Formatter};
use std::{env, io};
use std::{env, io, iter};
use std::{path::Path, path::PathBuf, str::FromStr};

use itertools::Itertools;
use pep440_rs::{Version, VersionSpecifiers};
use same_file::is_same_file;
use thiserror::Error;
use tracing::{debug, instrument, trace};
use which::{which, which_all};

use pep440_rs::{Version, VersionSpecifiers};
use uv_cache::Cache;
use uv_configuration::PreviewMode;
use uv_fs::Simplified;
Expand All @@ -25,6 +26,7 @@ use crate::virtualenv::{
conda_prefix_from_env, virtualenv_from_env, virtualenv_from_working_dir,
virtualenv_python_executable,
};
use crate::which::is_executable;
use crate::{Interpreter, PythonVersion};

/// A request to find a Python installation.
Expand Down Expand Up @@ -360,18 +362,17 @@ fn python_executables_from_search_path<'a>(
let search_path =
env::var_os("UV_TEST_PYTHON_PATH").unwrap_or(env::var_os("PATH").unwrap_or_default());

let possible_names: Vec<_> = version
.unwrap_or(&VersionRequest::Any)
.possible_names(implementation)
.collect();
let version_request = version.unwrap_or(&VersionRequest::Any);
let possible_names: Vec<_> = version_request.possible_names(implementation).collect();

trace!(
"Searching PATH for executables: {}",
possible_names.join(", ")
);

// Split and iterate over the paths instead of using `which_all` so we can
// check multiple names per directory while respecting the search path order
// 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();
search_dirs
.into_iter()
Expand All @@ -391,8 +392,11 @@ fn python_executables_from_search_path<'a>(
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<_>>()
})
.chain(find_all_minor(implementation, version_request, &dir_clone))
.filter(|path| !is_windows_store_shim(path))
.inspect(|path| trace!("Found possible Python executable: {}", path.display()))
.chain(
Expand All @@ -410,6 +414,71 @@ fn python_executables_from_search_path<'a>(
})
}

/// Find all acceptable `python3.x` minor versions.
///
/// For example, let's say `python` and `python3` are Python 3.10. When a user requests `>= 3.11`,
/// we still need to find a `python3.12` in PATH.
fn find_all_minor(
implementation: Option<&ImplementationName>,
version_request: &VersionRequest,
dir: &Path,
) -> impl Iterator<Item = PathBuf> {
match version_request {
VersionRequest::Any | VersionRequest::Major(_) | VersionRequest::Range(_) => {
let regex = if let Some(implementation) = implementation {
Regex::new(&format!(
r"^({}|python3)\.(?<minor>\d\d?){}$",
regex::escape(&implementation.to_string()),
regex::escape(EXE_SUFFIX)
))
.unwrap()
} else {
Regex::new(&format!(
r"^python3\.(?<minor>\d\d?){}$",
regex::escape(EXE_SUFFIX)
))
.unwrap()
};
let all_minors = fs_err::read_dir(dir)
.into_iter()
.flatten()
.flatten()
.map(|entry| entry.path())
.filter(move |path| {
let Some(filename) = path.file_name() else {
return false;
};
let Some(filename) = filename.to_str() else {
return false;
};
let Some(captures) = regex.captures(filename) else {
return false;
};

// Filter out interpreter we already know have a too low minor version.
let minor = captures["minor"].parse().ok();
if let Some(minor) = minor {
// Optimization: Skip generally unsupported Python versions without querying.
if minor < 7 {
return false;
}
// Optimization 2: Skip excluded Python (minor) versions without querying.
if !version_request.matches_major_minor(3, minor) {
return false;
}
}
true
})
.filter(|path| is_executable(path))
.collect::<Vec<_>>();
Either::Left(all_minors.into_iter())
}
VersionRequest::MajorMinor(_, _) | VersionRequest::MajorMinorPatch(_, _, _) => {
Either::Right(iter::empty())
}
}
}

/// Lazily iterate over all discoverable Python interpreters.
///
/// Note interpreters may be excluded by the given [`EnvironmentPreference`] and [`PythonPreference`].
Expand Down Expand Up @@ -1596,12 +1665,13 @@ mod tests {
use assert_fs::{prelude::*, TempDir};
use test_log::test;

use super::Error;
use crate::{
discovery::{PythonRequest, VersionRequest},
implementation::ImplementationName,
};

use super::Error;

#[test]
fn interpreter_request_from_str() {
assert_eq!(PythonRequest::parse("any"), PythonRequest::Any);
Expand Down
27 changes: 27 additions & 0 deletions crates/uv-python/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ mod python_version;
mod target;
mod version_files;
mod virtualenv;
mod which;

#[cfg(not(test))]
pub(crate) fn current_dir() -> Result<std::path::PathBuf, std::io::Error> {
Expand Down Expand Up @@ -1752,6 +1753,32 @@ mod tests {
Ok(())
}

#[test]
fn find_python_all_minors() -> Result<()> {
let mut context = TestContext::new()?;
context.add_python_interpreters(&[
(true, ImplementationName::CPython, "python", "3.10.0"),
(true, ImplementationName::CPython, "python3", "3.10.0"),
(true, ImplementationName::CPython, "python3.12", "3.12.0"),
])?;

let python = context.run(|| {
find_python_installation(
&PythonRequest::parse(">= 3.11"),
EnvironmentPreference::Any,
PythonPreference::OnlySystem,
&context.cache,
)
})??;
assert_eq!(
python.interpreter().python_full_version().to_string(),
"3.12.0",
"We should find matching minor version even if they aren't called `python` or `python3`"
);

Ok(())
}

#[test]
fn find_python_pypy_prefers_executable_with_implementation_name() -> Result<()> {
let mut context = TestContext::new()?;
Expand Down
40 changes: 40 additions & 0 deletions crates/uv-python/src/which.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
use std::path::Path;

/// Check whether a path in PATH is a valid executable.
///
/// Derived from `which`'s `Checker`.
pub(crate) fn is_executable(path: &Path) -> bool {
#[cfg(any(unix, target_os = "wasi", target_os = "redox"))]
{
if rustix::fs::access(path, rustix::fs::Access::EXEC_OK).is_err() {
return false;
}
}

#[cfg(target_os = "windows")]
{
let Ok(file_type) = fs_err::symlink_metadata(path).map(|metadata| metadata.file_type())
else {
return false;
};
if !file_type.is_file() && !file_type.is_symlink() {
return false;
}
if path.extension().is_none()
&& winsafe::GetBinaryType(&path.display().to_string()).is_err()
{
return false;
}
}

if cfg!(not(target_os = "windows")) {
if !fs_err::metadata(path)
.map(|metadata| metadata.is_file())
.unwrap_or(false)
{
return false;
}
}

true
}

0 comments on commit 7beae77

Please sign in to comment.