use RepoPath instead of MPath in a few places

Summary:
`RepoPath` represents any absolute path -- root, directory or file. There's a
lot of code that manually switches between directory and file entries --
abstract all of that away.

Reviewed By: farnz

Differential Revision: D6201383

fbshipit-source-id: 0047023a67a5484ddbdd00bb57bca3bfb7d4dd3f
This commit is contained in:
Siddharth Agarwal 2017-10-31 14:16:08 -07:00 committed by Facebook Github Bot
parent 1e7f9e7162
commit c8d6e7f954
11 changed files with 126 additions and 71 deletions

View File

@ -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<B> {
blobstore: B,
path: MPath, // XXX full path? Parent reference?
path: RepoPath,
nodeid: NodeHash,
ty: Type,
}
@ -60,13 +60,17 @@ impl<B> BlobEntry<B>
where
B: Blobstore<Key = String> + 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<Self> {
let path = match ty {
Type::Tree => RepoPath::dir(path)?,
_ => RepoPath::file(path)?,
};
Ok(Self {
blobstore,
path,
nodeid,
ty,
}
})
}
fn get_node(&self) -> BoxFuture<RawNodeBlob, Error> {
@ -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")
}
}

View File

@ -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<Option<Box<Entry<Error = Self::Error> + Sync>>, Self::Error> {
let res = self.files
.get(path)
.map({
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());
});
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<Box<Entry<Error = Self::Error> + 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()
}
}

View File

@ -81,12 +81,7 @@ pub(crate) fn get_entry_stream(
revlog_repo: RevlogRepo,
cs_rev: RevIdx,
) -> Box<Stream<Item = Box<Entry<Error = mercurial::Error>>, 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| {

View File

@ -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,
}

View File

@ -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<E> {
}
impl<E> MockManifest<E> {
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<E> {
path: MPath,
path: RepoPath,
content_factory: ContentFactory<E>,
phantom: PhantomData<E>,
}
@ -91,7 +92,7 @@ impl<E> Clone for MockEntry<E> {
}
impl<E> MockEntry<E> {
fn new(path: MPath, content_factory: ContentFactory<E>) -> Self {
fn new(path: RepoPath, content_factory: ContentFactory<E>) -> 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")
}
}

View File

@ -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<Content<Self::Error>, Self::Error>;
fn get_size(&self) -> BoxFuture<Option<usize>, 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<Entry<Error = Self::Error> + 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<E> Entry for Box<Entry<Error = E> + 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 {

View File

@ -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<u8> {
@ -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(

View File

@ -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)

View File

@ -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<Option<Self::Item>, 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<Option<Box<Entry<Error = Self::Error> + 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<Box<Entry<Error = Self::Error> + Sync>, Self::Error> {
@ -251,6 +248,22 @@ impl Manifest for RevlogManifest {
}
}
impl RevlogEntry {
fn new(repo: RevlogRepo, path: MPath, details: Details) -> Result<Self> {
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<Parents, 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()))
.map(|node| *node.parents())
@ -271,14 +281,7 @@ impl Entry for RevlogEntry {
}
fn get_raw_content(&self) -> BoxFuture<Blob<Vec<u8>>, 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<Content<Self::Error>, 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<Vec<u8>>) -> Blob<Vec<u8>> {
@ -363,7 +371,7 @@ fn strip_file_metadata(blob: &Blob<Vec<u8>>) -> Blob<Vec<u8>> {
&Blob::Dirty(ref bytes) => {
let (_, off) = file::File::extract_meta(bytes);
Blob::from(&bytes[off..])
},
}
_ => blob.clone(),
}
}

View File

@ -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<Revlog> {
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<Revlog> {
{
let inner = self.inner.read().expect("poisoned lock");

View File

@ -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())
})?;