From 6d12c1dac11655c65d84f91d103733b814b6f96f Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 12 Oct 2024 15:18:30 +0100 Subject: [PATCH] Use shared resolver state between add and lock --- crates/uv-once-map/src/lib.rs | 12 ++++++++++++ crates/uv/src/commands/project/add.rs | 21 ++++++++++++++++----- crates/uv/src/commands/project/export.rs | 6 +++++- crates/uv/src/commands/project/lock.rs | 20 +++++++------------- crates/uv/src/commands/project/remove.rs | 8 ++++---- crates/uv/src/commands/project/run.rs | 2 +- crates/uv/src/commands/project/sync.rs | 20 +++++++++++++++----- crates/uv/src/commands/project/tree.rs | 6 +++++- 8 files changed, 65 insertions(+), 30 deletions(-) diff --git a/crates/uv-once-map/src/lib.rs b/crates/uv-once-map/src/lib.rs index 2de152619c25..2568de59ecac 100644 --- a/crates/uv-once-map/src/lib.rs +++ b/crates/uv-once-map/src/lib.rs @@ -105,6 +105,18 @@ impl OnceMap { Value::Waiting(_) => None, } } + + /// Remove the result of a previous job, if any. + pub fn remove(&self, key: &Q) -> Option + where + K: Borrow, + { + let entry = self.items.remove(key)?; + match entry { + (_, Value::Filled(value)) => Some(value), + (_, Value::Waiting(_)) => None, + } + } } impl Default for OnceMap { diff --git a/crates/uv/src/commands/project/add.rs b/crates/uv/src/commands/project/add.rs index 4037c6505eae..e69916d5bf1c 100644 --- a/crates/uv/src/commands/project/add.rs +++ b/crates/uv/src/commands/project/add.rs @@ -1,11 +1,13 @@ +use std::collections::hash_map::Entry; +use std::fmt::Write; +use std::path::{Path, PathBuf}; + use anyhow::{bail, Context, Result}; use itertools::Itertools; use owo_colors::OwoColorize; use rustc_hash::{FxBuildHasher, FxHashMap}; -use std::collections::hash_map::Entry; -use std::fmt::Write; -use std::path::{Path, PathBuf}; use tracing::debug; +use url::Url; use uv_auth::{store_credentials_from_url, Credentials}; use uv_cache::Cache; @@ -17,7 +19,7 @@ use uv_configuration::{ }; use uv_dispatch::BuildDispatch; use uv_distribution::DistributionDatabase; -use uv_distribution_types::UnresolvedRequirement; +use uv_distribution_types::{UnresolvedRequirement, VersionId}; use uv_fs::Simplified; use uv_git::{GitReference, GIT_STORE}; use uv_normalize::PackageName; @@ -635,6 +637,7 @@ async fn lock_and_sync( project.workspace(), venv.interpreter(), settings.into(), + &state, Box::new(DefaultResolveLogger), connectivity, concurrency, @@ -726,6 +729,14 @@ async fn lock_and_sync( .with_pyproject_toml(toml::from_str(&content).map_err(ProjectError::TomlParse)?) .ok_or(ProjectError::TomlUpdate)?; + // Invalidate the project metadata. + if let VirtualProject::Project(ref project) = project { + let url = Url::from_file_path(project.project_root()) + .expect("project root is a valid URL"); + let version_id = VersionId::from_url(&url); + debug_assert!(state.index.distributions().remove(&version_id).is_some()); + } + // If the file was modified, we have to lock again, though the only expected change is // the addition of the minimum version specifiers. lock = project::lock::do_safe_lock( @@ -734,6 +745,7 @@ async fn lock_and_sync( project.workspace(), venv.interpreter(), settings.into(), + &state, Box::new(SummaryResolveLogger), connectivity, concurrency, @@ -779,7 +791,6 @@ async fn lock_and_sync( InstallOptions::default(), Modifications::Sufficient, settings.into(), - &state, Box::new(DefaultInstallLogger), connectivity, concurrency, diff --git a/crates/uv/src/commands/project/export.rs b/crates/uv/src/commands/project/export.rs index a4eb8a7d1533..bb7db729bd9a 100644 --- a/crates/uv/src/commands/project/export.rs +++ b/crates/uv/src/commands/project/export.rs @@ -19,7 +19,7 @@ use uv_workspace::{DiscoveryOptions, MemberDiscovery, VirtualProject, Workspace} use crate::commands::pip::loggers::DefaultResolveLogger; use crate::commands::project::lock::do_safe_lock; use crate::commands::project::{ProjectError, ProjectInterpreter}; -use crate::commands::{diagnostics, pip, ExitStatus, OutputWriter}; +use crate::commands::{diagnostics, pip, ExitStatus, OutputWriter, SharedState}; use crate::printer::Printer; use crate::settings::ResolverSettings; @@ -88,6 +88,9 @@ pub(crate) async fn export( .await? .into_interpreter(); + // Initialize any shared state. + let state = SharedState::default(); + // Lock the project. let lock = match do_safe_lock( locked, @@ -95,6 +98,7 @@ pub(crate) async fn export( project.workspace(), &interpreter, settings.as_ref(), + &state, Box::new(DefaultResolveLogger), connectivity, concurrency, diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index 3683efcd90b0..056908b3008f 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -101,6 +101,9 @@ pub(crate) async fn lock( .await? .into_interpreter(); + // Initialize any shared state. + let state = SharedState::default(); + // Perform the lock operation. match do_safe_lock( locked, @@ -108,6 +111,7 @@ pub(crate) async fn lock( &workspace, &interpreter, settings.as_ref(), + &state, Box::new(DefaultResolveLogger), connectivity, concurrency, @@ -153,6 +157,7 @@ pub(super) async fn do_safe_lock( workspace: &Workspace, interpreter: &Interpreter, settings: ResolverSettingsRef<'_>, + state: &SharedState, logger: Box, connectivity: Connectivity, concurrency: Concurrency, @@ -160,17 +165,6 @@ pub(super) async fn do_safe_lock( cache: &Cache, printer: Printer, ) -> Result { - // Use isolate state for universal resolution. When resolving, we don't enforce that the - // prioritized distributions match the current platform. So if we lock here, then try to - // install from the same state, and we end up performing a resolution during the sync (i.e., - // for the build dependencies of a source distribution), we may try to use incompatible - // distributions. - // TODO(charlie): In universal resolution, we should still track version compatibility! We - // just need to accept versions that are platform-incompatible. That would also make us more - // likely to (e.g.) download a wheel that we'll end up using when installing. This would - // make it safe to share the state. - let state = SharedState::default(); - if frozen { // Read the existing lockfile, but don't attempt to lock the project. let existing = read(workspace) @@ -189,7 +183,7 @@ pub(super) async fn do_safe_lock( interpreter, Some(existing), settings, - &state, + state, logger, connectivity, concurrency, @@ -215,7 +209,7 @@ pub(super) async fn do_safe_lock( interpreter, existing, settings, - &state, + state, logger, connectivity, concurrency, diff --git a/crates/uv/src/commands/project/remove.rs b/crates/uv/src/commands/project/remove.rs index 57685973b7cf..1e39f88b8646 100644 --- a/crates/uv/src/commands/project/remove.rs +++ b/crates/uv/src/commands/project/remove.rs @@ -166,6 +166,9 @@ pub(crate) async fn remove( ) .await?; + // Initialize any shared state. + let state = SharedState::default(); + // Lock and sync the environment, if necessary. let lock = project::lock::do_safe_lock( locked, @@ -173,6 +176,7 @@ pub(crate) async fn remove( project.workspace(), venv.interpreter(), settings.as_ref().into(), + &state, Box::new(DefaultResolveLogger), connectivity, concurrency, @@ -193,9 +197,6 @@ pub(crate) async fn remove( let extras = ExtrasSpecification::All; let install_options = InstallOptions::default(); - // Initialize any shared state. - let state = SharedState::default(); - project::sync::do_sync( InstallTarget::from(&project), &venv, @@ -206,7 +207,6 @@ pub(crate) async fn remove( install_options, Modifications::Exact, settings.as_ref().into(), - &state, Box::new(DefaultInstallLogger), connectivity, concurrency, diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index 9acd08b06791..d8bef123e747 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -529,6 +529,7 @@ pub(crate) async fn run( project.workspace(), venv.interpreter(), settings.as_ref().into(), + &state, if show_resolution { Box::new(DefaultResolveLogger) } else { @@ -576,7 +577,6 @@ pub(crate) async fn run( install_options, Modifications::Sufficient, settings.as_ref().into(), - &state, 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 8a768aabe30a..b80f5b0efbbc 100644 --- a/crates/uv/src/commands/project/sync.rs +++ b/crates/uv/src/commands/project/sync.rs @@ -103,12 +103,16 @@ pub(crate) async fn sync( ) .await?; + // Initialize any shared state. + let state = SharedState::default(); + let lock = match do_safe_lock( locked, frozen, target.workspace(), venv.interpreter(), settings.as_ref().into(), + &state, Box::new(DefaultResolveLogger), connectivity, concurrency, @@ -140,9 +144,6 @@ pub(crate) async fn sync( Err(err) => return Err(err.into()), }; - // Initialize any shared state. - let state = SharedState::default(); - // Perform the sync operation. do_sync( target, @@ -154,7 +155,6 @@ pub(crate) async fn sync( install_options, modifications, settings.as_ref().into(), - &state, Box::new(DefaultInstallLogger), connectivity, concurrency, @@ -179,7 +179,6 @@ pub(super) async fn do_sync( install_options: InstallOptions, modifications: Modifications, settings: InstallerSettingsRef<'_>, - state: &SharedState, logger: Box, connectivity: Connectivity, concurrency: Concurrency, @@ -187,6 +186,17 @@ pub(super) async fn do_sync( cache: &Cache, printer: Printer, ) -> Result<(), ProjectError> { + // Use isolated state for universal resolution. When resolving, we don't enforce that the + // prioritized distributions match the current platform. So if we lock here, then try to + // install from the same state, and we end up performing a resolution during the sync (i.e., + // for the build dependencies of a source distribution), we may try to use incompatible + // distributions. + // TODO(charlie): In universal resolution, we should still track version compatibility! We + // just need to accept versions that are platform-incompatible. That would also make us more + // likely to (e.g.) download a wheel that we'll end up using when installing. This would + // make it safe to share the state. + let state = SharedState::default(); + // Extract the project settings. let InstallerSettingsRef { index_locations, diff --git a/crates/uv/src/commands/project/tree.rs b/crates/uv/src/commands/project/tree.rs index d6857ef1a36f..4a53e6f14c75 100644 --- a/crates/uv/src/commands/project/tree.rs +++ b/crates/uv/src/commands/project/tree.rs @@ -13,7 +13,7 @@ use uv_workspace::{DiscoveryOptions, Workspace}; use crate::commands::pip::loggers::DefaultResolveLogger; use crate::commands::pip::resolution_markers; use crate::commands::project::ProjectInterpreter; -use crate::commands::{project, ExitStatus}; +use crate::commands::{project, ExitStatus, SharedState}; use crate::printer::Printer; use crate::settings::ResolverSettings; @@ -59,6 +59,9 @@ pub(crate) async fn tree( .await? .into_interpreter(); + // Initialize any shared state. + let state = SharedState::default(); + // Update the lockfile, if necessary. let lock = project::lock::do_safe_lock( locked, @@ -66,6 +69,7 @@ pub(crate) async fn tree( &workspace, &interpreter, settings.as_ref(), + &state, Box::new(DefaultResolveLogger), connectivity, concurrency,