merged_tree: allow building trees with variable-arity overrides

When restoring (`jj restore`) a 3-sided conflict from one tree into a
2-sided tree (or a resolved tree), we'll need to extend the size arity
of the target tree to that of the source tree. I had not considered
this case before. This patch relaxes the constraint in
`MergedTreeBuilder` to allow such cases. The additional trees are
based on empty trees with only the larger overrides in them.
This commit is contained in:
Martin von Zweigbergk 2023-08-31 08:44:57 -07:00 committed by Martin von Zweigbergk
parent d29c831ef9
commit 5ef0be73c1
2 changed files with 38 additions and 17 deletions

View file

@ -926,10 +926,7 @@ impl MergedTreeBuilder {
/// a legacy tree, conflicts can be written either as a multi-way `Merge`
/// value or as a resolved `Merge` value using `TreeValue::Conflict`.
pub fn set_or_remove(&mut self, path: RepoPath, values: Merge<Option<TreeValue>>) {
if let MergedTreeId::Merge(base_tree_ids) = &self.base_tree_id {
if !values.is_resolved() {
assert_eq!(values.num_sides(), base_tree_ids.num_sides());
}
if let MergedTreeId::Merge(_) = &self.base_tree_id {
assert!(!values
.iter()
.flatten()
@ -981,9 +978,16 @@ impl MergedTreeBuilder {
fn write_merged_trees(
self,
base_tree_ids: Merge<TreeId>,
mut base_tree_ids: Merge<TreeId>,
store: &Arc<Store>,
) -> Result<Merge<TreeId>, BackendError> {
let num_sides = self
.overrides
.values()
.map(|value| value.num_sides())
.max()
.unwrap_or(0);
base_tree_ids.pad_to(num_sides, store.empty_tree_id());
// Create a single-tree builder for each base tree
let mut tree_builders =
base_tree_ids.map(|base_tree_id| TreeBuilder::new(store.clone(), base_tree_id.clone()));
@ -996,7 +1000,8 @@ impl MergedTreeBuilder {
builder.set_or_remove(path.clone(), value.clone());
}
}
Err(values) => {
Err(mut values) => {
values.pad_to(num_sides, &None);
// This path was overridden with a conflicted value. Apply each term to
// its corresponding builder.
for (builder, value) in zip(tree_builders.iter_mut(), values) {

View file

@ -21,7 +21,7 @@ use jj_lib::repo::Repo;
use jj_lib::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin};
use jj_lib::tree::merge_trees;
use pretty_assertions::assert_eq;
use testutils::{create_single_tree, write_file, write_normal_file, TestRepo};
use testutils::{create_single_tree, write_file, TestRepo};
fn file_value(file_id: &FileId) -> TreeValue {
TreeValue::File {
@ -40,7 +40,8 @@ fn test_from_legacy_tree() {
// file1: regular file without conflicts
let file1_path = RepoPath::from_internal_string("no_conflict");
let file1_id = write_normal_file(&mut tree_builder, &file1_path, "foo");
let file1_id = write_file(store.as_ref(), &file1_path, "foo");
tree_builder.set(file1_path.clone(), file_value(&file1_id));
// file2: 3-way conflict
let file2_path = RepoPath::from_internal_string("3way");
@ -105,13 +106,11 @@ fn test_from_legacy_tree() {
// dir1: directory without conflicts
let dir1_basename = RepoPathComponent::from("dir1");
write_normal_file(
&mut tree_builder,
&RepoPath::root()
.join(&dir1_basename)
.join(&RepoPathComponent::from("file")),
"foo",
);
let dir1_filename = RepoPath::root()
.join(&dir1_basename)
.join(&RepoPathComponent::from("file"));
let dir1_filename_id = write_file(store.as_ref(), &dir1_filename, "file5_v2");
tree_builder.set(dir1_filename.clone(), file_value(&dir1_filename_id));
let tree_id = tree_builder.write_tree();
let tree = store.get_tree(&RepoPath::root(), &tree_id).unwrap();
@ -125,7 +124,7 @@ fn test_from_legacy_tree() {
assert_eq!(
merged_tree.value(&file1_path.components()[0]),
MergedTreeValue::Resolved(Some(&TreeValue::File {
id: file1_id,
id: file1_id.clone(),
executable: false,
}))
);
@ -191,7 +190,8 @@ fn test_from_legacy_tree() {
let recreated_legacy_id = tree_builder.write_tree(store).unwrap();
assert_eq!(recreated_legacy_id, MergedTreeId::Legacy(tree_id.clone()));
// Create the merged tree by starting from an empty merged tree.
// Create the merged tree by starting from an empty merged tree and adding
// entries from the merged tree we created before
let empty_merged_id_builder: MergeBuilder<_> = std::iter::repeat(store.empty_tree_id())
.take(5)
.cloned()
@ -203,6 +203,22 @@ fn test_from_legacy_tree() {
}
let recreated_merged_id = tree_builder.write_tree(store).unwrap();
assert_eq!(recreated_merged_id, merged_tree.id());
// Create the merged tree by adding the same (variable-arity) entries as we
// added to the single-tree TreeBuilder.
let mut tree_builder = MergedTreeBuilder::new(MergedTreeId::Merge(Merge::resolved(
store.empty_tree_id().clone(),
)));
// Add the entries out of order, so we test both increasing and reducing the
// arity (going up from 1-way to 3-way to 5-way, then to 3-way again)
tree_builder.set_or_remove(file1_path, Merge::normal(file_value(&file1_id)));
tree_builder.set_or_remove(file2_path, file2_conflict);
tree_builder.set_or_remove(file5_path, file5_conflict);
tree_builder.set_or_remove(file3_path, file3_conflict);
tree_builder.set_or_remove(file4_path, file4_conflict);
tree_builder.set_or_remove(dir1_filename, Merge::normal(file_value(&dir1_filename_id)));
let recreated_merged_id = tree_builder.write_tree(store).unwrap();
assert_eq!(recreated_merged_id, merged_tree.id());
}
#[test]