From 6ca71d3545c78ced847842971294fb74400a8248 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 7 Aug 2024 19:20:45 -0400 Subject: [PATCH 1/3] Add TODOs --- crates/uv/src/commands/project/run.rs | 15 ++++++++++----- crates/uv/src/commands/tool/run.rs | 3 +++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index 79089e88c252..a04e356cf0fd 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -89,7 +89,7 @@ pub(crate) async fn run( // Initialize any shared state. let state = SharedState::default(); - let reporter = PythonDownloadReporter::single(printer.filter(show_resolution)); + let reporter = PythonDownloadReporter::single(printer); // Determine whether the command to execute is a PEP 723 script. let temp_dir; @@ -134,6 +134,7 @@ pub(crate) async fn run( if let Some(dependencies) = metadata.dependencies { let requirements = dependencies.into_iter().map(Requirement::from).collect(); let spec = RequirementsSpecification::from_requirements(requirements); + // STOPSHIP(charlie): Here, we resolve and install. let environment = CachedEnvironment::get_or_create( spec, interpreter, @@ -150,7 +151,7 @@ pub(crate) async fn run( Some(environment.into_interpreter()) } else { - // Create a virtual environment + // Create a virtual environment. temp_dir = cache.environment()?; let environment = uv_virtualenv::create_venv( temp_dir.path(), @@ -265,6 +266,12 @@ pub(crate) async fn run( .await? }; + // STOPSHIP(charlie): Here, we resolve and install... It'd be nice to avoid showing any + // output if we resolve from the lockfile alone, and nice to avoid showing any output + // if we determine that the environment is up-to-date (fast paths). + // + // If we _do_ have work to do, we should show a single spinner for the whole operation, + // and then a summary of the changes. Maybe just the `in Xms` messages with the counts. let lock = match project::lock::do_safe_lock( locked, frozen, @@ -449,9 +456,7 @@ pub(crate) async fn run( .into_interpreter() }; - // TODO(charlie): Pass the already-installed versions as preferences, or even as the - // "installed" packages, so that we can skip re-installing them in the ephemeral - // environment. + // STOPSHIP(charlie): We should show a spinner here, and perhaps use `CachedEnvironment`? Some(match spec.filter(|spec| !spec.is_empty()) { None => { diff --git a/crates/uv/src/commands/tool/run.rs b/crates/uv/src/commands/tool/run.rs index c3b14643e37d..00f44ce92c91 100644 --- a/crates/uv/src/commands/tool/run.rs +++ b/crates/uv/src/commands/tool/run.rs @@ -38,6 +38,7 @@ use crate::printer::Printer; use crate::settings::ResolverInstallerSettings; /// The user-facing command used to invoke a tool run. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] pub(crate) enum ToolRunCommand { /// via the `uvx` alias Uvx, @@ -413,6 +414,8 @@ async fn get_or_create_environment( // TODO(zanieb): When implementing project-level tools, discover the project and check if it has the tool. // TODO(zanieb): Determine if we should layer on top of the project environment if it is present. + // STOPSHIP(charlie): Same deal here with respect to spinners. + let environment = CachedEnvironment::get_or_create( spec, interpreter, From ed22fa14fea4ad79f70932e30385781adcd43c64 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 7 Aug 2024 20:15:53 -0400 Subject: [PATCH 2/3] Add logger --- crates/uv/src/commands/pip/install.rs | 2 + crates/uv/src/commands/pip/loggers.rs | 214 ++++++++++++++++++ crates/uv/src/commands/pip/mod.rs | 1 + crates/uv/src/commands/pip/operations.rs | 61 +---- crates/uv/src/commands/pip/sync.rs | 2 + crates/uv/src/commands/project/add.rs | 2 + crates/uv/src/commands/project/environment.rs | 17 +- crates/uv/src/commands/project/mod.rs | 4 + crates/uv/src/commands/project/remove.rs | 2 + crates/uv/src/commands/project/run.rs | 42 +++- crates/uv/src/commands/project/sync.rs | 4 + crates/uv/src/commands/tool/install.rs | 2 + crates/uv/src/commands/tool/run.rs | 12 +- 13 files changed, 297 insertions(+), 68 deletions(-) create mode 100644 crates/uv/src/commands/pip/loggers.rs diff --git a/crates/uv/src/commands/pip/install.rs b/crates/uv/src/commands/pip/install.rs index 681d130863ce..7cedbff6bd9f 100644 --- a/crates/uv/src/commands/pip/install.rs +++ b/crates/uv/src/commands/pip/install.rs @@ -29,6 +29,7 @@ use uv_resolver::{ }; use uv_types::{BuildIsolation, HashStrategy}; +use crate::commands::pip::loggers::DefaultInstallLogger; use crate::commands::pip::operations::Modifications; use crate::commands::pip::{operations, resolution_environment}; use crate::commands::{elapsed, ExitStatus, SharedState}; @@ -383,6 +384,7 @@ pub(crate) async fn pip_install( &build_dispatch, &cache, &environment, + Box::new(DefaultInstallLogger), dry_run, printer, preview, diff --git a/crates/uv/src/commands/pip/loggers.rs b/crates/uv/src/commands/pip/loggers.rs new file mode 100644 index 000000000000..9bc5e6946783 --- /dev/null +++ b/crates/uv/src/commands/pip/loggers.rs @@ -0,0 +1,214 @@ +use std::fmt; +use std::fmt::Write; + +use itertools::Itertools; +use owo_colors::OwoColorize; + +use distribution_types::{CachedDist, InstalledDist, InstalledMetadata, LocalDist, Name}; + +use crate::commands::{elapsed, ChangeEvent, ChangeEventKind}; +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; + + /// Log the completion of the preparation phase. + fn on_prepare(&self, count: usize, start: std::time::Instant, printer: Printer) -> fmt::Result; + + /// Log the completion of the uninstallation phase. + fn on_uninstall( + &self, + count: usize, + start: std::time::Instant, + printer: Printer, + ) -> fmt::Result; + + /// Log the completion of the installation phase. + fn on_install(&self, count: usize, start: std::time::Instant, printer: Printer) -> fmt::Result; + + /// Log the completion of the operation. + fn on_complete( + &self, + installed: Vec, + reinstalled: Vec, + uninstalled: Vec, + printer: Printer, + ) -> fmt::Result; +} + +/// The default logger for install operations. +#[derive(Debug, Default, Clone, Copy)] +pub(crate) struct DefaultInstallLogger; + +impl InstallLogger for DefaultInstallLogger { + fn on_audit(&self, count: usize, start: std::time::Instant, printer: Printer) -> fmt::Result { + let s = if count == 1 { "" } else { "s" }; + writeln!( + printer.stderr(), + "{}", + format!( + "Audited {} {}", + format!("{} package{}", count, s).bold(), + format!("in {}", elapsed(start.elapsed())).dimmed() + ) + .dimmed() + ) + } + + fn on_prepare(&self, count: usize, start: std::time::Instant, printer: Printer) -> fmt::Result { + let s = if count == 1 { "" } else { "s" }; + writeln!( + printer.stderr(), + "{}", + format!( + "Prepared {} {}", + format!("{} package{}", count, s).bold(), + format!("in {}", elapsed(start.elapsed())).dimmed() + ) + .dimmed() + ) + } + + fn on_uninstall( + &self, + count: usize, + start: std::time::Instant, + printer: Printer, + ) -> fmt::Result { + let s = if count == 1 { "" } else { "s" }; + writeln!( + printer.stderr(), + "{}", + format!( + "Uninstalled {} {}", + format!("{} package{}", count, s).bold(), + format!("in {}", elapsed(start.elapsed())).dimmed() + ) + .dimmed() + ) + } + + fn on_install(&self, count: usize, start: std::time::Instant, printer: Printer) -> fmt::Result { + let s = if count == 1 { "" } else { "s" }; + writeln!( + printer.stderr(), + "{}", + format!( + "Installed {} {}", + format!("{} package{}", count, s).bold(), + format!("in {}", elapsed(start.elapsed())).dimmed() + ) + .dimmed() + ) + } + + fn on_complete( + &self, + installed: Vec, + reinstalled: Vec, + uninstalled: Vec, + printer: Printer, + ) -> fmt::Result { + for event in uninstalled + .into_iter() + .chain(reinstalled) + .map(|distribution| ChangeEvent { + dist: LocalDist::from(distribution), + kind: ChangeEventKind::Removed, + }) + .chain(installed.into_iter().map(|distribution| ChangeEvent { + dist: LocalDist::from(distribution), + kind: ChangeEventKind::Added, + })) + .sorted_unstable_by(|a, b| { + a.dist + .name() + .cmp(b.dist.name()) + .then_with(|| a.kind.cmp(&b.kind)) + .then_with(|| a.dist.installed_version().cmp(&b.dist.installed_version())) + }) + { + match event.kind { + ChangeEventKind::Added => { + writeln!( + printer.stderr(), + " {} {}{}", + "+".green(), + event.dist.name().bold(), + event.dist.installed_version().dimmed() + )?; + } + ChangeEventKind::Removed => { + writeln!( + printer.stderr(), + " {} {}{}", + "-".red(), + event.dist.name().bold(), + event.dist.installed_version().dimmed() + )?; + } + } + } + Ok(()) + } +} + +/// A logger that only shows installs and uninstalls, the minimal logging necessary to understand +/// environment changes. +#[derive(Debug, Default, Clone, Copy)] +pub(crate) struct SummaryInstallLogger; + +impl InstallLogger for SummaryInstallLogger { + fn on_audit(&self, count: usize, start: std::time::Instant, printer: Printer) -> fmt::Result { + Ok(()) + } + + fn on_prepare(&self, count: usize, start: std::time::Instant, printer: Printer) -> fmt::Result { + Ok(()) + } + + fn on_uninstall( + &self, + count: usize, + start: std::time::Instant, + printer: Printer, + ) -> fmt::Result { + let s = if count == 1 { "" } else { "s" }; + writeln!( + printer.stderr(), + "{}", + format!( + "Uninstalled {} {}", + format!("{} package{}", count, s).bold(), + format!("in {}", elapsed(start.elapsed())).dimmed() + ) + .dimmed() + ) + } + + fn on_install(&self, count: usize, start: std::time::Instant, printer: Printer) -> fmt::Result { + let s = if count == 1 { "" } else { "s" }; + writeln!( + printer.stderr(), + "{}", + format!( + "Installed {} {}", + format!("{} package{}", count, s).bold(), + format!("in {}", elapsed(start.elapsed())).dimmed() + ) + .dimmed() + ) + } + + fn on_complete( + &self, + installed: Vec, + reinstalled: Vec, + uninstalled: Vec, + printer: Printer, + ) -> fmt::Result { + Ok(()) + } +} diff --git a/crates/uv/src/commands/pip/mod.rs b/crates/uv/src/commands/pip/mod.rs index 01a3609730d0..eba0c15cfb08 100644 --- a/crates/uv/src/commands/pip/mod.rs +++ b/crates/uv/src/commands/pip/mod.rs @@ -10,6 +10,7 @@ pub(crate) mod compile; pub(crate) mod freeze; pub(crate) mod install; pub(crate) mod list; +pub(crate) mod loggers; pub(crate) mod operations; pub(crate) mod show; pub(crate) mod sync; diff --git a/crates/uv/src/commands/pip/operations.rs b/crates/uv/src/commands/pip/operations.rs index 375bf29c1b12..974b2ef9a264 100644 --- a/crates/uv/src/commands/pip/operations.rs +++ b/crates/uv/src/commands/pip/operations.rs @@ -1,11 +1,10 @@ //! Common operations shared across the `pip` API and subcommands. -use std::fmt::{self, Write}; -use std::path::PathBuf; - use anyhow::{anyhow, Context}; use itertools::Itertools; use owo_colors::OwoColorize; +use std::fmt::{self, Write}; +use std::path::PathBuf; use tracing::debug; use distribution_types::{ @@ -40,6 +39,7 @@ use uv_resolver::{ use uv_types::{HashStrategy, InFlight, InstalledPackagesProvider}; use uv_warnings::warn_user; +use crate::commands::pip::loggers::InstallLogger; use crate::commands::reporters::{InstallReporter, PrepareReporter, ResolverReporter}; use crate::commands::{compile_bytecode, elapsed, ChangeEvent, ChangeEventKind, DryRunEvent}; use crate::printer::Printer; @@ -322,6 +322,7 @@ pub(crate) async fn install( build_dispatch: &BuildDispatch<'_>, cache: &Cache, venv: &PythonEnvironment, + logger: Box, dry_run: bool, printer: Printer, preview: PreviewMode, @@ -365,17 +366,7 @@ pub(crate) async fn install( // Nothing to do. if remote.is_empty() && cached.is_empty() && reinstalls.is_empty() && extraneous.is_empty() { - let s = if resolution.len() == 1 { "" } else { "s" }; - writeln!( - printer.stderr(), - "{}", - format!( - "Audited {} {}", - format!("{} package{}", resolution.len(), s).bold(), - format!("in {}", elapsed(start.elapsed())).dimmed() - ) - .dimmed() - )?; + logger.on_audit(resolution.len(), start, printer)?; return Ok(()); } @@ -410,17 +401,7 @@ pub(crate) async fn install( .await .context("Failed to prepare distributions")?; - let s = if wheels.len() == 1 { "" } else { "s" }; - writeln!( - printer.stderr(), - "{}", - format!( - "Prepared {} {}", - format!("{} package{}", wheels.len(), s).bold(), - format!("in {}", elapsed(start.elapsed())).dimmed() - ) - .dimmed() - )?; + logger.on_prepare(wheels.len(), start, printer)?; wheels }; @@ -453,21 +434,7 @@ pub(crate) async fn install( } } - let s = if extraneous.len() + reinstalls.len() == 1 { - "" - } else { - "s" - }; - writeln!( - printer.stderr(), - "{}", - format!( - "Uninstalled {} {}", - format!("{} package{}", extraneous.len() + reinstalls.len(), s).bold(), - format!("in {}", elapsed(start.elapsed())).dimmed() - ) - .dimmed() - )?; + logger.on_uninstall(extraneous.len() + reinstalls.len(), start, printer)?; } // Install the resolved distributions. @@ -483,17 +450,7 @@ pub(crate) async fn install( // task. .install_blocking(wheels)?; - let s = if wheels.len() == 1 { "" } else { "s" }; - writeln!( - printer.stderr(), - "{}", - format!( - "Installed {} {}", - format!("{} package{}", wheels.len(), s).bold(), - format!("in {}", elapsed(start.elapsed())).dimmed() - ) - .dimmed() - )?; + logger.on_install(wheels.len(), start, printer)?; } if compile { @@ -501,7 +458,7 @@ pub(crate) async fn install( } // Notify the user of any environment modifications. - report_modifications(wheels, reinstalls, extraneous, printer)?; + logger.on_complete(wheels, reinstalls, extraneous, printer)?; Ok(()) } diff --git a/crates/uv/src/commands/pip/sync.rs b/crates/uv/src/commands/pip/sync.rs index 852724275aab..b24a155f2410 100644 --- a/crates/uv/src/commands/pip/sync.rs +++ b/crates/uv/src/commands/pip/sync.rs @@ -28,6 +28,7 @@ use uv_resolver::{ }; use uv_types::{BuildIsolation, HashStrategy}; +use crate::commands::pip::loggers::DefaultInstallLogger; use crate::commands::pip::operations::Modifications; use crate::commands::pip::{operations, resolution_environment}; use crate::commands::{ExitStatus, SharedState}; @@ -331,6 +332,7 @@ pub(crate) async fn pip_sync( &build_dispatch, &cache, &environment, + Box::new(DefaultInstallLogger), dry_run, printer, preview, diff --git a/crates/uv/src/commands/project/add.rs b/crates/uv/src/commands/project/add.rs index 0942a3055a66..a21c584607f2 100644 --- a/crates/uv/src/commands/project/add.rs +++ b/crates/uv/src/commands/project/add.rs @@ -24,6 +24,7 @@ use uv_workspace::pyproject::{DependencyType, Source, SourceError}; use uv_workspace::pyproject_mut::{ArrayEdit, PyProjectTomlMut}; use uv_workspace::{DiscoveryOptions, VirtualProject, Workspace}; +use crate::commands::pip::loggers::DefaultInstallLogger; use crate::commands::pip::operations::Modifications; use crate::commands::pip::resolution_environment; use crate::commands::project::ProjectError; @@ -400,6 +401,7 @@ pub(crate) async fn add( Modifications::Sufficient, settings.as_ref().into(), &state, + Box::new(DefaultInstallLogger), preview, connectivity, concurrency, diff --git a/crates/uv/src/commands/project/environment.rs b/crates/uv/src/commands/project/environment.rs index eb8a8b35212a..1622f8b0731e 100644 --- a/crates/uv/src/commands/project/environment.rs +++ b/crates/uv/src/commands/project/environment.rs @@ -1,5 +1,10 @@ use tracing::debug; +use crate::commands::pip::loggers::InstallLogger; +use crate::commands::project::{resolve_environment, sync_environment}; +use crate::commands::SharedState; +use crate::printer::Printer; +use crate::settings::ResolverInstallerSettings; use cache_key::{cache_digest, hash_digest}; use distribution_types::Resolution; use uv_cache::{Cache, CacheBucket}; @@ -8,11 +13,6 @@ use uv_configuration::{Concurrency, PreviewMode}; use uv_python::{Interpreter, PythonEnvironment}; use uv_requirements::RequirementsSpecification; -use crate::commands::project::{resolve_environment, sync_environment}; -use crate::commands::SharedState; -use crate::printer::Printer; -use crate::settings::ResolverInstallerSettings; - /// A [`PythonEnvironment`] stored in the cache. #[derive(Debug)] pub(crate) struct CachedEnvironment(PythonEnvironment); @@ -31,6 +31,7 @@ impl CachedEnvironment { interpreter: Interpreter, settings: &ResolverInstallerSettings, state: &SharedState, + install: Box, preview: PreviewMode, connectivity: Connectivity, concurrency: Concurrency, @@ -55,6 +56,8 @@ impl CachedEnvironment { }; // Resolve the requirements with the interpreter. + // STOPSHIP(charlie): We should suppress all output here, but show progress (so we show + // builds). let graph = resolve_environment( &interpreter, spec, @@ -103,11 +106,15 @@ impl CachedEnvironment { false, true, )?; + + // STOPSHIP(charlie): We should suppress all output here, but show progress (so we show + // downloads and builds), and show a summary at the end. sync_environment( venv, &resolution, settings.as_ref().into(), state, + install, preview, connectivity, concurrency, diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index eeafd8905764..7cc23415dcf4 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -31,6 +31,7 @@ use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy}; use uv_warnings::{warn_user, warn_user_once}; use uv_workspace::Workspace; +use crate::commands::pip::loggers::{DefaultInstallLogger, InstallLogger}; use crate::commands::pip::operations::Modifications; use crate::commands::reporters::{PythonDownloadReporter, ResolverReporter}; use crate::commands::{pip, SharedState}; @@ -639,6 +640,7 @@ pub(crate) async fn sync_environment( resolution: &Resolution, settings: InstallerSettingsRef<'_>, state: &SharedState, + logger: Box, preview: PreviewMode, connectivity: Connectivity, concurrency: Concurrency, @@ -745,6 +747,7 @@ pub(crate) async fn sync_environment( &build_dispatch, cache, &venv, + logger, dry_run, printer, preview, @@ -950,6 +953,7 @@ pub(crate) async fn update_environment( &build_dispatch, cache, &venv, + Box::new(DefaultInstallLogger), dry_run, printer, preview, diff --git a/crates/uv/src/commands/project/remove.rs b/crates/uv/src/commands/project/remove.rs index c6ff75ad0ea4..f3f2bec0e639 100644 --- a/crates/uv/src/commands/project/remove.rs +++ b/crates/uv/src/commands/project/remove.rs @@ -11,6 +11,7 @@ use uv_workspace::pyproject::DependencyType; use uv_workspace::pyproject_mut::PyProjectTomlMut; use uv_workspace::{DiscoveryOptions, ProjectWorkspace, VirtualProject, Workspace}; +use crate::commands::pip::loggers::DefaultInstallLogger; use crate::commands::pip::operations::Modifications; use crate::commands::{project, ExitStatus, SharedState}; use crate::printer::Printer; @@ -143,6 +144,7 @@ pub(crate) async fn remove( Modifications::Exact, settings.as_ref().into(), &state, + Box::new(DefaultInstallLogger), preview, connectivity, concurrency, diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index a04e356cf0fd..3fd02a783fde 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -25,6 +25,7 @@ use uv_requirements::{RequirementsSource, RequirementsSpecification}; use uv_warnings::warn_user_once; use uv_workspace::{DiscoveryOptions, VirtualProject, Workspace, WorkspaceError}; +use crate::commands::pip::loggers::{DefaultInstallLogger, InstallLogger, SummaryInstallLogger}; use crate::commands::pip::operations::Modifications; use crate::commands::project::environment::CachedEnvironment; use crate::commands::project::{ProjectError, WorkspacePython}; @@ -89,7 +90,8 @@ pub(crate) async fn run( // Initialize any shared state. let state = SharedState::default(); - let reporter = PythonDownloadReporter::single(printer); + // Initialize any output reporters. + let download_reporter = PythonDownloadReporter::single(printer); // Determine whether the command to execute is a PEP 723 script. let temp_dir; @@ -125,7 +127,7 @@ pub(crate) async fn run( python_fetch, &client_builder, cache, - Some(&reporter), + Some(&download_reporter), ) .await? .into_interpreter(); @@ -140,12 +142,17 @@ pub(crate) async fn run( interpreter, &settings, &state, + if show_resolution { + Box::new(DefaultInstallLogger) + } else { + Box::new(SummaryInstallLogger) + }, preview, connectivity, concurrency, native_tls, cache, - printer.filter(show_resolution), + printer, ) .await?; @@ -234,7 +241,7 @@ pub(crate) async fn run( python_fetch, &client_builder, cache, - Some(&reporter), + Some(&download_reporter), ) .await? .into_interpreter() @@ -261,7 +268,8 @@ pub(crate) async fn run( connectivity, native_tls, cache, - printer.filter(show_resolution), + // printer.filter(show_resolution), + printer, ) .await? }; @@ -272,6 +280,8 @@ pub(crate) async fn run( // // If we _do_ have work to do, we should show a single spinner for the whole operation, // and then a summary of the changes. Maybe just the `in Xms` messages with the counts. + // + // Ok, let's do the same as with CachedEnvironment? Only show spinners for the lock. let lock = match project::lock::do_safe_lock( locked, frozen, @@ -283,7 +293,8 @@ pub(crate) async fn run( concurrency, native_tls, cache, - printer.filter(show_resolution), + // printer.filter(show_resolution), + printer, ) .await { @@ -298,6 +309,7 @@ pub(crate) async fn run( Err(err) => return Err(err.into()), }; + // STOPSHIP(charlie): Only show the output summary and spinners. project::sync::do_sync( &project, &venv, @@ -307,12 +319,17 @@ pub(crate) async fn run( Modifications::Sufficient, settings.as_ref().into(), &state, + if show_resolution { + Box::new(DefaultInstallLogger) + } else { + Box::new(SummaryInstallLogger) + }, preview, connectivity, concurrency, native_tls, cache, - printer.filter(show_resolution), + printer, ) .await?; @@ -333,7 +350,7 @@ pub(crate) async fn run( python_fetch, &client_builder, cache, - Some(&reporter), + Some(&download_reporter), ) .await?; @@ -450,7 +467,7 @@ pub(crate) async fn run( python_fetch, &client_builder, cache, - Some(&reporter), + Some(&download_reporter), ) .await? .into_interpreter() @@ -479,12 +496,17 @@ pub(crate) async fn run( interpreter, &settings, &state, + if show_resolution { + Box::new(DefaultInstallLogger) + } else { + Box::new(SummaryInstallLogger) + }, preview, connectivity, concurrency, native_tls, cache, - printer.filter(show_resolution), + printer, ) .await? .into() diff --git a/crates/uv/src/commands/project/sync.rs b/crates/uv/src/commands/project/sync.rs index 4722a877cc5c..3aaf717d0081 100644 --- a/crates/uv/src/commands/project/sync.rs +++ b/crates/uv/src/commands/project/sync.rs @@ -16,6 +16,7 @@ use uv_types::{BuildIsolation, HashStrategy}; use uv_warnings::warn_user_once; use uv_workspace::{DiscoveryOptions, VirtualProject, Workspace}; +use crate::commands::pip::loggers::{DefaultInstallLogger, InstallLogger}; use crate::commands::pip::operations::Modifications; use crate::commands::project::lock::do_safe_lock; use crate::commands::project::{ProjectError, SharedState}; @@ -111,6 +112,7 @@ pub(crate) async fn sync( modifications, settings.as_ref().into(), &state, + Box::new(DefaultInstallLogger), preview, connectivity, concurrency, @@ -133,6 +135,7 @@ pub(super) async fn do_sync( modifications: Modifications, settings: InstallerSettingsRef<'_>, state: &SharedState, + logger: Box, preview: PreviewMode, connectivity: Connectivity, concurrency: Concurrency, @@ -260,6 +263,7 @@ pub(super) async fn do_sync( &build_dispatch, cache, venv, + logger, dry_run, printer, preview, diff --git a/crates/uv/src/commands/tool/install.rs b/crates/uv/src/commands/tool/install.rs index fadc5968895a..bc57d3411b33 100644 --- a/crates/uv/src/commands/tool/install.rs +++ b/crates/uv/src/commands/tool/install.rs @@ -28,6 +28,7 @@ use uv_warnings::{warn_user, warn_user_once}; use crate::commands::reporters::PythonDownloadReporter; +use crate::commands::pip::loggers::DefaultInstallLogger; use crate::commands::{ project::{resolve_environment, resolve_names, sync_environment, update_environment}, tool::common::matching_packages, @@ -322,6 +323,7 @@ pub(crate) async fn install( &resolution.into(), settings.as_ref().into(), &state, + Box::new(DefaultInstallLogger), preview, connectivity, concurrency, diff --git a/crates/uv/src/commands/tool/run.rs b/crates/uv/src/commands/tool/run.rs index 00f44ce92c91..7a50c3733b5d 100644 --- a/crates/uv/src/commands/tool/run.rs +++ b/crates/uv/src/commands/tool/run.rs @@ -29,6 +29,7 @@ use uv_warnings::{warn_user, warn_user_once}; use crate::commands::reporters::PythonDownloadReporter; +use crate::commands::pip::loggers::{DefaultInstallLogger, InstallLogger, SummaryInstallLogger}; use crate::commands::project::resolve_names; use crate::commands::{ project::environment::CachedEnvironment, tool::common::matching_packages, tool_list, @@ -98,6 +99,7 @@ pub(crate) async fn run( let (from, environment) = get_or_create_environment( &from, with, + show_resolution, python.as_deref(), &settings, isolated, @@ -108,7 +110,7 @@ pub(crate) async fn run( concurrency, native_tls, cache, - printer.filter(show_resolution), + printer, ) .await?; @@ -283,6 +285,7 @@ fn warn_executable_not_provided_by_package( async fn get_or_create_environment( from: &str, with: &[RequirementsSource], + show_resolution: bool, python: Option<&str>, settings: &ResolverInstallerSettings, isolated: bool, @@ -416,11 +419,18 @@ async fn get_or_create_environment( // STOPSHIP(charlie): Same deal here with respect to spinners. + let install_logger: Box = if show_resolution { + Box::new(DefaultInstallLogger) + } else { + Box::new(SummaryInstallLogger) + }; + let environment = CachedEnvironment::get_or_create( spec, interpreter, settings, &state, + install_logger, preview, connectivity, concurrency, From 4eedb6154f7089478ba84c0fbf05be99fc94a08e Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 7 Aug 2024 20:25:54 -0400 Subject: [PATCH 3/3] Add loggers --- crates/uv/src/commands/pip/compile.rs | 3 +- crates/uv/src/commands/pip/install.rs | 4 +- crates/uv/src/commands/pip/loggers.rs | 79 ++++++++++++++++--- crates/uv/src/commands/pip/operations.rs | 30 +------ crates/uv/src/commands/pip/sync.rs | 4 +- crates/uv/src/commands/project/add.rs | 3 +- crates/uv/src/commands/project/environment.rs | 8 +- crates/uv/src/commands/project/lock.rs | 12 ++- crates/uv/src/commands/project/mod.rs | 11 ++- crates/uv/src/commands/project/remove.rs | 3 +- crates/uv/src/commands/project/run.rs | 33 ++++---- crates/uv/src/commands/project/sync.rs | 3 +- crates/uv/src/commands/project/tree.rs | 2 + crates/uv/src/commands/tool/install.rs | 5 +- crates/uv/src/commands/tool/run.rs | 27 ++++--- crates/uv/src/printer.rs | 10 --- crates/uv/tests/run.rs | 13 +++ crates/uv/tests/tool_run.rs | 19 +++++ 18 files changed, 174 insertions(+), 95 deletions(-) diff --git a/crates/uv/src/commands/pip/compile.rs b/crates/uv/src/commands/pip/compile.rs index b390ca2997c5..95d2ef4d03c1 100644 --- a/crates/uv/src/commands/pip/compile.rs +++ b/crates/uv/src/commands/pip/compile.rs @@ -38,6 +38,7 @@ use uv_resolver::{ use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy, InFlight}; use uv_warnings::warn_user; +use crate::commands::pip::loggers::DefaultResolveLogger; use crate::commands::pip::{operations, resolution_environment}; use crate::commands::ExitStatus; use crate::printer::Printer; @@ -360,9 +361,9 @@ pub(crate) async fn pip_compile( &build_dispatch, concurrency, options, + Box::new(DefaultResolveLogger), printer, preview, - false, ) .await { diff --git a/crates/uv/src/commands/pip/install.rs b/crates/uv/src/commands/pip/install.rs index 7cedbff6bd9f..2be0d05ffcee 100644 --- a/crates/uv/src/commands/pip/install.rs +++ b/crates/uv/src/commands/pip/install.rs @@ -29,7 +29,7 @@ use uv_resolver::{ }; use uv_types::{BuildIsolation, HashStrategy}; -use crate::commands::pip::loggers::DefaultInstallLogger; +use crate::commands::pip::loggers::{DefaultInstallLogger, DefaultResolveLogger}; use crate::commands::pip::operations::Modifications; use crate::commands::pip::{operations, resolution_environment}; use crate::commands::{elapsed, ExitStatus, SharedState}; @@ -351,9 +351,9 @@ pub(crate) async fn pip_install( &build_dispatch, concurrency, options, + Box::new(DefaultResolveLogger), printer, preview, - false, ) .await { diff --git a/crates/uv/src/commands/pip/loggers.rs b/crates/uv/src/commands/pip/loggers.rs index 9bc5e6946783..9c48a5e48b3b 100644 --- a/crates/uv/src/commands/pip/loggers.rs +++ b/crates/uv/src/commands/pip/loggers.rs @@ -50,7 +50,7 @@ impl InstallLogger for DefaultInstallLogger { "{}", format!( "Audited {} {}", - format!("{} package{}", count, s).bold(), + format!("{count} package{s}").bold(), format!("in {}", elapsed(start.elapsed())).dimmed() ) .dimmed() @@ -64,7 +64,7 @@ impl InstallLogger for DefaultInstallLogger { "{}", format!( "Prepared {} {}", - format!("{} package{}", count, s).bold(), + format!("{count} package{s}").bold(), format!("in {}", elapsed(start.elapsed())).dimmed() ) .dimmed() @@ -83,7 +83,7 @@ impl InstallLogger for DefaultInstallLogger { "{}", format!( "Uninstalled {} {}", - format!("{} package{}", count, s).bold(), + format!("{count} package{s}").bold(), format!("in {}", elapsed(start.elapsed())).dimmed() ) .dimmed() @@ -97,7 +97,7 @@ impl InstallLogger for DefaultInstallLogger { "{}", format!( "Installed {} {}", - format!("{} package{}", count, s).bold(), + format!("{count} package{s}").bold(), format!("in {}", elapsed(start.elapsed())).dimmed() ) .dimmed() @@ -161,11 +161,21 @@ impl InstallLogger for DefaultInstallLogger { pub(crate) struct SummaryInstallLogger; impl InstallLogger for SummaryInstallLogger { - 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, + ) -> fmt::Result { Ok(()) } - fn on_prepare(&self, count: usize, start: std::time::Instant, printer: Printer) -> fmt::Result { + fn on_prepare( + &self, + _count: usize, + _start: std::time::Instant, + _printer: Printer, + ) -> fmt::Result { Ok(()) } @@ -181,7 +191,7 @@ impl InstallLogger for SummaryInstallLogger { "{}", format!( "Uninstalled {} {}", - format!("{} package{}", count, s).bold(), + format!("{count} package{s}").bold(), format!("in {}", elapsed(start.elapsed())).dimmed() ) .dimmed() @@ -195,7 +205,7 @@ impl InstallLogger for SummaryInstallLogger { "{}", format!( "Installed {} {}", - format!("{} package{}", count, s).bold(), + format!("{count} package{s}").bold(), format!("in {}", elapsed(start.elapsed())).dimmed() ) .dimmed() @@ -204,10 +214,57 @@ impl InstallLogger for SummaryInstallLogger { fn on_complete( &self, - installed: Vec, - reinstalled: Vec, - uninstalled: Vec, + _installed: Vec, + _reinstalled: Vec, + _uninstalled: Vec, + _printer: Printer, + ) -> fmt::Result { + Ok(()) + } +} + +/// A trait to handle logging during resolve operations. +pub(crate) trait ResolveLogger { + /// Log the completion of the operation. + fn on_complete(&self, count: usize, start: std::time::Instant, printer: Printer) + -> fmt::Result; +} + +/// The default logger for resolve operations. +#[derive(Debug, Default, Clone, Copy)] +pub(crate) struct DefaultResolveLogger; + +impl ResolveLogger for DefaultResolveLogger { + fn on_complete( + &self, + count: usize, + start: std::time::Instant, printer: Printer, + ) -> fmt::Result { + let s = if count == 1 { "" } else { "s" }; + writeln!( + printer.stderr(), + "{}", + format!( + "Resolved {} {}", + format!("{count} package{s}").bold(), + format!("in {}", elapsed(start.elapsed())).dimmed() + ) + .dimmed() + ) + } +} + +/// A logger that doesn't show any output. +#[derive(Debug, Default, Clone, Copy)] +pub(crate) struct SummaryResolveLogger; + +impl ResolveLogger for SummaryResolveLogger { + fn on_complete( + &self, + _count: usize, + _start: std::time::Instant, + _printer: Printer, ) -> fmt::Result { Ok(()) } diff --git a/crates/uv/src/commands/pip/operations.rs b/crates/uv/src/commands/pip/operations.rs index 974b2ef9a264..3438cc52cfce 100644 --- a/crates/uv/src/commands/pip/operations.rs +++ b/crates/uv/src/commands/pip/operations.rs @@ -3,7 +3,7 @@ use anyhow::{anyhow, Context}; use itertools::Itertools; use owo_colors::OwoColorize; -use std::fmt::{self, Write}; +use std::fmt::Write; use std::path::PathBuf; use tracing::debug; @@ -39,7 +39,7 @@ use uv_resolver::{ use uv_types::{HashStrategy, InFlight, InstalledPackagesProvider}; use uv_warnings::warn_user; -use crate::commands::pip::loggers::InstallLogger; +use crate::commands::pip::loggers::{InstallLogger, ResolveLogger}; use crate::commands::reporters::{InstallReporter, PrepareReporter, ResolverReporter}; use crate::commands::{compile_bytecode, elapsed, ChangeEvent, ChangeEventKind, DryRunEvent}; use crate::printer::Printer; @@ -106,9 +106,9 @@ pub(crate) async fn resolve( build_dispatch: &BuildDispatch<'_>, concurrency: Concurrency, options: Options, + logger: Box, printer: Printer, preview: PreviewMode, - quiet: bool, ) -> Result { let start = std::time::Instant::now(); @@ -262,33 +262,11 @@ pub(crate) async fn resolve( resolver.resolve().await? }; - if !quiet { - resolution_success(&resolution, start, printer)?; - } + logger.on_complete(resolution.len(), start, printer)?; Ok(resolution) } -// Prints a success message after completing resolution. -pub(crate) fn resolution_success( - resolution: &ResolutionGraph, - start: std::time::Instant, - printer: Printer, -) -> fmt::Result { - let s = if resolution.len() == 1 { "" } else { "s" }; - - writeln!( - printer.stderr(), - "{}", - format!( - "Resolved {} {}", - format!("{} package{}", resolution.len(), s).bold(), - format!("in {}", elapsed(start.elapsed())).dimmed() - ) - .dimmed() - ) -} - #[derive(Debug, Clone, Copy)] pub(crate) enum Modifications { /// Use `pip install` semantics, whereby existing installations are left as-is, unless they are diff --git a/crates/uv/src/commands/pip/sync.rs b/crates/uv/src/commands/pip/sync.rs index b24a155f2410..bc7c52009e91 100644 --- a/crates/uv/src/commands/pip/sync.rs +++ b/crates/uv/src/commands/pip/sync.rs @@ -28,7 +28,7 @@ use uv_resolver::{ }; use uv_types::{BuildIsolation, HashStrategy}; -use crate::commands::pip::loggers::DefaultInstallLogger; +use crate::commands::pip::loggers::{DefaultInstallLogger, DefaultResolveLogger}; use crate::commands::pip::operations::Modifications; use crate::commands::pip::{operations, resolution_environment}; use crate::commands::{ExitStatus, SharedState}; @@ -299,9 +299,9 @@ pub(crate) async fn pip_sync( &build_dispatch, concurrency, options, + Box::new(DefaultResolveLogger), printer, preview, - false, ) .await { diff --git a/crates/uv/src/commands/project/add.rs b/crates/uv/src/commands/project/add.rs index a21c584607f2..dc7087cb4386 100644 --- a/crates/uv/src/commands/project/add.rs +++ b/crates/uv/src/commands/project/add.rs @@ -24,7 +24,7 @@ use uv_workspace::pyproject::{DependencyType, Source, SourceError}; use uv_workspace::pyproject_mut::{ArrayEdit, PyProjectTomlMut}; use uv_workspace::{DiscoveryOptions, VirtualProject, Workspace}; -use crate::commands::pip::loggers::DefaultInstallLogger; +use crate::commands::pip::loggers::{DefaultInstallLogger, DefaultResolveLogger}; use crate::commands::pip::operations::Modifications; use crate::commands::pip::resolution_environment; use crate::commands::project::ProjectError; @@ -268,6 +268,7 @@ pub(crate) async fn add( project.workspace(), venv.interpreter(), settings.as_ref().into(), + Box::new(DefaultResolveLogger), preview, connectivity, concurrency, diff --git a/crates/uv/src/commands/project/environment.rs b/crates/uv/src/commands/project/environment.rs index 1622f8b0731e..5bb00f28f4b5 100644 --- a/crates/uv/src/commands/project/environment.rs +++ b/crates/uv/src/commands/project/environment.rs @@ -1,6 +1,6 @@ use tracing::debug; -use crate::commands::pip::loggers::InstallLogger; +use crate::commands::pip::loggers::{InstallLogger, ResolveLogger}; use crate::commands::project::{resolve_environment, sync_environment}; use crate::commands::SharedState; use crate::printer::Printer; @@ -31,6 +31,7 @@ impl CachedEnvironment { interpreter: Interpreter, settings: &ResolverInstallerSettings, state: &SharedState, + resolve: Box, install: Box, preview: PreviewMode, connectivity: Connectivity, @@ -56,13 +57,12 @@ impl CachedEnvironment { }; // Resolve the requirements with the interpreter. - // STOPSHIP(charlie): We should suppress all output here, but show progress (so we show - // builds). let graph = resolve_environment( &interpreter, spec, settings.as_ref().into(), state, + resolve, preview, connectivity, concurrency, @@ -107,8 +107,6 @@ impl CachedEnvironment { true, )?; - // STOPSHIP(charlie): We should suppress all output here, but show progress (so we show - // downloads and builds), and show a summary at the end. sync_environment( venv, &resolution, diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index 567168cc7a03..d0d8eb011d5a 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -27,6 +27,7 @@ use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy}; use uv_warnings::{warn_user, warn_user_once}; use uv_workspace::{DiscoveryOptions, Workspace}; +use crate::commands::pip::loggers::{DefaultResolveLogger, ResolveLogger, SummaryResolveLogger}; use crate::commands::project::{find_requires_python, FoundInterpreter, ProjectError, SharedState}; use crate::commands::{pip, ExitStatus}; use crate::printer::Printer; @@ -84,6 +85,7 @@ pub(crate) async fn lock( &workspace, &interpreter, settings.as_ref(), + Box::new(DefaultResolveLogger), preview, connectivity, concurrency, @@ -117,6 +119,7 @@ pub(super) async fn do_safe_lock( workspace: &Workspace, interpreter: &Interpreter, settings: ResolverSettingsRef<'_>, + logger: Box, preview: PreviewMode, connectivity: Connectivity, concurrency: Concurrency, @@ -157,6 +160,7 @@ pub(super) async fn do_safe_lock( Some(&existing), settings, &state, + logger, preview, connectivity, concurrency, @@ -186,6 +190,7 @@ pub(super) async fn do_safe_lock( existing.as_ref(), settings, &state, + logger, preview, connectivity, concurrency, @@ -213,6 +218,7 @@ async fn do_lock( existing_lock: Option<&Lock>, settings: ResolverSettingsRef<'_>, state: &SharedState, + logger: Box, preview: PreviewMode, connectivity: Connectivity, concurrency: Concurrency, @@ -469,9 +475,9 @@ async fn do_lock( &build_dispatch, concurrency, options, + Box::new(SummaryResolveLogger), printer, preview, - true, ) .await .inspect_err(|err| debug!("Resolution with `uv.lock` failed: {err}")) @@ -547,16 +553,16 @@ async fn do_lock( &build_dispatch, concurrency, options, + Box::new(SummaryResolveLogger), printer, preview, - true, ) .await? } }; // Print the success message after completing resolution. - pip::operations::resolution_success(&resolution, start, printer)?; + logger.on_complete(resolution.len(), start, printer)?; // Notify the user of any resolution diagnostics. pip::operations::diagnose_resolution(resolution.diagnostics(), printer)?; diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index 7cc23415dcf4..de70fea703c4 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -31,7 +31,7 @@ use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy}; use uv_warnings::{warn_user, warn_user_once}; use uv_workspace::Workspace; -use crate::commands::pip::loggers::{DefaultInstallLogger, InstallLogger}; +use crate::commands::pip::loggers::{InstallLogger, ResolveLogger}; use crate::commands::pip::operations::Modifications; use crate::commands::reporters::{PythonDownloadReporter, ResolverReporter}; use crate::commands::{pip, SharedState}; @@ -489,6 +489,7 @@ pub(crate) async fn resolve_environment<'a>( spec: RequirementsSpecification, settings: ResolverSettingsRef<'_>, state: &SharedState, + logger: Box, preview: PreviewMode, connectivity: Connectivity, concurrency: Concurrency, @@ -627,9 +628,9 @@ pub(crate) async fn resolve_environment<'a>( &resolve_dispatch, concurrency, options, + logger, printer, preview, - false, ) .await?) } @@ -766,6 +767,8 @@ pub(crate) async fn update_environment( spec: RequirementsSpecification, settings: &ResolverInstallerSettings, state: &SharedState, + resolve: Box, + install: Box, preview: PreviewMode, connectivity: Connectivity, concurrency: Concurrency, @@ -925,9 +928,9 @@ pub(crate) async fn update_environment( &build_dispatch, concurrency, options, + resolve, printer, preview, - false, ) .await { @@ -953,7 +956,7 @@ pub(crate) async fn update_environment( &build_dispatch, cache, &venv, - Box::new(DefaultInstallLogger), + install, dry_run, printer, preview, diff --git a/crates/uv/src/commands/project/remove.rs b/crates/uv/src/commands/project/remove.rs index f3f2bec0e639..8603ec6ff6eb 100644 --- a/crates/uv/src/commands/project/remove.rs +++ b/crates/uv/src/commands/project/remove.rs @@ -11,7 +11,7 @@ use uv_workspace::pyproject::DependencyType; use uv_workspace::pyproject_mut::PyProjectTomlMut; use uv_workspace::{DiscoveryOptions, ProjectWorkspace, VirtualProject, Workspace}; -use crate::commands::pip::loggers::DefaultInstallLogger; +use crate::commands::pip::loggers::{DefaultInstallLogger, DefaultResolveLogger}; use crate::commands::pip::operations::Modifications; use crate::commands::{project, ExitStatus, SharedState}; use crate::printer::Printer; @@ -114,6 +114,7 @@ pub(crate) async fn remove( project.workspace(), venv.interpreter(), settings.as_ref().into(), + Box::new(DefaultResolveLogger), preview, connectivity, concurrency, diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index 3fd02a783fde..ed7fd1bccae3 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -25,7 +25,9 @@ use uv_requirements::{RequirementsSource, RequirementsSpecification}; use uv_warnings::warn_user_once; use uv_workspace::{DiscoveryOptions, VirtualProject, Workspace, WorkspaceError}; -use crate::commands::pip::loggers::{DefaultInstallLogger, InstallLogger, SummaryInstallLogger}; +use crate::commands::pip::loggers::{ + DefaultInstallLogger, DefaultResolveLogger, SummaryInstallLogger, SummaryResolveLogger, +}; use crate::commands::pip::operations::Modifications; use crate::commands::project::environment::CachedEnvironment; use crate::commands::project::{ProjectError, WorkspacePython}; @@ -136,12 +138,16 @@ pub(crate) async fn run( if let Some(dependencies) = metadata.dependencies { let requirements = dependencies.into_iter().map(Requirement::from).collect(); let spec = RequirementsSpecification::from_requirements(requirements); - // STOPSHIP(charlie): Here, we resolve and install. let environment = CachedEnvironment::get_or_create( spec, interpreter, &settings, &state, + if show_resolution { + Box::new(DefaultResolveLogger) + } else { + Box::new(SummaryResolveLogger) + }, if show_resolution { Box::new(DefaultInstallLogger) } else { @@ -268,32 +274,27 @@ pub(crate) async fn run( connectivity, native_tls, cache, - // printer.filter(show_resolution), printer, ) .await? }; - // STOPSHIP(charlie): Here, we resolve and install... It'd be nice to avoid showing any - // output if we resolve from the lockfile alone, and nice to avoid showing any output - // if we determine that the environment is up-to-date (fast paths). - // - // If we _do_ have work to do, we should show a single spinner for the whole operation, - // and then a summary of the changes. Maybe just the `in Xms` messages with the counts. - // - // Ok, let's do the same as with CachedEnvironment? Only show spinners for the lock. let lock = match project::lock::do_safe_lock( locked, frozen, project.workspace(), venv.interpreter(), settings.as_ref().into(), + if show_resolution { + Box::new(DefaultResolveLogger) + } else { + Box::new(SummaryResolveLogger) + }, preview, connectivity, concurrency, native_tls, cache, - // printer.filter(show_resolution), printer, ) .await @@ -309,7 +310,6 @@ pub(crate) async fn run( Err(err) => return Err(err.into()), }; - // STOPSHIP(charlie): Only show the output summary and spinners. project::sync::do_sync( &project, &venv, @@ -473,8 +473,6 @@ pub(crate) async fn run( .into_interpreter() }; - // STOPSHIP(charlie): We should show a spinner here, and perhaps use `CachedEnvironment`? - Some(match spec.filter(|spec| !spec.is_empty()) { None => { // Create a virtual environment @@ -496,6 +494,11 @@ pub(crate) async fn run( interpreter, &settings, &state, + if show_resolution { + Box::new(DefaultResolveLogger) + } else { + Box::new(SummaryResolveLogger) + }, if show_resolution { Box::new(DefaultInstallLogger) } else { diff --git a/crates/uv/src/commands/project/sync.rs b/crates/uv/src/commands/project/sync.rs index 3aaf717d0081..0c35a29f8b8d 100644 --- a/crates/uv/src/commands/project/sync.rs +++ b/crates/uv/src/commands/project/sync.rs @@ -16,7 +16,7 @@ use uv_types::{BuildIsolation, HashStrategy}; use uv_warnings::warn_user_once; use uv_workspace::{DiscoveryOptions, VirtualProject, Workspace}; -use crate::commands::pip::loggers::{DefaultInstallLogger, InstallLogger}; +use crate::commands::pip::loggers::{DefaultInstallLogger, DefaultResolveLogger, InstallLogger}; use crate::commands::pip::operations::Modifications; use crate::commands::project::lock::do_safe_lock; use crate::commands::project::{ProjectError, SharedState}; @@ -79,6 +79,7 @@ pub(crate) async fn sync( project.workspace(), venv.interpreter(), settings.as_ref().into(), + Box::new(DefaultResolveLogger), preview, connectivity, concurrency, diff --git a/crates/uv/src/commands/project/tree.rs b/crates/uv/src/commands/project/tree.rs index 91fbfffc775b..9fcf892b0be1 100644 --- a/crates/uv/src/commands/project/tree.rs +++ b/crates/uv/src/commands/project/tree.rs @@ -13,6 +13,7 @@ use uv_resolver::TreeDisplay; use uv_warnings::warn_user_once; use uv_workspace::{DiscoveryOptions, Workspace}; +use crate::commands::pip::loggers::DefaultResolveLogger; use crate::commands::project::FoundInterpreter; use crate::commands::{project, ExitStatus}; use crate::printer::Printer; @@ -70,6 +71,7 @@ pub(crate) async fn tree( &workspace, &interpreter, settings.as_ref(), + Box::new(DefaultResolveLogger), preview, connectivity, concurrency, diff --git a/crates/uv/src/commands/tool/install.rs b/crates/uv/src/commands/tool/install.rs index bc57d3411b33..117be3707074 100644 --- a/crates/uv/src/commands/tool/install.rs +++ b/crates/uv/src/commands/tool/install.rs @@ -28,7 +28,7 @@ use uv_warnings::{warn_user, warn_user_once}; use crate::commands::reporters::PythonDownloadReporter; -use crate::commands::pip::loggers::DefaultInstallLogger; +use crate::commands::pip::loggers::{DefaultInstallLogger, DefaultResolveLogger}; use crate::commands::{ project::{resolve_environment, resolve_names, sync_environment, update_environment}, tool::common::matching_packages, @@ -276,6 +276,8 @@ pub(crate) async fn install( spec, &settings, &state, + Box::new(DefaultResolveLogger), + Box::new(DefaultInstallLogger), preview, connectivity, concurrency, @@ -300,6 +302,7 @@ pub(crate) async fn install( spec, settings.as_ref().into(), &state, + Box::new(DefaultResolveLogger), preview, connectivity, concurrency, diff --git a/crates/uv/src/commands/tool/run.rs b/crates/uv/src/commands/tool/run.rs index 7a50c3733b5d..a44f8d1b808f 100644 --- a/crates/uv/src/commands/tool/run.rs +++ b/crates/uv/src/commands/tool/run.rs @@ -29,7 +29,9 @@ use uv_warnings::{warn_user, warn_user_once}; use crate::commands::reporters::PythonDownloadReporter; -use crate::commands::pip::loggers::{DefaultInstallLogger, InstallLogger, SummaryInstallLogger}; +use crate::commands::pip::loggers::{ + DefaultInstallLogger, DefaultResolveLogger, SummaryInstallLogger, SummaryResolveLogger, +}; use crate::commands::project::resolve_names; use crate::commands::{ project::environment::CachedEnvironment, tool::common::matching_packages, tool_list, @@ -151,7 +153,7 @@ pub(crate) async fn run( &executable.to_string_lossy(), &from.name, &site_packages, - &invocation_source, + invocation_source, ); let mut handle = match process.spawn() { @@ -236,7 +238,7 @@ fn warn_executable_not_provided_by_package( executable: &str, from_package: &PackageName, site_packages: &SitePackages, - invocation_source: &ToolRunCommand, + invocation_source: ToolRunCommand, ) { let packages = matching_packages(executable, site_packages); if !packages @@ -417,20 +419,21 @@ async fn get_or_create_environment( // TODO(zanieb): When implementing project-level tools, discover the project and check if it has the tool. // TODO(zanieb): Determine if we should layer on top of the project environment if it is present. - // STOPSHIP(charlie): Same deal here with respect to spinners. - - let install_logger: Box = if show_resolution { - Box::new(DefaultInstallLogger) - } else { - Box::new(SummaryInstallLogger) - }; - let environment = CachedEnvironment::get_or_create( spec, interpreter, settings, &state, - install_logger, + if show_resolution { + Box::new(DefaultResolveLogger) + } else { + Box::new(SummaryResolveLogger) + }, + if show_resolution { + Box::new(DefaultInstallLogger) + } else { + Box::new(SummaryInstallLogger) + }, preview, connectivity, concurrency, diff --git a/crates/uv/src/printer.rs b/crates/uv/src/printer.rs index cf559eb0b6ce..3cd038515395 100644 --- a/crates/uv/src/printer.rs +++ b/crates/uv/src/printer.rs @@ -45,16 +45,6 @@ impl Printer { Self::NoProgress => Stderr::Enabled, } } - - /// Filter the [`Printer`], casting to [`Printer::Quiet`] if the condition is false. - #[must_use] - pub(crate) fn filter(self, condition: bool) -> Self { - if condition { - self - } else { - Self::Quiet - } - } } #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/crates/uv/tests/run.rs b/crates/uv/tests/run.rs index e4ce07b226f6..5500bf941e8f 100644 --- a/crates/uv/tests/run.rs +++ b/crates/uv/tests/run.rs @@ -926,6 +926,19 @@ fn run_without_output() -> Result<()> { " })?; + // On the first run, we only show the summary line for each environment. + uv_snapshot!(context.filters(), context.run().env_remove("UV_SHOW_RESOLUTION").arg("--with").arg("iniconfig").arg("main.py"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv run` is experimental and may change without warning + Installed 4 packages in [TIME] + Installed 1 package in [TIME] + "###); + + // Subsequent runs are quiet. uv_snapshot!(context.filters(), context.run().env_remove("UV_SHOW_RESOLUTION").arg("--with").arg("iniconfig").arg("main.py"), @r###" success: true exit_code: 0 diff --git a/crates/uv/tests/tool_run.rs b/crates/uv/tests/tool_run.rs index 14799aa8723f..1f283d6ea9b1 100644 --- a/crates/uv/tests/tool_run.rs +++ b/crates/uv/tests/tool_run.rs @@ -838,6 +838,25 @@ fn tool_run_without_output() { let tool_dir = context.temp_dir.child("tools"); let bin_dir = context.temp_dir.child("bin"); + // On the first run, only show the summary line. + uv_snapshot!(context.filters(), context.tool_run() + .env_remove("UV_SHOW_RESOLUTION") + .arg("--") + .arg("pytest") + .arg("--version") + .env("UV_TOOL_DIR", tool_dir.as_os_str()) + .env("XDG_BIN_HOME", bin_dir.as_os_str()), @r###" + success: true + exit_code: 0 + ----- stdout ----- + pytest 8.1.1 + + ----- stderr ----- + warning: `uv tool run` is experimental and may change without warning + Installed [N] packages in [TIME] + "###); + + // Subsequent runs are quiet. uv_snapshot!(context.filters(), context.tool_run() .env_remove("UV_SHOW_RESOLUTION") .arg("--")