From 5ddc8b13388b108512ea7981ad597b23ff1fb7a8 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Sun, 25 Nov 2018 14:16:49 +0000 Subject: [PATCH 1/8] Test PackageId Debug & Display --- src/cargo/core/package_id.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index 0be2cd9e40d..ae7279291f3 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -195,4 +195,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()); + } } From 2e35475121aaf00b11220f93d7a1c8a2cb70ea24 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 26 Nov 2018 10:50:26 +0000 Subject: [PATCH 2/8] Add debug info to git::two_deps_only_update_one test --- tests/testsuite/git.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) 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!( From 0593e1c1670f1e38ade8f7358e7435a1f3bda66f Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Sun, 18 Nov 2018 21:03:55 +0000 Subject: [PATCH 3/8] Intern PackageId --- src/cargo/core/package_id.rs | 100 ++++++++++++++++------------- src/cargo/core/source/source_id.rs | 8 +++ 2 files changed, 62 insertions(+), 46 deletions(-) diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index ae7279291f3..4120f6779cc 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -1,9 +1,9 @@ -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::sync::Mutex; use semver; use serde::de; @@ -13,19 +13,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, PartialEq, Eq, Hash, 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 +86,37 @@ 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) - } -} -impl Eq for PackageId {} -impl Ord for PackageId { - fn cmp(&self, other: &PackageId) -> Ordering { - (*self.inner).cmp(&*other.inner) + } + )) } } 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 +130,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> { diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index ea29d39c863..c65ba4e63cd 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 { + self.inner == other.inner + } + + pub fn full_hash(&self, into: &mut S) { + self.inner.hash(into) + } } impl PartialOrd for SourceId { From 0abfcc0dd4515fe4add913a36f5c4adc3dcba42b Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Sun, 25 Nov 2018 14:18:36 +0000 Subject: [PATCH 4/8] Make PackageId derive Copy --- src/cargo/core/package_id.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index 4120f6779cc..7231c137251 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -18,7 +18,7 @@ lazy_static! { } /// Identifier for a specific version of a package in a specific source. -#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct PackageId { inner: &'static PackageIdInner, } From a73c5171f8a55fa698b38014f62bacdfcab0dea0 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Sun, 25 Nov 2018 14:20:42 +0000 Subject: [PATCH 5/8] Add pointer equality to PartialEq for PackageId --- src/cargo/core/package_id.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index 7231c137251..885d2c61caf 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -3,6 +3,7 @@ use std::fmt::{self, Formatter}; use std::hash; use std::hash::Hash; use std::path::Path; +use std::ptr; use std::sync::Mutex; use semver; @@ -18,7 +19,7 @@ lazy_static! { } /// Identifier for a specific version of a package in a specific source. -#[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[derive(Clone, Copy, Eq, Hash, PartialOrd, Ord)] pub struct PackageId { inner: &'static PackageIdInner, } @@ -96,6 +97,15 @@ impl<'de> de::Deserialize<'de> for PackageId { } } +impl PartialEq for PackageId { + fn eq(&self, other: &PackageId) -> bool { + if ptr::eq(self.inner, other.inner) { + return true; + } + (*self.inner).eq(&*other.inner) + } +} + impl PackageId { pub fn new(name: &str, version: T, sid: SourceId) -> CargoResult { let v = version.to_semver()?; From ea87ca3e981eb5b05ef69eb1b052e9bc5ae9e6aa Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 26 Nov 2018 17:32:27 +0000 Subject: [PATCH 6/8] Use pointer eq/hash for SourceIdInner --- src/cargo/core/source/source_id.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index c65ba4e63cd..120816b864a 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -337,11 +337,11 @@ impl SourceId { } pub fn full_eq(&self, other: &SourceId) -> bool { - self.inner == other.inner + ptr::eq(self.inner, other.inner) } pub fn full_hash(&self, into: &mut S) { - self.inner.hash(into) + (self.inner as *const SourceIdInner).hash(into) } } From e269936633bb095bea75978cf5440570c228e287 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 26 Nov 2018 17:32:47 +0000 Subject: [PATCH 7/8] Re-expose custom SourceId equality in outer PackageId --- src/cargo/core/package_id.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index 885d2c61caf..2f9f9f21497 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -19,7 +19,7 @@ lazy_static! { } /// Identifier for a specific version of a package in a specific source. -#[derive(Clone, Copy, Eq, Hash, PartialOrd, Ord)] +#[derive(Clone, Copy, Eq, PartialOrd, Ord)] pub struct PackageId { inner: &'static PackageIdInner, } @@ -102,7 +102,17 @@ impl PartialEq for PackageId { if ptr::eq(self.inner, other.inner) { return true; } - (*self.inner).eq(&*other.inner) + self.inner.name == other.inner.name + && self.inner.version == other.inner.version + && self.inner.source_id == other.inner.source_id + } +} + +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); } } From efd03bdecfa108d649a6e0acc266bcdcbd2690c5 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 26 Nov 2018 17:45:52 +0000 Subject: [PATCH 8/8] A nicer way to hash a pointer --- src/cargo/core/source/source_id.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index 120816b864a..8a5557e6ce1 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -341,7 +341,7 @@ impl SourceId { } pub fn full_hash(&self, into: &mut S) { - (self.inner as *const SourceIdInner).hash(into) + ptr::NonNull::from(self.inner).hash(into) } }