-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix(resolver): Report unmatched versions, rather than saying no package #14897
Changes from all commits
4d7913b
31a884e
eb0daec
efe17de
271df34
0160cb7
e48a008
015e980
d2fcf6b
06b5ec5
0ed5c21
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,10 @@ | ||
use std::fmt; | ||
use std::fmt::Write as _; | ||
use std::task::Poll; | ||
|
||
use crate::core::{Dependency, PackageId, Registry, Summary}; | ||
use crate::sources::source::QueryKind; | ||
use crate::sources::IndexSummary; | ||
use crate::util::edit_distance::edit_distance; | ||
use crate::util::{GlobalContext, OptVersionReq, VersionExt}; | ||
use anyhow::Error; | ||
|
@@ -301,10 +303,23 @@ pub(super) fn activation_error( | |
|
||
msg | ||
} else { | ||
// Maybe something is wrong with the available versions | ||
let mut version_candidates = loop { | ||
match registry.query_vec(&new_dep, QueryKind::AlternativeVersions) { | ||
Poll::Ready(Ok(candidates)) => break candidates, | ||
Poll::Ready(Err(e)) => return to_resolve_err(e), | ||
Poll::Pending => match registry.block_until_ready() { | ||
Ok(()) => continue, | ||
Err(e) => return to_resolve_err(e), | ||
}, | ||
} | ||
}; | ||
version_candidates.sort_unstable_by_key(|a| a.as_summary().version().clone()); | ||
|
||
// Maybe the user mistyped the name? Like `dep-thing` when `Dep_Thing` | ||
// was meant. So we try asking the registry for a `fuzzy` search for suggestions. | ||
let candidates = loop { | ||
match registry.query_vec(&new_dep, QueryKind::Alternatives) { | ||
let name_candidates = loop { | ||
match registry.query_vec(&new_dep, QueryKind::AlternativeNames) { | ||
Poll::Ready(Ok(candidates)) => break candidates, | ||
Poll::Ready(Err(e)) => return to_resolve_err(e), | ||
Poll::Pending => match registry.block_until_ready() { | ||
|
@@ -313,58 +328,102 @@ pub(super) fn activation_error( | |
}, | ||
} | ||
}; | ||
|
||
let mut candidates: Vec<_> = candidates.into_iter().map(|s| s.into_summary()).collect(); | ||
|
||
candidates.sort_unstable_by_key(|a| a.name()); | ||
candidates.dedup_by(|a, b| a.name() == b.name()); | ||
let mut candidates: Vec<_> = candidates | ||
let mut name_candidates: Vec<_> = name_candidates | ||
.into_iter() | ||
.map(|s| s.into_summary()) | ||
.collect(); | ||
name_candidates.sort_unstable_by_key(|a| a.name()); | ||
name_candidates.dedup_by(|a, b| a.name() == b.name()); | ||
let mut name_candidates: Vec<_> = name_candidates | ||
.iter() | ||
.filter_map(|n| Some((edit_distance(&*new_dep.package_name(), &*n.name(), 3)?, n))) | ||
.collect(); | ||
candidates.sort_by_key(|o| o.0); | ||
let mut msg: String; | ||
if candidates.is_empty() { | ||
msg = format!("no matching package named `{}` found\n", dep.package_name()); | ||
} else { | ||
msg = format!( | ||
"no matching package found\nsearched package name: `{}`\n", | ||
name_candidates.sort_by_key(|o| o.0); | ||
|
||
let mut msg = String::new(); | ||
if !version_candidates.is_empty() { | ||
let _ = writeln!( | ||
&mut msg, | ||
"no matching versions for `{}` found", | ||
dep.package_name() | ||
); | ||
let mut names = candidates | ||
for candidate in version_candidates { | ||
match candidate { | ||
IndexSummary::Candidate(summary) => { | ||
// HACK: If this was a real candidate, we wouldn't hit this case. | ||
// so it must be a patch which get normalized to being a candidate | ||
let _ = | ||
writeln!(&mut msg, " version {} is unavailable", summary.version()); | ||
} | ||
IndexSummary::Yanked(summary) => { | ||
let _ = writeln!(&mut msg, " version {} is yanked", summary.version()); | ||
} | ||
IndexSummary::Offline(summary) => { | ||
let _ = writeln!(&mut msg, " version {} is not cached", summary.version()); | ||
} | ||
IndexSummary::Unsupported(summary, schema_version) => { | ||
if let Some(rust_version) = summary.rust_version() { | ||
// HACK: technically its unsupported and we shouldn't make assumptions | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For now, the It'll be even worse in a follow up where I'll add "failed to parse but could get basic data". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. Thank you! |
||
// about the entry but this is limited and for diagnostics purposes | ||
let _ = writeln!( | ||
&mut msg, | ||
" version {} requires cargo {}", | ||
summary.version(), | ||
rust_version | ||
); | ||
} else { | ||
let _ = writeln!( | ||
&mut msg, | ||
" version {} requires a Cargo version that supports index version {}", | ||
summary.version(), | ||
schema_version | ||
); | ||
} | ||
} | ||
} | ||
} | ||
} else if !name_candidates.is_empty() { | ||
let _ = writeln!(&mut msg, "no matching package found",); | ||
let _ = writeln!(&mut msg, "searched package name: `{}`", dep.package_name()); | ||
let mut names = name_candidates | ||
.iter() | ||
.take(3) | ||
.map(|c| c.1.name().as_str()) | ||
.collect::<Vec<_>>(); | ||
|
||
if candidates.len() > 3 { | ||
if name_candidates.len() > 3 { | ||
names.push("..."); | ||
} | ||
// Vertically align first suggestion with missing crate name | ||
// so a typo jumps out at you. | ||
msg.push_str("perhaps you meant: "); | ||
msg.push_str(&names.iter().enumerate().fold( | ||
String::default(), | ||
|acc, (i, el)| match i { | ||
let suggestions = names | ||
.iter() | ||
.enumerate() | ||
.fold(String::default(), |acc, (i, el)| match i { | ||
0 => acc + el, | ||
i if names.len() - 1 == i && candidates.len() <= 3 => acc + " or " + el, | ||
i if names.len() - 1 == i && name_candidates.len() <= 3 => acc + " or " + el, | ||
_ => acc + ", " + el, | ||
}, | ||
)); | ||
msg.push('\n'); | ||
}); | ||
let _ = writeln!(&mut msg, "perhaps you meant: {suggestions}"); | ||
} else { | ||
let _ = writeln!( | ||
&mut msg, | ||
"no matching package named `{}` found", | ||
dep.package_name() | ||
); | ||
} | ||
|
||
let mut location_searched_msg = registry.describe_source(dep.source_id()); | ||
if location_searched_msg.is_empty() { | ||
location_searched_msg = format!("{}", dep.source_id()); | ||
} | ||
|
||
msg.push_str(&format!("location searched: {}\n", location_searched_msg)); | ||
msg.push_str("required by "); | ||
msg.push_str(&describe_path_in_context( | ||
resolver_ctx, | ||
&parent.package_id(), | ||
)); | ||
let _ = writeln!(&mut msg, "location searched: {}", location_searched_msg); | ||
let _ = write!( | ||
&mut msg, | ||
"required by {}", | ||
describe_path_in_context(resolver_ctx, &parent.package_id()), | ||
); | ||
|
||
msg | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how this happens. It it possible to add a unit test to cover this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how to hit this. This was originally an assert but I hit it because we switched everything to
Candidate
. That should be fixed but I figured a vague error is better for users than a crash in case I missed something or things change in the future.