Enforce and backtrack on invalid versions in source metadata (#2954)

## Summary

If we build a source distribution from the registry, and the version
doesn't match that of the filename, we should error, just as we do for
mismatched package names. However, we should also backtrack here, which
we didn't previously.

Closes https://github.com/astral-sh/uv/issues/2953.

## Test Plan

Verified that `cargo run pip install docutils --verbose --no-cache
--reinstall` installs `docutils==0.21` instead of the invalid
`docutils==0.21.post1`.

In the logs, I see:

```
WARN Unable to extract metadata for docutils: Package metadata version `0.21` does not match given version `0.21.post1`
```
This commit is contained in:
Charlie Marsh 2024-04-10 01:13:33 -04:00 committed by GitHub
parent 997f3c9161
commit c4472ebbb9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 105 additions and 32 deletions

View file

@ -1,6 +1,7 @@
use std::borrow::Cow;
use std::path::Path;
use pep440_rs::Version;
use url::Url;
use uv_normalize::PackageName;
@ -28,6 +29,15 @@ impl BuildableSource<'_> {
}
}
/// Return the [`Version`] of the source, if available.
pub fn version(&self) -> Option<&Version> {
match self {
Self::Dist(SourceDist::Registry(dist)) => Some(&dist.filename.version),
Self::Dist(_) => None,
Self::Url(_) => None,
}
}
/// Return the [`BuildableSource`] as a [`SourceDist`], if it is a distribution.
pub fn as_dist(&self) -> Option<&SourceDist> {
match self {

View file

@ -3,6 +3,7 @@ use tokio::task::JoinError;
use zip::result::ZipError;
use distribution_filename::WheelFilenameError;
use pep440_rs::Version;
use uv_client::BetterReqwestError;
use uv_normalize::PackageName;
@ -47,6 +48,8 @@ pub enum Error {
given: PackageName,
metadata: PackageName,
},
#[error("Package metadata version `{metadata}` does not match given version `{given}`")]
VersionMismatch { given: Version, metadata: Version },
#[error("Failed to parse metadata from built wheel")]
Metadata(#[from] pypi_types::MetadataError),
#[error("Failed to read `dist-info` metadata from built wheel")]

View file

@ -1109,14 +1109,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
let metadata = read_wheel_metadata(&filename, cache_shard.join(&disk_filename))?;
// Validate the metadata.
if let Some(name) = source.name() {
if metadata.name != *name {
return Err(Error::NameMismatch {
metadata: metadata.name,
given: name.clone(),
});
}
}
validate(source, &metadata)?;
debug!("Finished building: {source}");
Ok((disk_filename, filename, metadata))
@ -1138,14 +1131,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
debug!("Found static `PKG-INFO` for: {source}");
// Validate the metadata.
if let Some(name) = source.name() {
if metadata.name != *name {
return Err(Error::NameMismatch {
metadata: metadata.name,
given: name.clone(),
});
}
}
validate(source, &metadata)?;
return Ok(Some(metadata));
}
@ -1161,14 +1147,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
debug!("Found static `pyproject.toml` for: {source}");
// Validate the metadata.
if let Some(name) = source.name() {
if metadata.name != *name {
return Err(Error::NameMismatch {
metadata: metadata.name,
given: name.clone(),
});
}
}
validate(source, &metadata)?;
return Ok(Some(metadata));
}
@ -1208,14 +1187,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
let metadata = Metadata23::parse_metadata(&content)?;
// Validate the metadata.
if let Some(name) = source.name() {
if metadata.name != *name {
return Err(Error::NameMismatch {
metadata: metadata.name,
given: name.clone(),
});
}
}
validate(source, &metadata)?;
Ok(Some(metadata))
}
@ -1278,6 +1250,29 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
}
}
/// Validate that the source distribution matches the built metadata.
fn validate(source: &BuildableSource<'_>, metadata: &Metadata23) -> Result<(), Error> {
if let Some(name) = source.name() {
if metadata.name != *name {
return Err(Error::NameMismatch {
metadata: metadata.name.clone(),
given: name.clone(),
});
}
}
if let Some(version) = source.version() {
if metadata.version != *version {
return Err(Error::VersionMismatch {
metadata: metadata.version.clone(),
given: version.clone(),
});
}
}
Ok(())
}
/// Read an existing HTTP-cached [`Revision`], if it exists.
pub(crate) fn read_http_revision(cache_entry: &CacheEntry) -> Result<Option<Revision>, Error> {
match fs_err::File::open(cache_entry.path()) {

View file

@ -446,6 +446,15 @@ impl PubGrubReportFormatter<'_> {
reason: reason.clone(),
});
}
IncompletePackage::InconsistentMetadata(reason) => {
hints.insert(
PubGrubHint::InconsistentVersionMetadata {
package: package.clone(),
version: version.clone(),
reason: reason.clone(),
},
);
}
IncompletePackage::InvalidStructure(reason) => {
hints.insert(
PubGrubHint::InvalidVersionStructure {
@ -530,6 +539,15 @@ pub(crate) enum PubGrubHint {
#[derivative(PartialEq = "ignore", Hash = "ignore")]
reason: String,
},
/// Metadata for a package version was inconsistent (e.g., the package name did not match that
/// of the file).
InconsistentVersionMetadata {
package: PubGrubPackage,
#[derivative(PartialEq = "ignore", Hash = "ignore")]
version: Version,
#[derivative(PartialEq = "ignore", Hash = "ignore")]
reason: String,
},
/// The structure of a package version was invalid (e.g., multiple `.dist-info` directories).
InvalidVersionStructure {
package: PubGrubPackage,
@ -629,6 +647,21 @@ impl std::fmt::Display for PubGrubHint {
textwrap::indent(reason, " ")
)
}
PubGrubHint::InconsistentVersionMetadata {
package,
version,
reason,
} => {
write!(
f,
"{}{} Metadata for {}=={} was inconsistent:\n{}",
"hint".bold().cyan(),
":".bold(),
package.bold(),
version.bold(),
textwrap::indent(reason, " ")
)
}
}
}
}

View file

@ -93,6 +93,8 @@ pub(crate) enum IncompletePackage {
Offline,
/// The wheel metadata was found, but could not be parsed.
InvalidMetadata(String),
/// The wheel metadata was found, but the metadata was inconsistent.
InconsistentMetadata(String),
/// The wheel has an invalid structure.
InvalidStructure(String),
}
@ -646,6 +648,13 @@ impl<
);
return Ok(None);
}
MetadataResponse::InconsistentMetadata(err) => {
self.unavailable_packages.insert(
package_name.clone(),
UnavailablePackage::InvalidMetadata(err.to_string()),
);
return Ok(None);
}
MetadataResponse::InvalidStructure(err) => {
self.unavailable_packages.insert(
package_name.clone(),
@ -945,6 +954,7 @@ impl<
));
}
MetadataResponse::InvalidMetadata(err) => {
warn!("Unable to extract metadata for {package_name}: {err}");
self.incomplete_packages
.entry(package_name.clone())
.or_default()
@ -956,7 +966,21 @@ impl<
"the package metadata could not be parsed".to_string(),
));
}
MetadataResponse::InconsistentMetadata(err) => {
warn!("Unable to extract metadata for {package_name}: {err}");
self.incomplete_packages
.entry(package_name.clone())
.or_default()
.insert(
version.clone(),
IncompletePackage::InconsistentMetadata(err.to_string()),
);
return Ok(Dependencies::Unavailable(
"the package metadata was inconsistent".to_string(),
));
}
MetadataResponse::InvalidStructure(err) => {
warn!("Unable to extract metadata for {package_name}: {err}");
self.incomplete_packages
.entry(package_name.clone())
.or_default()

View file

@ -38,6 +38,8 @@ pub enum MetadataResponse {
Found(Metadata23),
/// The wheel metadata was found, but could not be parsed.
InvalidMetadata(Box<pypi_types::MetadataError>),
/// The wheel metadata was found, but the metadata was inconsistent.
InconsistentMetadata(Box<uv_distribution::Error>),
/// The wheel has an invalid structure.
InvalidStructure(Box<install_wheel_rs::Error>),
/// The wheel metadata was not found in the cache and the network is not available.
@ -185,6 +187,12 @@ impl<'a, Context: BuildContext + Send + Sync> ResolverProvider
}
kind => Err(uv_client::Error::from(kind).into()),
},
uv_distribution::Error::VersionMismatch { .. } => {
Ok(MetadataResponse::InconsistentMetadata(Box::new(err)))
}
uv_distribution::Error::NameMismatch { .. } => {
Ok(MetadataResponse::InconsistentMetadata(Box::new(err)))
}
uv_distribution::Error::Metadata(err) => {
Ok(MetadataResponse::InvalidMetadata(Box::new(err)))
}