ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

conflicts: replace remaining uses of backend::Conflict

Browse source
This commit is contained in:
Martin von Zweigbergk 2023-05-31 13:55:17 -07:00 committed by Martin von Zweigbergk
parent 9907cccf37
commit c378503991
5 changed files with 112 additions and 89 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
lib/src/conflicts.rs
View file

@ -40,6 +40,11 @@ pub struct Conflict<T> {
}
impl<T> Conflict<T> {
pub fn new(removes: Vec<T>, adds: Vec<T>) -> Self {
assert_eq!(adds.len(), removes.len() + 1);
Conflict { removes, adds }
}
pub fn removes(&self) -> &[T] {
&self.removes
}
@ -208,15 +213,14 @@ fn write_diff_hunks(hunks: &[DiffHunk], file: &mut dyn Write) -> std::io::Result
pub fn materialize_conflict(
store: &Store,
path: &RepoPath,
conflict: &backend::Conflict,
conflict: &Conflict<Option<TreeValue>>,
output: &mut dyn Write,
) -> std::io::Result<()> {
let conflict = Conflict::from_backend_conflict(conflict);
match extract_file_conflict_as_single_hunk(store, path, &conflict) {
match extract_file_conflict_as_single_hunk(store, path, conflict) {
None => {
// Unless all terms are regular files, we can't do much better than to try to
// describe the conflict.
describe_conflict(&conflict, output)
describe_conflict(conflict, output)
}
Some(content) => materialize_merge_result(&content, output),
}
@ -446,6 +450,10 @@ pub fn update_conflict_from_content(
content: &[u8],
) -> BackendResult<Option<ConflictId>> {
let conflict = store.read_conflict(path, conflict_id)?;
let mut conflict = Conflict::from_backend_conflict(&conflict);
// TODO: Check that the conflict only involves files and convert it to a
// `Conflict<Option<FileId>>` 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
@ -458,10 +466,6 @@ pub fn update_conflict_from_content(
return Ok(Some(conflict_id.clone()));
}
let mut conflict = Conflict::from_backend_conflict(&conflict);
// TODO: Check that the conflict only involves files and convert it to a
// `Conflict<Option<FileId>>` so we can remove the wildcard pattern in the loops
// further down.
let mut removed_content = vec![vec![]; conflict.removes().len()];
let mut added_content = vec![vec![]; conflict.adds().len()];
// TODO: Change to let-else once our MSRV is above 1.65

3
lib/src/working_copy.rs
View file

@ -34,7 +34,7 @@ use thiserror::Error;
use crate::backend::{
BackendError, ConflictId, FileId, MillisSinceEpoch, ObjectId, SymlinkId, TreeId, TreeValue,
};
use crate::conflicts::{materialize_conflict, update_conflict_from_content};
use crate::conflicts::{materialize_conflict, update_conflict_from_content, Conflict};
use crate::gitignore::GitIgnoreFile;
use crate::lock::FileLock;
use crate::matchers::{DifferenceMatcher, Matcher, PrefixMatcher};
@ -777,6 +777,7 @@ impl TreeState {
) -> Result<FileState, CheckoutError> {
create_parent_dirs(&self.working_copy_path, path)?;
let conflict = self.store.read_conflict(path, id)?;
let conflict = Conflict::from_backend_conflict(&conflict);
let mut file = OpenOptions::new()
.write(true)
.create_new(true) // Don't overwrite un-ignored file. Don't follow symlink.

173
lib/tests/test_conflicts.rs
View file

@ -12,19 +12,19 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use jujutsu_lib::backend::{Conflict, ConflictTerm, FileId, TreeValue};
use jujutsu_lib::conflicts::{materialize_conflict, parse_conflict, update_conflict_from_content};
use jujutsu_lib::backend::{FileId, TreeValue};
use jujutsu_lib::conflicts::{
materialize_conflict, parse_conflict, update_conflict_from_content, Conflict,
};
use jujutsu_lib::repo::Repo;
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::store::Store;
use testutils::TestRepo;
fn file_conflict_term(file_id: &FileId) -> ConflictTerm {
ConflictTerm {
value: TreeValue::File {
id: file_id.clone(),
executable: false,
},
fn file_value(file_id: &FileId) -> TreeValue {
TreeValue::File {
id: file_id.clone(),
executable: false,
}
}
@ -69,10 +69,10 @@ line 5
// The left side should come first. The diff should be use the smaller (right)
// side, and the left side should be a snapshot.
let mut conflict = Conflict {
removes: vec![file_conflict_term(&base_id)],
adds: vec![file_conflict_term(&left_id), file_conflict_term(&right_id)],
};
let conflict = Conflict::new(
vec![Some(file_value(&base_id))],
vec![Some(file_value(&left_id)), Some(file_value(&right_id))],
);
insta::assert_snapshot!(
&materialize_conflict_string(store, &path, &conflict),
@r###"
@ -93,7 +93,10 @@ line 5
);
// Swap the positive terms in the conflict. The diff should still use the right
// side, but now the right side should come first.
conflict.adds.reverse();
let conflict = Conflict::new(
vec![Some(file_value(&base_id))],
vec![Some(file_value(&right_id)), Some(file_value(&left_id))],
);
insta::assert_snapshot!(
&materialize_conflict_string(store, &path, &conflict),
@r###"
@ -159,14 +162,14 @@ line 3
// The order of (a, b, c) should be preserved. For all cases, the "a" side
// should be a snapshot.
let conflict = Conflict {
removes: vec![file_conflict_term(&base_id), file_conflict_term(&base_id)],
adds: vec![
file_conflict_term(&a_id),
file_conflict_term(&b_id),
file_conflict_term(&c_id),
let conflict = Conflict::new(
vec![Some(file_value(&base_id)), Some(file_value(&base_id))],
vec![
Some(file_value(&a_id)),
Some(file_value(&b_id)),
Some(file_value(&c_id)),
],
};
);
insta::assert_snapshot!(
&materialize_conflict_string(store, &path, &conflict),
@r###"
@ -187,14 +190,14 @@ line 3
line 3
"###
);
let conflict = Conflict {
removes: vec![file_conflict_term(&base_id), file_conflict_term(&base_id)],
adds: vec![
file_conflict_term(&c_id),
file_conflict_term(&b_id),
file_conflict_term(&a_id),
let conflict = Conflict::new(
vec![Some(file_value(&base_id)), Some(file_value(&base_id))],
vec![
Some(file_value(&c_id)),
Some(file_value(&b_id)),
Some(file_value(&a_id)),
],
};
);
insta::assert_snapshot!(
&materialize_conflict_string(store, &path, &conflict),
@r###"
@ -215,14 +218,14 @@ line 3
line 3
"###
);
let conflict = Conflict {
removes: vec![file_conflict_term(&base_id), file_conflict_term(&base_id)],
adds: vec![
file_conflict_term(&c_id),
file_conflict_term(&a_id),
file_conflict_term(&b_id),
let conflict = Conflict::new(
vec![Some(file_value(&base_id)), Some(file_value(&base_id))],
vec![
Some(file_value(&c_id)),
Some(file_value(&a_id)),
Some(file_value(&b_id)),
],
};
);
insta::assert_snapshot!(
&materialize_conflict_string(store, &path, &conflict),
@r###"
@ -282,10 +285,10 @@ line 5 right
",
);
let conflict = Conflict {
removes: vec![file_conflict_term(&base_id)],
adds: vec![file_conflict_term(&left_id), file_conflict_term(&right_id)],
};
let conflict = Conflict::new(
vec![Some(file_value(&base_id))],
vec![Some(file_value(&left_id)), Some(file_value(&right_id))],
);
let mut result: Vec<u8> = vec![];
materialize_conflict(store, &path, &conflict, &mut result).unwrap();
insta::assert_snapshot!(
@ -315,7 +318,7 @@ line 5 right
// The first add should always be from the left side
insta::assert_debug_snapshot!(
parse_conflict(&result, conflict.removes.len(), conflict.adds.len()),
parse_conflict(&result, conflict.removes().len(), conflict.adds().len()),
@r###"
Some(
[
@ -382,13 +385,13 @@ line 5
);
// left modifies a line, right deletes the same line.
let conflict = Conflict {
removes: vec![file_conflict_term(&base_id)],
adds: vec![
file_conflict_term(&modified_id),
file_conflict_term(&deleted_id),
let conflict = Conflict::new(
vec![Some(file_value(&base_id))],
vec![
Some(file_value(&modified_id)),
Some(file_value(&deleted_id)),
],
};
);
insta::assert_snapshot!(&materialize_conflict_string(store, &path, &conflict), @r###"
line 1
line 2
@ -404,13 +407,13 @@ line 5
);
// right modifies a line, left deletes the same line.
let conflict = Conflict {
removes: vec![file_conflict_term(&base_id)],
adds: vec![
file_conflict_term(&deleted_id),
file_conflict_term(&modified_id),
let conflict = Conflict::new(
vec![Some(file_value(&base_id))],
vec![
Some(file_value(&deleted_id)),
Some(file_value(&modified_id)),
],
};
);
insta::assert_snapshot!(&materialize_conflict_string(store, &path, &conflict), @r###"
line 1
line 2
@ -426,10 +429,10 @@ line 5
);
// modify/delete conflict at the file level
let conflict = Conflict {
removes: vec![file_conflict_term(&base_id)],
adds: vec![file_conflict_term(&modified_id)],
};
let conflict = Conflict::new(
vec![Some(file_value(&base_id))],
vec![Some(file_value(&modified_id)), None],
);
insta::assert_snapshot!(&materialize_conflict_string(store, &path, &conflict), @r###"
<<<<<<<
%%%%%%%
@ -634,14 +637,16 @@ fn test_update_conflict_from_content() {
let base_file_id = testutils::write_file(store, &path, "line 1\nline 2\nline 3\n");
let left_file_id = testutils::write_file(store, &path, "left 1\nline 2\nleft 3\n");
let right_file_id = testutils::write_file(store, &path, "right 1\nline 2\nright 3\n");
let conflict = Conflict {
removes: vec![file_conflict_term(&base_file_id)],
adds: vec![
file_conflict_term(&left_file_id),
file_conflict_term(&right_file_id),
let conflict = Conflict::new(
vec![Some(file_value(&base_file_id))],
vec![
Some(file_value(&left_file_id)),
Some(file_value(&right_file_id)),
],
};
let conflict_id = store.write_conflict(&path, &conflict).unwrap();
);
let conflict_id = store
.write_conflict(&path, &conflict.to_backend_conflict())
.unwrap();
// If the content is unchanged compared to the materialized value, we get the
// old conflict id back.
@ -671,20 +676,21 @@ fn test_update_conflict_from_content() {
assert_ne!(result, None);
assert_ne!(result, Some(conflict_id));
let new_conflict = store.read_conflict(&path, &result.unwrap()).unwrap();
let new_conflict = Conflict::from_backend_conflict(&new_conflict);
// Calculate expected new FileIds
let new_base_file_id = testutils::write_file(store, &path, "resolved 1\nline 2\nline 3\n");
let new_left_file_id = testutils::write_file(store, &path, "resolved 1\nline 2\nleft 3\n");
let new_right_file_id = testutils::write_file(store, &path, "resolved 1\nline 2\nright 3\n");
assert_eq!(
new_conflict,
Conflict {
removes: vec![file_conflict_term(&new_base_file_id)],
adds: vec![
file_conflict_term(&new_left_file_id),
file_conflict_term(&new_right_file_id)
Conflict::new(
vec![Some(file_value(&new_base_file_id))],
vec![
Some(file_value(&new_left_file_id)),
Some(file_value(&new_right_file_id))
]
}
)
)
);
}
#[test]
@ -695,11 +701,13 @@ fn test_update_conflict_from_content_modify_delete() {
let path = RepoPath::from_internal_string("dir/file");
let before_file_id = testutils::write_file(store, &path, "line 1\nline 2 before\nline 3\n");
let after_file_id = testutils::write_file(store, &path, "line 1\nline 2 after\nline 3\n");
let conflict = Conflict {
removes: vec![file_conflict_term(&before_file_id)],
adds: vec![file_conflict_term(&after_file_id)],
};
let conflict_id = store.write_conflict(&path, &conflict).unwrap();
let conflict = Conflict::new(
vec![Some(file_value(&before_file_id))],
vec![Some(file_value(&after_file_id)), None],
);
let conflict_id = store
.write_conflict(&path, &conflict.to_backend_conflict())
.unwrap();
// If the content is unchanged compared to the materialized value, we get the
// old conflict id back.
@ -723,6 +731,7 @@ fn test_update_conflict_from_content_modify_delete() {
assert_ne!(result, None);
assert_ne!(result, Some(conflict_id));
let new_conflict = store.read_conflict(&path, &result.unwrap()).unwrap();
let new_conflict = Conflict::from_backend_conflict(&new_conflict);
// Calculate expected new FileIds
let new_base_file_id = testutils::write_file(store, &path, "line 1\nline 2 before\nline 3\n");
let new_left_file_id =
@ -730,14 +739,18 @@ fn test_update_conflict_from_content_modify_delete() {
assert_eq!(
new_conflict,
Conflict {
removes: vec![file_conflict_term(&new_base_file_id)],
adds: vec![file_conflict_term(&new_left_file_id)]
}
)
Conflict::new(
vec![Some(file_value(&new_base_file_id))],
vec![Some(file_value(&new_left_file_id)), None]
)
);
}
fn materialize_conflict_string(store: &Store, path: &RepoPath, conflict: &Conflict) -> String {
fn materialize_conflict_string(
store: &Store,
path: &RepoPath,
conflict: &Conflict<Option<TreeValue>>,
) -> String {
let mut result: Vec<u8> = vec![];
materialize_conflict(store, path, conflict, &mut result).unwrap();
String::from_utf8(result).unwrap()

2
src/commands/mod.rs
View file

@ -32,6 +32,7 @@ use indexmap::{IndexMap, IndexSet};
use itertools::Itertools;
use jujutsu_lib::backend::{CommitId, ObjectId, TreeValue};
use jujutsu_lib::commit::Commit;
use jujutsu_lib::conflicts::Conflict;
use jujutsu_lib::dag_walk::topo_order_reverse;
use jujutsu_lib::default_index_store::{DefaultIndexStore, ReadonlyIndexWrapper};
use jujutsu_lib::git_backend::GitBackend;
@ -1340,6 +1341,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 conflict = Conflict::from_backend_conflict(&conflict);
let mut contents = vec![];
conflicts::materialize_conflict(repo.store(), &path, &conflict, &mut contents).unwrap();
ui.request_pager();

3
src/diff_util.rs
View file

@ -20,6 +20,7 @@ use std::sync::Arc;
use itertools::Itertools;
use jujutsu_lib::backend::{ObjectId, TreeValue};
use jujutsu_lib::commit::Commit;
use jujutsu_lib::conflicts::Conflict;
use jujutsu_lib::diff::{Diff, DiffHunk};
use jujutsu_lib::files::DiffLine;
use jujutsu_lib::matchers::Matcher;
@ -306,6 +307,7 @@ fn diff_content(
}
TreeValue::Conflict(id) => {
let conflict = repo.store().read_conflict(path, id).unwrap();
let conflict = Conflict::from_backend_conflict(&conflict);
let mut content = vec![];
conflicts::materialize_conflict(repo.store(), path, &conflict, &mut content).unwrap();
Ok(content)
@ -456,6 +458,7 @@ fn git_diff_part(
mode = "100644".to_string();
hash = id.hex();
let conflict = repo.store().read_conflict(path, id).unwrap();
let conflict = Conflict::from_backend_conflict(&conflict);
conflicts::materialize_conflict(repo.store(), path, &conflict, &mut content).unwrap();
}
}