From 432e57d07022cf39bddd192313bcc386c95e8ff0 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 24 Feb 2024 20:31:47 -0500 Subject: [PATCH] Re-sync editables on-change (#1959) ## Summary Like #1955, but for `pip sync`. Closes https://github.com/astral-sh/uv/issues/1957. --- crates/uv-installer/src/plan.rs | 12 ++-- crates/uv-resolver/src/resolver/urls.rs | 18 +++--- crates/uv/src/commands/pip_install.rs | 6 +- crates/uv/src/commands/pip_sync.rs | 34 ++++++++-- crates/uv/tests/pip_sync.rs | 83 +++++++++++++++++++++++++ 5 files changed, 130 insertions(+), 23 deletions(-) diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index 222c57ace..77e269b24 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -444,16 +444,16 @@ fn not_modified_cache(cache_entry: &CacheEntry, artifact: &Path) -> Result Result { // Determine the modification time of the installed distribution. - let dist_metadata = fs_err::metadata(&dist.path)?; + let dist_metadata = fs_err::metadata(dist.path.join("METADATA"))?; let dist_timestamp = Timestamp::from_metadata(&dist_metadata); // Determine the modification time of the wheel. - if let Some(artifact_timestamp) = ArchiveTimestamp::from_path(artifact)? { - Ok(dist_timestamp >= artifact_timestamp.timestamp()) - } else { + let Some(artifact_timestamp) = ArchiveTimestamp::from_path(artifact)? else { // The artifact doesn't exist, so it's not fresh. - Ok(false) - } + return Ok(false); + }; + + Ok(dist_timestamp >= artifact_timestamp.timestamp()) } #[derive(Debug, Default)] diff --git a/crates/uv-resolver/src/resolver/urls.rs b/crates/uv-resolver/src/resolver/urls.rs index d206d6fe6..4e33ae51b 100644 --- a/crates/uv-resolver/src/resolver/urls.rs +++ b/crates/uv-resolver/src/resolver/urls.rs @@ -42,29 +42,27 @@ impl Urls { } // Add any editable requirements. If there are any conflicts, return an error. - for (editable_requirement, metadata) in &manifest.editables { - if let Some(previous) = - urls.insert(metadata.name.clone(), editable_requirement.url.clone()) - { + for (requirement, metadata) in &manifest.editables { + if let Some(previous) = urls.insert(metadata.name.clone(), requirement.url.clone()) { if cache_key::CanonicalUrl::new(previous.raw()) - != cache_key::CanonicalUrl::new(editable_requirement.raw()) + != cache_key::CanonicalUrl::new(requirement.raw()) { return Err(ResolveError::ConflictingUrlsDirect( metadata.name.clone(), previous.verbatim().to_string(), - editable_requirement.url.verbatim().to_string(), + requirement.verbatim().to_string(), )); } } - for req in &metadata.requires_dist { - if let Some(pep508_rs::VersionOrUrl::Url(url)) = &req.version_or_url { - if let Some(previous) = urls.insert(req.name.clone(), url.clone()) { + for requirement in &metadata.requires_dist { + if let Some(pep508_rs::VersionOrUrl::Url(url)) = &requirement.version_or_url { + if let Some(previous) = urls.insert(requirement.name.clone(), url.clone()) { if cache_key::CanonicalUrl::new(previous.raw()) != cache_key::CanonicalUrl::new(url.raw()) { return Err(ResolveError::ConflictingUrlsDirect( - req.name.clone(), + requirement.name.clone(), previous.verbatim().to_string(), url.verbatim().to_string(), )); diff --git a/crates/uv/src/commands/pip_install.rs b/crates/uv/src/commands/pip_install.rs index 0745bc7ea..6fd4e3e96 100644 --- a/crates/uv/src/commands/pip_install.rs +++ b/crates/uv/src/commands/pip_install.rs @@ -496,14 +496,16 @@ async fn install( ) -> Result<(), Error> { 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 requirements = resolution.requirements(); + + // Map the built editables to their resolved form. let editables = built_editables .into_iter() .map(ResolvedEditable::Built) .collect::>(); + // 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 { local, remote, diff --git a/crates/uv/src/commands/pip_sync.rs b/crates/uv/src/commands/pip_sync.rs index 6b894cd65..5ca4c17f2 100644 --- a/crates/uv/src/commands/pip_sync.rs +++ b/crates/uv/src/commands/pip_sync.rs @@ -5,13 +5,15 @@ use itertools::Itertools; use owo_colors::OwoColorize; use tracing::debug; -use distribution_types::{IndexLocations, InstalledMetadata, LocalDist, LocalEditable, Name}; +use distribution_types::{ + IndexLocations, InstalledDist, InstalledMetadata, LocalDist, LocalEditable, Name, +}; use install_wheel_rs::linker::LinkMode; use platform_host::Platform; use platform_tags::Tags; use pypi_types::Yanked; use requirements_txt::EditableRequirement; -use uv_cache::Cache; +use uv_cache::{ArchiveTimestamp, Cache}; use uv_client::{Connectivity, FlatIndex, FlatIndexClient, RegistryClient, RegistryClientBuilder}; use uv_dispatch::BuildDispatch; use uv_fs::Normalized; @@ -392,6 +394,18 @@ async fn resolve_editables( build_dispatch: &BuildDispatch<'_>, mut printer: Printer, ) -> Result { + /// Returns `true` if the installed distribution is up-to-date. + fn not_modified(editable: &EditableRequirement, installed: &InstalledDist) -> bool { + let Ok(Some(installed_at)) = ArchiveTimestamp::from_path(installed.path().join("METADATA")) + else { + return false; + }; + let Ok(Some(modified_at)) = ArchiveTimestamp::from_path(&editable.path) else { + return false; + }; + installed_at > modified_at + } + // Partition the editables into those that are already installed, and those that must be built. let mut installed = Vec::with_capacity(editables.len()); let mut uninstalled = Vec::with_capacity(editables.len()); @@ -401,7 +415,13 @@ async fn resolve_editables( let existing = site_packages.get_editables(editable.raw()); match existing.as_slice() { [] => uninstalled.push(editable), - [dist] => installed.push((*dist).clone()), + [dist] => { + if not_modified(&editable, dist) { + installed.push((*dist).clone()); + } else { + uninstalled.push(editable); + } + } _ => { uninstalled.push(editable); } @@ -414,11 +434,15 @@ async fn resolve_editables( let existing = site_packages.get_editables(editable.raw()); match existing.as_slice() { [] => uninstalled.push(editable), - [dist] => { + [dist] if not_modified(&editable, dist) => { if packages.contains(dist.name()) { uninstalled.push(editable); } else { - installed.push((*dist).clone()); + if not_modified(&editable, dist) { + installed.push((*dist).clone()); + } else { + uninstalled.push(editable); + } } } _ => { diff --git a/crates/uv/tests/pip_sync.rs b/crates/uv/tests/pip_sync.rs index 6ff9dcfb0..3b439fed3 100644 --- a/crates/uv/tests/pip_sync.rs +++ b/crates/uv/tests/pip_sync.rs @@ -2799,3 +2799,86 @@ fn pip_entrypoints() -> Result<()> { Ok(()) } + +#[test] +fn invalidate_on_change() -> Result<()> { + let context = TestContext::new("3.12"); + + // Create an editable package. + let editable_dir = assert_fs::TempDir::new()?; + let pyproject_toml = editable_dir.child("pyproject.toml"); + pyproject_toml.write_str( + r#"[project] +name = "example" +version = "0.0.0" +dependencies = [ + "anyio==4.0.0" +] +requires-python = ">=3.8" +"#, + )?; + + // Write to a requirements file. + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str(&format!("-e {}", editable_dir.path().display()))?; + + let filters = [(r"\(from file://.*\)", "(from [WORKSPACE_DIR])")] + .into_iter() + .chain(INSTA_FILTERS.to_vec()) + .collect::>(); + + uv_snapshot!(filters, command(&context) + .arg("requirements.in"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Built 1 editable in [TIME] + Installed 1 package in [TIME] + + example==0.0.0 (from [WORKSPACE_DIR]) + "### + ); + + // Re-installing should be a no-op. + uv_snapshot!(filters, command(&context) + .arg("requirements.in"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Audited 1 package in [TIME] + "### + ); + + // Modify the editable package. + pyproject_toml.write_str( + r#"[project] +name = "example" +version = "0.0.0" +dependencies = [ + "anyio==3.7.1" +] +requires-python = ">=3.8" +"#, + )?; + + // Re-installing should update the package. + uv_snapshot!(filters, command(&context) + .arg("requirements.in"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Built 1 editable in [TIME] + Uninstalled 1 package in [TIME] + Installed 1 package in [TIME] + - example==0.0.0 (from [WORKSPACE_DIR]) + + example==0.0.0 (from [WORKSPACE_DIR]) + "### + ); + + Ok(()) +}