From ef5f97f8d7bdeb3331ada4ff0220a2222b89b3bb Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sun, 6 Aug 2023 09:21:35 -0700 Subject: [PATCH] conflicts: move `Merge` to `merge` module The `merge` module now seems like the obvious place for this type. --- cli/src/commands/mod.rs | 2 +- lib/src/backend.rs | 2 +- lib/src/commit_builder.rs | 2 +- lib/src/conflicts.rs | 462 +-------------------------------- lib/src/files.rs | 3 +- lib/src/git_backend.rs | 2 +- lib/src/local_backend.rs | 2 +- lib/src/merge.rs | 459 ++++++++++++++++++++++++++++++++ lib/src/merged_tree.rs | 2 +- lib/src/op_store.rs | 2 +- lib/src/refs.rs | 3 +- lib/src/simple_op_store.rs | 2 +- lib/src/store.rs | 2 +- lib/src/tree.rs | 3 +- lib/tests/test_conflicts.rs | 3 +- lib/tests/test_merged_tree.rs | 2 +- lib/tests/test_refs.rs | 2 +- lib/tests/test_working_copy.rs | 2 +- 18 files changed, 480 insertions(+), 477 deletions(-) diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index 6c0465c75..3d97ed0f4 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -33,10 +33,10 @@ use indexmap::{IndexMap, IndexSet}; use itertools::Itertools; use jj_lib::backend::{CommitId, ObjectId, TreeValue}; use jj_lib::commit::Commit; -use jj_lib::conflicts::Merge; use jj_lib::dag_walk::topo_order_reverse; use jj_lib::git_backend::GitBackend; use jj_lib::matchers::EverythingMatcher; +use jj_lib::merge::Merge; use jj_lib::op_store::WorkspaceId; use jj_lib::repo::{ReadonlyRepo, Repo}; use jj_lib::repo_path::RepoPath; diff --git a/lib/src/backend.rs b/lib/src/backend.rs index 06ef84fa9..f9274eac9 100644 --- a/lib/src/backend.rs +++ b/lib/src/backend.rs @@ -23,8 +23,8 @@ use std::vec::Vec; use thiserror::Error; -use crate::conflicts::Merge; use crate::content_hash::ContentHash; +use crate::merge::Merge; use crate::repo_path::{RepoPath, RepoPathComponent}; pub trait ObjectId { diff --git a/lib/src/commit_builder.rs b/lib/src/commit_builder.rs index c58049824..26c3e4d4d 100644 --- a/lib/src/commit_builder.rs +++ b/lib/src/commit_builder.rs @@ -18,7 +18,7 @@ use std::sync::Arc; use crate::backend::{self, BackendResult, ChangeId, CommitId, Signature, TreeId}; use crate::commit::Commit; -use crate::conflicts::Merge; +use crate::merge::Merge; use crate::repo::{MutableRepo, Repo}; use crate::settings::{JJRng, UserSettings}; diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index 496803063..941389a77 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -14,22 +14,14 @@ #![allow(missing_docs)] -use std::borrow::Borrow; -use std::hash::Hash; use std::io::Write; -use std::sync::Arc; use itertools::Itertools; -use crate::backend::{BackendError, BackendResult, FileId, ObjectId, TreeId, TreeValue}; -use crate::content_hash::ContentHash; use crate::diff::{find_line_ranges, Diff, DiffHunk}; +use crate::files; use crate::files::{ContentHunk, MergeResult}; -use crate::merge::trivial_merge; -use crate::repo_path::RepoPath; -use crate::store::Store; -use crate::tree::Tree; -use crate::{backend, files}; +use crate::merge::Merge; const CONFLICT_START_LINE: &[u8] = b"<<<<<<<\n"; const CONFLICT_END_LINE: &[u8] = b">>>>>>>\n"; @@ -37,452 +29,6 @@ const CONFLICT_DIFF_LINE: &[u8] = b"%%%%%%%\n"; const CONFLICT_MINUS_LINE: &[u8] = b"-------\n"; const CONFLICT_PLUS_LINE: &[u8] = b"+++++++\n"; -/// A generic representation of merged values. -/// -/// There is exactly one more `adds()` than `removes()`. When interpreted as a -/// series of diffs, the merge's (i+1)-st add is matched with the i-th -/// remove. The zeroth add is considered a diff from the non-existent state. -#[derive(PartialEq, Eq, Hash, Clone, Debug)] -pub struct Merge { - removes: Vec, - adds: Vec, -} - -impl Merge { - pub fn new(removes: Vec, adds: Vec) -> Self { - assert_eq!(adds.len(), removes.len() + 1); - Merge { removes, adds } - } - - /// Creates a `Merge` with a single resolved value. - pub fn resolved(value: T) -> Self { - Merge::new(vec![], vec![value]) - } - - /// Create a `Merge` from a `removes` and `adds`, padding with `None` to - /// make sure that there is exactly one more `adds` than `removes`. - pub fn from_legacy_form( - removes: impl IntoIterator, - adds: impl IntoIterator, - ) -> Merge> { - let mut removes = removes.into_iter().map(Some).collect_vec(); - let mut adds = adds.into_iter().map(Some).collect_vec(); - while removes.len() + 1 < adds.len() { - removes.push(None); - } - while adds.len() < removes.len() + 1 { - adds.push(None); - } - Merge::new(removes, adds) - } - - /// Returns the removes and adds as a pair. - pub fn take(self) -> (Vec, Vec) { - (self.removes, self.adds) - } - - pub fn removes(&self) -> &[T] { - &self.removes - } - - pub fn adds(&self) -> &[T] { - &self.adds - } - - /// Whether this merge is resolved. Does not resolve trivial merges. - pub fn is_resolved(&self) -> bool { - self.removes.is_empty() - } - - /// Returns the resolved value, if this merge is resolved. Does not - /// resolve trivial merges. - pub fn as_resolved(&self) -> Option<&T> { - if let [value] = &self.adds[..] { - Some(value) - } else { - None - } - } - - /// Simplify the merge by joining diffs like A->B and B->C into A->C. - /// Also drops trivial diffs like A->A. - pub fn simplify(mut self) -> Self - where - T: PartialEq, - { - let mut add_index = 0; - while add_index < self.adds.len() { - let add = &self.adds[add_index]; - if let Some(remove_index) = self.removes.iter().position(|remove| remove == add) { - // Move the value to the `add_index-1`th diff, then delete the `remove_index`th - // diff. - self.adds.swap(remove_index + 1, add_index); - self.removes.remove(remove_index); - self.adds.remove(remove_index + 1); - } else { - add_index += 1; - } - } - self - } - - pub fn resolve_trivial(&self) -> Option<&T> - where - T: Eq + Hash, - { - trivial_merge(&self.removes, &self.adds) - } - - /// Creates a new merge by applying `f` to each remove and add. - pub fn map<'a, U>(&'a self, mut f: impl FnMut(&'a T) -> U) -> Merge { - self.maybe_map(|term| Some(f(term))).unwrap() - } - - /// Creates a new merge by applying `f` to each remove and add, returning - /// `None if `f` returns `None` for any of them. - pub fn maybe_map<'a, U>(&'a self, mut f: impl FnMut(&'a T) -> Option) -> Option> { - let removes = self.removes.iter().map(&mut f).collect::>()?; - let adds = self.adds.iter().map(&mut f).collect::>()?; - Some(Merge { removes, adds }) - } - - /// Creates a new merge by applying `f` to each remove and add, returning - /// `Err if `f` returns `Err` for any of them. - pub fn try_map<'a, U, E>( - &'a self, - mut f: impl FnMut(&'a T) -> Result, - ) -> Result, E> { - let removes = self.removes.iter().map(&mut f).try_collect()?; - let adds = self.adds.iter().map(&mut f).try_collect()?; - Ok(Merge { removes, adds }) - } -} - -impl Merge> { - /// Creates lists of `removes` and `adds` from a `Merge` by dropping - /// `None` values. Note that the conversion is lossy: the order of `None` - /// values is not preserved when converting back to a `Merge`. - pub fn into_legacy_form(self) -> (Vec, Vec) { - ( - self.removes.into_iter().flatten().collect(), - self.adds.into_iter().flatten().collect(), - ) - } -} - -impl Merge> { - /// Flattens a nested merge into a regular merge. - /// - /// Let's say we have a 3-way merge of 3-way merges like this: - /// - /// 4 5 7 8 - /// 3 6 - /// 1 2 - /// 0 - /// - /// Flattening that results in this 9-way merge: - /// - /// 4 5 0 7 8 - /// 3 2 1 6 - pub fn flatten(mut self) -> Merge { - self.removes.reverse(); - self.adds.reverse(); - let mut result = self.adds.pop().unwrap(); - while let Some(mut remove) = self.removes.pop() { - // Add removes reversed, and with the first element moved last, so we preserve - // the diffs - let first_add = remove.adds.remove(0); - result.removes.extend(remove.adds); - result.removes.push(first_add); - result.adds.extend(remove.removes); - let add = self.adds.pop().unwrap(); - result.removes.extend(add.removes); - result.adds.extend(add.adds); - } - assert!(self.adds.is_empty()); - result - } -} - -impl ContentHash for Merge { - fn hash(&self, state: &mut impl digest::Update) { - self.removes().hash(state); - self.adds().hash(state); - } -} - -impl Merge { - // Creates a resolved merge for a legacy tree id (same as - // `Merge::resolved()`). - // TODO(#1624): delete when all callers have been updated to support tree-level - // conflicts - pub fn from_legacy_tree_id(value: TreeId) -> Self { - Merge { - removes: vec![], - adds: vec![value], - } - } - - // TODO(#1624): delete when all callers have been updated to support tree-level - // conflicts - pub fn as_legacy_tree_id(&self) -> &TreeId { - self.as_resolved().unwrap() - } -} - -impl Merge> { - /// Create a `Merge` from a `backend::Conflict`, padding with `None` to - /// make sure that there is exactly one more `adds()` than `removes()`. - pub fn from_backend_conflict(conflict: backend::Conflict) -> Self { - let removes = conflict.removes.into_iter().map(|term| term.value); - let adds = conflict.adds.into_iter().map(|term| term.value); - Merge::from_legacy_form(removes, adds) - } - - /// Creates a `backend::Conflict` from a `Merge` by dropping `None` - /// values. Note that the conversion is lossy: the order of `None` values is - /// not preserved when converting back to a `Merge`. - pub fn into_backend_conflict(self) -> backend::Conflict { - let (removes, adds) = self.into_legacy_form(); - let removes = removes - .into_iter() - .map(|value| backend::ConflictTerm { value }) - .collect(); - let adds = adds - .into_iter() - .map(|value| backend::ConflictTerm { value }) - .collect(); - backend::Conflict { removes, adds } - } - - pub fn materialize( - &self, - store: &Store, - path: &RepoPath, - output: &mut dyn Write, - ) -> std::io::Result<()> { - if let Some(file_merge) = self.to_file_merge() { - let content = file_merge.extract_as_single_hunk(store, path); - materialize_merge_result(&content, output) - } else { - // Unless all terms are regular files, we can't do much better than to try to - // describe the merge. - self.describe(output) - } - } - - pub fn to_file_merge(&self) -> Option>> { - self.maybe_map(|term| match term { - None => Some(None), - Some(TreeValue::File { - id, - executable: false, - }) => Some(Some(id.clone())), - _ => None, - }) - } - - /// Give a summary description of the conflict's "removes" and "adds" - pub fn describe(&self, file: &mut dyn Write) -> std::io::Result<()> { - file.write_all(b"Conflict:\n")?; - for term in self.removes().iter().flatten() { - file.write_all(format!(" Removing {}\n", describe_conflict_term(term)).as_bytes())?; - } - for term in self.adds().iter().flatten() { - file.write_all(format!(" Adding {}\n", describe_conflict_term(term)).as_bytes())?; - } - Ok(()) - } - - /// Returns `None` if there are no conflict markers in `content`. - pub fn update_from_content( - &self, - store: &Store, - path: &RepoPath, - content: &[u8], - ) -> BackendResult>>> { - // TODO: Check that the conflict only involves files and convert it to a - // `Merge>` so we can remove the wildcard pattern in the loops - // further down. - - // First check if the new content is unchanged compared to the old content. If - // it is, we don't need parse the content or write any new objects to the - // store. This is also a way of making sure that unchanged tree/file - // conflicts (for example) are not converted to regular files in the working - // copy. - let mut old_content = Vec::with_capacity(content.len()); - self.materialize(store, path, &mut old_content).unwrap(); - if content == old_content { - return Ok(Some(self.clone())); - } - - let mut removed_content = vec![vec![]; self.removes().len()]; - let mut added_content = vec![vec![]; self.adds().len()]; - let Some(hunks) = parse_conflict(content, self.removes().len(), self.adds().len()) else { - // Either there are no self markers of they don't have the expected arity - return Ok(None); - }; - for hunk in hunks { - if let Some(slice) = hunk.as_resolved() { - for buf in &mut removed_content { - buf.extend_from_slice(&slice.0); - } - for buf in &mut added_content { - buf.extend_from_slice(&slice.0); - } - } else { - let (removes, adds) = hunk.take(); - for (i, buf) in removes.into_iter().enumerate() { - removed_content[i].extend(buf.0); - } - for (i, buf) in adds.into_iter().enumerate() { - added_content[i].extend(buf.0); - } - } - } - // Now write the new files contents we found by parsing the file - // with conflict markers. Update the Merge object with the new - // FileIds. - let mut new_removes = vec![]; - for (i, buf) in removed_content.iter().enumerate() { - match &self.removes()[i] { - Some(TreeValue::File { id: _, executable }) => { - let file_id = store.write_file(path, &mut buf.as_slice())?; - let new_value = TreeValue::File { - id: file_id, - executable: *executable, - }; - new_removes.push(Some(new_value)); - } - None if buf.is_empty() => { - // The missing side of a conflict is still represented by - // the empty string we materialized it as - new_removes.push(None); - } - _ => { - // The user edited a non-file side. This should never happen. We consider the - // conflict resolved for now. - return Ok(None); - } - } - } - let mut new_adds = vec![]; - for (i, buf) in added_content.iter().enumerate() { - match &self.adds()[i] { - Some(TreeValue::File { id: _, executable }) => { - let file_id = store.write_file(path, &mut buf.as_slice())?; - let new_value = TreeValue::File { - id: file_id, - executable: *executable, - }; - new_adds.push(Some(new_value)); - } - None if buf.is_empty() => { - // The missing side of a conflict is still represented by - // the empty string we materialized it as => nothing to do - new_adds.push(None); - } - _ => { - // The user edited a non-file side. This should never happen. We consider the - // conflict resolved for now. - return Ok(None); - } - } - } - Ok(Some(Merge::new(new_removes, new_adds))) - } -} - -impl Merge> { - pub fn extract_as_single_hunk(&self, store: &Store, path: &RepoPath) -> Merge { - self.map(|term| get_file_contents(store, path, term)) - } -} - -impl Merge> -where - T: Borrow, -{ - /// If every non-`None` term of a `Merge>` - /// is a `TreeValue::Tree`, this converts it to - /// a `Merge`, with empty trees instead of - /// any `None` terms. Otherwise, returns `None`. - pub fn to_tree_merge( - &self, - store: &Arc, - dir: &RepoPath, - ) -> Result>, BackendError> { - let tree_id_merge = self.maybe_map(|term| match term { - None => Some(None), - Some(value) => { - if let TreeValue::Tree(id) = value.borrow() { - Some(Some(id)) - } else { - None - } - } - }); - if let Some(tree_id_merge) = tree_id_merge { - let get_tree = |id: &Option<&TreeId>| -> Result { - if let Some(id) = id { - store.get_tree(dir, id) - } else { - Ok(Tree::null(store.clone(), dir.clone())) - } - }; - Ok(Some(tree_id_merge.try_map(get_tree)?)) - } else { - Ok(None) - } - } -} - -fn describe_conflict_term(value: &TreeValue) -> String { - match value { - TreeValue::File { - id, - executable: false, - } => { - format!("file with id {}", id.hex()) - } - TreeValue::File { - id, - executable: true, - } => { - format!("executable file with id {}", id.hex()) - } - TreeValue::Symlink(id) => { - format!("symlink with id {}", id.hex()) - } - TreeValue::Tree(id) => { - format!("tree with id {}", id.hex()) - } - TreeValue::GitSubmodule(id) => { - format!("Git submodule with id {}", id.hex()) - } - TreeValue::Conflict(id) => { - format!("Conflict with id {}", id.hex()) - } - } -} - -fn get_file_contents(store: &Store, path: &RepoPath, term: &Option) -> ContentHunk { - match term { - Some(id) => { - let mut content = vec![]; - store - .read_file(path, id) - .unwrap() - .read_to_end(&mut content) - .unwrap(); - ContentHunk(content) - } - // If the conflict had removed the file on one side, we pretend that the file - // was empty there. - None => ContentHunk(vec![]), - } -} - fn write_diff_hunks(hunks: &[DiffHunk], file: &mut dyn Write) -> std::io::Result<()> { for hunk in hunks { match hunk { @@ -512,12 +58,12 @@ pub fn materialize_merge_result( output: &mut dyn Write, ) -> std::io::Result<()> { let removed_slices = single_hunk - .removes + .removes() .iter() .map(|hunk| hunk.0.as_slice()) .collect_vec(); let added_slices = single_hunk - .adds + .adds() .iter() .map(|hunk| hunk.0.as_slice()) .collect_vec(); diff --git a/lib/src/files.rs b/lib/src/files.rs index 1aa123d48..4dd967f8b 100644 --- a/lib/src/files.rs +++ b/lib/src/files.rs @@ -20,10 +20,9 @@ use std::ops::Range; use itertools::Itertools; -use crate::conflicts::Merge; use crate::diff; use crate::diff::{Diff, DiffHunk}; -use crate::merge::trivial_merge; +use crate::merge::{trivial_merge, Merge}; #[derive(PartialEq, Eq, Clone, Debug)] pub struct DiffLine<'a> { diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index e8bbcdb46..2bfaecd36 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -32,9 +32,9 @@ use crate::backend::{ ChangeId, Commit, CommitId, Conflict, ConflictId, ConflictTerm, FileId, MillisSinceEpoch, ObjectId, Signature, SymlinkId, Timestamp, Tree, TreeId, TreeValue, }; -use crate::conflicts::Merge; use crate::file_util::{IoResultExt as _, PathError}; use crate::lock::FileLock; +use crate::merge::Merge; use crate::repo_path::{RepoPath, RepoPathComponent}; use crate::stacked_table::{ MutableTable, ReadonlyTable, TableSegment, TableStore, TableStoreError, diff --git a/lib/src/local_backend.rs b/lib/src/local_backend.rs index 57858906d..b94270f5c 100644 --- a/lib/src/local_backend.rs +++ b/lib/src/local_backend.rs @@ -30,9 +30,9 @@ use crate::backend::{ ConflictId, ConflictTerm, FileId, MillisSinceEpoch, ObjectId, Signature, SymlinkId, Timestamp, Tree, TreeId, TreeValue, }; -use crate::conflicts::Merge; use crate::content_hash::blake2b_hash; use crate::file_util::persist_content_addressed_temp_file; +use crate::merge::Merge; use crate::repo_path::{RepoPath, RepoPathComponent}; const COMMIT_ID_LENGTH: usize = 64; diff --git a/lib/src/merge.rs b/lib/src/merge.rs index b3deeefec..1923a394a 100644 --- a/lib/src/merge.rs +++ b/lib/src/merge.rs @@ -14,11 +14,22 @@ #![allow(missing_docs)] +use std::borrow::Borrow; use std::collections::HashMap; use std::hash::Hash; +use std::io::Write; +use std::sync::Arc; use itertools::Itertools; +use crate::backend::{BackendError, BackendResult, FileId, ObjectId, TreeId, TreeValue}; +use crate::content_hash::ContentHash; +use crate::files::ContentHunk; +use crate::repo_path::RepoPath; +use crate::store::Store; +use crate::tree::Tree; +use crate::{backend, conflicts}; + /// Attempt to resolve trivial conflicts between the inputs. There must be /// exactly one more adds than removes. pub fn trivial_merge<'a, T>(removes: &'a [T], adds: &'a [T]) -> Option<&'a T> @@ -153,3 +164,451 @@ mod tests { assert_eq!(trivial_merge(&[0, 1], &[2, 3, 4]), None); } } + +/// A generic representation of merged values. +/// +/// There is exactly one more `adds()` than `removes()`. When interpreted as a +/// series of diffs, the merge's (i+1)-st add is matched with the i-th +/// remove. The zeroth add is considered a diff from the non-existent state. +#[derive(PartialEq, Eq, Hash, Clone, Debug)] +pub struct Merge { + removes: Vec, + adds: Vec, +} + +impl Merge { + pub fn new(removes: Vec, adds: Vec) -> Self { + assert_eq!(adds.len(), removes.len() + 1); + Merge { removes, adds } + } + + /// Creates a `Merge` with a single resolved value. + pub fn resolved(value: T) -> Self { + Merge::new(vec![], vec![value]) + } + + /// Create a `Merge` from a `removes` and `adds`, padding with `None` to + /// make sure that there is exactly one more `adds` than `removes`. + pub fn from_legacy_form( + removes: impl IntoIterator, + adds: impl IntoIterator, + ) -> Merge> { + let mut removes = removes.into_iter().map(Some).collect_vec(); + let mut adds = adds.into_iter().map(Some).collect_vec(); + while removes.len() + 1 < adds.len() { + removes.push(None); + } + while adds.len() < removes.len() + 1 { + adds.push(None); + } + Merge::new(removes, adds) + } + + /// Returns the removes and adds as a pair. + pub fn take(self) -> (Vec, Vec) { + (self.removes, self.adds) + } + + pub fn removes(&self) -> &[T] { + &self.removes + } + + pub fn adds(&self) -> &[T] { + &self.adds + } + + /// Whether this merge is resolved. Does not resolve trivial merges. + pub fn is_resolved(&self) -> bool { + self.removes.is_empty() + } + + /// Returns the resolved value, if this merge is resolved. Does not + /// resolve trivial merges. + pub fn as_resolved(&self) -> Option<&T> { + if let [value] = &self.adds[..] { + Some(value) + } else { + None + } + } + + /// Simplify the merge by joining diffs like A->B and B->C into A->C. + /// Also drops trivial diffs like A->A. + pub fn simplify(mut self) -> Self + where + T: PartialEq, + { + let mut add_index = 0; + while add_index < self.adds.len() { + let add = &self.adds[add_index]; + if let Some(remove_index) = self.removes.iter().position(|remove| remove == add) { + // Move the value to the `add_index-1`th diff, then delete the `remove_index`th + // diff. + self.adds.swap(remove_index + 1, add_index); + self.removes.remove(remove_index); + self.adds.remove(remove_index + 1); + } else { + add_index += 1; + } + } + self + } + + pub fn resolve_trivial(&self) -> Option<&T> + where + T: Eq + Hash, + { + trivial_merge(&self.removes, &self.adds) + } + + /// Creates a new merge by applying `f` to each remove and add. + pub fn map<'a, U>(&'a self, mut f: impl FnMut(&'a T) -> U) -> Merge { + self.maybe_map(|term| Some(f(term))).unwrap() + } + + /// Creates a new merge by applying `f` to each remove and add, returning + /// `None if `f` returns `None` for any of them. + pub fn maybe_map<'a, U>(&'a self, mut f: impl FnMut(&'a T) -> Option) -> Option> { + let removes = self.removes.iter().map(&mut f).collect::>()?; + let adds = self.adds.iter().map(&mut f).collect::>()?; + Some(Merge { removes, adds }) + } + + /// Creates a new merge by applying `f` to each remove and add, returning + /// `Err if `f` returns `Err` for any of them. + pub fn try_map<'a, U, E>( + &'a self, + mut f: impl FnMut(&'a T) -> Result, + ) -> Result, E> { + let removes = self.removes.iter().map(&mut f).try_collect()?; + let adds = self.adds.iter().map(&mut f).try_collect()?; + Ok(Merge { removes, adds }) + } +} + +impl Merge> { + /// Creates lists of `removes` and `adds` from a `Merge` by dropping + /// `None` values. Note that the conversion is lossy: the order of `None` + /// values is not preserved when converting back to a `Merge`. + pub fn into_legacy_form(self) -> (Vec, Vec) { + ( + self.removes.into_iter().flatten().collect(), + self.adds.into_iter().flatten().collect(), + ) + } +} + +impl Merge> { + /// Flattens a nested merge into a regular merge. + /// + /// Let's say we have a 3-way merge of 3-way merges like this: + /// + /// 4 5 7 8 + /// 3 6 + /// 1 2 + /// 0 + /// + /// Flattening that results in this 9-way merge: + /// + /// 4 5 0 7 8 + /// 3 2 1 6 + pub fn flatten(mut self) -> Merge { + self.removes.reverse(); + self.adds.reverse(); + let mut result = self.adds.pop().unwrap(); + while let Some(mut remove) = self.removes.pop() { + // Add removes reversed, and with the first element moved last, so we preserve + // the diffs + let first_add = remove.adds.remove(0); + result.removes.extend(remove.adds); + result.removes.push(first_add); + result.adds.extend(remove.removes); + let add = self.adds.pop().unwrap(); + result.removes.extend(add.removes); + result.adds.extend(add.adds); + } + assert!(self.adds.is_empty()); + result + } +} + +impl ContentHash for Merge { + fn hash(&self, state: &mut impl digest::Update) { + self.removes().hash(state); + self.adds().hash(state); + } +} + +impl Merge { + // Creates a resolved merge for a legacy tree id (same as + // `Merge::resolved()`). + // TODO(#1624): delete when all callers have been updated to support tree-level + // conflicts + pub fn from_legacy_tree_id(value: TreeId) -> Self { + Merge { + removes: vec![], + adds: vec![value], + } + } + + // TODO(#1624): delete when all callers have been updated to support tree-level + // conflicts + pub fn as_legacy_tree_id(&self) -> &TreeId { + self.as_resolved().unwrap() + } +} + +impl Merge> { + /// Create a `Merge` from a `backend::Conflict`, padding with `None` to + /// make sure that there is exactly one more `adds()` than `removes()`. + pub fn from_backend_conflict(conflict: backend::Conflict) -> Self { + let removes = conflict.removes.into_iter().map(|term| term.value); + let adds = conflict.adds.into_iter().map(|term| term.value); + Merge::from_legacy_form(removes, adds) + } + + /// Creates a `backend::Conflict` from a `Merge` by dropping `None` + /// values. Note that the conversion is lossy: the order of `None` values is + /// not preserved when converting back to a `Merge`. + pub fn into_backend_conflict(self) -> backend::Conflict { + let (removes, adds) = self.into_legacy_form(); + let removes = removes + .into_iter() + .map(|value| backend::ConflictTerm { value }) + .collect(); + let adds = adds + .into_iter() + .map(|value| backend::ConflictTerm { value }) + .collect(); + backend::Conflict { removes, adds } + } + + pub fn materialize( + &self, + store: &Store, + path: &RepoPath, + output: &mut dyn Write, + ) -> std::io::Result<()> { + if let Some(file_merge) = self.to_file_merge() { + let content = file_merge.extract_as_single_hunk(store, path); + conflicts::materialize_merge_result(&content, output) + } else { + // Unless all terms are regular files, we can't do much better than to try to + // describe the merge. + self.describe(output) + } + } + + pub fn to_file_merge(&self) -> Option>> { + self.maybe_map(|term| match term { + None => Some(None), + Some(TreeValue::File { + id, + executable: false, + }) => Some(Some(id.clone())), + _ => None, + }) + } + + /// Give a summary description of the conflict's "removes" and "adds" + pub fn describe(&self, file: &mut dyn Write) -> std::io::Result<()> { + file.write_all(b"Conflict:\n")?; + for term in self.removes().iter().flatten() { + file.write_all(format!(" Removing {}\n", describe_conflict_term(term)).as_bytes())?; + } + for term in self.adds().iter().flatten() { + file.write_all(format!(" Adding {}\n", describe_conflict_term(term)).as_bytes())?; + } + Ok(()) + } + + /// Returns `None` if there are no conflict markers in `content`. + pub fn update_from_content( + &self, + store: &Store, + path: &RepoPath, + content: &[u8], + ) -> BackendResult>>> { + // TODO: Check that the conflict only involves files and convert it to a + // `Merge>` so we can remove the wildcard pattern in the loops + // further down. + + // First check if the new content is unchanged compared to the old content. If + // it is, we don't need parse the content or write any new objects to the + // store. This is also a way of making sure that unchanged tree/file + // conflicts (for example) are not converted to regular files in the working + // copy. + let mut old_content = Vec::with_capacity(content.len()); + self.materialize(store, path, &mut old_content).unwrap(); + if content == old_content { + return Ok(Some(self.clone())); + } + + let mut removed_content = vec![vec![]; self.removes().len()]; + let mut added_content = vec![vec![]; self.adds().len()]; + let Some(hunks) = + conflicts::parse_conflict(content, self.removes().len(), self.adds().len()) + else { + // Either there are no self markers of they don't have the expected arity + return Ok(None); + }; + for hunk in hunks { + if let Some(slice) = hunk.as_resolved() { + for buf in &mut removed_content { + buf.extend_from_slice(&slice.0); + } + for buf in &mut added_content { + buf.extend_from_slice(&slice.0); + } + } else { + let (removes, adds) = hunk.take(); + for (i, buf) in removes.into_iter().enumerate() { + removed_content[i].extend(buf.0); + } + for (i, buf) in adds.into_iter().enumerate() { + added_content[i].extend(buf.0); + } + } + } + // Now write the new files contents we found by parsing the file + // with conflict markers. Update the Merge object with the new + // FileIds. + let mut new_removes = vec![]; + for (i, buf) in removed_content.iter().enumerate() { + match &self.removes()[i] { + Some(TreeValue::File { id: _, executable }) => { + let file_id = store.write_file(path, &mut buf.as_slice())?; + let new_value = TreeValue::File { + id: file_id, + executable: *executable, + }; + new_removes.push(Some(new_value)); + } + None if buf.is_empty() => { + // The missing side of a conflict is still represented by + // the empty string we materialized it as + new_removes.push(None); + } + _ => { + // The user edited a non-file side. This should never happen. We consider the + // conflict resolved for now. + return Ok(None); + } + } + } + let mut new_adds = vec![]; + for (i, buf) in added_content.iter().enumerate() { + match &self.adds()[i] { + Some(TreeValue::File { id: _, executable }) => { + let file_id = store.write_file(path, &mut buf.as_slice())?; + let new_value = TreeValue::File { + id: file_id, + executable: *executable, + }; + new_adds.push(Some(new_value)); + } + None if buf.is_empty() => { + // The missing side of a conflict is still represented by + // the empty string we materialized it as => nothing to do + new_adds.push(None); + } + _ => { + // The user edited a non-file side. This should never happen. We consider the + // conflict resolved for now. + return Ok(None); + } + } + } + Ok(Some(Merge::new(new_removes, new_adds))) + } +} + +impl Merge> { + pub fn extract_as_single_hunk(&self, store: &Store, path: &RepoPath) -> Merge { + self.map(|term| get_file_contents(store, path, term)) + } +} + +impl Merge> +where + T: Borrow, +{ + /// If every non-`None` term of a `Merge>` + /// is a `TreeValue::Tree`, this converts it to + /// a `Merge`, with empty trees instead of + /// any `None` terms. Otherwise, returns `None`. + pub fn to_tree_merge( + &self, + store: &Arc, + dir: &RepoPath, + ) -> Result>, BackendError> { + let tree_id_merge = self.maybe_map(|term| match term { + None => Some(None), + Some(value) => { + if let TreeValue::Tree(id) = value.borrow() { + Some(Some(id)) + } else { + None + } + } + }); + if let Some(tree_id_merge) = tree_id_merge { + let get_tree = |id: &Option<&TreeId>| -> Result { + if let Some(id) = id { + store.get_tree(dir, id) + } else { + Ok(Tree::null(store.clone(), dir.clone())) + } + }; + Ok(Some(tree_id_merge.try_map(get_tree)?)) + } else { + Ok(None) + } + } +} + +fn describe_conflict_term(value: &TreeValue) -> String { + match value { + TreeValue::File { + id, + executable: false, + } => { + format!("file with id {}", id.hex()) + } + TreeValue::File { + id, + executable: true, + } => { + format!("executable file with id {}", id.hex()) + } + TreeValue::Symlink(id) => { + format!("symlink with id {}", id.hex()) + } + TreeValue::Tree(id) => { + format!("tree with id {}", id.hex()) + } + TreeValue::GitSubmodule(id) => { + format!("Git submodule with id {}", id.hex()) + } + TreeValue::Conflict(id) => { + format!("Conflict with id {}", id.hex()) + } + } +} + +fn get_file_contents(store: &Store, path: &RepoPath, term: &Option) -> ContentHunk { + match term { + Some(id) => { + let mut content = vec![]; + store + .read_file(path, id) + .unwrap() + .read_to_end(&mut content) + .unwrap(); + ContentHunk(content) + } + // If the conflict had removed the file on one side, we pretend that the file + // was empty there. + None => ContentHunk(vec![]), + } +} diff --git a/lib/src/merged_tree.rs b/lib/src/merged_tree.rs index ce718ba09..a5fc05e86 100644 --- a/lib/src/merged_tree.rs +++ b/lib/src/merged_tree.rs @@ -22,7 +22,7 @@ use itertools::Itertools; use crate::backend; use crate::backend::{ConflictId, TreeValue}; -use crate::conflicts::Merge; +use crate::merge::Merge; use crate::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin}; use crate::store::Store; use crate::tree::{try_resolve_file_conflict, Tree, TreeMergeError}; diff --git a/lib/src/op_store.rs b/lib/src/op_store.rs index d186200c3..16c0ab9ee 100644 --- a/lib/src/op_store.rs +++ b/lib/src/op_store.rs @@ -21,7 +21,7 @@ use once_cell::sync::Lazy; use thiserror::Error; use crate::backend::{id_type, CommitId, ObjectId, Timestamp}; -use crate::conflicts::Merge; +use crate::merge::Merge; content_hash! { #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Hash)] diff --git a/lib/src/refs.rs b/lib/src/refs.rs index a4b6ef685..eaa2d9a9f 100644 --- a/lib/src/refs.rs +++ b/lib/src/refs.rs @@ -15,9 +15,8 @@ #![allow(missing_docs)] use crate::backend::CommitId; -use crate::conflicts::Merge; use crate::index::Index; -use crate::merge::trivial_merge; +use crate::merge::{trivial_merge, Merge}; use crate::op_store::{BranchTarget, RefTarget, RefTargetOptionExt}; pub fn merge_ref_targets( diff --git a/lib/src/simple_op_store.rs b/lib/src/simple_op_store.rs index ec83f740e..e14444379 100644 --- a/lib/src/simple_op_store.rs +++ b/lib/src/simple_op_store.rs @@ -25,9 +25,9 @@ use tempfile::{NamedTempFile, PersistError}; use thiserror::Error; use crate::backend::{CommitId, MillisSinceEpoch, ObjectId, Timestamp}; -use crate::conflicts::Merge; use crate::content_hash::blake2b_hash; use crate::file_util::persist_content_addressed_temp_file; +use crate::merge::Merge; use crate::op_store::{ BranchTarget, OpStore, OpStoreError, OpStoreResult, Operation, OperationId, OperationMetadata, RefTarget, View, ViewId, WorkspaceId, diff --git a/lib/src/store.rs b/lib/src/store.rs index 724c61aaf..5339041a5 100644 --- a/lib/src/store.rs +++ b/lib/src/store.rs @@ -24,7 +24,7 @@ use crate::backend::{ Backend, BackendResult, ChangeId, CommitId, ConflictId, FileId, SymlinkId, TreeId, TreeValue, }; use crate::commit::Commit; -use crate::conflicts::Merge; +use crate::merge::Merge; use crate::repo_path::RepoPath; use crate::tree::Tree; use crate::tree_builder::TreeBuilder; diff --git a/lib/src/tree.rs b/lib/src/tree.rs index bfd934495..a02374db7 100644 --- a/lib/src/tree.rs +++ b/lib/src/tree.rs @@ -27,10 +27,9 @@ use crate::backend::{ BackendError, ConflictId, FileId, ObjectId, TreeEntriesNonRecursiveIterator, TreeEntry, TreeId, TreeValue, }; -use crate::conflicts::Merge; use crate::files::MergeResult; use crate::matchers::{EverythingMatcher, Matcher}; -use crate::merge::trivial_merge; +use crate::merge::{trivial_merge, Merge}; use crate::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin}; use crate::store::Store; use crate::{backend, files}; diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index 23f89680a..08d7cc233 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -13,7 +13,8 @@ // limitations under the License. use jj_lib::backend::{FileId, TreeValue}; -use jj_lib::conflicts::{parse_conflict, Merge}; +use jj_lib::conflicts::parse_conflict; +use jj_lib::merge::Merge; use jj_lib::repo::Repo; use jj_lib::repo_path::RepoPath; use jj_lib::store::Store; diff --git a/lib/tests/test_merged_tree.rs b/lib/tests/test_merged_tree.rs index b2822aa9b..9516883cb 100644 --- a/lib/tests/test_merged_tree.rs +++ b/lib/tests/test_merged_tree.rs @@ -14,7 +14,7 @@ use itertools::Itertools; use jj_lib::backend::{FileId, TreeValue}; -use jj_lib::conflicts::Merge; +use jj_lib::merge::Merge; use jj_lib::merged_tree::{MergedTree, MergedTreeValue}; use jj_lib::repo::Repo; use jj_lib::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin}; diff --git a/lib/tests/test_refs.rs b/lib/tests/test_refs.rs index 7c5ea4cb0..3f787d6a9 100644 --- a/lib/tests/test_refs.rs +++ b/lib/tests/test_refs.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use jj_lib::conflicts::Merge; +use jj_lib::merge::Merge; use jj_lib::op_store::RefTarget; use jj_lib::refs::merge_ref_targets; use jj_lib::repo::Repo; diff --git a/lib/tests/test_working_copy.rs b/lib/tests/test_working_copy.rs index 34a7cb10e..d29cde0a3 100644 --- a/lib/tests/test_working_copy.rs +++ b/lib/tests/test_working_copy.rs @@ -22,8 +22,8 @@ use std::sync::Arc; use itertools::Itertools; use jj_lib::backend::{ObjectId, TreeId, TreeValue}; -use jj_lib::conflicts::Merge; use jj_lib::fsmonitor::FsmonitorKind; +use jj_lib::merge::Merge; use jj_lib::op_store::{OperationId, WorkspaceId}; use jj_lib::repo::{ReadonlyRepo, Repo}; use jj_lib::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin};