diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index 0be2cd9e40d..2f9f9f21497 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -1,9 +1,10 @@ -use std::cmp::Ordering; +use std::collections::HashSet; use std::fmt::{self, Formatter}; use std::hash; use std::hash::Hash; use std::path::Path; -use std::sync::Arc; +use std::ptr; +use std::sync::Mutex; use semver; use serde::de; @@ -13,19 +14,41 @@ use core::interning::InternedString; use core::source::SourceId; use util::{CargoResult, ToSemver}; +lazy_static! { + static ref PACKAGE_ID_CACHE: Mutex> = Mutex::new(HashSet::new()); +} + /// Identifier for a specific version of a package in a specific source. -#[derive(Clone)] +#[derive(Clone, Copy, Eq, PartialOrd, Ord)] pub struct PackageId { - inner: Arc, + inner: &'static PackageIdInner, } -#[derive(PartialEq, PartialOrd, Eq, Ord)] +#[derive(PartialOrd, Eq, Ord)] struct PackageIdInner { name: InternedString, version: semver::Version, source_id: SourceId, } +// Custom equality that uses full equality of SourceId, rather than its custom equality. +impl PartialEq for PackageIdInner { + fn eq(&self, other: &Self) -> bool { + self.name == other.name + && self.version == other.version + && self.source_id.full_eq(&other.source_id) + } +} + +// Custom hash that is coherent with the custom equality above. +impl Hash for PackageIdInner { + fn hash(&self, into: &mut S) { + self.name.hash(into); + self.version.hash(into); + self.source_id.full_hash(into); + } +} + impl ser::Serialize for PackageId { fn serialize(&self, s: S) -> Result where @@ -64,51 +87,56 @@ impl<'de> de::Deserialize<'de> for PackageId { }; let source_id = SourceId::from_url(url).map_err(de::Error::custom)?; - Ok(PackageId { - inner: Arc::new(PackageIdInner { + Ok(PackageId::wrap( + PackageIdInner { name: InternedString::new(name), version, source_id, - }), - }) - } -} - -impl Hash for PackageId { - fn hash(&self, state: &mut S) { - self.inner.name.hash(state); - self.inner.version.hash(state); - self.inner.source_id.hash(state); + } + )) } } impl PartialEq for PackageId { fn eq(&self, other: &PackageId) -> bool { - (*self.inner).eq(&*other.inner) - } -} -impl PartialOrd for PackageId { - fn partial_cmp(&self, other: &PackageId) -> Option { - (*self.inner).partial_cmp(&*other.inner) + if ptr::eq(self.inner, other.inner) { + return true; + } + self.inner.name == other.inner.name + && self.inner.version == other.inner.version + && self.inner.source_id == other.inner.source_id } } -impl Eq for PackageId {} -impl Ord for PackageId { - fn cmp(&self, other: &PackageId) -> Ordering { - (*self.inner).cmp(&*other.inner) + +impl<'a> Hash for PackageId { + fn hash(&self, state: &mut S) { + self.inner.name.hash(state); + self.inner.version.hash(state); + self.inner.source_id.hash(state); } } impl PackageId { pub fn new(name: &str, version: T, sid: SourceId) -> CargoResult { let v = version.to_semver()?; - Ok(PackageId { - inner: Arc::new(PackageIdInner { + + Ok(PackageId::wrap( + PackageIdInner { name: InternedString::new(name), version: v, source_id: sid, - }), - }) + } + )) + } + + fn wrap(inner: PackageIdInner) -> PackageId { + let mut cache = PACKAGE_ID_CACHE.lock().unwrap(); + let inner = cache.get(&inner).map(|&x| x).unwrap_or_else(|| { + let inner = Box::leak(Box::new(inner)); + cache.insert(inner); + inner + }); + PackageId { inner } } pub fn name(&self) -> InternedString { @@ -122,23 +150,23 @@ impl PackageId { } pub fn with_precise(&self, precise: Option) -> PackageId { - PackageId { - inner: Arc::new(PackageIdInner { + PackageId::wrap( + PackageIdInner { name: self.inner.name, version: self.inner.version.clone(), source_id: self.inner.source_id.with_precise(precise), - }), - } + } + ) } pub fn with_source_id(&self, source: SourceId) -> PackageId { - PackageId { - inner: Arc::new(PackageIdInner { + PackageId::wrap( + PackageIdInner { name: self.inner.name, version: self.inner.version.clone(), source_id: source, - }), - } + } + ) } pub fn stable_hash<'a>(&'a self, workspace: &'a Path) -> PackageIdStableHash<'a> { @@ -195,4 +223,27 @@ mod tests { assert!(PackageId::new("foo", "bar", repo).is_err()); assert!(PackageId::new("foo", "", repo).is_err()); } + + #[test] + fn debug() { + let loc = CRATES_IO_INDEX.to_url().unwrap(); + let pkg_id = PackageId::new("foo", "1.0.0", SourceId::for_registry(&loc).unwrap()).unwrap(); + assert_eq!(r#"PackageId { name: "foo", version: "1.0.0", source: "registry `https://github.com/rust-lang/crates.io-index`" }"#, format!("{:?}", pkg_id)); + + let pretty = r#" +PackageId { + name: "foo", + version: "1.0.0", + source: "registry `https://github.com/rust-lang/crates.io-index`" +} +"#.trim(); + assert_eq!(pretty, format!("{:#?}", pkg_id)); + } + + #[test] + fn display() { + let loc = CRATES_IO_INDEX.to_url().unwrap(); + let pkg_id = PackageId::new("foo", "1.0.0", SourceId::for_registry(&loc).unwrap()).unwrap(); + assert_eq!("foo v1.0.0", pkg_id.to_string()); + } } diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index ea29d39c863..8a5557e6ce1 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -335,6 +335,14 @@ impl SourceId { } self.hash(into) } + + pub fn full_eq(&self, other: &SourceId) -> bool { + ptr::eq(self.inner, other.inner) + } + + pub fn full_hash(&self, into: &mut S) { + ptr::NonNull::from(self.inner).hash(into) + } } impl PartialOrd for SourceId { diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index ea89a964bdc..027f5ec77e9 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -11,6 +11,7 @@ use std::thread; use support::paths::{self, CargoPathExt}; use support::sleep_ms; use support::{basic_lib_manifest, basic_manifest, git, main_file, path2url, project}; +use support::Project; #[test] fn cargo_compile_simple_git_dep() { @@ -1090,6 +1091,18 @@ fn two_deps_only_update_one() { ).file("src/main.rs", "fn main() {}") .build(); + fn oid_to_short_sha(oid: git2::Oid) -> String { + oid.to_string()[..8].to_string() + } + fn git_repo_head_sha(p: &Project) -> String { + let repo = git2::Repository::open(p.root()).unwrap(); + let head = repo.head().unwrap().target().unwrap(); + oid_to_short_sha(head) + } + + println!("dep1 head sha: {}", git_repo_head_sha(&git1)); + println!("dep2 head sha: {}", git_repo_head_sha(&git2)); + p.cargo("build") .with_stderr( "[UPDATING] git repository `[..]`\n\ @@ -1106,7 +1119,8 @@ fn two_deps_only_update_one() { .unwrap(); let repo = git2::Repository::open(&git1.root()).unwrap(); git::add(&repo); - git::commit(&repo); + let oid = git::commit(&repo); + println!("dep1 head sha: {}", oid_to_short_sha(oid)); p.cargo("update -p dep1") .with_stderr(&format!(