From 0acb6165dcdf2e803fbd6ec244aa4dbec69896a2 Mon Sep 17 00:00:00 2001
From: Jo <10510431+j178@users.noreply.github.com>
Date: Wed, 17 Jul 2024 22:48:04 +0800
Subject: [PATCH] Sort `ManagedPythonInstallation` by version (#5140)

## 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`
```
---
 crates/uv-python/src/installation.rs |  8 +++++++-
 crates/uv-python/src/managed.rs      | 14 ++++++--------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/crates/uv-python/src/installation.rs b/crates/uv-python/src/installation.rs
index 92995a6ab87c..11a15ff73bac 100644
--- a/crates/uv-python/src/installation.rs
+++ b/crates/uv-python/src/installation.rs
@@ -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()))
     }
 }
diff --git a/crates/uv-python/src/managed.rs b/crates/uv-python/src/managed.rs
index db40b95ace2c..6e87c40b311c 100644
--- a/crates/uv-python/src/managed.rs
+++ b/crates/uv-python/src/managed.rs
@@ -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};
@@ -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())),
@@ -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(),
@@ -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.