Skip to content

Commit

Permalink
Sort ManagedPythonInstallation by version (#5140)
Browse files Browse the repository at this point in the history
## Summary
Resolves #5139

`PythonInstallationKey` was sorted as a string, which caused `3.8` to
appear before `3.11`. This update changes the sorting of
`PythonInstallationKey` to be a descending order by version.

## Test Plan
```sh
$ cargo run -- python install 3.8 3.12
$ cargo run -- tool run -v python -V
DEBUG uv 0.2.25
warning: `uv tool run` is experimental and may change without warning.
DEBUG Searching for Python interpreter in managed installations, system path, or `py` launcher
DEBUG Searching for managed installations at `C:\Users\xx\AppData\Roaming\uv\data\python`
DEBUG Found managed Python `cpython-3.12.3-windows-x86_64-none`
DEBUG Found cpython 3.12.3 at `C:\Users\xx\AppData\Roaming\uv\data\python\cpython-3.12.3-windows-x86_64-none\install\python.exe` (managed installations)
DEBUG Using request timeout of 30s
DEBUG Using request timeout of 30s
DEBUG Acquired lock for `C:\Users\nigel\AppData\Roaming\uv\data\tools`
DEBUG Using existing environment for tool `httpx`: C:\Users\xx\AppData\Roaming\uv\data\tools\httpx
DEBUG Using existing tool `httpx`
DEBUG Running `httpx -v`
```
  • Loading branch information
j178 committed Jul 17, 2024
1 parent 6705093 commit 0acb616
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 9 deletions.
8 changes: 7 additions & 1 deletion crates/uv-python/src/installation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,14 @@ impl PartialOrd for PythonInstallationKey {
Some(self.cmp(other))
}
}

impl Ord for PythonInstallationKey {
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.to_string().cmp(&other.to_string())
self.implementation
.cmp(&other.implementation)
.then_with(|| other.version().cmp(&self.version()))
.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()))
}
}
14 changes: 6 additions & 8 deletions crates/uv-python/src/managed.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use core::fmt;
use fs_err as fs;
use std::collections::BTreeSet;
use itertools::Itertools;
use std::ffi::OsStr;
use std::io::{self, Write};
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -134,17 +134,15 @@ impl ManagedPythonInstallations {

/// Iterate over each Python installation in this directory.
///
/// Pythons are sorted descending by name, such that we get deterministic
/// ordering across platforms. This also results in newer Python versions coming first,
/// but should not be relied on — instead the installations should be sorted later by
/// the parsed Python version.
/// Pythons are sorted by [`PythonInstallationKey`], for the same implementation name, the newest versions come first.
/// This ensures a consistent ordering across all platforms.
pub fn find_all(
&self,
) -> Result<impl DoubleEndedIterator<Item = ManagedPythonInstallation>, Error> {
let dirs = match fs_err::read_dir(&self.root) {
Ok(installation_dirs) => {
// Collect sorted directory paths; `read_dir` is not stable across platforms
let directories: BTreeSet<_> = installation_dirs
let directories: Vec<_> = installation_dirs
.filter_map(|read_dir| match read_dir {
Ok(entry) => match entry.file_type() {
Ok(file_type) => file_type.is_dir().then_some(Ok(entry.path())),
Expand All @@ -159,7 +157,7 @@ impl ManagedPythonInstallations {
})?;
directories
}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => BTreeSet::default(),
Err(err) if err.kind() == std::io::ErrorKind::NotFound => vec![],
Err(err) => {
return Err(Error::ReadError {
dir: self.root.clone(),
Expand All @@ -176,7 +174,7 @@ impl ManagedPythonInstallations {
})
.ok()
})
.rev())
.sorted_unstable_by_key(|installation| installation.key().clone()))
}

/// Iterate over Python installations that support the current platform.
Expand Down

0 comments on commit 0acb616

Please sign in to comment.