diff --git a/blobrepo/src/file.rs b/blobrepo/src/file.rs index 6df0d0f079..359624bb59 100644 --- a/blobrepo/src/file.rs +++ b/blobrepo/src/file.rs @@ -10,7 +10,7 @@ use futures::future::Future; use futures_ext::{BoxFuture, FutureExt}; use mercurial::file; -use mercurial_types::{Blob, MPath, NodeHash, Parents}; +use mercurial_types::{Blob, MPath, NodeHash, Parents, RepoPath}; use mercurial_types::manifest::{Content, Entry, Manifest, Type}; use blobstore::Blobstore; @@ -23,7 +23,7 @@ use utils::{get_node, RawNodeBlob}; pub struct BlobEntry { blobstore: B, - path: MPath, // XXX full path? Parent reference? + path: RepoPath, nodeid: NodeHash, ty: Type, } @@ -60,13 +60,17 @@ impl BlobEntry where B: Blobstore + Sync + Clone, { - pub fn new(blobstore: B, path: MPath, nodeid: NodeHash, ty: Type) -> Self { - Self { + pub fn new(blobstore: B, path: MPath, nodeid: NodeHash, ty: Type) -> Result { + let path = match ty { + Type::Tree => RepoPath::dir(path)?, + _ => RepoPath::file(path)?, + }; + Ok(Self { blobstore, path, nodeid, ty, - } + }) } fn get_node(&self) -> BoxFuture { @@ -158,7 +162,13 @@ where &self.nodeid } - fn get_path(&self) -> &MPath { + fn get_path(&self) -> &RepoPath { &self.path } + + fn get_mpath(&self) -> &MPath { + self.path + .mpath() + .expect("entries should always have an associated path") + } } diff --git a/blobrepo/src/manifest.rs b/blobrepo/src/manifest.rs index 1d92c1023e..0b885e566a 100644 --- a/blobrepo/src/manifest.rs +++ b/blobrepo/src/manifest.rs @@ -9,7 +9,7 @@ use std::collections::BTreeMap; use futures::future::{Future, IntoFuture}; -use futures::stream; +use futures::stream::{self, Stream}; use futures_ext::{BoxFuture, BoxStream, FutureExt, StreamExt}; use mercurial::manifest::revlog::{self, Details}; @@ -67,15 +67,15 @@ where &self, path: &MPath, ) -> BoxFuture + Sync>>, Self::Error> { - let res = self.files - .get(path) - .map({ - let blobstore = self.blobstore.clone(); - move |d| BlobEntry::new(blobstore, path.clone(), *d.nodeid(), d.flag()) - }) - .map(|e| e.boxed()); + let res = self.files.get(path).map({ + let blobstore = self.blobstore.clone(); + move |d| BlobEntry::new(blobstore, path.clone(), *d.nodeid(), d.flag()) + }); - Ok(res).into_future().boxify() + match res { + Some(e_res) => e_res.map(|e| Some(e.boxed())).into_future().boxify(), + None => Ok(None).into_future().boxify(), + } } fn list(&self) -> BoxStream + Sync>, Self::Error> { @@ -86,7 +86,8 @@ where let blobstore = self.blobstore.clone(); move |(path, d)| BlobEntry::new(blobstore.clone(), path, *d.nodeid(), d.flag()) }) - .map(|e| e.boxed()); - stream::iter_ok(entries).boxify() + .map(|e_res| e_res.map(|e| e.boxed())); + // TODO: (sid0) T23193289 replace with stream::iter_result once that becomes available + stream::iter_ok(entries).and_then(|x| x).boxify() } } diff --git a/cmds/blobimport/manifest.rs b/cmds/blobimport/manifest.rs index 73289695d9..def41759da 100644 --- a/cmds/blobimport/manifest.rs +++ b/cmds/blobimport/manifest.rs @@ -81,12 +81,7 @@ pub(crate) fn get_entry_stream( revlog_repo: RevlogRepo, cs_rev: RevIdx, ) -> Box>, Error = Error> + Send> { - let revlog = match entry.get_type() { - Type::File | Type::Executable | Type::Symlink => { - revlog_repo.get_file_revlog(entry.get_path()) - } - Type::Tree => revlog_repo.get_tree_revlog(entry.get_path()), - }; + let revlog = revlog_repo.get_path_revlog(entry.get_path()); let linkrev = revlog .and_then(|file_revlog| { diff --git a/eden_server/src/main.rs b/eden_server/src/main.rs index 93f30a9d2b..7a8d49762f 100644 --- a/eden_server/src/main.rs +++ b/eden_server/src/main.rs @@ -139,7 +139,7 @@ impl TreeMetadata { { TreeMetadata { hash: entry.get_hash().clone(), - path: PathBuf::from(OsString::from_vec(entry.get_path().to_vec())), + path: PathBuf::from(OsString::from_vec(entry.get_mpath().to_vec())), ty: entry.get_type(), size, } diff --git a/mercurial-types/mocks/manifest.rs b/mercurial-types/mocks/manifest.rs index 0a04a92f3b..75326211d8 100644 --- a/mercurial-types/mocks/manifest.rs +++ b/mercurial-types/mocks/manifest.rs @@ -10,7 +10,7 @@ use std::sync::Arc; use futures::{stream, IntoFuture}; use futures_ext::{BoxFuture, BoxStream, FutureExt, StreamExt}; -use mercurial_types::{Blob, Entry, MPath, Manifest, Type}; +use mercurial_types::{Blob, Entry, MPath, Manifest, RepoPath, Type}; use mercurial_types::blobnode::Parents; use mercurial_types::manifest::Content; use mercurial_types::nodehash::NodeHash; @@ -27,8 +27,9 @@ pub struct MockManifest { } impl MockManifest { - fn p(p: &'static str) -> MPath { - MPath::new(p).expect(&format!("invalid path {}", p)) + fn p(p: &'static str) -> RepoPath { + // This should also allow directory paths eventually. + RepoPath::file(p).expect(&format!("invalid path {}", p)) } pub fn new(paths: Vec<&'static str>) -> Self { @@ -73,7 +74,7 @@ where } struct MockEntry { - path: MPath, + path: RepoPath, content_factory: ContentFactory, phantom: PhantomData, } @@ -91,7 +92,7 @@ impl Clone for MockEntry { } impl MockEntry { - fn new(path: MPath, content_factory: ContentFactory) -> Self { + fn new(path: RepoPath, content_factory: ContentFactory) -> Self { MockEntry { path, content_factory, @@ -123,7 +124,12 @@ where fn get_hash(&self) -> &NodeHash { unimplemented!(); } - fn get_path(&self) -> &MPath { + fn get_path(&self) -> &RepoPath { &self.path } + fn get_mpath(&self) -> &MPath { + self.path + .mpath() + .expect("entries should always have an associated path") + } } diff --git a/mercurial-types/src/manifest.rs b/mercurial-types/src/manifest.rs index 04d3e41e84..fdfe99a683 100644 --- a/mercurial-types/src/manifest.rs +++ b/mercurial-types/src/manifest.rs @@ -15,7 +15,7 @@ use blob::Blob; use blobnode::Parents; use futures_ext::{BoxFuture, BoxStream, FutureExt, StreamExt}; use nodehash::NodeHash; -use path::MPath; +use path::{MPath, RepoPath}; /// Interface for a manifest pub trait Manifest: Send + 'static { @@ -167,7 +167,8 @@ pub trait Entry: Send + 'static { fn get_content(&self) -> BoxFuture, Self::Error>; fn get_size(&self) -> BoxFuture, Self::Error>; fn get_hash(&self) -> &NodeHash; - fn get_path(&self) -> &MPath; + fn get_path(&self) -> &RepoPath; + fn get_mpath(&self) -> &MPath; fn boxed(self) -> Box + Sync> where @@ -253,9 +254,13 @@ where self.entry.get_hash() } - fn get_path(&self) -> &MPath { + fn get_path(&self) -> &RepoPath { self.entry.get_path() } + + fn get_mpath(&self) -> &MPath { + self.entry.get_mpath() + } } impl Entry for Box + Sync> @@ -288,9 +293,13 @@ where (**self).get_hash() } - fn get_path(&self) -> &MPath { + fn get_path(&self) -> &RepoPath { (**self).get_path() } + + fn get_mpath(&self) -> &MPath { + (**self).get_mpath() + } } impl Display for Type { diff --git a/mercurial-types/src/path.rs b/mercurial-types/src/path.rs index 825391459e..79be43a1e1 100644 --- a/mercurial-types/src/path.rs +++ b/mercurial-types/src/path.rs @@ -81,6 +81,14 @@ impl RepoPath { } } + pub fn mpath(&self) -> Option<&MPath> { + match *self { + RepoPath::RootPath => None, + RepoPath::DirectoryPath(ref path) => Some(path), + RepoPath::FilePath(ref path) => Some(path), + } + } + /// Serialize this RepoPath into a string. This shouldn't (yet) be considered stable if the /// definition of RepoPath changes. pub fn serialize(&self) -> Vec { @@ -160,6 +168,11 @@ impl MPath { Ok(MPath { elements }) } + /// Create a new empty `MPath`. + pub fn empty() -> Self { + MPath { elements: vec![] } + } + fn verify(p: &[u8]) -> Result<()> { if p.contains(&0) { bail!(ErrorKind::InvalidPath( diff --git a/mercurial/src/errors.rs b/mercurial/src/errors.rs index 2df5f8289d..5ba7add3a1 100644 --- a/mercurial/src/errors.rs +++ b/mercurial/src/errors.rs @@ -24,6 +24,10 @@ error_chain! { description("repo error") display("{}", msg) } + Path(msg: String) { + description("path error") + display("{}", msg) + } UnknownReq(req: String) { description("unknown repo requirement") display("Unknown requirement \"{}\"", req) diff --git a/mercurial/src/manifest/revlog.rs b/mercurial/src/manifest/revlog.rs index 5136a8cfeb..660d8c8a6b 100644 --- a/mercurial/src/manifest/revlog.rs +++ b/mercurial/src/manifest/revlog.rs @@ -16,7 +16,7 @@ use futures_ext::{BoxFuture, BoxStream, FutureExt, StreamExt}; use errors::*; use file; -use mercurial_types::{Blob, BlobNode, MPath, NodeHash, Parents}; +use mercurial_types::{Blob, BlobNode, MPath, NodeHash, Parents, RepoPath}; use mercurial_types::manifest::{Content, Entry, Manifest, Type}; use RevlogRepo; @@ -197,7 +197,7 @@ where pub struct RevlogEntry { repo: RevlogRepo, - path: MPath, + path: RepoPath, details: Details, } @@ -209,13 +209,12 @@ impl Stream for RevlogListStream { fn poll(&mut self) -> Poll, Self::Error> { let v = self.0.next().map(|(path, details)| { - RevlogEntry { - repo: self.1.clone(), - path, - details, - } + RevlogEntry::new(self.1.clone(), path, details) }); - Ok(Async::Ready(v)) + match v { + Some(v) => v.map(|x| Async::Ready(Some(x))), + None => Ok(Async::Ready(None)), + } } } @@ -227,15 +226,13 @@ impl Manifest for RevlogManifest { path: &MPath, ) -> BoxFuture + Sync>>, Self::Error> { let repo = self.repo.as_ref().expect("missing repo").clone(); - let res = RevlogManifest::lookup(self, path).map(|details| { - RevlogEntry { - repo: repo, - path: path.clone(), - details: *details, - } - }); + let res = RevlogManifest::lookup(self, path) + .map(|details| RevlogEntry::new(repo, path.clone(), *details)); - Ok(res.map(|e| e.boxed())).into_future().boxify() + match res { + Some(v) => v.map(|e| Some(e.boxed())).into_future().boxify(), + None => Ok(None).into_future().boxify(), + } } fn list(&self) -> BoxStream + Sync>, Self::Error> { @@ -251,6 +248,22 @@ impl Manifest for RevlogManifest { } } +impl RevlogEntry { + fn new(repo: RevlogRepo, path: MPath, details: Details) -> Result { + let path = match details.flag() { + Type::Tree => RepoPath::dir(path) + .chain_err(|| ErrorKind::Path("error while creating RepoPath".into()))?, + _ => RepoPath::file(path) + .chain_err(|| ErrorKind::Path("error while creating RepoPath".into()))?, + }; + Ok(RevlogEntry { + repo, + path, + details, + }) + } +} + impl Entry for RevlogEntry { type Error = Error; @@ -259,10 +272,7 @@ impl Entry for RevlogEntry { } fn get_parents(&self) -> BoxFuture { - let revlog = match self.get_type() { - Type::Tree => self.repo.get_tree_revlog(self.get_path()), - _ => self.repo.get_file_revlog(self.get_path()), - }; + let revlog = self.repo.get_path_revlog(self.get_path()); revlog .and_then(|revlog| revlog.get_rev_by_nodeid(self.get_hash())) .map(|node| *node.parents()) @@ -271,14 +281,7 @@ impl Entry for RevlogEntry { } fn get_raw_content(&self) -> BoxFuture>, Self::Error> { - let revlog = { - if self.get_type() == Type::Tree { - self.repo.get_tree_revlog(self.get_path()) - } else { - self.repo.get_file_revlog(self.get_path()) - } - }; - + let revlog = self.repo.get_path_revlog(self.get_path()); revlog .and_then(|revlog| revlog.get_rev_by_nodeid(self.get_hash())) .map(|node| node.as_blob().clone()) @@ -297,10 +300,7 @@ impl Entry for RevlogEntry { } fn get_content(&self) -> BoxFuture, Self::Error> { - let revlog = match self.get_type() { - Type::Tree => self.repo.get_tree_revlog(self.get_path()), - _ => self.repo.get_file_revlog(self.get_path()), - }; + let revlog = self.repo.get_path_revlog(self.get_path()); revlog .and_then(|revlog| revlog.get_rev_by_nodeid(self.get_hash())) @@ -320,7 +320,9 @@ impl Entry for RevlogEntry { let revlog_manifest = RevlogManifest::parse_with_prefix( Some(self.repo.clone()), &data, - self.get_path(), + self.get_path() + .mpath() + .expect("trees should always have a path"), )?; Ok(Content::Tree(Box::new(revlog_manifest))) } @@ -353,9 +355,15 @@ impl Entry for RevlogEntry { self.details.nodeid() } - fn get_path(&self) -> &MPath { + fn get_path(&self) -> &RepoPath { &self.path } + + fn get_mpath(&self) -> &MPath { + self.path + .mpath() + .expect("entries should always have an associated path") + } } fn strip_file_metadata(blob: &Blob>) -> Blob> { @@ -363,7 +371,7 @@ fn strip_file_metadata(blob: &Blob>) -> Blob> { &Blob::Dirty(ref bytes) => { let (_, off) = file::File::extract_meta(bytes); Blob::from(&bytes[off..]) - }, + } _ => blob.clone(), } } diff --git a/mercurial/src/revlogrepo.rs b/mercurial/src/revlogrepo.rs index 1a6fd632b3..b4b9e5d266 100644 --- a/mercurial/src/revlogrepo.rs +++ b/mercurial/src/revlogrepo.rs @@ -21,7 +21,7 @@ use futures_ext::{BoxFuture, BoxStream, FutureExt, StreamExt}; use asyncmemo::{Asyncmemo, Filler}; use bookmarks::{Bookmarks, BoxedBookmarks}; use mercurial_types::{fsencode, BlobNode, Changeset, MPath, MPathElement, Manifest, NodeHash, - Repo, NULL_HASH}; + Repo, RepoPath, NULL_HASH}; use stockbookmarks::StockBookmarks; use storage_types::Version; @@ -232,6 +232,15 @@ impl RevlogRepo { &self.requirements } + pub fn get_path_revlog(&self, path: &RepoPath) -> Result { + match *path { + // TODO avoid creating a new MPath here + RepoPath::RootPath => self.get_tree_revlog(&MPath::empty()), + RepoPath::DirectoryPath(ref path) => self.get_tree_revlog(path), + RepoPath::FilePath(ref path) => self.get_file_revlog(path), + } + } + pub fn get_tree_revlog(&self, path: &MPath) -> Result { { let inner = self.inner.read().expect("poisoned lock"); diff --git a/vfs/src/manifest_vfs.rs b/vfs/src/manifest_vfs.rs index 286d90e3d7..27c52bf22b 100644 --- a/vfs/src/manifest_vfs.rs +++ b/vfs/src/manifest_vfs.rs @@ -42,7 +42,7 @@ where .and_then(|entries| { let mut path_tree = Tree::new(); for (entry_idx, entry) in entries.iter().enumerate() { - let mut path = entry.get_path().clone().into_iter(); + let mut path = entry.get_mpath().clone().into_iter(); let leaf_key = path.next_back().ok_or_else(|| { ErrorKind::ManifestInvalidPath("the path shouldn't be empty".into()) })?;