From 361ae355587be5dc1eac604c60e7d35c9ba4c1aa Mon Sep 17 00:00:00 2001 From: Siddharth Agarwal Date: Wed, 11 Apr 2018 13:33:18 -0700 Subject: [PATCH] mercurial: define a HgNodeKey key for node keys Summary: This is used in a bunch of places throughout bundle2-resolver, and is the default key that should be used throughout Mercurial. Reviewed By: farnz Differential Revision: D7578081 fbshipit-source-id: 8f44b54189b22b0aba8f08853bbd2bbbaa3efc5c --- bundle2-resolver/src/changegroup/filelog.rs | 75 +++++++++++---------- bundle2-resolver/src/resolver.rs | 25 +++++-- bundle2-resolver/src/upload_blobs.rs | 5 +- bundle2-resolver/src/wirepackparser.rs | 34 +++++----- mercurial/src/lib.rs | 2 +- mercurial/src/nodehash.rs | 9 +++ 6 files changed, 86 insertions(+), 64 deletions(-) diff --git a/bundle2-resolver/src/changegroup/filelog.rs b/bundle2-resolver/src/changegroup/filelog.rs index c063e6161d..a5aa34fc76 100644 --- a/bundle2-resolver/src/changegroup/filelog.rs +++ b/bundle2-resolver/src/changegroup/filelog.rs @@ -33,8 +33,7 @@ pub struct FilelogDeltaed { #[derive(Debug, Clone, Eq, PartialEq)] pub struct Filelog { - pub path: RepoPath, - pub node: mercurial::NodeHash, + pub node_key: mercurial::HgNodeKey, pub p1: Option, pub p2: Option, pub linknode: mercurial::NodeHash, @@ -44,9 +43,8 @@ pub struct Filelog { impl UploadableBlob for Filelog { type Value = Shared>>; - fn upload(self, repo: &BlobRepo) -> Result<((mercurial::NodeHash, RepoPath), Self::Value)> { - let path = self.path; - let node = self.node; + fn upload(self, repo: &BlobRepo) -> Result<(mercurial::HgNodeKey, Self::Value)> { + let node_key = self.node_key; let p1 = self.p1.map(|p1| NodeHash::new(p1.sha1().clone())); let p2 = self.p2.map(|p1| NodeHash::new(p1.sha1().clone())); @@ -55,8 +53,8 @@ impl UploadableBlob for Filelog { manifest::Type::File(FileType::Regular), p1, p2, - path.clone(), - ).map(move |(_node, fut)| ((node, path), fut.map_err(Error::compat).boxify().shared())) + node_key.path.clone(), + ).map(move |(_node, fut)| (node_key, fut.map_err(Error::compat).boxify().shared())) } } @@ -80,8 +78,10 @@ where .decode(node.clone(), base.into_option(), delta) .and_then(move |blob| { Ok(Filelog { - path: RepoPath::file(path)?, - node, + node_key: mercurial::HgNodeKey { + path: RepoPath::FilePath(path), + hash: node, + }, p1: p1.into_option(), p2: p2.into_option(), linknode, @@ -167,9 +167,10 @@ impl DeltaCache { impl Arbitrary for Filelog { fn arbitrary(g: &mut G) -> Self { Filelog { - path: RepoPath::file(MPath::arbitrary(g)) - .unwrap_or(RepoPath::file(MPath::new(b"test").unwrap()).unwrap()), - node: mercurial::NodeHash::arbitrary(g), + node_key: mercurial::HgNodeKey { + path: RepoPath::FilePath(MPath::arbitrary(g)), + hash: mercurial::NodeHash::arbitrary(g), + }, p1: mercurial::NodeHash::arbitrary(g).into_option(), p2: mercurial::NodeHash::arbitrary(g).into_option(), linknode: mercurial::NodeHash::arbitrary(g), @@ -185,15 +186,9 @@ impl Arbitrary for Filelog { let mut result = Vec::new(); - if self.path.mpath() != Some(&MPath::new(b"test").unwrap()) { + if self.node_key.hash != mercurial::NULL_HASH { let mut f = self.clone(); - f.path = RepoPath::file(MPath::new(b"test").unwrap()).unwrap(); - append(&mut result, f); - } - - if self.node != mercurial::NULL_HASH { - let mut f = self.clone(); - f.node = mercurial::NULL_HASH; + f.node_key.hash = mercurial::NULL_HASH; append(&mut result, f); } @@ -281,9 +276,9 @@ mod tests { fn filelog_to_deltaed(f: &Filelog) -> FilelogDeltaed { FilelogDeltaed { - path: f.path.mpath().unwrap().clone(), + path: f.node_key.path.mpath().unwrap().clone(), chunk: CgDeltaChunk { - node: f.node.clone(), + node: f.node_key.hash.clone(), p1: f.p1.clone().unwrap_or(NULL_HASH), p2: f.p2.clone().unwrap_or(NULL_HASH), base: NULL_HASH, @@ -343,8 +338,10 @@ mod tests { #[test] fn two_fulltext_files() { let f1 = Filelog { - path: RepoPath::file(MPath::new(b"test").unwrap()).unwrap(), - node: ONES_HASH, + node_key: mercurial::HgNodeKey { + path: RepoPath::FilePath(MPath::new(b"test").unwrap()), + hash: ONES_HASH, + }, p1: Some(TWOS_HASH), p2: Some(THREES_HASH), linknode: FOURS_HASH, @@ -352,8 +349,10 @@ mod tests { }; let f2 = Filelog { - path: RepoPath::file(MPath::new(b"test2").unwrap()).unwrap(), - node: FIVES_HASH, + node_key: mercurial::HgNodeKey { + path: RepoPath::FilePath(MPath::new(b"test2").unwrap()), + hash: FIVES_HASH, + }, p1: Some(SIXES_HASH), p2: Some(SEVENS_HASH), linknode: EIGHTS_HASH, @@ -368,8 +367,10 @@ mod tests { fn files_check_order(correct_order: bool) { let f1 = Filelog { - path: RepoPath::file(MPath::new(b"test").unwrap()).unwrap(), - node: ONES_HASH, + node_key: mercurial::HgNodeKey { + path: RepoPath::FilePath(MPath::new(b"test").unwrap()), + hash: ONES_HASH, + }, p1: Some(TWOS_HASH), p2: Some(THREES_HASH), linknode: FOURS_HASH, @@ -377,8 +378,10 @@ mod tests { }; let f2 = Filelog { - path: RepoPath::file(MPath::new(b"test2").unwrap()).unwrap(), - node: FIVES_HASH, + node_key: mercurial::HgNodeKey { + path: RepoPath::FilePath(MPath::new(b"test2").unwrap()), + hash: FIVES_HASH, + }, p1: Some(SIXES_HASH), p2: Some(SEVENS_HASH), linknode: EIGHTS_HASH, @@ -388,7 +391,7 @@ mod tests { let f1_deltaed = filelog_to_deltaed(&f1); let mut f2_deltaed = filelog_to_deltaed(&f2); - f2_deltaed.chunk.base = f1.node.clone(); + f2_deltaed.chunk.base = f1.node_key.hash.clone(); f2_deltaed.chunk.delta = compute_delta(f1.blob.as_slice().unwrap(), f2.blob.as_slice().unwrap()); @@ -445,17 +448,17 @@ mod tests { let mut hash_gen = NodeHashGen::new(); let mut f = f.clone(); - f.node = hash_gen.next(); + f.node_key.hash = hash_gen.next(); let mut fs = fs.clone(); for el in fs.iter_mut() { - el.node = hash_gen.next(); + el.node_key.hash = hash_gen.next(); } let mut deltas = vec![filelog_to_deltaed(&f)]; for filelog in &fs { let mut delta = filelog_to_deltaed(filelog); - delta.chunk.base = f.node.clone(); + delta.chunk.base = f.node_key.hash.clone(); delta.chunk.delta = compute_delta(f.blob.as_slice().unwrap(), filelog.blob.as_slice().unwrap()); deltas.push(delta); @@ -471,7 +474,7 @@ mod tests { let mut fs = fs.clone(); for el in fs.iter_mut() { - el.node = hash_gen.next(); + el.node_key.hash = hash_gen.next(); } let deltas = { @@ -483,7 +486,7 @@ mod tests { for (prev, next) in fs.iter().zip(it) { let mut delta = filelog_to_deltaed(next); - delta.chunk.base = prev.node.clone(); + delta.chunk.base = prev.node_key.hash.clone(); delta.chunk.delta = compute_delta(prev.blob.as_slice().unwrap(), next.blob.as_slice().unwrap()); deltas.push(delta); diff --git a/bundle2-resolver/src/resolver.rs b/bundle2-resolver/src/resolver.rs index c1a0dc7073..139a3a3d90 100644 --- a/bundle2-resolver/src/resolver.rs +++ b/bundle2-resolver/src/resolver.rs @@ -31,9 +31,8 @@ use wirepackparser::{TreemanifestBundle2Parser, TreemanifestEntry}; type PartId = u32; type Changesets = Vec<(mercurial::NodeHash, RevlogChangeset)>; -type Filelogs = HashMap<(mercurial::NodeHash, RepoPath), ::Value>; -type Manifests = - HashMap<(mercurial::NodeHash, RepoPath), ::Value>; +type Filelogs = HashMap::Value>; +type Manifests = HashMap::Value>; type UploadedChangesets = HashMap; /// The resolve function takes a bundle2, interprets it's content as Changesets, Filelogs and @@ -507,7 +506,10 @@ fn walk_manifests( }; if details.is_tree() { - let key = (nodehash, RepoPath::DirectoryPath(next_path)); + let key = mercurial::HgNodeKey { + path: RepoPath::DirectoryPath(next_path), + hash: nodehash, + }; if let Some(&(ref manifest_content, ref blobfuture)) = manifests.get(&key) { entries.push( @@ -518,14 +520,18 @@ fn walk_manifests( .boxify(), ); entries.append(&mut walk_helper( - &key.1, + &key.path, manifest_content, manifests, filelogs, )?); } } else { - if let Some(blobfuture) = filelogs.get(&(nodehash, RepoPath::FilePath(next_path))) { + let key = mercurial::HgNodeKey { + path: RepoPath::FilePath(next_path), + hash: nodehash, + }; + if let Some(blobfuture) = filelogs.get(&key) { entries.push( blobfuture .clone() @@ -540,8 +546,13 @@ fn walk_manifests( Ok(entries) } + let root_key = mercurial::HgNodeKey { + path: RepoPath::root(), + hash: manifest_root_id.clone().into_nodehash(), + }; + let &(ref manifest_content, ref manifest_root) = manifests - .get(&(manifest_root_id.clone().into_nodehash(), RepoPath::root())) + .get(&root_key) .ok_or_else(|| format_err!("Missing root tree manifest"))?; Ok(( diff --git a/bundle2-resolver/src/upload_blobs.rs b/bundle2-resolver/src/upload_blobs.rs index f8a4deeae0..c51e9cf579 100644 --- a/bundle2-resolver/src/upload_blobs.rs +++ b/bundle2-resolver/src/upload_blobs.rs @@ -12,14 +12,13 @@ use futures_ext::{BoxFuture, FutureExt}; use blobrepo::BlobRepo; use mercurial; -use mercurial_types::RepoPath; use errors::*; pub trait UploadableBlob { type Value: Send + 'static; - fn upload(self, repo: &BlobRepo) -> Result<((mercurial::NodeHash, RepoPath), Self::Value)>; + fn upload(self, repo: &BlobRepo) -> Result<(mercurial::HgNodeKey, Self::Value)>; } #[derive(PartialEq, Eq)] @@ -33,7 +32,7 @@ pub fn upload_blobs( repo: Arc, blobs: S, ubtype: UploadBlobsType, -) -> BoxFuture, Error> +) -> BoxFuture, Error> where S: Stream + Send + 'static, B: UploadableBlob, diff --git a/bundle2-resolver/src/wirepackparser.rs b/bundle2-resolver/src/wirepackparser.rs index d72d9a205c..9432472b05 100644 --- a/bundle2-resolver/src/wirepackparser.rs +++ b/bundle2-resolver/src/wirepackparser.rs @@ -58,30 +58,27 @@ where #[derive(Debug, Eq, PartialEq)] pub struct TreemanifestEntry { - pub node: mercurial::NodeHash, + pub node_key: mercurial::HgNodeKey, pub data: Bytes, pub p1: Option, pub p2: Option, - pub path: RepoPath, pub manifest_content: ManifestContent, } impl TreemanifestEntry { fn new( - node: mercurial::NodeHash, + node_key: mercurial::HgNodeKey, data: Bytes, p1: mercurial::NodeHash, p2: mercurial::NodeHash, - path: RepoPath, ) -> Result { let manifest_content = ManifestContent::parse(data.as_ref())?; Ok(Self { - node, + node_key, data, p1: p1.into_option(), p2: p2.into_option(), - path, manifest_content, }) } @@ -93,9 +90,8 @@ impl UploadableBlob for TreemanifestEntry { Shared>>, ); - fn upload(self, repo: &BlobRepo) -> Result<((mercurial::NodeHash, RepoPath), Self::Value)> { - let path = self.path; - let node = self.node; + fn upload(self, repo: &BlobRepo) -> Result<(mercurial::HgNodeKey, Self::Value)> { + let node_key = self.node_key; let manifest_content = self.manifest_content; let p1 = self.p1.map(|p1| NodeHash::new(p1.sha1().clone())); let p2 = self.p2.map(|p1| NodeHash::new(p1.sha1().clone())); @@ -104,10 +100,10 @@ impl UploadableBlob for TreemanifestEntry { manifest::Type::Tree, p1, p2, - path.clone(), + node_key.path.clone(), ).map(move |(_node, value)| { ( - (node, path), + node_key, ( manifest_content, value.map_err(Error::compat).boxify().shared(), @@ -172,13 +168,15 @@ impl WirePackPartProcessor for TreemanifestPartProcessor { return Err(ErrorKind::MalformedTreemanifestPart(msg).into()); } - let node = unwrap_field(&mut self.node, "node")?; + let node_key = mercurial::HgNodeKey { + path: unwrap_field(&mut self.path, "path")?, + hash: unwrap_field(&mut self.node, "node")?, + }; let bytes = Bytes::from(delta::apply("".as_bytes(), &data_entry.delta)); let p1 = unwrap_field(&mut self.p1, "p1")?; let p2 = unwrap_field(&mut self.p2, "p2")?; - let path = unwrap_field(&mut self.path, "path")?; - Ok(Some(TreemanifestEntry::new(node, bytes, p1, p2, path)?)) + Ok(Some(TreemanifestEntry::new(node_key, bytes, p1, p2)?)) } fn end(&mut self) -> Result> { @@ -337,7 +335,10 @@ mod test { } fn get_expected_entry() -> TreemanifestEntry { - let node = nodehash_mocks::ONES_HASH; + let node_key = mercurial::HgNodeKey { + path: RepoPath::root(), + hash: nodehash_mocks::ONES_HASH, + }; let p1 = nodehash_mocks::TWOS_HASH; let p2 = nodehash_mocks::THREES_HASH; @@ -347,8 +348,7 @@ mod test { data }; - let entry = - TreemanifestEntry::new(node, Bytes::from(data), p1, p2, RepoPath::root()).unwrap(); + let entry = TreemanifestEntry::new(node_key, Bytes::from(data), p1, p2).unwrap(); assert_eq!( entry.manifest_content, diff --git a/mercurial/src/lib.rs b/mercurial/src/lib.rs index 447f066c57..1686e395a8 100644 --- a/mercurial/src/lib.rs +++ b/mercurial/src/lib.rs @@ -72,5 +72,5 @@ pub use errors::*; pub use blobnode::{BlobNode, Parents}; pub use changeset::RevlogChangeset; pub use manifest::{EntryContent, RevlogEntry}; -pub use nodehash::{EntryId, HgChangesetId, HgManifestId, NodeHash, NULL_HASH}; +pub use nodehash::{EntryId, HgChangesetId, HgManifestId, HgNodeKey, NodeHash, NULL_HASH}; pub use revlogrepo::{RevlogManifest, RevlogRepo, RevlogRepoOptions}; diff --git a/mercurial/src/nodehash.rs b/mercurial/src/nodehash.rs index 3ab67b0ecc..4152784922 100644 --- a/mercurial/src/nodehash.rs +++ b/mercurial/src/nodehash.rs @@ -14,6 +14,7 @@ use ascii::{AsciiStr, AsciiString}; use quickcheck::{single_shrinker, Arbitrary, Gen}; use errors::*; +use mercurial_types::RepoPath; use mercurial_types::hash::{self, Sha1}; use serde; use sql_types::{HgChangesetIdSql, HgManifestIdSql}; @@ -266,3 +267,11 @@ impl Display for EntryId { self.0.fmt(fmt) } } + +/// A (path, hash) combination. This is the key used throughout Mercurial for manifest and file +/// nodes. +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +pub struct HgNodeKey { + pub path: RepoPath, + pub hash: NodeHash, +}