ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

tree: support filtering diff by matcher

Browse source 
This change teaches `Tree::diff()` to filter by a matcher. It only
filters the result so far; it does not restrict the tree walk to what
`Matcher::visit()` says is necessary yet. It also doesn't teach the
CLI to create a matcher and pass it in.
This commit is contained in:
Martin von Zweigbergk 2021-06-05 14:29:40 -07:00
parent b593e552b8
commit dd4c47f373
8 changed files with 252 additions and 75 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
lib/src/matchers.rs
View file

@ -19,21 +19,21 @@ use std::collections::{HashMap, HashSet};
use crate::repo_path::{RepoPath, RepoPathComponent};
#[derive(PartialEq, Eq, Debug)]
pub struct Visit<'a> {
dirs: VisitDirs<'a>,
files: VisitFiles<'a>,
pub struct Visit<'matcher> {
pub dirs: VisitDirs<'matcher>,
pub files: VisitFiles<'matcher>,
}
#[derive(PartialEq, Eq, Debug)]
pub enum VisitDirs<'a> {
pub enum VisitDirs<'matcher> {
All,
Set(&'a HashSet<RepoPathComponent>),
Set(&'matcher HashSet<RepoPathComponent>),
}
#[derive(PartialEq, Eq, Debug)]
pub enum VisitFiles<'a> {
pub enum VisitFiles<'matcher> {
All,
Set(&'a HashSet<RepoPathComponent>),
Set(&'matcher HashSet<RepoPathComponent>),
}
pub trait Matcher {
@ -64,7 +64,7 @@ pub struct FilesMatcher {
}
impl FilesMatcher {
fn new(files: HashSet<RepoPath>) -> Self {
pub fn new(files: HashSet<RepoPath>) -> Self {
let mut dirs = Dirs::new();
for f in &files {
dirs.add_file(f);

137
lib/src/tree.rs
View file

@ -20,6 +20,7 @@ use std::pin::Pin;
use std::sync::Arc;
use crate::files::MergeResult;
use crate::matchers::{EverythingMatcher, Matcher};
use crate::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin};
use crate::store::{
Conflict, ConflictId, ConflictPart, StoreError, TreeEntriesNonRecursiveIter, TreeEntry, TreeId,
@ -164,15 +165,19 @@ impl Tree {
}
}
pub fn diff<'a>(&'a self, other: &'a Tree) -> TreeDiffIterator {
recursive_tree_diff(self.clone(), other.clone())
pub fn diff<'matcher>(
&self,
other: &Tree,
matcher: &'matcher dyn Matcher,
) -> TreeDiffIterator<'matcher> {
recursive_tree_diff(self.clone(), other.clone(), matcher)
}
pub fn diff_summary(&self, other: &Tree) -> DiffSummary {
pub fn diff_summary(&self, other: &Tree, matcher: &dyn Matcher) -> DiffSummary {
let mut modified = vec![];
let mut added = vec![];
let mut removed = vec![];
for (file, diff) in self.diff(other) {
for (file, diff) in self.diff(other, matcher) {
match diff {
Diff::Modified(_, _) => modified.push(file.clone()),
Diff::Added(_) => added.push(file.clone()),
@ -269,24 +274,30 @@ impl<T> Diff<T> {
}
}
struct TreeEntryDiffIterator<'a> {
it1: Peekable<TreeEntriesNonRecursiveIter<'a>>,
it2: Peekable<TreeEntriesNonRecursiveIter<'a>>,
struct TreeEntryDiffIterator<'trees, 'matcher> {
it1: Peekable<TreeEntriesNonRecursiveIter<'trees>>,
it2: Peekable<TreeEntriesNonRecursiveIter<'trees>>,
// TODO: Restrict walk according to Matcher::visit()
_matcher: &'matcher dyn Matcher,
}
impl<'a> TreeEntryDiffIterator<'a> {
fn new(tree1: &'a Tree, tree2: &'a Tree) -> Self {
impl<'trees, 'matcher> TreeEntryDiffIterator<'trees, 'matcher> {
fn new(tree1: &'trees Tree, tree2: &'trees Tree, matcher: &'matcher dyn Matcher) -> Self {
let it1 = tree1.entries_non_recursive().peekable();
let it2 = tree2.entries_non_recursive().peekable();
TreeEntryDiffIterator { it1, it2 }
TreeEntryDiffIterator {
it1,
it2,
_matcher: matcher,
}
}
}
impl<'a> Iterator for TreeEntryDiffIterator<'a> {
impl<'trees, 'matcher> Iterator for TreeEntryDiffIterator<'trees, 'matcher> {
type Item = (
RepoPathComponent,
Option<&'a TreeValue>,
Option<&'a TreeValue>,
Option<&'trees TreeValue>,
Option<&'trees TreeValue>,
);
fn next(&mut self) -> Option<Self::Item> {
@ -339,39 +350,54 @@ impl<'a> Iterator for TreeEntryDiffIterator<'a> {
}
}
fn diff_entries<'a>(tree1: &'a Tree, tree2: &'a Tree) -> TreeEntryDiffIterator<'a> {
TreeEntryDiffIterator::new(tree1, tree2)
fn diff_entries<'trees, 'matcher>(
tree1: &'trees Tree,
tree2: &'trees Tree,
matcher: &'matcher dyn Matcher,
) -> TreeEntryDiffIterator<'trees, 'matcher> {
// TODO: make TreeEntryDiffIterator an enum with one variant that iterates over
// the tree entries and filters by the matcher (i.e. what
// TreeEntryDiffIterator does now) and another variant that iterates over
// what the matcher says to visit
TreeEntryDiffIterator::new(tree1, tree2, matcher)
}
pub fn recursive_tree_diff(root1: Tree, root2: Tree) -> TreeDiffIterator {
TreeDiffIterator::new(RepoPath::root(), root1, root2)
pub fn recursive_tree_diff(root1: Tree, root2: Tree, matcher: &dyn Matcher) -> TreeDiffIterator {
TreeDiffIterator::new(RepoPath::root(), root1, root2, matcher)
}
pub struct TreeDiffIterator {
pub struct TreeDiffIterator<'matcher> {
dir: RepoPath,
tree1: Pin<Box<Tree>>,
tree2: Pin<Box<Tree>>,
matcher: &'matcher dyn Matcher,
// Iterator over the diffs between tree1 and tree2
entry_iterator: TreeEntryDiffIterator<'static>,
entry_iterator: TreeEntryDiffIterator<'static, 'matcher>,
// This is used for making sure that when a directory gets replaced by a file, we
// yield the value for the addition of the file after we yield the values
// for removing files in the directory.
added_file: Option<(RepoPath, TreeValue)>,
// Iterator over the diffs of a subdirectory, if we're currently visiting one.
subdir_iterator: Option<Box<TreeDiffIterator>>,
subdir_iterator: Option<Box<TreeDiffIterator<'matcher>>>,
}
impl TreeDiffIterator {
fn new(dir: RepoPath, tree1: Tree, tree2: Tree) -> TreeDiffIterator {
impl<'matcher> TreeDiffIterator<'matcher> {
fn new(
dir: RepoPath,
tree1: Tree,
tree2: Tree,
matcher: &'matcher dyn Matcher,
) -> TreeDiffIterator {
let tree1 = Box::pin(tree1);
let tree2 = Box::pin(tree2);
let root_entry_iterator: TreeEntryDiffIterator = diff_entries(&tree1, &tree2);
let root_entry_iterator: TreeEntryDiffIterator<'static> =
let root_entry_iterator: TreeEntryDiffIterator = diff_entries(&tree1, &tree2, matcher);
let root_entry_iterator: TreeEntryDiffIterator<'static, 'matcher> =
unsafe { std::mem::transmute(root_entry_iterator) };
Self {
dir,
tree1,
tree2,
matcher,
entry_iterator: root_entry_iterator,
added_file: None,
subdir_iterator: None,
@ -379,7 +405,7 @@ impl TreeDiffIterator {
}
}
impl Iterator for TreeDiffIterator {
impl Iterator for TreeDiffIterator<'_> {
type Item = (RepoPath, Diff<TreeValue>);
fn next(&mut self) -> Option<Self::Item> {
@ -400,17 +426,17 @@ impl Iterator for TreeDiffIterator {
let tree_before = matches!(before, Some(TreeValue::Tree(_)));
let tree_after = matches!(after, Some(TreeValue::Tree(_)));
if tree_before || tree_after {
let subdir = RepoPathComponent::from(name.as_str());
let subdir_path = self.dir.join(&subdir);
let subdir = &name;
let subdir_path = self.dir.join(subdir);
let before_tree = match before {
Some(TreeValue::Tree(id_before)) => {
self.tree1.known_sub_tree(&subdir, &id_before)
self.tree1.known_sub_tree(subdir, &id_before)
}
_ => Tree::null(self.tree1.store().clone(), subdir_path.clone()),
};
let after_tree = match after {
Some(TreeValue::Tree(id_after)) => {
self.tree2.known_sub_tree(&subdir, &id_after)
self.tree2.known_sub_tree(subdir, &id_after)
}
_ => Tree::null(self.tree2.store().clone(), subdir_path.clone()),
};
@ -418,33 +444,36 @@ impl Iterator for TreeDiffIterator {
subdir_path,
before_tree,
after_tree,
self.matcher,
)));
}
let file_path = self.dir.join(&RepoPathComponent::from(name.as_str()));
if !tree_before && tree_after {
if let Some(file_before) = before {
return Some((file_path, Diff::Removed(file_before.clone())));
}
} else if tree_before && !tree_after {
if let Some(file_after) = after {
self.added_file = Some((file_path, file_after.clone()));
}
} else if !tree_before && !tree_after {
match (before, after) {
(Some(file_before), Some(file_after)) => {
return Some((
file_path,
Diff::Modified(file_before.clone(), file_after.clone()),
));
}
(None, Some(file_after)) => {
return Some((file_path, Diff::Added(file_after.clone())));
}
(Some(file_before), None) => {
let file_path = self.dir.join(&name);
if self.matcher.matches(&file_path) {
if !tree_before && tree_after {
if let Some(file_before) = before {
return Some((file_path, Diff::Removed(file_before.clone())));
}
(None, None) => {
panic!("unexpected diff")
} else if tree_before && !tree_after {
if let Some(file_after) = after {
self.added_file = Some((file_path, file_after.clone()));
}
} else if !tree_before && !tree_after {
match (before, after) {
(Some(file_before), Some(file_after)) => {
return Some((
file_path,
Diff::Modified(file_before.clone(), file_after.clone()),
));
}
(None, Some(file_after)) => {
return Some((file_path, Diff::Added(file_after.clone())));
}
(Some(file_before), None) => {
return Some((file_path, Diff::Removed(file_before.clone())));
}
(None, None) => {
panic!("unexpected diff")
}
}
}
}
@ -475,7 +504,9 @@ pub fn merge_trees(
// Start with a tree identical to side 1 and modify based on changes from base
// to side 2.
let mut new_tree = side1_tree.data().clone();
for (basename, maybe_base, maybe_side2) in diff_entries(base_tree, side2_tree) {
for (basename, maybe_base, maybe_side2) in
diff_entries(base_tree, side2_tree, &EverythingMatcher)
{
let maybe_side1 = side1_tree.value(&basename);
if maybe_side1 == maybe_base {
// side 1 is unchanged: use the value from side 2

3
lib/src/working_copy.rs
View file

@ -37,6 +37,7 @@ use crate::commit::Commit;
use crate::commit_builder::CommitBuilder;
use crate::gitignore::GitIgnoreFile;
use crate::lock::FileLock;
use crate::matchers::EverythingMatcher;
use crate::repo::ReadonlyRepo;
use crate::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin};
use crate::settings::UserSettings;
@ -501,7 +502,7 @@ impl TreeState {
removed_files: 0,
};
for (path, diff) in old_tree.diff(&new_tree) {
for (path, diff) in old_tree.diff(&new_tree, &EverythingMatcher) {
let disk_path = path.to_fs_path(&self.working_copy_path);
// TODO: Check that the file has not changed before overwriting/removing it.

12
lib/tests/test_commit_builder.rs
View file

@ -13,6 +13,7 @@
// limitations under the License.
use jujutsu_lib::commit_builder::CommitBuilder;
use jujutsu_lib::matchers::EverythingMatcher;
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::settings::UserSettings;
use jujutsu_lib::testutils;
@ -49,7 +50,10 @@ fn test_initial(use_git: bool) {
assert_eq!(commit.committer().name, settings.user_name());
assert_eq!(commit.committer().email, settings.user_email());
assert_eq!(
store.root_commit().tree().diff_summary(&commit.tree()),
store
.root_commit()
.tree()
.diff_summary(&commit.tree(), &EverythingMatcher),
DiffSummary {
modified: vec![],
added: vec![dir_file_path, root_file_path],
@ -119,7 +123,7 @@ fn test_rewrite(use_git: bool) {
store
.root_commit()
.tree()
.diff_summary(&rewritten_commit.tree()),
.diff_summary(&rewritten_commit.tree(), &EverythingMatcher),
DiffSummary {
modified: vec![],
added: vec![dir_file_path.clone(), root_file_path],
@ -127,7 +131,9 @@ fn test_rewrite(use_git: bool) {
}
);
assert_eq!(
initial_commit.tree().diff_summary(&rewritten_commit.tree()),
initial_commit
.tree()
.diff_summary(&rewritten_commit.tree(), &EverythingMatcher),
DiffSummary {
modified: vec![dir_file_path],
added: vec![],

142
lib/tests/test_diff_summary.rs
View file

@ -12,9 +12,11 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use jujutsu_lib::matchers::{EverythingMatcher, FilesMatcher};
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::testutils;
use jujutsu_lib::tree::DiffSummary;
use maplit::hashset;
use test_case::test_case;
#[test_case(false ; "local store")]
@ -47,7 +49,7 @@ fn test_types(use_git: bool) {
);
assert_eq!(
tree1.diff_summary(&tree2),
tree1.diff_summary(&tree2, &EverythingMatcher),
DiffSummary {
modified: vec![modified_path],
added: vec![added_path],
@ -69,7 +71,7 @@ fn test_tree_file_transition(use_git: bool) {
let tree2 = testutils::create_tree(&repo, &[(&dir_path, "contents")]);
assert_eq!(
tree1.diff_summary(&tree2),
tree1.diff_summary(&tree2, &EverythingMatcher),
DiffSummary {
modified: vec![],
added: vec![dir_path.clone()],
@ -77,7 +79,7 @@ fn test_tree_file_transition(use_git: bool) {
}
);
assert_eq!(
tree2.diff_summary(&tree1),
tree2.diff_summary(&tree1, &EverythingMatcher),
DiffSummary {
modified: vec![],
added: vec![dir_file_path],
@ -127,7 +129,7 @@ fn test_sorting(use_git: bool) {
);
assert_eq!(
tree1.diff_summary(&tree2),
tree1.diff_summary(&tree2, &EverythingMatcher),
DiffSummary {
modified: vec![a_path.clone(), f_a_path.clone(), f_f_a_path.clone()],
added: vec![
@ -142,7 +144,7 @@ fn test_sorting(use_git: bool) {
}
);
assert_eq!(
tree2.diff_summary(&tree1),
tree2.diff_summary(&tree1, &EverythingMatcher),
DiffSummary {
modified: vec![a_path, f_a_path, f_f_a_path],
added: vec![],
@ -150,3 +152,133 @@ fn test_sorting(use_git: bool) {
}
);
}
#[test_case(false ; "local store")]
#[test_case(true ; "git store")]
fn test_matcher_dir_file_transition(use_git: bool) {
let settings = testutils::user_settings();
let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
let a_path = RepoPath::from_internal_string("a");
let a_a_path = RepoPath::from_internal_string("a/a");
let tree1 = testutils::create_tree(&repo, &[(&a_path, "before")]);
let tree2 = testutils::create_tree(&repo, &[(&a_a_path, "after")]);
let matcher = FilesMatcher::new(hashset! {a_path.clone()});
assert_eq!(
tree1.diff_summary(&tree2, &matcher),
DiffSummary {
modified: vec![],
added: vec![],
removed: vec![a_path.clone()]
}
);
assert_eq!(
tree2.diff_summary(&tree1, &matcher),
DiffSummary {
modified: vec![],
added: vec![a_path.clone()],
removed: vec![]
}
);
let matcher = FilesMatcher::new(hashset! {a_a_path.clone()});
assert_eq!(
tree1.diff_summary(&tree2, &matcher),
DiffSummary {
modified: vec![],
added: vec![a_a_path.clone()],
removed: vec![]
}
);
assert_eq!(
tree2.diff_summary(&tree1, &matcher),
DiffSummary {
modified: vec![],
added: vec![],
removed: vec![a_a_path.clone()]
}
);
let matcher = FilesMatcher::new(hashset! {a_path.clone(), a_a_path.clone()});
assert_eq!(
tree1.diff_summary(&tree2, &matcher),
DiffSummary {
modified: vec![],
added: vec![a_a_path.clone()],
removed: vec![a_path.clone()]
}
);
assert_eq!(
tree2.diff_summary(&tree1, &matcher),
DiffSummary {
modified: vec![],
added: vec![a_path.clone()],
removed: vec![a_a_path.clone()]
}
);
}
#[test_case(false ; "local store")]
#[test_case(true ; "git store")]
fn test_matcher_normal_cases(use_git: bool) {
let settings = testutils::user_settings();
let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
let a_path = RepoPath::from_internal_string("a");
let dir1_a_path = RepoPath::from_internal_string("dir1/a");
let dir2_b_path = RepoPath::from_internal_string("dir2/b");
let z_path = RepoPath::from_internal_string("z");
let tree1 = testutils::create_tree(&repo, &[(&a_path, "before"), (&dir1_a_path, "before")]);
// File "a" gets modified
// File "dir1/a" gets modified
// File "dir2/b" gets created
// File "z" gets created
let tree2 = testutils::create_tree(
&repo,
&[
(&a_path, "after"),
(&dir1_a_path, "after"),
(&dir2_b_path, "after"),
(&z_path, "after"),
],
);
let matcher = FilesMatcher::new(hashset! {a_path.clone(), z_path.clone()});
assert_eq!(
tree1.diff_summary(&tree2, &matcher),
DiffSummary {
modified: vec![a_path.clone()],
added: vec![z_path.clone()],
removed: vec![]
}
);
assert_eq!(
tree2.diff_summary(&tree1, &matcher),
DiffSummary {
modified: vec![a_path.clone()],
added: vec![],
removed: vec![z_path.clone()]
}
);
let matcher = FilesMatcher::new(hashset! {dir1_a_path.clone(), dir2_b_path.clone()});
assert_eq!(
tree1.diff_summary(&tree2, &matcher),
DiffSummary {
modified: vec![dir1_a_path.clone()],
added: vec![dir2_b_path.clone()],
removed: vec![]
}
);
assert_eq!(
tree2.diff_summary(&tree1, &matcher),
DiffSummary {
modified: vec![dir1_a_path.clone()],
added: vec![],
removed: vec![dir2_b_path.clone()]
}
);
}

5
lib/tests/test_working_copy.rs
View file

@ -20,6 +20,7 @@ use std::sync::Arc;
use itertools::Itertools;
use jujutsu_lib::commit_builder::CommitBuilder;
use jujutsu_lib::matchers::EverythingMatcher;
use jujutsu_lib::repo::ReadonlyRepo;
use jujutsu_lib::repo_path::{RepoPath, RepoPathComponent};
use jujutsu_lib::settings::UserSettings;
@ -185,7 +186,9 @@ fn test_checkout_file_transitions(use_git: bool) {
// Check that the working copy is clean.
let (reloaded_repo, after_commit) = wc.commit(&settings, repo);
repo = reloaded_repo;
let diff_summary = right_commit.tree().diff_summary(&after_commit.tree());
let diff_summary = right_commit
.tree()
.diff_summary(&after_commit.tree(), &EverythingMatcher);
assert_eq!(diff_summary.modified, vec![]);
assert_eq!(diff_summary.added, vec![]);
assert_eq!(diff_summary.removed, vec![]);

9
src/commands.rs
View file

@ -39,6 +39,7 @@ use jujutsu_lib::evolution::{
use jujutsu_lib::files::DiffLine;
use jujutsu_lib::git::GitFetchError;
use jujutsu_lib::index::HexPrefix;
use jujutsu_lib::matchers::EverythingMatcher;
use jujutsu_lib::op_heads_store::OpHeadsStore;
use jujutsu_lib::op_store::{OpStore, OpStoreError, OperationId};
use jujutsu_lib::operation::Operation;
@ -1052,12 +1053,12 @@ fn cmd_diff(
}
let repo = repo_command.repo();
if sub_matches.is_present("summary") {
let summary = from_tree.diff_summary(&to_tree);
let summary = from_tree.diff_summary(&to_tree, &EverythingMatcher);
show_diff_summary(ui, repo.working_copy_path(), &summary)?;
} else {
let mut formatter = ui.stdout_formatter();
formatter.add_label(String::from("diff"))?;
for (path, diff) in from_tree.diff(&to_tree) {
for (path, diff) in from_tree.diff(&to_tree, &EverythingMatcher) {
let ui_path = ui.format_file_path(repo.working_copy_path(), &path);
match diff {
Diff::Added(TreeValue::Normal {
@ -1211,7 +1212,9 @@ fn cmd_status(
ui.write("Working copy : ")?;
ui.write_commit_summary(repo.as_repo_ref(), &commit)?;
ui.write("\n")?;
let summary = commit.parents()[0].tree().diff_summary(&commit.tree());
let summary = commit.parents()[0]
.tree()
.diff_summary(&commit.tree(), &EverythingMatcher);
if summary.is_empty() {
ui.write("The working copy is clean\n")?;
} else {

3
src/diff_edit.rs
View file

@ -18,6 +18,7 @@ use std::path::{Path, PathBuf};
use std::process::Command;
use std::sync::Arc;
use jujutsu_lib::matchers::EverythingMatcher;
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::store::{StoreError, TreeId, TreeValue};
use jujutsu_lib::store_wrapper::StoreWrapper;
@ -106,7 +107,7 @@ pub fn edit_diff(
let store = left_tree.store();
let mut left_tree_builder = store.tree_builder(store.empty_tree_id().clone());
let mut right_tree_builder = store.tree_builder(store.empty_tree_id().clone());
for (file_path, diff) in left_tree.diff(&right_tree) {
for (file_path, diff) in left_tree.diff(&right_tree, &EverythingMatcher) {
let (left_value, right_value) = diff.as_options();
if let Some(value) = left_value {
add_to_tree(store, &mut left_tree_builder, &file_path, value).unwrap();