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

Refactor unavailable metadata to shrink the resolver #9769

Merged
merged 7 commits into from
Dec 10, 2024
Merged
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
8 changes: 4 additions & 4 deletions crates/uv-installer/src/site_packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ impl SitePackages {

// Determine the dependencies for the given package.
let Ok(metadata) = distribution.metadata() else {
diagnostics.push(SitePackagesDiagnostic::IncompletePackage {
diagnostics.push(SitePackagesDiagnostic::MetadataUnavailable {
package: package.clone(),
path: distribution.path().to_owned(),
});
Expand Down Expand Up @@ -405,7 +405,7 @@ impl IntoIterator for SitePackages {

#[derive(Debug)]
pub enum SitePackagesDiagnostic {
IncompletePackage {
MetadataUnavailable {
/// The package that is missing metadata.
package: PackageName,
/// The path to the package.
Expand Down Expand Up @@ -445,7 +445,7 @@ impl Diagnostic for SitePackagesDiagnostic {
/// Convert the diagnostic into a user-facing message.
fn message(&self) -> String {
match self {
Self::IncompletePackage { package, path } => format!(
Self::MetadataUnavailable { package, path } => format!(
"The package `{package}` is broken or incomplete (unable to read `METADATA`). Consider recreating the virtualenv, or removing the package directory at: {}.", path.display(),
),
Self::IncompatiblePythonVersion {
Expand Down Expand Up @@ -482,7 +482,7 @@ impl Diagnostic for SitePackagesDiagnostic {
/// Returns `true` if the [`PackageName`] is involved in this diagnostic.
fn includes(&self, name: &PackageName) -> bool {
match self {
Self::IncompletePackage { package, .. } => name == package,
Self::MetadataUnavailable { package, .. } => name == package,
Self::IncompatiblePythonVersion { package, .. } => name == package,
Self::MissingDependency { package, .. } => name == package,
Self::IncompatibleDependency {
Expand Down
6 changes: 3 additions & 3 deletions crates/uv-resolver/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::pubgrub::{PubGrubPackage, PubGrubPackageInner, PubGrubReportFormatter
use crate::python_requirement::PythonRequirement;
use crate::resolution::ConflictingDistributionError;
use crate::resolver::{
IncompletePackage, ResolverEnvironment, UnavailablePackage, UnavailableReason,
MetadataUnavailable, ResolverEnvironment, UnavailablePackage, UnavailableReason,
};
use crate::Options;

Expand Down Expand Up @@ -145,7 +145,7 @@ pub struct NoSolutionError {
index_locations: IndexLocations,
index_capabilities: IndexCapabilities,
unavailable_packages: FxHashMap<PackageName, UnavailablePackage>,
incomplete_packages: FxHashMap<PackageName, BTreeMap<Version, IncompletePackage>>,
incomplete_packages: FxHashMap<PackageName, BTreeMap<Version, MetadataUnavailable>>,
fork_urls: ForkUrls,
env: ResolverEnvironment,
workspace_members: BTreeSet<PackageName>,
Expand All @@ -163,7 +163,7 @@ impl NoSolutionError {
index_locations: IndexLocations,
index_capabilities: IndexCapabilities,
unavailable_packages: FxHashMap<PackageName, UnavailablePackage>,
incomplete_packages: FxHashMap<PackageName, BTreeMap<Version, IncompletePackage>>,
incomplete_packages: FxHashMap<PackageName, BTreeMap<Version, MetadataUnavailable>>,
fork_urls: ForkUrls,
env: ResolverEnvironment,
workspace_members: BTreeSet<PackageName>,
Expand Down
72 changes: 11 additions & 61 deletions crates/uv-resolver/src/pubgrub/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::error::ErrorTree;
use crate::fork_urls::ForkUrls;
use crate::prerelease::AllowPrerelease;
use crate::python_requirement::{PythonRequirement, PythonRequirementSource};
use crate::resolver::{IncompletePackage, UnavailablePackage, UnavailableReason};
use crate::resolver::{MetadataUnavailable, UnavailablePackage, UnavailableReason};
use crate::{Flexibility, Options, RequiresPython, ResolverEnvironment};

use super::{PubGrubPackage, PubGrubPackageInner, PubGrubPython};
Expand Down Expand Up @@ -548,7 +548,7 @@ impl PubGrubReportFormatter<'_> {
index_capabilities: &IndexCapabilities,
available_indexes: &FxHashMap<PackageName, BTreeSet<IndexUrl>>,
unavailable_packages: &FxHashMap<PackageName, UnavailablePackage>,
incomplete_packages: &FxHashMap<PackageName, BTreeMap<Version, IncompletePackage>>,
incomplete_packages: &FxHashMap<PackageName, BTreeMap<Version, MetadataUnavailable>>,
fork_urls: &ForkUrls,
env: &ResolverEnvironment,
workspace_members: &BTreeSet<PackageName>,
Expand Down Expand Up @@ -679,7 +679,7 @@ impl PubGrubReportFormatter<'_> {
index_capabilities: &IndexCapabilities,
available_indexes: &FxHashMap<PackageName, BTreeSet<IndexUrl>>,
unavailable_packages: &FxHashMap<PackageName, UnavailablePackage>,
incomplete_packages: &FxHashMap<PackageName, BTreeMap<Version, IncompletePackage>>,
incomplete_packages: &FxHashMap<PackageName, BTreeMap<Version, MetadataUnavailable>>,
hints: &mut IndexSet<PubGrubHint>,
) {
let no_find_links = index_locations.flat_indexes().peekable().peek().is_none();
Expand All @@ -694,11 +694,6 @@ impl PubGrubReportFormatter<'_> {
Some(UnavailablePackage::Offline) => {
hints.insert(PubGrubHint::Offline);
}
Some(UnavailablePackage::MissingMetadata) => {
hints.insert(PubGrubHint::MissingPackageMetadata {
package: package.clone(),
});
}
Some(UnavailablePackage::InvalidMetadata(reason)) => {
hints.insert(PubGrubHint::InvalidPackageMetadata {
package: package.clone(),
Expand All @@ -720,37 +715,31 @@ impl PubGrubReportFormatter<'_> {
for (version, incomplete) in versions.iter().rev() {
if set.contains(version) {
match incomplete {
IncompletePackage::Offline => {
MetadataUnavailable::Offline => {
hints.insert(PubGrubHint::Offline);
}
IncompletePackage::MissingMetadata => {
hints.insert(PubGrubHint::MissingVersionMetadata {
package: package.clone(),
version: version.clone(),
});
}
IncompletePackage::InvalidMetadata(reason) => {
MetadataUnavailable::InvalidMetadata(reason) => {
hints.insert(PubGrubHint::InvalidVersionMetadata {
package: package.clone(),
version: version.clone(),
reason: reason.clone(),
reason: reason.to_string(),
});
}
IncompletePackage::InconsistentMetadata(reason) => {
MetadataUnavailable::InconsistentMetadata(reason) => {
hints.insert(PubGrubHint::InconsistentVersionMetadata {
package: package.clone(),
version: version.clone(),
reason: reason.clone(),
reason: reason.to_string(),
});
}
IncompletePackage::InvalidStructure(reason) => {
MetadataUnavailable::InvalidStructure(reason) => {
hints.insert(PubGrubHint::InvalidVersionStructure {
package: package.clone(),
version: version.clone(),
reason: reason.clone(),
reason: reason.to_string(),
});
}
IncompletePackage::RequiresPython(requires_python, python_version) => {
MetadataUnavailable::RequiresPython(requires_python, python_version) => {
hints.insert(PubGrubHint::IncompatibleBuildRequirement {
package: package.clone(),
version: version.clone(),
Expand Down Expand Up @@ -882,8 +871,6 @@ pub(crate) enum PubGrubHint {
NoIndex,
/// A package was not found in the registry, but network access was disabled.
Offline,
/// Metadata for a package could not be found.
MissingPackageMetadata { package: PubGrubPackage },
/// Metadata for a package could not be parsed.
InvalidPackageMetadata {
package: PubGrubPackage,
Expand All @@ -896,12 +883,6 @@ pub(crate) enum PubGrubHint {
// excluded from `PartialEq` and `Hash`
reason: String,
},
/// Metadata for a package version could not be found.
MissingVersionMetadata {
package: PubGrubPackage,
// excluded from `PartialEq` and `Hash`
version: Version,
},
/// Metadata for a package version could not be parsed.
InvalidVersionMetadata {
package: PubGrubPackage,
Expand Down Expand Up @@ -992,18 +973,12 @@ enum PubGrubHintCore {
},
NoIndex,
Offline,
MissingPackageMetadata {
package: PubGrubPackage,
},
InvalidPackageMetadata {
package: PubGrubPackage,
},
InvalidPackageStructure {
package: PubGrubPackage,
},
MissingVersionMetadata {
package: PubGrubPackage,
},
InvalidVersionMetadata {
package: PubGrubPackage,
},
Expand Down Expand Up @@ -1052,18 +1027,12 @@ impl From<PubGrubHint> for PubGrubHintCore {
}
PubGrubHint::NoIndex => Self::NoIndex,
PubGrubHint::Offline => Self::Offline,
PubGrubHint::MissingPackageMetadata { package, .. } => {
Self::MissingPackageMetadata { package }
}
PubGrubHint::InvalidPackageMetadata { package, .. } => {
Self::InvalidPackageMetadata { package }
}
PubGrubHint::InvalidPackageStructure { package, .. } => {
Self::InvalidPackageStructure { package }
}
PubGrubHint::MissingVersionMetadata { package, .. } => {
Self::MissingVersionMetadata { package }
}
PubGrubHint::InvalidVersionMetadata { package, .. } => {
Self::InvalidVersionMetadata { package }
}
Expand Down Expand Up @@ -1162,15 +1131,6 @@ impl std::fmt::Display for PubGrubHint {
":".bold(),
)
}
Self::MissingPackageMetadata { package } => {
write!(
f,
"{}{} Metadata for `{}` could not be found, as the wheel is missing a `METADATA` file",
"hint".bold().cyan(),
":".bold(),
package.bold()
)
}
Self::InvalidPackageMetadata { package, reason } => {
write!(
f,
Expand All @@ -1191,16 +1151,6 @@ impl std::fmt::Display for PubGrubHint {
textwrap::indent(reason, " ")
)
}
Self::MissingVersionMetadata { package, version } => {
write!(
f,
"{}{} Metadata for `{}` ({}) could not be found, as the wheel is missing a `METADATA` file",
"hint".bold().cyan(),
":".bold(),
package.cyan(),
format!("v{version}").cyan(),
)
}
Self::InvalidVersionMetadata {
package,
version,
Expand Down
62 changes: 34 additions & 28 deletions crates/uv-resolver/src/resolver/availability.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::fmt::{Display, Formatter};

use crate::resolver::MetadataUnavailable;
use uv_distribution_types::IncompatibleDist;
use uv_pep440::{Version, VersionSpecifiers};

Expand All @@ -21,17 +22,15 @@ impl Display for UnavailableReason {
}
}

/// The package version is unavailable and cannot be used. Unlike [`PackageUnavailable`], this
/// The package version is unavailable and cannot be used. Unlike [`MetadataUnavailable`], this
/// applies to a single version of the package.
///
/// Most variant are from [`MetadataResponse`] without the error source (since we don't format
/// the source).
/// Most variant are from [`MetadataResponse`] without the error source, since we don't format
/// the source and we want to merge unavailable messages across versions.
#[derive(Debug, Clone, Eq, PartialEq)]
pub(crate) enum UnavailableVersion {
/// Version is incompatible because it has no usable distributions
IncompatibleDist(IncompatibleDist),
/// The wheel metadata was not found.
MissingMetadata,
/// The wheel metadata was found, but could not be parsed.
InvalidMetadata,
/// The wheel metadata was found, but the metadata was inconsistent.
Expand All @@ -49,7 +48,6 @@ impl UnavailableVersion {
pub(crate) fn message(&self) -> String {
match self {
UnavailableVersion::IncompatibleDist(invalid_dist) => format!("{invalid_dist}"),
UnavailableVersion::MissingMetadata => "not include a `METADATA` file".into(),
UnavailableVersion::InvalidMetadata => "invalid metadata".into(),
UnavailableVersion::InconsistentMetadata => "inconsistent metadata".into(),
UnavailableVersion::InvalidStructure => "an invalid package format".into(),
Expand All @@ -63,7 +61,6 @@ impl UnavailableVersion {
pub(crate) fn singular_message(&self) -> String {
match self {
UnavailableVersion::IncompatibleDist(invalid_dist) => invalid_dist.singular_message(),
UnavailableVersion::MissingMetadata => format!("does {self}"),
UnavailableVersion::InvalidMetadata => format!("has {self}"),
UnavailableVersion::InconsistentMetadata => format!("has {self}"),
UnavailableVersion::InvalidStructure => format!("has {self}"),
Expand All @@ -75,7 +72,6 @@ impl UnavailableVersion {
pub(crate) fn plural_message(&self) -> String {
match self {
UnavailableVersion::IncompatibleDist(invalid_dist) => invalid_dist.plural_message(),
UnavailableVersion::MissingMetadata => format!("do {self}"),
UnavailableVersion::InvalidMetadata => format!("have {self}"),
UnavailableVersion::InconsistentMetadata => format!("have {self}"),
UnavailableVersion::InvalidStructure => format!("have {self}"),
Expand All @@ -91,6 +87,22 @@ impl Display for UnavailableVersion {
}
}

impl From<&MetadataUnavailable> for UnavailableVersion {
fn from(reason: &MetadataUnavailable) -> Self {
match reason {
MetadataUnavailable::Offline => UnavailableVersion::Offline,
MetadataUnavailable::InvalidMetadata(_) => UnavailableVersion::InvalidMetadata,
MetadataUnavailable::InconsistentMetadata(_) => {
UnavailableVersion::InconsistentMetadata
}
MetadataUnavailable::InvalidStructure(_) => UnavailableVersion::InvalidStructure,
MetadataUnavailable::RequiresPython(requires_python, _python_version) => {
UnavailableVersion::RequiresPython(requires_python.clone())
}
}
}
}

/// The package is unavailable and cannot be used.
#[derive(Debug, Clone, Eq, PartialEq)]
pub(crate) enum UnavailablePackage {
Expand All @@ -100,8 +112,6 @@ pub(crate) enum UnavailablePackage {
Offline,
/// The package was not found in the registry.
NotFound,
/// The package metadata was not found.
MissingMetadata,
/// The package metadata was found, but could not be parsed.
InvalidMetadata(String),
/// The package has an invalid structure.
Expand All @@ -114,7 +124,6 @@ impl UnavailablePackage {
UnavailablePackage::NoIndex => "not found in the provided package locations",
UnavailablePackage::Offline => "not found in the cache",
UnavailablePackage::NotFound => "not found in the package registry",
UnavailablePackage::MissingMetadata => "not include a `METADATA` file",
UnavailablePackage::InvalidMetadata(_) => "invalid metadata",
UnavailablePackage::InvalidStructure(_) => "an invalid package format",
}
Expand All @@ -125,7 +134,6 @@ impl UnavailablePackage {
UnavailablePackage::NoIndex => format!("was {self}"),
UnavailablePackage::Offline => format!("was {self}"),
UnavailablePackage::NotFound => format!("was {self}"),
UnavailablePackage::MissingMetadata => format!("does {self}"),
UnavailablePackage::InvalidMetadata(_) => format!("has {self}"),
UnavailablePackage::InvalidStructure(_) => format!("has {self}"),
}
Expand All @@ -138,22 +146,20 @@ impl Display for UnavailablePackage {
}
}

/// 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 not found.
MissingMetadata,
/// 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),
/// The source distribution has a `requires-python` requirement that is not met by the installed
/// Python version (and static metadata is not available).
RequiresPython(VersionSpecifiers, Version),
impl From<&MetadataUnavailable> for UnavailablePackage {
fn from(reason: &MetadataUnavailable) -> Self {
match reason {
MetadataUnavailable::Offline => Self::Offline,
MetadataUnavailable::InvalidMetadata(err) => Self::InvalidMetadata(err.to_string()),
MetadataUnavailable::InconsistentMetadata(err) => {
Self::InvalidMetadata(err.to_string())
}
MetadataUnavailable::InvalidStructure(err) => Self::InvalidStructure(err.to_string()),
MetadataUnavailable::RequiresPython(..) => {
unreachable!("`requires-python` is only known upfront for registry distributions")
}
}
}
}

#[derive(Debug, Clone)]
Expand Down
Loading
Loading