From 3b495a520984ccc24ca4a7282febafbf49a0f751 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Fri, 6 Jul 2018 12:08:00 -0400 Subject: [PATCH 01/12] make index lookup robust to _ vs - --- src/cargo/sources/registry/index.rs | 98 ++++++++++++++++++----------- 1 file changed, 60 insertions(+), 38 deletions(-) diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 3f349af0b2a..56a05b8d6fd 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -99,51 +99,73 @@ impl<'cfg> RegistryIndex<'cfg> { .collect::(); // see module comment for why this is structured the way it is - let path = match fs_name.len() { + let raw_path = match fs_name.len() { 1 => format!("1/{}", fs_name), 2 => format!("2/{}", fs_name), 3 => format!("3/{}/{}", &fs_name[..1], fs_name), _ => format!("{}/{}/{}", &fs_name[0..2], &fs_name[2..4], fs_name), }; + let num_hyphen_underscore = raw_path.chars() + .filter(|&c| c == '_' || c == '-') + .count(); let mut ret = Vec::new(); - let mut hit_closure = false; - let err = load.load(&root, Path::new(&path), &mut |contents| { - hit_closure = true; - let contents = str::from_utf8(contents) - .map_err(|_| format_err!("registry index file was not valid utf-8"))?; - ret.reserve(contents.lines().count()); - let lines = contents.lines().map(|s| s.trim()).filter(|l| !l.is_empty()); - - let online = !self.config.cli_unstable().offline; - // Attempt forwards-compatibility on the index by ignoring - // everything that we ourselves don't understand, that should - // allow future cargo implementations to break the - // interpretation of each line here and older cargo will simply - // ignore the new lines. - ret.extend(lines.filter_map(|line| { - let (summary, locked) = match self.parse_registry_package(line) { - Ok(p) => p, - Err(e) => { - info!("failed to parse `{}` registry package: {}", name, e); - trace!("line: {}", line); - return None; - } - }; - if online || load.is_crate_downloaded(summary.package_id()) { - Some((summary, locked)) + // Crates.io treats hyphen and underscores as interchangeable + // but, the index and old cargo do not. So the index must store uncanonicalized version + // of the name so old cargos can find it. + // This loop tries all possible combinations of + // hyphen and underscores to find the uncanonicalized one. + for hyphen_combination_num in 0u16..(1 << num_hyphen_underscore) { + let path = raw_path.chars() + .scan(0u32, |s, c| if c == '_' || c == '-' { + let out = Some(if (hyphen_combination_num & (1u16 << *s)) > 0 { '_' } else { '-' }); + *s += 1; + out } else { - None - } - })); - - Ok(()) - }); - - // We ignore lookup failures as those are just crates which don't exist - // or we haven't updated the registry yet. If we actually ran the - // closure though then we care about those errors. - if hit_closure { - err?; + Some(c) + }) + .collect::(); + let mut hit_closure = false; + let err = load.load(&root, Path::new(&path), &mut |contents| { + hit_closure = true; + let contents = str::from_utf8(contents) + .map_err(|_| format_err!("registry index file was not valid utf-8"))?; + ret.reserve(contents.lines().count()); + let lines = contents.lines().map(|s| s.trim()).filter(|l| !l.is_empty()); + + let online = !self.config.cli_unstable().offline; + // Attempt forwards-compatibility on the index by ignoring + // everything that we ourselves don't understand, that should + // allow future cargo implementations to break the + // interpretation of each line here and older cargo will simply + // ignore the new lines. + ret.extend(lines.filter_map(|line| { + let (summary, locked) = match self.parse_registry_package(line) { + Ok(p) => p, + Err(e) => { + info!("failed to parse `{}` registry package: {}", name, e); + trace!("line: {}", line); + return None; + } + }; + if online || load.is_crate_downloaded(summary.package_id()) { + Some((summary, locked)) + } else { + None + } + })); + + Ok(()) + }); + + // We ignore lookup failures as those are just crates which don't exist + // or we haven't updated the registry yet. If we actually ran the + // closure though then we care about those errors. + if hit_closure { + err?; + // Crates.io ensures that there is only one hyphen and underscore equivalent + // result in the index so return when we find it. + return Ok(ret); + } } Ok(ret) From d0d9377be3e072aa18aa4ce27d6374e454e5faeb Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Sat, 7 Jul 2018 11:44:58 -0400 Subject: [PATCH 02/12] cargo +stable fmt --- src/cargo/sources/registry/index.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 56a05b8d6fd..643c87771dc 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -115,13 +115,20 @@ impl<'cfg> RegistryIndex<'cfg> { // This loop tries all possible combinations of // hyphen and underscores to find the uncanonicalized one. for hyphen_combination_num in 0u16..(1 << num_hyphen_underscore) { - let path = raw_path.chars() - .scan(0u32, |s, c| if c == '_' || c == '-' { - let out = Some(if (hyphen_combination_num & (1u16 << *s)) > 0 { '_' } else { '-' }); - *s += 1; - out - } else { - Some(c) + let path = raw_path + .chars() + .scan(0u32, |s, c| { + if c == '_' || c == '-' { + let out = Some(if (hyphen_combination_num & (1u16 << *s)) > 0 { + '_' + } else { + '-' + }); + *s += 1; + out + } else { + Some(c) + } }) .collect::(); let mut hit_closure = false; From 78a04868f5aad7e4d64bcc3613a7c6f8e2499e11 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Sat, 7 Jul 2018 12:06:08 -0400 Subject: [PATCH 03/12] start with the name as given, and cap the number to be switched --- src/cargo/sources/registry/index.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 643c87771dc..6eba1a92779 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -105,21 +105,22 @@ impl<'cfg> RegistryIndex<'cfg> { 3 => format!("3/{}/{}", &fs_name[..1], fs_name), _ => format!("{}/{}/{}", &fs_name[0..2], &fs_name[2..4], fs_name), }; - let num_hyphen_underscore = raw_path.chars() - .filter(|&c| c == '_' || c == '-') - .count(); + let num_hyphen_underscore = ::std::cmp::min( + raw_path.chars().filter(|&c| c == '_' || c == '-').count(), + 10, + ) as u16; let mut ret = Vec::new(); // Crates.io treats hyphen and underscores as interchangeable // but, the index and old cargo do not. So the index must store uncanonicalized version // of the name so old cargos can find it. - // This loop tries all possible combinations of + // This loop tries all possible combinations of switching // hyphen and underscores to find the uncanonicalized one. for hyphen_combination_num in 0u16..(1 << num_hyphen_underscore) { let path = raw_path .chars() - .scan(0u32, |s, c| { - if c == '_' || c == '-' { - let out = Some(if (hyphen_combination_num & (1u16 << *s)) > 0 { + .scan(0u16, |s, c| { + if (c == '_' || c == '-') && *s <= num_hyphen_underscore { + let out = Some(if (c == '_') ^ ((hyphen_combination_num & (1u16 << *s)) > 0) { '_' } else { '-' From 3fafca1742204dc58b489b1b8125500567a8eac6 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Mon, 9 Jul 2018 18:02:46 -0400 Subject: [PATCH 04/12] add test for wrong_case in cargo.toml --- tests/testsuite/registry.rs | 68 +++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 4d632a7b925..2b93cbf6e97 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -141,6 +141,74 @@ required by package `foo v0.0.1 ([..])` ); } +#[test] +fn wrong_case() { + Package::new("init", "0.0.1").publish(); + + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + Init = ">= 0.0.0" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + // TODO: #5678 to make this work or fleas give better error message + assert_that( + p.cargo("build"), + execs().with_status(101).with_stderr( + "\ +[UPDATING] registry [..] +error: no matching package named `Init` found +location searched: registry [..] +required by package `foo v0.0.1 ([..])` +", + ), + ); +} + +#[test] +fn mis_hyphenated() { + Package::new("mis-hyphenated", "0.0.1").publish(); + + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + mis_hyphenated = ">= 0.0.0" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + // TODO: #2775 to make this work or fleas give better error message + assert_that( + p.cargo("build"), + execs().with_status(101).with_stderr( + "\ +[UPDATING] registry [..] +error: no matching package named `mis_hyphenated` found +location searched: registry [..] +required by package `foo v0.0.1 ([..])` +", + ), + ); +} + #[test] fn wrong_version() { let p = project("foo") From 8a717d32db9a9841f7d4a4e3105a344bb888b5db Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Tue, 10 Jul 2018 11:05:44 -0400 Subject: [PATCH 05/12] add test for wrong_case in registry --- tests/testsuite/registry.rs | 4 ++-- tests/testsuite/resolve.rs | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 2b93cbf6e97..af3943d869d 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -161,7 +161,7 @@ fn wrong_case() { .file("src/main.rs", "fn main() {}") .build(); - // TODO: #5678 to make this work or fleas give better error message + // TODO: #5678 to make this work or at least give better error message assert_that( p.cargo("build"), execs().with_status(101).with_stderr( @@ -195,7 +195,7 @@ fn mis_hyphenated() { .file("src/main.rs", "fn main() {}") .build(); - // TODO: #2775 to make this work or fleas give better error message + // TODO: #2775 to make this work or at least give better error message assert_that( p.cargo("build"), execs().with_status(101).with_stderr( diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs index 24d5ff9efb0..98a0ce972b9 100644 --- a/tests/testsuite/resolve.rs +++ b/tests/testsuite/resolve.rs @@ -440,6 +440,46 @@ fn resolving_incompat_versions() { ); } +#[test] +fn resolving_wrong_case_from_registry() { + // In the future we may #5678 allow this to happen. + // For back compatibility reasons, we probably won't. + // But we may want to future prove ourselves by understanding it. + // This test documents the current behavior. + let reg = registry(vec![ + pkg!(("foo", "1.0.0")), + pkg!("bar" => ["Foo"]), + ]); + + assert!( + resolve( + &pkg_id("root"), + vec![dep("bar")], + ® + ).is_err() + ); +} + +#[test] +fn resolving_mis_hyphenated_from_registry() { + // In the future we may #2775 allow this to happen. + // For back compatibility reasons, we probably won't. + // But we may want to future prove ourselves by understanding it. + // This test documents the current behavior. + let reg = registry(vec![ + pkg!(("fo-o", "1.0.0")), + pkg!("bar" => ["fo_o"]), + ]); + + assert!( + resolve( + &pkg_id("root"), + vec![dep("bar")], + ® + ).is_err() + ); +} + #[test] fn resolving_backtrack() { let reg = registry(vec![ From 84cc3d8b09261d29003daa3c4814dc55dc0f4526 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Wed, 11 Jul 2018 12:46:52 -0400 Subject: [PATCH 06/12] allow each source to recommend packages that are close to a dependency --- src/cargo/core/registry.rs | 32 ++++++++++++++++++++--------- src/cargo/core/resolver/mod.rs | 24 ++++++++++++++++------ src/cargo/core/resolver/types.rs | 4 ++-- src/cargo/core/source/mod.rs | 11 ++++++++++ src/cargo/sources/directory.rs | 8 ++++++++ src/cargo/sources/git/source.rs | 7 +++++++ src/cargo/sources/path.rs | 7 +++++++ src/cargo/sources/registry/index.rs | 6 ++---- src/cargo/sources/registry/mod.rs | 18 ++++++++++++---- src/cargo/sources/replaced.rs | 13 ++++++++++++ tests/testsuite/registry.rs | 6 ++++-- tests/testsuite/resolve.rs | 4 ++-- 12 files changed, 110 insertions(+), 30 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index aebf315cb94..451efcb57e6 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -14,11 +14,11 @@ use sources::config::SourceConfigMap; /// See also `core::Source`. pub trait Registry { /// Attempt to find the packages that match a dependency request. - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()>; + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary), fuzzy: bool) -> CargoResult<()>; - fn query_vec(&mut self, dep: &Dependency) -> CargoResult> { + fn query_vec(&mut self, dep: &Dependency, fuzzy: bool) -> CargoResult> { let mut ret = Vec::new(); - self.query(dep, &mut |s| ret.push(s))?; + self.query(dep, &mut |s| ret.push(s), fuzzy)?; Ok(ret) } } @@ -395,7 +395,7 @@ http://doc.crates.io/specifying-dependencies.html#overriding-dependencies } impl<'cfg> Registry for PackageRegistry<'cfg> { - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary), fuzzy: bool) -> CargoResult<()> { assert!(self.patches_locked); let (override_summary, n, to_warn) = { // Look for an override and get ready to query the real source. @@ -476,7 +476,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { // already selected, then we skip this `summary`. let locked = &self.locked; let all_patches = &self.patches_available; - return source.query(dep, &mut |summary| { + let callback = &mut |summary: Summary| { for patch in patches.iter() { let patch = patch.package_id().version(); if summary.package_id().version() == patch { @@ -484,7 +484,12 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { } } f(lock(locked, all_patches, summary)) - }); + }; + return if fuzzy { + source.fuzzy_query(dep, callback) + } else { + source.query(dep, callback) + }; } // If we have an override summary then we query the source @@ -496,10 +501,17 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { } let mut n = 0; let mut to_warn = None; - source.query(dep, &mut |summary| { - n += 1; - to_warn = Some(summary); - })?; + { + let callback = &mut |summary| { + n += 1; + to_warn = Some(summary); + }; + if fuzzy { + source.fuzzy_query(dep, callback)?; + } else { + source.query(dep, callback)?; + } + } (override_summary, n, to_warn) } } diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 37ddf9691ef..7b7df93a540 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -922,17 +922,16 @@ fn activation_error( return format_err!("{}", msg); } - // Once we're all the way down here, we're definitely lost in the - // weeds! We didn't actually find any candidates, so we need to + // We didn't actually find any candidates, so we need to // give an error message that nothing was found. // - // Note that we re-query the registry with a new dependency that - // allows any version so we can give some nicer error reporting - // which indicates a few versions that were actually found. + // Maybe the user mistyped the ver_req? Like `dep="2"` when `dep=".2"` + // was meant. So we re-query the registry with `deb="*"` 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); - let mut candidates = match registry.query_vec(&new_dep) { + let mut candidates = match registry.query_vec(&new_dep, false) { Ok(candidates) => candidates, Err(e) => return e, }; @@ -977,12 +976,25 @@ fn activation_error( 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 = Vec::new(); + if let Err(e) = registry.query(&new_dep, &mut |s| candidates.push(s.name()), true) { + return e + }; + candidates.sort_unstable(); + candidates.dedup(); let mut msg = format!( "no matching package named `{}` found\n\ location searched: {}\n", dep.name(), dep.source_id() ); + if !candidates.is_empty() { + msg.push_str("did you mean: "); + msg.push_str(&candidates.join(" or ")); + msg.push_str("\n"); + } msg.push_str("required by "); msg.push_str(&describe_path(&graph.path_to_top(parent.package_id()))); diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 502d4a0c313..8017cd063a6 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -52,7 +52,7 @@ impl<'a> RegistryQueryer<'a> { summary: s, replace: None, }); - })?; + }, false)?; for candidate in ret.iter_mut() { let summary = &candidate.summary; @@ -66,7 +66,7 @@ impl<'a> RegistryQueryer<'a> { }; debug!("found an override for {} {}", dep.name(), dep.version_req()); - let mut summaries = self.registry.query_vec(dep)?.into_iter(); + let mut summaries = self.registry.query_vec(dep, false)?.into_iter(); let s = summaries.next().ok_or_else(|| { format_err!( "no matching package for override `{}` found\n\ diff --git a/src/cargo/core/source/mod.rs b/src/cargo/core/source/mod.rs index c36480aab27..4f0b0a4f962 100644 --- a/src/cargo/core/source/mod.rs +++ b/src/cargo/core/source/mod.rs @@ -25,6 +25,12 @@ pub trait Source { /// Attempt to find the packages that match a dependency request. fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()>; + /// Attempt to find the packages that are close to a dependency request. + /// Each source gets to define what `close` means for it. + /// path/git sources may return all dependencies that are at that uri. + /// where as an Index source may return dependencies that have the same canonicalization. + fn fuzzy_query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()>; + fn query_vec(&mut self, dep: &Dependency) -> CargoResult> { let mut ret = Vec::new(); self.query(dep, &mut |s| ret.push(s))?; @@ -79,6 +85,11 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box { (**self).query(dep, f) } + /// Forwards to `Source::query` + fn fuzzy_query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + (**self).fuzzy_query(dep, f) + } + /// Forwards to `Source::source_id` fn source_id(&self) -> &SourceId { (**self).source_id() diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index bf20a270d2e..91c3e50bc7a 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -54,6 +54,14 @@ impl<'cfg> Source for DirectorySource<'cfg> { Ok(()) } + fn fuzzy_query(&mut self, _dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + let packages = self.packages.values().map(|p| &p.0); + for summary in packages.map(|pkg| pkg.summary().clone()) { + f(summary); + } + Ok(()) + } + fn supports_checksums(&self) -> bool { true } diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index fdf3e6082e6..6d5d2cfb5f8 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -130,6 +130,13 @@ impl<'cfg> Source for GitSource<'cfg> { src.query(dep, f) } + fn fuzzy_query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + let src = self.path_source + .as_mut() + .expect("BUG: update() must be called before query()"); + src.fuzzy_query(dep, f) + } + fn supports_checksums(&self) -> bool { false } diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index f058b25700f..a5d2e9591d2 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -508,6 +508,13 @@ impl<'cfg> Source for PathSource<'cfg> { Ok(()) } + fn fuzzy_query(&mut self, _dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + for s in self.packages.iter().map(|p| p.summary()) { + f(s.clone()) + } + Ok(()) + } + fn supports_checksums(&self) -> bool { false } diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 6eba1a92779..4768e6dea8b 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -208,7 +208,7 @@ impl<'cfg> RegistryIndex<'cfg> { Ok((summary, yanked.unwrap_or(false))) } - pub fn query( + pub fn query_inner( &mut self, dep: &Dependency, load: &mut RegistryData, @@ -242,9 +242,7 @@ impl<'cfg> RegistryIndex<'cfg> { }); for summary in summaries { - if dep.matches(&summary) { - f(summary); - } + f(summary); } Ok(()) } diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 0d72ae74659..7dc1ea2813f 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -463,9 +463,11 @@ impl<'cfg> Source for RegistrySource<'cfg> { if dep.source_id().precise().is_some() && !self.updated { debug!("attempting query without update"); let mut called = false; - self.index.query(dep, &mut *self.ops, &mut |s| { - called = true; - f(s); + self.index.query_inner(dep, &mut *self.ops, &mut |s| { + if dep.matches(&s) { + called = true; + f(s); + } })?; if called { return Ok(()); @@ -475,7 +477,15 @@ impl<'cfg> Source for RegistrySource<'cfg> { } } - self.index.query(dep, &mut *self.ops, f) + self.index.query_inner(dep, &mut *self.ops, &mut |s| { + if dep.matches(&s) { + f(s); + } + }) + } + + fn fuzzy_query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + self.index.query_inner(dep, &mut *self.ops, f) } fn supports_checksums(&self) -> bool { diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs index 16d867c17bd..997731aff52 100644 --- a/src/cargo/sources/replaced.rs +++ b/src/cargo/sources/replaced.rs @@ -35,6 +35,19 @@ impl<'cfg> Source for ReplacedSource<'cfg> { Ok(()) } + fn fuzzy_query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + let (replace_with, to_replace) = (&self.replace_with, &self.to_replace); + let dep = dep.clone().map_source(to_replace, replace_with); + + self.inner + .fuzzy_query( + &dep, + &mut |summary| f(summary.map_source(replace_with, to_replace)), + ) + .chain_err(|| format!("failed to query replaced source {}", self.to_replace))?; + Ok(()) + } + fn supports_checksums(&self) -> bool { self.inner.supports_checksums() } diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index af3943d869d..89a60aa0f72 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -161,7 +161,7 @@ fn wrong_case() { .file("src/main.rs", "fn main() {}") .build(); - // TODO: #5678 to make this work or at least give better error message + // #5678 to make this work assert_that( p.cargo("build"), execs().with_status(101).with_stderr( @@ -169,6 +169,7 @@ fn wrong_case() { [UPDATING] registry [..] error: no matching package named `Init` found location searched: registry [..] +did you mean: init required by package `foo v0.0.1 ([..])` ", ), @@ -195,7 +196,7 @@ fn mis_hyphenated() { .file("src/main.rs", "fn main() {}") .build(); - // TODO: #2775 to make this work or at least give better error message + // #2775 to make this work assert_that( p.cargo("build"), execs().with_status(101).with_stderr( @@ -203,6 +204,7 @@ fn mis_hyphenated() { [UPDATING] registry [..] error: no matching package named `mis_hyphenated` found location searched: registry [..] +did you mean: mis-hyphenated required by package `foo v0.0.1 ([..])` ", ), diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs index 98a0ce972b9..3634cef7cb4 100644 --- a/tests/testsuite/resolve.rs +++ b/tests/testsuite/resolve.rs @@ -28,9 +28,9 @@ fn resolve_with_config( ) -> CargoResult> { struct MyRegistry<'a>(&'a [Summary]); impl<'a> Registry for MyRegistry<'a> { - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary), fuzzy: bool) -> CargoResult<()> { for summary in self.0.iter() { - if dep.matches(summary) { + if fuzzy || dep.matches(summary) { f(summary.clone()); } } From 3f5036ae4dd02ce62d78346ab29e7d1180c70398 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Wed, 11 Jul 2018 16:55:25 -0400 Subject: [PATCH 07/12] update test to get ci passing --- tests/testsuite/build.rs | 1 + tests/testsuite/directory.rs | 1 + tests/testsuite/path.rs | 1 + 3 files changed, 3 insertions(+) diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 3c02873d8ac..4b2ac3546eb 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -1020,6 +1020,7 @@ fn cargo_compile_with_dep_name_mismatch() { execs().with_status(101).with_stderr(&format!( r#"error: no matching package named `notquitebar` found location searched: {proj_dir}/bar +did you mean: bar required by package `foo v0.0.1 ({proj_dir})` "#, proj_dir = p.url() diff --git a/tests/testsuite/directory.rs b/tests/testsuite/directory.rs index db4d3057f96..7bf497d12b8 100644 --- a/tests/testsuite/directory.rs +++ b/tests/testsuite/directory.rs @@ -230,6 +230,7 @@ error: failed to compile `bar v0.1.0`, intermediate artifacts can be found at `[ Caused by: no matching package named `baz` found location searched: registry `https://github.com/rust-lang/crates.io-index` +did you mean: bar or foo required by package `bar v0.1.0` ", ), diff --git a/tests/testsuite/path.rs b/tests/testsuite/path.rs index 98fb7c5e9dd..e7ea0f024b0 100644 --- a/tests/testsuite/path.rs +++ b/tests/testsuite/path.rs @@ -1239,6 +1239,7 @@ fn invalid_path_dep_in_workspace_with_lockfile() { "\ error: no matching package named `bar` found location searched: [..] +did you mean: foo required by package `foo v0.5.0 ([..])` ", ), From 2bac06c738c9b5ad932934fe4b07215e1c5c0546 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Thu, 12 Jul 2018 13:25:42 -0400 Subject: [PATCH 08/12] sort suggestions by distance --- src/cargo/core/resolver/mod.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 7b7df93a540..351f195b89f 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -59,6 +59,7 @@ use core::PackageIdSpec; use core::{Dependency, PackageId, Registry, Summary}; use util::config::Config; use util::errors::{CargoError, CargoResult}; +use util::lev_distance::lev_distance; use util::profile; use self::context::{Activations, Context}; @@ -982,8 +983,6 @@ fn activation_error( if let Err(e) = registry.query(&new_dep, &mut |s| candidates.push(s.name()), true) { return e }; - candidates.sort_unstable(); - candidates.dedup(); let mut msg = format!( "no matching package named `{}` found\n\ location searched: {}\n", @@ -991,8 +990,21 @@ fn activation_error( dep.source_id() ); if !candidates.is_empty() { + candidates.sort_unstable(); + candidates.dedup(); + candidates.sort_by_key(|o| lev_distance(&*new_dep.name(), &*o)); + let mut names = candidates + .iter() + .take(3) + .map(|c| c.to_string()) + .collect::>(); + + if candidates.len() > 3 { + names.push("...".into()); + } + msg.push_str("did you mean: "); - msg.push_str(&candidates.join(" or ")); + msg.push_str(&names.join(", ")); msg.push_str("\n"); } msg.push_str("required by "); From 335f64180fb6351378b8b0b62df50b9ea8f6209e Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Fri, 13 Jul 2018 16:27:42 -0400 Subject: [PATCH 09/12] move the Uncanonicalizing loop to a Iterator --- src/cargo/sources/registry/index.rs | 82 ++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 26 deletions(-) diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 4768e6dea8b..af5ef73f198 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -11,6 +11,61 @@ use sources::registry::RegistryData; use sources::registry::{RegistryPackage, INDEX_LOCK}; use util::{internal, CargoResult, Config, Filesystem}; +/// Crates.io treats hyphen and underscores as interchangeable +/// but, the index and old cargo do not. So the index must store uncanonicalized version +/// of the name so old cargos can find it. +/// This loop tries all possible combinations of switching +/// hyphen and underscores to find the uncanonicalized one. +/// As all stored inputs have the correct spelling, we start with the spelling as provided. +struct UncanonicalizedIter<'s> { + input: &'s str, + num_hyphen_underscore: u32, + hyphen_combination_num: u16, +} + +impl<'s> UncanonicalizedIter<'s> { + fn new(input: &'s str) -> Self { + let num_hyphen_underscore = input.chars().filter(|&c| c == '_' || c == '-').count() as u32; + UncanonicalizedIter { + input, + num_hyphen_underscore, + hyphen_combination_num: 0, + } + } +} + +impl<'s> Iterator for UncanonicalizedIter<'s> { + type Item = String; + + fn next(&mut self) -> Option { + if self.hyphen_combination_num > 0 && self.hyphen_combination_num.trailing_zeros() >= self.num_hyphen_underscore { + return None; + } + + let ret = Some(self.input + .chars() + .scan(0u16, |s, c| { + // the check against 15 here's to prevent + // shift overflow on inputs with more then 15 hyphens + if (c == '_' || c == '-') && *s <= 15 { + let switch = (self.hyphen_combination_num & (1u16 << *s)) > 0; + let out = if (c == '_') ^ switch { + '_' + } else { + '-' + }; + *s += 1; + Some(out) + } else { + Some(c) + } + }) + .collect()); + self.hyphen_combination_num += 1; + ret + } +} + pub struct RegistryIndex<'cfg> { source_id: SourceId, path: Filesystem, @@ -105,33 +160,8 @@ impl<'cfg> RegistryIndex<'cfg> { 3 => format!("3/{}/{}", &fs_name[..1], fs_name), _ => format!("{}/{}/{}", &fs_name[0..2], &fs_name[2..4], fs_name), }; - let num_hyphen_underscore = ::std::cmp::min( - raw_path.chars().filter(|&c| c == '_' || c == '-').count(), - 10, - ) as u16; let mut ret = Vec::new(); - // Crates.io treats hyphen and underscores as interchangeable - // but, the index and old cargo do not. So the index must store uncanonicalized version - // of the name so old cargos can find it. - // This loop tries all possible combinations of switching - // hyphen and underscores to find the uncanonicalized one. - for hyphen_combination_num in 0u16..(1 << num_hyphen_underscore) { - let path = raw_path - .chars() - .scan(0u16, |s, c| { - if (c == '_' || c == '-') && *s <= num_hyphen_underscore { - let out = Some(if (c == '_') ^ ((hyphen_combination_num & (1u16 << *s)) > 0) { - '_' - } else { - '-' - }); - *s += 1; - out - } else { - Some(c) - } - }) - .collect::(); + for path in UncanonicalizedIter::new(&raw_path).take(1024) { let mut hit_closure = false; let err = load.load(&root, Path::new(&path), &mut |contents| { hit_closure = true; From fc7ef626f7f4a515ea5ab088b92e6982aed90da8 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Fri, 13 Jul 2018 16:39:09 -0400 Subject: [PATCH 10/12] add tests --- src/cargo/sources/registry/index.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index af5ef73f198..7541dd70c50 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -66,6 +66,30 @@ impl<'s> Iterator for UncanonicalizedIter<'s> { } } +#[test] +fn no_hyphen() { + assert_eq!( + UncanonicalizedIter::new("test").collect::>(), + vec!["test".to_string()] + ) +} + +#[test] +fn two_hyphen() { + assert_eq!( + UncanonicalizedIter::new("te-_st").collect::>(), + vec!["te-_st".to_string(), "te__st".to_string(), "te--st".to_string(), "te_-st".to_string()] + ) +} + +#[test] +fn overflow_hyphen() { + assert_eq!( + UncanonicalizedIter::new("te-_-_-_-_-_-_-_-_-st").take(100).count(), + 100 + ) +} + pub struct RegistryIndex<'cfg> { source_id: SourceId, path: Filesystem, From 98e1bdb7629c9628fc4614ba1259f0170e8cd490 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Fri, 13 Jul 2018 18:05:24 -0400 Subject: [PATCH 11/12] oops... --- tests/testsuite/directory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testsuite/directory.rs b/tests/testsuite/directory.rs index 7bf497d12b8..4434abb8108 100644 --- a/tests/testsuite/directory.rs +++ b/tests/testsuite/directory.rs @@ -230,7 +230,7 @@ error: failed to compile `bar v0.1.0`, intermediate artifacts can be found at `[ Caused by: no matching package named `baz` found location searched: registry `https://github.com/rust-lang/crates.io-index` -did you mean: bar or foo +did you mean: bar, foo required by package `bar v0.1.0` ", ), From 95b4640b32b692629c353734aa906dd31ee54aec Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Mon, 16 Jul 2018 15:28:53 -0400 Subject: [PATCH 12/12] only suggest lev_distance < 4 --- src/bin/cargo/main.rs | 9 ++++----- src/cargo/core/resolver/mod.rs | 37 +++++++++++++++++++++------------- tests/testsuite/build.rs | 1 - 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs index e5338e833d6..2d7456ab8cc 100644 --- a/src/bin/cargo/main.rs +++ b/src/bin/cargo/main.rs @@ -113,12 +113,11 @@ fn find_closest(config: &Config, cmd: &str) -> Option { let cmds = list_commands(config); // Only consider candidates with a lev_distance of 3 or less so we don't // suggest out-of-the-blue options. - let mut filtered = cmds.iter() - .map(|&(ref c, _)| (lev_distance(c, cmd), c)) + cmds.into_iter() + .map(|(c, _)| (lev_distance(&c, cmd), c)) .filter(|&(d, _)| d < 4) - .collect::>(); - filtered.sort_by(|a, b| a.0.cmp(&b.0)); - filtered.get(0).map(|slot| slot.1.clone()) + .min_by_key(|a| a.0) + .map(|slot| slot.1) } fn execute_external_subcommand(config: &Config, cmd: &str, args: &[&str]) -> CliResult { diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 351f195b89f..5de359717fa 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -231,7 +231,9 @@ fn activate_deps_loop( // to amortize the cost of the current time lookup. ticks += 1; if let Some(config) = config { - if config.shell().is_err_tty() && !printed && ticks % 1000 == 0 + if config.shell().is_err_tty() + && !printed + && ticks % 1000 == 0 && start.elapsed() - deps_time > time_to_print { printed = true; @@ -858,12 +860,14 @@ fn activation_error( msg.push_str("\nversions that meet the requirements `"); msg.push_str(&dep.version_req().to_string()); msg.push_str("` are: "); - msg.push_str(&candidates - .iter() - .map(|v| v.summary.version()) - .map(|v| v.to_string()) - .collect::>() - .join(", ")); + msg.push_str( + &candidates + .iter() + .map(|v| v.summary.version()) + .map(|v| v.to_string()) + .collect::>() + .join(", "), + ); let mut conflicting_activations: Vec<_> = conflicting_activations.iter().collect(); conflicting_activations.sort_unstable(); @@ -926,7 +930,7 @@ 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=".2"` + // Maybe the user mistyped the ver_req? Like `dep="2"` when `dep="0.2"` // was meant. So we re-query the registry with `deb="*"` so we can // list a few versions that were actually found. let all_req = semver::VersionReq::parse("*").unwrap(); @@ -981,8 +985,16 @@ fn activation_error( // was meant. So we try asking the registry for a `fuzzy` search for suggestions. let mut candidates = Vec::new(); if let Err(e) = registry.query(&new_dep, &mut |s| candidates.push(s.name()), true) { - return e + return e; }; + candidates.sort_unstable(); + candidates.dedup(); + let mut candidates: Vec<_> = candidates + .iter() + .map(|n| (lev_distance(&*new_dep.name(), &*n), n)) + .filter(|&(d, _)| d < 4) + .collect(); + candidates.sort_by_key(|o| o.0); let mut msg = format!( "no matching package named `{}` found\n\ location searched: {}\n", @@ -990,17 +1002,14 @@ fn activation_error( dep.source_id() ); if !candidates.is_empty() { - candidates.sort_unstable(); - candidates.dedup(); - candidates.sort_by_key(|o| lev_distance(&*new_dep.name(), &*o)); let mut names = candidates .iter() .take(3) - .map(|c| c.to_string()) + .map(|c| c.1.as_str()) .collect::>(); if candidates.len() > 3 { - names.push("...".into()); + names.push("..."); } msg.push_str("did you mean: "); diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 4b2ac3546eb..3c02873d8ac 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -1020,7 +1020,6 @@ fn cargo_compile_with_dep_name_mismatch() { execs().with_status(101).with_stderr(&format!( r#"error: no matching package named `notquitebar` found location searched: {proj_dir}/bar -did you mean: bar required by package `foo v0.0.1 ({proj_dir})` "#, proj_dir = p.url()