Skip to content

Commit

Permalink
fix(resolver): Don't report all versions as rejected (rust-lang#14921)
Browse files Browse the repository at this point in the history
### What does this PR try to resolve?

When I copy/pasted the alternative-names code in rust-lang#14897, I didn't notice
that the
wildcard dependency was used. This includes a rename of the variable to
make it stand out more.

I also renamed `AlternativeVersions` because there is another check we
do that better matches that name, so I renamed it to `RejectedVersions`.

### How should we test and review this PR?

This does not have a test yet because overly-constrictive dep specs have
higher precedence atm (which is what I was working on when I found
this).

### Additional information
  • Loading branch information
weihanglo authored Dec 11, 2024
2 parents ad5b493 + 3aab145 commit 7ec959e
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 13 deletions.
2 changes: 1 addition & 1 deletion crates/resolver-tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub fn resolve_with_global_context_raw(
for summary in self.list.iter() {
let matched = match kind {
QueryKind::Exact => dep.matches(summary),
QueryKind::AlternativeVersions => dep.matches(summary),
QueryKind::RejectedVersions => dep.matches(summary),
QueryKind::AlternativeNames => true,
QueryKind::Normalized => true,
};
Expand Down
12 changes: 6 additions & 6 deletions src/cargo/core/resolver/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,11 @@ pub(super) fn activation_error(
// Maybe the user mistyped the ver_req? Like `dep="2"` when `dep="0.2"`
// was meant. So we re-query the registry with `dep="*"` so we can
// list a few versions that were actually found.
let mut new_dep = dep.clone();
new_dep.set_version_req(OptVersionReq::Any);
let mut wild_dep = dep.clone();
wild_dep.set_version_req(OptVersionReq::Any);

let candidates = loop {
match registry.query_vec(&new_dep, QueryKind::Exact) {
match registry.query_vec(&wild_dep, QueryKind::Exact) {
Poll::Ready(Ok(candidates)) => break candidates,
Poll::Ready(Err(e)) => return to_resolve_err(e),
Poll::Pending => match registry.block_until_ready() {
Expand Down Expand Up @@ -305,7 +305,7 @@ pub(super) fn activation_error(
} else {
// Maybe something is wrong with the available versions
let mut version_candidates = loop {
match registry.query_vec(&new_dep, QueryKind::AlternativeVersions) {
match registry.query_vec(&dep, QueryKind::RejectedVersions) {
Poll::Ready(Ok(candidates)) => break candidates,
Poll::Ready(Err(e)) => return to_resolve_err(e),
Poll::Pending => match registry.block_until_ready() {
Expand All @@ -319,7 +319,7 @@ pub(super) fn activation_error(
// 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 name_candidates = loop {
match registry.query_vec(&new_dep, QueryKind::AlternativeNames) {
match registry.query_vec(&wild_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() {
Expand All @@ -336,7 +336,7 @@ pub(super) fn activation_error(
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)))
.filter_map(|n| Some((edit_distance(&*wild_dep.package_name(), &*n.name(), 3)?, n)))
.collect();
name_candidates.sort_by_key(|o| o.0);

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/sources/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ impl<'gctx> Source for DirectorySource<'gctx> {
}
let packages = self.packages.values().map(|p| &p.0);
let matches = packages.filter(|pkg| match kind {
QueryKind::Exact | QueryKind::AlternativeVersions => dep.matches(pkg.summary()),
QueryKind::Exact | QueryKind::RejectedVersions => dep.matches(pkg.summary()),
QueryKind::AlternativeNames => true,
QueryKind::Normalized => dep.matches(pkg.summary()),
});
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/sources/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ impl<'gctx> Source for PathSource<'gctx> {
self.load()?;
if let Some(s) = self.package.as_ref().map(|p| p.summary()) {
let matched = match kind {
QueryKind::Exact | QueryKind::AlternativeVersions => dep.matches(s),
QueryKind::Exact | QueryKind::RejectedVersions => dep.matches(s),
QueryKind::AlternativeNames => true,
QueryKind::Normalized => dep.matches(s),
};
Expand Down Expand Up @@ -332,7 +332,7 @@ impl<'gctx> Source for RecursivePathSource<'gctx> {
.map(|p| p.summary())
{
let matched = match kind {
QueryKind::Exact | QueryKind::AlternativeVersions => dep.matches(s),
QueryKind::Exact | QueryKind::RejectedVersions => dep.matches(s),
QueryKind::AlternativeNames => true,
QueryKind::Normalized => dep.matches(s),
};
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/sources/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ impl<'gctx> Source for RegistrySource<'gctx> {
.index
.query_inner(dep.package_name(), &req, &mut *self.ops, &mut |s| {
let matched = match kind {
QueryKind::Exact | QueryKind::AlternativeVersions => {
QueryKind::Exact | QueryKind::RejectedVersions => {
if req.is_precise() && self.gctx.cli_unstable().unstable_options {
dep.matches_prerelease(s.as_summary())
} else {
Expand All @@ -816,7 +816,7 @@ impl<'gctx> Source for RegistrySource<'gctx> {
// leak through if they're in a whitelist (aka if they were
// previously in `Cargo.lock`
match s {
s @ _ if kind == QueryKind::AlternativeVersions => callback(s),
s @ _ if kind == QueryKind::RejectedVersions => callback(s),
s @ IndexSummary::Candidate(_) => callback(s),
s @ IndexSummary::Yanked(_) => {
if self.yanked_whitelist.contains(&s.package_id()) {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/sources/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ pub enum QueryKind {
///
/// Path/Git sources may return all dependencies that are at that URI,
/// whereas an `Registry` source may return dependencies that are yanked or invalid.
AlternativeVersions,
RejectedVersions,
/// A query for packages close to the given dependency requirement.
///
/// Each source gets to define what `close` means for it.
Expand Down

0 comments on commit 7ec959e

Please sign in to comment.