Use +- install output for Python versions (#5201)

## Summary Follow-up to #4939. Uses a format that's closer to `uv pip install`, with some special-casing for single Pythons. ## Test Plan A few examples: ![Screenshot 2024-07-18 at 8 39 35 PM](https://github.com/user-attachments/assets/868d4c87-d8f4-4e5f-a52c-1f7a3e8d6a16) ![Screenshot 2024-07-18 at 8 39 46 PM](https://github.com/user-attachments/assets/3a12461e-9d9b-4c33-a685-55ca7256ff52) ![Screenshot 2024-07-18 at 8 39 27 PM](https://github.com/user-attachments/assets/1059e6d6-a445-4531-8496-59bc51d01663) ![Screenshot 2024-07-18 at 8 39 54 PM](https://github.com/user-attachments/assets/dcb69e86-8702-402b-a0cd-6f827b04a6ab)
astral-sh · Jul 19, 2024 · d54ae4e · d54ae4e
1 parent 360079f
commit d54ae4e
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 64 deletions.
diff --git a/crates/uv-python/src/installation.rs b/crates/uv-python/src/installation.rs
@@ -350,7 +350,7 @@ impl Ord for PythonInstallationKey {
     fn cmp(&self, other: &Self) -> std::cmp::Ordering {
         self.implementation
             .cmp(&other.implementation)
-            .then_with(|| other.version().cmp(&self.version()))
+            .then_with(|| self.version().cmp(&other.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/src/commands/python/install.rs b/crates/uv/src/commands/python/install.rs
@@ -11,14 +11,14 @@ use tracing::debug;
 use uv_cache::Cache;
 use uv_client::Connectivity;
 use uv_configuration::PreviewMode;
-use uv_fs::Simplified;
 use uv_python::downloads::{DownloadResult, ManagedPythonDownload, PythonDownloadRequest};
 use uv_python::managed::{ManagedPythonInstallation, ManagedPythonInstallations};
 use uv_python::{
     requests_from_version_file, PythonRequest, PYTHON_VERSIONS_FILENAME, PYTHON_VERSION_FILENAME,
 };
 use uv_warnings::warn_user_once;
 
+use crate::commands::python::{ChangeEvent, ChangeEventKind};
 use crate::commands::reporters::PythonDownloadReporter;
 use crate::commands::{elapsed, ExitStatus};
 use crate::printer::Printer;
@@ -76,6 +76,7 @@ pub(crate) async fn install(
 
     let installed_installations: Vec<_> = installations.find_all()?.collect();
     let mut unfilled_requests = Vec::new();
+    let mut uninstalled = Vec::new();
     for (request, download_request) in requests.iter().zip(download_requests) {
         if matches!(requests.as_slice(), [PythonRequest::Any]) {
             writeln!(printer.stderr(), "Searching for Python installations")?;
@@ -101,12 +102,8 @@ pub(crate) async fn install(
                 )?;
             }
             if reinstall {
-                writeln!(
-                    printer.stderr(),
-                    "Uninstalling {}",
-                    installation.key().green()
-                )?;
                 fs::remove_dir_all(installation.path())?;
+                uninstalled.push(installation.key().clone());
                 unfilled_requests.push(download_request);
             }
         } else {
@@ -152,41 +149,32 @@ pub(crate) async fn install(
             let result = download
                 .fetch(&client, installations_dir, Some(&reporter))
                 .await;
-            (download.python_version(), result)
+            (download.key(), result)
         })
         .buffered(4)
         .collect::<Vec<_>>()
         .await;
 
+    let mut installed = vec![];
     let mut failed = false;
-    for (version, result) in results {
+    for (key, result) in results {
         match result {
             Ok(download) => {
                 let path = match download {
                     // We should only encounter already-available during concurrent installs
                     DownloadResult::AlreadyAvailable(path) => path,
-                    DownloadResult::Fetched(path) => {
-                        writeln!(
-                            printer.stderr(),
-                            "Installed {} to: {}",
-                            format!("Python {version}").cyan(),
-                            path.user_display().cyan()
-                        )?;
-                        path
-                    }
+                    DownloadResult::Fetched(path) => path,
                 };
 
+                installed.push(key.clone());
+
                 // Ensure the installations have externally managed markers
-                let installed = ManagedPythonInstallation::new(path.clone())?;
-                installed.ensure_externally_managed()?;
+                let managed = ManagedPythonInstallation::new(path.clone())?;
+                managed.ensure_externally_managed()?;
             }
             Err(err) => {
                 failed = true;
-                writeln!(
-                    printer.stderr(),
-                    "Failed to install {}: {err}",
-                    version.green()
-                )?;
+                writeln!(printer.stderr(), "Failed to install {}: {err}", key.green())?;
             }
         }
     }
@@ -198,17 +186,54 @@ pub(crate) async fn install(
         return Ok(ExitStatus::Failure);
     }
 
-    let s = if downloads.len() == 1 { "" } else { "s" };
-    writeln!(
-        printer.stderr(),
-        "{}",
-        format!(
-            "Installed {} {}",
-            format!("{} version{s}", downloads.len()).bold(),
-            format!("in {}", elapsed(start.elapsed())).dimmed()
-        )
-        .dimmed()
-    )?;
+    if let [installed] = installed.as_slice() {
+        // Ex) "Installed Python 3.9.7 in 1.68s"
+        writeln!(
+            printer.stderr(),
+            "{}",
+            format!(
+                "Installed {} {}",
+                format!("Python {}", installed.version()).bold(),
+                format!("in {}", elapsed(start.elapsed())).dimmed()
+            )
+            .dimmed()
+        )?;
+    } else {
+        // Ex) "Installed 2 versions in 1.68s"
+        let s = if installed.len() == 1 { "" } else { "s" };
+        writeln!(
+            printer.stderr(),
+            "{}",
+            format!(
+                "Installed {} {}",
+                format!("{} version{s}", installed.len()).bold(),
+                format!("in {}", elapsed(start.elapsed())).dimmed()
+            )
+            .dimmed()
+        )?;
+    }
+
+    for event in uninstalled
+        .into_iter()
+        .map(|key| ChangeEvent {
+            key,
+            kind: ChangeEventKind::Removed,
+        })
+        .chain(installed.into_iter().map(|key| ChangeEvent {
+            key,
+            kind: ChangeEventKind::Added,
+        }))
+        .sorted_unstable_by(|a, b| a.key.cmp(&b.key).then_with(|| a.kind.cmp(&b.kind)))
+    {
+        match event.kind {
+            ChangeEventKind::Added => {
+                writeln!(printer.stderr(), " {} {}", "+".green(), event.key.bold(),)?;
+            }
+            ChangeEventKind::Removed => {
+                writeln!(printer.stderr(), " {} {}", "-".red(), event.key.bold(),)?;
+            }
+        }
+    }
 
     Ok(ExitStatus::Success)
 }
diff --git a/crates/uv/src/commands/python/mod.rs b/crates/uv/src/commands/python/mod.rs
@@ -4,3 +4,17 @@ pub(crate) mod install;
 pub(crate) mod list;
 pub(crate) mod pin;
 pub(crate) mod uninstall;
+
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
+pub(super) enum ChangeEventKind {
+    /// The Python version was uninstalled.
+    Removed,
+    /// The Python version was installed.
+    Added,
+}
+
+#[derive(Debug)]
+pub(super) struct ChangeEvent {
+    key: uv_python::PythonInstallationKey,
+    kind: ChangeEventKind,
+}
diff --git a/crates/uv/src/commands/python/uninstall.rs b/crates/uv/src/commands/python/uninstall.rs
@@ -12,6 +12,7 @@ use uv_python::managed::ManagedPythonInstallations;
 use uv_python::PythonRequest;
 use uv_warnings::warn_user_once;
 
+use crate::commands::python::{ChangeEvent, ChangeEventKind};
 use crate::commands::{elapsed, ExitStatus};
 use crate::printer::Printer;
 
@@ -68,18 +69,7 @@ pub(crate) async fn uninstall(
             .filter(|installation| download_request.satisfied_by_key(installation.key()))
         {
             found = true;
-            if matching_installations.insert(installation.clone()) {
-                if matches!(requests.as_slice(), [PythonRequest::Any]) {
-                    writeln!(printer.stderr(), "Found: {}", installation.key().green(),)?;
-                } else {
-                    writeln!(
-                        printer.stderr(),
-                        "Found existing installation for {}: {}",
-                        request.cyan(),
-                        installation.key().green(),
-                    )?;
-                }
-            }
+            matching_installations.insert(installation.clone());
         }
         if !found {
             if matches!(requests.as_slice(), [PythonRequest::Any]) {
@@ -114,8 +104,9 @@ pub(crate) async fn uninstall(
         .collect::<Vec<_>>()
         .await;
 
+    let mut uninstalled = vec![];
     let mut failed = false;
-    for (key, result) in results.iter().sorted_by_key(|(key, _)| key) {
+    for (key, result) in results {
         if let Err(err) = result {
             failed = true;
             writeln!(
@@ -124,7 +115,7 @@ pub(crate) async fn uninstall(
                 key.green()
             )?;
         } else {
-            writeln!(printer.stderr(), "Uninstalled: {}", key.green())?;
+            uninstalled.push(key.clone());
         }
     }
 
@@ -135,21 +126,50 @@ pub(crate) async fn uninstall(
         return Ok(ExitStatus::Failure);
     }
 
-    let s = if matching_installations.len() == 1 {
-        ""
+    if let [uninstalled] = uninstalled.as_slice() {
+        // Ex) "Uninstalled Python 3.9.7 in 1.68s"
+        writeln!(
+            printer.stderr(),
+            "{}",
+            format!(
+                "Uninstalled {} {}",
+                format!("Python {}", uninstalled.version()).bold(),
+                format!("in {}", elapsed(start.elapsed())).dimmed()
+            )
+            .dimmed()
+        )?;
     } else {
-        "s"
-    };
-    writeln!(
-        printer.stderr(),
-        "{}",
-        format!(
-            "Uninstalled {} {}",
-            format!("{} version{s}", matching_installations.len()).bold(),
-            format!("in {}", elapsed(start.elapsed())).dimmed()
-        )
-        .dimmed()
-    )?;
+        // Ex) "Uninstalled 2 versions in 1.68s"
+        let s = if uninstalled.len() == 1 { "" } else { "s" };
+        writeln!(
+            printer.stderr(),
+            "{}",
+            format!(
+                "Uninstalled {} {}",
+                format!("{} version{s}", uninstalled.len()).bold(),
+                format!("in {}", elapsed(start.elapsed())).dimmed()
+            )
+            .dimmed()
+        )?;
+    }
+
+    for event in uninstalled
+        .into_iter()
+        .map(|key| ChangeEvent {
+            key,
+            kind: ChangeEventKind::Removed,
+        })
+        .sorted_unstable_by(|a, b| a.key.cmp(&b.key).then_with(|| a.kind.cmp(&b.kind)))
+    {
+        match event.kind {
+            ChangeEventKind::Added => {
+                writeln!(printer.stderr(), " {} {}", "+".green(), event.key.bold(),)?;
+            }
+            ChangeEventKind::Removed => {
+                writeln!(printer.stderr(), " {} {}", "-".red(), event.key.bold(),)?;
+            }
+        }
+    }
 
     Ok(ExitStatus::Success)
 }