Use canonical URL to key redirect map (#2764)

## Summary

This fixes a potential bug that revealed itself in
https://github.com/astral-sh/uv/pull/2761. We don't run into this now
because we always use "allowed URLs", stores the "last" compatible URL
in the map. But the use of the "raw" URL (rather than the "canonical"
URL) means that other writers have to follow that same assumption and
iterate over dependencies in-order.
This commit is contained in:
Charlie Marsh 2024-04-01 17:57:13 -04:00 committed by GitHub
parent d2bbc07b76
commit 999d653404
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 16 additions and 7 deletions

View file

@ -2,6 +2,7 @@ use std::borrow::Cow;
use std::hash::BuildHasherDefault;
use anyhow::Result;
use cache_key::CanonicalUrl;
use dashmap::DashMap;
use itertools::Itertools;
use owo_colors::OwoColorize;
@ -68,7 +69,7 @@ impl ResolutionGraph {
pins: &FilePins,
packages: &OnceMap<PackageName, VersionsResponse>,
distributions: &OnceMap<PackageId, Metadata23>,
redirects: &DashMap<Url, Url>,
redirects: &DashMap<CanonicalUrl, Url>,
state: &State<UvDependencyProvider>,
preferences: &Preferences,
editables: Editables,
@ -119,7 +120,7 @@ impl ResolutionGraph {
let pinned_package = if let Some((editable, _)) = editables.get(package_name) {
Dist::from_editable(package_name.clone(), editable.clone())?
} else {
let url = redirects.get(url).map_or_else(
let url = redirects.get(&CanonicalUrl::new(url)).map_or_else(
|| url.clone(),
|precise| apply_redirect(url, precise.value()),
);
@ -224,7 +225,7 @@ impl ResolutionGraph {
.or_insert_with(Vec::new)
.push(extra.clone());
} else {
let url = redirects.get(url).map_or_else(
let url = redirects.get(&CanonicalUrl::new(url)).map_or_else(
|| url.clone(),
|precise| apply_redirect(url, precise.value()),
);

View file

@ -1,3 +1,4 @@
use cache_key::CanonicalUrl;
use dashmap::DashMap;
use url::Url;
@ -21,5 +22,5 @@ pub struct InMemoryIndex {
/// A map from source URL to precise URL. For example, the source URL
/// `git+https://github.com/pallets/flask.git` could be redirected to
/// `git+https://github.com/pallets/flask.git@c2f65dd1cfff0672b902fd5b30815f0b4137214c`.
pub(crate) redirects: DashMap<Url, Url>,
pub(crate) redirects: DashMap<CanonicalUrl, Url>,
}

View file

@ -6,6 +6,7 @@ use std::ops::Deref;
use std::sync::Arc;
use anyhow::Result;
use cache_key::CanonicalUrl;
use dashmap::{DashMap, DashSet};
use futures::{FutureExt, StreamExt};
use itertools::Itertools;
@ -949,13 +950,19 @@ impl<
if let Some(precise) = precise {
match distribution {
SourceDist::DirectUrl(sdist) => {
self.index.redirects.insert(sdist.url.to_url(), precise);
self.index
.redirects
.insert(CanonicalUrl::new(&sdist.url), precise);
}
SourceDist::Git(sdist) => {
self.index.redirects.insert(sdist.url.to_url(), precise);
self.index
.redirects
.insert(CanonicalUrl::new(&sdist.url), precise);
}
SourceDist::Path(sdist) => {
self.index.redirects.insert(sdist.url.to_url(), precise);
self.index
.redirects
.insert(CanonicalUrl::new(&sdist.url), precise);
}
SourceDist::Registry(_) => {}
}