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
72 changes: 42 additions & 30 deletions crates/uv-python/src/installation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,36 +92,48 @@ impl PythonInstallation {
let request = request.unwrap_or(&PythonRequest::Default);

// Search for the installation
match Self::find(request, environments, preference, cache) {
Ok(venv) => Ok(venv),
// If missing and allowed, perform a fetch
Err(Error::MissingPython(err))
Copy link
Member Author

@zanieb zanieb Jan 23, 2025

Choose a reason for hiding this comment

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

#10716 added a throw of Error::Discovery instead of a PythonNotFound error (which is mapped to MissingPython transparently here) which means we no longer fallback.

Ideally we'd implement #10716 higher up, like here? but couldn't think of a clean way to do that.

if preference.allows_managed()
&& python_downloads.is_automatic()
&& client_builder.connectivity.is_online() =>
{
if let Some(request) = PythonDownloadRequest::from_request(request) {
debug!("Requested Python not found, checking for available download...");
match Self::fetch(
request.fill()?,
client_builder,
cache,
reporter,
python_install_mirror,
pypy_install_mirror,
)
.await
{
Ok(installation) => Ok(installation),
Err(Error::Download(downloads::Error::NoDownloadFound(_))) => {
Err(Error::MissingPython(err))
}
Err(err) => Err(err),
}
} else {
Err(Error::MissingPython(err))
}
}
let err = match Self::find(request, environments, preference, cache) {
Ok(installation) => return Ok(installation),
Err(err) => err,
};

let downloads_enabled = preference.allows_managed()
&& python_downloads.is_automatic()
&& client_builder.connectivity.is_online();

if !downloads_enabled {
return Err(err);
}

match err {
// If Python is missing, we should attempt a download
Error::MissingPython(_) => {}
// If we raised a non-critical error, we should attempt a download
Error::Discovery(ref err) if !err.is_critical() => {}
Comment on lines +111 to +112
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the key change. The rest is just refactoring so we can match on multiple error types cleanly.

// Otherwise, this is fatal
_ => return Err(err),
}

// If we can't convert the request to a download, throw the original error
let Some(request) = PythonDownloadRequest::from_request(request) else {
return Err(err);
};

debug!("Requested Python not found, checking for available download...");
match Self::fetch(
request.fill()?,
client_builder,
cache,
reporter,
python_install_mirror,
pypy_install_mirror,
)
.await
{
Ok(installation) => Ok(installation),
// Throw the original error if we couldn't find a download
Err(Error::Download(downloads::Error::NoDownloadFound(_))) => Err(err),
// But if the download failed, throw that error
Err(err) => Err(err),
}
}
Expand Down
10 changes: 3 additions & 7 deletions crates/uv/tests/it/python_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,16 +188,12 @@ fn python_install_automatic() {
.env("UV_TEST_PYTHON_PATH", context.bin_dir.as_os_str())
.arg("-p").arg("3.11")
.arg("python").arg("-c").arg("import sys; print(sys.version_info[:2])"), @r###"
success: false
exit_code: 2
success: true
exit_code: 0
----- stdout -----
(3, 11)

----- stderr -----
error: Failed to inspect Python interpreter from search path at `[BIN]/python3`
Caused by: Querying Python at `[BIN]/python3` failed with exit status exit status: 1

[stderr]
error: intentionally broken python executable
"###);
}
}
Expand Down
Loading