From 874b1fbbc537154015a94740567c7583f17c10f4 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 13 Jul 2026 16:52:50 -0700 Subject: [PATCH] Defer sync client setup until installation is needed --- crates/uv/src/commands/pip/operations.rs | 422 +++++++++++++++-------- crates/uv/src/commands/project/sync.rs | 143 +++++--- crates/uv/tests/sync/sync.rs | 13 + 3 files changed, 400 insertions(+), 178 deletions(-) diff --git a/crates/uv/src/commands/pip/operations.rs b/crates/uv/src/commands/pip/operations.rs index 1617896fb497d..367774bb55cd4 100644 --- a/crates/uv/src/commands/pip/operations.rs +++ b/crates/uv/src/commands/pip/operations.rs @@ -4,6 +4,7 @@ use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::fmt::Write; use std::path::{Path, PathBuf}; use std::sync::Arc; +use std::time::{Duration, Instant}; use anyhow::{Context, anyhow}; use itertools::Itertools; @@ -19,9 +20,10 @@ use uv_configuration::{ use uv_dispatch::BuildDispatch; use uv_distribution::{DistributionDatabase, SourcedDependencyGroups}; use uv_distribution_types::{ - CachedDist, DependencyMetadata, Diagnostic, Dist, InstalledDist, InstalledVersion, LocalDist, - NameRequirementSpecification, Requirement, ResolutionDiagnostic, UnresolvedRequirement, - UnresolvedRequirementSpecification, VersionOrUrlRef, + CachedDist, ConfigSettings, DependencyMetadata, Diagnostic, Dist, ExtraBuildRequires, + ExtraBuildVariables, IndexLocations, InstalledDist, InstalledVersion, LocalDist, + NameRequirementSpecification, PackageConfigSettings, Requirement, ResolutionDiagnostic, + UnresolvedRequirement, UnresolvedRequirementSpecification, VersionOrUrlRef, }; use uv_distribution_types::{DistributionMetadata, InstalledMetadata, Name, Resolution}; use uv_fs::{CWD, Simplified, normalize_path_under}; @@ -563,6 +565,106 @@ pub(crate) enum BytecodeCompilation { Installed, } +/// An installation plan and the time required to create it. +pub(crate) struct InstallationPlan { + plan: Plan, + elapsed: Duration, +} + +impl InstallationPlan { + /// Determine the changes required to make an environment satisfy a resolution. + pub(crate) fn build( + resolution: &Resolution, + site_packages: SitePackages, + installation: InstallationStrategy, + reinstall: &Reinstall, + build_options: &BuildOptions, + hasher: &HashStrategy, + index_locations: &IndexLocations, + config_settings: &ConfigSettings, + config_settings_package: &PackageConfigSettings, + extra_build_requires: &ExtraBuildRequires, + extra_build_variables: &ExtraBuildVariables, + cache: &Cache, + venv: &PythonEnvironment, + tags: &Tags, + ) -> Result { + let start = Instant::now(); + let plan = Planner::new(resolution) + .build( + site_packages, + installation, + reinstall, + build_options, + hasher, + index_locations, + config_settings, + config_settings_package, + extra_build_requires, + extra_build_variables, + cache, + venv, + tags, + ) + .context("Failed to determine installation plan")?; + + Ok(Self { + plan, + elapsed: start.elapsed(), + }) + } + + /// Returns `true` if executing the plan would not modify the environment. + pub(crate) fn is_noop( + &self, + modifications: Modifications, + compile: Option, + dry_run: DryRun, + ) -> bool { + self.plan.cached.is_empty() + && self.plan.remote.is_empty() + && self.plan.reinstalls.is_empty() + && (self.plan.extraneous.is_empty() + || matches!(modifications, Modifications::Sufficient)) + && (compile.is_none() || dry_run.enabled()) + } + + /// Complete an installation that was determined to be a no-op. + pub(crate) fn finish_noop( + self, + resolution: &Resolution, + modifications: Modifications, + compile: Option, + logger: &dyn InstallLogger, + dry_run: DryRun, + printer: Printer, + ) -> Result { + debug_assert!(self.is_noop(modifications, compile, dry_run)); + + let (plan, start) = self.into_parts(); + if dry_run.enabled() { + report_dry_run( + dry_run, + resolution, + plan, + modifications, + start, + logger, + printer, + ) + } else { + logger.on_check(resolution.len(), start, printer, dry_run)?; + Ok(Changelog::default()) + } + } + + fn into_parts(self) -> (Plan, Instant) { + let now = Instant::now(); + let start = now.checked_sub(self.elapsed).unwrap_or(now); + (self.plan, start) + } +} + /// Install a set of requirements into the current environment. /// /// Returns a [`Changelog`] summarizing the changes made to the environment. @@ -589,155 +691,199 @@ pub(crate) async fn install( printer: Printer, preview: Preview, ) -> Result { - let start = std::time::Instant::now(); - - // Partition into those that should be linked from the cache (`local`), those that need to be - // downloaded (`remote`), and those that should be removed (`extraneous`). - let plan = Planner::new(resolution) - .build( - site_packages, - installation, - reinstall, - build_options, - hasher, - build_dispatch.locations(), - build_dispatch.config_settings(), - build_dispatch.config_settings_package(), - build_dispatch.extra_build_requires(), - build_dispatch.extra_build_variables(), - cache, - venv, - tags, - ) - .context("Failed to determine installation plan")?; - - if dry_run.enabled() { - return report_dry_run( - dry_run, - resolution, - plan, - modifications, - start, - logger.as_ref(), - printer, - ); - } + let plan = InstallationPlan::build( + resolution, + site_packages, + installation, + reinstall, + build_options, + hasher, + build_dispatch.locations(), + build_dispatch.config_settings(), + build_dispatch.config_settings_package(), + build_dispatch.extra_build_requires(), + build_dispatch.extra_build_variables(), + cache, + venv, + tags, + )?; + + plan.execute( + resolution, + modifications, + build_options, + link_mode, + compile, + hasher, + tags, + client, + in_flight, + concurrency, + build_dispatch, + cache, + venv, + logger, + installer_metadata, + dry_run, + printer, + preview, + ) + .await +} - let Plan { - cached, - remote, - reinstalls, - extraneous, - } = plan; +impl InstallationPlan { + /// Execute a previously computed installation plan. + pub(crate) async fn execute( + self, + resolution: &Resolution, + modifications: Modifications, + build_options: &BuildOptions, + link_mode: LinkMode, + compile: Option, + hasher: &HashStrategy, + tags: &Tags, + client: &RegistryClient, + in_flight: &InFlight, + concurrency: &Concurrency, + build_dispatch: &BuildDispatch<'_>, + cache: &Cache, + venv: &PythonEnvironment, + logger: Box, + installer_metadata: bool, + dry_run: DryRun, + printer: Printer, + preview: Preview, + ) -> Result { + let (plan, start) = self.into_parts(); + + if dry_run.enabled() { + return report_dry_run( + dry_run, + resolution, + plan, + modifications, + start, + logger.as_ref(), + printer, + ); + } - // If we're in `install` mode, ignore any extraneous distributions. - let extraneous = match modifications { - Modifications::Sufficient => vec![], - Modifications::Exact => extraneous, - }; + let Plan { + cached, + remote, + reinstalls, + extraneous, + } = plan; + + // If we're in `install` mode, ignore any extraneous distributions. + let extraneous = match modifications { + Modifications::Sufficient => vec![], + Modifications::Exact => extraneous, + }; - // Nothing to do. - if remote.is_empty() - && cached.is_empty() - && reinstalls.is_empty() - && extraneous.is_empty() - && compile.is_none() - { - logger.on_check(resolution.len(), start, printer, dry_run)?; - return Ok(Changelog::default()); - } + // Nothing to do. + if remote.is_empty() + && cached.is_empty() + && reinstalls.is_empty() + && extraneous.is_empty() + && compile.is_none() + { + logger.on_check(resolution.len(), start, printer, dry_run)?; + return Ok(Changelog::default()); + } - // Partition into two sets: those that require build isolation, and those that disable it. This - // is effectively a heuristic to make `--no-build-isolation` work "more often" by way of giving - // `--no-build-isolation` packages "access" to the rest of the environment. - let (isolated_phase, shared_phase) = Plan { - cached, - remote, - reinstalls, - extraneous, - } - .partition(|name| build_dispatch.build_isolation().is_isolated(Some(name))); + // Partition into two sets: those that require build isolation, and those that disable it. This + // is effectively a heuristic to make `--no-build-isolation` work "more often" by way of giving + // `--no-build-isolation` packages "access" to the rest of the environment. + let (isolated_phase, shared_phase) = Plan { + cached, + remote, + reinstalls, + extraneous, + } + .partition(|name| build_dispatch.build_isolation().is_isolated(Some(name))); - let has_isolated_phase = !isolated_phase.is_empty(); - let has_shared_phase = !shared_phase.is_empty(); + let has_isolated_phase = !isolated_phase.is_empty(); + let has_shared_phase = !shared_phase.is_empty(); - let mut installs = vec![]; - let mut uninstalls = vec![]; + let mut installs = vec![]; + let mut uninstalls = vec![]; - // Execute the isolated-build phase. - if has_isolated_phase { - let (isolated_installs, isolated_uninstalls) = execute_plan( - isolated_phase, - None, - resolution, - build_options, - link_mode, - hasher, - tags, - client, - in_flight, - concurrency, - build_dispatch, - cache, - venv, - logger.as_ref(), - installer_metadata, - printer, - preview, - ) - .await?; - installs.extend(isolated_installs); - uninstalls.extend(isolated_uninstalls); - } + // Execute the isolated-build phase. + if has_isolated_phase { + let (isolated_installs, isolated_uninstalls) = execute_plan( + isolated_phase, + None, + resolution, + build_options, + link_mode, + hasher, + tags, + client, + in_flight, + concurrency, + build_dispatch, + cache, + venv, + logger.as_ref(), + installer_metadata, + printer, + preview, + ) + .await?; + installs.extend(isolated_installs); + uninstalls.extend(isolated_uninstalls); + } - if has_shared_phase { - let (shared_installs, shared_uninstalls) = execute_plan( - shared_phase, - if has_isolated_phase { - Some(InstallPhase::Shared) - } else { - None - }, - resolution, - build_options, - link_mode, - hasher, - tags, - client, - in_flight, - concurrency, - build_dispatch, - cache, - venv, - logger.as_ref(), - installer_metadata, - printer, - preview, - ) - .await?; - installs.extend(shared_installs); - uninstalls.extend(shared_uninstalls); - } + if has_shared_phase { + let (shared_installs, shared_uninstalls) = execute_plan( + shared_phase, + if has_isolated_phase { + Some(InstallPhase::Shared) + } else { + None + }, + resolution, + build_options, + link_mode, + hasher, + tags, + client, + in_flight, + concurrency, + build_dispatch, + cache, + venv, + logger.as_ref(), + installer_metadata, + printer, + preview, + ) + .await?; + installs.extend(shared_installs); + uninstalls.extend(shared_uninstalls); + } - if let Some(compile) = compile { - match compile { - BytecodeCompilation::All => { - compile_bytecode(venv, concurrency, cache, printer).await?; - } - BytecodeCompilation::Installed => { - let files = python_source_files_for_installs(venv, &installs); - compile_bytecode_files(files, venv, concurrency, cache, printer).await?; + if let Some(compile) = compile { + match compile { + BytecodeCompilation::All => { + compile_bytecode(venv, concurrency, cache, printer).await?; + } + BytecodeCompilation::Installed => { + let files = python_source_files_for_installs(venv, &installs); + compile_bytecode_files(files, venv, concurrency, cache, printer).await?; + } } } - } - // Construct a summary of the changes made to the environment. - let changelog = Changelog::from_local(installs, uninstalls); + // Construct a summary of the changes made to the environment. + let changelog = Changelog::from_local(installs, uninstalls); - // Notify the user of any environment modifications. - logger.on_complete(&changelog, printer, dry_run)?; + // Notify the user of any environment modifications. + logger.on_complete(&changelog, printer, dry_run)?; - Ok(changelog) + Ok(changelog) + } } type PythonSourceFileIterator = Box>>; diff --git a/crates/uv/src/commands/project/sync.rs b/crates/uv/src/commands/project/sync.rs index 08ec095f723c2..8e1ba1bc86c03 100644 --- a/crates/uv/src/commands/project/sync.rs +++ b/crates/uv/src/commands/project/sync.rs @@ -774,9 +774,55 @@ pub(crate) async fn do_sync( // Constrain any build requirements marked as `match-runtime = true`. let extra_build_requires = extra_build_requires.match_runtime(&resolution)?; + // Extract the hashes from the lockfile. + let hasher = HashStrategy::from_resolution(&resolution, HashCheckingMode::Verify)?; + // Populate credentials from the target. store_credentials_from_target(target, &client_builder)?; + let bytecode_compilation = compile_bytecode.then_some(operations::BytecodeCompilation::All); + let site_packages = SitePackages::from_environment(venv)?; + let installation_plan = operations::InstallationPlan::build( + &resolution, + site_packages, + InstallationStrategy::Strict, + reinstall, + build_options, + &hasher, + index_locations, + config_setting, + config_settings_package, + &extra_build_requires, + extra_build_variables, + cache, + venv, + &tags, + )?; + + // Avoid constructing an HTTP client and build dispatch when planning shows that there is no + // installation work to perform. + if installation_plan.is_noop(modifications, bytecode_compilation, dry_run) { + maybe_check_malware( + &target, + &resolution, + &malware_check_client_builder, + concurrency, + cache, + preview, + malware_settings, + ) + .await?; + + return Ok(installation_plan.finish_noop( + &resolution, + modifications, + bytecode_compilation, + logger.as_ref(), + dry_run, + printer, + )?); + } + // Initialize the registry client. let client = RegistryClientBuilder::new(client_builder, cache.clone()) .index_locations(index_locations.clone()) @@ -801,9 +847,6 @@ pub(crate) async fn do_sync( // optional on the downstream APIs. let build_hasher = HashStrategy::default(); - // Extract the hashes from the lockfile. - let hasher = HashStrategy::from_resolution(&resolution, HashCheckingMode::Verify)?; - // Resolve the flat indexes from `--find-links`. let flat_index = { let client = FlatIndexClient::new(client.cached_client(), client.connectivity(), cache); @@ -841,53 +884,73 @@ pub(crate) async fn do_sync( ); // Run a malware check against OSV before installing. - if malware_settings.enabled { - if !preview.is_enabled(PreviewFeature::MalwareCheck) { - warn_user!( - "Malware checks are experimental and may change without warning. Pass `--preview-features {}` to disable this warning.", - PreviewFeature::MalwareCheck - ); - } - check_malware( - &target, + maybe_check_malware( + &target, + &resolution, + &malware_check_client_builder, + concurrency, + cache, + preview, + malware_settings, + ) + .await?; + + // Sync the environment. + let changelog = installation_plan + .execute( &resolution, - &malware_check_client_builder, + modifications, + build_options, + link_mode, + bytecode_compilation, + &hasher, + &tags, + &client, + state.in_flight(), concurrency, - malware_settings.malware_check_url.clone(), + &build_dispatch, cache, + venv, + logger, + installer_metadata, + dry_run, + printer, + preview, ) .await?; - } - let site_packages = SitePackages::from_environment(venv)?; + Ok(changelog) +} - // Sync the environment. - let changelog = operations::install( - &resolution, - site_packages, - InstallationStrategy::Strict, - modifications, - reinstall, - build_options, - link_mode, - compile_bytecode.then_some(operations::BytecodeCompilation::All), - &hasher, - &tags, - &client, - state.in_flight(), +/// Run a malware check against OSV if malware checking is enabled. +async fn maybe_check_malware( + target: &InstallTarget<'_>, + resolution: &Resolution, + client_builder: &BaseClientBuilder<'_>, + concurrency: &Concurrency, + cache: &Cache, + preview: Preview, + malware_settings: &MalwareCheckSettings, +) -> Result<(), ProjectError> { + if !malware_settings.enabled { + return Ok(()); + } + + if !preview.is_enabled(PreviewFeature::MalwareCheck) { + warn_user!( + "Malware checks are experimental and may change without warning. Pass `--preview-features {}` to disable this warning.", + PreviewFeature::MalwareCheck + ); + } + check_malware( + target, + resolution, + client_builder, concurrency, - &build_dispatch, + malware_settings.malware_check_url.clone(), cache, - venv, - logger, - installer_metadata, - dry_run, - printer, - preview, ) - .await?; - - Ok(changelog) + .await } /// Run a malware check against OSV before installing dependencies. diff --git a/crates/uv/tests/sync/sync.rs b/crates/uv/tests/sync/sync.rs index 17f53c2312124..b117d98916f7c 100644 --- a/crates/uv/tests/sync/sync.rs +++ b/crates/uv/tests/sync/sync.rs @@ -339,6 +339,19 @@ fn frozen() -> Result<()> { + sniffio==1.3.1 "); + // A no-op frozen sync should determine the installation plan without constructing a registry + // client, so the `uv_client::base_client` debug target emits no messages. + uv_snapshot!(context.filters(), context.sync() + .arg("--frozen") + .env(EnvVars::RUST_LOG, "uv_client::base_client=debug"), @" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Checked 3 packages in [TIME] + "); + Ok(()) }