Auto merge of #5476 - dwijnand:Registry-vs-Source-confusion, r=matklad

Try & simplify Registry vs Source confusion

Refs #3006.

I tried to go further and replace references of `Registry` with `PackageRegistry`, but I ran into lifetime issue in `ops::resolve_with_previous` which I think has to do with no longer passing `PackageRegistry` as a `Registry` trait object (with a new and distinct lifetime).

Author: bors

src/cargo/core/registry.rs

@@ -21,28 +21,6 @@ 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<T> {
-    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 +513,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<Url, Vec<PackageId>>, summary: Summary) -> Summary {
@@ -635,81 +605,3 @@ fn lock(locked: &LockedMap, patches: &HashMap<Url, Vec<PackageId>>, summary: Sum
         dep
     })
 }
-
-#[cfg(test)]
-pub mod test {
-    use core::{Dependency, Registry, Summary};
-    use util::CargoResult;
-
-    pub struct RegistryBuilder {
-        summaries: Vec<Summary>,
-        overrides: Vec<Summary>,
-    }
-
-    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<Summary>) -> 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<Summary>) -> RegistryBuilder {
-            self.overrides.extend(summaries.into_iter());
-            self
-        }
-
-        fn query_overrides(&self, dep: &Dependency) -> Vec<Summary> {
-            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(())
-            }
-        }
-
-        fn supports_checksums(&self) -> bool {
-            false
-        }
-
-        fn requires_precise(&self) -> bool {
-            false
-        }
-    }
-}

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,10 +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 source will return summaries with
+    /// checksums listed.
+    fn supports_checksums(&self) -> bool;
+
+    /// 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<Vec<Summary>> {
+        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.
@@ -47,6 +64,21 @@ pub trait Source: Registry {
 }
 
 impl<'a, T: Source + ?Sized + 'a> Source for Box<T> {
+    /// 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::query`
+    fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> {
+        (**self).query(dep, f)
+    }
+
     /// Forwards to `Source::source_id`
     fn source_id(&self) -> &SourceId {
         (**self).source_id()

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};
@@ -44,7 +44,7 @@ impl<'cfg> Debug for DirectorySource<'cfg> {
     }
 }
 
-impl<'cfg> Registry for DirectorySource<'cfg> {
+impl<'cfg> Source 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()));
@@ -61,9 +61,7 @@ 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
     }

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;
@@ -124,7 +124,7 @@ impl<'cfg> Debug for GitSource<'cfg> {
     }
 }
 
-impl<'cfg> Registry for GitSource<'cfg> {
+impl<'cfg> Source for GitSource<'cfg> {
     fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> {
         let src = self.path_source
             .as_mut()
@@ -139,9 +139,7 @@ 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
     }

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;
@@ -475,7 +475,7 @@ impl<'cfg> Debug for PathSource<'cfg> {
     }
 }
 
-impl<'cfg> Registry for PathSource<'cfg> {
+impl<'cfg> Source 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) {
@@ -492,9 +492,7 @@ 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
     }

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};
@@ -420,7 +420,7 @@ impl<'cfg> RegistrySource<'cfg> {
     }
 }
 
-impl<'cfg> Registry for RegistrySource<'cfg> {
+impl<'cfg> Source 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
@@ -449,9 +449,7 @@ 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
     }

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> {
@@ -21,7 +21,7 @@ impl<'cfg> ReplacedSource<'cfg> {
     }
 }
 
-impl<'cfg> Registry for ReplacedSource<'cfg> {
+impl<'cfg> Source 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);
@@ -42,9 +42,7 @@ 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
     }

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();