From f8dcf54ca8ad04184246fb12c45b04b3654d64b7 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 23 Oct 2014 12:01:48 -0700 Subject: [PATCH] Make SourceKind private This is a bit of an implementation detail of the `SourceId` and we should be able to get away without exposing it. --- src/bin/git_checkout.rs | 2 +- src/cargo/core/mod.rs | 3 +-- src/cargo/core/package_id.rs | 4 ++-- src/cargo/core/source.rs | 22 +++++++++++++++------- src/cargo/ops/cargo_rustc/fingerprint.rs | 7 ++----- src/cargo/ops/registry.rs | 2 +- src/cargo/sources/git/source.rs | 8 ++++---- src/cargo/util/toml.rs | 5 ++--- tests/resolve.rs | 13 ++++++------- 9 files changed, 34 insertions(+), 32 deletions(-) diff --git a/src/bin/git_checkout.rs b/src/bin/git_checkout.rs index a903f28d7..a42cbbd03 100644 --- a/src/bin/git_checkout.rs +++ b/src/bin/git_checkout.rs @@ -30,7 +30,7 @@ pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult }) .map_err(|e| CliError::from_boxed(e, 1))); - let source_id = SourceId::for_git(&url, reference.as_slice(), None); + let source_id = SourceId::for_git(&url, reference.as_slice()); let mut config = try!(Config::new(shell, None, None).map_err(|e| { CliError::from_boxed(e, 1) diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 50d86caa4..f7ffee337 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -6,8 +6,7 @@ pub use self::package_id_spec::PackageIdSpec; pub use self::registry::Registry; pub use self::resolver::Resolve; pub use self::shell::{Shell, MultiShell, ShellConfig}; -pub use self::source::{PathKind, RegistryKind}; -pub use self::source::{Source, SourceId, SourceMap, SourceSet, GitKind}; +pub use self::source::{Source, SourceId, SourceMap, SourceSet}; pub use self::summary::Summary; pub mod source; diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index 1442a7815..f9ee93b40 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -159,13 +159,13 @@ impl Show for PackageId { #[cfg(test)] mod tests { use super::{PackageId, CENTRAL_REPO}; - use core::source::{RegistryKind, SourceId}; + use core::source::SourceId; use util::ToUrl; #[test] fn invalid_version_handled_nicely() { let loc = CENTRAL_REPO.to_url().unwrap(); - let repo = SourceId::new(RegistryKind, loc); + let repo = SourceId::for_registry(&loc); assert!(PackageId::new("foo", "1.0", &repo).is_err()); assert!(PackageId::new("foo", "1", &repo).is_err()); diff --git a/src/cargo/core/source.rs b/src/cargo/core/source.rs index 743c48f7f..08a105b0e 100644 --- a/src/cargo/core/source.rs +++ b/src/cargo/core/source.rs @@ -46,7 +46,7 @@ pub trait Source: Registry { } #[deriving(Encodable, Decodable, Show, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub enum SourceKind { +enum SourceKind { /// GitKind() represents a git repository GitKind(String), /// represents a local path @@ -72,7 +72,7 @@ struct SourceIdInner { } impl SourceId { - pub fn new(kind: SourceKind, url: Url) -> SourceId { + fn new(kind: SourceKind, url: Url) -> SourceId { SourceId { inner: Arc::new(SourceIdInner { kind: kind, @@ -109,7 +109,8 @@ impl SourceId { } url.query = None; let precise = mem::replace(&mut url.fragment, None); - SourceId::for_git(&url, reference.as_slice(), precise) + SourceId::for_git(&url, reference.as_slice()) + .with_precise(precise) }, "registry" => { let url = url.to_url().unwrap(); @@ -155,9 +156,8 @@ impl SourceId { Ok(SourceId::new(PathKind, url)) } - pub fn for_git(url: &Url, reference: &str, precise: Option) -> SourceId { + pub fn for_git(url: &Url, reference: &str) -> SourceId { SourceId::new(GitKind(reference.to_string()), url.clone()) - .with_precise(precise) } pub fn for_registry(url: &Url) -> SourceId { @@ -173,7 +173,6 @@ impl SourceId { } pub fn get_url(&self) -> &Url { &self.inner.url } - pub fn get_kind(&self) -> &SourceKind { &self.inner.kind } pub fn is_path(&self) -> bool { self.inner.kind == PathKind } pub fn is_registry(&self) -> bool { self.inner.kind == RegistryKind } @@ -196,7 +195,9 @@ impl SourceId { }; box PathSource::new(&path, self) as Box }, - RegistryKind => box RegistrySource::new(self, config) as Box, + RegistryKind => { + box RegistrySource::new(self, config) as Box + } } } @@ -204,6 +205,13 @@ impl SourceId { self.inner.precise.as_ref().map(|s| s.as_slice()) } + pub fn git_reference(&self) -> Option<&str> { + match self.inner.kind { + GitKind(ref s) => Some(s.as_slice()), + _ => None, + } + } + pub fn with_precise(&self, v: Option) -> SourceId { SourceId { inner: Arc::new(SourceIdInner { diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index 4027d3a2f..195e75cae 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -3,7 +3,7 @@ use std::hash::{Hash, Hasher}; use std::hash::sip::SipHasher; use std::io::{fs, File, USER_RWX, BufferedReader}; -use core::{Package, Target, PathKind}; +use core::{Package, Target}; use util; use util::{CargoResult, Fresh, Dirty, Freshness, internal, Require, profile}; @@ -54,10 +54,7 @@ pub fn prepare_target(cx: &mut Context, pkg: &Package, target: &Target, // constant (which is the responsibility of the source) let use_pkg = { let doc = target.get_profile().is_doc(); - let path = match *pkg.get_summary().get_source_id().get_kind() { - PathKind => true, - _ => false, - }; + let path = pkg.get_summary().get_source_id().is_path(); doc || !path }; diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 2998dda1e..fbef2c629 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -7,7 +7,7 @@ use git2; use registry::{Registry, NewCrate, NewCrateDependency}; use core::source::Source; -use core::{Package, MultiShell, SourceId, RegistryKind}; +use core::{Package, MultiShell, SourceId}; use core::manifest::ManifestMetadata; use ops; use sources::{PathSource, RegistrySource}; diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index c2731fa78..870bcaa3e 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -4,7 +4,7 @@ use std::hash::sip::SipHasher; use std::mem; use url::{mod, Url}; -use core::source::{Source, SourceId, GitKind}; +use core::source::{Source, SourceId}; use core::{Package, PackageId, Summary, Registry, Dependency}; use util::{CargoResult, Config, to_hex}; use sources::PathSource; @@ -28,9 +28,9 @@ impl<'a, 'b> GitSource<'a, 'b> { config: &'a mut Config<'b>) -> GitSource<'a, 'b> { assert!(source_id.is_git(), "id is not git, id={}", source_id); - let reference = match *source_id.get_kind() { - GitKind(ref reference) => reference, - _ => fail!("Not a git source; id={}", source_id) + let reference = match source_id.git_reference() { + Some(reference) => reference, + None => fail!("Not a git source; id={}", source_id), }; let remote = GitRemote::new(source_id.get_url()); diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index 465280331..27082011d 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -9,7 +9,7 @@ use toml; use semver; use serialize::{Decodable, Decoder}; -use core::{SourceId, GitKind}; +use core::SourceId; use core::manifest::{LibKind, Lib, Dylib, Profile, ManifestMetadata}; use core::{Summary, Manifest, Target, Dependency, PackageId}; use core::package_id::Metadata; @@ -526,11 +526,10 @@ fn process_dependencies<'a>(cx: &mut Context<'a>, dev: bool, let new_source_id = match details.git { Some(ref git) => { - let kind = GitKind(reference.clone()); let loc = try!(git.as_slice().to_url().map_err(|e| { human(e) })); - Some(SourceId::new(kind, loc)) + Some(SourceId::for_git(&loc, reference.as_slice())) } None => { details.path.as_ref().map(|path| { diff --git a/tests/resolve.rs b/tests/resolve.rs index 30c8c04fc..44fae3425 100644 --- a/tests/resolve.rs +++ b/tests/resolve.rs @@ -7,7 +7,7 @@ use std::collections::HashMap; use hamcrest::{assert_that, equal_to, contains}; -use cargo::core::source::{SourceId, RegistryKind, GitKind}; +use cargo::core::source::SourceId; use cargo::core::{Dependency, PackageId, Summary, Registry}; use cargo::util::{CargoResult, ToUrl}; use cargo::core::resolver::{mod, ResolveEverything}; @@ -29,7 +29,7 @@ trait ToDep { impl ToDep for &'static str { fn to_dep(self) -> Dependency { let url = "http://example.com".to_url().unwrap(); - let source_id = SourceId::new(RegistryKind, url); + let source_id = SourceId::for_registry(&url); Dependency::parse(self, Some("1.0.0"), &source_id).unwrap() } } @@ -71,7 +71,7 @@ macro_rules! pkg( fn registry_loc() -> SourceId { let remote = "http://example.com".to_url().unwrap(); - SourceId::new(RegistryKind, remote) + SourceId::for_registry(&remote) } fn pkg(name: &str) -> Summary { @@ -84,8 +84,7 @@ fn pkg_id(name: &str) -> PackageId { fn pkg_id_loc(name: &str, loc: &str) -> PackageId { let remote = loc.to_url(); - let source_id = SourceId::new(GitKind("master".to_string()), - remote.unwrap()); + let source_id = SourceId::for_git(&remote.unwrap(), "master"); PackageId::new(name, "1.0.0", &source_id).unwrap() } @@ -97,13 +96,13 @@ fn pkg_loc(name: &str, loc: &str) -> Summary { fn dep(name: &str) -> Dependency { dep_req(name, "1.0.0") } fn dep_req(name: &str, req: &str) -> Dependency { let url = "http://example.com".to_url().unwrap(); - let source_id = SourceId::new(RegistryKind, url); + let source_id = SourceId::for_registry(&url); Dependency::parse(name, Some(req), &source_id).unwrap() } fn dep_loc(name: &str, location: &str) -> Dependency { let url = location.to_url().unwrap(); - let source_id = SourceId::new(GitKind("master".to_string()), url); + let source_id = SourceId::for_git(&url, "master"); Dependency::parse(name, Some("1.0.0"), &source_id).unwrap() }