Surface invalid metadata as hints in error reports (#2850)

## Summary

Closes #2847.
This commit is contained in:
Charlie Marsh 2024-04-09 23:12:10 -04:00 committed by GitHub
parent ee9059978a
commit 7ae06b3b46
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 229 additions and 27 deletions

1
Cargo.lock generated
View file

@ -4786,6 +4786,7 @@ dependencies = [
"requirements-txt", "requirements-txt",
"rkyv", "rkyv",
"rustc-hash", "rustc-hash",
"textwrap",
"thiserror", "thiserror",
"tokio", "tokio",
"tokio-stream", "tokio-stream",

View file

@ -48,6 +48,7 @@ petgraph = { workspace = true }
pubgrub = { workspace = true } pubgrub = { workspace = true }
rkyv = { workspace = true } rkyv = { workspace = true }
rustc-hash = { workspace = true } rustc-hash = { workspace = true }
textwrap = { workspace = true }
thiserror = { workspace = true } thiserror = { workspace = true }
tokio = { workspace = true, features = ["macros"] } tokio = { workspace = true, features = ["macros"] }
tokio-stream = { workspace = true } tokio-stream = { workspace = true }

View file

@ -1,4 +1,4 @@
use std::collections::BTreeSet; use std::collections::{BTreeMap, BTreeSet};
use std::fmt::Formatter; use std::fmt::Formatter;
use std::ops::Deref; use std::ops::Deref;
@ -20,7 +20,7 @@ use crate::candidate_selector::CandidateSelector;
use crate::dependency_provider::UvDependencyProvider; use crate::dependency_provider::UvDependencyProvider;
use crate::pubgrub::{PubGrubPackage, PubGrubPython, PubGrubReportFormatter}; use crate::pubgrub::{PubGrubPackage, PubGrubPython, PubGrubReportFormatter};
use crate::python_requirement::PythonRequirement; use crate::python_requirement::PythonRequirement;
use crate::resolver::{UnavailablePackage, VersionsResponse}; use crate::resolver::{IncompletePackage, UnavailablePackage, VersionsResponse};
#[derive(Debug, thiserror::Error)] #[derive(Debug, thiserror::Error)]
pub enum ResolveError { pub enum ResolveError {
@ -125,6 +125,7 @@ impl From<pubgrub::error::PubGrubError<UvDependencyProvider>> for ResolveError {
python_requirement: None, python_requirement: None,
index_locations: None, index_locations: None,
unavailable_packages: FxHashMap::default(), unavailable_packages: FxHashMap::default(),
incomplete_packages: FxHashMap::default(),
}) })
} }
pubgrub::error::PubGrubError::SelfDependency { package, version } => { pubgrub::error::PubGrubError::SelfDependency { package, version } => {
@ -146,6 +147,7 @@ pub struct NoSolutionError {
python_requirement: Option<PythonRequirement>, python_requirement: Option<PythonRequirement>,
index_locations: Option<IndexLocations>, index_locations: Option<IndexLocations>,
unavailable_packages: FxHashMap<PackageName, UnavailablePackage>, unavailable_packages: FxHashMap<PackageName, UnavailablePackage>,
incomplete_packages: FxHashMap<PackageName, BTreeMap<Version, IncompletePackage>>,
} }
impl std::error::Error for NoSolutionError {} impl std::error::Error for NoSolutionError {}
@ -167,6 +169,7 @@ impl std::fmt::Display for NoSolutionError {
&self.selector, &self.selector,
&self.index_locations, &self.index_locations,
&self.unavailable_packages, &self.unavailable_packages,
&self.incomplete_packages,
) { ) {
write!(f, "\n\n{hint}")?; write!(f, "\n\n{hint}")?;
} }
@ -261,6 +264,30 @@ impl NoSolutionError {
self self
} }
/// Update the incomplete packages attached to the error.
#[must_use]
pub(crate) fn with_incomplete_packages(
mut self,
incomplete_packages: &DashMap<PackageName, DashMap<Version, IncompletePackage>>,
) -> Self {
let mut new = FxHashMap::default();
for package in self.derivation_tree.packages() {
if let PubGrubPackage::Package(name, ..) = package {
if let Some(entry) = incomplete_packages.get(name) {
let versions = entry.value();
for entry in versions {
let (version, reason) = entry.pair();
new.entry(name.clone())
.or_insert_with(BTreeMap::default)
.insert(version.clone(), reason.clone());
}
}
}
}
self.incomplete_packages = new;
self
}
/// Update the Python requirements attached to the error. /// Update the Python requirements attached to the error.
#[must_use] #[must_use]
pub(crate) fn with_python_requirement( pub(crate) fn with_python_requirement(

View file

@ -1,6 +1,6 @@
use std::borrow::Cow; use std::borrow::Cow;
use std::cmp::Ordering; use std::cmp::Ordering;
use std::collections::BTreeSet; use std::collections::{BTreeMap, BTreeSet};
use std::ops::Bound; use std::ops::Bound;
use derivative::Derivative; use derivative::Derivative;
@ -17,7 +17,7 @@ use uv_normalize::PackageName;
use crate::candidate_selector::CandidateSelector; use crate::candidate_selector::CandidateSelector;
use crate::python_requirement::PythonRequirement; use crate::python_requirement::PythonRequirement;
use crate::resolver::UnavailablePackage; use crate::resolver::{IncompletePackage, UnavailablePackage};
use super::PubGrubPackage; use super::PubGrubPackage;
@ -342,6 +342,7 @@ impl PubGrubReportFormatter<'_> {
selector: &Option<CandidateSelector>, selector: &Option<CandidateSelector>,
index_locations: &Option<IndexLocations>, index_locations: &Option<IndexLocations>,
unavailable_packages: &FxHashMap<PackageName, UnavailablePackage>, unavailable_packages: &FxHashMap<PackageName, UnavailablePackage>,
incomplete_packages: &FxHashMap<PackageName, BTreeMap<Version, IncompletePackage>>,
) -> IndexSet<PubGrubHint> { ) -> IndexSet<PubGrubHint> {
/// Returns `true` if pre-releases were allowed for a package. /// Returns `true` if pre-releases were allowed for a package.
fn allowed_prerelease(package: &PubGrubPackage, selector: &CandidateSelector) -> bool { fn allowed_prerelease(package: &PubGrubPackage, selector: &CandidateSelector) -> bool {
@ -354,7 +355,7 @@ impl PubGrubReportFormatter<'_> {
let mut hints = IndexSet::default(); let mut hints = IndexSet::default();
match derivation_tree { match derivation_tree {
DerivationTree::External(external) => match external { DerivationTree::External(external) => match external {
External::NoVersions(package, set, _) => { External::Unavailable(package, set, _) | External::NoVersions(package, set, _) => {
// Check for no versions due to pre-release options // Check for no versions due to pre-release options
if let Some(selector) = selector { if let Some(selector) = selector {
let any_prerelease = set.iter().any(|(start, end)| { let any_prerelease = set.iter().any(|(start, end)| {
@ -404,6 +405,7 @@ impl PubGrubReportFormatter<'_> {
index_locations.flat_index().peekable().peek().is_none(); index_locations.flat_index().peekable().peek().is_none();
if let PubGrubPackage::Package(name, ..) = package { if let PubGrubPackage::Package(name, ..) = package {
// Add hints due to the package being entirely unavailable.
match unavailable_packages.get(name) { match unavailable_packages.get(name) {
Some(UnavailablePackage::NoIndex) => { Some(UnavailablePackage::NoIndex) => {
if no_find_links { if no_find_links {
@ -413,13 +415,55 @@ impl PubGrubReportFormatter<'_> {
Some(UnavailablePackage::Offline) => { Some(UnavailablePackage::Offline) => {
hints.insert(PubGrubHint::Offline); hints.insert(PubGrubHint::Offline);
} }
_ => {} Some(UnavailablePackage::InvalidMetadata(reason)) => {
hints.insert(PubGrubHint::InvalidPackageMetadata {
package: package.clone(),
reason: reason.clone(),
});
}
Some(UnavailablePackage::InvalidStructure(reason)) => {
hints.insert(PubGrubHint::InvalidPackageStructure {
package: package.clone(),
reason: reason.clone(),
});
}
Some(UnavailablePackage::NotFound) => {}
None => {}
}
// Add hints due to the package being unavailable at specific versions.
if let Some(versions) = incomplete_packages.get(name) {
for (version, incomplete) in versions.iter().rev() {
if set.contains(version) {
match incomplete {
IncompletePackage::Offline => {
hints.insert(PubGrubHint::Offline);
}
IncompletePackage::InvalidMetadata(reason) => {
hints.insert(PubGrubHint::InvalidVersionMetadata {
package: package.clone(),
version: version.clone(),
reason: reason.clone(),
});
}
IncompletePackage::InvalidStructure(reason) => {
hints.insert(
PubGrubHint::InvalidVersionStructure {
package: package.clone(),
version: version.clone(),
reason: reason.clone(),
},
);
}
}
break;
}
}
} }
} }
} }
} }
External::NotRoot(..) => {} External::NotRoot(..) => {}
External::Unavailable(..) => {}
External::FromDependencyOf(..) => {} External::FromDependencyOf(..) => {}
}, },
DerivationTree::Derived(derived) => { DerivationTree::Derived(derived) => {
@ -428,12 +472,14 @@ impl PubGrubReportFormatter<'_> {
selector, selector,
index_locations, index_locations,
unavailable_packages, unavailable_packages,
incomplete_packages,
)); ));
hints.extend(self.hints( hints.extend(self.hints(
&derived.cause2, &derived.cause2,
selector, selector,
index_locations, index_locations,
unavailable_packages, unavailable_packages,
incomplete_packages,
)); ));
} }
} }
@ -462,8 +508,36 @@ pub(crate) enum PubGrubHint {
/// Requirements were unavailable due to lookups in the index being disabled and no extra /// Requirements were unavailable due to lookups in the index being disabled and no extra
/// index was provided via `--find-links` /// index was provided via `--find-links`
NoIndex, NoIndex,
/// A package was not found in the registry, but /// A package was not found in the registry, but network access was disabled.
Offline, Offline,
/// Metadata for a package could not be parsed.
InvalidPackageMetadata {
package: PubGrubPackage,
#[derivative(PartialEq = "ignore", Hash = "ignore")]
reason: String,
},
/// The structure of a package was invalid (e.g., multiple `.dist-info` directories).
InvalidPackageStructure {
package: PubGrubPackage,
#[derivative(PartialEq = "ignore", Hash = "ignore")]
reason: String,
},
/// Metadata for a package version could not be parsed.
InvalidVersionMetadata {
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,
#[derivative(PartialEq = "ignore", Hash = "ignore")]
version: Version,
#[derivative(PartialEq = "ignore", Hash = "ignore")]
reason: String,
},
} }
impl std::fmt::Display for PubGrubHint { impl std::fmt::Display for PubGrubHint {
@ -505,6 +579,56 @@ impl std::fmt::Display for PubGrubHint {
":".bold(), ":".bold(),
) )
} }
Self::InvalidPackageMetadata { package, reason } => {
write!(
f,
"{}{} Metadata for {} could not be parsed:\n{}",
"hint".bold().cyan(),
":".bold(),
package.bold(),
textwrap::indent(reason, " ")
)
}
Self::InvalidPackageStructure { package, reason } => {
write!(
f,
"{}{} The structure of {} was invalid:\n{}",
"hint".bold().cyan(),
":".bold(),
package.bold(),
textwrap::indent(reason, " ")
)
}
Self::InvalidVersionMetadata {
package,
version,
reason,
} => {
write!(
f,
"{}{} Metadata for {}=={} could not be parsed:\n{}",
"hint".bold().cyan(),
":".bold(),
package.bold(),
version.bold(),
textwrap::indent(reason, " ")
)
}
Self::InvalidVersionStructure {
package,
version,
reason,
} => {
write!(
f,
"{}{} The structure of {}=={} was invalid:\n{}",
"hint".bold().cyan(),
":".bold(),
package.bold(),
version.bold(),
textwrap::indent(reason, " ")
)
}
} }
} }
} }

View file

@ -71,19 +71,30 @@ pub(crate) enum UnavailableVersion {
IncompatibleDist(IncompatibleDist), IncompatibleDist(IncompatibleDist),
} }
/// The package is unavailable and cannot be used /// The package is unavailable and cannot be used.
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub(crate) enum UnavailablePackage { pub(crate) enum UnavailablePackage {
/// Index lookups were disabled (i.e., `--no-index`) and the package was not found in a flat index (i.e. from `--find-links`) /// Index lookups were disabled (i.e., `--no-index`) and the package was not found in a flat index (i.e. from `--find-links`).
NoIndex, NoIndex,
/// Network requests were disabled (i.e., `--offline`), and the package was not found in the cache. /// Network requests were disabled (i.e., `--offline`), and the package was not found in the cache.
Offline, Offline,
/// The package was not found in the registry /// The package was not found in the registry.
NotFound, NotFound,
/// The package metadata was found, but could not be parsed.
InvalidMetadata(String),
/// The package has an invalid structure.
InvalidStructure(String),
}
/// The package is unavailable at specific versions.
#[derive(Debug, Clone)]
pub(crate) enum IncompletePackage {
/// Network requests were disabled (i.e., `--offline`), and the wheel metadata was not found in the cache.
Offline,
/// The wheel metadata was found, but could not be parsed. /// The wheel metadata was found, but could not be parsed.
InvalidMetadata, InvalidMetadata(String),
/// The wheel has an invalid structure. /// The wheel has an invalid structure.
InvalidStructure, InvalidStructure(String),
} }
enum ResolverVersion { enum ResolverVersion {
@ -113,8 +124,10 @@ pub struct Resolver<
selector: CandidateSelector, selector: CandidateSelector,
index: &'a InMemoryIndex, index: &'a InMemoryIndex,
installed_packages: &'a InstalledPackages, installed_packages: &'a InstalledPackages,
/// Incompatibilities for packages that are entirely unavailable /// Incompatibilities for packages that are entirely unavailable.
unavailable_packages: DashMap<PackageName, UnavailablePackage>, unavailable_packages: DashMap<PackageName, UnavailablePackage>,
/// Incompatibilities for packages that are unavailable at specific versions.
incomplete_packages: DashMap<PackageName, DashMap<Version, IncompletePackage>>,
/// The set of all registry-based packages visited during resolution. /// The set of all registry-based packages visited during resolution.
visited: DashSet<PackageName>, visited: DashSet<PackageName>,
reporter: Option<Arc<dyn Reporter>>, reporter: Option<Arc<dyn Reporter>>,
@ -185,6 +198,7 @@ impl<
Ok(Self { Ok(Self {
index, index,
unavailable_packages: DashMap::default(), unavailable_packages: DashMap::default(),
incomplete_packages: DashMap::default(),
visited: DashSet::default(), visited: DashSet::default(),
selector: CandidateSelector::for_resolution(options, &manifest, markers), selector: CandidateSelector::for_resolution(options, &manifest, markers),
dependency_mode: options.dependency_mode, dependency_mode: options.dependency_mode,
@ -247,7 +261,8 @@ impl<
.with_selector(self.selector.clone()) .with_selector(self.selector.clone())
.with_python_requirement(&self.python_requirement) .with_python_requirement(&self.python_requirement)
.with_index_locations(self.provider.index_locations()) .with_index_locations(self.provider.index_locations())
.with_unavailable_packages(&self.unavailable_packages), .with_unavailable_packages(&self.unavailable_packages)
.with_incomplete_packages(&self.incomplete_packages),
) )
} else { } else {
err err
@ -352,10 +367,10 @@ impl<
UnavailablePackage::NotFound => { UnavailablePackage::NotFound => {
"was not found in the package registry" "was not found in the package registry"
} }
UnavailablePackage::InvalidMetadata => { UnavailablePackage::InvalidMetadata(_) => {
"was found, but the metadata could not be parsed" "was found, but the metadata could not be parsed"
} }
UnavailablePackage::InvalidStructure => { UnavailablePackage::InvalidStructure(_) => {
"was found, but has an invalid format" "was found, but has an invalid format"
} }
}) })
@ -624,14 +639,18 @@ impl<
.insert(package_name.clone(), UnavailablePackage::Offline); .insert(package_name.clone(), UnavailablePackage::Offline);
return Ok(None); return Ok(None);
} }
MetadataResponse::InvalidMetadata(_) => { MetadataResponse::InvalidMetadata(err) => {
self.unavailable_packages self.unavailable_packages.insert(
.insert(package_name.clone(), UnavailablePackage::InvalidMetadata); package_name.clone(),
UnavailablePackage::InvalidMetadata(err.to_string()),
);
return Ok(None); return Ok(None);
} }
MetadataResponse::InvalidStructure(_) => { MetadataResponse::InvalidStructure(err) => {
self.unavailable_packages self.unavailable_packages.insert(
.insert(package_name.clone(), UnavailablePackage::InvalidStructure); package_name.clone(),
UnavailablePackage::InvalidStructure(err.to_string()),
);
return Ok(None); return Ok(None);
} }
}; };
@ -913,20 +932,38 @@ impl<
.await .await
.ok_or(ResolveError::Unregistered)?; .ok_or(ResolveError::Unregistered)?;
let metadata = match *response { let metadata = match &*response {
MetadataResponse::Found(ref metadata) => metadata, MetadataResponse::Found(metadata) => metadata,
MetadataResponse::Offline => { MetadataResponse::Offline => {
self.incomplete_packages
.entry(package_name.clone())
.or_default()
.insert(version.clone(), IncompletePackage::Offline);
return Ok(Dependencies::Unavailable( return Ok(Dependencies::Unavailable(
"network connectivity is disabled, but the metadata wasn't found in the cache" "network connectivity is disabled, but the metadata wasn't found in the cache"
.to_string(), .to_string(),
)); ));
} }
MetadataResponse::InvalidMetadata(_) => { MetadataResponse::InvalidMetadata(err) => {
self.incomplete_packages
.entry(package_name.clone())
.or_default()
.insert(
version.clone(),
IncompletePackage::InvalidMetadata(err.to_string()),
);
return Ok(Dependencies::Unavailable( return Ok(Dependencies::Unavailable(
"the package metadata could not be parsed".to_string(), "the package metadata could not be parsed".to_string(),
)); ));
} }
MetadataResponse::InvalidStructure(_) => { MetadataResponse::InvalidStructure(err) => {
self.incomplete_packages
.entry(package_name.clone())
.or_default()
.insert(
version.clone(),
IncompletePackage::InvalidStructure(err.to_string()),
);
return Ok(Dependencies::Unavailable( return Ok(Dependencies::Unavailable(
"the package has an invalid format".to_string(), "the package has an invalid format".to_string(),
)); ));

View file

@ -4555,6 +4555,12 @@ fn invalid_metadata_requires_python() -> Result<()> {
----- stderr ----- ----- stderr -----
× No solution found when resolving dependencies: × No solution found when resolving dependencies:
Because validation==2.0.0 is unusable because the package metadata could not be parsed and you require validation==2.0.0, we can conclude that the requirements are unsatisfiable. Because validation==2.0.0 is unusable because the package metadata could not be parsed and you require validation==2.0.0, we can conclude that the requirements are unsatisfiable.
hint: Metadata for validation==2.0.0 could not be parsed:
Failed to parse version: Unexpected end of version specifier, expected operator:
12
^^
"### "###
); );
@ -4581,6 +4587,9 @@ fn invalid_metadata_multiple_dist_info() -> Result<()> {
----- stderr ----- ----- stderr -----
× No solution found when resolving dependencies: × No solution found when resolving dependencies:
Because validation==3.0.0 is unusable because the package has an invalid format and you require validation==3.0.0, we can conclude that the requirements are unsatisfiable. Because validation==3.0.0 is unusable because the package has an invalid format and you require validation==3.0.0, we can conclude that the requirements are unsatisfiable.
hint: The structure of validation==3.0.0 was invalid:
Multiple .dist-info directories found: validation-2.0.0, validation-3.0.0
"### "###
); );

View file

@ -1106,6 +1106,9 @@ fn mismatched_name() -> Result<()> {
----- stderr ----- ----- stderr -----
× No solution found when resolving dependencies: × No solution found when resolving dependencies:
Because foo was found, but has an invalid format and you require foo, we can conclude that the requirements are unsatisfiable. Because foo was found, but has an invalid format and you require foo, we can conclude that the requirements are unsatisfiable.
hint: The structure of foo was invalid:
The .dist-info directory tomli-2.0.1 does not start with the normalized package name: foo
"### "###
); );