facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-11 17:27:53 +03:00
Code Issues Projects Releases Wiki Activity

manifest: add bfs diff to bindings crate

Browse Source 
Summary: Add support for calling the new BFS diff implementation from Python. This diff adds the appropriate glue code to the bindings crate and adds a config option (`treemanifest.bfsdiff`) to enable the new functionality.

Reviewed By: xavierd

Differential Revision: D17334739

fbshipit-source-id: 24aac21910e74a42d625c93bed7fa3aa08e167c0
This commit is contained in:
Arun Kulshreshtha 2019-09-12 18:26:17 -07:00 committed by Facebook Github Bot
parent db1873cb7b
commit 980bcdd0f8
4 changed files with 79 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
edenscm/hgext/treemanifest/__init__.py
View File

@ -143,6 +143,14 @@ implementation rather than the C++ one.
[treemanifest]
rustmanifest = True
`treemanifest.bfsdiff` causes the Rust implementation to use a breadth-first
traversal during the diff operation rather than the usual depth-first traversal.
This setting has no effect if treemanifest.rustmanifest is not enabeld.
::
[treemanifest]
bfsdiff = True
"""
from __future__ import absolute_import
@ -1031,16 +1039,22 @@ def _userustmanifest(manifestlog):
def _buildtree(manifestlog, node=None):
# this code seems to belong in manifestlog but I have no idea how
# manifestlog objects work
if _userustmanifest(manifestlog):
manifestbuilder = rustmanifest.treemanifest
else:
manifestbuilder = cstore.treemanifest
store = manifestlog.datastore
if node is not None and node != nullid:
tree = manifestbuilder(store, node)
if _userustmanifest(manifestlog):
bfsdiff = manifestlog.ui.configbool("treemanifest", "bfsdiff", False)
kwargs = {"bfsdiff": bfsdiff}
if node is not None and node != nullid:
kwargs["node"] = node
return rustmanifest.treemanifest(store, **kwargs)
else:
tree = manifestbuilder(store)
return tree
# XXX: The C++ treemanifest constructor does not support
# keyword arguments, so we need to manually call it with
# the correct number of arguments rather than relying on
# argument unpacking.
if node is not None and node != nullid:
return cstore.treemanifest(store, node)
else:
return cstore.treemanifest(store)
def _finalize(manifestlog, tree, p1node=None, p2node=None):

14
edenscmnative/bindings/modules/pymanifest/src/lib.rs
View File

@ -71,11 +71,13 @@ pub fn init_module(py: Python, package: &str) -> PyResult<PyModule> {
py_class!(class treemanifest |py| {
data underlying: RefCell<manifest::Tree>;
data use_bfs_diff: bool;
def __new__(
_cls,
store: PyObject,
node: Option<&PyBytes> = None
node: Option<&PyBytes> = None,
bfsdiff: bool = false
) -> PyResult<treemanifest> {
let store = PythonDataStore::new(store);
let manifest_store = Arc::new(ManifestStore::new(store));
@ -83,13 +85,14 @@ py_class!(class treemanifest |py| {
None => manifest::Tree::ephemeral(manifest_store),
Some(value) => manifest::Tree::durable(manifest_store, pybytes_to_node(py, value)?),
};
treemanifest::create_instance(py, RefCell::new(underlying))
treemanifest::create_instance(py, RefCell::new(underlying), bfsdiff)
}
// Returns a new instance of treemanifest that contains the same data as the base.
def copy(&self) -> PyResult<treemanifest> {
let tree = self.underlying(py);
treemanifest::create_instance(py, tree.clone())
let use_bfs_diff = *self.use_bfs_diff(py);
treemanifest::create_instance(py, tree.clone(), use_bfs_diff)
}
// Returns (node, flag) for a given `path` in the manifest.
@ -227,7 +230,8 @@ py_class!(class treemanifest |py| {
None => Box::new(AlwaysMatcher::new()),
Some(pyobj) => Box::new(PythonMatcher::new(py, pyobj)),
};
for entry in manifest::diff(&this_tree, &other_tree, &matcher) {
for entry in manifest::diff(&this_tree, &other_tree, &matcher, *self.use_bfs_diff(py)) {
let entry = entry.map_pyerr::<exc::RuntimeError>(py)?;
let path = path_to_pybytes(py, &entry.path);
let diff_left = convert_side_diff(py, entry.diff_type.left());
@ -250,7 +254,7 @@ py_class!(class treemanifest |py| {
None => Box::new(AlwaysMatcher::new()),
Some(pyobj) => Box::new(PythonMatcher::new(py, pyobj)),
};
for entry in manifest::diff(&this_tree, &other_tree, &matcher) {
for entry in manifest::diff(&this_tree, &other_tree, &matcher, *self.use_bfs_diff(py)) {
let entry = entry.map_pyerr::<exc::RuntimeError>(py)?;
match entry.diff_type {
DiffType::LeftOnly(_) => {

4
lib/manifest/src/lib.rs
View File

@ -68,4 +68,6 @@ pub enum FsNode {
mod file;
pub mod tree;
pub use crate::file::{FileMetadata, FileType};
pub use crate::tree::{compat_subtree_diff, diff, BfsDiff, DiffEntry, DiffType, Tree, TreeStore};
pub use crate::tree::{
compat_subtree_diff, diff, BfsDiff, Diff, DiffEntry, DiffType, Tree, TreeStore,
};

71
lib/manifest/src/tree/mod.rs
View File

@ -504,7 +504,22 @@ where
}
}
/// Returns an iterator over all the differences between two [`Tree`]s. Keeping in mind that
/// Wrapper around `Diff` and `BfsDiff`, allowing the diff algorithm to be dynamically
/// chosen via user configuration.
pub fn diff<'a, M: Matcher>(
left: &'a Tree,
right: &'a Tree,
matcher: &'a M,
bfs_diff: bool,
) -> Box<dyn Iterator<Item = Fallible<DiffEntry>> + 'a> {
if bfs_diff {
Box::new(BfsDiff::new(left, right, matcher))
} else {
Box::new(Diff::new(left, right, matcher))
}
}
/// An iterator over all the differences between two [`Tree`]s. Keeping in mind that
/// manifests operate over files, the difference space is limited to three cases described by
/// [`DiffType`]:
/// * a file may be present only in the left tree manifest
@ -515,18 +530,6 @@ where
/// directory in the `right` tree manifest, the differences returned will be:
/// 1. DiffEntry("foo", LeftOnly(_))
/// 2. DiffEntry(file, RightOnly(_)) for all `file`s under the "foo" directory
pub fn diff<'a, M>(left: &'a Tree, right: &'a Tree, matcher: &'a M) -> Diff<'a, M> {
Diff {
left: left.root_cursor(),
step_left: false,
right: right.root_cursor(),
step_right: false,
matcher,
}
}
/// An iterator over the differences between two tree manifests.
/// See [`diff()`].
pub struct Diff<'a, M> {
left: Cursor<'a>,
step_left: bool,
@ -535,6 +538,18 @@ pub struct Diff<'a, M> {
matcher: &'a M,
}
impl<'a, M> Diff<'a, M> {
pub fn new(left: &'a Tree, right: &'a Tree, matcher: &'a M) -> Self {
Self {
left: left.root_cursor(),
step_left: false,
right: right.root_cursor(),
step_right: false,
matcher,
}
}
}
/// Represents a file that is different between two tree manifests.
#[derive(Clone, Debug, Ord, PartialOrd, Eq, PartialEq, Hash)]
pub struct DiffEntry {
@ -1351,7 +1366,7 @@ mod tests {
right.insert(repo_path_buf("a3/b1"), meta("40")).unwrap();
assert_eq!(
diff(&left, &right, &AlwaysMatcher::new())
Diff::new(&left, &right, &AlwaysMatcher::new())
.collect::<Fallible<Vec<_>>>()
.unwrap(),
vec!(
@ -1368,7 +1383,7 @@ mod tests {
right.flush().unwrap();
assert_eq!(
diff(&left, &right, &AlwaysMatcher::new())
Diff::new(&left, &right, &AlwaysMatcher::new())
.collect::<Fallible<Vec<_>>>()
.unwrap(),
vec!(
@ -1386,7 +1401,9 @@ mod tests {
left.insert(repo_path_buf("a1/b2"), meta("40")).unwrap();
left.insert(repo_path_buf("a2/b2/c2"), meta("30")).unwrap();
assert!(diff(&left, &right, &AlwaysMatcher::new()).next().is_none());
assert!(Diff::new(&left, &right, &AlwaysMatcher::new())
.next()
.is_none());
}
#[test]
@ -1394,10 +1411,12 @@ mod tests {
// Leaving the store empty intentionaly so that we get a panic if anything is read from it.
let left = Tree::durable(Arc::new(TestStore::new()), node("10"));
let right = Tree::durable(Arc::new(TestStore::new()), node("10"));
assert!(diff(&left, &right, &AlwaysMatcher::new()).next().is_none());
assert!(Diff::new(&left, &right, &AlwaysMatcher::new())
.next()
.is_none());
let right = Tree::durable(Arc::new(TestStore::new()), node("20"));
assert!(diff(&left, &right, &AlwaysMatcher::new())
assert!(Diff::new(&left, &right, &AlwaysMatcher::new())
.next()
.unwrap()
.is_err());
@ -1414,7 +1433,7 @@ mod tests {
right.insert(repo_path_buf("a2/b2"), meta("40")).unwrap();
assert_eq!(
diff(&left, &right, &AlwaysMatcher::new())
Diff::new(&left, &right, &AlwaysMatcher::new())
.collect::<Fallible<Vec<_>>>()
.unwrap(),
vec!(
@ -1438,7 +1457,7 @@ mod tests {
right.insert(repo_path_buf("a2/b2/c2"), meta("30")).unwrap();
assert_eq!(
diff(&left, &right, &AlwaysMatcher::new())
Diff::new(&left, &right, &AlwaysMatcher::new())
.collect::<Fallible<Vec<_>>>()
.unwrap(),
vec!(
@ -1455,7 +1474,7 @@ mod tests {
right.flush().unwrap();
assert_eq!(
diff(&left, &right, &AlwaysMatcher::new())
Diff::new(&left, &right, &AlwaysMatcher::new())
.collect::<Fallible<Vec<_>>>()
.unwrap(),
vec!(
@ -1483,7 +1502,7 @@ mod tests {
right.insert(repo_path_buf("a3/b1"), meta("40")).unwrap();
assert_eq!(
diff(&left, &right, &TreeMatcher::from_rules(["a1/b1"].iter()))
Diff::new(&left, &right, &TreeMatcher::from_rules(["a1/b1"].iter()))
.collect::<Fallible<Vec<_>>>()
.unwrap(),
vec!(DiffEntry::new(
@ -1492,7 +1511,7 @@ mod tests {
),)
);
assert_eq!(
diff(&left, &right, &TreeMatcher::from_rules(["a1/b2"].iter()))
Diff::new(&left, &right, &TreeMatcher::from_rules(["a1/b2"].iter()))
.collect::<Fallible<Vec<_>>>()
.unwrap(),
vec!(DiffEntry::new(
@ -1501,7 +1520,7 @@ mod tests {
),)
);
assert_eq!(
diff(&left, &right, &TreeMatcher::from_rules(["a2/b2"].iter()))
Diff::new(&left, &right, &TreeMatcher::from_rules(["a2/b2"].iter()))
.collect::<Fallible<Vec<_>>>()
.unwrap(),
vec!(DiffEntry::new(
@ -1510,7 +1529,7 @@ mod tests {
),)
);
assert_eq!(
diff(&left, &right, &TreeMatcher::from_rules(["*/b2"].iter()))
Diff::new(&left, &right, &TreeMatcher::from_rules(["*/b2"].iter()))
.collect::<Fallible<Vec<_>>>()
.unwrap(),
vec!(
@ -1522,7 +1541,7 @@ mod tests {
)
);
assert!(
diff(&left, &right, &TreeMatcher::from_rules(["a3/**"].iter()))
Diff::new(&left, &right, &TreeMatcher::from_rules(["a3/**"].iter()))
.next()
.is_none()
);