From d54ae4e381b7663604f53dd0676ef26792997f4b Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 18 Jul 2024 20:46:33 -0400 Subject: [PATCH] Use +- install output for Python versions (#5201) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Follow-up to https://github.com/astral-sh/uv/pull/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) --- crates/uv-python/src/installation.rs | 2 +- crates/uv/src/commands/python/install.rs | 95 ++++++++++++++-------- crates/uv/src/commands/python/mod.rs | 14 ++++ crates/uv/src/commands/python/uninstall.rs | 76 ++++++++++------- 4 files changed, 123 insertions(+), 64 deletions(-) diff --git a/crates/uv-python/src/installation.rs b/crates/uv-python/src/installation.rs index 75b8c2c17dc5..0efe5417486b 100644 --- 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 index 6af8cf282e58..95a2235f6983 100644 --- a/crates/uv/src/commands/python/install.rs +++ b/crates/uv/src/commands/python/install.rs @@ -11,7 +11,6 @@ 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::{ @@ -19,6 +18,7 @@ use uv_python::{ }; 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::>() .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 index b63ec59d5fe8..80a39fae14e1 100644 --- 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 index 5ab7c9dc6be3..aded478c2692 100644 --- 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::>() .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) }