Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve versions appearing in multiple extra indexes #2135

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions crates/uv-client/src/registry_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,31 +183,39 @@ impl RegistryClient {
pub async fn simple(
&self,
package_name: &PackageName,
) -> Result<(IndexUrl, OwnedArchive<SimpleMetadata>), Error> {
) -> Result<Vec<(IndexUrl, OwnedArchive<SimpleMetadata>)>, Error> {
let mut it = self.index_urls.indexes().peekable();
if it.peek().is_none() {
return Err(ErrorKind::NoIndex(package_name.as_ref().to_string()).into());
}

let mut results = vec![];
for index in it {
let result = self.simple_single_index(package_name, index).await?;

return match result {
Ok(metadata) => Ok((index.clone(), metadata)),
match result {
Ok(metadata) => {
results.push((index.clone(), metadata));
}
Err(CachedClientError::Client(err)) => match err.into_kind() {
ErrorKind::Offline(_) => continue,
ErrorKind::RequestError(err) => {
if err.status() == Some(StatusCode::NOT_FOUND)
|| err.status() == Some(StatusCode::FORBIDDEN)
{
continue;
} else {
return Err(ErrorKind::RequestError(err).into());
}
Err(ErrorKind::RequestError(err).into())
}
other => Err(other.into()),
other => {
return Err(other.into());
}
},
Err(CachedClientError::Callback(err)) => Err(err),
};
Err(CachedClientError::Callback(err)) => {
return Err(err);
}
}
}

match self.connectivity {
Expand Down
13 changes: 9 additions & 4 deletions crates/uv-dev/src/resolve_many.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,15 @@ async fn find_latest_version(
client: &RegistryClient,
package_name: &PackageName,
) -> Option<Version> {
let (_, raw_simple_metadata) = client.simple(package_name).await.ok()?;
let simple_metadata = OwnedArchive::deserialize(&raw_simple_metadata);
let version = simple_metadata.into_iter().next()?.version;
Some(version)
let results = client.simple(package_name).await.ok()?;

results
.into_iter()
.filter_map(|(_index, raw_simple_metadata)| {
let simple_metadata = OwnedArchive::deserialize(&raw_simple_metadata);
Some(simple_metadata.into_iter().next()?.version)
})
.max()
}

pub(crate) async fn resolve_many(args: ResolveManyArgs) -> Result<()> {
Expand Down
65 changes: 50 additions & 15 deletions crates/uv-resolver/src/candidate_selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,16 @@ impl CandidateSelector {
&'a self,
package_name: &'a PackageName,
range: &'a Range<Version>,
version_map: &'a VersionMap,
version_maps: &'a [VersionMap],
) -> Option<Candidate<'a>> {
// If the package has a preference (e.g., an existing version from an existing lockfile),
// and the preference satisfies the current range, use that.
if let Some(version) = self.preferences.get(package_name) {
if range.contains(version) {
if let Some(file) = version_map.get(version) {
return Some(Candidate::new(package_name, version, file));
for version_map in version_maps {
if let Some(file) = version_map.get(version) {
return Some(Candidate::new(package_name, version, file));
}
}
}
}
Expand Down Expand Up @@ -126,29 +128,29 @@ impl CandidateSelector {
"selecting candidate for package {:?} with range {:?} with {} versions",
package_name,
range,
version_map.len()
version_maps
.iter()
.map(|version_map| version_map.len())
.sum::<usize>(),
);
match &self.resolution_strategy {
ResolutionStrategy::Highest => Self::select_candidate(
version_map.iter().rev(),
package_name,
range,
allow_prerelease,
),
ResolutionStrategy::Highest => {
Self::select_highest_candidate(version_maps, package_name, range, allow_prerelease)
}
ResolutionStrategy::Lowest => {
Self::select_candidate(version_map.iter(), package_name, range, allow_prerelease)
Self::select_lowest_candidate(version_maps, package_name, range, allow_prerelease)
}
ResolutionStrategy::LowestDirect(direct_dependencies) => {
if direct_dependencies.contains(package_name) {
Self::select_candidate(
version_map.iter(),
Self::select_lowest_candidate(
version_maps,
package_name,
range,
allow_prerelease,
)
} else {
Self::select_candidate(
version_map.iter().rev(),
Self::select_highest_candidate(
version_maps,
package_name,
range,
allow_prerelease,
Expand All @@ -158,6 +160,39 @@ impl CandidateSelector {
}
}

fn select_highest_candidate<'a>(
version_maps: &'a [VersionMap],
package_name: &'a PackageName,
range: &Range<Version>,
allow_prerelease: AllowPreRelease,
) -> Option<Candidate<'a>> {
version_maps
.iter()
.filter_map(|version_map| {
Self::select_candidate(
version_map.iter().rev(),
package_name,
range,
allow_prerelease,
)
})
.max_by_key(|candidate| candidate.version)
}

fn select_lowest_candidate<'a>(
version_maps: &'a [VersionMap],
package_name: &'a PackageName,
range: &Range<Version>,
allow_prerelease: AllowPreRelease,
) -> Option<Candidate<'a>> {
version_maps
.iter()
.filter_map(|version_map| {
Self::select_candidate(version_map.iter(), package_name, range, allow_prerelease)
})
.min_by_key(|candidate| candidate.version)
}

/// Select the first-matching [`Candidate`] from a set of candidate versions and files,
/// preferring wheels over source distributions.
fn select_candidate<'a>(
Expand Down
18 changes: 10 additions & 8 deletions crates/uv-resolver/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,14 +206,16 @@ impl NoSolutionError {
// we represent the state of the resolver at the time of failure.
if visited.contains(name) {
if let Some(response) = package_versions.get(name) {
if let VersionsResponse::Found(ref version_map) = *response {
available_versions.insert(
package.clone(),
version_map
.iter()
.map(|(version, _)| version.clone())
.collect(),
);
if let VersionsResponse::Found(ref version_maps) = *response {
for version_map in version_maps {
available_versions.insert(
package.clone(),
version_map
.iter()
.map(|(version, _)| version.clone())
.collect(),
);
}
}
}
}
Expand Down
26 changes: 20 additions & 6 deletions crates/uv-resolver/src/finder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use anyhow::Result;
use futures::{stream, Stream, StreamExt, TryStreamExt};
use itertools::Itertools;
use rustc_hash::FxHashMap;
use uv_traits::NoBinary;

Expand Down Expand Up @@ -67,11 +68,14 @@ 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 results = self.client.simple(&requirement.name).await?;
let results = results
.iter()
.map(|(index, raw_metadata)| (index, OwnedArchive::deserialize(&raw_metadata)))
.collect::<Vec<_>>();

// Pick a version that satisfies the requirement.
let Some(dist) = self.select(requirement, metadata, &index, flat_index) else {
let Some(dist) = self.select(requirement, results, flat_index) else {
return Err(ResolveError::NotFound(requirement.clone()));
};

Expand Down Expand Up @@ -126,8 +130,7 @@ impl<'a> DistFinder<'a> {
fn select(
&self,
requirement: &Requirement,
metadata: SimpleMetadata,
index: &IndexUrl,
metadata_tuples: Vec<(&IndexUrl, SimpleMetadata)>,
flat_index: Option<&FlatDistributions>,
) -> Option<Dist> {
let no_binary = match self.no_binary {
Expand Down Expand Up @@ -161,7 +164,18 @@ impl<'a> DistFinder<'a> {
(None, None, None)
};

for SimpleMetadatum { version, files } in metadata.into_iter().rev() {
for (index, SimpleMetadatum { version, files }) in metadata_tuples
.into_iter()
.map(|(index, metadata)| {
metadata
.into_iter()
.map(move |metadatum| (index, metadatum))
.rev()
})
.kmerge_by(|(_index1, metadata1), (_index2, metadata2)| {
metadata1.version >= metadata2.version
})
{
// If we iterated past the first-compatible version, break.
if best_version
.as_ref()
Expand Down
28 changes: 16 additions & 12 deletions crates/uv-resolver/src/resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,14 @@ impl ResolutionGraph {

// Add its hashes to the index.
if let Some(versions_response) = packages.get(package_name) {
if let VersionsResponse::Found(ref version_map) = *versions_response {
hashes.insert(package_name.clone(), {
let mut hashes = version_map.hashes(version);
hashes.sort_unstable();
hashes
});
if let VersionsResponse::Found(ref version_maps) = *versions_response {
for version_map in version_maps {
hashes.insert(package_name.clone(), {
let mut hashes = version_map.hashes(version);
hashes.sort_unstable();
hashes
});
}
}
}

Expand All @@ -113,12 +115,14 @@ impl ResolutionGraph {

// Add its hashes to the index.
if let Some(versions_response) = packages.get(package_name) {
if let VersionsResponse::Found(ref version_map) = *versions_response {
hashes.insert(package_name.clone(), {
let mut hashes = version_map.hashes(version);
hashes.sort_unstable();
hashes
});
if let VersionsResponse::Found(ref version_maps) = *versions_response {
for version_map in version_maps {
hashes.insert(package_name.clone(), {
let mut hashes = version_map.hashes(version);
hashes.sort_unstable();
hashes
});
}
}
}

Expand Down
13 changes: 7 additions & 6 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -614,8 +614,8 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
.ok_or(ResolveError::Unregistered)?;
self.visited.insert(package_name.clone());

let version_map = match *versions_response {
VersionsResponse::Found(ref version_map) => version_map,
let version_maps = match *versions_response {
VersionsResponse::Found(ref version_maps) => version_maps,
// Short-circuit if we do not find any versions for the package
VersionsResponse::NoIndex => {
self.unavailable_packages
Expand Down Expand Up @@ -646,7 +646,8 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
}

// Find a version.
let Some(candidate) = self.selector.select(package_name, range, version_map) else {
let Some(candidate) = self.selector.select(package_name, range, version_maps)
else {
// Short circuit: we couldn't find _any_ versions for a package.
return Ok(None);
};
Expand Down Expand Up @@ -1008,8 +1009,8 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
.await
.ok_or(ResolveError::Unregistered)?;

let version_map = match *versions_response {
VersionsResponse::Found(ref version_map) => version_map,
let version_maps = match *versions_response {
VersionsResponse::Found(ref version_maps) => version_maps,
// Short-circuit if we did not find any versions for the package
VersionsResponse::NoIndex => {
self.unavailable_packages
Expand All @@ -1033,7 +1034,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {

// Try to find a compatible version. If there aren't any compatible versions,
// short-circuit and return `None`.
let Some(candidate) = self.selector.select(&package_name, &range, version_map)
let Some(candidate) = self.selector.select(&package_name, &range, version_maps)
else {
return Ok(None);
};
Expand Down
Loading
Loading