Skip to content

Try & simplify Registry vs Source confusion #5476

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 0 additions & 108 deletions src/cargo/core/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
}
}
36 changes: 34 additions & 2 deletions src/cargo/core/source/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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()
Expand Down
6 changes: 2 additions & 4 deletions src/cargo/sources/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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()));
Expand All @@ -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
}
Expand Down
6 changes: 2 additions & 4 deletions src/cargo/sources/git/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand All @@ -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
}
Expand Down
6 changes: 2 additions & 4 deletions src/cargo/sources/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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
}
Expand Down
6 changes: 2 additions & 4 deletions src/cargo/sources/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
6 changes: 2 additions & 4 deletions src/cargo/sources/replaced.rs
Original file line number Diff line number Diff line change
@@ -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> {
Expand All @@ -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);
Expand All @@ -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
}
Expand Down
6 changes: 0 additions & 6 deletions tests/testsuite/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down