Skip to content

Commit

Permalink
Auto merge of #12659 - Eh2406:Prerelease_candidates, r=epage
Browse files Browse the repository at this point in the history 
Prerelease candidates error message

### What does this PR try to resolve?

Error messages reporting on versions that do not match the request incorrectly ignore pre-release versions. This is because the version requirement `"*"` cannot match prerelease versions. #12315

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

Sorry for the large amount of white space changes, fmt got to fmt. 🤷‍♂️

The process was:
- Revise commit from #12316 (thanks to `@loloicci)` that change the requirement from `"*"` to `Any`
- Move the handling of our special "did you mean to specify a pre-release" code and update tests
- some small re-factoring

### Additional information

The old "did you mean to specify a pre-release" #7191 check only occurred when version requirement does not match any versions and you depended on a package that did not have any non-prerelease versions. Making it rarely useful.
The new one will appear any time your version requirement does not match any versions and the package does have pre-release versions. Which may be too common.
I'm open to suggestions for better heuristic.

It's also not clear that the new message make sense in the case of patched versions.
  • Loading branch information
@bors
bors committed Sep 14, 2023
2 parents bc26ac0 + 50f1455 commit 8d43632
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 123 deletions.
4 changes: 2 additions & 2 deletions src/cargo/core/dependency.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,8 @@ impl Dependency {
}

/// Sets the version requirement for this dependency.
pub fn set_version_req(&mut self, req: VersionReq) -> &mut Dependency {
Rc::make_mut(&mut self.inner).req = OptVersionReq::Req(req);
pub fn set_version_req(&mut self, req: OptVersionReq) -> &mut Dependency {
Rc::make_mut(&mut self.inner).req = req;
self
}

Expand Down
220 changes: 107 additions & 113 deletions src/cargo/core/resolver/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::task::Poll;
use crate::core::{Dependency, PackageId, Registry, Summary};
use crate::sources::source::QueryKind;
use crate::util::edit_distance::edit_distance;
use crate::util::{Config, VersionExt};
use crate::util::{Config, OptVersionReq, VersionExt};
use anyhow::Error;

use super::context::Context;
Expand Down Expand Up @@ -224,9 +224,8 @@ 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 all_req = semver::VersionReq::parse("*").unwrap();
let mut new_dep = dep.clone();
new_dep.set_version_req(all_req);
new_dep.set_version_req(OptVersionReq::Any);

let mut candidates = loop {
match registry.query_vec(&new_dep, QueryKind::Exact) {
Expand All @@ -241,127 +240,122 @@ pub(super) fn activation_error(

candidates.sort_unstable_by(|a, b| b.version().cmp(a.version()));

let mut msg =
if !candidates.is_empty() {
let versions = {
let mut versions = candidates
.iter()
.take(3)
.map(|cand| cand.version().to_string())
.collect::<Vec<_>>();

if candidates.len() > 3 {
versions.push("...".into());
}
let mut msg = if !candidates.is_empty() {
let versions = {
let mut versions = candidates
.iter()
.take(3)
.map(|cand| cand.version().to_string())
.collect::<Vec<_>>();

versions.join(", ")
};
if candidates.len() > 3 {
versions.push("...".into());
}

let locked_version = dep
.version_req()
.locked_version()
.map(|v| format!(" (locked to {})", v))
.unwrap_or_default();
versions.join(", ")
};

let locked_version = dep
.version_req()
.locked_version()
.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",
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(cx, &parent.package_id()));

let mut msg = format!(
"failed to select a version for the requirement `{} = \"{}\"`{}\n\
candidate versions found which didn't match: {}\n\
location searched: {}\n",
dep.package_name(),
dep.version_req(),
locked_version,
versions,
registry.describe_source(dep.source_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!(
"\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(
"\nconsider running `cargo update` to update \
a path dependency's locked version",
);
msg.push_str("required by ");
msg.push_str(&describe_path_in_context(cx, &parent.package_id()));

// 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(
"\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?");
}

if registry.is_replaced(dep.source_id()) {
msg.push_str("\nperhaps a crate was updated and forgotten to be re-vendored?");
msg
} else {
// 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 mut candidates = loop {
match registry.query_vec(&new_dep, QueryKind::Fuzzy) {
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),
},
}
};

msg
candidates.sort_unstable_by_key(|a| a.name());
candidates.dedup_by(|a, b| a.name() == b.name());
let mut candidates: Vec<_> = 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 {
// 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 mut candidates = loop {
match registry.query_vec(&new_dep, QueryKind::Fuzzy) {
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),
},
}
};

candidates.sort_unstable_by_key(|a| a.name());
candidates.dedup_by(|a, b| a.name() == b.name());
let mut candidates: Vec<_> = candidates
msg = format!(
"no matching package found\nsearched package name: `{}`\n",
dep.package_name()
);
let mut names = 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",
dep.package_name()
);

// If dependency package name is equal to the name of the candidate here
// it may be a prerelease package which hasn't been specified correctly
if dep.package_name() == candidates[0].1.name()
&& candidates[0].1.package_id().version().is_prerelease()
{
msg.push_str("prerelease package needs to be specified explicitly\n");
msg.push_str(&format!(
"{name} = {{ version = \"{version}\" }}",
name = candidates[0].1.name(),
version = candidates[0].1.package_id().version()
));
} else {
let mut names = candidates
.iter()
.take(3)
.map(|c| c.1.name().as_str())
.collect::<Vec<_>>();

if 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 {
0 => acc + el,
i if names.len() - 1 == i && candidates.len() <= 3 => acc + " or " + el,
_ => acc + ", " + el,
},
));
}
msg.push('\n');
.take(3)
.map(|c| c.1.name().as_str())
.collect::<Vec<_>>();

if candidates.len() > 3 {
names.push("...");
}
msg.push_str(&format!("location searched: {}\n", dep.source_id()));
msg.push_str("required by ");
msg.push_str(&describe_path_in_context(cx, &parent.package_id()));
// 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 {
0 => acc + el,
i if names.len() - 1 == i && candidates.len() <= 3 => acc + " or " + el,
_ => acc + ", " + el,
},
));
msg.push('\n');
}
msg.push_str(&format!("location searched: {}\n", dep.source_id()));
msg.push_str("required by ");
msg.push_str(&describe_path_in_context(cx, &parent.package_id()));

msg
};
msg
};

if let Some(config) = config {
if config.offline() {
Expand Down
7 changes: 3 additions & 4 deletions src/cargo/util/toml/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use cargo_platform::Platform;
use cargo_util::paths;
use itertools::Itertools;
use lazycell::LazyCell;
use semver::{self, VersionReq};
use serde::de::{self, IntoDeserializer as _, Unexpected};
use serde::ser;
use serde::{Deserialize, Serialize};
Expand All @@ -30,8 +29,8 @@ use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY};
use crate::util::errors::{CargoResult, ManifestError};
use crate::util::interning::InternedString;
use crate::util::{
self, config::ConfigRelativePath, validate_package_name, Config, IntoUrl, PartialVersion,
VersionReqExt,
self, config::ConfigRelativePath, validate_package_name, Config, IntoUrl, OptVersionReq,
PartialVersion,
};

pub mod embedded;
Expand Down Expand Up @@ -2656,7 +2655,7 @@ impl TomlManifest {
replacement.unused_keys(),
&mut cx.warnings,
);
dep.set_version_req(VersionReq::exact(version))
dep.set_version_req(OptVersionReq::exact(version))
.lock_version(version);
replace.push((spec, dep));
}
Expand Down
42 changes: 42 additions & 0 deletions tests/testsuite/patch.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2658,3 +2658,45 @@ failed to select a version for `qux` which could resolve this conflict"#,
)
.run();
}

#[cargo_test]
fn mismatched_version_with_prerelease() {
Package::new("prerelease-deps", "0.0.1").publish();
// A patch to a location that has an prerelease version
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
prerelease-deps = "0.1.0"
[patch.crates-io]
prerelease-deps = { path = "./prerelease-deps" }
"#,
)
.file("src/lib.rs", "")
.file(
"prerelease-deps/Cargo.toml",
&basic_manifest("prerelease-deps", "0.1.1-pre1"),
)
.file("prerelease-deps/src/lib.rs", "")
.build();

p.cargo("generate-lockfile")
.with_status(101)
.with_stderr(
r#"[UPDATING] `dummy-registry` index
[ERROR] failed to select a version for the requirement `prerelease-deps = "^0.1.0"`
candidate versions found which didn't match: 0.1.1-pre1, 0.0.1
location searched: `dummy-registry` index (which is replacing registry `crates-io`)
required by package `foo v0.1.0 [..]`
if you are looking for the prerelease package it needs to be specified explicitly
prerelease-deps = { version = "0.1.1-pre1" }
perhaps a crate was updated and forgotten to be re-vendored?"#,
)
.run();
}
8 changes: 4 additions & 4 deletions tests/testsuite/registry.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1756,12 +1756,12 @@ fn use_semver_package_incorrectly() {
.with_status(101)
.with_stderr(
"\
error: no matching package found
searched package name: `a`
prerelease package needs to be specified explicitly
a = { version = \"0.1.1-alpha.0\" }
error: failed to select a version for the requirement `a = \"^0.1\"`
candidate versions found which didn't match: 0.1.1-alpha.0
location searched: [..]
required by package `b v0.1.0 ([..])`
if you are looking for the prerelease package it needs to be specified explicitly
a = { version = \"0.1.1-alpha.0\" }
",
)
.run();
Expand Down

0 comments on commit 8d43632

Please sign in to comment.