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`
This commit is contained in:
Charlie Marsh 2024-03-27 12:15:14 -04:00 committed by GitHub
parent 384355bb57
commit dc957d7322
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 137 additions and 5 deletions

View file

@ -23,6 +23,11 @@ impl Error {
*self.kind *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. /// Create a new error from a JSON parsing error.
pub(crate) fn from_json_err(err: serde_json::Error, url: Url) -> Self { pub(crate) fn from_json_err(err: serde_json::Error, url: Url) -> Self {
ErrorKind::BadJson { source: err, url }.into() ErrorKind::BadJson { source: err, url }.into()

View file

@ -70,11 +70,29 @@ impl<'a> DistFinder<'a> {
match requirement.version_or_url.as_ref() { match requirement.version_or_url.as_ref() {
None | Some(VersionOrUrl::VersionSpecifier(_)) => { None | Some(VersionOrUrl::VersionSpecifier(_)) => {
// Query the index(es) (cached) to get the URLs for the available files. // Query the index(es) (cached) to get the URLs for the available files.
let (index, raw_metadata) = self.client.simple(&requirement.name).await?; let dist = match self.client.simple(&requirement.name).await {
let metadata = OwnedArchive::deserialize(&raw_metadata); Ok((index, raw_metadata)) => {
let metadata = OwnedArchive::deserialize(&raw_metadata);
// Pick a version that satisfies the requirement. // Pick a version that satisfies the requirement.
let Some(dist) = self.select(requirement, metadata, &index, flat_index) else { 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())); 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 /// Wheels are preferred to source distributions unless `no_binary` excludes wheels
/// for the requirement. /// for the requirement.
fn select( fn select_from_index(
&self, &self,
requirement: &Requirement, requirement: &Requirement,
metadata: SimpleMetadata, metadata: SimpleMetadata,
@ -248,6 +266,27 @@ impl<'a> DistFinder<'a> {
best_wheel.map_or(best_sdist, |(wheel, ..)| Some(wheel)) 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<Dist> {
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 { pub trait Reporter: Send + Sync {

View file

@ -2480,6 +2480,94 @@ fn find_links() -> Result<()> {
Ok(()) 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. /// Install without network access via the `--offline` flag.
#[test] #[test]
fn offline() -> Result<()> { fn offline() -> Result<()> {