mirrors/jj
1
0
Fork 1
mirror of https://github.com/martinvonz/jj.git synced 2024-12-26 14:00:51 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

trees: make tree diff return an iterator instead of taking a callback

Browse source 
This is yet another step towards making it easy to propagate
`BrokenPipe` errors. The `jj diff` code (naturally) diffs two trees
and prints the diffs. If the printing fails, we shouldn't just crash
like we do today.

The new code is probably slower since it does more copying (the
callback got references to the `FileRepoPath` and `TreeValue`). I hope
that won't make a noticeable difference. At least `jj diff -r
334afbc76fbd --summary` didn't seem to get measurably slower.
This commit is contained in:
Martin von Zweigbergk 2021-04-07 11:27:40 -07:00
parent 8b2ce18254
commit f4a41f3880
5 changed files with 260 additions and 229 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
lib/src/tree.rs
View file

@ -17,14 +17,13 @@ use std::fmt::{Debug, Error, Formatter};
use std::pin::Pin;
use std::sync::Arc;
use crate::matchers::AlwaysMatcher;
use crate::repo_path::{
DirRepoPath, DirRepoPathComponent, FileRepoPath, RepoPath, RepoPathComponent, RepoPathJoin,
};
use crate::store;
use crate::store::{ConflictId, TreeEntriesNonRecursiveIter, TreeEntry, TreeId, TreeValue};
use crate::store_wrapper::StoreWrapper;
use crate::trees::{recursive_tree_diff, TreeValueDiff};
use crate::trees::{recursive_tree_diff, Diff, TreeDiffIterator};
#[derive(Clone)]
pub struct Tree {
@ -165,19 +164,21 @@ impl Tree {
}
}
pub fn diff(&self, other: &Tree, callback: &mut impl FnMut(&FileRepoPath, TreeValueDiff)) {
recursive_tree_diff(self.clone(), other.clone(), &AlwaysMatcher {}, callback);
pub fn diff<'a>(&'a self, other: &'a Tree) -> TreeDiffIterator {
recursive_tree_diff(self.clone(), other.clone())
}
pub fn diff_summary(&self, other: &Tree) -> DiffSummary {
let mut modified = vec![];
let mut added = vec![];
let mut removed = vec![];
self.diff(other, &mut |file, diff| match diff {
TreeValueDiff::Modified(_, _) => modified.push(file.clone()),
TreeValueDiff::Added(_) => added.push(file.clone()),
TreeValueDiff::Removed(_) => removed.push(file.clone()),
});
for (file, diff) in self.diff(other) {
match diff {
Diff::Modified(_, _) => modified.push(file.clone()),
Diff::Added(_) => added.push(file.clone()),
Diff::Removed(_) => removed.push(file.clone()),
}
}
modified.sort();
added.sort();
removed.sort();

190
lib/src/trees.rs
View file

@ -14,10 +14,10 @@
use std::cmp::Ordering;
use std::iter::Peekable;
use std::pin::Pin;
use crate::files;
use crate::files::MergeResult;
use crate::matchers::Matcher;
use crate::repo_path::{
DirRepoPath, DirRepoPathComponent, FileRepoPath, FileRepoPathComponent, RepoPathJoin,
};
@ -124,91 +124,119 @@ fn diff_entries<'a>(tree1: &'a Tree, tree2: &'a Tree) -> TreeEntryDiffIterator<'
TreeEntryDiffIterator::new(tree1, tree2)
}
pub fn recursive_tree_diff<M>(
root1: Tree,
root2: Tree,
matcher: &M,
callback: &mut impl FnMut(&FileRepoPath, TreeValueDiff),
) where
M: Matcher,
{
internal_recursive_tree_diff(vec![(DirRepoPath::root(), root1, root2)], matcher, callback)
pub fn recursive_tree_diff(root1: Tree, root2: Tree) -> TreeDiffIterator {
TreeDiffIterator::new(DirRepoPath::root(), root1, root2)
}
fn internal_recursive_tree_diff<M>(
work: Vec<(DirRepoPath, Tree, Tree)>,
_matcher: &M,
callback: &mut impl FnMut(&FileRepoPath, TreeValueDiff),
) where
M: Matcher,
{
let mut new_work = Vec::new();
// Diffs for which to invoke the callback after having visited subtrees. This is
// used for making sure that when a directory gets replaced by a file, we
// call the callback for the addition of the file after we call the callback
pub struct TreeDiffIterator {
dir: DirRepoPath,
tree1: Pin<Box<Tree>>,
tree2: Pin<Box<Tree>>,
// Iterator over the diffs between tree1 and tree2
entry_iterator: TreeEntryDiffIterator<'static>,
// 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.
let mut late_file_diffs = Vec::new();
for (dir, tree1, tree2) in &work {
for (name, diff) in diff_entries(tree1, tree2) {
let file_path = dir.join(&FileRepoPathComponent::from(name.as_str()));
let subdir = DirRepoPathComponent::from(name.as_str());
let subdir_path = dir.join(&subdir);
// TODO: simplify this mess
match diff {
TreeValueDiff::Modified(TreeValue::Tree(id_before), TreeValue::Tree(id_after)) => {
new_work.push((
subdir_path,
tree1.known_sub_tree(&subdir, &id_before),
tree2.known_sub_tree(&subdir, &id_after),
));
}
TreeValueDiff::Modified(TreeValue::Tree(id_before), file_after) => {
new_work.push((
subdir_path.clone(),
tree1.known_sub_tree(&subdir, &id_before),
Tree::null(tree2.store().clone(), subdir_path),
));
late_file_diffs.push((file_path, TreeValueDiff::Added(file_after)));
}
TreeValueDiff::Modified(file_before, TreeValue::Tree(id_after)) => {
new_work.push((
subdir_path.clone(),
Tree::null(tree1.store().clone(), subdir_path),
tree2.known_sub_tree(&subdir, &id_after),
));
callback(&file_path, TreeValueDiff::Removed(file_before));
}
TreeValueDiff::Modified(_, _) => {
callback(&file_path, diff);
}
TreeValueDiff::Added(TreeValue::Tree(id_after)) => {
new_work.push((
subdir_path.clone(),
Tree::null(tree1.store().clone(), subdir_path),
tree2.known_sub_tree(&subdir, &id_after),
));
}
TreeValueDiff::Added(_) => {
callback(&file_path, diff);
}
TreeValueDiff::Removed(TreeValue::Tree(id_before)) => {
new_work.push((
subdir_path.clone(),
tree1.known_sub_tree(&subdir, &id_before),
Tree::null(tree2.store().clone(), subdir_path),
));
}
TreeValueDiff::Removed(_) => {
callback(&file_path, diff);
}
}
added_file: Option<(FileRepoPath, TreeValue)>,
// Iterator over the diffs of a subdirectory, if we're currently visiting one.
subdir_iterator: Option<Box<TreeDiffIterator>>,
}
impl TreeDiffIterator {
fn new(dir: DirRepoPath, tree1: Tree, tree2: Tree) -> 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> =
unsafe { std::mem::transmute(root_entry_iterator) };
Self {
dir,
tree1,
tree2,
entry_iterator: root_entry_iterator,
added_file: None,
subdir_iterator: None,
}
}
if !new_work.is_empty() {
internal_recursive_tree_diff(new_work, _matcher, callback)
}
for (file_path, diff) in late_file_diffs {
callback(&file_path, diff);
}
impl Iterator for TreeDiffIterator {
type Item = (FileRepoPath, Diff<TreeValue>);
fn next(&mut self) -> Option<Self::Item> {
loop {
// First return results from any subdirectory we're currently visiting.
if let Some(subdir_iterator) = &mut self.subdir_iterator {
if let Some(element) = subdir_iterator.next() {
return Some(element);
}
}
if let Some((name, value)) = self.added_file.take() {
return Some((name, Diff::Added(value)));
}
// Note: whenever we say "file" below, it may also be a symlink or a conflict.
if let Some((name, diff)) = self.entry_iterator.next() {
let file_path = self.dir.join(&FileRepoPathComponent::from(name.as_str()));
let subdir = DirRepoPathComponent::from(name.as_str());
let subdir_path = self.dir.join(&subdir);
// TODO: simplify this mess
match diff {
Diff::Modified(TreeValue::Tree(id_before), TreeValue::Tree(id_after)) => {
self.subdir_iterator = Some(Box::new(TreeDiffIterator::new(
subdir_path,
self.tree1.known_sub_tree(&subdir, &id_before),
self.tree2.known_sub_tree(&subdir, &id_after),
)));
}
Diff::Modified(TreeValue::Tree(id_before), file_after) => {
self.subdir_iterator = Some(Box::new(TreeDiffIterator::new(
subdir_path.clone(),
self.tree1.known_sub_tree(&subdir, &id_before),
Tree::null(self.tree2.store().clone(), subdir_path),
)));
self.added_file = Some((file_path, file_after.clone()));
}
Diff::Modified(file_before, TreeValue::Tree(id_after)) => {
self.subdir_iterator = Some(Box::new(TreeDiffIterator::new(
subdir_path.clone(),
Tree::null(self.tree1.store().clone(), subdir_path),
self.tree2.known_sub_tree(&subdir, &id_after),
)));
return Some((file_path, Diff::Removed(file_before.clone())));
}
Diff::Modified(file_before, file_after) => {
return Some((
file_path,
Diff::Modified(file_before.clone(), file_after.clone()),
));
}
Diff::Added(TreeValue::Tree(id_after)) => {
self.subdir_iterator = Some(Box::new(TreeDiffIterator::new(
subdir_path.clone(),
Tree::null(self.tree1.store().clone(), subdir_path),
self.tree2.known_sub_tree(&subdir, &id_after),
)));
}
Diff::Added(value_after) => {
return Some((file_path, Diff::Added(value_after.clone())));
}
Diff::Removed(TreeValue::Tree(id_before)) => {
self.subdir_iterator = Some(Box::new(TreeDiffIterator::new(
subdir_path.clone(),
self.tree1.known_sub_tree(&subdir, &id_before),
Tree::null(self.tree2.store().clone(), subdir_path),
)));
}
Diff::Removed(value_before) => {
return Some((file_path, Diff::Removed(value_before.clone())));
}
}
} else {
return None;
}
}
}
}

30
lib/src/working_copy.rs
View file

@ -42,7 +42,7 @@ use crate::repo_path::{
use crate::settings::UserSettings;
use crate::store::{CommitId, FileId, MillisSinceEpoch, StoreError, SymlinkId, TreeId, TreeValue};
use crate::store_wrapper::StoreWrapper;
use crate::trees::TreeValueDiff;
use crate::trees::Diff;
#[derive(Debug, PartialEq, Eq, Clone)]
pub enum FileType {
@ -470,14 +470,14 @@ impl TreeState {
removed_files: 0,
};
old_tree.diff(&new_tree, &mut |path, diff| {
for (path, diff) in old_tree.diff(&new_tree) {
let disk_path = self
.working_copy_path
.join(PathBuf::from(path.to_internal_string()));
// TODO: Check that the file has not changed before overwriting/removing it.
match diff {
TreeValueDiff::Removed(_before) => {
Diff::Removed(_before) => {
fs::remove_file(&disk_path).ok();
let mut parent_dir = disk_path.parent().unwrap();
loop {
@ -489,15 +489,15 @@ impl TreeState {
self.file_states.remove(&path);
stats.removed_files += 1;
}
TreeValueDiff::Added(after) => {
Diff::Added(after) => {
let file_state = match after {
TreeValue::Normal { id, executable } => {
self.write_file(&disk_path, path, id, *executable)
self.write_file(&disk_path, &path, &id, executable)
}
TreeValue::Symlink(id) => self.write_symlink(&disk_path, path, id),
TreeValue::Symlink(id) => self.write_symlink(&disk_path, &path, &id),
TreeValue::GitSubmodule(_id) => {
println!("ignoring git submodule at {:?}", path);
return;
continue;
}
TreeValue::Tree(_id) => {
panic!("unexpected tree entry in diff at {:?}", path);
@ -512,7 +512,7 @@ impl TreeState {
self.file_states.insert(path.clone(), file_state);
stats.added_files += 1;
}
TreeValueDiff::Modified(before, after) => {
Diff::Modified(before, after) => {
fs::remove_file(&disk_path).ok();
let file_state = match (before, after) {
(
@ -524,19 +524,19 @@ impl TreeState {
) if id == old_id => {
// Optimization for when only the executable bit changed
assert_ne!(executable, old_executable);
self.set_executable(&disk_path, *executable);
self.set_executable(&disk_path, executable);
let mut file_state = self.file_states.get(&path).unwrap().clone();
file_state.mark_executable(*executable);
file_state.mark_executable(executable);
file_state
}
(_, TreeValue::Normal { id, executable }) => {
self.write_file(&disk_path, path, id, *executable)
self.write_file(&disk_path, &path, &id, executable)
}
(_, TreeValue::Symlink(id)) => self.write_symlink(&disk_path, path, id),
(_, TreeValue::Symlink(id)) => self.write_symlink(&disk_path, &path, &id),
(_, TreeValue::GitSubmodule(_id)) => {
println!("ignoring git submodule at {:?}", path);
self.file_states.remove(path);
return;
self.file_states.remove(&path);
continue;
}
(_, TreeValue::Tree(_id)) => {
panic!("unexpected tree entry in diff at {:?}", path);
@ -553,7 +553,7 @@ impl TreeState {
stats.updated_files += 1;
}
}
});
}
self.tree_id = tree_id;
self.save();
Ok(stats)

246
src/commands.rs
View file

@ -44,7 +44,7 @@ use jujube_lib::settings::UserSettings;
use jujube_lib::store::{CommitId, StoreError, Timestamp, TreeValue};
use jujube_lib::store_wrapper::StoreWrapper;
use jujube_lib::tree::Tree;
use jujube_lib::trees::TreeValueDiff;
use jujube_lib::trees::Diff;
use jujube_lib::working_copy::{CheckoutStats, WorkingCopy};
use jujube_lib::{conflicts, files, git};
use pest::Parser;
@ -777,7 +777,6 @@ fn cmd_diff(
}
let mut repo = get_repo(ui, &matches)?;
let mut_repo = Arc::get_mut(&mut repo).unwrap();
if sub_matches.is_present("from") || sub_matches.is_present("to") {}
let from_tree;
let to_tree;
if sub_matches.is_present("from") || sub_matches.is_present("to") {
@ -800,136 +799,139 @@ fn cmd_diff(
} else {
let mut styler = ui.styler();
styler.add_label(String::from("diff"));
from_tree.diff(&to_tree, &mut |path, diff| match diff {
TreeValueDiff::Added(TreeValue::Normal {
id,
executable: false,
}) => {
styler.add_label(String::from("header"));
styler.write_str(&format!("added file {}:\n", path.to_internal_string()));
styler.remove_label();
for (path, diff) in from_tree.diff(&to_tree) {
match diff {
Diff::Added(TreeValue::Normal {
id,
executable: false,
}) => {
styler.add_label(String::from("header"));
styler.write_str(&format!("added file {}:\n", path.to_internal_string()));
styler.remove_label();
let mut file_reader = repo.store().read_file(path, id).unwrap();
styler.write_from_reader(&mut file_reader);
}
TreeValueDiff::Modified(
TreeValue::Normal {
id: id_left,
executable: left_executable,
},
TreeValue::Normal {
id: id_right,
executable: right_executable,
},
) if left_executable == right_executable => {
styler.add_label(String::from("header"));
if *left_executable {
let mut file_reader = repo.store().read_file(&path, &id).unwrap();
styler.write_from_reader(&mut file_reader);
}
Diff::Modified(
TreeValue::Normal {
id: id_left,
executable: left_executable,
},
TreeValue::Normal {
id: id_right,
executable: right_executable,
},
) if left_executable == right_executable => {
styler.add_label(String::from("header"));
if left_executable {
styler.write_str(&format!(
"modified executable file {}:\n",
path.to_internal_string()
));
} else {
styler
.write_str(&format!("modified file {}:\n", path.to_internal_string()));
}
styler.remove_label();
let mut file_reader_left = repo.store().read_file(&path, &id_left).unwrap();
let mut buffer_left = vec![];
file_reader_left.read_to_end(&mut buffer_left).unwrap();
let mut file_reader_right = repo.store().read_file(&path, &id_right).unwrap();
let mut buffer_right = vec![];
file_reader_right.read_to_end(&mut buffer_right).unwrap();
print_diff(
buffer_left.as_slice(),
buffer_right.as_slice(),
styler.as_mut(),
);
}
Diff::Modified(
TreeValue::Conflict(id_left),
TreeValue::Normal {
id: id_right,
executable: false,
},
) => {
styler.add_label(String::from("header"));
styler.write_str(&format!(
"modified executable file {}:\n",
"resolved conflict in file {}:\n",
path.to_internal_string()
));
} else {
styler.write_str(&format!("modified file {}:\n", path.to_internal_string()));
styler.remove_label();
let conflict_left = repo.store().read_conflict(&id_left).unwrap();
let mut buffer_left = vec![];
conflicts::materialize_conflict(
repo.store(),
&path.to_repo_path(),
&conflict_left,
&mut buffer_left,
);
let mut file_reader_right = repo.store().read_file(&path, &id_right).unwrap();
let mut buffer_right = vec![];
file_reader_right.read_to_end(&mut buffer_right).unwrap();
print_diff(
buffer_left.as_slice(),
buffer_right.as_slice(),
styler.as_mut(),
);
}
styler.remove_label();
Diff::Modified(
TreeValue::Normal {
id: id_left,
executable: false,
},
TreeValue::Conflict(id_right),
) => {
styler.add_label(String::from("header"));
styler.write_str(&format!(
"new conflict in file {}:\n",
path.to_internal_string()
));
styler.remove_label();
let mut file_reader_left = repo.store().read_file(path, id_left).unwrap();
let mut buffer_left = vec![];
file_reader_left.read_to_end(&mut buffer_left).unwrap();
let mut file_reader_right = repo.store().read_file(path, id_right).unwrap();
let mut buffer_right = vec![];
file_reader_right.read_to_end(&mut buffer_right).unwrap();
let mut file_reader_left = repo.store().read_file(&path, &id_left).unwrap();
let mut buffer_left = vec![];
file_reader_left.read_to_end(&mut buffer_left).unwrap();
let conflict_right = repo.store().read_conflict(&id_right).unwrap();
let mut buffer_right = vec![];
conflicts::materialize_conflict(
repo.store(),
&path.to_repo_path(),
&conflict_right,
&mut buffer_right,
);
print_diff(
buffer_left.as_slice(),
buffer_right.as_slice(),
styler.as_mut(),
);
}
TreeValueDiff::Modified(
TreeValue::Conflict(id_left),
TreeValue::Normal {
id: id_right,
print_diff(
buffer_left.as_slice(),
buffer_right.as_slice(),
styler.as_mut(),
);
}
Diff::Removed(TreeValue::Normal {
id,
executable: false,
},
) => {
styler.add_label(String::from("header"));
styler.write_str(&format!(
"resolved conflict in file {}:\n",
path.to_internal_string()
));
styler.remove_label();
}) => {
styler.add_label(String::from("header"));
styler.write_str(&format!("removed file {}:\n", path.to_internal_string()));
styler.remove_label();
let conflict_left = repo.store().read_conflict(id_left).unwrap();
let mut buffer_left = vec![];
conflicts::materialize_conflict(
repo.store(),
&path.to_repo_path(),
&conflict_left,
&mut buffer_left,
);
let mut file_reader_right = repo.store().read_file(path, id_right).unwrap();
let mut buffer_right = vec![];
file_reader_right.read_to_end(&mut buffer_right).unwrap();
print_diff(
buffer_left.as_slice(),
buffer_right.as_slice(),
styler.as_mut(),
);
let mut file_reader = repo.store().read_file(&path, &id).unwrap();
styler.write_from_reader(&mut file_reader);
}
other => {
writeln!(
styler,
"unhandled diff case in path {:?}: {:?}",
path, other
)
.unwrap();
}
}
TreeValueDiff::Modified(
TreeValue::Normal {
id: id_left,
executable: false,
},
TreeValue::Conflict(id_right),
) => {
styler.add_label(String::from("header"));
styler.write_str(&format!(
"new conflict in file {}:\n",
path.to_internal_string()
));
styler.remove_label();
let mut file_reader_left = repo.store().read_file(path, id_left).unwrap();
let mut buffer_left = vec![];
file_reader_left.read_to_end(&mut buffer_left).unwrap();
let conflict_right = repo.store().read_conflict(id_right).unwrap();
let mut buffer_right = vec![];
conflicts::materialize_conflict(
repo.store(),
&path.to_repo_path(),
&conflict_right,
&mut buffer_right,
);
print_diff(
buffer_left.as_slice(),
buffer_right.as_slice(),
styler.as_mut(),
);
}
TreeValueDiff::Removed(TreeValue::Normal {
id,
executable: false,
}) => {
styler.add_label(String::from("header"));
styler.write_str(&format!("removed file {}:\n", path.to_internal_string()));
styler.remove_label();
let mut file_reader = repo.store().read_file(path, id).unwrap();
styler.write_from_reader(&mut file_reader);
}
other => {
writeln!(
styler,
"unhandled diff case in path {:?}: {:?}",
path, other
)
.unwrap();
}
});
}
styler.remove_label();
}
Ok(())

4
src/diff_edit.rs
View file

@ -98,7 +98,7 @@ pub fn edit_diff(left_tree: &Tree, right_tree: &Tree) -> Result<TreeId, DiffEdit
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());
left_tree.diff(&right_tree, &mut |file_path, diff| {
for (file_path, diff) in left_tree.diff(&right_tree) {
let (left_value, right_value) = diff.as_options();
let repo_path = file_path.to_repo_path();
if let Some(value) = left_value {
@ -107,7 +107,7 @@ pub fn edit_diff(left_tree: &Tree, right_tree: &Tree) -> Result<TreeId, DiffEdit
if let Some(value) = right_value {
add_to_tree(store, &mut right_tree_builder, &repo_path, value).unwrap();
}
});
}
let left_partial_tree_id = left_tree_builder.write_tree();
let right_partial_tree_id = right_tree_builder.write_tree();
let right_partial_tree = store.get_tree(&DirRepoPath::root(), &right_partial_tree_id)?;