Skip to content

Use generic pubgrub incompatibility reason #3335

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.

Sign up for GitHub

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

Merged
konstin merged 6 commits into from
May 8, 2024
Merged

Use generic pubgrub incompatibility reason #3335

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f93f342
Use generic pubgrub incompatibility reason
konstin May 3, 2024
d663d8f
clippy
konstin May 3, 2024
a56a2d4
Remove plural from unavailable package
konstin May 7, 2024
d0adff0
Rustfmt
konstin May 7, 2024
af262a8
Update pubgrub
konstin May 7, 2024
b491904
Update pubgrub
konstin May 8, 2024
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
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ path-absolutize = { version = "3.1.1" }
pathdiff = { version = "0.2.1" }
petgraph = { version = "0.6.4" }
platform-info = { version = "2.0.2" }
pubgrub = { git = "https://github.com/astral-sh/pubgrub", rev = "c26e485213e39582c6f2e4d45c0328422670e7a7" }
pubgrub = { git = "https://github.com/astral-sh/pubgrub", rev = "70e4ad93edc17601eaeee167e04b5bbf705b88b5" }
pyo3 = { version = "0.21.0" }
pyo3-log = { version = "0.10.0" }
rand = { version = "0.8.5" }
Expand Down
38 changes: 19 additions & 19 deletions crates/distribution-types/src/prioritized_distribution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,59 +53,59 @@ impl Display for IncompatibleDist {
match self {
Self::Wheel(incompatibility) => match incompatibility {
IncompatibleWheel::NoBinary => {
f.write_str("no source distribution is available and using wheels is disabled")
f.write_str("has no available source distribution and using wheels is disabled")
}
IncompatibleWheel::Tag(tag) => match tag {
IncompatibleTag::Invalid => {
f.write_str("no wheels are available with valid tags")
}
IncompatibleTag::Python => {
f.write_str("no wheels are available with a matching Python implementation")
f.write_str("has no wheels are available with valid tags")
}
IncompatibleTag::Python => f.write_str(
"has no wheels are available with a matching Python implementation",
),
IncompatibleTag::Abi => {
f.write_str("no wheels are available with a matching Python ABI")
f.write_str("has no wheels are available with a matching Python ABI")
}
IncompatibleTag::Platform => {
f.write_str("no wheels are available with a matching platform")
f.write_str("has no wheels are available with a matching platform")
}
},
IncompatibleWheel::Yanked(yanked) => match yanked {
Yanked::Bool(_) => f.write_str("it was yanked"),
Yanked::Bool(_) => f.write_str("was yanked"),
Yanked::Reason(reason) => write!(
f,
"it was yanked (reason: {})",
"was yanked (reason: {})",
reason.trim().trim_end_matches('.')
),
},
IncompatibleWheel::ExcludeNewer(ts) => match ts {
Some(_) => f.write_str("it was published after the exclude newer time"),
None => f.write_str("it has no publish time"),
Some(_) => f.write_str("was published after the exclude newer time"),
None => f.write_str("has no publish time"),
},
IncompatibleWheel::RequiresPython(python) => {
write!(f, "it requires at python {python}")
write!(f, "requires at python {python}")
}
},
Self::Source(incompatibility) => match incompatibility {
IncompatibleSource::NoBuild => {
f.write_str("no wheels are usable and building from source is disabled")
f.write_str("has no usable wheels and building from source is disabled")
}
IncompatibleSource::Yanked(yanked) => match yanked {
Yanked::Bool(_) => f.write_str("it was yanked"),
Yanked::Bool(_) => f.write_str("was yanked"),
Yanked::Reason(reason) => write!(
f,
"it was yanked (reason: {})",
"was yanked (reason: {})",
reason.trim().trim_end_matches('.')
),
},
IncompatibleSource::ExcludeNewer(ts) => match ts {
Some(_) => f.write_str("it was published after the exclude newer time"),
None => f.write_str("it has no publish time"),
Some(_) => f.write_str("was published after the exclude newer time"),
None => f.write_str("has no publish time"),
},
IncompatibleSource::RequiresPython(python) => {
write!(f, "it requires python {python}")
write!(f, "requires python {python}")
}
},
Self::Unavailable => f.write_str("no distributions are available"),
Self::Unavailable => f.write_str("has no available distributions"),
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion crates/uv-resolver/src/dependency_provider.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use pubgrub::solver::{Dependencies, DependencyProvider};
use pep440_rs::Version;

use crate::pubgrub::{PubGrubPackage, PubGrubPriority};
use crate::resolver::UnavailableReason;

/// We don't use a dependency provider, we interact with state directly, but we still need this one
/// for type
Expand All @@ -15,6 +16,8 @@ impl DependencyProvider for UvDependencyProvider {
type P = PubGrubPackage;
type V = Version;
type VS = Range<Version>;
type M = UnavailableReason;

fn prioritize(&self, _package: &Self::P, _range: &Self::VS) -> Self::Priority {
unimplemented!()
}
Expand All @@ -34,7 +37,7 @@ impl DependencyProvider for UvDependencyProvider {
&self,
_package: &Self::P,
_version: &Self::V,
) -> Result<Dependencies<Vec<(Self::P, Self::VS)>>, Self::Err> {
) -> Result<Dependencies<Vec<(Self::P, Self::VS)>, Self::M>, Self::Err> {
unimplemented!()
}
}
9 changes: 6 additions & 3 deletions crates/uv-resolver/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ use crate::dependency_provider::UvDependencyProvider;
use crate::pubgrub::{PubGrubPackage, PubGrubPython, PubGrubReportFormatter};
use crate::python_requirement::PythonRequirement;
use crate::resolver::{
IncompletePackage, SharedMap, SharedSet, UnavailablePackage, VersionsResponse,
IncompletePackage, SharedMap, SharedSet, UnavailablePackage, UnavailableReason,
VersionsResponse,
};

#[derive(Debug, thiserror::Error)]
Expand Down Expand Up @@ -119,7 +120,9 @@ impl<T> From<tokio::sync::mpsc::error::SendError<T>> for ResolveError {

/// Given a [`DerivationTree`], collapse any [`External::FromDependencyOf`] incompatibilities
/// wrap an [`PubGrubPackage::Extra`] package.
fn collapse_extra_proxies(derivation_tree: &mut DerivationTree<PubGrubPackage, Range<Version>>) {
fn collapse_extra_proxies(
derivation_tree: &mut DerivationTree<PubGrubPackage, Range<Version>, UnavailableReason>,
) {
match derivation_tree {
DerivationTree::External(_) => {}
DerivationTree::Derived(derived) => {
Expand Down Expand Up @@ -193,7 +196,7 @@ impl From<pubgrub::error::PubGrubError<UvDependencyProvider>> for ResolveError {
/// A wrapper around [`pubgrub::error::PubGrubError::NoSolution`] that displays a resolution failure report.
#[derive(Debug)]
pub struct NoSolutionError {
derivation_tree: DerivationTree<PubGrubPackage, Range<Version>>,
derivation_tree: DerivationTree<PubGrubPackage, Range<Version>, UnavailableReason>,
available_versions: IndexMap<PubGrubPackage, BTreeSet<Version>>,
selector: Option<CandidateSelector>,
python_requirement: Option<PythonRequirement>,
Expand Down
70 changes: 37 additions & 33 deletions crates/uv-resolver/src/pubgrub/report.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use uv_normalize::PackageName;

use crate::candidate_selector::CandidateSelector;
use crate::python_requirement::PythonRequirement;
use crate::resolver::{IncompletePackage, UnavailablePackage};
use crate::resolver::{IncompletePackage, UnavailablePackage, UnavailableReason};

use super::PubGrubPackage;

Expand All @@ -30,15 +30,20 @@ pub(crate) struct PubGrubReportFormatter<'a> {
pub(crate) python_requirement: Option<&'a PythonRequirement>,
}

impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<'_> {
impl ReportFormatter<PubGrubPackage, Range<Version>, UnavailableReason>
for PubGrubReportFormatter<'_>
{
type Output = String;

fn format_external(&self, external: &External<PubGrubPackage, Range<Version>>) -> Self::Output {
fn format_external(
&self,
external: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
) -> Self::Output {
match external {
External::NotRoot(package, version) => {
format!("we are solving dependencies of {package} {version}")
}
External::NoVersions(package, set, reason) => {
External::NoVersions(package, set) => {
if matches!(package, PubGrubPackage::Python(_)) {
if let Some(python) = self.python_requirement {
if python.target() == python.installed() {
Expand Down Expand Up @@ -79,16 +84,6 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
}
let set = self.simplify_set(set, package);

// Check for a reason
if let Some(reason) = reason {
let formatted = if set.as_ref() == &Range::full() {
format!("{package} {reason}")
} else {
format!("{package}{set} {reason}")
};
return formatted;
}

if set.as_ref() == &Range::full() {
format!("there are no versions of {package}")
} else if set.as_singleton().is_some() {
Expand All @@ -112,17 +107,26 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
}
}
}
External::Unavailable(package, set, reason) => match package {
External::Custom(package, set, reason) => match package {
PubGrubPackage::Root(Some(name)) => {
format!("{name} cannot be used because {reason}")
}
PubGrubPackage::Root(None) => {
format!("your requirements cannot be used because {reason}")
}
_ => format!(
"{}is unusable because {reason}",
Padded::new("", &PackageRange::compatibility(package, set), " ")
),
_ => match reason {
UnavailableReason::Package(reason) => {
// While there may be a term attached, this error applies to the entire
// package, so we show it for the entire package
format!("{}{reason}", Padded::new("", &package, " "))
}
UnavailableReason::Version(reason) => {
format!(
"{}{reason}",
Padded::new("", &PackageRange::compatibility(package, set), " ")
)
}
},
},
External::FromDependencyOf(package, package_set, dependency, dependency_set) => {
let package_set = self.simplify_set(package_set, package);
Expand Down Expand Up @@ -198,8 +202,8 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
/// Simplest case, we just combine two external incompatibilities.
fn explain_both_external(
&self,
external1: &External<PubGrubPackage, Range<Version>>,
external2: &External<PubGrubPackage, Range<Version>>,
external1: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
external2: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
current_terms: &Map<PubGrubPackage, Term<Range<Version>>>,
) -> String {
let external = self.format_both_external(external1, external2);
Expand All @@ -216,9 +220,9 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
fn explain_both_ref(
&self,
ref_id1: usize,
derived1: &Derived<PubGrubPackage, Range<Version>>,
derived1: &Derived<PubGrubPackage, Range<Version>, UnavailableReason>,
ref_id2: usize,
derived2: &Derived<PubGrubPackage, Range<Version>>,
derived2: &Derived<PubGrubPackage, Range<Version>, UnavailableReason>,
current_terms: &Map<PubGrubPackage, Term<Range<Version>>>,
) -> String {
// TODO: order should be chosen to make it more logical.
Expand All @@ -243,8 +247,8 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
fn explain_ref_and_external(
&self,
ref_id: usize,
derived: &Derived<PubGrubPackage, Range<Version>>,
external: &External<PubGrubPackage, Range<Version>>,
derived: &Derived<PubGrubPackage, Range<Version>, UnavailableReason>,
external: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
current_terms: &Map<PubGrubPackage, Term<Range<Version>>>,
) -> String {
// TODO: order should be chosen to make it more logical.
Expand All @@ -265,7 +269,7 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
/// Add an external cause to the chain of explanations.
fn and_explain_external(
&self,
external: &External<PubGrubPackage, Range<Version>>,
external: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
current_terms: &Map<PubGrubPackage, Term<Range<Version>>>,
) -> String {
let external = self.format_external(external);
Expand All @@ -282,7 +286,7 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
fn and_explain_ref(
&self,
ref_id: usize,
derived: &Derived<PubGrubPackage, Range<Version>>,
derived: &Derived<PubGrubPackage, Range<Version>, UnavailableReason>,
current_terms: &Map<PubGrubPackage, Term<Range<Version>>>,
) -> String {
let derived = self.format_terms(&derived.terms);
Expand All @@ -299,8 +303,8 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
/// Add an already explained incompat to the chain of explanations.
fn and_explain_prior_and_external(
&self,
prior_external: &External<PubGrubPackage, Range<Version>>,
external: &External<PubGrubPackage, Range<Version>>,
prior_external: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
external: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
current_terms: &Map<PubGrubPackage, Term<Range<Version>>>,
) -> String {
let external = self.format_both_external(prior_external, external);
Expand All @@ -318,8 +322,8 @@ impl PubGrubReportFormatter<'_> {
/// Format two external incompatibilities, combining them if possible.
fn format_both_external(
&self,
external1: &External<PubGrubPackage, Range<Version>>,
external2: &External<PubGrubPackage, Range<Version>>,
external1: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
external2: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
) -> String {
match (external1, external2) {
(
Expand Down Expand Up @@ -387,7 +391,7 @@ impl PubGrubReportFormatter<'_> {
/// their requirements.
pub(crate) fn hints(
&self,
derivation_tree: &DerivationTree<PubGrubPackage, Range<Version>>,
derivation_tree: &DerivationTree<PubGrubPackage, Range<Version>, UnavailableReason>,
selector: &Option<CandidateSelector>,
index_locations: &Option<IndexLocations>,
unavailable_packages: &FxHashMap<PackageName, UnavailablePackage>,
Expand All @@ -404,7 +408,7 @@ impl PubGrubReportFormatter<'_> {
let mut hints = IndexSet::default();
match derivation_tree {
DerivationTree::External(external) => match external {
External::Unavailable(package, set, _) | External::NoVersions(package, set, _) => {
External::Custom(package, set, _) | External::NoVersions(package, set) => {
// Check for no versions due to pre-release options
if let Some(selector) = selector {
let any_prerelease = set.iter().any(|(start, end)| {
Expand Down
Loading