Skip to content

Commit

Permalink
conflicts: pass around conflict contents without materialization
Browse files Browse the repository at this point in the history
We haven't decided how conflict diffs should be rendered, but whatever style
we'll choose, we'll need raw unmaterialized conflict contents.
  • Loading branch information
yuja committed Aug 28, 2024
1 parent 73a8b13 commit 9c9e564
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 14 deletions.
3 changes: 2 additions & 1 deletion cli/src/commands/file/show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -120,7 +121,7 @@ fn write_tree_entries<P: AsRef<RepoPath>>(
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())?;
Expand Down
19 changes: 14 additions & 5 deletions cli/src/diff_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -718,10 +719,15 @@ fn diff_content(path: &RepoPath, value: MaterializedTreeValue) -> io::Result<Fil
id: _,
contents,
executable: _,
} => 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(),
Expand Down Expand Up @@ -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 } => {
Expand Down
6 changes: 5 additions & 1 deletion cli/src/merge_tools/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 4 additions & 5 deletions lib/src/conflicts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,9 @@ pub enum MaterializedTreeValue {
},
FileConflict {
id: Merge<Option<FileId>>,
contents: Vec<u8>,
// TODO: or Vec<(FileId, Box<dyn Read>)> so that caller can stop reading
// when null bytes found?
contents: Merge<BString>,
executable: bool,
},
OtherConflict {
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 7 additions & 1 deletion lib/src/default_index/revset_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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:?}");
Expand Down
8 changes: 7 additions & 1 deletion lib/src/local_working_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 9c9e564

Please sign in to comment.