From 9c9e564dc476bb35ef8f5165382c63c361769163 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 26 Aug 2024 17:15:48 +0900 Subject: [PATCH] conflicts: pass around conflict contents without materialization We haven't decided how conflict diffs should be rendered, but whatever style we'll choose, we'll need raw unmaterialized conflict contents. --- cli/src/commands/file/show.rs | 3 ++- cli/src/diff_util.rs | 19 ++++++++++++++----- cli/src/merge_tools/builtin.rs | 6 +++++- lib/src/conflicts.rs | 9 ++++----- lib/src/default_index/revset_engine.rs | 8 +++++++- lib/src/local_working_copy.rs | 8 +++++++- 6 files changed, 39 insertions(+), 14 deletions(-) diff --git a/cli/src/commands/file/show.rs b/cli/src/commands/file/show.rs index cad828ee61..04f65be130 100644 --- a/cli/src/commands/file/show.rs +++ b/cli/src/commands/file/show.rs @@ -16,6 +16,7 @@ use std::io; use std::io::Write; use jj_lib::backend::BackendResult; +use jj_lib::conflicts::materialize_merge_result; use jj_lib::conflicts::materialize_tree_value; use jj_lib::conflicts::MaterializedTreeValue; use jj_lib::fileset::FilePattern; @@ -120,7 +121,7 @@ fn write_tree_entries>( io::copy(&mut reader, &mut ui.stdout_formatter().as_mut())?; } MaterializedTreeValue::FileConflict { contents, .. } => { - ui.stdout_formatter().write_all(&contents)?; + materialize_merge_result(&contents, &mut ui.stdout_formatter())?; } MaterializedTreeValue::OtherConflict { id } => { ui.stdout_formatter().write_all(id.describe().as_bytes())?; diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index 0c90227683..ad9671c72f 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -31,6 +31,7 @@ use jj_lib::backend::CommitId; use jj_lib::backend::CopyRecord; use jj_lib::backend::TreeValue; use jj_lib::commit::Commit; +use jj_lib::conflicts::materialize_merge_result; use jj_lib::conflicts::materialized_diff_stream; use jj_lib::conflicts::MaterializedTreeDiffEntry; use jj_lib::conflicts::MaterializedTreeValue; @@ -718,10 +719,15 @@ fn diff_content(path: &RepoPath, value: MaterializedTreeValue) -> io::Result Ok(FileContent { - is_binary: false, - contents, - }), + } => { + let mut data = vec![]; + materialize_merge_result(&contents, &mut data) + .expect("Failed to materialize conflict to in-memory buffer"); + Ok(FileContent { + is_binary: false, + contents: data, + }) + } MaterializedTreeValue::OtherConflict { id } => Ok(FileContent { is_binary: false, contents: id.describe().into_bytes(), @@ -1030,9 +1036,12 @@ fn git_diff_part( } => { mode = if executable { "100755" } else { "100644" }; hash = DUMMY_HASH.to_owned(); + let mut data = vec![]; + materialize_merge_result(&contents, &mut data) + .expect("Failed to materialize conflict to in-memory buffer"); content = FileContent { is_binary: false, // TODO: are we sure this is never binary? - contents, + contents: data, }; } MaterializedTreeValue::OtherConflict { id } => { diff --git a/cli/src/merge_tools/builtin.rs b/cli/src/merge_tools/builtin.rs index 761dc7b40c..68354cb1a7 100644 --- a/cli/src/merge_tools/builtin.rs +++ b/cli/src/merge_tools/builtin.rs @@ -12,6 +12,7 @@ use jj_lib::backend::BackendResult; use jj_lib::backend::FileId; use jj_lib::backend::MergedTreeId; use jj_lib::backend::TreeValue; +use jj_lib::conflicts::materialize_merge_result; use jj_lib::conflicts::materialize_tree_value; use jj_lib::conflicts::MaterializedTreeValue; use jj_lib::diff::Diff; @@ -210,8 +211,11 @@ fn read_file_contents( contents, executable: _, } => { + let mut buf = Vec::new(); + materialize_merge_result(&contents, &mut buf) + .expect("Failed to materialize conflict to in-memory buffer"); // TODO: Render the ID somehow? - let contents = buf_to_file_contents(None, contents); + let contents = buf_to_file_contents(None, buf); Ok(FileInfo { file_mode: scm_record::FileMode(mode::NORMAL), contents, diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index fe806a0c56..54fb6b13cf 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -147,7 +147,9 @@ pub enum MaterializedTreeValue { }, FileConflict { id: Merge>, - contents: Vec, + // TODO: or Vec<(FileId, Box)> so that caller can stop reading + // when null bytes found? + contents: Merge, executable: bool, }, OtherConflict { @@ -207,14 +209,11 @@ async fn materialize_tree_value_no_access_denied( panic!("cannot materialize legacy conflict object at path {path:?}"); } Err(conflict) => { - let mut contents = vec![]; let Some(file_merge) = conflict.to_file_merge() else { return Ok(MaterializedTreeValue::OtherConflict { id: conflict }); }; let file_merge = file_merge.simplify(); - let content = extract_as_single_hunk(&file_merge, store, path).await?; - materialize_merge_result(&content, &mut contents) - .expect("Failed to materialize conflict to in-memory buffer"); + let contents = extract_as_single_hunk(&file_merge, store, path).await?; let executable = if let Some(merge) = conflict.to_executable_merge() { merge.resolve_trivial().copied().unwrap_or_default() } else { diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index 0a38aa0e42..c1c4662248 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/lib/src/default_index/revset_engine.rs @@ -42,6 +42,7 @@ use crate::backend::ChangeId; use crate::backend::CommitId; use crate::backend::MillisSinceEpoch; use crate::commit::Commit; +use crate::conflicts::materialize_merge_result; use crate::conflicts::materialize_tree_value; use crate::conflicts::MaterializedTreeValue; use crate::default_index::AsCompositeIndex; @@ -1252,7 +1253,12 @@ fn to_file_content(path: &RepoPath, value: MaterializedTreeValue) -> BackendResu } MaterializedTreeValue::Symlink { id: _, target } => Ok(target.into_bytes()), MaterializedTreeValue::GitSubmodule(_) => Ok(vec![]), - MaterializedTreeValue::FileConflict { contents, .. } => Ok(contents), + MaterializedTreeValue::FileConflict { contents, .. } => { + let mut content = vec![]; + materialize_merge_result(&contents, &mut content) + .expect("Failed to materialize conflict to in-memory buffer"); + Ok(content) + } MaterializedTreeValue::OtherConflict { .. } => Ok(vec![]), MaterializedTreeValue::Tree(id) => { panic!("Unexpected tree with id {id:?} in diff at path {path:?}"); diff --git a/lib/src/local_working_copy.rs b/lib/src/local_working_copy.rs index dd9edf7d86..a60fec77cc 100644 --- a/lib/src/local_working_copy.rs +++ b/lib/src/local_working_copy.rs @@ -60,6 +60,7 @@ use crate::backend::TreeId; use crate::backend::TreeValue; use crate::commit::Commit; use crate::conflicts; +use crate::conflicts::materialize_merge_result; use crate::conflicts::materialize_tree_value; use crate::conflicts::MaterializedTreeValue; use crate::file_util::check_symlink_support; @@ -1461,7 +1462,12 @@ impl TreeState { id: _, contents, executable, - } => self.write_conflict(&disk_path, contents, executable)?, + } => { + let mut data = vec![]; + materialize_merge_result(&contents, &mut data) + .expect("Failed to materialize conflict to in-memory buffer"); + self.write_conflict(&disk_path, data, executable)? + } MaterializedTreeValue::OtherConflict { id } => { // Unless all terms are regular files, we can't do much // better than trying to describe the merge.