Add support for relative URLs in simple metadata responses (#721)

## Summary

This PR adds support for relative URLs in the simple JSON responses. We
already support relative URLs for HTML responses, but the handling has
been consolidated between the two. Similar to index URLs, we now store
the base alongside the metadata, and use the base when resolving the
URL.

Closes #455.

## Test Plan

`cargo test` (to test HTML indexes). Separately, I also ran `cargo run
-p puffin-cli -- pip-compile requirements.in -n
--index-url=http://localhost:3141/packages/pypi/+simple` on the
`zb/relative` branch with `packse` running, and forced both HTML and
JSON by limiting the `accept` header.
This commit is contained in:
Charlie Marsh 2023-12-27 09:53:21 -04:00 committed by GitHub
parent ae83a74309
commit 007f52bb4e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 285 additions and 134 deletions

View file

@ -4,6 +4,7 @@ use rustc_hash::FxHashMap;
use distribution_types::{Dist, DistributionMetadata, IndexUrl, Name};
use pep508_rs::{Requirement, VersionOrUrl};
use puffin_normalize::PackageName;
use pypi_types::BaseUrl;
use crate::file::DistFile;
use crate::prerelease_mode::PreReleaseStrategy;
@ -145,7 +146,7 @@ impl CandidateSelector {
}
/// Select the first-matching [`Candidate`] from a set of candidate versions and files,
/// preferring wheels over sdists.
/// preferring wheels over source distributions.
fn select_candidate<'a>(
versions: impl Iterator<Item = (&'a PubGrubVersion, ResolvableFile<'a>)>,
package_name: &'a PackageName,
@ -242,12 +243,13 @@ impl<'a> Candidate<'a> {
}
/// Return the [`Dist`] to use when resolving the candidate.
pub(crate) fn into_distribution(self, index: IndexUrl) -> Dist {
pub(crate) fn into_distribution(self, index: IndexUrl, base: BaseUrl) -> Dist {
Dist::from_registry(
self.name().clone(),
self.version().clone().into(),
self.resolve().clone().into(),
index,
base,
)
}
}

View file

@ -12,6 +12,7 @@ use pep508_rs::Requirement;
use puffin_distribution::DistributionDatabaseError;
use puffin_normalize::PackageName;
use puffin_traits::OnceMap;
use pypi_types::BaseUrl;
use crate::pubgrub::{PubGrubPackage, PubGrubReportFormatter, PubGrubVersion};
use crate::version_map::VersionMap;
@ -144,12 +145,12 @@ impl NoSolutionError {
/// Only packages used in the error's derivation tree will be retrieved.
pub(crate) fn update_available_versions(
mut self,
package_versions: &OnceMap<PackageName, (IndexUrl, VersionMap)>,
package_versions: &OnceMap<PackageName, (IndexUrl, BaseUrl, VersionMap)>,
) -> Self {
for package in self.derivation_tree.packages() {
if let PubGrubPackage::Package(name, ..) = package {
if let Some(entry) = package_versions.get(name) {
let (_, version_map) = entry.value();
let (_, _, version_map) = entry.value();
self.available_versions.insert(
package.clone(),
version_map

View file

@ -6,7 +6,7 @@ use anyhow::Result;
use futures::{stream, Stream, StreamExt, TryStreamExt};
use rustc_hash::FxHashMap;
use distribution_types::{Dist, IndexUrl, Resolution};
use distribution_types::{Dist, File, Resolution};
use pep440_rs::Version;
use pep508_rs::{Requirement, VersionOrUrl};
use platform_tags::{TagPriority, Tags};
@ -53,12 +53,18 @@ 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, metadata) = self.client.simple(&requirement.name).await?;
let (index, base, metadata) = self.client.simple(&requirement.name).await?;
// Pick a version that satisfies the requirement.
let Some(distribution) = self.select(requirement, &index, metadata) else {
let Some(ParsedFile {
name,
version,
file,
}) = self.select(requirement, metadata)
else {
return Err(ResolveError::NotFound(requirement.clone()));
};
let distribution = Dist::from_registry(name, version, file, index, base);
if let Some(reporter) = self.reporter.as_ref() {
reporter.on_progress(&distribution);
@ -103,15 +109,10 @@ impl<'a> DistFinder<'a> {
}
/// select a version that satisfies the requirement, preferring wheels to source distributions.
fn select(
&self,
requirement: &Requirement,
index: &IndexUrl,
metadata: SimpleMetadata,
) -> Option<Dist> {
fn select(&self, requirement: &Requirement, metadata: SimpleMetadata) -> Option<ParsedFile> {
let mut best_version: Option<Version> = None;
let mut best_wheel: Option<(Dist, TagPriority)> = None;
let mut best_sdist: Option<Dist> = None;
let mut best_wheel: Option<(ParsedFile, TagPriority)> = None;
let mut best_sdist: Option<ParsedFile> = None;
for (version, files) in metadata.into_iter().rev() {
// If we iterated past the first-compatible version, break.
@ -151,7 +152,11 @@ impl<'a> DistFinder<'a> {
.map_or(true, |(.., existing)| priority > *existing)
{
best_wheel = Some((
Dist::from_registry(wheel.name, wheel.version, file, index.clone()),
ParsedFile {
name: wheel.name,
version: wheel.version,
file,
},
priority,
));
}
@ -177,12 +182,11 @@ impl<'a> DistFinder<'a> {
}
best_version = Some(sdist.version.clone());
best_sdist = Some(Dist::from_registry(
sdist.name,
sdist.version,
best_sdist = Some(ParsedFile {
name: sdist.name,
version: sdist.version,
file,
index.clone(),
));
});
}
}
}
@ -191,6 +195,16 @@ impl<'a> DistFinder<'a> {
}
}
#[derive(Debug)]
struct ParsedFile {
/// The [`PackageName`] extracted from the [`File`].
name: PackageName,
/// The version extracted from the [`File`].
version: Version,
/// The underlying [`File`].
file: File,
}
pub trait Reporter: Send + Sync {
/// Callback to invoke when a package is resolved to a specific distribution.
fn on_progress(&self, dist: &Dist);

View file

@ -2,6 +2,7 @@ use rustc_hash::FxHashMap;
use distribution_types::{File, IndexUrl};
use puffin_normalize::PackageName;
use pypi_types::BaseUrl;
use crate::candidate_selector::Candidate;
@ -10,14 +11,20 @@ use crate::candidate_selector::Candidate;
/// For example, given `Flask==3.0.0`, the [`FilePins`] would contain a mapping from `Flask` to
/// `3.0.0` to the specific wheel or source distribution archive that was pinned for that version.
#[derive(Debug, Default)]
pub(crate) struct FilePins(FxHashMap<PackageName, FxHashMap<pep440_rs::Version, (IndexUrl, File)>>);
pub(crate) struct FilePins(
FxHashMap<PackageName, FxHashMap<pep440_rs::Version, (IndexUrl, BaseUrl, File)>>,
);
impl FilePins {
/// Pin a candidate package.
pub(crate) fn insert(&mut self, candidate: &Candidate, index: &IndexUrl) {
pub(crate) fn insert(&mut self, candidate: &Candidate, index: &IndexUrl, base: &BaseUrl) {
self.0.entry(candidate.name().clone()).or_default().insert(
candidate.version().clone().into(),
(index.clone(), candidate.install().clone().into()),
(
index.clone(),
base.clone(),
candidate.install().clone().into(),
),
);
}
@ -26,7 +33,7 @@ impl FilePins {
&self,
name: &PackageName,
version: &pep440_rs::Version,
) -> Option<&(IndexUrl, File)> {
) -> Option<&(IndexUrl, BaseUrl, File)> {
self.0.get(name)?.get(version)
}
}

View file

@ -55,12 +55,12 @@ impl ResolutionGraph {
match package {
PubGrubPackage::Package(package_name, None, None) => {
let version = Version::from(version.clone());
let (index, file) = pins
let (index, base, file) = pins
.get(package_name, &version)
.expect("Every package should be pinned")
.clone();
let pinned_package =
Dist::from_registry(package_name.clone(), version, file, index);
Dist::from_registry(package_name.clone(), version, file, index, base);
let index = petgraph.add_node(pinned_package);
inverse.insert(package_name, index);
@ -89,12 +89,12 @@ impl ResolutionGraph {
if !metadata.provides_extras.contains(extra) {
let version = Version::from(version.clone());
let (index, file) = pins
let (index, base, file) = pins
.get(package_name, &version)
.expect("Every package should be pinned")
.clone();
let pinned_package =
Dist::from_registry(package_name.clone(), version, file, index);
Dist::from_registry(package_name.clone(), version, file, index, base);
diagnostics.push(Diagnostic::MissingExtra {
dist: pinned_package,

View file

@ -28,7 +28,7 @@ use puffin_client::RegistryClient;
use puffin_distribution::{DistributionDatabase, DistributionDatabaseError};
use puffin_normalize::PackageName;
use puffin_traits::{BuildContext, OnceMap};
use pypi_types::Metadata21;
use pypi_types::{BaseUrl, Metadata21};
use crate::candidate_selector::CandidateSelector;
use crate::error::ResolveError;
@ -44,7 +44,7 @@ use crate::version_map::VersionMap;
use crate::yanks::AllowedYanks;
use crate::ResolutionOptions;
type VersionMapResponse = Result<(IndexUrl, VersionMap), puffin_client::Error>;
type VersionMapResponse = Result<(IndexUrl, BaseUrl, VersionMap), puffin_client::Error>;
type WheelMetadataResponse = Result<(Metadata21, Option<Url>), DistributionDatabaseError>;
pub trait ResolverProvider: Send + Sync {
@ -113,9 +113,10 @@ impl<'a, Context: BuildContext + Send + Sync> ResolverProvider
Box::pin(
self.client
.simple(package_name)
.map_ok(move |(index, metadata)| {
.map_ok(move |(index, base, metadata)| {
(
index,
base,
VersionMap::from_metadata(
metadata,
package_name,
@ -479,7 +480,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
let Some(entry) = self.index.packages.get(package_name) else {
continue;
};
let (index, version_map) = entry.value();
let (index, base, version_map) = entry.value();
// Try to find a compatible version. If there aren't any compatible versions,
// short-circuit and return `None`.
@ -490,7 +491,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
// Emit a request to fetch the metadata for this version.
if self.index.distributions.register(&candidate.package_id()) {
let distribution = candidate.into_distribution(index.clone());
let distribution = candidate.into_distribution(index.clone(), base.clone());
request_sink.unbounded_send(Request::Dist(distribution))?;
}
}
@ -553,7 +554,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
PubGrubPackage::Package(package_name, extra, None) => {
// Wait for the metadata to be available.
let entry = self.index.packages.wait(package_name).await;
let (index, version_map) = entry.value();
let (index, base, version_map) = entry.value();
if let Some(extra) = extra {
debug!(
@ -588,13 +589,13 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
// We want to return a package pinned to a specific version; but we _also_ want to
// store the exact file that we selected to satisfy that version.
pins.insert(&candidate, index);
pins.insert(&candidate, index, base);
let version = candidate.version().clone();
// Emit a request to fetch the metadata for this version.
if self.index.distributions.register(&candidate.package_id()) {
let distribution = candidate.into_distribution(index.clone());
let distribution = candidate.into_distribution(index.clone(), base.clone());
request_sink.unbounded_send(Request::Dist(distribution))?;
}
@ -698,9 +699,11 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
while let Some(response) = response_stream.next().await {
match response? {
Response::Package(package_name, index, version_map) => {
Response::Package(package_name, index, base, version_map) => {
trace!("Received package metadata for: {package_name}");
self.index.packages.done(package_name, (index, version_map));
self.index
.packages
.done(package_name, (index, base, version_map));
}
Response::Dist(Dist::Built(distribution), metadata, ..) => {
trace!("Received built distribution metadata for: {distribution}");
@ -738,12 +741,12 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
match request {
// Fetch package metadata from the registry.
Request::Package(package_name) => {
let (index, metadata) = self
let (index, base, metadata) = self
.provider
.get_version_map(&package_name)
.await
.map_err(ResolveError::Client)?;
Ok(Response::Package(package_name, index, metadata))
Ok(Response::Package(package_name, index, base, metadata))
}
Request::Dist(dist) => {
@ -848,7 +851,7 @@ enum Request {
#[allow(clippy::large_enum_variant)]
enum Response {
/// The returned metadata for a package hosted on a registry.
Package(PackageName, IndexUrl, VersionMap),
Package(PackageName, IndexUrl, BaseUrl, VersionMap),
/// The returned metadata for a distribution.
Dist(Dist, Metadata21, Option<Url>),
}
@ -858,7 +861,7 @@ enum Response {
pub(crate) struct Index {
/// A map from package name to the metadata for that package and the index where the metadata
/// came from.
pub(crate) packages: OnceMap<PackageName, (IndexUrl, VersionMap)>,
pub(crate) packages: OnceMap<PackageName, (IndexUrl, BaseUrl, VersionMap)>,
/// A map from distribution SHA to metadata for that distribution.
pub(crate) distributions: OnceMap<PackageId, Metadata21>,