diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index fada8a3f1..aae2b1ce1 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -51,7 +51,7 @@ use jj_lib::settings::UserSettings; use jj_lib::tree::{merge_trees, Tree}; use jj_lib::working_copy::SnapshotOptions; use jj_lib::workspace::Workspace; -use jj_lib::{file_util, revset}; +use jj_lib::{conflicts, file_util, revset}; use maplit::{hashmap, hashset}; use tracing::instrument; @@ -1449,9 +1449,7 @@ fn cmd_cat(ui: &mut Ui, command: &CommandHelper, args: &CatArgs) -> Result<(), C Some(TreeValue::Conflict(id)) => { let conflict = repo.store().read_conflict(&path, &id)?; let mut contents = vec![]; - conflict - .materialize(repo.store(), &path, &mut contents) - .unwrap(); + conflicts::materialize(&conflict, repo.store(), &path, &mut contents).unwrap(); ui.request_pager(); ui.stdout_formatter().write_all(&contents)?; } diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index d78e79bae..c67f164e8 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -27,7 +27,7 @@ use jj_lib::repo::{ReadonlyRepo, Repo}; use jj_lib::repo_path::RepoPath; use jj_lib::settings::{ConfigResultExt as _, UserSettings}; use jj_lib::tree::{Tree, TreeDiffIterator}; -use jj_lib::{diff, files, rewrite, tree}; +use jj_lib::{conflicts, diff, files, rewrite, tree}; use tracing::instrument; use crate::cli_util::{CommandError, WorkspaceCommandHelper}; @@ -345,9 +345,7 @@ fn diff_content( TreeValue::Conflict(id) => { let conflict = repo.store().read_conflict(path, id).unwrap(); let mut content = vec![]; - conflict - .materialize(repo.store(), path, &mut content) - .unwrap(); + conflicts::materialize(&conflict, repo.store(), path, &mut content).unwrap(); Ok(content) } } @@ -496,9 +494,7 @@ fn git_diff_part( mode = "100644".to_string(); hash = id.hex(); let conflict = repo.store().read_conflict(path, id).unwrap(); - conflict - .materialize(repo.store(), path, &mut content) - .unwrap(); + conflicts::materialize(&conflict, repo.store(), path, &mut content).unwrap(); } } let hash = hash[0..10].to_string(); diff --git a/cli/src/merge_tools.rs b/cli/src/merge_tools.rs index 42d67f7e0..ba4d24938 100644 --- a/cli/src/merge_tools.rs +++ b/cli/src/merge_tools.rs @@ -22,7 +22,8 @@ use std::sync::Arc; use config::ConfigError; use itertools::Itertools; use jj_lib::backend::{TreeId, TreeValue}; -use jj_lib::conflicts::materialize_merge_result; +use jj_lib::conflicts; +use jj_lib::conflicts::{extract_as_single_hunk, materialize_merge_result}; use jj_lib::gitignore::GitIgnoreFile; use jj_lib::matchers::{EverythingMatcher, Matcher}; use jj_lib::repo_path::RepoPath; @@ -256,7 +257,7 @@ pub fn run_mergetool( sides: file_merge.adds().len(), }); }; - let content = file_merge.extract_as_single_hunk(tree.store(), repo_path); + let content = extract_as_single_hunk(&file_merge, tree.store(), repo_path); let editor = get_merge_tool_from_settings(ui, settings)?; let initial_output_content: Vec = if editor.merge_tool_edits_conflict_markers { @@ -324,7 +325,8 @@ pub fn run_mergetool( let mut new_tree_value: Option = None; if editor.merge_tool_edits_conflict_markers { - if let Some(new_conflict) = conflict.update_from_content( + if let Some(new_conflict) = conflicts::update_from_content( + &conflict, tree.store(), repo_path, output_file_contents.as_slice(), diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index 6d1257ed4..95fc1a83c 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -18,10 +18,13 @@ use std::io::Write; use itertools::Itertools; +use crate::backend::{BackendResult, FileId, TreeValue}; use crate::diff::{find_line_ranges, Diff, DiffHunk}; use crate::files; use crate::files::{ContentHunk, MergeResult}; use crate::merge::Merge; +use crate::repo_path::RepoPath; +use crate::store::Store; const CONFLICT_START_LINE: &[u8] = b"<<<<<<<\n"; const CONFLICT_END_LINE: &[u8] = b">>>>>>>\n"; @@ -53,6 +56,47 @@ fn write_diff_hunks(hunks: &[DiffHunk], file: &mut dyn Write) -> std::io::Result Ok(()) } +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![]), + } +} + +pub fn extract_as_single_hunk( + merge: &Merge>, + store: &Store, + path: &RepoPath, +) -> Merge { + merge.map(|term| get_file_contents(store, path, term)) +} + +pub fn materialize( + conflict: &Merge>, + store: &Store, + path: &RepoPath, + output: &mut dyn Write, +) -> std::io::Result<()> { + if let Some(file_merge) = conflict.to_file_merge() { + let content = extract_as_single_hunk(&file_merge, 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. + conflict.describe(output) + } +} + pub fn materialize_merge_result( single_hunk: &Merge, output: &mut dyn Write, @@ -249,3 +293,102 @@ fn parse_conflict_hunk(input: &[u8]) -> Merge { Merge::new(removes, adds) } + +/// Returns `None` if there are no conflict markers in `content`. +pub fn update_from_content( + conflict: &Merge>, + 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()); + materialize(conflict, store, path, &mut old_content).unwrap(); + if content == old_content { + return Ok(Some(conflict.clone())); + } + + let mut removed_content = vec![vec![]; conflict.removes().len()]; + let mut added_content = vec![vec![]; conflict.adds().len()]; + let Some(hunks) = parse_conflict(content, conflict.removes().len(), conflict.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 &conflict.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 &conflict.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))) +} diff --git a/lib/src/merge.rs b/lib/src/merge.rs index 7721b075d..586cffd7e 100644 --- a/lib/src/merge.rs +++ b/lib/src/merge.rs @@ -22,13 +22,12 @@ use std::sync::Arc; use itertools::Itertools; -use crate::backend::{BackendError, BackendResult, FileId, ObjectId, TreeId, TreeValue}; +use crate::backend; +use crate::backend::{BackendError, 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. @@ -336,22 +335,6 @@ impl Merge> { 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), @@ -374,112 +357,6 @@ impl Merge> { } 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> @@ -549,23 +426,6 @@ fn describe_conflict_term(value: &TreeValue) -> String { } } -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![]), - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index 29676d52e..d7e0bead2 100644 --- a/lib/src/working_copy.rs +++ b/lib/src/working_copy.rs @@ -42,6 +42,7 @@ use tracing::{instrument, trace_span}; use crate::backend::{ BackendError, ConflictId, FileId, MillisSinceEpoch, ObjectId, SymlinkId, TreeId, TreeValue, }; +use crate::conflicts; #[cfg(feature = "watchman")] use crate::fsmonitor::watchman; use crate::fsmonitor::FsmonitorKind; @@ -967,9 +968,9 @@ impl TreeState { let mut content = vec![]; file.read_to_end(&mut content).unwrap(); let conflict = self.store.read_conflict(repo_path, &conflict_id)?; - if let Some(new_conflict) = conflict - .update_from_content(self.store.as_ref(), repo_path, &content) - .unwrap() + if let Some(new_conflict) = + conflicts::update_from_content(&conflict, self.store.as_ref(), repo_path, &content) + .unwrap() { if new_conflict != conflict { let new_conflict_id = self.store.write_conflict(repo_path, &new_conflict)?; @@ -1109,8 +1110,7 @@ impl TreeState { err, })?; let mut conflict_data = vec![]; - conflict - .materialize(self.store.as_ref(), path, &mut conflict_data) + conflicts::materialize(&conflict, self.store.as_ref(), path, &mut conflict_data) .expect("Failed to materialize conflict to in-memory buffer"); file.write_all(&conflict_data) .map_err(|err| CheckoutError::IoError { diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index 08d7cc233..6e1ef9a09 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -13,7 +13,7 @@ // limitations under the License. use jj_lib::backend::{FileId, TreeValue}; -use jj_lib::conflicts::parse_conflict; +use jj_lib::conflicts::{materialize, parse_conflict, update_from_content}; use jj_lib::merge::Merge; use jj_lib::repo::Repo; use jj_lib::repo_path::RepoPath; @@ -289,7 +289,7 @@ line 5 right vec![Some(file_value(&left_id)), Some(file_value(&right_id))], ); let mut result: Vec = vec![]; - conflict.materialize(store, &path, &mut result).unwrap(); + materialize(&conflict, store, &path, &mut result).unwrap(); insta::assert_snapshot!( String::from_utf8(result.clone()).unwrap(), @r###" @@ -662,28 +662,23 @@ fn test_update_conflict_from_content() { // If the content is unchanged compared to the materialized value, we get the // old conflict id back. let mut materialized = vec![]; - conflict - .materialize(store, &path, &mut materialized) - .unwrap(); - let result = conflict - .update_from_content(store, &path, &materialized) - .unwrap(); + materialize(&conflict, store, &path, &mut materialized).unwrap(); + let result = update_from_content(&conflict, store, &path, &materialized).unwrap(); assert_eq!(result, Some(conflict.clone())); // If the conflict is resolved, we get None back to indicate that. - let result = conflict - .update_from_content(store, &path, b"resolved 1\nline 2\nresolved 3\n") - .unwrap(); + let result = + update_from_content(&conflict, store, &path, b"resolved 1\nline 2\nresolved 3\n").unwrap(); assert_eq!(result, None); // If the conflict is partially resolved, we get a new conflict back. - let result = conflict - .update_from_content( - store, - &path, - b"resolved 1\nline 2\n<<<<<<<\n%%%%%%%\n-line 3\n+left 3\n+++++++\nright 3\n>>>>>>>\n", - ) - .unwrap(); + let result = update_from_content( + &conflict, + store, + &path, + b"resolved 1\nline 2\n<<<<<<<\n%%%%%%%\n-line 3\n+left 3\n+++++++\nright 3\n>>>>>>>\n", + ) + .unwrap(); let new_conflict = result.unwrap(); assert_ne!(new_conflict, conflict); // Calculate expected new FileIds @@ -718,22 +713,16 @@ fn test_update_conflict_from_content_modify_delete() { // If the content is unchanged compared to the materialized value, we get the // old conflict id back. let mut materialized = vec![]; - conflict - .materialize(store, &path, &mut materialized) - .unwrap(); - let result = conflict - .update_from_content(store, &path, &materialized) - .unwrap(); + materialize(&conflict, store, &path, &mut materialized).unwrap(); + let result = update_from_content(&conflict, store, &path, &materialized).unwrap(); assert_eq!(result, Some(conflict.clone())); // If the conflict is resolved, we get None back to indicate that. - let result = conflict - .update_from_content(store, &path, b"resolved\n") - .unwrap(); + let result = update_from_content(&conflict, store, &path, b"resolved\n").unwrap(); assert_eq!(result, None); // If the conflict is modified, we get a new conflict back. - let result = conflict.update_from_content( + let result = update_from_content(&conflict, store, &path, b"<<<<<<<\n%%%%%%%\n line 1\n-line 2 before\n+line 2 modified after\n line 3\n+++++++\n>>>>>>>\n", @@ -760,6 +749,6 @@ fn materialize_conflict_string( conflict: &Merge>, ) -> String { let mut result: Vec = vec![]; - conflict.materialize(store, path, &mut result).unwrap(); + materialize(conflict, store, path, &mut result).unwrap(); String::from_utf8(result).unwrap() }