Skip to content

Commit caf3c37

Browse files
borsbors
committed
Auto merge of #10883 - epage:fuzz, r=Eh2406
refactor(source): Open query API for adding more types of queries ### What does this PR try to resolve? This refactors the Source/Registry traits from accepting a `fuzzy: bool` to accepting an enum so we can add alternative query styles in the future, as discussed in the Cargo team meeting for fixing #10729 The expected fix for `cargo add` at this point would be - Add `QueryKind::Normalized` - Initially, we can make it like Exact for path sources and like Fuzzy for registry sources - Switch cargo-add to using that kind everywhere (both where `Exact` and `Fuzzy` are used) - A test to ensure this fixed it - Rename `Fuzzy` to `Alternatives` or something to clarify its actual intent ### How should we test and review this PR? The refactor is broken down into multiple commits, so ideally review a commit at a time to more easily see how it evolved. ### Additional information Future possibilities - Supporting normalized search on all sources - Doing version / source matching for normalized results (probably not needed for cargo-add but will make the API less surprising for future users)
2 parents d8d30a7 + a645c4f commit caf3c37

File tree

14 files changed

+99
-140
lines changed

14 files changed

+99
-140
lines changed

‎crates/resolver-tests/src/lib.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use std::time::Instant;
1212

1313
use cargo::core::dependency::DepKind;
1414
use cargo::core::resolver::{self, ResolveOpts, VersionPreferences};
15-
use cargo::core::source::{GitReference, SourceId};
15+
use cargo::core::source::{GitReference, QueryKind, SourceId};
1616
use cargo::core::Resolve;
1717
use cargo::core::{Dependency, PackageId, Registry, Summary};
1818
use cargo::util::{CargoResult, Config, Graph, IntoUrl};
@@ -128,11 +128,15 @@ pub fn resolve_with_config_raw(
128128
fn query(
129129
&mut self,
130130
dep: &Dependency,
131+
kind: QueryKind,
131132
f: &mut dyn FnMut(Summary),
132-
fuzzy: bool,
133133
) -> Poll<CargoResult<()>> {
134134
for summary in self.list.iter() {
135-
if fuzzy || dep.matches(summary) {
135+
let matched = match kind {
136+
QueryKind::Exact => dep.matches(summary),
137+
QueryKind::Fuzzy => true,
138+
};
139+
if matched {
136140
self.used.insert(summary.package_id());
137141
f(summary.clone());
138142
}

‎src/cargo/core/compiler/future_incompat.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Support for future-incompatible warning reporting.
22
33
use crate::core::compiler::BuildContext;
4-
use crate::core::{Dependency, PackageId, Workspace};
4+
use crate::core::{Dependency, PackageId, QueryKind, Workspace};
55
use crate::sources::SourceConfigMap;
66
use crate::util::{iter_join, CargoResult, Config};
77
use anyhow::{bail, format_err, Context};
@@ -293,7 +293,7 @@ fn get_updates(ws: &Workspace<'_>, package_ids: &BTreeSet<PackageId>) -> Option<
293293
Ok(dep) => dep,
294294
Err(_) => return false,
295295
};
296-
match source.query_vec(&dep) {
296+
match source.query_vec(&dep, QueryKind::Exact) {
297297
Poll::Ready(Ok(sum)) => {
298298
summaries.push((pkg_id, sum));
299299
false

‎src/cargo/core/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ pub use self::package_id_spec::PackageIdSpec;
88
pub use self::registry::Registry;
99
pub use self::resolver::{Resolve, ResolveVersion};
1010
pub use self::shell::{Shell, Verbosity};
11-
pub use self::source::{GitReference, Source, SourceId, SourceMap};
11+
pub use self::source::{GitReference, QueryKind, Source, SourceId, SourceMap};
1212
pub use self::summary::{FeatureMap, FeatureValue, Summary};
1313
pub use self::workspace::{
1414
find_workspace_root, resolve_relative_path, MaybePackage, Workspace, WorkspaceConfig,

‎src/cargo/core/registry.rs

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::collections::{HashMap, HashSet};
22
use std::task::Poll;
33

44
use crate::core::PackageSet;
5-
use crate::core::{Dependency, PackageId, Source, SourceId, SourceMap, Summary};
5+
use crate::core::{Dependency, PackageId, QueryKind, Source, SourceId, SourceMap, Summary};
66
use crate::sources::config::SourceConfigMap;
77
use crate::util::errors::CargoResult;
88
use crate::util::interning::InternedString;
@@ -19,14 +19,13 @@ pub trait Registry {
1919
fn query(
2020
&mut self,
2121
dep: &Dependency,
22+
kind: QueryKind,
2223
f: &mut dyn FnMut(Summary),
23-
fuzzy: bool,
2424
) -> Poll<CargoResult<()>>;
2525

26-
fn query_vec(&mut self, dep: &Dependency, fuzzy: bool) -> Poll<CargoResult<Vec<Summary>>> {
26+
fn query_vec(&mut self, dep: &Dependency, kind: QueryKind) -> Poll<CargoResult<Vec<Summary>>> {
2727
let mut ret = Vec::new();
28-
self.query(dep, &mut |s| ret.push(s), fuzzy)
29-
.map_ok(|()| ret)
28+
self.query(dep, kind, &mut |s| ret.push(s)).map_ok(|()| ret)
3029
}
3130

3231
fn describe_source(&self, source: SourceId) -> String;
@@ -327,7 +326,7 @@ impl<'cfg> PackageRegistry<'cfg> {
327326
.get_mut(dep.source_id())
328327
.expect("loaded source not present");
329328

330-
let summaries = match source.query_vec(dep)? {
329+
let summaries = match source.query_vec(dep, QueryKind::Exact)? {
331330
Poll::Ready(deps) => deps,
332331
Poll::Pending => {
333332
deps_pending.push(dep_remaining);
@@ -483,7 +482,7 @@ impl<'cfg> PackageRegistry<'cfg> {
483482
for &s in self.overrides.iter() {
484483
let src = self.sources.get_mut(s).unwrap();
485484
let dep = Dependency::new_override(dep.package_name(), s);
486-
let mut results = match src.query_vec(&dep) {
485+
let mut results = match src.query_vec(&dep, QueryKind::Exact) {
487486
Poll::Ready(results) => results?,
488487
Poll::Pending => return Poll::Pending,
489488
};
@@ -575,8 +574,8 @@ impl<'cfg> Registry for PackageRegistry<'cfg> {
575574
fn query(
576575
&mut self,
577576
dep: &Dependency,
577+
kind: QueryKind,
578578
f: &mut dyn FnMut(Summary),
579-
fuzzy: bool,
580579
) -> Poll<CargoResult<()>> {
581580
assert!(self.patches_locked);
582581
let (override_summary, n, to_warn) = {
@@ -671,11 +670,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> {
671670
}
672671
f(lock(locked, all_patches, summary))
673672
};
674-
return if fuzzy {
675-
source.fuzzy_query(dep, callback)
676-
} else {
677-
source.query(dep, callback)
678-
};
673+
return source.query(dep, kind, callback);
679674
}
680675

681676
// If we have an override summary then we query the source
@@ -694,11 +689,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> {
694689
n += 1;
695690
to_warn = Some(summary);
696691
};
697-
let pend = if fuzzy {
698-
source.fuzzy_query(dep, callback)?
699-
} else {
700-
source.query(dep, callback)?
701-
};
692+
let pend = source.query(dep, kind, callback);
702693
if pend.is_pending() {
703694
return Poll::Pending;
704695
}
@@ -889,7 +880,7 @@ fn summary_for_patch(
889880
// No summaries found, try to help the user figure out what is wrong.
890881
if let Some(locked) = locked {
891882
// Since the locked patch did not match anything, try the unlocked one.
892-
let orig_matches = match source.query_vec(orig_patch) {
883+
let orig_matches = match source.query_vec(orig_patch, QueryKind::Exact) {
893884
Poll::Pending => return Poll::Pending,
894885
Poll::Ready(deps) => deps,
895886
}
@@ -914,7 +905,7 @@ fn summary_for_patch(
914905
// Try checking if there are *any* packages that match this by name.
915906
let name_only_dep = Dependency::new_override(orig_patch.package_name(), orig_patch.source_id());
916907

917-
let name_summaries = match source.query_vec(&name_only_dep) {
908+
let name_summaries = match source.query_vec(&name_only_dep, QueryKind::Exact) {
918909
Poll::Pending => return Poll::Pending,
919910
Poll::Ready(deps) => deps,
920911
}

‎src/cargo/core/resolver/dep_cache.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ use crate::core::resolver::{
1616
ActivateError, ActivateResult, CliFeatures, RequestedFeatures, ResolveOpts, VersionOrdering,
1717
VersionPreferences,
1818
};
19-
use crate::core::{Dependency, FeatureValue, PackageId, PackageIdSpec, Registry, Summary};
19+
use crate::core::{
20+
Dependency, FeatureValue, PackageId, PackageIdSpec, QueryKind, Registry, Summary,
21+
};
2022
use crate::util::errors::CargoResult;
2123
use crate::util::interning::InternedString;
2224

@@ -100,13 +102,9 @@ impl<'a> RegistryQueryer<'a> {
100102
}
101103

102104
let mut ret = Vec::new();
103-
let ready = self.registry.query(
104-
dep,
105-
&mut |s| {
106-
ret.push(s);
107-
},
108-
false,
109-
)?;
105+
let ready = self.registry.query(dep, QueryKind::Exact, &mut |s| {
106+
ret.push(s);
107+
})?;
110108
if ready.is_pending() {
111109
self.registry_cache.insert(dep.clone(), Poll::Pending);
112110
return Poll::Pending;
@@ -127,7 +125,7 @@ impl<'a> RegistryQueryer<'a> {
127125
dep.version_req()
128126
);
129127

130-
let mut summaries = match self.registry.query_vec(dep, false)? {
128+
let mut summaries = match self.registry.query_vec(dep, QueryKind::Exact)? {
131129
Poll::Ready(s) => s.into_iter(),
132130
Poll::Pending => {
133131
self.registry_cache.insert(dep.clone(), Poll::Pending);

‎src/cargo/core/resolver/errors.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::fmt;
22
use std::task::Poll;
33

4-
use crate::core::{Dependency, PackageId, Registry, Summary};
4+
use crate::core::{Dependency, PackageId, QueryKind, Registry, Summary};
55
use crate::util::lev_distance::lev_distance;
66
use crate::util::{Config, VersionExt};
77
use anyhow::Error;
@@ -228,7 +228,7 @@ pub(super) fn activation_error(
228228
new_dep.set_version_req(all_req);
229229

230230
let mut candidates = loop {
231-
match registry.query_vec(&new_dep, false) {
231+
match registry.query_vec(&new_dep, QueryKind::Exact) {
232232
Poll::Ready(Ok(candidates)) => break candidates,
233233
Poll::Ready(Err(e)) => return to_resolve_err(e),
234234
Poll::Pending => match registry.block_until_ready() {
@@ -294,7 +294,7 @@ pub(super) fn activation_error(
294294
// Maybe the user mistyped the name? Like `dep-thing` when `Dep_Thing`
295295
// was meant. So we try asking the registry for a `fuzzy` search for suggestions.
296296
let mut candidates = loop {
297-
match registry.query_vec(&new_dep, true) {
297+
match registry.query_vec(&new_dep, QueryKind::Fuzzy) {
298298
Poll::Ready(Ok(candidates)) => break candidates,
299299
Poll::Ready(Err(e)) => return to_resolve_err(e),
300300
Poll::Pending => match registry.block_until_ready() {

‎src/cargo/core/source/mod.rs

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,16 @@ pub trait Source {
2929
fn requires_precise(&self) -> bool;
3030

3131
/// Attempts to find the packages that match a dependency request.
32-
fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> Poll<CargoResult<()>>;
33-
34-
/// Attempts to find the packages that are close to a dependency request.
35-
/// Each source gets to define what `close` means for it.
36-
/// Path/Git sources may return all dependencies that are at that URI,
37-
/// whereas an `Index` source may return dependencies that have the same canonicalization.
38-
fn fuzzy_query(
32+
fn query(
3933
&mut self,
4034
dep: &Dependency,
35+
kind: QueryKind,
4136
f: &mut dyn FnMut(Summary),
4237
) -> Poll<CargoResult<()>>;
4338

44-
fn query_vec(&mut self, dep: &Dependency) -> Poll<CargoResult<Vec<Summary>>> {
39+
fn query_vec(&mut self, dep: &Dependency, kind: QueryKind) -> Poll<CargoResult<Vec<Summary>>> {
4540
let mut ret = Vec::new();
46-
self.query(dep, &mut |s| ret.push(s)).map_ok(|_| ret)
41+
self.query(dep, kind, &mut |s| ret.push(s)).map_ok(|_| ret)
4742
}
4843

4944
/// Ensure that the source is fully up-to-date for the current session on the next query.
@@ -115,6 +110,15 @@ pub trait Source {
115110
fn block_until_ready(&mut self) -> CargoResult<()>;
116111
}
117112

113+
#[derive(Copy, Clone, PartialEq, Eq)]
114+
pub enum QueryKind {
115+
Exact,
116+
/// Each source gets to define what `close` means for it.
117+
/// Path/Git sources may return all dependencies that are at that URI,
118+
/// whereas an `Index` source may return dependencies that have the same canonicalization.
119+
Fuzzy,
120+
}
121+
118122
pub enum MaybePackage {
119123
Ready(Package),
120124
Download { url: String, descriptor: String },
@@ -142,17 +146,13 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box<T> {
142146
}
143147

144148
/// Forwards to `Source::query`.
145-
fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> Poll<CargoResult<()>> {
146-
(**self).query(dep, f)
147-
}
148-
149-
/// Forwards to `Source::query`.
150-
fn fuzzy_query(
149+
fn query(
151150
&mut self,
152151
dep: &Dependency,
152+
kind: QueryKind,
153153
f: &mut dyn FnMut(Summary),
154154
) -> Poll<CargoResult<()>> {
155-
(**self).fuzzy_query(dep, f)
155+
(**self).query(dep, kind, f)
156156
}
157157

158158
fn invalidate_cache(&mut self) {
@@ -216,16 +216,13 @@ impl<'a, T: Source + ?Sized + 'a> Source for &'a mut T {
216216
(**self).requires_precise()
217217
}
218218

219-
fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> Poll<CargoResult<()>> {
220-
(**self).query(dep, f)
221-
}
222-
223-
fn fuzzy_query(
219+
fn query(
224220
&mut self,
225221
dep: &Dependency,
222+
kind: QueryKind,
226223
f: &mut dyn FnMut(Summary),
227224
) -> Poll<CargoResult<()>> {
228-
(**self).fuzzy_query(dep, f)
225+
(**self).query(dep, kind, f)
229226
}
230227

231228
fn invalidate_cache(&mut self) {

‎src/cargo/ops/cargo_add/mod.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use toml_edit::Item as TomlItem;
1919
use crate::core::dependency::DepKind;
2020
use crate::core::registry::PackageRegistry;
2121
use crate::core::Package;
22+
use crate::core::QueryKind;
2223
use crate::core::Registry;
2324
use crate::core::Shell;
2425
use crate::core::Workspace;
@@ -443,8 +444,7 @@ fn get_latest_dependency(
443444
}
444445
MaybeWorkspace::Other(query) => {
445446
let possibilities = loop {
446-
let fuzzy = true;
447-
match registry.query_vec(&query, fuzzy) {
447+
match registry.query_vec(&query, QueryKind::Fuzzy) {
448448
std::task::Poll::Ready(res) => {
449449
break res?;
450450
}
@@ -485,8 +485,8 @@ fn select_package(
485485
}
486486
MaybeWorkspace::Other(query) => {
487487
let possibilities = loop {
488-
let fuzzy = false; // Returns all for path/git
489-
match registry.query_vec(&query, fuzzy) {
488+
// Exact to avoid returning all for path/git
489+
match registry.query_vec(&query, QueryKind::Exact) {
490490
std::task::Poll::Ready(res) => {
491491
break res?;
492492
}
@@ -600,7 +600,7 @@ fn populate_available_features(
600600
MaybeWorkspace::Other(query) => query,
601601
};
602602
let possibilities = loop {
603-
match registry.query_vec(&query, true) {
603+
match registry.query_vec(&query, QueryKind::Fuzzy) {
604604
std::task::Poll::Ready(res) => {
605605
break res?;
606606
}

‎src/cargo/ops/common_for_install_and_uninstall.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize};
1111
use toml_edit::easy as toml;
1212

1313
use crate::core::compiler::Freshness;
14-
use crate::core::{Dependency, FeatureValue, Package, PackageId, Source, SourceId};
14+
use crate::core::{Dependency, FeatureValue, Package, PackageId, QueryKind, Source, SourceId};
1515
use crate::ops::{self, CompileFilter, CompileOptions};
1616
use crate::sources::PathSource;
1717
use crate::util::errors::CargoResult;
@@ -540,7 +540,7 @@ where
540540
}
541541

542542
let deps = loop {
543-
match source.query_vec(&dep)? {
543+
match source.query_vec(&dep, QueryKind::Exact)? {
544544
Poll::Ready(deps) => break deps,
545545
Poll::Pending => source.block_until_ready()?,
546546
}

0 commit comments

Comments
 (0)