From f947326a0c7c984792420f2d875f7f519cc7adfa Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 3 May 2018 23:14:21 +0100 Subject: [PATCH 1/8] Start moving methods from Registry to Source For now just move `supports_checksums` and `requires_precise` methods down from `Registry` to `Source`, as they're not as entangled as `query`. --- src/cargo/core/registry.rs | 32 ------------------------------- src/cargo/core/source/mod.rs | 18 +++++++++++++++++ src/cargo/sources/directory.rs | 12 ++++++------ src/cargo/sources/git/source.rs | 12 ++++++------ src/cargo/sources/path.rs | 12 ++++++------ src/cargo/sources/registry/mod.rs | 12 ++++++------ src/cargo/sources/replaced.rs | 12 ++++++------ tests/testsuite/resolve.rs | 6 ------ 8 files changed, 48 insertions(+), 68 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index ba1866bc897..b219105bff0 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -21,28 +21,12 @@ pub trait Registry { self.query(dep, &mut |s| ret.push(s))?; Ok(ret) } - - /// Returns whether or not this registry will return summaries with - /// checksums listed. - fn supports_checksums(&self) -> bool; - - /// Returns whether or not this registry will return summaries with - /// the `precise` field in the source id listed. - fn requires_precise(&self) -> bool; } impl<'a, T: ?Sized + Registry + 'a> Registry for Box { fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { (**self).query(dep, f) } - - fn supports_checksums(&self) -> bool { - (**self).supports_checksums() - } - - fn requires_precise(&self) -> bool { - (**self).requires_precise() - } } /// This structure represents a registry of known packages. It internally @@ -535,14 +519,6 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { f(self.lock(override_summary)); Ok(()) } - - fn supports_checksums(&self) -> bool { - false - } - - fn requires_precise(&self) -> bool { - false - } } fn lock(locked: &LockedMap, patches: &HashMap>, summary: Summary) -> Summary { @@ -703,13 +679,5 @@ pub mod test { Ok(()) } } - - fn supports_checksums(&self) -> bool { - false - } - - fn requires_precise(&self) -> bool { - false - } } } diff --git a/src/cargo/core/source/mod.rs b/src/cargo/core/source/mod.rs index b842a5792cf..10418714bff 100644 --- a/src/cargo/core/source/mod.rs +++ b/src/cargo/core/source/mod.rs @@ -14,6 +14,14 @@ pub trait Source: Registry { /// Returns the `SourceId` corresponding to this source fn source_id(&self) -> &SourceId; + /// Returns whether or not this registry will return summaries with + /// checksums listed. + fn supports_checksums(&self) -> bool; + + /// Returns whether or not this registry will return summaries with + /// the `precise` field in the source id listed. + fn requires_precise(&self) -> bool; + /// The update method performs any network operations required to /// get the entire list of all names, versions and dependencies of /// packages managed by the Source. @@ -52,6 +60,16 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box { (**self).source_id() } + /// Forwards to `Source::supports_checksums` + fn supports_checksums(&self) -> bool { + (**self).supports_checksums() + } + + /// Forwards to `Source::requires_precise` + fn requires_precise(&self) -> bool { + (**self).requires_precise() + } + /// Forwards to `Source::update` fn update(&mut self) -> CargoResult<()> { (**self).update() diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index a4d36ead4e2..21a167ac1b3 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -53,6 +53,12 @@ impl<'cfg> Registry for DirectorySource<'cfg> { } Ok(()) } +} + +impl<'cfg> Source for DirectorySource<'cfg> { + fn source_id(&self) -> &SourceId { + &self.source_id + } fn supports_checksums(&self) -> bool { true @@ -61,12 +67,6 @@ impl<'cfg> Registry for DirectorySource<'cfg> { fn requires_precise(&self) -> bool { true } -} - -impl<'cfg> Source for DirectorySource<'cfg> { - fn source_id(&self) -> &SourceId { - &self.source_id - } fn update(&mut self) -> CargoResult<()> { self.packages.clear(); diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index 98e42310171..332f37aa240 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -131,6 +131,12 @@ impl<'cfg> Registry for GitSource<'cfg> { .expect("BUG: update() must be called before query()"); src.query(dep, f) } +} + +impl<'cfg> Source for GitSource<'cfg> { + fn source_id(&self) -> &SourceId { + &self.source_id + } fn supports_checksums(&self) -> bool { false @@ -139,12 +145,6 @@ impl<'cfg> Registry for GitSource<'cfg> { fn requires_precise(&self) -> bool { true } -} - -impl<'cfg> Source for GitSource<'cfg> { - fn source_id(&self) -> &SourceId { - &self.source_id - } fn update(&mut self) -> CargoResult<()> { let lock = diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index aa654abee60..e24e4996812 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -484,6 +484,12 @@ impl<'cfg> Registry for PathSource<'cfg> { } Ok(()) } +} + +impl<'cfg> Source for PathSource<'cfg> { + fn source_id(&self) -> &SourceId { + &self.source_id + } fn supports_checksums(&self) -> bool { false @@ -492,12 +498,6 @@ impl<'cfg> Registry for PathSource<'cfg> { fn requires_precise(&self) -> bool { false } -} - -impl<'cfg> Source for PathSource<'cfg> { - fn source_id(&self) -> &SourceId { - &self.source_id - } fn update(&mut self) -> CargoResult<()> { if !self.updated { diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 96cce76d460..19c270ab018 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -441,6 +441,12 @@ impl<'cfg> Registry for RegistrySource<'cfg> { self.index.query(dep, &mut *self.ops, f) } +} + +impl<'cfg> Source for RegistrySource<'cfg> { + fn source_id(&self) -> &SourceId { + &self.source_id + } fn supports_checksums(&self) -> bool { true @@ -449,12 +455,6 @@ impl<'cfg> Registry for RegistrySource<'cfg> { fn requires_precise(&self) -> bool { false } -} - -impl<'cfg> Source for RegistrySource<'cfg> { - fn source_id(&self) -> &SourceId { - &self.source_id - } fn update(&mut self) -> CargoResult<()> { // If we have an imprecise version then we don't know what we're going diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs index 3f223ef6162..84bab7f5407 100644 --- a/src/cargo/sources/replaced.rs +++ b/src/cargo/sources/replaced.rs @@ -34,6 +34,12 @@ impl<'cfg> Registry for ReplacedSource<'cfg> { .chain_err(|| format!("failed to query replaced source {}", self.to_replace))?; Ok(()) } +} + +impl<'cfg> Source for ReplacedSource<'cfg> { + fn source_id(&self) -> &SourceId { + &self.to_replace + } fn supports_checksums(&self) -> bool { self.inner.supports_checksums() @@ -42,12 +48,6 @@ impl<'cfg> Registry for ReplacedSource<'cfg> { fn requires_precise(&self) -> bool { self.inner.requires_precise() } -} - -impl<'cfg> Source for ReplacedSource<'cfg> { - fn source_id(&self) -> &SourceId { - &self.to_replace - } fn update(&mut self) -> CargoResult<()> { self.inner diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs index e6377a5418a..4ba61b32191 100644 --- a/tests/testsuite/resolve.rs +++ b/tests/testsuite/resolve.rs @@ -36,12 +36,6 @@ fn resolve_with_config( } Ok(()) } - fn supports_checksums(&self) -> bool { - false - } - fn requires_precise(&self) -> bool { - false - } } let mut registry = MyRegistry(registry); let summary = Summary::new(pkg.clone(), deps, BTreeMap::new(), None, false).unwrap(); From 53bd095f44aec666ca6f5f5a4cf2a53f26b27a64 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 3 May 2018 23:26:42 +0100 Subject: [PATCH 2/8] Duplicate query in Source, drop Registry parent type .. and move query definitions to the struct impls. --- src/cargo/core/source/mod.rs | 22 +++++++++++++--- src/cargo/sources/directory.rs | 20 +++++++++----- src/cargo/sources/git/source.rs | 16 ++++++++--- src/cargo/sources/path.rs | 20 +++++++++----- src/cargo/sources/registry/mod.rs | 44 ++++++++++++++++++------------- src/cargo/sources/replaced.rs | 12 +++++++-- 6 files changed, 94 insertions(+), 40 deletions(-) diff --git a/src/cargo/core/source/mod.rs b/src/cargo/core/source/mod.rs index 10418714bff..1c6eed63315 100644 --- a/src/cargo/core/source/mod.rs +++ b/src/cargo/core/source/mod.rs @@ -1,7 +1,7 @@ use std::collections::hash_map::{HashMap, IterMut, Values}; use std::fmt; -use core::{Package, PackageId, Registry}; +use core::{Dependency, Package, PackageId, Summary}; use util::CargoResult; mod source_id; @@ -10,18 +10,27 @@ pub use self::source_id::{GitReference, SourceId}; /// A Source finds and downloads remote packages based on names and /// versions. -pub trait Source: Registry { +pub trait Source { /// Returns the `SourceId` corresponding to this source fn source_id(&self) -> &SourceId; - /// Returns whether or not this registry will return summaries with + /// Returns whether or not this source will return summaries with /// checksums listed. fn supports_checksums(&self) -> bool; - /// Returns whether or not this registry will return summaries with + /// Returns whether or not this source will return summaries with /// the `precise` field in the source id listed. fn requires_precise(&self) -> bool; + /// Attempt to find the packages that match a dependency request. + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()>; + + fn query_vec(&mut self, dep: &Dependency) -> CargoResult> { + let mut ret = Vec::new(); + self.query(dep, &mut |s| ret.push(s))?; + Ok(ret) + } + /// The update method performs any network operations required to /// get the entire list of all names, versions and dependencies of /// packages managed by the Source. @@ -70,6 +79,11 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box { (**self).requires_precise() } + /// Forwards to `Source::query` + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + (**self).query(dep, f) + } + /// Forwards to `Source::update` fn update(&mut self) -> CargoResult<()> { (**self).update() diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index 21a167ac1b3..2156164cf8c 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -36,6 +36,15 @@ impl<'cfg> DirectorySource<'cfg> { packages: HashMap::new(), } } + + fn query(&mut self, dep: &Dependency, f: &mut 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(()) + } } impl<'cfg> Debug for DirectorySource<'cfg> { @@ -46,12 +55,7 @@ impl<'cfg> Debug for DirectorySource<'cfg> { impl<'cfg> Registry for DirectorySource<'cfg> { fn query(&mut self, dep: &Dependency, f: &mut 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(()) + self.query(dep, f) } } @@ -68,6 +72,10 @@ impl<'cfg> Source for DirectorySource<'cfg> { true } + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + self.query(dep, f) + } + fn update(&mut self) -> CargoResult<()> { self.packages.clear(); let entries = self.root.read_dir().chain_err(|| { diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index 332f37aa240..34e7f2c7c21 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -52,6 +52,13 @@ impl<'cfg> GitSource<'cfg> { self.remote.url() } + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + let src = self.path_source + .as_mut() + .expect("BUG: update() must be called before query()"); + Source::query(src, dep, f) + } + pub fn read_packages(&mut self) -> CargoResult> { if self.path_source.is_none() { self.update()?; @@ -126,10 +133,7 @@ impl<'cfg> Debug for GitSource<'cfg> { impl<'cfg> Registry for GitSource<'cfg> { fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - let src = self.path_source - .as_mut() - .expect("BUG: update() must be called before query()"); - src.query(dep, f) + self.query(dep, f) } } @@ -146,6 +150,10 @@ impl<'cfg> Source for GitSource<'cfg> { true } + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + self.query(dep, f) + } + fn update(&mut self) -> CargoResult<()> { let lock = self.config diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index e24e4996812..c8b9ab09575 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -62,6 +62,15 @@ impl<'cfg> PathSource<'cfg> { self.packages.push(pkg); } + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + for s in self.packages.iter().map(|p| p.summary()) { + if dep.matches(s) { + f(s.clone()) + } + } + Ok(()) + } + pub fn root_package(&mut self) -> CargoResult { trace!("root_package; source={:?}", self); @@ -477,12 +486,7 @@ impl<'cfg> Debug for PathSource<'cfg> { impl<'cfg> Registry for PathSource<'cfg> { fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - for s in self.packages.iter().map(|p| p.summary()) { - if dep.matches(s) { - f(s.clone()) - } - } - Ok(()) + self.query(dep, f) } } @@ -499,6 +503,10 @@ impl<'cfg> Source for PathSource<'cfg> { false } + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + self.query(dep, f) + } + fn update(&mut self) -> CargoResult<()> { if !self.updated { let packages = self.read_packages()?; diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 19c270ab018..4f6ae6bc580 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -352,6 +352,27 @@ impl<'cfg> RegistrySource<'cfg> { } } + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + // If this is a precise dependency, then it came from a lockfile 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 + // updated, so we fall back to performing a lazy update. + if dep.source_id().precise().is_some() && !self.updated { + let mut called = false; + self.index.query(dep, &mut *self.ops, &mut |s| { + called = true; + f(s); + })?; + if called { + return Ok(()); + } else { + self.do_update()?; + } + } + + self.index.query(dep, &mut *self.ops, f) + } + /// Decode the configuration stored within the registry. /// /// This requires that the index has been at least checked out. @@ -422,24 +443,7 @@ impl<'cfg> RegistrySource<'cfg> { impl<'cfg> Registry for RegistrySource<'cfg> { fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - // If this is a precise dependency, then it came from a lockfile 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 - // updated, so we fall back to performing a lazy update. - if dep.source_id().precise().is_some() && !self.updated { - let mut called = false; - self.index.query(dep, &mut *self.ops, &mut |s| { - called = true; - f(s); - })?; - if called { - return Ok(()); - } else { - self.do_update()?; - } - } - - self.index.query(dep, &mut *self.ops, f) + self.query(dep, f) } } @@ -456,6 +460,10 @@ impl<'cfg> Source for RegistrySource<'cfg> { false } + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + self.query(dep, f) + } + fn update(&mut self) -> CargoResult<()> { // If we have an imprecise version then we don't know what we're going // to look for, so we always attempt to perform an update here. diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs index 84bab7f5407..53fa3eab78b 100644 --- a/src/cargo/sources/replaced.rs +++ b/src/cargo/sources/replaced.rs @@ -19,9 +19,7 @@ impl<'cfg> ReplacedSource<'cfg> { inner: src, } } -} -impl<'cfg> Registry for ReplacedSource<'cfg> { fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { let (replace_with, to_replace) = (&self.replace_with, &self.to_replace); let dep = dep.clone().map_source(to_replace, replace_with); @@ -36,6 +34,12 @@ impl<'cfg> Registry for ReplacedSource<'cfg> { } } +impl<'cfg> Registry for ReplacedSource<'cfg> { + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + self.query(dep, f) + } +} + impl<'cfg> Source for ReplacedSource<'cfg> { fn source_id(&self) -> &SourceId { &self.to_replace @@ -49,6 +53,10 @@ impl<'cfg> Source for ReplacedSource<'cfg> { self.inner.requires_precise() } + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + self.query(dep, f) + } + fn update(&mut self) -> CargoResult<()> { self.inner .update() From 5f1c7a3073f6cca5107538816d0e7e66ba563c9d Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 4 May 2018 14:00:35 +0100 Subject: [PATCH 3/8] Drop unnecessary Registry for Box --- src/cargo/core/registry.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index b219105bff0..95b49053ff8 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -23,12 +23,6 @@ pub trait Registry { } } -impl<'a, T: ?Sized + Registry + 'a> Registry for Box { - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - (**self).query(dep, f) - } -} - /// This structure represents a registry of known packages. It internally /// contains a number of `Box` instances which are used to load a /// `Package` from. From 29f738fef8034c8131ca60072deb88f908f066a2 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 4 May 2018 14:05:23 +0100 Subject: [PATCH 4/8] Remove all "impl Registry for XSource" blocks! --- src/cargo/sources/directory.rs | 8 +------- src/cargo/sources/git/source.rs | 8 +------- src/cargo/sources/path.rs | 8 +------- src/cargo/sources/registry/mod.rs | 8 +------- src/cargo/sources/replaced.rs | 8 +------- 5 files changed, 5 insertions(+), 35 deletions(-) diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index 2156164cf8c..4b55fe49f2d 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -8,7 +8,7 @@ use hex; use serde_json; -use core::{Dependency, Package, PackageId, Registry, Source, SourceId, Summary}; +use core::{Dependency, Package, PackageId, Source, SourceId, Summary}; use sources::PathSource; use util::{Config, Sha256}; use util::errors::{CargoResult, CargoResultExt}; @@ -53,12 +53,6 @@ impl<'cfg> Debug for DirectorySource<'cfg> { } } -impl<'cfg> Registry for DirectorySource<'cfg> { - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - self.query(dep, f) - } -} - impl<'cfg> Source for DirectorySource<'cfg> { fn source_id(&self) -> &SourceId { &self.source_id diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index 34e7f2c7c21..3256b2d1604 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -4,7 +4,7 @@ use url::Url; use core::source::{Source, SourceId}; use core::GitReference; -use core::{Dependency, Package, PackageId, Registry, Summary}; +use core::{Dependency, Package, PackageId, Summary}; use util::Config; use util::errors::CargoResult; use util::hex::short_hash; @@ -131,12 +131,6 @@ impl<'cfg> Debug for GitSource<'cfg> { } } -impl<'cfg> Registry for GitSource<'cfg> { - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - self.query(dep, f) - } -} - impl<'cfg> Source for GitSource<'cfg> { fn source_id(&self) -> &SourceId { &self.source_id diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index c8b9ab09575..3303cdb7ad8 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -8,7 +8,7 @@ use glob::Pattern; use ignore::Match; use ignore::gitignore::GitignoreBuilder; -use core::{Dependency, Package, PackageId, Registry, Source, SourceId, Summary}; +use core::{Dependency, Package, PackageId, Source, SourceId, Summary}; use ops; use util::{self, internal, CargoResult}; use util::paths; @@ -484,12 +484,6 @@ impl<'cfg> Debug for PathSource<'cfg> { } } -impl<'cfg> Registry for PathSource<'cfg> { - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - self.query(dep, f) - } -} - impl<'cfg> Source for PathSource<'cfg> { fn source_id(&self) -> &SourceId { &self.source_id diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 4f6ae6bc580..1d8c0b42d02 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -167,7 +167,7 @@ use flate2::read::GzDecoder; use semver::Version; use tar::Archive; -use core::{Package, PackageId, Registry, Source, SourceId, Summary}; +use core::{Package, PackageId, Source, SourceId, Summary}; use core::dependency::{Dependency, Kind}; use sources::PathSource; use util::{internal, CargoResult, Config, FileLock, Filesystem}; @@ -441,12 +441,6 @@ impl<'cfg> RegistrySource<'cfg> { } } -impl<'cfg> Registry for RegistrySource<'cfg> { - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - self.query(dep, f) - } -} - impl<'cfg> Source for RegistrySource<'cfg> { fn source_id(&self) -> &SourceId { &self.source_id diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs index 53fa3eab78b..a9cce6a17d5 100644 --- a/src/cargo/sources/replaced.rs +++ b/src/cargo/sources/replaced.rs @@ -1,4 +1,4 @@ -use core::{Dependency, Package, PackageId, Registry, Source, SourceId, Summary}; +use core::{Dependency, Package, PackageId, Source, SourceId, Summary}; use util::errors::{CargoResult, CargoResultExt}; pub struct ReplacedSource<'cfg> { @@ -34,12 +34,6 @@ impl<'cfg> ReplacedSource<'cfg> { } } -impl<'cfg> Registry for ReplacedSource<'cfg> { - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - self.query(dep, f) - } -} - impl<'cfg> Source for ReplacedSource<'cfg> { fn source_id(&self) -> &SourceId { &self.to_replace From c1010af002a0851025d14bb742eaabd3b3539e61 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 4 May 2018 14:07:51 +0100 Subject: [PATCH 5/8] Move query definitions back into Source impls! --- src/cargo/sources/directory.rs | 16 +++++-------- src/cargo/sources/git/source.rs | 12 ++++------ src/cargo/sources/path.rs | 16 +++++-------- src/cargo/sources/registry/mod.rs | 40 ++++++++++++++----------------- src/cargo/sources/replaced.rs | 24 ++++++++----------- 5 files changed, 44 insertions(+), 64 deletions(-) diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index 4b55fe49f2d..647c8b475de 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -36,15 +36,6 @@ impl<'cfg> DirectorySource<'cfg> { packages: HashMap::new(), } } - - fn query(&mut self, dep: &Dependency, f: &mut 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(()) - } } impl<'cfg> Debug for DirectorySource<'cfg> { @@ -67,7 +58,12 @@ impl<'cfg> Source for DirectorySource<'cfg> { } fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - self.query(dep, f) + 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(()) } fn update(&mut self) -> CargoResult<()> { diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index 3256b2d1604..775222ce4f3 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -52,13 +52,6 @@ impl<'cfg> GitSource<'cfg> { self.remote.url() } - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - let src = self.path_source - .as_mut() - .expect("BUG: update() must be called before query()"); - Source::query(src, dep, f) - } - pub fn read_packages(&mut self) -> CargoResult> { if self.path_source.is_none() { self.update()?; @@ -145,7 +138,10 @@ impl<'cfg> Source for GitSource<'cfg> { } fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - self.query(dep, f) + let src = self.path_source + .as_mut() + .expect("BUG: update() must be called before query()"); + Source::query(src, dep, f) } fn update(&mut self) -> CargoResult<()> { diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 3303cdb7ad8..e30b8103e3b 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -62,15 +62,6 @@ impl<'cfg> PathSource<'cfg> { self.packages.push(pkg); } - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - for s in self.packages.iter().map(|p| p.summary()) { - if dep.matches(s) { - f(s.clone()) - } - } - Ok(()) - } - pub fn root_package(&mut self) -> CargoResult { trace!("root_package; source={:?}", self); @@ -498,7 +489,12 @@ impl<'cfg> Source for PathSource<'cfg> { } fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - self.query(dep, f) + for s in self.packages.iter().map(|p| p.summary()) { + if dep.matches(s) { + f(s.clone()) + } + } + Ok(()) } fn update(&mut self) -> CargoResult<()> { diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 1d8c0b42d02..d9571c23dee 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -352,27 +352,6 @@ impl<'cfg> RegistrySource<'cfg> { } } - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - // If this is a precise dependency, then it came from a lockfile 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 - // updated, so we fall back to performing a lazy update. - if dep.source_id().precise().is_some() && !self.updated { - let mut called = false; - self.index.query(dep, &mut *self.ops, &mut |s| { - called = true; - f(s); - })?; - if called { - return Ok(()); - } else { - self.do_update()?; - } - } - - self.index.query(dep, &mut *self.ops, f) - } - /// Decode the configuration stored within the registry. /// /// This requires that the index has been at least checked out. @@ -455,7 +434,24 @@ impl<'cfg> Source for RegistrySource<'cfg> { } fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - self.query(dep, f) + // If this is a precise dependency, then it came from a lockfile 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 + // updated, so we fall back to performing a lazy update. + if dep.source_id().precise().is_some() && !self.updated { + let mut called = false; + self.index.query(dep, &mut *self.ops, &mut |s| { + called = true; + f(s); + })?; + if called { + return Ok(()); + } else { + self.do_update()?; + } + } + + self.index.query(dep, &mut *self.ops, f) } fn update(&mut self) -> CargoResult<()> { diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs index a9cce6a17d5..9d2ea2c41e0 100644 --- a/src/cargo/sources/replaced.rs +++ b/src/cargo/sources/replaced.rs @@ -19,19 +19,6 @@ impl<'cfg> ReplacedSource<'cfg> { inner: src, } } - - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - let (replace_with, to_replace) = (&self.replace_with, &self.to_replace); - let dep = dep.clone().map_source(to_replace, replace_with); - - self.inner - .query( - &dep, - &mut |summary| f(summary.map_source(replace_with, to_replace)), - ) - .chain_err(|| format!("failed to query replaced source {}", self.to_replace))?; - Ok(()) - } } impl<'cfg> Source for ReplacedSource<'cfg> { @@ -48,7 +35,16 @@ impl<'cfg> Source for ReplacedSource<'cfg> { } fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - self.query(dep, f) + let (replace_with, to_replace) = (&self.replace_with, &self.to_replace); + let dep = dep.clone().map_source(to_replace, replace_with); + + self.inner + .query( + &dep, + &mut |summary| f(summary.map_source(replace_with, to_replace)), + ) + .chain_err(|| format!("failed to query replaced source {}", self.to_replace))?; + Ok(()) } fn update(&mut self) -> CargoResult<()> { From 23312ce57cdf270f1b867af29e328126188973c2 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 4 May 2018 14:10:24 +0100 Subject: [PATCH 6/8] Re-simplify GitSource::query --- src/cargo/sources/git/source.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index 775222ce4f3..dfdf58996fc 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -141,7 +141,7 @@ impl<'cfg> Source for GitSource<'cfg> { let src = self.path_source .as_mut() .expect("BUG: update() must be called before query()"); - Source::query(src, dep, f) + src.query(dep, f) } fn update(&mut self) -> CargoResult<()> { From 6acedd8a728cc10eef2ae01f96ed6009962dba66 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 4 May 2018 14:15:48 +0100 Subject: [PATCH 7/8] Re-organise Source impls method definition order Optimised for minimised PR diff, and thus reviewability --- src/cargo/core/source/mod.rs | 10 +++++----- src/cargo/sources/directory.rs | 18 +++++++++--------- src/cargo/sources/git/source.rs | 14 +++++++------- src/cargo/sources/path.rs | 18 +++++++++--------- src/cargo/sources/registry/mod.rs | 24 ++++++++++++------------ src/cargo/sources/replaced.rs | 24 ++++++++++++------------ 6 files changed, 54 insertions(+), 54 deletions(-) diff --git a/src/cargo/core/source/mod.rs b/src/cargo/core/source/mod.rs index 1c6eed63315..c36480aab27 100644 --- a/src/cargo/core/source/mod.rs +++ b/src/cargo/core/source/mod.rs @@ -64,11 +64,6 @@ pub trait Source { } impl<'a, T: Source + ?Sized + 'a> Source for Box { - /// Forwards to `Source::source_id` - fn source_id(&self) -> &SourceId { - (**self).source_id() - } - /// Forwards to `Source::supports_checksums` fn supports_checksums(&self) -> bool { (**self).supports_checksums() @@ -84,6 +79,11 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box { (**self).query(dep, f) } + /// Forwards to `Source::source_id` + fn source_id(&self) -> &SourceId { + (**self).source_id() + } + /// Forwards to `Source::update` fn update(&mut self) -> CargoResult<()> { (**self).update() diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index 647c8b475de..bf20a270d2e 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -45,8 +45,13 @@ impl<'cfg> Debug for DirectorySource<'cfg> { } impl<'cfg> Source for DirectorySource<'cfg> { - fn source_id(&self) -> &SourceId { - &self.source_id + fn query(&mut self, dep: &Dependency, f: &mut 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(()) } fn supports_checksums(&self) -> bool { @@ -57,13 +62,8 @@ impl<'cfg> Source for DirectorySource<'cfg> { true } - fn query(&mut self, dep: &Dependency, f: &mut 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(()) + fn source_id(&self) -> &SourceId { + &self.source_id } fn update(&mut self) -> CargoResult<()> { diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index dfdf58996fc..f940eb4a78c 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -125,8 +125,11 @@ impl<'cfg> Debug for GitSource<'cfg> { } impl<'cfg> Source for GitSource<'cfg> { - fn source_id(&self) -> &SourceId { - &self.source_id + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + let src = self.path_source + .as_mut() + .expect("BUG: update() must be called before query()"); + src.query(dep, f) } fn supports_checksums(&self) -> bool { @@ -137,11 +140,8 @@ impl<'cfg> Source for GitSource<'cfg> { true } - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - let src = self.path_source - .as_mut() - .expect("BUG: update() must be called before query()"); - src.query(dep, f) + fn source_id(&self) -> &SourceId { + &self.source_id } fn update(&mut self) -> CargoResult<()> { diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index e30b8103e3b..c7a0fdf75c4 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -476,8 +476,13 @@ impl<'cfg> Debug for PathSource<'cfg> { } impl<'cfg> Source for PathSource<'cfg> { - fn source_id(&self) -> &SourceId { - &self.source_id + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + for s in self.packages.iter().map(|p| p.summary()) { + if dep.matches(s) { + f(s.clone()) + } + } + Ok(()) } fn supports_checksums(&self) -> bool { @@ -488,13 +493,8 @@ impl<'cfg> Source for PathSource<'cfg> { false } - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - for s in self.packages.iter().map(|p| p.summary()) { - if dep.matches(s) { - f(s.clone()) - } - } - Ok(()) + fn source_id(&self) -> &SourceId { + &self.source_id } fn update(&mut self) -> CargoResult<()> { diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index d9571c23dee..2e6f63228b9 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -421,18 +421,6 @@ impl<'cfg> RegistrySource<'cfg> { } impl<'cfg> Source for RegistrySource<'cfg> { - fn source_id(&self) -> &SourceId { - &self.source_id - } - - fn supports_checksums(&self) -> bool { - true - } - - fn requires_precise(&self) -> bool { - false - } - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { // If this is a precise dependency, then it came from a lockfile and in // theory the registry is known to contain this version. If, however, we @@ -454,6 +442,18 @@ impl<'cfg> Source for RegistrySource<'cfg> { self.index.query(dep, &mut *self.ops, f) } + fn supports_checksums(&self) -> bool { + true + } + + fn requires_precise(&self) -> bool { + false + } + + fn source_id(&self) -> &SourceId { + &self.source_id + } + fn update(&mut self) -> CargoResult<()> { // If we have an imprecise version then we don't know what we're going // to look for, so we always attempt to perform an update here. diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs index 9d2ea2c41e0..16d867c17bd 100644 --- a/src/cargo/sources/replaced.rs +++ b/src/cargo/sources/replaced.rs @@ -22,18 +22,6 @@ impl<'cfg> ReplacedSource<'cfg> { } impl<'cfg> Source for ReplacedSource<'cfg> { - fn source_id(&self) -> &SourceId { - &self.to_replace - } - - fn supports_checksums(&self) -> bool { - self.inner.supports_checksums() - } - - fn requires_precise(&self) -> bool { - self.inner.requires_precise() - } - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { let (replace_with, to_replace) = (&self.replace_with, &self.to_replace); let dep = dep.clone().map_source(to_replace, replace_with); @@ -47,6 +35,18 @@ impl<'cfg> Source for ReplacedSource<'cfg> { Ok(()) } + fn supports_checksums(&self) -> bool { + self.inner.supports_checksums() + } + + fn requires_precise(&self) -> bool { + self.inner.requires_precise() + } + + fn source_id(&self) -> &SourceId { + &self.to_replace + } + fn update(&mut self) -> CargoResult<()> { self.inner .update() From 05e819427c64cbbda5bc8f8d752980afaf0abae8 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 4 May 2018 15:24:08 +0100 Subject: [PATCH 8/8] Trash RegistryBuilder, dead cfg(test) code --- src/cargo/core/registry.rs | 70 -------------------------------------- 1 file changed, 70 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 95b49053ff8..904179c7a20 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -605,73 +605,3 @@ fn lock(locked: &LockedMap, patches: &HashMap>, summary: Sum dep }) } - -#[cfg(test)] -pub mod test { - use core::{Dependency, Registry, Summary}; - use util::CargoResult; - - pub struct RegistryBuilder { - summaries: Vec, - overrides: Vec, - } - - impl RegistryBuilder { - pub fn new() -> RegistryBuilder { - RegistryBuilder { - summaries: vec![], - overrides: vec![], - } - } - - pub fn summary(mut self, summary: Summary) -> RegistryBuilder { - self.summaries.push(summary); - self - } - - pub fn summaries(mut self, summaries: Vec) -> RegistryBuilder { - self.summaries.extend(summaries.into_iter()); - self - } - - pub fn add_override(mut self, summary: Summary) -> RegistryBuilder { - self.overrides.push(summary); - self - } - - pub fn overrides(mut self, summaries: Vec) -> RegistryBuilder { - self.overrides.extend(summaries.into_iter()); - self - } - - fn query_overrides(&self, dep: &Dependency) -> Vec { - self.overrides - .iter() - .filter(|s| s.name() == dep.name()) - .map(|s| s.clone()) - .collect() - } - } - - impl Registry for RegistryBuilder { - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { - debug!("querying; dep={:?}", dep); - - let overrides = self.query_overrides(dep); - - if overrides.is_empty() { - for s in self.summaries.iter() { - if dep.matches(s) { - f(s.clone()); - } - } - Ok(()) - } else { - for s in overrides { - f(s); - } - Ok(()) - } - } - } -}