From 466e719c7a296b713219a8749755ffdd8310f0f6 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Wed, 16 Dec 2020 16:05:06 -0500 Subject: [PATCH 1/9] Source::{fuzzy_}query{_vec} can say "try again" --- crates/resolver-tests/src/lib.rs | 5 +- src/cargo/core/registry.rs | 185 +++++++++++------- src/cargo/core/resolver/dep_cache.rs | 34 +++- src/cargo/core/resolver/errors.rs | 11 +- src/cargo/core/resolver/mod.rs | 65 +++--- src/cargo/core/source/mod.rs | 30 ++- .../ops/common_for_install_and_uninstall.rs | 12 +- src/cargo/sources/directory.rs | 13 +- src/cargo/sources/git/source.rs | 9 +- src/cargo/sources/path.rs | 13 +- src/cargo/sources/registry/index.rs | 7 +- src/cargo/sources/registry/mod.rs | 29 ++- src/cargo/sources/replaced.rs | 15 +- 13 files changed, 282 insertions(+), 146 deletions(-) diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index b32dfc04330..e4db0b98b7a 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -8,6 +8,7 @@ use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::fmt; use std::fmt::Write; use std::rc::Rc; +use std::task::Poll; use std::time::Instant; use cargo::core::dependency::DepKind; @@ -130,14 +131,14 @@ pub fn resolve_with_config_raw( dep: &Dependency, f: &mut dyn FnMut(Summary), fuzzy: bool, - ) -> CargoResult<()> { + ) -> CargoResult> { for summary in self.list.iter() { if fuzzy || dep.matches(summary) { self.used.insert(summary.package_id()); f(summary.clone()); } } - Ok(()) + Ok(Poll::Ready(())) } fn describe_source(&self, _src: SourceId) -> String { diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 0380c447d39..59bff6b1df3 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -1,4 +1,5 @@ use std::collections::{HashMap, HashSet}; +use std::task::Poll; use crate::core::PackageSet; use crate::core::{Dependency, PackageId, Source, SourceId, SourceMap, Summary}; @@ -21,12 +22,11 @@ pub trait Registry { dep: &Dependency, f: &mut dyn FnMut(Summary), fuzzy: bool, - ) -> CargoResult<()>; + ) -> CargoResult>; - fn query_vec(&mut self, dep: &Dependency, fuzzy: bool) -> CargoResult> { + fn query_vec(&mut self, dep: &Dependency, fuzzy: bool) -> CargoResult>> { let mut ret = Vec::new(); - self.query(dep, &mut |s| ret.push(s), fuzzy)?; - Ok(ret) + Ok(self.query(dep, &mut |s| ret.push(s), fuzzy)?.map(|_| ret)) } fn describe_source(&self, source: SourceId) -> String; @@ -260,67 +260,85 @@ impl<'cfg> PackageRegistry<'cfg> { // Remember that each dependency listed in `[patch]` has to resolve to // precisely one package, so that's why we're just creating a flat list // of summaries which should be the same length as `deps` above. - let unlocked_summaries = deps - .iter() - .map(|(orig_patch, locked)| { - // Remove double reference in orig_patch. Is there maybe a - // magic pattern that could avoid this? - let orig_patch = *orig_patch; - // Use the locked patch if it exists, otherwise use the original. - let dep = match locked { - Some((locked_patch, _locked_id)) => locked_patch, - None => orig_patch, - }; - debug!( - "registering a patch for `{}` with `{}`", - url, - dep.package_name() - ); + let unlocked_summaries = loop { + let try_summaries = deps + .iter() + .map(|(orig_patch, locked)| { + // Remove double reference in orig_patch. Is there maybe a + // magic pattern that could avoid this? + let orig_patch = *orig_patch; + // Use the locked patch if it exists, otherwise use the original. + let dep = match locked { + Some((locked_patch, _locked_id)) => locked_patch, + None => orig_patch, + }; + debug!( + "registering a patch for `{}` with `{}`", + url, + dep.package_name() + ); - // Go straight to the source for resolving `dep`. Load it as we - // normally would and then ask it directly for the list of summaries - // corresponding to this `dep`. - self.ensure_loaded(dep.source_id(), Kind::Normal) + // Go straight to the source for resolving `dep`. Load it as we + // normally would and then ask it directly for the list of summaries + // corresponding to this `dep`. + self.ensure_loaded(dep.source_id(), Kind::Normal) + .chain_err(|| { + anyhow::format_err!( + "failed to load source for dependency `{}`", + dep.package_name() + ) + })?; + + let source = self + .sources + .get_mut(dep.source_id()) + .expect("loaded source not present"); + let summaries = match source.query_vec(dep)? { + Poll::Ready(deps) => deps, + Poll::Pending => return Ok(Poll::Pending), + }; + let (summary, should_unlock) = summary_for_patch( + orig_patch, locked, summaries, source, + ) .chain_err(|| { - anyhow::format_err!( - "failed to load source for dependency `{}`", - dep.package_name() - ) - })?; - - let source = self - .sources - .get_mut(dep.source_id()) - .expect("loaded source not present"); - let summaries = source.query_vec(dep)?; - let (summary, should_unlock) = - summary_for_patch(orig_patch, locked, summaries, source).chain_err(|| { format!( "patch for `{}` in `{}` failed to resolve", orig_patch.package_name(), url, ) })?; - debug!( - "patch summary is {:?} should_unlock={:?}", - summary, should_unlock - ); - if let Some(unlock_id) = should_unlock { - unlock_patches.push((orig_patch.clone(), unlock_id)); - } + debug!( + "patch summary is {:?} should_unlock={:?}", + summary, should_unlock + ); + if let Some(unlock_id) = should_unlock { + unlock_patches.push((orig_patch.clone(), unlock_id)); + } - if *summary.package_id().source_id().canonical_url() == canonical { - anyhow::bail!( - "patch for `{}` in `{}` points to the same source, but \ + if *summary.package_id().source_id().canonical_url() == canonical { + anyhow::bail!( + "patch for `{}` in `{}` points to the same source, but \ patches must point to different sources", - dep.package_name(), - url - ); - } - Ok(summary) - }) - .collect::>>() - .chain_err(|| anyhow::format_err!("failed to resolve patches for `{}`", url))?; + dep.package_name(), + url + ); + } + Ok(Poll::Ready(summary)) + }) + .collect::>>() + .chain_err(|| anyhow::format_err!("failed to resolve patches for `{}`", url))?; + if try_summaries.iter().all(|p| p.is_ready()) { + break try_summaries + .into_iter() + .map(|p| match p { + Poll::Ready(s) => s, + Poll::Pending => unreachable!("we just check for this!"), + }) + .collect::>(); + } else { + // TODO: dont hot loop for it to be Ready + } + }; let mut name_and_version = HashSet::new(); for summary in unlocked_summaries.iter() { @@ -396,9 +414,13 @@ impl<'cfg> PackageRegistry<'cfg> { for &s in self.overrides.iter() { let src = self.sources.get_mut(s).unwrap(); let dep = Dependency::new_override(dep.package_name(), s); - let mut results = src.query_vec(&dep)?; - if !results.is_empty() { - return Ok(Some(results.remove(0))); + match src.query_vec(&dep)? { + Poll::Ready(mut results) => { + if !results.is_empty() { + return Ok(Some(results.remove(0))); + } + } + Poll::Pending => bail!("overrides have to be on path deps, how did we get here?"), } } Ok(None) @@ -487,7 +509,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { dep: &Dependency, f: &mut dyn FnMut(Summary), fuzzy: bool, - ) -> CargoResult<()> { + ) -> CargoResult> { assert!(self.patches_locked); let (override_summary, n, to_warn) = { // Look for an override and get ready to query the real source. @@ -521,7 +543,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { Some(summary) => (summary, 1, Some(patch)), None => { f(patch); - return Ok(()); + return Ok(Poll::Ready(())); } } } else { @@ -549,7 +571,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { let source = self.sources.get_mut(dep.source_id()); match (override_summary, source) { (Some(_), None) => anyhow::bail!("override found but no real ones"), - (None, None) => return Ok(()), + (None, None) => return Ok(Poll::Ready(())), // If we don't have an override then we just ship // everything upstairs after locking the summary @@ -597,10 +619,13 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { n += 1; to_warn = Some(summary); }; - if fuzzy { - source.fuzzy_query(dep, callback)?; + let pend = if fuzzy { + source.fuzzy_query(dep, callback)? } else { - source.query(dep, callback)?; + source.query(dep, callback)? + }; + if pend.is_pending() { + return Ok(Poll::Pending); } } (override_summary, n, to_warn) @@ -615,7 +640,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { self.warn_bad_override(&override_summary, &summary)?; } f(self.lock(override_summary)); - Ok(()) + Ok(Poll::Ready(())) } fn describe_source(&self, id: SourceId) -> String { @@ -779,7 +804,20 @@ fn summary_for_patch( // No summaries found, try to help the user figure out what is wrong. if let Some((_locked_patch, locked_id)) = locked { // Since the locked patch did not match anything, try the unlocked one. - let orig_matches = source.query_vec(orig_patch).unwrap_or_else(|e| { + let orig_matches = loop { + match source.query_vec(orig_patch) { + Ok(Poll::Ready(deps)) => { + break Ok(deps); + } + Ok(Poll::Pending) => { + // TODO: dont hot loop for it to be Ready + } + Err(x) => { + break Err(x); + } + } + } + .unwrap_or_else(|e| { log::warn!( "could not determine unlocked summaries for dep {:?}: {:?}", orig_patch, @@ -794,7 +832,20 @@ fn summary_for_patch( } // Try checking if there are *any* packages that match this by name. let name_only_dep = Dependency::new_override(orig_patch.package_name(), orig_patch.source_id()); - let name_summaries = source.query_vec(&name_only_dep).unwrap_or_else(|e| { + let name_summaries = loop { + match source.query_vec(&name_only_dep) { + Ok(Poll::Ready(deps)) => { + break Ok(deps); + } + Ok(Poll::Pending) => { + // TODO: dont hot loop for it to be Ready + } + Err(x) => { + break Err(x); + } + } + } + .unwrap_or_else(|e| { log::warn!( "failed to do name-only summary query for {:?}: {:?}", name_only_dep, diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index 1f6c49ca0fb..4cc9aa03ecd 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -22,6 +22,7 @@ use log::debug; use std::cmp::Ordering; use std::collections::{BTreeSet, HashMap, HashSet}; use std::rc::Rc; +use std::task::Poll; pub struct RegistryQueryer<'a> { pub registry: &'a mut (dyn Registry + 'a), @@ -32,7 +33,7 @@ pub struct RegistryQueryer<'a> { /// specify minimum dependency versions to be used. minimal_versions: bool, /// a cache of `Candidate`s that fulfil a `Dependency` - registry_cache: HashMap>>, + registry_cache: HashMap>>>, /// a cache of `Dependency`s that are required for a `Summary` summary_cache: HashMap< (Option, Summary, ResolveOpts), @@ -67,6 +68,10 @@ impl<'a> RegistryQueryer<'a> { } } + pub fn all_ready(&self) -> bool { + self.registry_cache.values().all(|r| r.is_ready()) + } + pub fn used_replacement_for(&self, p: PackageId) -> Option<(PackageId, PackageId)> { self.used_replacements.get(&p).map(|r| (p, r.package_id())) } @@ -119,20 +124,24 @@ impl<'a> RegistryQueryer<'a> { /// any candidates are returned which match an override then the override is /// applied by performing a second query for what the override should /// return. - pub fn query(&mut self, dep: &Dependency) -> CargoResult>> { + pub fn query(&mut self, dep: &Dependency) -> CargoResult>>> { self.warn_colliding_git_sources(dep.source_id())?; if let Some(out) = self.registry_cache.get(dep).cloned() { return Ok(out); } let mut ret = Vec::new(); - self.registry.query( + let ready = self.registry.query( dep, &mut |s| { ret.push(s); }, false, )?; + if ready.is_pending() { + self.registry_cache.insert(dep.clone(), Poll::Pending); + return Ok(Poll::Pending); + } for summary in ret.iter_mut() { let mut potential_matches = self .replacements @@ -149,7 +158,13 @@ impl<'a> RegistryQueryer<'a> { dep.version_req() ); - let mut summaries = self.registry.query_vec(dep, false)?.into_iter(); + let mut summaries = match self.registry.query_vec(dep, false)? { + Poll::Ready(s) => s.into_iter(), + Poll::Pending => { + self.registry_cache.insert(dep.clone(), Poll::Pending); + return Ok(Poll::Pending); + } + }; let s = summaries.next().ok_or_else(|| { anyhow::format_err!( "no matching package for override `{}` found\n\ @@ -231,7 +246,7 @@ impl<'a> RegistryQueryer<'a> { } }); - let out = Rc::new(ret); + let out = Poll::Ready(Rc::new(ret)); self.registry_cache.insert(dep.clone(), out.clone()); @@ -267,15 +282,16 @@ impl<'a> RegistryQueryer<'a> { // which can satisfy that dependency. let mut deps = deps .into_iter() - .map(|(dep, features)| { - let candidates = self.query(&dep).chain_err(|| { + .filter_map(|(dep, features)| match self.query(&dep) { + Ok(Poll::Ready(candidates)) => Some(Ok((dep, candidates, features))), + Ok(Poll::Pending) => None, // we can ignore Pending deps, resolved will be repeatedly called until there are none to ignore + Err(x) => Some(Err(x).chain_err(|| { anyhow::format_err!( "failed to get `{}` as a dependency of {}", dep.package_name(), describe_path(&cx.parents.path_to_bottom(&candidate.package_id())), ) - })?; - Ok((dep, candidates, features)) + })), }) .collect::>>()?; diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index 8e9968db4e8..ce5ea1b06dd 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -1,4 +1,5 @@ use std::fmt; +use std::task::Poll; use crate::core::{Dependency, PackageId, Registry, Summary}; use crate::util::lev_distance::lev_distance; @@ -214,9 +215,13 @@ pub(super) fn activation_error( 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, false) { - Ok(candidates) => candidates, - Err(e) => return to_resolve_err(e), + + let mut candidates = loop { + match registry.query_vec(&new_dep, false) { + Ok(Poll::Ready(candidates)) => break candidates, + Ok(Poll::Pending) => (), // TODO: dont hot loop for it to be Ready + Err(e) => return to_resolve_err(e), + } }; candidates.sort_unstable_by(|a, b| b.version().cmp(a.version())); diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 0531f3dba89..eb8d78a9575 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -73,6 +73,8 @@ pub use self::features::{ForceAllTargets, HasDevUnits}; pub use self::resolve::{Resolve, ResolveVersion}; pub use self::types::{ResolveBehavior, ResolveOpts}; +use std::task::Poll; + mod conflict_cache; mod context; mod dep_cache; @@ -127,15 +129,22 @@ pub fn resolve( config: Option<&Config>, check_public_visible_dependencies: bool, ) -> CargoResult { - let cx = Context::new(check_public_visible_dependencies); let _p = profile::start("resolving"); let minimal_versions = match config { Some(config) => config.cli_unstable().minimal_versions, None => false, }; - let mut registry = - RegistryQueryer::new(registry, replacements, try_to_use, minimal_versions, config); - let cx = activate_deps_loop(cx, &mut registry, summaries, config)?; + let (registry, cx) = loop { + let mut registry = + RegistryQueryer::new(registry, replacements, try_to_use, minimal_versions, config); + let cx = Context::new(check_public_visible_dependencies); + let cx = activate_deps_loop(cx, &mut registry, summaries, config)?; + if registry.all_ready() { + break (registry, cx); + } else { + // TODO: dont hot loop for it to be Ready + } + }; let mut cksums = HashMap::new(); for (summary, _) in cx.activations.values() { @@ -852,30 +861,34 @@ fn generalize_conflicting( // A dep is equivalent to one of the things it can resolve to. // Thus, if all the things it can resolve to have already ben determined // to be conflicting, then we can just say that we conflict with the parent. - if let Some(others) = registry + if let Some(others) = match registry .query(critical_parents_dep) .expect("an already used dep now error!?") - .iter() - .rev() // the last one to be tried is the least likely to be in the cache, so start with that. - .map(|other| { - past_conflicting_activations - .find( - dep, - &|id| { - if id == other.package_id() { - // we are imagining that we used other instead - Some(backtrack_critical_age) - } else { - cx.is_active(id) - } - }, - Some(other.package_id()), - // we only care about things that are newer then critical_age - backtrack_critical_age, - ) - .map(|con| (other.package_id(), con)) - }) - .collect::>>() + { + Poll::Ready(q) => q, + Poll::Pending => unreachable!("an already used dep now pending!?"), + } + .iter() + .rev() // the last one to be tried is the least likely to be in the cache, so start with that. + .map(|other| { + past_conflicting_activations + .find( + dep, + &|id| { + if id == other.package_id() { + // we are imagining that we used other instead + Some(backtrack_critical_age) + } else { + cx.is_active(id) + } + }, + Some(other.package_id()), + // we only care about things that are newer then critical_age + backtrack_critical_age, + ) + .map(|con| (other.package_id(), con)) + }) + .collect::>>() { let mut con = conflicting_activations.clone(); // It is always valid to combine previously inserted conflicts. diff --git a/src/cargo/core/source/mod.rs b/src/cargo/core/source/mod.rs index f61e9636374..fdf66c2e97c 100644 --- a/src/cargo/core/source/mod.rs +++ b/src/cargo/core/source/mod.rs @@ -1,5 +1,6 @@ use std::collections::hash_map::HashMap; use std::fmt; +use std::task::Poll; use crate::core::package::PackageSet; use crate::core::{Dependency, Package, PackageId, Summary}; @@ -28,18 +29,21 @@ pub trait Source { fn requires_precise(&self) -> bool; /// Attempts to find the packages that match a dependency request. - fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()>; + fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult>; /// Attempts 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, /// whereas an `Index` source may return dependencies that have the same canonicalization. - fn fuzzy_query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()>; + fn fuzzy_query( + &mut self, + dep: &Dependency, + f: &mut dyn FnMut(Summary), + ) -> CargoResult>; - fn query_vec(&mut self, dep: &Dependency) -> CargoResult> { + fn query_vec(&mut self, dep: &Dependency) -> CargoResult>> { let mut ret = Vec::new(); - self.query(dep, &mut |s| ret.push(s))?; - Ok(ret) + Ok(self.query(dep, &mut |s| ret.push(s))?.map(|_| ret)) } /// Performs any network operations required to get the entire list of all names, @@ -130,12 +134,16 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box { } /// Forwards to `Source::query`. - fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { + fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult> { (**self).query(dep, f) } /// Forwards to `Source::query`. - fn fuzzy_query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { + fn fuzzy_query( + &mut self, + dep: &Dependency, + f: &mut dyn FnMut(Summary), + ) -> CargoResult> { (**self).fuzzy_query(dep, f) } @@ -197,11 +205,15 @@ impl<'a, T: Source + ?Sized + 'a> Source for &'a mut T { (**self).requires_precise() } - fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { + fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult> { (**self).query(dep, f) } - fn fuzzy_query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { + fn fuzzy_query( + &mut self, + dep: &Dependency, + f: &mut dyn FnMut(Summary), + ) -> CargoResult> { (**self).fuzzy_query(dep, f) } diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 3ab77a50389..c655e7a1e8e 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -3,6 +3,7 @@ use std::env; use std::io::prelude::*; use std::io::SeekFrom; use std::path::{Path, PathBuf}; +use std::task::Poll; use anyhow::{bail, format_err}; use serde::{Deserialize, Serialize}; @@ -537,7 +538,16 @@ where source.update()?; } - let deps = source.query_vec(&dep)?; + let deps = loop { + match source.query_vec(&dep)? { + Poll::Ready(deps) => { + break deps; + } + Poll::Pending => { + // TODO: dont hot loop for it to be Ready + } + } + }; match deps.iter().map(|p| p.package_id()).max() { Some(pkgid) => { let pkg = Box::new(source).download_now(pkgid, config)?; diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index 3e6daf034b8..cee8dc2bb9c 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -1,6 +1,7 @@ use std::collections::HashMap; use std::fmt::{self, Debug, Formatter}; use std::path::{Path, PathBuf}; +use std::task::Poll; use serde::Deserialize; @@ -42,21 +43,25 @@ impl<'cfg> Debug for DirectorySource<'cfg> { } impl<'cfg> Source for DirectorySource<'cfg> { - fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { + fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult> { let packages = self.packages.values().map(|p| &p.0); let matches = packages.filter(|pkg| dep.matches(pkg.summary())); for summary in matches.map(|pkg| pkg.summary().clone()) { f(summary); } - Ok(()) + Ok(Poll::Ready(())) } - fn fuzzy_query(&mut self, _dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { + fn fuzzy_query( + &mut self, + _dep: &Dependency, + f: &mut dyn FnMut(Summary), + ) -> CargoResult> { let packages = self.packages.values().map(|p| &p.0); for summary in packages.map(|pkg| pkg.summary().clone()) { f(summary); } - Ok(()) + Ok(Poll::Ready(())) } fn supports_checksums(&self) -> bool { diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index 0723e360628..3545ae8d873 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -9,6 +9,7 @@ use crate::util::Config; use anyhow::Context; use log::trace; use std::fmt::{self, Debug, Formatter}; +use std::task::Poll; use url::Url; pub struct GitSource<'cfg> { @@ -83,7 +84,7 @@ impl<'cfg> Debug for GitSource<'cfg> { } impl<'cfg> Source for GitSource<'cfg> { - fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { + fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult> { let src = self .path_source .as_mut() @@ -91,7 +92,11 @@ impl<'cfg> Source for GitSource<'cfg> { src.query(dep, f) } - fn fuzzy_query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { + fn fuzzy_query( + &mut self, + dep: &Dependency, + f: &mut dyn FnMut(Summary), + ) -> CargoResult> { let src = self .path_source .as_mut() diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 64b0f77ed5a..90b90da14fe 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -1,6 +1,7 @@ use std::fmt::{self, Debug, Formatter}; use std::fs; use std::path::{Path, PathBuf}; +use std::task::Poll; use filetime::FileTime; use ignore::gitignore::GitignoreBuilder; @@ -469,20 +470,24 @@ impl<'cfg> Debug for PathSource<'cfg> { } impl<'cfg> Source for PathSource<'cfg> { - fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { + fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult> { for s in self.packages.iter().map(|p| p.summary()) { if dep.matches(s) { f(s.clone()) } } - Ok(()) + Ok(Poll::Ready(())) } - fn fuzzy_query(&mut self, _dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { + fn fuzzy_query( + &mut self, + _dep: &Dependency, + f: &mut dyn FnMut(Summary), + ) -> CargoResult> { for s in self.packages.iter().map(|p| p.summary()) { f(s.clone()) } - Ok(()) + Ok(Poll::Ready(())) } fn supports_checksums(&self) -> bool { diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index c88726402f5..233888eb417 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -78,6 +78,7 @@ use std::collections::{HashMap, HashSet}; use std::fs; use std::path::Path; use std::str; +use std::task::Poll; /// Crates.io treats hyphen and underscores as interchangeable, but the index and old Cargo do not. /// Therefore, the index must store uncanonicalized version of the name so old Cargo's can find it. @@ -391,11 +392,11 @@ impl<'cfg> RegistryIndex<'cfg> { load: &mut dyn RegistryData, yanked_whitelist: &HashSet, f: &mut dyn FnMut(Summary), - ) -> CargoResult<()> { + ) -> CargoResult> { if self.config.offline() && self.query_inner_with_online(dep, load, yanked_whitelist, f, false)? != 0 { - return Ok(()); + return Ok(Poll::Ready(())); // If offline, and there are no matches, try again with online. // This is necessary for dependencies that are not used (such as // target-cfg or optional), but are not downloaded. Normally the @@ -406,7 +407,7 @@ impl<'cfg> RegistryIndex<'cfg> { // offline will be displayed. } self.query_inner_with_online(dep, load, yanked_whitelist, f, true)?; - Ok(()) + Ok(Poll::Ready(())) } fn query_inner_with_online( diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 495df40bfce..ca1c90bfc7a 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -164,6 +164,7 @@ use std::collections::HashSet; use std::fs::{File, OpenOptions}; use std::io::Write; use std::path::{Path, PathBuf}; +use std::task::Poll; use flate2::read::GzDecoder; use log::debug; @@ -657,7 +658,7 @@ impl<'cfg> RegistrySource<'cfg> { } impl<'cfg> Source for RegistrySource<'cfg> { - fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { + fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult> { // If this is a precise dependency, then it came from a lock file and in // theory the registry is known to contain this version. If, however, we // come back with no summaries, then our registry may need to be @@ -665,15 +666,19 @@ 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_inner(dep, &mut *self.ops, &self.yanked_whitelist, &mut |s| { - if dep.matches(&s) { - called = true; - f(s); - } - })?; + let pend = + self.index + .query_inner(dep, &mut *self.ops, &self.yanked_whitelist, &mut |s| { + if dep.matches(&s) { + called = true; + f(s); + } + })?; + if pend.is_pending() { + return Ok(Poll::Pending); + } if called { - return Ok(()); + return Ok(Poll::Ready(())); } else { debug!("falling back to an update"); self.do_update()?; @@ -688,7 +693,11 @@ impl<'cfg> Source for RegistrySource<'cfg> { }) } - fn fuzzy_query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { + fn fuzzy_query( + &mut self, + dep: &Dependency, + f: &mut dyn FnMut(Summary), + ) -> CargoResult> { self.index .query_inner(dep, &mut *self.ops, &self.yanked_whitelist, f) } diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs index 7f4a622fd84..b52a8e83060 100644 --- a/src/cargo/sources/replaced.rs +++ b/src/cargo/sources/replaced.rs @@ -1,6 +1,7 @@ use crate::core::source::MaybePackage; use crate::core::{Dependency, Package, PackageId, Source, SourceId, Summary}; use crate::util::errors::{CargoResult, CargoResultExt}; +use std::task::Poll; pub struct ReplacedSource<'cfg> { to_replace: SourceId, @@ -39,7 +40,7 @@ impl<'cfg> Source for ReplacedSource<'cfg> { self.inner.requires_precise() } - fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { + fn query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult> { let (replace_with, to_replace) = (self.replace_with, self.to_replace); let dep = dep.clone().map_source(to_replace, replace_with); @@ -47,11 +48,14 @@ impl<'cfg> Source for ReplacedSource<'cfg> { .query(&dep, &mut |summary| { f(summary.map_source(replace_with, to_replace)) }) - .chain_err(|| format!("failed to query replaced source {}", self.to_replace))?; - Ok(()) + .chain_err(|| format!("failed to query replaced source {}", self.to_replace)) } - fn fuzzy_query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { + fn fuzzy_query( + &mut self, + dep: &Dependency, + f: &mut dyn FnMut(Summary), + ) -> CargoResult> { let (replace_with, to_replace) = (self.replace_with, self.to_replace); let dep = dep.clone().map_source(to_replace, replace_with); @@ -59,8 +63,7 @@ impl<'cfg> Source for ReplacedSource<'cfg> { .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(()) + .chain_err(|| format!("failed to query replaced source {}", self.to_replace)) } fn update(&mut self) -> CargoResult<()> { From c3541997a8cec972c23d9014d2fcf6c885a467c9 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Fri, 18 Dec 2020 12:48:15 -0500 Subject: [PATCH 2/9] RegistryData::load can say "try again" --- src/cargo/sources/registry/index.rs | 168 ++++++++++++++++++--------- src/cargo/sources/registry/local.rs | 5 +- src/cargo/sources/registry/mod.rs | 55 ++++++--- src/cargo/sources/registry/remote.rs | 22 +++- 4 files changed, 170 insertions(+), 80 deletions(-) diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 233888eb417..31746dfc5b5 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -260,16 +260,23 @@ impl<'cfg> RegistryIndex<'cfg> { } /// Returns the hash listed for a specified `PackageId`. - pub fn hash(&mut self, pkg: PackageId, load: &mut dyn RegistryData) -> CargoResult<&str> { - let req = VersionReq::exact(pkg.version()); - let summary = self - .summaries(pkg.name(), &req, load)? - .next() - .ok_or_else(|| internal(format!("no hash listed for {}", pkg)))?; - summary - .summary - .checksum() - .ok_or_else(|| internal(format!("no hash listed for {}", pkg))) + pub fn hash(&mut self, pkg: PackageId, load: &mut dyn RegistryData) -> CargoResult> { + Ok( + match self.summaries(pkg.name(), &VersionReq::exact(pkg.version()), load)? { + Poll::Ready(mut summary) => { + let summary = summary + .next() + .ok_or_else(|| internal(format!("no hash listed for {}", pkg)))?; + Poll::Ready( + summary + .summary + .checksum() + .ok_or_else(|| internal(format!("no hash listed for {}", pkg)))?, + ) + } + Poll::Pending => Poll::Pending, + }, + ) } /// Load a list of summaries for `name` package in this registry which @@ -284,7 +291,7 @@ impl<'cfg> RegistryIndex<'cfg> { name: InternedString, req: &'b VersionReq, load: &mut dyn RegistryData, - ) -> CargoResult + 'b> + ) -> CargoResult + 'b>> where 'a: 'b, { @@ -297,7 +304,12 @@ impl<'cfg> RegistryIndex<'cfg> { // has run previously this will parse a Cargo-specific cache file rather // than the registry itself. In effect this is intended to be a quite // cheap operation. - let summaries = self.load_summaries(name, load)?; + let summaries = match self.load_summaries(name, load)? { + Poll::Ready(x) => x, + Poll::Pending => { + return Ok(Poll::Pending); + } + }; // Iterate over our summaries, extract all relevant ones which match our // version requirement, and then parse all corresponding rows in the @@ -306,35 +318,37 @@ impl<'cfg> RegistryIndex<'cfg> { // minimize the amount of work being done here and parse as little as // necessary. let raw_data = &summaries.raw_data; - Ok(summaries - .versions - .iter_mut() - .filter_map(move |(k, v)| if req.matches(k) { Some(v) } else { None }) - .filter_map( - move |maybe| match maybe.parse(config, raw_data, source_id) { - Ok(summary) => Some(summary), - Err(e) => { - info!("failed to parse `{}` registry package: {}", name, e); - None - } - }, - ) - .filter(move |is| { - is.summary - .unstable_gate(namespaced_features, weak_dep_features) - .is_ok() - })) + Ok(Poll::Ready( + summaries + .versions + .iter_mut() + .filter_map(move |(k, v)| if req.matches(k) { Some(v) } else { None }) + .filter_map( + move |maybe| match maybe.parse(config, raw_data, source_id) { + Ok(summary) => Some(summary), + Err(e) => { + info!("failed to parse `{}` registry package: {}", name, e); + None + } + }, + ) + .filter(move |is| { + is.summary + .unstable_gate(namespaced_features, weak_dep_features) + .is_ok() + }), + )) } fn load_summaries( &mut self, name: InternedString, load: &mut dyn RegistryData, - ) -> CargoResult<&mut Summaries> { + ) -> CargoResult> { // If we've previously loaded what versions are present for `name`, just // return that since our cache should still be valid. if self.summaries_cache.contains_key(&name) { - return Ok(self.summaries_cache.get_mut(&name).unwrap()); + return Ok(Poll::Ready(self.summaries_cache.get_mut(&name).unwrap())); } // Prepare the `RegistryData` which will lazily initialize internal data @@ -352,19 +366,25 @@ impl<'cfg> RegistryIndex<'cfg> { .chars() .flat_map(|c| c.to_lowercase()) .collect::(); - 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 raw_path = |name: String| match name.len() { + 1 => format!("1/{}", name), + 2 => format!("2/{}", name), + 3 => format!("3/{}/{}", &name[..1], name), + _ => format!("{}/{}/{}", &name[0..2], &name[2..4], name), }; + let mut any_pending = false; + // Attempt to handle misspellings by searching for a chain of related // names to the original `raw_path` name. Only return summaries // associated with the first hit, however. The resolver will later // reject any candidates that have the wrong name, and with this it'll // along the way produce helpful "did you mean?" suggestions. - for path in UncanonicalizedIter::new(&raw_path).take(1024) { + for (i, path) in UncanonicalizedIter::new(&fs_name) + .map(raw_path) + .take(1024) + .enumerate() + { let summaries = Summaries::parse( index_version.as_deref(), root, @@ -374,16 +394,30 @@ impl<'cfg> RegistryIndex<'cfg> { load, self.config, )?; - if let Some(summaries) = summaries { + if summaries.is_pending() { + if i == 0 { + // If we have not herd back about the name as requested + // then dont ask about other spellings yet. + // This prevents us spamming all the variations in the + // case where we have the correct spelling. + return Ok(Poll::Pending); + } + any_pending = true; + } + if let Poll::Ready(Some(summaries)) = summaries { self.summaries_cache.insert(name, summaries); - return Ok(self.summaries_cache.get_mut(&name).unwrap()); + return Ok(Poll::Ready(self.summaries_cache.get_mut(&name).unwrap())); } } + if any_pending { + return Ok(Poll::Pending); + } + // If nothing was found then this crate doesn't exists, so just use an // empty `Summaries` list. self.summaries_cache.insert(name, Summaries::default()); - Ok(self.summaries_cache.get_mut(&name).unwrap()) + Ok(Poll::Ready(self.summaries_cache.get_mut(&name).unwrap())) } pub fn query_inner( @@ -394,7 +428,8 @@ impl<'cfg> RegistryIndex<'cfg> { f: &mut dyn FnMut(Summary), ) -> CargoResult> { if self.config.offline() - && self.query_inner_with_online(dep, load, yanked_whitelist, f, false)? != 0 + && self.query_inner_with_online(dep, load, yanked_whitelist, f, false)? + != Poll::Ready(0) { return Ok(Poll::Ready(())); // If offline, and there are no matches, try again with online. @@ -406,8 +441,9 @@ impl<'cfg> RegistryIndex<'cfg> { // indicating that the required dependency is unavailable while // offline will be displayed. } - self.query_inner_with_online(dep, load, yanked_whitelist, f, true)?; - Ok(Poll::Ready(())) + Ok(self + .query_inner_with_online(dep, load, yanked_whitelist, f, true)? + .map(|_| ())) } fn query_inner_with_online( @@ -417,10 +453,17 @@ impl<'cfg> RegistryIndex<'cfg> { yanked_whitelist: &HashSet, f: &mut dyn FnMut(Summary), online: bool, - ) -> CargoResult { + ) -> CargoResult> { let source_id = self.source_id; - let summaries = self - .summaries(dep.package_name(), dep.version_req(), load)? + + let summaries = match self.summaries(dep.package_name(), dep.version_req(), load)? { + Poll::Ready(x) => x, + Poll::Pending => { + return Ok(Poll::Pending); + } + }; + + let summaries = summaries // First filter summaries for `--offline`. If we're online then // everything is a candidate, otherwise if we're offline we're only // going to consider candidates which are actually present on disk. @@ -464,15 +507,21 @@ impl<'cfg> RegistryIndex<'cfg> { f(summary); count += 1; } - Ok(count) + Ok(Poll::Ready(count)) } pub fn is_yanked(&mut self, pkg: PackageId, load: &mut dyn RegistryData) -> CargoResult { let req = VersionReq::exact(pkg.version()); - let found = self - .summaries(pkg.name(), &req, load)? - .any(|summary| summary.yanked); - Ok(found) + loop { + match self.summaries(pkg.name(), &req, load)? { + Poll::Ready(mut summaries) => { + return Ok(summaries.any(|summary| summary.yanked)); + } + Poll::Pending => { + // TODO: dont hot loop for it to be Ready + } + } + } } } @@ -506,7 +555,7 @@ impl Summaries { source_id: SourceId, load: &mut dyn RegistryData, config: &Config, - ) -> CargoResult> { + ) -> CargoResult>> { // First up, attempt to load the cache. This could fail for all manner // of reasons, but consider all of them non-fatal and just log their // occurrence in case anyone is debugging anything. @@ -520,7 +569,7 @@ impl Summaries { if cfg!(debug_assertions) { cache_contents = Some(s.raw_data); } else { - return Ok(Some(s)); + return Ok(Poll::Ready(Some(s))); } } Err(e) => { @@ -565,14 +614,19 @@ impl Summaries { Ok(()) }); + if matches!(err, Ok(Poll::Pending)) { + assert!(!hit_closure); + return Ok(Poll::Pending); + } + // 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 { debug_assert!(cache_contents.is_none()); - return Ok(None); + return Ok(Poll::Ready(None)); } - err?; + let _ = err?; // If we've got debug assertions enabled and the cache was previously // present and considered fresh this is where the debug assertions @@ -598,7 +652,7 @@ impl Summaries { } } - Ok(Some(ret)) + Ok(Poll::Ready(Some(ret))) } /// Parses an open `File` which represents information previously cached by diff --git a/src/cargo/sources/registry/local.rs b/src/cargo/sources/registry/local.rs index 7276c688b58..641524e5663 100644 --- a/src/cargo/sources/registry/local.rs +++ b/src/cargo/sources/registry/local.rs @@ -8,6 +8,7 @@ use std::fs::File; use std::io::prelude::*; use std::io::SeekFrom; use std::path::Path; +use std::task::Poll; /// A local registry is a registry that lives on the filesystem as a set of /// `.crate` files with an `index` directory in the same format as a remote @@ -54,8 +55,8 @@ impl<'cfg> RegistryData for LocalRegistry<'cfg> { root: &Path, path: &Path, data: &mut dyn FnMut(&[u8]) -> CargoResult<()>, - ) -> CargoResult<()> { - data(&paths::read_bytes(&root.join(path))?) + ) -> CargoResult> { + Ok(Poll::Ready(data(&paths::read_bytes(&root.join(path))?)?)) } fn config(&mut self) -> CargoResult> { diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index ca1c90bfc7a..569e3280181 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -407,12 +407,14 @@ pub trait RegistryData { /// * `root` is the root path to the index. /// * `path` is the relative path to the package to load (like `ca/rg/cargo`). /// * `data` is a callback that will receive the raw bytes of the index JSON file. + /// + /// If `load` returns a `Poll::Pending` then it must not have called data. fn load( &self, root: &Path, path: &Path, data: &mut dyn FnMut(&[u8]) -> CargoResult<()>, - ) -> CargoResult<()>; + ) -> CargoResult>; /// Loads the `config.json` file and returns it. /// @@ -641,12 +643,17 @@ impl<'cfg> RegistrySource<'cfg> { // After we've loaded the package configure its summary's `checksum` // field with the checksum we know for this `PackageId`. let req = VersionReq::exact(package.version()); - let summary_with_cksum = self - .index - .summaries(package.name(), &req, &mut *self.ops)? - .map(|s| s.summary.clone()) - .next() - .expect("summary not found"); + let summary_with_cksum = loop { + match self.index.summaries(package.name(), &req, &mut *self.ops)? { + Poll::Ready(summaries) => { + break summaries; + } + Poll::Pending => (), // TODO: dont hot loop for it to be Ready + } + } + .map(|s| s.summary.clone()) + .next() + .expect("summary not found"); if let Some(cksum) = summary_with_cksum.checksum() { pkg.manifest_mut() .summary_mut() @@ -731,19 +738,37 @@ impl<'cfg> Source for RegistrySource<'cfg> { } fn download(&mut self, package: PackageId) -> CargoResult { - let hash = self.index.hash(package, &mut *self.ops)?; - match self.ops.download(package, hash)? { - MaybeLock::Ready(file) => self.get_pkg(package, &file).map(MaybePackage::Ready), - MaybeLock::Download { url, descriptor } => { - Ok(MaybePackage::Download { url, descriptor }) + loop { + match self.index.hash(package, &mut *self.ops)? { + Poll::Ready(hash) => { + return match self.ops.download(package, hash)? { + MaybeLock::Ready(file) => { + self.get_pkg(package, &file).map(MaybePackage::Ready) + } + MaybeLock::Download { url, descriptor } => { + Ok(MaybePackage::Download { url, descriptor }) + } + } + } + Poll::Pending => { + // TODO: dont hot loop for it to be Ready + } } } } fn finish_download(&mut self, package: PackageId, data: Vec) -> CargoResult { - let hash = self.index.hash(package, &mut *self.ops)?; - let file = self.ops.finish_download(package, hash, &data)?; - self.get_pkg(package, &file) + loop { + match self.index.hash(package, &mut *self.ops)? { + Poll::Ready(hash) => { + let file = self.ops.finish_download(package, hash, &data)?; + return self.get_pkg(package, &file); + } + Poll::Pending => { + // TODO: dont hot loop for it to be Ready + } + } + } } fn fingerprint(&self, pkg: &Package) -> CargoResult { diff --git a/src/cargo/sources/registry/remote.rs b/src/cargo/sources/registry/remote.rs index d3f9eb9c03c..771d9c96452 100644 --- a/src/cargo/sources/registry/remote.rs +++ b/src/cargo/sources/registry/remote.rs @@ -19,6 +19,7 @@ use std::io::SeekFrom; use std::mem; use std::path::Path; use std::str; +use std::task::Poll; fn make_dep_prefix(name: &str) -> String { match name.len() { @@ -175,7 +176,7 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> { _root: &Path, path: &Path, data: &mut dyn FnMut(&[u8]) -> CargoResult<()>, - ) -> CargoResult<()> { + ) -> CargoResult> { // Note that the index calls this method and the filesystem is locked // in the index, so we don't need to worry about an `update_index` // happening in a different process. @@ -187,7 +188,7 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> { Some(blob) => blob, None => anyhow::bail!("path `{}` is not a blob in the git repo", path.display()), }; - data(blob.content()) + Ok(Poll::Ready(data(blob.content())?)) } fn config(&mut self) -> CargoResult> { @@ -195,10 +196,19 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> { self.prepare()?; self.config.assert_package_cache_locked(&self.index_path); let mut config = None; - self.load(Path::new(""), Path::new("config.json"), &mut |json| { - config = Some(serde_json::from_slice(json)?); - Ok(()) - })?; + loop { + match self.load(Path::new(""), Path::new("config.json"), &mut |json| { + config = Some(serde_json::from_slice(json)?); + Ok(()) + })? { + Poll::Ready(_) => { + break; + } + Poll::Pending => { + // TODO: dont hot loop for it to be Ready + } + } + } trace!("config loaded"); Ok(config) } From ea37a74afd6b12bed3646ee8616614f6c677962c Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Fri, 18 Dec 2020 14:53:09 -0500 Subject: [PATCH 3/9] pull the loop up out of summary_for_patch --- src/cargo/core/registry.rs | 165 +++++++++++++++++++------------------ 1 file changed, 85 insertions(+), 80 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 59bff6b1df3..82104dc64bd 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -261,72 +261,76 @@ impl<'cfg> PackageRegistry<'cfg> { // precisely one package, so that's why we're just creating a flat list // of summaries which should be the same length as `deps` above. let unlocked_summaries = loop { - let try_summaries = deps - .iter() - .map(|(orig_patch, locked)| { - // Remove double reference in orig_patch. Is there maybe a - // magic pattern that could avoid this? - let orig_patch = *orig_patch; - // Use the locked patch if it exists, otherwise use the original. - let dep = match locked { - Some((locked_patch, _locked_id)) => locked_patch, - None => orig_patch, - }; - debug!( - "registering a patch for `{}` with `{}`", - url, - dep.package_name() - ); - - // Go straight to the source for resolving `dep`. Load it as we - // normally would and then ask it directly for the list of summaries - // corresponding to this `dep`. - self.ensure_loaded(dep.source_id(), Kind::Normal) - .chain_err(|| { - anyhow::format_err!( - "failed to load source for dependency `{}`", - dep.package_name() - ) - })?; - - let source = self - .sources - .get_mut(dep.source_id()) - .expect("loaded source not present"); - let summaries = match source.query_vec(dep)? { - Poll::Ready(deps) => deps, - Poll::Pending => return Ok(Poll::Pending), - }; - let (summary, should_unlock) = summary_for_patch( - orig_patch, locked, summaries, source, - ) - .chain_err(|| { - format!( - "patch for `{}` in `{}` failed to resolve", - orig_patch.package_name(), + let try_summaries = + deps.iter() + .map(|(orig_patch, locked)| { + // Remove double reference in orig_patch. Is there maybe a + // magic pattern that could avoid this? + let orig_patch = *orig_patch; + // Use the locked patch if it exists, otherwise use the original. + let dep = match locked { + Some((locked_patch, _locked_id)) => locked_patch, + None => orig_patch, + }; + debug!( + "registering a patch for `{}` with `{}`", url, - ) - })?; - debug!( - "patch summary is {:?} should_unlock={:?}", - summary, should_unlock - ); - if let Some(unlock_id) = should_unlock { - unlock_patches.push((orig_patch.clone(), unlock_id)); - } + dep.package_name() + ); - if *summary.package_id().source_id().canonical_url() == canonical { - anyhow::bail!( - "patch for `{}` in `{}` points to the same source, but \ - patches must point to different sources", - dep.package_name(), - url + // Go straight to the source for resolving `dep`. Load it as we + // normally would and then ask it directly for the list of summaries + // corresponding to this `dep`. + self.ensure_loaded(dep.source_id(), Kind::Normal) + .chain_err(|| { + anyhow::format_err!( + "failed to load source for dependency `{}`", + dep.package_name() + ) + })?; + + let source = self + .sources + .get_mut(dep.source_id()) + .expect("loaded source not present"); + let summaries = match source.query_vec(dep)? { + Poll::Ready(deps) => deps, + Poll::Pending => return Ok(Poll::Pending), + }; + let (summary, should_unlock) = + match summary_for_patch(orig_patch, locked, summaries, source) + .chain_err(|| { + format!( + "patch for `{}` in `{}` failed to resolve", + orig_patch.package_name(), + url, + ) + })? { + Poll::Ready(x) => x, + Poll::Pending => { + return Ok(Poll::Pending); + } + }; + debug!( + "patch summary is {:?} should_unlock={:?}", + summary, should_unlock ); - } - Ok(Poll::Ready(summary)) - }) - .collect::>>() - .chain_err(|| anyhow::format_err!("failed to resolve patches for `{}`", url))?; + if let Some(unlock_id) = should_unlock { + unlock_patches.push((orig_patch.clone(), unlock_id)); + } + + if *summary.package_id().source_id().canonical_url() == canonical { + anyhow::bail!( + "patch for `{}` in `{}` points to the same source, but \ + patches must point to different sources", + dep.package_name(), + url + ); + } + Ok(Poll::Ready(summary)) + }) + .collect::>>() + .chain_err(|| anyhow::format_err!("failed to resolve patches for `{}`", url))?; if try_summaries.iter().all(|p| p.is_ready()) { break try_summaries .into_iter() @@ -773,9 +777,9 @@ fn summary_for_patch( locked: &Option<(Dependency, PackageId)>, mut summaries: Vec, source: &mut dyn Source, -) -> CargoResult<(Summary, Option)> { +) -> CargoResult)>> { if summaries.len() == 1 { - return Ok((summaries.pop().unwrap(), None)); + return Ok(Poll::Ready((summaries.pop().unwrap(), None))); } if summaries.len() > 1 { // TODO: In the future, it might be nice to add all of these @@ -825,25 +829,26 @@ fn summary_for_patch( ); Vec::new() }); - let (summary, _) = summary_for_patch(orig_patch, &None, orig_matches, source)?; - // The unlocked version found a match. This returns a value to - // indicate that this entry should be unlocked. - return Ok((summary, Some(*locked_id))); + return Ok( + match summary_for_patch(orig_patch, &None, orig_matches, source)? { + Poll::Ready((summary, _)) => { + // The unlocked version found a match. This returns a value to + // indicate that this entry should be unlocked. + Poll::Ready((summary, Some(*locked_id))) + } + Poll::Pending => Poll::Pending, + }, + ); } // Try checking if there are *any* packages that match this by name. let name_only_dep = Dependency::new_override(orig_patch.package_name(), orig_patch.source_id()); - let name_summaries = loop { - match source.query_vec(&name_only_dep) { - Ok(Poll::Ready(deps)) => { - break Ok(deps); - } - Ok(Poll::Pending) => { - // TODO: dont hot loop for it to be Ready - } - Err(x) => { - break Err(x); - } + + let name_summaries = match source.query_vec(&name_only_dep) { + Ok(Poll::Ready(deps)) => Ok(deps), + Ok(Poll::Pending) => { + return Ok(Poll::Pending); } + Err(x) => Err(x), } .unwrap_or_else(|e| { log::warn!( From fdf08ebc8f3071b42bbb1c3019d162ece5c233f3 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Fri, 18 Dec 2020 15:24:10 -0500 Subject: [PATCH 4/9] reuse the resolvers cache --- src/cargo/core/resolver/dep_cache.rs | 34 +++++++++++++++++++++------- src/cargo/core/resolver/mod.rs | 10 ++++---- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index 4cc9aa03ecd..c8b78c85ac5 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -37,7 +37,7 @@ pub struct RegistryQueryer<'a> { /// a cache of `Dependency`s that are required for a `Summary` summary_cache: HashMap< (Option, Summary, ResolveOpts), - Rc<(HashSet, Rc>)>, + (Rc<(HashSet, Rc>)>, bool), >, /// all the cases we ended up using a supplied replacement used_replacements: HashMap, @@ -68,8 +68,21 @@ impl<'a> RegistryQueryer<'a> { } } - pub fn all_ready(&self) -> bool { - self.registry_cache.values().all(|r| r.is_ready()) + pub fn reset_pending(&mut self) -> bool { + let mut all_ready = true; + self.registry_cache.retain(|_, r| { + if !r.is_ready() { + all_ready = false; + } + r.is_ready() + }); + self.summary_cache.retain(|_, (_, r)| { + if !*r { + all_ready = false; + } + *r + }); + all_ready } pub fn used_replacement_for(&self, p: PackageId) -> Option<(PackageId, PackageId)> { @@ -269,9 +282,8 @@ impl<'a> RegistryQueryer<'a> { if let Some(out) = self .summary_cache .get(&(parent, candidate.clone(), opts.clone())) - .cloned() { - return Ok(out); + return Ok(out.0.clone()); } // First, figure out our set of dependencies based on the requested set // of features. This also calculates what features we're going to enable @@ -280,11 +292,15 @@ impl<'a> RegistryQueryer<'a> { // Next, transform all dependencies into a list of possible candidates // which can satisfy that dependency. + let mut all_ready = true; let mut deps = deps .into_iter() .filter_map(|(dep, features)| match self.query(&dep) { Ok(Poll::Ready(candidates)) => Some(Ok((dep, candidates, features))), - Ok(Poll::Pending) => None, // we can ignore Pending deps, resolved will be repeatedly called until there are none to ignore + Ok(Poll::Pending) => { + all_ready = false; + None // we can ignore Pending deps, resolved will be repeatedly called until there are none to ignore + } Err(x) => Some(Err(x).chain_err(|| { anyhow::format_err!( "failed to get `{}` as a dependency of {}", @@ -305,8 +321,10 @@ impl<'a> RegistryQueryer<'a> { // If we succeed we add the result to the cache so we can use it again next time. // We don't cache the failure cases as they don't impl Clone. - self.summary_cache - .insert((parent, candidate.clone(), opts.clone()), out.clone()); + self.summary_cache.insert( + (parent, candidate.clone(), opts.clone()), + (out.clone(), all_ready), + ); Ok(out) } diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index eb8d78a9575..4302bf45bff 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -134,13 +134,13 @@ pub fn resolve( Some(config) => config.cli_unstable().minimal_versions, None => false, }; - let (registry, cx) = loop { - let mut registry = - RegistryQueryer::new(registry, replacements, try_to_use, minimal_versions, config); + let mut registry = + RegistryQueryer::new(registry, replacements, try_to_use, minimal_versions, config); + let cx = loop { let cx = Context::new(check_public_visible_dependencies); let cx = activate_deps_loop(cx, &mut registry, summaries, config)?; - if registry.all_ready() { - break (registry, cx); + if registry.reset_pending() { + break cx; } else { // TODO: dont hot loop for it to be Ready } From 240cb5f59c840d807557f7bc9e26880c2cb087a2 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Fri, 18 Dec 2020 17:25:01 -0500 Subject: [PATCH 5/9] don't retry just for an error message --- src/cargo/core/resolver/errors.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index ce5ea1b06dd..a4356a0e21a 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -1,10 +1,8 @@ -use std::fmt; -use std::task::Poll; - use crate::core::{Dependency, PackageId, Registry, Summary}; use crate::util::lev_distance::lev_distance; use crate::util::Config; use anyhow::Error; +use std::fmt; use super::context::Context; use super::types::{ConflictMap, ConflictReason}; @@ -216,12 +214,12 @@ pub(super) fn activation_error( let mut new_dep = dep.clone(); new_dep.set_version_req(all_req); - let mut candidates = loop { - match registry.query_vec(&new_dep, false) { - Ok(Poll::Ready(candidates)) => break candidates, - Ok(Poll::Pending) => (), // TODO: dont hot loop for it to be Ready - Err(e) => return to_resolve_err(e), - } + let mut candidates = Vec::new(); + // we can ignore the `Pending` case because we are just in an error reporting path, + // and we have probably already triggered the query anyway. But, if we start getting reports + // of confusing errors that go away when called again this is a place to look. + if let Err(e) = registry.query(&new_dep, &mut |s| candidates.push(s), false) { + return to_resolve_err(e); }; candidates.sort_unstable_by(|a, b| b.version().cmp(a.version())); @@ -275,6 +273,9 @@ pub(super) 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), true) { + // we can ignore the `Pending` case because we are just in an error reporting path, + // and we have probably already triggered the query anyway. But, if we start getting reports + // of confusing errors that go away when called again this is a place to look. return to_resolve_err(e); }; candidates.sort_unstable_by_key(|a| a.name()); From 08f96cdf5530f192ee94cdd154650531dd5ace4c Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Mon, 21 Dec 2020 13:36:07 -0500 Subject: [PATCH 6/9] move Poll one layer up in is_yanked --- src/cargo/sources/registry/index.rs | 20 ++++++++------------ src/cargo/sources/registry/mod.rs | 11 ++++++++++- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 31746dfc5b5..e47e2a92a7c 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -510,18 +510,14 @@ impl<'cfg> RegistryIndex<'cfg> { Ok(Poll::Ready(count)) } - pub fn is_yanked(&mut self, pkg: PackageId, load: &mut dyn RegistryData) -> CargoResult { - let req = VersionReq::exact(pkg.version()); - loop { - match self.summaries(pkg.name(), &req, load)? { - Poll::Ready(mut summaries) => { - return Ok(summaries.any(|summary| summary.yanked)); - } - Poll::Pending => { - // TODO: dont hot loop for it to be Ready - } - } - } + pub fn is_yanked( + &mut self, + pkg: PackageId, + load: &mut dyn RegistryData, + ) -> CargoResult> { + Ok(self + .summaries(pkg.name(), &VersionReq::exact(pkg.version()), load)? + .map(|mut p| p.any(|summary| summary.yanked))) } } diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 569e3280181..7f22a0ee92f 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -787,6 +787,15 @@ impl<'cfg> Source for RegistrySource<'cfg> { if !self.updated { self.do_update()?; } - self.index.is_yanked(pkg, &mut *self.ops) + loop { + match self.index.is_yanked(pkg, &mut *self.ops)? { + Poll::Ready(yanked) => { + return Ok(yanked); + } + Poll::Pending => { + // TODO: dont hot loop for it to be Ready + } + } + } } } From 33190fa13799792810c0b9c5c6ff2aec6099aa6c Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Mon, 21 Dec 2020 14:12:58 -0500 Subject: [PATCH 7/9] just assert when downloading --- src/cargo/sources/registry/mod.rs | 53 ++++++++++++++----------------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 7f22a0ee92f..5ee14cd3a5b 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -643,17 +643,16 @@ impl<'cfg> RegistrySource<'cfg> { // After we've loaded the package configure its summary's `checksum` // field with the checksum we know for this `PackageId`. let req = VersionReq::exact(package.version()); - let summary_with_cksum = loop { + let summary_with_cksum = match self.index.summaries(package.name(), &req, &mut *self.ops)? { - Poll::Ready(summaries) => { - break summaries; + Poll::Ready(summaries) => summaries, + Poll::Pending => { + unreachable!("a downloaded dep now pending!?") } - Poll::Pending => (), // TODO: dont hot loop for it to be Ready } - } - .map(|s| s.summary.clone()) - .next() - .expect("summary not found"); + .map(|s| s.summary.clone()) + .next() + .expect("summary not found"); if let Some(cksum) = summary_with_cksum.checksum() { pkg.manifest_mut() .summary_mut() @@ -738,35 +737,29 @@ impl<'cfg> Source for RegistrySource<'cfg> { } fn download(&mut self, package: PackageId) -> CargoResult { - loop { - match self.index.hash(package, &mut *self.ops)? { - Poll::Ready(hash) => { - return match self.ops.download(package, hash)? { - MaybeLock::Ready(file) => { - self.get_pkg(package, &file).map(MaybePackage::Ready) - } - MaybeLock::Download { url, descriptor } => { - Ok(MaybePackage::Download { url, descriptor }) - } + match self.index.hash(package, &mut *self.ops)? { + Poll::Ready(hash) => { + return match self.ops.download(package, hash)? { + MaybeLock::Ready(file) => self.get_pkg(package, &file).map(MaybePackage::Ready), + MaybeLock::Download { url, descriptor } => { + Ok(MaybePackage::Download { url, descriptor }) } } - Poll::Pending => { - // TODO: dont hot loop for it to be Ready - } + } + Poll::Pending => { + unreachable!("we got to downloading a dep while pending!?") } } } fn finish_download(&mut self, package: PackageId, data: Vec) -> CargoResult { - loop { - match self.index.hash(package, &mut *self.ops)? { - Poll::Ready(hash) => { - let file = self.ops.finish_download(package, hash, &data)?; - return self.get_pkg(package, &file); - } - Poll::Pending => { - // TODO: dont hot loop for it to be Ready - } + match self.index.hash(package, &mut *self.ops)? { + Poll::Ready(hash) => { + let file = self.ops.finish_download(package, hash, &data)?; + return self.get_pkg(package, &file); + } + Poll::Pending => { + unreachable!("we got to downloading a dep while pending!?") } } } From 1d9e6756b564bae33402887783806d54bcf5d272 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Wed, 30 Dec 2020 17:08:11 -0500 Subject: [PATCH 8/9] add a PollExt so we can use expect instead of match --- src/cargo/core/registry.rs | 6 ++-- src/cargo/core/resolver/mod.rs | 52 ++++++++++++++----------------- src/cargo/sources/registry/mod.rs | 46 +++++++++++---------------- src/cargo/util/network.rs | 15 +++++++++ 4 files changed, 60 insertions(+), 59 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 82104dc64bd..5534a2355b0 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -6,6 +6,7 @@ use crate::core::{Dependency, PackageId, Source, SourceId, SourceMap, Summary}; use crate::sources::config::SourceConfigMap; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::interning::InternedString; +use crate::util::network::PollExt; use crate::util::{profile, CanonicalUrl, Config}; use anyhow::bail; use log::{debug, trace}; @@ -334,10 +335,7 @@ impl<'cfg> PackageRegistry<'cfg> { if try_summaries.iter().all(|p| p.is_ready()) { break try_summaries .into_iter() - .map(|p| match p { - Poll::Ready(s) => s, - Poll::Pending => unreachable!("we just check for this!"), - }) + .map(|p| p.expect("we just checked for this!")) .collect::>(); } else { // TODO: dont hot loop for it to be Ready diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 4302bf45bff..5a5dfeaf204 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -58,6 +58,7 @@ use crate::core::PackageIdSpec; use crate::core::{Dependency, PackageId, Registry, Summary}; use crate::util::config::Config; use crate::util::errors::CargoResult; +use crate::util::network::PollExt; use crate::util::profile; use self::context::Context; @@ -73,8 +74,6 @@ pub use self::features::{ForceAllTargets, HasDevUnits}; pub use self::resolve::{Resolve, ResolveVersion}; pub use self::types::{ResolveBehavior, ResolveOpts}; -use std::task::Poll; - mod conflict_cache; mod context; mod dep_cache; @@ -861,34 +860,31 @@ fn generalize_conflicting( // A dep is equivalent to one of the things it can resolve to. // Thus, if all the things it can resolve to have already ben determined // to be conflicting, then we can just say that we conflict with the parent. - if let Some(others) = match registry + if let Some(others) = registry .query(critical_parents_dep) .expect("an already used dep now error!?") - { - Poll::Ready(q) => q, - Poll::Pending => unreachable!("an already used dep now pending!?"), - } - .iter() - .rev() // the last one to be tried is the least likely to be in the cache, so start with that. - .map(|other| { - past_conflicting_activations - .find( - dep, - &|id| { - if id == other.package_id() { - // we are imagining that we used other instead - Some(backtrack_critical_age) - } else { - cx.is_active(id) - } - }, - Some(other.package_id()), - // we only care about things that are newer then critical_age - backtrack_critical_age, - ) - .map(|con| (other.package_id(), con)) - }) - .collect::>>() + .expect("an already used dep now pending!?") + .iter() + .rev() // the last one to be tried is the least likely to be in the cache, so start with that. + .map(|other| { + past_conflicting_activations + .find( + dep, + &|id| { + if id == other.package_id() { + // we are imagining that we used other instead + Some(backtrack_critical_age) + } else { + cx.is_active(id) + } + }, + Some(other.package_id()), + // we only care about things that are newer then critical_age + backtrack_critical_age, + ) + .map(|con| (other.package_id(), con)) + }) + .collect::>>() { let mut con = conflicting_activations.clone(); // It is always valid to combine previously inserted conflicts. diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 5ee14cd3a5b..8dbc3582b71 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -180,6 +180,7 @@ use crate::util::errors::CargoResultExt; use crate::util::hex; use crate::util::interning::InternedString; use crate::util::into_url::IntoUrl; +use crate::util::network::PollExt; use crate::util::{restricted_names, CargoResult, Config, Filesystem}; const PACKAGE_SOURCE_LOCK: &str = ".cargo-ok"; @@ -643,13 +644,10 @@ impl<'cfg> RegistrySource<'cfg> { // After we've loaded the package configure its summary's `checksum` // field with the checksum we know for this `PackageId`. let req = VersionReq::exact(package.version()); - let summary_with_cksum = - match self.index.summaries(package.name(), &req, &mut *self.ops)? { - Poll::Ready(summaries) => summaries, - Poll::Pending => { - unreachable!("a downloaded dep now pending!?") - } - } + let summary_with_cksum = self + .index + .summaries(package.name(), &req, &mut *self.ops)? + .expect("a downloaded dep now pending!?") .map(|s| s.summary.clone()) .next() .expect("summary not found"); @@ -737,31 +735,25 @@ impl<'cfg> Source for RegistrySource<'cfg> { } fn download(&mut self, package: PackageId) -> CargoResult { - match self.index.hash(package, &mut *self.ops)? { - Poll::Ready(hash) => { - return match self.ops.download(package, hash)? { - MaybeLock::Ready(file) => self.get_pkg(package, &file).map(MaybePackage::Ready), - MaybeLock::Download { url, descriptor } => { - Ok(MaybePackage::Download { url, descriptor }) - } - } - } - Poll::Pending => { - unreachable!("we got to downloading a dep while pending!?") + let hash = self + .index + .hash(package, &mut *self.ops)? + .expect("we got to downloading a dep while pending!?"); + match self.ops.download(package, hash)? { + MaybeLock::Ready(file) => self.get_pkg(package, &file).map(MaybePackage::Ready), + MaybeLock::Download { url, descriptor } => { + Ok(MaybePackage::Download { url, descriptor }) } } } fn finish_download(&mut self, package: PackageId, data: Vec) -> CargoResult { - match self.index.hash(package, &mut *self.ops)? { - Poll::Ready(hash) => { - let file = self.ops.finish_download(package, hash, &data)?; - return self.get_pkg(package, &file); - } - Poll::Pending => { - unreachable!("we got to downloading a dep while pending!?") - } - } + let hash = self + .index + .hash(package, &mut *self.ops)? + .expect("we got to downloading a dep while pending!?"); + let file = self.ops.finish_download(package, hash, &data)?; + self.get_pkg(package, &file) } fn fingerprint(&self, pkg: &Package) -> CargoResult { diff --git a/src/cargo/util/network.rs b/src/cargo/util/network.rs index 48f8f0baef0..329e82ea567 100644 --- a/src/cargo/util/network.rs +++ b/src/cargo/util/network.rs @@ -2,6 +2,21 @@ use anyhow::Error; use crate::util::errors::{CargoResult, HttpNot200}; use crate::util::Config; +use std::task::Poll; + +pub trait PollExt { + fn expect(self, msg: &str) -> T; +} + +impl PollExt for Poll { + #[track_caller] + fn expect(self, msg: &str) -> T { + match self { + Poll::Ready(val) => val, + Poll::Pending => panic!("{}", msg), + } + } +} pub struct Retry<'a> { config: &'a Config, From 44012fd431099a6d1bbd0c1c83490f1939e2da50 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Wed, 30 Dec 2020 17:14:50 -0500 Subject: [PATCH 9/9] I missed one unneeded loop. --- src/cargo/core/registry.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 5534a2355b0..f321e5689c0 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -806,18 +806,12 @@ fn summary_for_patch( // No summaries found, try to help the user figure out what is wrong. if let Some((_locked_patch, locked_id)) = locked { // Since the locked patch did not match anything, try the unlocked one. - let orig_matches = loop { - match source.query_vec(orig_patch) { - Ok(Poll::Ready(deps)) => { - break Ok(deps); - } - Ok(Poll::Pending) => { - // TODO: dont hot loop for it to be Ready - } - Err(x) => { - break Err(x); - } + let orig_matches = match source.query_vec(orig_patch) { + Ok(Poll::Ready(deps)) => Ok(deps), + Ok(Poll::Pending) => { + return Ok(Poll::Pending); } + Err(x) => Err(x), } .unwrap_or_else(|e| { log::warn!(