From d7cd78f0c562016950473927caf17564d63038af Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 11 Dec 2024 11:59:08 -0600 Subject: [PATCH 1/9] refactor(resolver): Pull out alternative look up logic --- src/cargo/core/resolver/errors.rs | 168 +++++++++++++++++++----------- 1 file changed, 106 insertions(+), 62 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index a44485adaa9..df1979821f2 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -6,6 +6,7 @@ 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::errors::CargoResult; use crate::util::{GlobalContext, OptVersionReq, VersionExt}; use anyhow::Error; @@ -218,29 +219,11 @@ pub(super) fn activation_error( // We didn't actually find any candidates, so we need to // give an error message that nothing was found. - // - // 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 wild_dep = dep.clone(); - wild_dep.set_version_req(OptVersionReq::Any); - - let candidates = loop { - 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() { - Ok(()) => continue, - Err(e) => return to_resolve_err(e), - }, - } - }; - - let mut candidates: Vec<_> = candidates.into_iter().map(|s| s.into_summary()).collect(); - - candidates.sort_unstable_by(|a, b| b.version().cmp(a.version())); - - let mut msg = if !candidates.is_empty() { + let mut msg = if let Some(candidates) = alt_versions(registry, dep) { + let candidates = match candidates { + Ok(c) => c, + Err(e) => return to_resolve_err(e), + }; let versions = { let mut versions = candidates .iter() @@ -303,45 +286,12 @@ pub(super) fn activation_error( msg } else { - // Maybe something is wrong with the available versions - let mut version_candidates = loop { - 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() { - 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 name_candidates = loop { - 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() { - Ok(()) => continue, - Err(e) => return to_resolve_err(e), - }, - } - }; - 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(&*wild_dep.package_name(), &*n.name(), 3)?, n))) - .collect(); - name_candidates.sort_by_key(|o| o.0); - let mut msg = String::new(); - if !version_candidates.is_empty() { + if let Some(version_candidates) = rejected_versions(registry, dep) { + let version_candidates = match version_candidates { + Ok(c) => c, + Err(e) => return to_resolve_err(e), + }; let _ = writeln!( &mut msg, "no matching versions for `{}` found", @@ -382,7 +332,11 @@ pub(super) fn activation_error( } } } - } else if !name_candidates.is_empty() { + } else if let Some(name_candidates) = alt_names(registry, dep) { + let name_candidates = match name_candidates { + Ok(c) => c, + Err(e) => return to_resolve_err(e), + }; let _ = writeln!(&mut msg, "no matching package found",); let _ = writeln!(&mut msg, "searched package name: `{}`", dep.package_name()); let mut names = name_candidates @@ -442,6 +396,96 @@ pub(super) fn activation_error( to_resolve_err(anyhow::format_err!("{}", msg)) } +// 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. +fn alt_versions( + registry: &mut dyn Registry, + dep: &Dependency, +) -> Option>> { + let mut wild_dep = dep.clone(); + wild_dep.set_version_req(OptVersionReq::Any); + + let candidates = loop { + match registry.query_vec(&wild_dep, QueryKind::Exact) { + Poll::Ready(Ok(candidates)) => break candidates, + Poll::Ready(Err(e)) => return Some(Err(e)), + Poll::Pending => match registry.block_until_ready() { + Ok(()) => continue, + Err(e) => return Some(Err(e)), + }, + } + }; + let mut candidates: Vec<_> = candidates.into_iter().map(|s| s.into_summary()).collect(); + candidates.sort_unstable_by(|a, b| b.version().cmp(a.version())); + if candidates.is_empty() { + None + } else { + Some(Ok(candidates)) + } +} + +/// Maybe something is wrong with the available versions +fn rejected_versions( + registry: &mut dyn Registry, + dep: &Dependency, +) -> Option>> { + let mut version_candidates = loop { + match registry.query_vec(&dep, QueryKind::RejectedVersions) { + Poll::Ready(Ok(candidates)) => break candidates, + Poll::Ready(Err(e)) => return Some(Err(e)), + Poll::Pending => match registry.block_until_ready() { + Ok(()) => continue, + Err(e) => return Some(Err(e)), + }, + } + }; + version_candidates.sort_unstable_by_key(|a| a.as_summary().version().clone()); + if version_candidates.is_empty() { + None + } else { + Some(Ok(version_candidates)) + } +} + +/// 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. +fn alt_names( + registry: &mut dyn Registry, + dep: &Dependency, +) -> Option>> { + let mut wild_dep = dep.clone(); + wild_dep.set_version_req(OptVersionReq::Any); + + let name_candidates = loop { + match registry.query_vec(&wild_dep, QueryKind::AlternativeNames) { + Poll::Ready(Ok(candidates)) => break candidates, + Poll::Ready(Err(e)) => return Some(Err(e)), + Poll::Pending => match registry.block_until_ready() { + Ok(()) => continue, + Err(e) => return Some(Err(e)), + }, + } + }; + 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 + .into_iter() + .filter_map(|n| Some((edit_distance(&*wild_dep.package_name(), &*n.name(), 3)?, n))) + .collect(); + name_candidates.sort_by_key(|o| o.0); + + if name_candidates.is_empty() { + None + } else { + Some(Ok(name_candidates)) + } +} + /// Returns String representation of dependency chain for a particular `pkgid` /// within given context. pub(super) fn describe_path_in_context(cx: &ResolverContext, id: &PackageId) -> String { From ba4f982b03796c9605987aae5c261602227b50da Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 11 Dec 2024 12:34:10 -0600 Subject: [PATCH 2/9] refactor(resolver): Share the buffer for errors --- src/cargo/core/resolver/errors.rs | 56 ++++++++++++++++++------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index df1979821f2..c55ede94b98 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -219,7 +219,8 @@ pub(super) fn activation_error( // We didn't actually find any candidates, so we need to // give an error message that nothing was found. - let mut msg = if let Some(candidates) = alt_versions(registry, dep) { + let mut msg = String::new(); + if let Some(candidates) = alt_versions(registry, dep) { let candidates = match candidates { Ok(c) => c, Err(e) => return to_resolve_err(e), @@ -244,49 +245,57 @@ pub(super) fn activation_error( .map(|v| format!(" (locked to {})", v)) .unwrap_or_default(); - let mut msg = format!( - "failed to select a version for the requirement `{} = \"{}\"`{}\n\ - candidate versions found which didn't match: {}\n\ - location searched: {}\n", + let _ = writeln!( + &mut msg, + "failed to select a version for the requirement `{} = \"{}\"`{}", dep.package_name(), dep.version_req(), locked_version, - versions, - registry.describe_source(dep.source_id()), ); - msg.push_str("required by "); - msg.push_str(&describe_path_in_context( - resolver_ctx, - &parent.package_id(), - )); + let _ = writeln!( + &mut msg, + "candidate versions found which didn't match: {versions}", + ); + let _ = writeln!( + &mut msg, + "location searched: {}", + registry.describe_source(dep.source_id()) + ); + let _ = write!( + &mut msg, + "required by {}", + describe_path_in_context(resolver_ctx, &parent.package_id()), + ); // If we have a pre-release candidate, then that may be what our user is looking for if let Some(pre) = candidates.iter().find(|c| c.version().is_prerelease()) { - msg.push_str("\nif you are looking for the prerelease package it needs to be specified explicitly"); - msg.push_str(&format!( + let _ = write!(&mut msg, "\nif you are looking for the prerelease package it needs to be specified explicitly"); + let _ = write!( + &mut msg, "\n {} = {{ version = \"{}\" }}", pre.name(), pre.version() - )); + ); } // If we have a path dependency with a locked version, then this may // indicate that we updated a sub-package and forgot to run `cargo // update`. In this case try to print a helpful error! if dep.source_id().is_path() && dep.version_req().is_locked() { - msg.push_str( + let _ = write!( + &mut msg, "\nconsider running `cargo update` to update \ a path dependency's locked version", ); } if registry.is_replaced(dep.source_id()) { - msg.push_str("\nperhaps a crate was updated and forgotten to be re-vendored?"); + let _ = write!( + &mut msg, + "\nperhaps a crate was updated and forgotten to be re-vendored?" + ); } - - msg } else { - let mut msg = String::new(); if let Some(version_candidates) = rejected_versions(registry, dep) { let version_candidates = match version_candidates { Ok(c) => c, @@ -378,13 +387,12 @@ pub(super) fn activation_error( "required by {}", describe_path_in_context(resolver_ctx, &parent.package_id()), ); - - msg - }; + } if let Some(gctx) = gctx { if gctx.offline() { - msg.push_str( + let _ = write!( + &mut msg, "\nAs a reminder, you're using offline mode (--offline) \ which can sometimes cause surprising resolution failures, \ if this error is too confusing you may wish to retry \ From 41d5e465bf93e7863d573787cb9b76875bcf11e4 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 11 Dec 2024 12:38:33 -0600 Subject: [PATCH 3/9] refactor(resolver): Track error message and hint separately --- src/cargo/core/resolver/errors.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index c55ede94b98..bbaae66cef4 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -220,6 +220,7 @@ pub(super) fn activation_error( // We didn't actually find any candidates, so we need to // give an error message that nothing was found. let mut msg = String::new(); + let mut hints = String::new(); if let Some(candidates) = alt_versions(registry, dep) { let candidates = match candidates { Ok(c) => c, @@ -269,9 +270,9 @@ pub(super) fn activation_error( // If we have a pre-release candidate, then that may be what our user is looking for if let Some(pre) = candidates.iter().find(|c| c.version().is_prerelease()) { - let _ = write!(&mut msg, "\nif you are looking for the prerelease package it needs to be specified explicitly"); + let _ = write!(&mut hints, "\nif you are looking for the prerelease package it needs to be specified explicitly"); let _ = write!( - &mut msg, + &mut hints, "\n {} = {{ version = \"{}\" }}", pre.name(), pre.version() @@ -283,7 +284,7 @@ pub(super) fn activation_error( // update`. In this case try to print a helpful error! if dep.source_id().is_path() && dep.version_req().is_locked() { let _ = write!( - &mut msg, + &mut hints, "\nconsider running `cargo update` to update \ a path dependency's locked version", ); @@ -291,7 +292,7 @@ pub(super) fn activation_error( if registry.is_replaced(dep.source_id()) { let _ = write!( - &mut msg, + &mut hints, "\nperhaps a crate was updated and forgotten to be re-vendored?" ); } @@ -392,7 +393,7 @@ pub(super) fn activation_error( if let Some(gctx) = gctx { if gctx.offline() { let _ = write!( - &mut msg, + &mut hints, "\nAs a reminder, you're using offline mode (--offline) \ which can sometimes cause surprising resolution failures, \ if this error is too confusing you may wish to retry \ @@ -401,7 +402,7 @@ pub(super) fn activation_error( } } - to_resolve_err(anyhow::format_err!("{}", msg)) + to_resolve_err(anyhow::format_err!("{msg}{hints}")) } // Maybe the user mistyped the ver_req? Like `dep="2"` when `dep="0.2"` From aecffa819ab3067b8ca45526ac5532e092b9cc11 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 11 Dec 2024 12:42:31 -0600 Subject: [PATCH 4/9] fix(resolver): Avoid an empty location in errors --- src/cargo/core/resolver/errors.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index bbaae66cef4..eaf672c2a56 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -257,11 +257,12 @@ pub(super) fn activation_error( &mut msg, "candidate versions found which didn't match: {versions}", ); - let _ = writeln!( - &mut msg, - "location searched: {}", - registry.describe_source(dep.source_id()) - ); + let mut location_searched_msg = registry.describe_source(dep.source_id()); + if location_searched_msg.is_empty() { + location_searched_msg = format!("{}", dep.source_id()); + } + + let _ = writeln!(&mut msg, "location searched: {}", location_searched_msg); let _ = write!( &mut msg, "required by {}", From f93ebac243d2c31f99a1d43d4b47774b9a925dbb Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 11 Dec 2024 12:45:30 -0600 Subject: [PATCH 5/9] refactor(resolver): Share context across messages --- src/cargo/core/resolver/errors.rs | 32 ++++++++++--------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index eaf672c2a56..357dbfca8fa 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -257,17 +257,6 @@ pub(super) fn activation_error( &mut msg, "candidate versions found which didn't match: {versions}", ); - let mut location_searched_msg = registry.describe_source(dep.source_id()); - if location_searched_msg.is_empty() { - location_searched_msg = format!("{}", dep.source_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()), - ); // If we have a pre-release candidate, then that may be what our user is looking for if let Some(pre) = candidates.iter().find(|c| c.version().is_prerelease()) { @@ -377,19 +366,18 @@ pub(super) fn activation_error( 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()); - } - - let _ = writeln!(&mut msg, "location searched: {}", location_searched_msg); - let _ = write!( - &mut msg, - "required by {}", - describe_path_in_context(resolver_ctx, &parent.package_id()), - ); + let mut location_searched_msg = registry.describe_source(dep.source_id()); + if location_searched_msg.is_empty() { + location_searched_msg = format!("{}", dep.source_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()), + ); if let Some(gctx) = gctx { if gctx.offline() { From 784dfbe130316df99635ae48128749d088d30dbb Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 11 Dec 2024 13:01:41 -0600 Subject: [PATCH 6/9] refactor(resolver): Flatten the error cases --- src/cargo/core/resolver/errors.rs | 128 +++++++++++++++--------------- 1 file changed, 63 insertions(+), 65 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index 357dbfca8fa..1b770b05256 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -286,71 +286,70 @@ pub(super) fn activation_error( "\nperhaps a crate was updated and forgotten to be re-vendored?" ); } - } else { - if let Some(version_candidates) = rejected_versions(registry, dep) { - let version_candidates = match version_candidates { - Ok(c) => c, - Err(e) => return to_resolve_err(e), - }; - let _ = writeln!( - &mut msg, - "no matching versions for `{}` found", - dep.package_name() - ); - 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 - // 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!( + } else if let Some(version_candidates) = rejected_versions(registry, dep) { + let version_candidates = match version_candidates { + Ok(c) => c, + Err(e) => return to_resolve_err(e), + }; + let _ = writeln!( + &mut msg, + "no matching versions for `{}` found", + dep.package_name() + ); + 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 + // 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 let Some(name_candidates) = alt_names(registry, dep) { - let name_candidates = match name_candidates { - Ok(c) => c, - Err(e) => return to_resolve_err(e), - }; - 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::>(); - - if name_candidates.len() > 3 { - names.push("..."); - } - // Vertically align first suggestion with missing crate name - // so a typo jumps out at you. - let suggestions = names + } + } else if let Some(name_candidates) = alt_names(registry, dep) { + let name_candidates = match name_candidates { + Ok(c) => c, + Err(e) => return to_resolve_err(e), + }; + 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::>(); + + if name_candidates.len() > 3 { + names.push("..."); + } + // Vertically align first suggestion with missing crate name + // so a typo jumps out at you. + let suggestions = + names .iter() .enumerate() .fold(String::default(), |acc, (i, el)| match i { @@ -358,14 +357,13 @@ pub(super) fn activation_error( i if names.len() - 1 == i && name_candidates.len() <= 3 => acc + " or " + el, _ => acc + ", " + el, }); - let _ = writeln!(&mut msg, "perhaps you meant: {suggestions}"); - } else { - let _ = writeln!( - &mut msg, - "no matching package named `{}` found", - dep.package_name() - ); - } + 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()); From 28b5cb15ee279c68141f8fe44e703ef2503c132f Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 11 Dec 2024 13:08:42 -0600 Subject: [PATCH 7/9] fix(resolver): In errors, show rejected versions over alt versions The user likely intended what they said and we should tell them why it didn't work. Rejected versions also imply alt versions below them. Fixes #4260 --- src/cargo/core/resolver/errors.rs | 90 +++++++++++++++---------------- tests/testsuite/artifact_dep.rs | 3 +- tests/testsuite/registry.rs | 10 ++-- 3 files changed, 50 insertions(+), 53 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index 1b770b05256..8c6fc97a2bd 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -221,7 +221,51 @@ pub(super) fn activation_error( // give an error message that nothing was found. let mut msg = String::new(); let mut hints = String::new(); - if let Some(candidates) = alt_versions(registry, dep) { + if let Some(version_candidates) = rejected_versions(registry, dep) { + let version_candidates = match version_candidates { + Ok(c) => c, + Err(e) => return to_resolve_err(e), + }; + let _ = writeln!( + &mut msg, + "no matching versions for `{}` found", + dep.package_name() + ); + 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 + // 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 let Some(candidates) = alt_versions(registry, dep) { let candidates = match candidates { Ok(c) => c, Err(e) => return to_resolve_err(e), @@ -286,50 +330,6 @@ pub(super) fn activation_error( "\nperhaps a crate was updated and forgotten to be re-vendored?" ); } - } else if let Some(version_candidates) = rejected_versions(registry, dep) { - let version_candidates = match version_candidates { - Ok(c) => c, - Err(e) => return to_resolve_err(e), - }; - let _ = writeln!( - &mut msg, - "no matching versions for `{}` found", - dep.package_name() - ); - 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 - // 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 let Some(name_candidates) = alt_names(registry, dep) { let name_candidates = match name_candidates { Ok(c) => c, diff --git a/tests/testsuite/artifact_dep.rs b/tests/testsuite/artifact_dep.rs index 28536e4cd41..1912d61e859 100644 --- a/tests/testsuite/artifact_dep.rs +++ b/tests/testsuite/artifact_dep.rs @@ -1752,10 +1752,9 @@ foo v0.1.0 ([ROOT]/foo) .with_stderr_data(str![[r#" [UPDATING] `dummy-registry` index [ERROR] failed to select a version for the requirement `bar = "^1.0"` (locked to 1.0.1) -candidate versions found which didn't match: 1.0.0 + version 1.0.1 requires a Cargo version that supports index version 3 location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `foo v0.1.0 ([ROOT]/foo)` -perhaps a crate was updated and forgotten to be re-vendored? "#]]) .run(); diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 2c3da8261a2..03d02e6b2d6 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -862,12 +862,11 @@ fn relying_on_a_yank_is_bad_http() { let _server = setup_http(); relying_on_a_yank_is_bad(str![[r#" [UPDATING] `dummy-registry` index -[ERROR] failed to select a version for the requirement `baz = "=0.0.2"` -candidate versions found which didn't match: 0.0.1 +[ERROR] no matching versions for `baz` found + version 0.0.2 is yanked location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `bar v0.0.1` ... which satisfies dependency `bar = "*"` of package `foo v0.0.1 ([ROOT]/foo)` -perhaps a crate was updated and forgotten to be re-vendored? "#]]); } @@ -876,12 +875,11 @@ perhaps a crate was updated and forgotten to be re-vendored? fn relying_on_a_yank_is_bad_git() { relying_on_a_yank_is_bad(str![[r#" [UPDATING] `dummy-registry` index -[ERROR] failed to select a version for the requirement `baz = "=0.0.2"` -candidate versions found which didn't match: 0.0.1 +[ERROR] no matching versions for `baz` found + version 0.0.2 is yanked location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `bar v0.0.1` ... which satisfies dependency `bar = "*"` of package `foo v0.0.1 ([ROOT]/foo)` -perhaps a crate was updated and forgotten to be re-vendored? "#]]); } From 9d2115362963fa3a3a7b23d9c5e6f4c740885fd0 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 11 Dec 2024 13:11:08 -0600 Subject: [PATCH 8/9] test(resolver): Verify wildcard dep isn't used to find rejected versions --- tests/testsuite/registry.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 03d02e6b2d6..4744cf90f98 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -3202,6 +3202,7 @@ fn ignores_unknown_index_version(expected: impl IntoData) { #[cargo_test] fn unknown_index_version_error() { + Package::new("bar", "0.0.1").publish(); // If the version field is not understood, it is ignored. Package::new("bar", "1.0.1") .schema_version(u32::MAX) @@ -3238,6 +3239,7 @@ required by package `foo v0.1.0 ([ROOT]/foo)` #[cargo_test] fn unknown_index_version_with_msrv_error() { + Package::new("bar", "0.0.1").publish(); // If the version field is not understood, it is ignored. Package::new("bar", "1.0.1") .schema_version(u32::MAX) From ccc00ced4d170ad7942cacd1571cae86db178e70 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 11 Dec 2024 13:18:42 -0600 Subject: [PATCH 9/9] fix(resolver): Include dep spec in rejected versions error --- src/cargo/core/resolver/errors.rs | 12 ++++++++++-- tests/testsuite/registry.rs | 16 ++++++++-------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index 8c6fc97a2bd..592f3ae7371 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -226,10 +226,18 @@ pub(super) fn activation_error( Ok(c) => c, Err(e) => return to_resolve_err(e), }; + + let locked_version = dep + .version_req() + .locked_version() + .map(|v| format!(" (locked to {})", v)) + .unwrap_or_default(); let _ = writeln!( &mut msg, - "no matching versions for `{}` found", - dep.package_name() + "failed to select a version for the requirement `{} = \"{}\"`{}", + dep.package_name(), + dep.version_req(), + locked_version ); for candidate in version_candidates { match candidate { diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 4744cf90f98..69f41e643a2 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -862,7 +862,7 @@ fn relying_on_a_yank_is_bad_http() { let _server = setup_http(); relying_on_a_yank_is_bad(str![[r#" [UPDATING] `dummy-registry` index -[ERROR] no matching versions for `baz` found +[ERROR] failed to select a version for the requirement `baz = "=0.0.2"` version 0.0.2 is yanked location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `bar v0.0.1` @@ -875,7 +875,7 @@ required by package `bar v0.0.1` fn relying_on_a_yank_is_bad_git() { relying_on_a_yank_is_bad(str![[r#" [UPDATING] `dummy-registry` index -[ERROR] no matching versions for `baz` found +[ERROR] failed to select a version for the requirement `baz = "=0.0.2"` version 0.0.2 is yanked location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `bar v0.0.1` @@ -922,7 +922,7 @@ fn yanks_in_lockfiles_are_ok_http() { "#]], str![[r#" [UPDATING] `dummy-registry` index -[ERROR] no matching versions for `bar` found +[ERROR] failed to select a version for the requirement `bar = "*"` version 0.0.1 is yanked location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `foo v0.0.1 ([ROOT]/foo)` @@ -940,7 +940,7 @@ fn yanks_in_lockfiles_are_ok_git() { "#]], str![[r#" [UPDATING] `dummy-registry` index -[ERROR] no matching versions for `bar` found +[ERROR] failed to select a version for the requirement `bar = "*"` version 0.0.1 is yanked location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `foo v0.0.1 ([ROOT]/foo)` @@ -993,7 +993,7 @@ fn yanks_in_lockfiles_are_ok_for_other_update_http() { "#]], str![[r#" [UPDATING] `dummy-registry` index -[ERROR] no matching versions for `bar` found +[ERROR] failed to select a version for the requirement `bar = "*"` version 0.0.1 is yanked location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `foo v0.0.1 ([ROOT]/foo)` @@ -1017,7 +1017,7 @@ fn yanks_in_lockfiles_are_ok_for_other_update_git() { "#]], str![[r#" [UPDATING] `dummy-registry` index -[ERROR] no matching versions for `bar` found +[ERROR] failed to select a version for the requirement `bar = "*"` version 0.0.1 is yanked location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `foo v0.0.1 ([ROOT]/foo)` @@ -3228,7 +3228,7 @@ fn unknown_index_version_error() { .with_status(101) .with_stderr_data(str![[r#" [UPDATING] `dummy-registry` index -[ERROR] no matching versions for `bar` found +[ERROR] failed to select a version for the requirement `bar = "^1.0"` version 1.0.1 requires a Cargo version that supports index version 4294967295 location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `foo v0.1.0 ([ROOT]/foo)` @@ -3266,7 +3266,7 @@ fn unknown_index_version_with_msrv_error() { .with_status(101) .with_stderr_data(str![[r#" [UPDATING] `dummy-registry` index -[ERROR] no matching versions for `bar` found +[ERROR] failed to select a version for the requirement `bar = "^1.0"` version 1.0.1 requires cargo 1.2345 location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `foo v0.1.0 ([ROOT]/foo)`