Skip to content

Commit

Permalink
Ignore Python 2 installations when querying for interpreters (#1905)
Browse files Browse the repository at this point in the history
## Summary

Fixes #1693

`uv` currently fails when a user has `python` 2 or older installed on
their system without a `python3` or `python3.exe` on their path because
the `get_interpreter_info.py` script fails executing (it uses some
Python 3+ APIs).

This PR fixes this by:

* Returning an explicit error code in `get_interpreter_info` if the
Python version isn't supported
* Skipping over this error in `python_query` if the user requested ANY
python version or a version >= 3.
* Error if the user requested a Python 2 version. 

## Test Plan

Error if the user requests a legacy python version. 

```
uv venv -p 2
  × Python 2 or older is not supported. Please use Python 3 or newer.
```


Ignore any python 2 installation when querying newer python
installations (using v4 here because I have python3 on the path and that
takes precedence over querying python)
```
 uv_interpreter::python_query::find_python selector=Major(4)
      0.005541s   0ms DEBUG uv_interpreter::interpreter Detecting markers for: /home/micha/.pyenv/shims/python
      0.059730s  54ms DEBUG uv_interpreter::python_query Found a Python 2 installation that isn't supported by uv, skipping.
      0.059983s  54ms DEBUG uv_interpreter::interpreter Using cached markers for: /usr/bin/python
  × No Python 4 In `PATH`. Is Python 4 installed?

```
  • Loading branch information
MichaReiser authored Feb 23, 2024
1 parent 73ed0f0 commit 829e147
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 8 deletions.
14 changes: 14 additions & 0 deletions crates/uv-interpreter/src/get_interpreter_info.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
""""
Queries information about the current Python interpreter and prints it as JSON.
Exit Codes:
0: Success
1: General failure
3: Python version 3 or newer is required
"""


import json
import os
import platform
Expand All @@ -13,6 +23,10 @@ def format_full_version(info):
return version


if sys.version_info[0] < 3:
sys.exit(3)


if hasattr(sys, "implementation"):
implementation_version = format_full_version(sys.implementation.version)
implementation_name = sys.implementation.name
Expand Down
15 changes: 10 additions & 5 deletions crates/uv-interpreter/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub struct Interpreter {
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)?;
let info = InterpreterInfo::query_cached(executable, cache)?;

debug_assert!(
info.sys_executable.is_absolute(),
Expand Down Expand Up @@ -290,16 +290,16 @@ impl Interpreter {
}

#[derive(Debug, Deserialize, Serialize, Clone)]
pub(crate) struct InterpreterQueryResult {
pub(crate) struct InterpreterInfo {
pub(crate) markers: MarkerEnvironment,
pub(crate) base_exec_prefix: PathBuf,
pub(crate) base_prefix: PathBuf,
pub(crate) stdlib: PathBuf,
pub(crate) sys_executable: PathBuf,
}

impl InterpreterQueryResult {
/// Return the resolved [`InterpreterQueryResult`] for the given Python executable.
impl InterpreterInfo {
/// Return the resolved [`InterpreterInfo`] for the given Python executable.
pub(crate) fn query(interpreter: &Path) -> Result<Self, Error> {
let script = include_str!("get_interpreter_info.py");
let output = if cfg!(windows)
Expand Down Expand Up @@ -349,6 +349,10 @@ impl InterpreterQueryResult {
// 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
if !output.status.success() || !output.stderr.is_empty() {
if output.status.code() == Some(3) {
return Err(Error::Python2OrOlder);
}

return Err(Error::PythonSubcommandOutput {
message: format!(
"Querying Python at `{}` failed with status {}",
Expand All @@ -359,7 +363,8 @@ impl InterpreterQueryResult {
stderr: String::from_utf8_lossy(&output.stderr).trim().to_string(),
});
}
let data = serde_json::from_slice::<Self>(&output.stdout).map_err(|err| {

let data: Self = serde_json::from_slice(&output.stdout).map_err(|err| {
Error::PythonSubcommandOutput {
message: format!(
"Querying Python at `{}` did not return the expected data: {err}",
Expand Down
2 changes: 2 additions & 0 deletions crates/uv-interpreter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ pub enum Error {
stdout: String,
stderr: String,
},
#[error("Python 2 or older is not supported. Please use Python 3 or newer.")]
Python2OrOlder,
#[error("Failed to write to cache")]
Encode(#[from] rmp_serde::encode::Error),
#[error("Broken virtualenv: Failed to parse pyvenv.cfg")]
Expand Down
26 changes: 23 additions & 3 deletions crates/uv-interpreter/src/python_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,20 @@ fn find_python(
continue;
}

let installation = PythonInstallation::Interpreter(Interpreter::query(
&path, platform, cache,
)?);
let interpreter = match Interpreter::query(&path, platform, cache) {
Ok(interpreter) => interpreter,
Err(Error::Python2OrOlder) => {
if selector.major() <= Some(2) {
return Err(Error::Python2OrOlder);
}
// Skip over Python 2 or older installation when querying for a recent python installation.
tracing::debug!("Found a Python 2 installation that isn't supported by uv, skipping.");
continue;
}
Err(error) => return Err(error),
};

let installation = PythonInstallation::Interpreter(interpreter);

if let Some(interpreter) = installation.select(selector, platform, cache)? {
return Ok(Some(interpreter));
Expand Down Expand Up @@ -306,6 +317,15 @@ impl PythonVersionSelector {
],
}
}

fn major(self) -> Option<u8> {
match self {
PythonVersionSelector::Default => None,
PythonVersionSelector::Major(major) => Some(major),
PythonVersionSelector::MajorMinor(major, _) => Some(major),
PythonVersionSelector::MajorMinorPatch(major, _, _) => Some(major),
}
}
}

mod windows {
Expand Down

0 comments on commit 829e147

Please sign in to comment.