From 8f28319db8e72e7fce78f094fb169f09520926eb Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 4 Apr 2024 12:43:07 -0400 Subject: [PATCH] Deduplicate editables during install commands --- crates/distribution-types/src/editable.rs | 52 +++++++++++++++++++++++ crates/uv-installer/src/downloader.rs | 4 +- crates/uv/src/commands/pip_compile.rs | 13 +++--- crates/uv/src/commands/pip_install.rs | 25 +++++------ crates/uv/src/commands/pip_sync.rs | 36 +++++++--------- crates/uv/tests/pip_compile.rs | 50 ++++++++++++++++++++++ 6 files changed, 136 insertions(+), 44 deletions(-) diff --git a/crates/distribution-types/src/editable.rs b/crates/distribution-types/src/editable.rs index 6b5c397df0bb..5894b1151727 100644 --- a/crates/distribution-types/src/editable.rs +++ b/crates/distribution-types/src/editable.rs @@ -1,4 +1,6 @@ use std::borrow::Cow; +use std::collections::btree_map::Entry; +use std::collections::BTreeMap; use std::path::PathBuf; use url::Url; @@ -41,3 +43,53 @@ impl std::fmt::Display for LocalEditable { std::fmt::Display::fmt(&self.url, f) } } + +/// A collection of [`LocalEditable`]s. +#[derive(Debug, Clone)] +pub struct LocalEditables(Vec); + +impl LocalEditables { + /// Merge and dedupe a list of [`LocalEditable`]s. + /// + /// This function will deduplicate any editables that point to identical paths, merging their + /// extras. + pub fn from_editables(editables: impl Iterator) -> Self { + let mut map = BTreeMap::new(); + for editable in editables { + match map.entry(editable.path.clone()) { + Entry::Vacant(entry) => { + entry.insert(editable); + } + Entry::Occupied(mut entry) => { + let existing = entry.get_mut(); + existing.extras.extend(editable.extras); + } + } + } + Self(map.into_values().collect()) + } + + /// Return the number of editables. + pub fn len(&self) -> usize { + self.0.len() + } + + /// Return whether the editables are empty. + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + /// Return the editables as a vector. + pub fn into_vec(self) -> Vec { + self.0 + } +} + +impl IntoIterator for LocalEditables { + type Item = LocalEditable; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} diff --git a/crates/uv-installer/src/downloader.rs b/crates/uv-installer/src/downloader.rs index 2f8f86091381..e84405be1a23 100644 --- a/crates/uv-installer/src/downloader.rs +++ b/crates/uv-installer/src/downloader.rs @@ -9,7 +9,7 @@ use tracing::instrument; use url::Url; use distribution_types::{ - BuildableSource, CachedDist, Dist, Identifier, LocalEditable, RemoteSource, + BuildableSource, CachedDist, Dist, Identifier, LocalEditable, LocalEditables, RemoteSource, }; use platform_tags::Tags; use uv_cache::Cache; @@ -117,7 +117,7 @@ impl<'a, Context: BuildContext + Send + Sync> Downloader<'a, Context> { #[instrument(skip_all)] pub async fn build_editables( &self, - editables: Vec, + editables: LocalEditables, editable_wheel_dir: &Path, ) -> Result, Error> { // Build editables in parallel diff --git a/crates/uv/src/commands/pip_compile.rs b/crates/uv/src/commands/pip_compile.rs index 31681e883a66..31f6fdd9404e 100644 --- a/crates/uv/src/commands/pip_compile.rs +++ b/crates/uv/src/commands/pip_compile.rs @@ -14,7 +14,7 @@ use owo_colors::OwoColorize; use tempfile::tempdir_in; use tracing::debug; -use distribution_types::{IndexLocations, LocalEditable, Verbatim}; +use distribution_types::{IndexLocations, LocalEditable, LocalEditables, Verbatim}; use platform_tags::Tags; use requirements_txt::EditableRequirement; use uv_auth::{KeyringProvider, GLOBAL_AUTH_STORE}; @@ -300,13 +300,10 @@ pub(crate) async fn pip_compile( } else { let start = std::time::Instant::now(); - let editables: Vec = editables - .into_iter() - .map(|editable| { - let EditableRequirement { url, extras, path } = editable; - Ok(LocalEditable { url, path, extras }) - }) - .collect::>()?; + let editables = LocalEditables::from_editables(editables.into_iter().map(|editable| { + let EditableRequirement { url, extras, path } = editable; + LocalEditable { url, path, extras } + })); let downloader = Downloader::new(&cache, &tags, &client, &build_dispatch) .with_reporter(DownloadReporter::from(printer).with_length(editables.len() as u64)); diff --git a/crates/uv/src/commands/pip_install.rs b/crates/uv/src/commands/pip_install.rs index ba11f3f7ada8..8e7b398e6754 100644 --- a/crates/uv/src/commands/pip_install.rs +++ b/crates/uv/src/commands/pip_install.rs @@ -10,8 +10,8 @@ use tempfile::tempdir_in; use tracing::debug; use distribution_types::{ - DistributionMetadata, IndexLocations, InstalledMetadata, LocalDist, LocalEditable, Name, - Resolution, + DistributionMetadata, IndexLocations, InstalledMetadata, LocalDist, LocalEditable, + LocalEditables, Name, Resolution, }; use install_wheel_rs::linker::LinkMode; use pep508_rs::{MarkerEnvironment, Requirement}; @@ -272,7 +272,7 @@ pub(crate) async fn pip_install( let editables = if editables.is_empty() { vec![] } else { - editable_wheel_dir = tempdir_in(venv.root())?; + editable_wheel_dir = tempdir_in(cache.root())?; build_editables( &editables, editable_wheel_dir.path(), @@ -447,17 +447,14 @@ async fn build_editables( let downloader = Downloader::new(cache, tags, client, build_dispatch) .with_reporter(DownloadReporter::from(printer).with_length(editables.len() as u64)); - let editables: Vec = editables - .iter() - .map(|editable| { - let EditableRequirement { url, extras, path } = editable; - Ok(LocalEditable { - url: url.clone(), - extras: extras.clone(), - path: path.clone(), - }) - }) - .collect::>()?; + let editables = LocalEditables::from_editables(editables.iter().map(|editable| { + let EditableRequirement { url, extras, path } = editable; + LocalEditable { + url: url.clone(), + extras: extras.clone(), + path: path.clone(), + } + })); let editables: Vec<_> = downloader .build_editables(editables, editable_wheel_dir) diff --git a/crates/uv/src/commands/pip_sync.rs b/crates/uv/src/commands/pip_sync.rs index c5f11d73f046..c437de4bca79 100644 --- a/crates/uv/src/commands/pip_sync.rs +++ b/crates/uv/src/commands/pip_sync.rs @@ -6,7 +6,7 @@ use owo_colors::OwoColorize; use tracing::debug; use distribution_types::{ - IndexLocations, InstalledMetadata, LocalDist, LocalEditable, Name, ResolvedDist, + IndexLocations, InstalledMetadata, LocalDist, LocalEditable, LocalEditables, Name, ResolvedDist, }; use install_wheel_rs::linker::LinkMode; use platform_tags::Tags; @@ -599,32 +599,28 @@ async fn resolve_editables( } else { let start = std::time::Instant::now(); - let temp_dir = tempfile::tempdir_in(cache.root())?; - let downloader = Downloader::new(cache, tags, client, build_dispatch) .with_reporter(DownloadReporter::from(printer).with_length(uninstalled.len() as u64)); - let local_editables: Vec = uninstalled - .iter() - .map(|editable| { - let EditableRequirement { url, path, extras } = editable; - Ok(LocalEditable { - url: url.clone(), - path: path.clone(), - extras: extras.clone(), - }) - }) - .collect::>()?; + let editables = LocalEditables::from_editables(uninstalled.iter().map(|editable| { + let EditableRequirement { url, path, extras } = editable; + LocalEditable { + url: url.clone(), + path: path.clone(), + extras: extras.clone(), + } + })); - let built_editables: Vec<_> = downloader - .build_editables(local_editables, temp_dir.path()) + let editable_wheel_dir = tempfile::tempdir_in(cache.root())?; + let editables: Vec<_> = downloader + .build_editables(editables, editable_wheel_dir.path()) .await .context("Failed to build editables")? .into_iter() .collect(); // Validate that the editables are compatible with the target Python version. - for editable in &built_editables { + for editable in &editables { if let Some(python_requires) = editable.metadata.requires_python.as_ref() { if !python_requires.contains(interpreter.python_version()) { return Err(anyhow!( @@ -637,19 +633,19 @@ async fn resolve_editables( } } - let s = if built_editables.len() == 1 { "" } else { "s" }; + let s = if editables.len() == 1 { "" } else { "s" }; writeln!( printer.stderr(), "{}", format!( "Built {} in {}", - format!("{} editable{}", built_editables.len(), s).bold(), + format!("{} editable{}", editables.len(), s).bold(), elapsed(start.elapsed()) ) .dimmed() )?; - (built_editables, Some(temp_dir)) + (editables, Some(editable_wheel_dir)) }; Ok(ResolvedEditables { diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs index a1259b62ae59..c4d199483ccb 100644 --- a/crates/uv/tests/pip_compile.rs +++ b/crates/uv/tests/pip_compile.rs @@ -3096,6 +3096,56 @@ fn compile_editable() -> Result<()> { Ok(()) } +/// If an editable is repeated, it should only be built once. +#[test] +fn deduplicate_editable() -> Result<()> { + let context = TestContext::new("3.12"); + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str(indoc! {r" + -e file://../../scripts/packages/black_editable + -e ${PROJECT_ROOT}/../../scripts/packages/black_editable + -e file://../../scripts/packages/black_editable[dev] + " + })?; + + uv_snapshot!(context.filters(), context.compile() + .arg(requirements_in.path()) + .current_dir(current_dir()?), @r###" + success: true + exit_code: 0 + ----- stdout ----- + # This file was autogenerated by uv via the following command: + # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z [TEMP_DIR]/requirements.in + -e file://../../scripts/packages/black_editable + aiohttp==3.9.3 + # via black + aiosignal==1.3.1 + # via aiohttp + attrs==23.2.0 + # via aiohttp + frozenlist==1.4.1 + # via + # aiohttp + # aiosignal + idna==3.6 + # via yarl + multidict==6.0.5 + # via + # aiohttp + # yarl + uvloop==0.19.0 + # via black + yarl==1.9.4 + # via aiohttp + + ----- stderr ----- + Built 1 editable in [TIME] + Resolved 9 packages in [TIME] + "###); + + Ok(()) +} + #[test] fn recursive_extras_direct_url() -> Result<()> { let context = TestContext::new("3.12");