Skip to content

Commit 1ad6aa8

Browse files
konstinkonstin
authored
Use generic pubgrub incompatibility reason (#3335)
Pubgrub got a new feature where all unavailability is a custom, instead of the reasonless `UnavailableDependencies` and our custom `String` type previously (pubgrub-rs/pubgrub#208). This PR introduces a `UnavailableReason` that tracks either an entire version being unusable, or a specific version. The error messages now also track this difference properly. The pubgrub commit is our main rebased onto the merged pubgrub-rs/pubgrub#208, i'll push `konsti/main-rebase-generic-reason` to `main` after checking for rebase problems.
1 parent bd7860d commit 1ad6aa8

File tree

11 files changed

+215
-165
lines changed

11 files changed

+215
-165
lines changed

‎Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ path-absolutize = { version = "3.1.1" }
101101
pathdiff = { version = "0.2.1" }
102102
petgraph = { version = "0.6.4" }
103103
platform-info = { version = "2.0.2" }
104-
pubgrub = { git = "https://github.com/astral-sh/pubgrub", rev = "c26e485213e39582c6f2e4d45c0328422670e7a7" }
104+
pubgrub = { git = "https://github.com/astral-sh/pubgrub", rev = "0e684a874c9fb8f74738cd8875524c80e3d4820b" }
105105
pyo3 = { version = "0.21.0" }
106106
pyo3-log = { version = "0.10.0" }
107107
rand = { version = "0.8.5" }

‎crates/distribution-types/src/prioritized_distribution.rs

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -53,59 +53,59 @@ impl Display for IncompatibleDist {
5353
match self {
5454
Self::Wheel(incompatibility) => match incompatibility {
5555
IncompatibleWheel::NoBinary => {
56-
f.write_str("no source distribution is available and using wheels is disabled")
56+
f.write_str("has no available source distribution and using wheels is disabled")
5757
}
5858
IncompatibleWheel::Tag(tag) => match tag {
5959
IncompatibleTag::Invalid => {
60-
f.write_str("no wheels are available with valid tags")
61-
}
62-
IncompatibleTag::Python => {
63-
f.write_str("no wheels are available with a matching Python implementation")
60+
f.write_str("has no wheels are available with valid tags")
6461
}
62+
IncompatibleTag::Python => f.write_str(
63+
"has no wheels are available with a matching Python implementation",
64+
),
6565
IncompatibleTag::Abi => {
66-
f.write_str("no wheels are available with a matching Python ABI")
66+
f.write_str("has no wheels are available with a matching Python ABI")
6767
}
6868
IncompatibleTag::Platform => {
69-
f.write_str("no wheels are available with a matching platform")
69+
f.write_str("has no wheels are available with a matching platform")
7070
}
7171
},
7272
IncompatibleWheel::Yanked(yanked) => match yanked {
73-
Yanked::Bool(_) => f.write_str("it was yanked"),
73+
Yanked::Bool(_) => f.write_str("was yanked"),
7474
Yanked::Reason(reason) => write!(
7575
f,
76-
"it was yanked (reason: {})",
76+
"was yanked (reason: {})",
7777
reason.trim().trim_end_matches('.')
7878
),
7979
},
8080
IncompatibleWheel::ExcludeNewer(ts) => match ts {
81-
Some(_) => f.write_str("it was published after the exclude newer time"),
82-
None => f.write_str("it has no publish time"),
81+
Some(_) => f.write_str("was published after the exclude newer time"),
82+
None => f.write_str("has no publish time"),
8383
},
8484
IncompatibleWheel::RequiresPython(python) => {
85-
write!(f, "it requires at python {python}")
85+
write!(f, "requires at python {python}")
8686
}
8787
},
8888
Self::Source(incompatibility) => match incompatibility {
8989
IncompatibleSource::NoBuild => {
90-
f.write_str("no wheels are usable and building from source is disabled")
90+
f.write_str("has no usable wheels and building from source is disabled")
9191
}
9292
IncompatibleSource::Yanked(yanked) => match yanked {
93-
Yanked::Bool(_) => f.write_str("it was yanked"),
93+
Yanked::Bool(_) => f.write_str("was yanked"),
9494
Yanked::Reason(reason) => write!(
9595
f,
96-
"it was yanked (reason: {})",
96+
"was yanked (reason: {})",
9797
reason.trim().trim_end_matches('.')
9898
),
9999
},
100100
IncompatibleSource::ExcludeNewer(ts) => match ts {
101-
Some(_) => f.write_str("it was published after the exclude newer time"),
102-
None => f.write_str("it has no publish time"),
101+
Some(_) => f.write_str("was published after the exclude newer time"),
102+
None => f.write_str("has no publish time"),
103103
},
104104
IncompatibleSource::RequiresPython(python) => {
105-
write!(f, "it requires python {python}")
105+
write!(f, "requires python {python}")
106106
}
107107
},
108-
Self::Unavailable => f.write_str("no distributions are available"),
108+
Self::Unavailable => f.write_str("has no available distributions"),
109109
}
110110
}
111111
}

‎crates/uv-resolver/src/dependency_provider.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use pubgrub::solver::{Dependencies, DependencyProvider};
66
use pep440_rs::Version;
77

88
use crate::pubgrub::{PubGrubPackage, PubGrubPriority};
9+
use crate::resolver::UnavailableReason;
910

1011
/// We don't use a dependency provider, we interact with state directly, but we still need this one
1112
/// for type
@@ -15,6 +16,8 @@ impl DependencyProvider for UvDependencyProvider {
1516
type P = PubGrubPackage;
1617
type V = Version;
1718
type VS = Range<Version>;
19+
type M = UnavailableReason;
20+
1821
fn prioritize(&self, _package: &Self::P, _range: &Self::VS) -> Self::Priority {
1922
unimplemented!()
2023
}
@@ -34,7 +37,7 @@ impl DependencyProvider for UvDependencyProvider {
3437
&self,
3538
_package: &Self::P,
3639
_version: &Self::V,
37-
) -> Result<Dependencies<Vec<(Self::P, Self::VS)>>, Self::Err> {
40+
) -> Result<Dependencies<Vec<(Self::P, Self::VS)>, Self::M>, Self::Err> {
3841
unimplemented!()
3942
}
4043
}

‎crates/uv-resolver/src/error.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ use crate::dependency_provider::UvDependencyProvider;
2323
use crate::pubgrub::{PubGrubPackage, PubGrubPython, PubGrubReportFormatter};
2424
use crate::python_requirement::PythonRequirement;
2525
use crate::resolver::{
26-
IncompletePackage, SharedMap, SharedSet, UnavailablePackage, VersionsResponse,
26+
IncompletePackage, SharedMap, SharedSet, UnavailablePackage, UnavailableReason,
27+
VersionsResponse,
2728
};
2829

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

120121
/// Given a [`DerivationTree`], collapse any [`External::FromDependencyOf`] incompatibilities
121122
/// wrap an [`PubGrubPackage::Extra`] package.
122-
fn collapse_extra_proxies(derivation_tree: &mut DerivationTree<PubGrubPackage, Range<Version>>) {
123+
fn collapse_extra_proxies(
124+
derivation_tree: &mut DerivationTree<PubGrubPackage, Range<Version>, UnavailableReason>,
125+
) {
123126
match derivation_tree {
124127
DerivationTree::External(_) => {}
125128
DerivationTree::Derived(derived) => {
@@ -193,7 +196,7 @@ impl From<pubgrub::error::PubGrubError<UvDependencyProvider>> for ResolveError {
193196
/// A wrapper around [`pubgrub::error::PubGrubError::NoSolution`] that displays a resolution failure report.
194197
#[derive(Debug)]
195198
pub struct NoSolutionError {
196-
derivation_tree: DerivationTree<PubGrubPackage, Range<Version>>,
199+
derivation_tree: DerivationTree<PubGrubPackage, Range<Version>, UnavailableReason>,
197200
available_versions: IndexMap<PubGrubPackage, BTreeSet<Version>>,
198201
selector: Option<CandidateSelector>,
199202
python_requirement: Option<PythonRequirement>,

‎crates/uv-resolver/src/pubgrub/report.rs

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use uv_normalize::PackageName;
1717

1818
use crate::candidate_selector::CandidateSelector;
1919
use crate::python_requirement::PythonRequirement;
20-
use crate::resolver::{IncompletePackage, UnavailablePackage};
20+
use crate::resolver::{IncompletePackage, UnavailablePackage, UnavailableReason};
2121

2222
use super::PubGrubPackage;
2323

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

33-
impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<'_> {
33+
impl ReportFormatter<PubGrubPackage, Range<Version>, UnavailableReason>
34+
for PubGrubReportFormatter<'_>
35+
{
3436
type Output = String;
3537

36-
fn format_external(&self, external: &External<PubGrubPackage, Range<Version>>) -> Self::Output {
38+
fn format_external(
39+
&self,
40+
external: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
41+
) -> Self::Output {
3742
match external {
3843
External::NotRoot(package, version) => {
3944
format!("we are solving dependencies of {package} {version}")
4045
}
41-
External::NoVersions(package, set, reason) => {
46+
External::NoVersions(package, set) => {
4247
if matches!(package, PubGrubPackage::Python(_)) {
4348
if let Some(python) = self.python_requirement {
4449
if python.target() == python.installed() {
@@ -79,16 +84,6 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
7984
}
8085
let set = self.simplify_set(set, package);
8186

82-
// Check for a reason
83-
if let Some(reason) = reason {
84-
let formatted = if set.as_ref() == &Range::full() {
85-
format!("{package} {reason}")
86-
} else {
87-
format!("{package}{set} {reason}")
88-
};
89-
return formatted;
90-
}
91-
9287
if set.as_ref() == &Range::full() {
9388
format!("there are no versions of {package}")
9489
} else if set.as_singleton().is_some() {
@@ -112,17 +107,26 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
112107
}
113108
}
114109
}
115-
External::Unavailable(package, set, reason) => match package {
110+
External::Custom(package, set, reason) => match package {
116111
PubGrubPackage::Root(Some(name)) => {
117112
format!("{name} cannot be used because {reason}")
118113
}
119114
PubGrubPackage::Root(None) => {
120115
format!("your requirements cannot be used because {reason}")
121116
}
122-
_ => format!(
123-
"{}is unusable because {reason}",
124-
Padded::new("", &PackageRange::compatibility(package, set), " ")
125-
),
117+
_ => match reason {
118+
UnavailableReason::Package(reason) => {
119+
// While there may be a term attached, this error applies to the entire
120+
// package, so we show it for the entire package
121+
format!("{}{reason}", Padded::new("", &package, " "))
122+
}
123+
UnavailableReason::Version(reason) => {
124+
format!(
125+
"{}{reason}",
126+
Padded::new("", &PackageRange::compatibility(package, set), " ")
127+
)
128+
}
129+
},
126130
},
127131
External::FromDependencyOf(package, package_set, dependency, dependency_set) => {
128132
let package_set = self.simplify_set(package_set, package);
@@ -198,8 +202,8 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
198202
/// Simplest case, we just combine two external incompatibilities.
199203
fn explain_both_external(
200204
&self,
201-
external1: &External<PubGrubPackage, Range<Version>>,
202-
external2: &External<PubGrubPackage, Range<Version>>,
205+
external1: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
206+
external2: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
203207
current_terms: &Map<PubGrubPackage, Term<Range<Version>>>,
204208
) -> String {
205209
let external = self.format_both_external(external1, external2);
@@ -216,9 +220,9 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
216220
fn explain_both_ref(
217221
&self,
218222
ref_id1: usize,
219-
derived1: &Derived<PubGrubPackage, Range<Version>>,
223+
derived1: &Derived<PubGrubPackage, Range<Version>, UnavailableReason>,
220224
ref_id2: usize,
221-
derived2: &Derived<PubGrubPackage, Range<Version>>,
225+
derived2: &Derived<PubGrubPackage, Range<Version>, UnavailableReason>,
222226
current_terms: &Map<PubGrubPackage, Term<Range<Version>>>,
223227
) -> String {
224228
// TODO: order should be chosen to make it more logical.
@@ -243,8 +247,8 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
243247
fn explain_ref_and_external(
244248
&self,
245249
ref_id: usize,
246-
derived: &Derived<PubGrubPackage, Range<Version>>,
247-
external: &External<PubGrubPackage, Range<Version>>,
250+
derived: &Derived<PubGrubPackage, Range<Version>, UnavailableReason>,
251+
external: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
248252
current_terms: &Map<PubGrubPackage, Term<Range<Version>>>,
249253
) -> String {
250254
// TODO: order should be chosen to make it more logical.
@@ -265,7 +269,7 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
265269
/// Add an external cause to the chain of explanations.
266270
fn and_explain_external(
267271
&self,
268-
external: &External<PubGrubPackage, Range<Version>>,
272+
external: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
269273
current_terms: &Map<PubGrubPackage, Term<Range<Version>>>,
270274
) -> String {
271275
let external = self.format_external(external);
@@ -282,7 +286,7 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
282286
fn and_explain_ref(
283287
&self,
284288
ref_id: usize,
285-
derived: &Derived<PubGrubPackage, Range<Version>>,
289+
derived: &Derived<PubGrubPackage, Range<Version>, UnavailableReason>,
286290
current_terms: &Map<PubGrubPackage, Term<Range<Version>>>,
287291
) -> String {
288292
let derived = self.format_terms(&derived.terms);
@@ -299,8 +303,8 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
299303
/// Add an already explained incompat to the chain of explanations.
300304
fn and_explain_prior_and_external(
301305
&self,
302-
prior_external: &External<PubGrubPackage, Range<Version>>,
303-
external: &External<PubGrubPackage, Range<Version>>,
306+
prior_external: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
307+
external: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
304308
current_terms: &Map<PubGrubPackage, Term<Range<Version>>>,
305309
) -> String {
306310
let external = self.format_both_external(prior_external, external);
@@ -318,8 +322,8 @@ impl PubGrubReportFormatter<'_> {
318322
/// Format two external incompatibilities, combining them if possible.
319323
fn format_both_external(
320324
&self,
321-
external1: &External<PubGrubPackage, Range<Version>>,
322-
external2: &External<PubGrubPackage, Range<Version>>,
325+
external1: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
326+
external2: &External<PubGrubPackage, Range<Version>, UnavailableReason>,
323327
) -> String {
324328
match (external1, external2) {
325329
(
@@ -387,7 +391,7 @@ impl PubGrubReportFormatter<'_> {
387391
/// their requirements.
388392
pub(crate) fn hints(
389393
&self,
390-
derivation_tree: &DerivationTree<PubGrubPackage, Range<Version>>,
394+
derivation_tree: &DerivationTree<PubGrubPackage, Range<Version>, UnavailableReason>,
391395
selector: &Option<CandidateSelector>,
392396
index_locations: &Option<IndexLocations>,
393397
unavailable_packages: &FxHashMap<PackageName, UnavailablePackage>,
@@ -404,7 +408,7 @@ impl PubGrubReportFormatter<'_> {
404408
let mut hints = IndexSet::default();
405409
match derivation_tree {
406410
DerivationTree::External(external) => match external {
407-
External::Unavailable(package, set, _) | External::NoVersions(package, set, _) => {
411+
External::Custom(package, set, _) | External::NoVersions(package, set) => {
408412
// Check for no versions due to pre-release options
409413
if let Some(selector) = selector {
410414
let any_prerelease = set.iter().any(|(start, end)| {

0 commit comments

Comments
 (0)