Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search PATH when python can't be found with py #1711

Merged
merged 7 commits into from
Feb 22, 2024
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
98 changes: 56 additions & 42 deletions crates/uv-interpreter/src/interpreter.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::ffi::{OsStr, OsString};
use std::io::Write;
use std::path::{Path, PathBuf};
use std::process::Command;

Expand All @@ -16,6 +17,7 @@ use uv_cache::{Cache, CacheBucket, CachedByTimestamp, Freshness, Timestamp};
use uv_fs::write_atomic_sync;

use crate::python_platform::PythonPlatform;
use crate::python_query::try_find_default_python;
use crate::virtual_env::detect_virtual_env;
use crate::{find_requested_python, Error, PythonVersion};

Expand All @@ -35,12 +37,7 @@ impl Interpreter {
/// Detect the interpreter info for the given Python executable.
pub fn query(executable: &Path, platform: &Platform, cache: &Cache) -> Result<Self, Error> {
let info = InterpreterQueryResult::query_cached(executable, cache)?;
debug_assert!(
info.base_prefix == info.base_exec_prefix,
"Not a virtualenv (Python: {}, prefix: {})",
executable.display(),
info.base_prefix.display()
);

debug_assert!(
info.sys_executable.is_absolute(),
"`sys.executable` is not an absolute Python; Python installation is broken: {}",
Expand Down Expand Up @@ -170,38 +167,18 @@ impl Interpreter {

// Look for the requested version with by search for `python{major}.{minor}` in `PATH` on
// Unix and `py --list-paths` on Windows.
if let Some(python_version) = python_version {
if let Some(interpreter) =
find_requested_python(&python_version.string, platform, cache)?
{
if version_matches(&interpreter) {
return Ok(Some(interpreter));
}
}
}
let interpreter = if let Some(python_version) = python_version {
find_requested_python(&python_version.string, platform, cache)?
} else {
try_find_default_python(platform, cache)?
};

// Python discovery failed to find the requested version, maybe the default Python in PATH
// matches?
if cfg!(unix) {
if let Some(executable) = Interpreter::find_executable("python3")? {
debug!("Resolved python3 to {}", executable.display());
let interpreter = Interpreter::query(&executable, &python_platform.0, cache)?;
if version_matches(&interpreter) {
return Ok(Some(interpreter));
}
}
} else if cfg!(windows) {
if let Some(executable) = Interpreter::find_executable("python.exe")? {
let interpreter = Interpreter::query(&executable, &python_platform.0, cache)?;
if version_matches(&interpreter) {
return Ok(Some(interpreter));
}
}
if let Some(interpreter) = interpreter {
debug_assert!(version_matches(&interpreter));
Ok(Some(interpreter))
} else {
unimplemented!("Only Windows and Unix are supported");
Ok(None)
}

Ok(None)
}

/// Find the Python interpreter in `PATH`, respecting `UV_PYTHON_PATH`.
Expand Down Expand Up @@ -324,13 +301,50 @@ pub(crate) struct InterpreterQueryResult {
impl InterpreterQueryResult {
/// Return the resolved [`InterpreterQueryResult`] for the given Python executable.
pub(crate) fn query(interpreter: &Path) -> Result<Self, Error> {
let output = Command::new(interpreter)
.args(["-c", include_str!("get_interpreter_info.py")])
.output()
.map_err(|err| Error::PythonSubcommandLaunch {
interpreter: interpreter.to_path_buf(),
err,
})?;
let script = include_str!("get_interpreter_info.py");
let output = if cfg!(windows)
&& interpreter
.extension()
.is_some_and(|extension| extension == "bat")
MichaReiser marked this conversation as resolved.
Show resolved Hide resolved
{
// Multiline arguments aren't well-supported in batch files and `pyenv-win`, for example, trips over it.
// We work around this batch limitation by passing the script via stdin instead.
// This is somewhat more expensive because we have to spawn a new thread to write the
// stdin to avoid deadlocks in case the child process waits for the parent to read stdout.
// The performance overhead is the reason why we only applies this to batch files.
// https://github.com/pyenv-win/pyenv-win/issues/589
let mut child = Command::new(interpreter)
.arg("-")
.stdin(std::process::Stdio::piped())
.stdout(std::process::Stdio::piped())
.spawn()
.map_err(|err| Error::PythonSubcommandLaunch {
interpreter: interpreter.to_path_buf(),
err,
})?;

let mut stdin = child.stdin.take().unwrap();

// From the Rust documentation:
// If the child process fills its stdout buffer, it may end up
// waiting until the parent reads the stdout, and not be able to
// read stdin in the meantime, causing a deadlock.
// Writing from another thread ensures that stdout is being read
// at the same time, avoiding the problem.
std::thread::spawn(move || {
stdin
.write_all(script.as_bytes())
.expect("failed to write to stdin");
});

child.wait_with_output()
} else {
Command::new(interpreter).arg("-c").arg(script).output()
}
.map_err(|err| Error::PythonSubcommandLaunch {
interpreter: interpreter.to_path_buf(),
err,
})?;

// stderr isn't technically a criterion for success, but i don't know of any cases where there
// should be stderr output and if there is, we want to know
Expand Down
13 changes: 6 additions & 7 deletions crates/uv-interpreter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,8 @@ use std::ffi::OsString;
use std::io;
use std::path::PathBuf;

use pep440_rs::Version;
use thiserror::Error;

use uv_fs::Normalized;

pub use crate::cfg::Configuration;
pub use crate::interpreter::Interpreter;
pub use crate::python_query::{find_default_python, find_requested_python};
Expand Down Expand Up @@ -43,14 +40,18 @@ pub enum Error {
#[error("Failed to run `py --list-paths` to find Python installations. Is Python installed?")]
PyList(#[source] io::Error),
#[cfg(windows)]
#[error("No Python {0} found through `py --list-paths`. Is Python {0} installed?")]
#[error(
"No Python {0} found through `py --list-paths` or in `PATH`. Is Python {0} installed?"
)]
NoSuchPython(String),
#[cfg(unix)]
#[error("No Python {0} In `PATH`. Is Python {0} installed?")]
NoSuchPython(String),
#[error("Neither `python` nor `python3` are in `PATH`. Is Python installed?")]
NoPythonInstalledUnix,
#[error("Could not find `python.exe` in PATH and `py --list-paths` did not list any Python versions. Is Python installed?")]
MichaReiser marked this conversation as resolved.
Show resolved Hide resolved
#[error(
"Could not find `python.exe` through `py --list-paths` or in 'PATH'. Is Python installed?"
)]
NoPythonInstalledWindows,
#[error("{message}:\n--- stdout:\n{stdout}\n--- stderr:\n{stderr}\n---")]
PythonSubcommandOutput {
Expand All @@ -64,6 +65,4 @@ pub enum Error {
Cfg(#[from] cfg::Error),
#[error("Error finding `{}` in PATH", _0.to_string_lossy())]
WhichError(OsString, #[source] which::Error),
#[error("Interpreter at `{}` has the wrong patch version. Expected: {}, actual: {}", _0.normalized_display(), _1, _2)]
PatchVersionMismatch(PathBuf, String, Version),
}
Loading