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..f321e5689c0 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -1,10 +1,12 @@ use std::collections::{HashMap, HashSet}; +use std::task::Poll; use crate::core::PackageSet; 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}; @@ -21,12 +23,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 +261,86 @@ 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() - ); - - // 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 = 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(), + 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, - ) - })?; - 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() + ); + + // 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 + ); + 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| p.expect("we just checked 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 +416,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 +511,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 +545,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 +573,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 +621,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 +642,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 { @@ -748,9 +775,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 @@ -779,7 +806,14 @@ 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 = 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!( "could not determine unlocked summaries for dep {:?}: {:?}", orig_patch, @@ -787,14 +821,28 @@ 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 = source.query_vec(&name_only_dep).unwrap_or_else(|e| { + + 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!( "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..c8b78c85ac5 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,11 +33,11 @@ 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), - Rc<(HashSet, Rc>)>, + (Rc<(HashSet, Rc>)>, bool), >, /// all the cases we ended up using a supplied replacement used_replacements: HashMap, @@ -67,6 +68,23 @@ impl<'a> RegistryQueryer<'a> { } } + 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)> { self.used_replacements.get(&p).map(|r| (p, r.package_id())) } @@ -119,20 +137,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 +171,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 +259,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()); @@ -254,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 @@ -265,17 +292,22 @@ 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() - .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) => { + 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 {}", dep.package_name(), describe_path(&cx.parents.path_to_bottom(&candidate.package_id())), ) - })?; - Ok((dep, candidates, features)) + })), }) .collect::>>()?; @@ -289,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/errors.rs b/src/cargo/core/resolver/errors.rs index 8e9968db4e8..a4356a0e21a 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -1,9 +1,8 @@ -use std::fmt; - 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}; @@ -214,9 +213,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 = 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())); @@ -270,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()); diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 0531f3dba89..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; @@ -127,7 +128,6 @@ 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, @@ -135,7 +135,15 @@ pub fn resolve( }; let mut registry = RegistryQueryer::new(registry, replacements, try_to_use, minimal_versions, config); - let cx = activate_deps_loop(cx, &mut registry, summaries, config)?; + let cx = loop { + let cx = Context::new(check_public_visible_dependencies); + let cx = activate_deps_loop(cx, &mut registry, summaries, config)?; + if registry.reset_pending() { + break cx; + } else { + // TODO: dont hot loop for it to be Ready + } + }; let mut cksums = HashMap::new(); for (summary, _) in cx.activations.values() { @@ -855,6 +863,7 @@ fn generalize_conflicting( if let Some(others) = registry .query(critical_parents_dep) .expect("an already used dep now error!?") + .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| { 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..e47e2a92a7c 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. @@ -259,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 @@ -283,7 +291,7 @@ impl<'cfg> RegistryIndex<'cfg> { name: InternedString, req: &'b VersionReq, load: &mut dyn RegistryData, - ) -> CargoResult + 'b> + ) -> CargoResult + 'b>> where 'a: 'b, { @@ -296,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 @@ -305,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 @@ -351,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, @@ -373,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( @@ -391,11 +426,12 @@ 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 + && self.query_inner_with_online(dep, load, yanked_whitelist, f, false)? + != Poll::Ready(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 @@ -405,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(()) + Ok(self + .query_inner_with_online(dep, load, yanked_whitelist, f, true)? + .map(|_| ())) } fn query_inner_with_online( @@ -416,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. @@ -463,15 +507,17 @@ 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) + 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))) } } @@ -505,7 +551,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. @@ -519,7 +565,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) => { @@ -564,14 +610,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 @@ -597,7 +648,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 495df40bfce..8dbc3582b71 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; @@ -179,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"; @@ -406,12 +408,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. /// @@ -643,6 +647,7 @@ impl<'cfg> RegistrySource<'cfg> { 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"); @@ -657,7 +662,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 +670,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 +697,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) } @@ -722,7 +735,10 @@ impl<'cfg> Source for RegistrySource<'cfg> { } fn download(&mut self, package: PackageId) -> CargoResult { - let hash = self.index.hash(package, &mut *self.ops)?; + 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 } => { @@ -732,7 +748,10 @@ impl<'cfg> Source for RegistrySource<'cfg> { } fn finish_download(&mut self, package: PackageId, data: Vec) -> CargoResult { - let hash = self.index.hash(package, &mut *self.ops)?; + 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) } @@ -753,6 +772,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 + } + } + } } } 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) } 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<()> { 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,