From a0a0a8d0e5377ab232530814d7debd0f1aeb7598 Mon Sep 17 00:00:00 2001 From: "Tomasz (Tom) Kramkowski" Date: Mon, 8 Dec 2025 15:04:06 +0000 Subject: [PATCH] Support dry-run in InstallLoggers --- crates/uv/src/commands/pip/install.rs | 6 +- crates/uv/src/commands/pip/loggers.rs | 144 +++++++++++++++++------ crates/uv/src/commands/pip/operations.rs | 48 +++----- crates/uv/src/commands/venv.rs | 6 +- 4 files changed, 129 insertions(+), 75 deletions(-) diff --git a/crates/uv/src/commands/pip/install.rs b/crates/uv/src/commands/pip/install.rs index dc1bcda6a..55fccdae0 100644 --- a/crates/uv/src/commands/pip/install.rs +++ b/crates/uv/src/commands/pip/install.rs @@ -1,5 +1,4 @@ use std::collections::BTreeSet; -use std::fmt::Write; use anyhow::Context; use itertools::Itertools; @@ -349,10 +348,7 @@ pub(crate) async fn pip_install( debug!("Requirement satisfied: {requirement}"); } } - DefaultInstallLogger.on_audit(requirements.len(), start, printer)?; - if dry_run.enabled() { - writeln!(printer.stderr(), "Would make no changes")?; - } + DefaultInstallLogger.on_audit(requirements.len(), start, printer, dry_run)?; return Ok(ExitStatus::Success); } diff --git a/crates/uv/src/commands/pip/loggers.rs b/crates/uv/src/commands/pip/loggers.rs index 091d4c58e..777efc842 100644 --- a/crates/uv/src/commands/pip/loggers.rs +++ b/crates/uv/src/commands/pip/loggers.rs @@ -6,6 +6,7 @@ use itertools::Itertools; use owo_colors::OwoColorize; use rustc_hash::{FxBuildHasher, FxHashMap}; +use uv_configuration::DryRun; use uv_distribution_types::{InstalledMetadata, Name}; use uv_normalize::PackageName; use uv_pep440::Version; @@ -17,7 +18,13 @@ use crate::printer::Printer; /// A trait to handle logging during install operations. pub(crate) trait InstallLogger { /// Log the completion of the audit phase. - fn on_audit(&self, count: usize, start: std::time::Instant, printer: Printer) -> fmt::Result; + fn on_audit( + &self, + count: usize, + start: std::time::Instant, + printer: Printer, + dry_run: DryRun, + ) -> fmt::Result; /// Log the completion of the preparation phase. fn on_prepare( @@ -26,6 +33,7 @@ pub(crate) trait InstallLogger { suffix: Option<&str>, start: std::time::Instant, printer: Printer, + dry_run: DryRun, ) -> fmt::Result; /// Log the completion of the uninstallation phase. @@ -34,13 +42,20 @@ pub(crate) trait InstallLogger { count: usize, start: std::time::Instant, printer: Printer, + dry_run: DryRun, ) -> fmt::Result; /// Log the completion of the installation phase. - fn on_install(&self, count: usize, start: std::time::Instant, printer: Printer) -> fmt::Result; + fn on_install( + &self, + count: usize, + start: std::time::Instant, + printer: Printer, + dry_run: DryRun, + ) -> fmt::Result; /// Log the completion of the operation. - fn on_complete(&self, changelog: &Changelog, printer: Printer) -> fmt::Result; + fn on_complete(&self, changelog: &Changelog, printer: Printer, dry_run: DryRun) -> fmt::Result; } /// The default logger for install operations. @@ -48,13 +63,19 @@ pub(crate) trait InstallLogger { pub(crate) struct DefaultInstallLogger; impl InstallLogger for DefaultInstallLogger { - fn on_audit(&self, count: usize, start: std::time::Instant, printer: Printer) -> fmt::Result { + fn on_audit( + &self, + count: usize, + start: std::time::Instant, + printer: Printer, + dry_run: DryRun, + ) -> fmt::Result { if count == 0 { writeln!( printer.stderr(), "{}", format!("Audited in {}", elapsed(start.elapsed())).dimmed() - ) + )?; } else { let s = if count == 1 { "" } else { "s" }; writeln!( @@ -66,8 +87,12 @@ impl InstallLogger for DefaultInstallLogger { format!("in {}", elapsed(start.elapsed())).dimmed() ) .dimmed() - ) + )?; } + if dry_run.enabled() { + writeln!(printer.stderr(), "Would make no changes")?; + } + Ok(()) } fn on_prepare( @@ -76,21 +101,26 @@ impl InstallLogger for DefaultInstallLogger { suffix: Option<&str>, start: std::time::Instant, printer: Printer, + dry_run: DryRun, ) -> fmt::Result { let s = if count == 1 { "" } else { "s" }; + let what = if let Some(suffix) = suffix { + format!("{count} package{s} {suffix}") + } else { + format!("{count} package{s}") + }; + let what = what.bold(); writeln!( printer.stderr(), "{}", - format!( - "Prepared {} {}", - if let Some(suffix) = suffix { - format!("{count} package{s} {suffix}") - } else { - format!("{count} package{s}") - } - .bold(), - format!("in {}", elapsed(start.elapsed())).dimmed() - ) + if dry_run.enabled() { + format!("Would download {what}") + } else { + format!( + "Prepared {what} {}", + format!("in {}", elapsed(start.elapsed())).dimmed() + ) + } .dimmed() ) } @@ -100,35 +130,57 @@ impl InstallLogger for DefaultInstallLogger { count: usize, start: std::time::Instant, printer: Printer, + dry_run: DryRun, ) -> fmt::Result { let s = if count == 1 { "" } else { "s" }; + let what = format!("{count} package{s}"); + let what = what.bold(); writeln!( printer.stderr(), "{}", - format!( - "Uninstalled {} {}", - format!("{count} package{s}").bold(), - format!("in {}", elapsed(start.elapsed())).dimmed() - ) + if dry_run.enabled() { + format!("Would uninstall {what}") + } else { + format!( + "Uninstalled {what} {}", + format!("in {}", elapsed(start.elapsed())).dimmed() + ) + } .dimmed() ) } - fn on_install(&self, count: usize, start: std::time::Instant, printer: Printer) -> fmt::Result { + fn on_install( + &self, + count: usize, + start: std::time::Instant, + printer: Printer, + dry_run: DryRun, + ) -> fmt::Result { let s = if count == 1 { "" } else { "s" }; + let what = format!("{count} package{s}"); + let what = what.bold(); writeln!( printer.stderr(), "{}", - format!( - "Installed {} {}", - format!("{count} package{s}").bold(), - format!("in {}", elapsed(start.elapsed())).dimmed() - ) + if dry_run.enabled() { + format!("Would install {what}") + } else { + format!( + "Installed {what} {}", + format!("in {}", elapsed(start.elapsed())).dimmed() + ) + } .dimmed() ) } - fn on_complete(&self, changelog: &Changelog, printer: Printer) -> fmt::Result { + fn on_complete( + &self, + changelog: &Changelog, + printer: Printer, + _dry_run: DryRun, + ) -> fmt::Result { for event in changelog .uninstalled .iter() @@ -202,6 +254,7 @@ impl InstallLogger for SummaryInstallLogger { _count: usize, _start: std::time::Instant, _printer: Printer, + _dry_run: DryRun, ) -> fmt::Result { Ok(()) } @@ -212,6 +265,7 @@ impl InstallLogger for SummaryInstallLogger { _suffix: Option<&str>, _start: std::time::Instant, _printer: Printer, + _dry_run: DryRun, ) -> fmt::Result { Ok(()) } @@ -221,15 +275,27 @@ impl InstallLogger for SummaryInstallLogger { count: usize, start: std::time::Instant, printer: Printer, + dry_run: DryRun, ) -> fmt::Result { - DefaultInstallLogger.on_uninstall(count, start, printer) + DefaultInstallLogger.on_uninstall(count, start, printer, dry_run) } - fn on_install(&self, count: usize, start: std::time::Instant, printer: Printer) -> fmt::Result { - DefaultInstallLogger.on_install(count, start, printer) + fn on_install( + &self, + count: usize, + start: std::time::Instant, + printer: Printer, + dry_run: DryRun, + ) -> fmt::Result { + DefaultInstallLogger.on_install(count, start, printer, dry_run) } - fn on_complete(&self, _changelog: &Changelog, _printer: Printer) -> fmt::Result { + fn on_complete( + &self, + _changelog: &Changelog, + _printer: Printer, + _dry_run: DryRun, + ) -> fmt::Result { Ok(()) } } @@ -253,6 +319,7 @@ impl InstallLogger for UpgradeInstallLogger { _count: usize, _start: std::time::Instant, _printer: Printer, + _dry_run: DryRun, ) -> fmt::Result { Ok(()) } @@ -263,6 +330,7 @@ impl InstallLogger for UpgradeInstallLogger { _suffix: Option<&str>, _start: std::time::Instant, _printer: Printer, + _dry_run: DryRun, ) -> fmt::Result { Ok(()) } @@ -272,6 +340,7 @@ impl InstallLogger for UpgradeInstallLogger { _count: usize, _start: std::time::Instant, _printer: Printer, + _dry_run: DryRun, ) -> fmt::Result { Ok(()) } @@ -281,11 +350,18 @@ impl InstallLogger for UpgradeInstallLogger { _count: usize, _start: std::time::Instant, _printer: Printer, + _dry_run: DryRun, ) -> fmt::Result { Ok(()) } - fn on_complete(&self, changelog: &Changelog, printer: Printer) -> fmt::Result { + fn on_complete( + &self, + changelog: &Changelog, + printer: Printer, + // TODO(tk): Adjust format for dry_run + _dry_run: DryRun, + ) -> fmt::Result { // Index the removals by package name. let removals: FxHashMap<&PackageName, BTreeSet> = changelog.uninstalled.iter().fold( @@ -387,7 +463,7 @@ impl InstallLogger for UpgradeInstallLogger { } // Follow-up with a detailed summary of all changes. - DefaultInstallLogger.on_complete(changelog, printer)?; + DefaultInstallLogger.on_complete(changelog, printer, _dry_run)?; Ok(()) } diff --git a/crates/uv/src/commands/pip/operations.rs b/crates/uv/src/commands/pip/operations.rs index 01db57db0..64e03dfe9 100644 --- a/crates/uv/src/commands/pip/operations.rs +++ b/crates/uv/src/commands/pip/operations.rs @@ -509,7 +509,7 @@ pub(crate) async fn install( && extraneous.is_empty() && !compile { - logger.on_audit(resolution.len(), start, printer)?; + logger.on_audit(resolution.len(), start, printer, dry_run)?; return Ok(Changelog::default()); } @@ -593,7 +593,7 @@ pub(crate) async fn install( let changelog = Changelog::new(installs, uninstalls); // Notify the user of any environment modifications. - logger.on_complete(&changelog, printer)?; + logger.on_complete(&changelog, printer, dry_run)?; Ok(changelog) } @@ -660,7 +660,13 @@ async fn execute_plan( .prepare(remote.clone(), in_flight, resolution) .await?; - logger.on_prepare(wheels.len(), phase.map(InstallPhase::label), start, printer)?; + logger.on_prepare( + wheels.len(), + phase.map(InstallPhase::label), + start, + printer, + DryRun::Disabled, + )?; wheels }; @@ -702,7 +708,7 @@ async fn execute_plan( } } - logger.on_uninstall(uninstalls.len(), start, printer)?; + logger.on_uninstall(uninstalls.len(), start, printer, DryRun::Disabled)?; } // Install the resolved distributions. @@ -721,7 +727,7 @@ async fn execute_plan( // task. .install_blocking(installs)?; - logger.on_install(installs.len(), start, printer)?; + logger.on_install(installs.len(), start, printer, DryRun::Disabled)?; } Ok((installs, uninstalls)) @@ -857,8 +863,7 @@ fn report_dry_run( // Nothing to do. if remote.is_empty() && cached.is_empty() && reinstalls.is_empty() && extraneous.is_empty() { - DefaultInstallLogger.on_audit(resolution.len(), start, printer)?; - writeln!(printer.stderr(), "Would make no changes")?; + DefaultInstallLogger.on_audit(resolution.len(), start, printer, dry_run)?; return Ok(()); } @@ -866,16 +871,7 @@ fn report_dry_run( let wheels = if remote.is_empty() { vec![] } else { - let s = if remote.len() == 1 { "" } else { "s" }; - writeln!( - printer.stderr(), - "{}", - format!( - "Would download {}", - format!("{} package{}", remote.len(), s).bold(), - ) - .dimmed() - )?; + DefaultInstallLogger.on_prepare(remote.len(), None, start, printer, dry_run)?; remote.clone() }; @@ -883,28 +879,14 @@ fn report_dry_run( let uninstalls = extraneous.len() + reinstalls.len(); if uninstalls > 0 { - let s = if uninstalls == 1 { "" } else { "s" }; - writeln!( - printer.stderr(), - "{}", - format!( - "Would uninstall {}", - format!("{uninstalls} package{s}").bold(), - ) - .dimmed() - )?; + DefaultInstallLogger.on_uninstall(uninstalls, start, printer, dry_run)?; } // Install the resolved distributions. let installs = wheels.len() + cached.len(); if installs > 0 { - let s = if installs == 1 { "" } else { "s" }; - writeln!( - printer.stderr(), - "{}", - format!("Would install {}", format!("{installs} package{s}").bold()).dimmed() - )?; + DefaultInstallLogger.on_install(installs, start, printer, dry_run)?; } // TODO(charlie): DRY this up with `report_modifications`. The types don't quite line up. diff --git a/crates/uv/src/commands/venv.rs b/crates/uv/src/commands/venv.rs index 13fe8fb51..eb7526520 100644 --- a/crates/uv/src/commands/venv.rs +++ b/crates/uv/src/commands/venv.rs @@ -10,8 +10,8 @@ use thiserror::Error; use uv_cache::Cache; use uv_client::{BaseClientBuilder, FlatIndexClient, RegistryClientBuilder}; use uv_configuration::{ - BuildOptions, Concurrency, Constraints, DependencyGroups, IndexStrategy, KeyringProviderType, - NoBinary, NoBuild, SourceStrategy, + BuildOptions, Concurrency, Constraints, DependencyGroups, DryRun, IndexStrategy, + KeyringProviderType, NoBinary, NoBuild, SourceStrategy, }; use uv_dispatch::{BuildDispatch, SharedState}; use uv_distribution_types::{ @@ -310,7 +310,7 @@ pub(crate) async fn venv( .map_err(|err| VenvError::Seed(err.into()))?; let changelog = Changelog::from_installed(installed); - DefaultInstallLogger.on_complete(&changelog, printer)?; + DefaultInstallLogger.on_complete(&changelog, printer, DryRun::Disabled)?; } // Determine the appropriate activation command.