Skip to content

Commit

Permalink
Avoid panicking when encountering an invalid Python version during `u…
Browse files Browse the repository at this point in the history
…v python list` (#7131)

Closes #7129

Not entirely sure about the best approach yet.
  • Loading branch information
zanieb committed Sep 6, 2024
1 parent 8a0e1fd commit 8eff8aa
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 11 deletions.
4 changes: 3 additions & 1 deletion crates/uv-python/src/downloads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,9 @@ impl ManagedPythonDownload {
}

pub fn python_version(&self) -> PythonVersion {
self.key.version()
self.key
.version()
.expect("Managed Python downloads should always have valid versions")
}

/// Return the [`Url`] to use when downloading the distribution. If a mirror is set via the
Expand Down
10 changes: 7 additions & 3 deletions crates/uv-python/src/installation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,8 @@ impl PythonInstallationKey {
&self.implementation
}

pub fn version(&self) -> PythonVersion {
pub fn version(&self) -> Result<PythonVersion, String> {
PythonVersion::from_str(&format!("{}.{}.{}", self.major, self.minor, self.patch))
.expect("Python installation keys must have valid Python versions")
}

pub fn arch(&self) -> &Arch {
Expand Down Expand Up @@ -338,7 +337,12 @@ impl Ord for PythonInstallationKey {
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.implementation
.cmp(&other.implementation)
.then_with(|| self.version().cmp(&other.version()))
.then_with(|| {
self.major
.cmp(&other.major)
.then_with(|| self.minor.cmp(&other.minor))
.then_with(|| self.patch.cmp(&other.patch))
})
.then_with(|| self.os.to_string().cmp(&other.os.to_string()))
.then_with(|| self.arch.to_string().cmp(&other.arch.to_string()))
.then_with(|| self.libc.to_string().cmp(&other.libc.to_string()))
Expand Down
12 changes: 9 additions & 3 deletions crates/uv-python/src/managed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,9 @@ impl ManagedPythonInstallation {

/// The [`PythonVersion`] of the toolchain.
pub fn version(&self) -> PythonVersion {
self.key.version()
self.key
.version()
.expect("Managed Python installations should always have valid versions")
}

pub fn implementation(&self) -> &ImplementationName {
Expand Down Expand Up @@ -329,13 +331,17 @@ impl ManagedPythonInstallation {
let stdlib = if matches!(self.key.os, Os(target_lexicon::OperatingSystem::Windows)) {
self.python_dir().join("Lib")
} else {
let version = self
.key
.version()
.expect("Managed Python installations should always have valid versions");
let python = if matches!(
self.key.implementation,
LenientImplementationName::Known(ImplementationName::PyPy)
) {
format!("pypy{}", self.key.version().python_version())
format!("pypy{}", version.python_version())
} else {
format!("python{}", self.key.version().python_version())
format!("python{}", version.python_version())
};
self.python_dir().join("lib").join(python)
};
Expand Down
8 changes: 7 additions & 1 deletion crates/uv/src/commands/python/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,13 @@ pub(crate) async fn install(
"{}",
format!(
"Installed {} {}",
format!("Python {}", installed.version()).bold(),
format!(
"Python {}",
installed.version().expect(
"Managed Python installations should always have valid versions"
)
)
.bold(),
format!("in {}", elapsed(start.elapsed())).dimmed()
)
.dimmed()
Expand Down
13 changes: 11 additions & 2 deletions crates/uv/src/commands/python/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::fmt::Write;
use anyhow::Result;
use owo_colors::OwoColorize;
use rustc_hash::FxHashSet;
use tracing::warn;
use uv_cache::Cache;
use uv_fs::Simplified;
use uv_python::downloads::PythonDownloadRequest;
Expand Down Expand Up @@ -106,9 +107,17 @@ pub(crate) async fn list(
}
}

let version = match key.version() {
Err(err) => {
warn!("Excluding {key} due to invalid Python version: {err}");
continue;
}
Ok(version) => version,
};

// Only show the latest patch version for each download unless all were requested
if !matches!(kind, Kind::System) {
if let [major, minor, ..] = key.version().release() {
if let [major, minor, ..] = version.release() {
if !seen_minor.insert((
*key.os(),
*major,
Expand All @@ -122,7 +131,7 @@ pub(crate) async fn list(
}
}
}
if let [major, minor, patch] = key.version().release() {
if let [major, minor, patch] = version.release() {
if !seen_patch.insert((
*key.os(),
*major,
Expand Down
8 changes: 7 additions & 1 deletion crates/uv/src/commands/python/uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,13 @@ async fn do_uninstall(
"{}",
format!(
"Uninstalled {} {}",
format!("Python {}", uninstalled.version()).bold(),
format!(
"Python {}",
uninstalled.version().expect(
"Managed Python installations should always have valid versions"
)
)
.bold(),
format!("in {}", elapsed(start.elapsed())).dimmed()
)
.dimmed()
Expand Down

0 comments on commit 8eff8aa

Please sign in to comment.