From dc957d7322356ed2ffaea7575eee1a859f2c9534 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 27 Mar 2024 12:15:14 -0400 Subject: [PATCH] Respect `--no-index` with `--find-links` in `pip sync` (#2692) ## Summary In `pip sync`, we weren't properly handling cases in which a package _only_ existed in `--find-links` (e.g., the user passed `--offline` or `--no-index`). I plan to explore removing `Finder` entirely to avoid these mismatch bugs between `pip sync` and other commands, but this is fine for now. Closes https://github.com/astral-sh/uv/issues/2688. ## Test Plan `cargo test` --- crates/uv-client/src/error.rs | 5 ++ crates/uv-resolver/src/finder.rs | 49 ++++++++++++++++-- crates/uv/tests/pip_sync.rs | 88 ++++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+), 5 deletions(-) diff --git a/crates/uv-client/src/error.rs b/crates/uv-client/src/error.rs index 6d51d08f9..38bc66bad 100644 --- a/crates/uv-client/src/error.rs +++ b/crates/uv-client/src/error.rs @@ -23,6 +23,11 @@ impl Error { *self.kind } + /// Get a reference to the [`ErrorKind`] variant of this error. + pub fn kind(&self) -> &ErrorKind { + &self.kind + } + /// Create a new error from a JSON parsing error. pub(crate) fn from_json_err(err: serde_json::Error, url: Url) -> Self { ErrorKind::BadJson { source: err, url }.into() diff --git a/crates/uv-resolver/src/finder.rs b/crates/uv-resolver/src/finder.rs index 253349cd9..cd1433214 100644 --- a/crates/uv-resolver/src/finder.rs +++ b/crates/uv-resolver/src/finder.rs @@ -70,11 +70,29 @@ impl<'a> DistFinder<'a> { match requirement.version_or_url.as_ref() { None | Some(VersionOrUrl::VersionSpecifier(_)) => { // Query the index(es) (cached) to get the URLs for the available files. - let (index, raw_metadata) = self.client.simple(&requirement.name).await?; - let metadata = OwnedArchive::deserialize(&raw_metadata); + let dist = match self.client.simple(&requirement.name).await { + Ok((index, raw_metadata)) => { + let metadata = OwnedArchive::deserialize(&raw_metadata); - // Pick a version that satisfies the requirement. - let Some(dist) = self.select(requirement, metadata, &index, flat_index) else { + // Pick a version that satisfies the requirement. + self.select_from_index(requirement, metadata, &index, flat_index) + } + Err(err) => match err.kind() { + uv_client::ErrorKind::PackageNotFound(_) + | uv_client::ErrorKind::NoIndex(_) + | uv_client::ErrorKind::Offline(_) => { + if let Some(flat_index) = self.flat_index.get(&requirement.name) { + Self::select_from_flat_index(requirement, flat_index) + } else { + return Err(ResolveError::Client(err)); + } + } + _ => return Err(ResolveError::Client(err)), + }, + }; + + // Verify that a distribution was found. + let Some(dist) = dist else { return Err(ResolveError::NotFound(requirement.clone())); }; @@ -126,7 +144,7 @@ impl<'a> DistFinder<'a> { /// /// Wheels are preferred to source distributions unless `no_binary` excludes wheels /// for the requirement. - fn select( + fn select_from_index( &self, requirement: &Requirement, metadata: SimpleMetadata, @@ -248,6 +266,27 @@ impl<'a> DistFinder<'a> { best_wheel.map_or(best_sdist, |(wheel, ..)| Some(wheel)) } + + /// Select a matching version from a flat index. + fn select_from_flat_index( + requirement: &Requirement, + flat_index: &FlatDistributions, + ) -> Option { + let matching_override = match &requirement.version_or_url { + None => flat_index.iter().next(), + Some(VersionOrUrl::Url(_)) => None, + Some(VersionOrUrl::VersionSpecifier(specifiers)) => flat_index + .iter() + .find(|(version, _)| specifiers.contains(version)), + }; + + let (_, resolvable_dist) = matching_override?; + + resolvable_dist.compatible_wheel().map_or_else( + || resolvable_dist.compatible_source().cloned(), + |(dist, _)| Some(dist.clone()), + ) + } } pub trait Reporter: Send + Sync { diff --git a/crates/uv/tests/pip_sync.rs b/crates/uv/tests/pip_sync.rs index e21c79bb8..aa5e2ab2d 100644 --- a/crates/uv/tests/pip_sync.rs +++ b/crates/uv/tests/pip_sync.rs @@ -2480,6 +2480,94 @@ fn find_links() -> Result<()> { Ok(()) } +/// Sync using `--find-links` with `--no-index`, which should accept the local wheel. +#[test] +fn find_links_no_index_match() -> Result<()> { + let context = TestContext::new("3.12"); + + let requirements_txt = context.temp_dir.child("requirements.txt"); + requirements_txt.write_str(indoc! {r" + tqdm==1000.0.0 + "})?; + + uv_snapshot!(context.filters(), command(&context) + .arg("requirements.txt") + .arg("--no-index") + .arg("--find-links") + .arg(context.workspace_root.join("scripts/wheels/")), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Downloaded 1 package in [TIME] + Installed 1 package in [TIME] + + tqdm==1000.0.0 + "### + ); + + Ok(()) +} + +/// Sync using `--find-links` with `--offline`, which should accept the local wheel. +#[test] +fn find_links_offline_match() -> Result<()> { + let context = TestContext::new("3.12"); + + let requirements_txt = context.temp_dir.child("requirements.txt"); + requirements_txt.write_str(indoc! {r" + tqdm==1000.0.0 + "})?; + + uv_snapshot!(context.filters(), command(&context) + .arg("requirements.txt") + .arg("--offline") + .arg("--find-links") + .arg(context.workspace_root.join("scripts/wheels/")), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Downloaded 1 package in [TIME] + Installed 1 package in [TIME] + + tqdm==1000.0.0 + "### + ); + + Ok(()) +} + +/// Sync using `--find-links` with `--offline`, which should fail to find `numpy`. +#[test] +fn find_links_offline_no_match() -> Result<()> { + let context = TestContext::new("3.12"); + + let requirements_txt = context.temp_dir.child("requirements.txt"); + requirements_txt.write_str(indoc! {r" + numpy + tqdm==1000.0.0 + "})?; + + uv_snapshot!(context.filters(), command(&context) + .arg("requirements.txt") + .arg("--offline") + .arg("--find-links") + .arg(context.workspace_root.join("scripts/wheels/")), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Network connectivity is disabled, but the requested data wasn't found in the cache for: `numpy` + "### + ); + + Ok(()) +} + /// Install without network access via the `--offline` flag. #[test] fn offline() -> Result<()> {