Skip to content

Commit

Permalink
Deduplicate editables during install commands (#2820)
Browse files Browse the repository at this point in the history
## Summary

Closes #2819.
  • Loading branch information
charliermarsh authored Apr 4, 2024
1 parent 661787b commit ab8368a
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 44 deletions.
52 changes: 52 additions & 0 deletions crates/distribution-types/src/editable.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<LocalEditable>);

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<Item = LocalEditable>) -> 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<LocalEditable> {
self.0
}
}

impl IntoIterator for LocalEditables {
type Item = LocalEditable;
type IntoIter = std::vec::IntoIter<LocalEditable>;

fn into_iter(self) -> Self::IntoIter {
self.0.into_iter()
}
}
4 changes: 2 additions & 2 deletions crates/uv-installer/src/downloader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -117,7 +117,7 @@ impl<'a, Context: BuildContext + Send + Sync> Downloader<'a, Context> {
#[instrument(skip_all)]
pub async fn build_editables(
&self,
editables: Vec<LocalEditable>,
editables: LocalEditables,
editable_wheel_dir: &Path,
) -> Result<Vec<BuiltEditable>, Error> {
// Build editables in parallel
Expand Down
13 changes: 5 additions & 8 deletions crates/uv/src/commands/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -300,13 +300,10 @@ pub(crate) async fn pip_compile(
} else {
let start = std::time::Instant::now();

let editables: Vec<LocalEditable> = editables
.into_iter()
.map(|editable| {
let EditableRequirement { url, extras, path } = editable;
Ok(LocalEditable { url, path, extras })
})
.collect::<Result<_>>()?;
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));
Expand Down
25 changes: 11 additions & 14 deletions crates/uv/src/commands/pip_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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<LocalEditable> = editables
.iter()
.map(|editable| {
let EditableRequirement { url, extras, path } = editable;
Ok(LocalEditable {
url: url.clone(),
extras: extras.clone(),
path: path.clone(),
})
})
.collect::<Result<_>>()?;
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)
Expand Down
36 changes: 16 additions & 20 deletions crates/uv/src/commands/pip_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<LocalEditable> = uninstalled
.iter()
.map(|editable| {
let EditableRequirement { url, path, extras } = editable;
Ok(LocalEditable {
url: url.clone(),
path: path.clone(),
extras: extras.clone(),
})
})
.collect::<Result<_>>()?;
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!(
Expand All @@ -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 {
Expand Down
50 changes: 50 additions & 0 deletions crates/uv/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down

0 comments on commit ab8368a

Please sign in to comment.