From fb264ea65669fb8f55f98ebf97e19555f9037c7d Mon Sep 17 00:00:00 2001
From: Zanie Blue <contact@zanie.dev>
Date: Tue, 3 Dec 2024 09:10:51 -0600
Subject: [PATCH] Fix Python executable installation when multiple patch
 versions are requested

---
 crates/uv/src/commands/python/install.rs | 35 ++++++++++++++++++-----
 crates/uv/tests/it/python_install.rs     | 36 ++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/crates/uv/src/commands/python/install.rs b/crates/uv/src/commands/python/install.rs
index 7bd67d48ab73..cdb5b86dff37 100644
--- a/crates/uv/src/commands/python/install.rs
+++ b/crates/uv/src/commands/python/install.rs
@@ -300,9 +300,11 @@ pub(crate) async fn install(
         None
     };
 
+    let installations: Vec<_> = downloaded.iter().chain(satisfied.iter().copied()).collect();
+
     // Ensure that the installations are _complete_ for both downloaded installations and existing
     // installations that match the request
-    for installation in downloaded.iter().chain(satisfied.iter().copied()) {
+    for installation in &installations {
         installation.ensure_externally_managed()?;
         installation.ensure_canonical_executables()?;
 
@@ -353,7 +355,13 @@ pub(crate) async fn install(
                     );
 
                     // Figure out what installation it references, if any
-                    let existing = find_matching_bin_link(&existing_installations, &target);
+                    let existing = find_matching_bin_link(
+                        installations
+                            .iter()
+                            .copied()
+                            .chain(existing_installations.iter()),
+                        &target,
+                    );
 
                     match existing {
                         None => {
@@ -373,7 +381,7 @@ pub(crate) async fn install(
                                 target.simplified_display()
                             );
                         }
-                        Some(existing) if existing == installation => {
+                        Some(existing) if existing == *installation => {
                             // The existing link points to the same installation, so we're done unless
                             // they requested we reinstall
                             if !(reinstall || force) {
@@ -429,6 +437,17 @@ pub(crate) async fn install(
 
                     // Replace the existing link
                     fs_err::remove_file(&to)?;
+
+                    if let Some(existing) = existing {
+                        // Ensure we do not report installation of this executable for an existing
+                        // key if we undo it
+                        changelog
+                            .installed_executables
+                            .entry(existing.key().clone())
+                            .or_default()
+                            .remove(&target);
+                    }
+
                     installation.create_bin_link(&target)?;
                     debug!(
                         "Updated executable at `{}` to {}",
@@ -562,6 +581,10 @@ pub(crate) fn format_executables(
         return String::new();
     };
 
+    if installed.is_empty() {
+        return String::new();
+    }
+
     let names = installed
         .iter()
         .filter_map(|path| path.file_name())
@@ -612,7 +635,7 @@ fn warn_if_not_on_path(bin: &Path) {
 /// Like [`ManagedPythonInstallation::is_bin_link`], but this method will only resolve the
 /// given path one time.
 fn find_matching_bin_link<'a>(
-    installations: &'a [ManagedPythonInstallation],
+    mut installations: impl Iterator<Item = &'a ManagedPythonInstallation>,
     path: &Path,
 ) -> Option<&'a ManagedPythonInstallation> {
     let target = if cfg!(unix) {
@@ -630,7 +653,5 @@ fn find_matching_bin_link<'a>(
         unreachable!("Only Windows and Unix are supported")
     };
 
-    installations
-        .iter()
-        .find(|installation| installation.executable() == target)
+    installations.find(|installation| installation.executable() == target)
 }
diff --git a/crates/uv/tests/it/python_install.rs b/crates/uv/tests/it/python_install.rs
index 2a4d3886eacf..99699255b0b1 100644
--- a/crates/uv/tests/it/python_install.rs
+++ b/crates/uv/tests/it/python_install.rs
@@ -227,6 +227,42 @@ fn python_install_preview() {
 
     // The executable should be removed
     bin_python.assert(predicate::path::missing());
+
+    // Install multiple patch versions
+    uv_snapshot!(context.filters(), context.python_install().arg("--preview").arg("3.12.7").arg("3.12.6"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Installed 2 versions in [TIME]
+     + cpython-3.12.6-[PLATFORM]
+     + cpython-3.12.7-[PLATFORM] (python3.12)
+    "###);
+
+    let bin_python = context
+        .temp_dir
+        .child("bin")
+        .child(format!("python3.12{}", std::env::consts::EXE_SUFFIX));
+
+    // The link should be for the newer patch version
+    if cfg!(unix) {
+        insta::with_settings!({
+            filters => context.filters(),
+        }, {
+            insta::assert_snapshot!(
+                read_link_path(&bin_python), @"[TEMP_DIR]/managed/cpython-3.12.7-[PLATFORM]/bin/python3.12"
+            );
+        });
+    } else {
+        insta::with_settings!({
+            filters => context.filters(),
+        }, {
+            insta::assert_snapshot!(
+                read_link_path(&bin_python), @"[TEMP_DIR]/managed/cpython-3.12.7-[PLATFORM]/python"
+            );
+        });
+    }
 }
 
 #[test]