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

backend: rename ConflictPart to ConflictTerm

Browse source 
It took a while before I realized that conflicts could be modeled as
simple algebraic expressions with positive and negative terms (they
were modeled as recursive 3-way conflicts initially). We've been
thinking of them that way for a while now, so let's make the
`ConflictPart` name match that model.
This commit is contained in:
Martin von Zweigbergk 2023-02-17 14:34:41 -08:00 committed by Martin von Zweigbergk
parent e48ace56d1
commit a87125d08b
10 changed files with 119 additions and 119 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
lib/src/backend.rs
View file

@ -152,7 +152,7 @@ content_hash! {
content_hash! {
#[derive(Debug, PartialEq, Eq, Clone)]
pub struct ConflictPart {
pub struct ConflictTerm {
// TODO: Store e.g. CommitId here too? Labels (theirs/ours/base)? Would those still be
// useful e.g. after rebasing this conflict?
pub value: TreeValue,
@ -166,8 +166,8 @@ content_hash! {
// In a simple 3-way merge of B and C with merge base A, the conflict will be { add: [B, C],
// remove: [A] }. Also note that a conflict of the form { add: [A], remove: [] } is the
// same as non-conflict A.
pub removes: Vec<ConflictPart>,
pub adds: Vec<ConflictPart>,
pub removes: Vec<ConflictTerm>,
pub adds: Vec<ConflictTerm>,
}
}

40
lib/src/conflicts.rs
View file

@ -17,7 +17,7 @@ use std::io::{Cursor, Write};
use itertools::Itertools;
use crate::backend::{BackendResult, Conflict, ConflictId, ConflictPart, ObjectId, TreeValue};
use crate::backend::{BackendResult, Conflict, ConflictId, ConflictTerm, ObjectId, TreeValue};
use crate::diff::{find_line_ranges, Diff, DiffHunk};
use crate::files;
use crate::files::{ConflictHunk, MergeHunk, MergeResult};
@ -30,8 +30,8 @@ const CONFLICT_DIFF_LINE: &[u8] = b"%%%%%%%\n";
const CONFLICT_MINUS_LINE: &[u8] = b"-------\n";
const CONFLICT_PLUS_LINE: &[u8] = b"+++++++\n";
fn describe_conflict_part(part: &ConflictPart) -> String {
match &part.value {
fn describe_conflict_term(term: &ConflictTerm) -> String {
match &term.value {
TreeValue::File {
id,
executable: false,
@ -62,21 +62,21 @@ fn describe_conflict_part(part: &ConflictPart) -> String {
/// Give a summary description of a conflict's "adds" and "removes"
pub fn describe_conflict(conflict: &Conflict, file: &mut dyn Write) -> std::io::Result<()> {
file.write_all(b"Conflict:\n")?;
for part in &conflict.removes {
file.write_all(format!(" Removing {}\n", describe_conflict_part(part)).as_bytes())?;
for term in &conflict.removes {
file.write_all(format!(" Removing {}\n", describe_conflict_term(term)).as_bytes())?;
}
for part in &conflict.adds {
file.write_all(format!(" Adding {}\n", describe_conflict_part(part)).as_bytes())?;
for term in &conflict.adds {
file.write_all(format!(" Adding {}\n", describe_conflict_term(term)).as_bytes())?;
}
Ok(())
}
fn file_parts(parts: &[ConflictPart]) -> Vec<&ConflictPart> {
parts
fn file_terms(terms: &[ConflictTerm]) -> Vec<&ConflictTerm> {
terms
.iter()
.filter(|part| {
.filter(|term| {
matches!(
part.value,
term.value,
TreeValue::File {
executable: false,
..
@ -86,11 +86,11 @@ fn file_parts(parts: &[ConflictPart]) -> Vec<&ConflictPart> {
.collect_vec()
}
fn get_file_contents(store: &Store, path: &RepoPath, part: &ConflictPart) -> Vec<u8> {
fn get_file_contents(store: &Store, path: &RepoPath, term: &ConflictTerm) -> Vec<u8> {
if let TreeValue::File {
id,
executable: false,
} = &part.value
} = &term.value
{
let mut content: Vec<u8> = vec![];
store
@ -100,7 +100,7 @@ fn get_file_contents(store: &Store, path: &RepoPath, part: &ConflictPart) -> Vec
.unwrap();
content
} else {
panic!("unexpectedly got a non-file conflict part");
panic!("unexpectedly got a non-file conflict term");
}
}
@ -136,7 +136,7 @@ pub fn materialize_conflict(
) -> std::io::Result<()> {
match extract_file_conflict_as_single_hunk(store, path, conflict) {
None => {
// Unless all parts are regular files, we can't do much better than to try to
// Unless all terms are regular files, we can't do much better than to try to
// describe the conflict.
describe_conflict(conflict, output)
}
@ -144,24 +144,24 @@ pub fn materialize_conflict(
}
}
/// Only works if all parts of the conflict are regular, non-executable files
/// Only works if all terms of the conflict are regular, non-executable files
pub fn extract_file_conflict_as_single_hunk(
store: &Store,
path: &RepoPath,
conflict: &Conflict,
) -> Option<ConflictHunk> {
let file_adds = file_parts(&conflict.adds);
let file_removes = file_parts(&conflict.removes);
let file_adds = file_terms(&conflict.adds);
let file_removes = file_terms(&conflict.removes);
if file_adds.len() != conflict.adds.len() || file_removes.len() != conflict.removes.len() {
return None;
}
let mut added_content = file_adds
.iter()
.map(|part| get_file_contents(store, path, part))
.map(|term| get_file_contents(store, path, term))
.collect_vec();
let mut removed_content = file_removes
.iter()
.map(|part| get_file_contents(store, path, part))
.map(|term| get_file_contents(store, path, term))
.collect_vec();
// If the conflict had removed the file on one side, we pretend that the file
// was empty there.

24
lib/src/git_backend.rs
View file

@ -24,7 +24,7 @@ use prost::Message;
use crate::backend::{
make_root_commit, Backend, BackendError, BackendResult, ChangeId, Commit, CommitId, Conflict,
ConflictId, ConflictPart, FileId, MillisSinceEpoch, ObjectId, Signature, SymlinkId, Timestamp,
ConflictId, ConflictTerm, FileId, MillisSinceEpoch, ObjectId, Signature, SymlinkId, Timestamp,
Tree, TreeId, TreeValue,
};
use crate::repo_path::{RepoPath, RepoPathComponent};
@ -370,15 +370,15 @@ impl Backend for GitBackend {
file.read_to_string(&mut data)?;
let json: serde_json::Value = serde_json::from_str(&data).unwrap();
Ok(Conflict {
removes: conflict_part_list_from_json(json.get("removes").unwrap()),
adds: conflict_part_list_from_json(json.get("adds").unwrap()),
removes: conflict_term_list_from_json(json.get("removes").unwrap()),
adds: conflict_term_list_from_json(json.get("adds").unwrap()),
})
}
fn write_conflict(&self, _path: &RepoPath, conflict: &Conflict) -> BackendResult<ConflictId> {
let json = serde_json::json!({
"removes": conflict_part_list_to_json(&conflict.removes),
"adds": conflict_part_list_to_json(&conflict.adds),
"removes": conflict_term_list_to_json(&conflict.removes),
"adds": conflict_term_list_to_json(&conflict.adds),
});
let json_string = json.to_string();
let bytes = json_string.as_bytes();
@ -548,27 +548,27 @@ impl Backend for GitBackend {
}
}
fn conflict_part_list_to_json(parts: &[ConflictPart]) -> serde_json::Value {
serde_json::Value::Array(parts.iter().map(conflict_part_to_json).collect())
fn conflict_term_list_to_json(parts: &[ConflictTerm]) -> serde_json::Value {
serde_json::Value::Array(parts.iter().map(conflict_term_to_json).collect())
}
fn conflict_part_list_from_json(json: &serde_json::Value) -> Vec<ConflictPart> {
fn conflict_term_list_from_json(json: &serde_json::Value) -> Vec<ConflictTerm> {
json.as_array()
.unwrap()
.iter()
.map(conflict_part_from_json)
.map(conflict_term_from_json)
.collect()
}
fn conflict_part_to_json(part: &ConflictPart) -> serde_json::Value {
fn conflict_term_to_json(part: &ConflictTerm) -> serde_json::Value {
serde_json::json!({
"value": tree_value_to_json(&part.value),
})
}
fn conflict_part_from_json(json: &serde_json::Value) -> ConflictPart {
fn conflict_term_from_json(json: &serde_json::Value) -> ConflictTerm {
let json_value = json.get("value").unwrap();
ConflictPart {
ConflictTerm {
value: tree_value_from_json(json_value),
}
}

26
lib/src/local_backend.rs
View file

@ -24,7 +24,7 @@ use tempfile::{NamedTempFile, PersistError};
use crate::backend::{
make_root_commit, Backend, BackendError, BackendResult, ChangeId, Commit, CommitId, Conflict,
ConflictId, ConflictPart, FileId, MillisSinceEpoch, ObjectId, Signature, SymlinkId, Timestamp,
ConflictId, ConflictTerm, FileId, MillisSinceEpoch, ObjectId, Signature, SymlinkId, Timestamp,
Tree, TreeId, TreeValue,
};
use crate::content_hash::blake2b_hash;
@ -401,34 +401,34 @@ fn signature_from_proto(proto: crate::protos::store::commit::Signature) -> Signa
fn conflict_to_proto(conflict: &Conflict) -> crate::protos::store::Conflict {
let mut proto = crate::protos::store::Conflict::default();
for part in &conflict.adds {
proto.adds.push(conflict_part_to_proto(part));
for term in &conflict.adds {
proto.adds.push(conflict_term_to_proto(term));
}
for part in &conflict.removes {
proto.removes.push(conflict_part_to_proto(part));
for term in &conflict.removes {
proto.removes.push(conflict_term_to_proto(term));
}
proto
}
fn conflict_from_proto(proto: crate::protos::store::Conflict) -> Conflict {
let mut conflict = Conflict::default();
for part in proto.removes {
conflict.removes.push(conflict_part_from_proto(part))
for term in proto.removes {
conflict.removes.push(conflict_term_from_proto(term))
}
for part in proto.adds {
conflict.adds.push(conflict_part_from_proto(part))
for term in proto.adds {
conflict.adds.push(conflict_term_from_proto(term))
}
conflict
}
fn conflict_part_from_proto(proto: crate::protos::store::conflict::Part) -> ConflictPart {
ConflictPart {
fn conflict_term_from_proto(proto: crate::protos::store::conflict::Term) -> ConflictTerm {
ConflictTerm {
value: tree_value_from_proto(proto.content.unwrap()),
}
}
fn conflict_part_to_proto(part: &ConflictPart) -> crate::protos::store::conflict::Part {
crate::protos::store::conflict::Part {
fn conflict_term_to_proto(part: &ConflictTerm) -> crate::protos::store::conflict::Term {
crate::protos::store::conflict::Term {
content: Some(tree_value_to_proto(&part.value)),
}
}

6
lib/src/protos/store.proto
View file

@ -63,10 +63,10 @@ message Commit {
}
message Conflict {
message Part {
message Term {
TreeValue content = 1;
}
repeated Part removes = 1;
repeated Part adds = 2;
repeated Term removes = 1;
repeated Term adds = 2;
}

6
lib/src/protos/store.rs
View file

@ -93,15 +93,15 @@ pub mod commit {
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct Conflict {
#[prost(message, repeated, tag = "1")]
pub removes: ::prost::alloc::vec::Vec<conflict::Part>,
pub removes: ::prost::alloc::vec::Vec<conflict::Term>,
#[prost(message, repeated, tag = "2")]
pub adds: ::prost::alloc::vec::Vec<conflict::Part>,
pub adds: ::prost::alloc::vec::Vec<conflict::Term>,
}
/// Nested message and enum types in `Conflict`.
pub mod conflict {
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct Part {
pub struct Term {
#[prost(message, optional, tag = "1")]
pub content: ::core::option::Option<super::TreeValue>,
}

40
lib/src/tree.rs
View file

@ -23,7 +23,7 @@ use itertools::Itertools;
use thiserror::Error;
use crate::backend::{
BackendError, Conflict, ConflictId, ConflictPart, FileId, ObjectId,
BackendError, Conflict, ConflictId, ConflictTerm, FileId, ObjectId,
TreeEntriesNonRecursiveIterator, TreeEntry, TreeId, TreeValue,
};
use crate::files::MergeResult;
@ -611,17 +611,17 @@ fn merge_tree_value(
// resolved state, the absence of a state, or a conflicted state.
let mut conflict = Conflict::default();
if let Some(base) = maybe_base {
conflict.removes.push(ConflictPart {
conflict.removes.push(ConflictTerm {
value: base.clone(),
});
}
if let Some(side1) = maybe_side1 {
conflict.adds.push(ConflictPart {
conflict.adds.push(ConflictTerm {
value: side1.clone(),
});
}
if let Some(side2) = maybe_side2 {
conflict.adds.push(ConflictPart {
conflict.adds.push(ConflictTerm {
value: side2.clone(),
});
}
@ -667,8 +667,8 @@ fn try_resolve_file_conflict(
let mut regular_delta = 0;
let mut removed_file_ids = vec![];
let mut added_file_ids = vec![];
for part in &conflict.removes {
match &part.value {
for term in &conflict.removes {
match &term.value {
TreeValue::File { id, executable } => {
if *executable {
exec_delta -= 1;
@ -682,8 +682,8 @@ fn try_resolve_file_conflict(
}
}
}
for part in &conflict.adds {
match &part.value {
for term in &conflict.adds {
match &term.value {
TreeValue::File { id, executable } => {
if *executable {
exec_delta += 1;
@ -739,19 +739,19 @@ fn try_resolve_file_conflict(
}
}
fn conflict_part_to_conflict(
fn conflict_term_to_conflict(
store: &Store,
path: &RepoPath,
part: &ConflictPart,
term: &ConflictTerm,
) -> Result<Conflict, BackendError> {
match &part.value {
match &term.value {
TreeValue::Conflict(id) => {
let conflict = store.read_conflict(path, id)?;
Ok(conflict)
}
other => Ok(Conflict {
removes: vec![],
adds: vec![ConflictPart {
adds: vec![ConflictTerm {
value: other.clone(),
}],
}),
@ -797,27 +797,27 @@ fn simplify_conflict(
// First expand any diffs with nested conflicts.
let mut new_removes = vec![];
let mut new_adds = vec![];
for part in &conflict.adds {
match part.value {
for term in &conflict.adds {
match term.value {
TreeValue::Conflict(_) => {
let conflict = conflict_part_to_conflict(store, path, part)?;
let conflict = conflict_term_to_conflict(store, path, term)?;
new_removes.extend_from_slice(&conflict.removes);
new_adds.extend_from_slice(&conflict.adds);
}
_ => {
new_adds.push(part.clone());
new_adds.push(term.clone());
}
}
}
for part in &conflict.removes {
match part.value {
for term in &conflict.removes {
match term.value {
TreeValue::Conflict(_) => {
let conflict = conflict_part_to_conflict(store, path, part)?;
let conflict = conflict_term_to_conflict(store, path, term)?;
new_removes.extend_from_slice(&conflict.adds);
new_adds.extend_from_slice(&conflict.removes);
}
_ => {
new_removes.push(part.clone());
new_removes.push(term.clone());
}
}
}

38
lib/tests/test_conflicts.rs
View file

@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use jujutsu_lib::backend::{Conflict, ConflictPart, FileId, TreeValue};
use jujutsu_lib::backend::{Conflict, ConflictTerm, FileId, TreeValue};
use jujutsu_lib::conflicts::{materialize_conflict, parse_conflict, update_conflict_from_content};
use jujutsu_lib::files::{ConflictHunk, MergeHunk};
use jujutsu_lib::repo::Repo;
@ -20,8 +20,8 @@ use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::store::Store;
use testutils::TestRepo;
fn file_conflict_part(file_id: &FileId) -> ConflictPart {
ConflictPart {
fn file_conflict_term(file_id: &FileId) -> ConflictTerm {
ConflictTerm {
value: TreeValue::File {
id: file_id.clone(),
executable: false,
@ -69,8 +69,8 @@ line 5
);
let mut conflict = Conflict {
removes: vec![file_conflict_part(&base_id)],
adds: vec![file_conflict_part(&left_id), file_conflict_part(&right_id)],
removes: vec![file_conflict_term(&base_id)],
adds: vec![file_conflict_term(&left_id), file_conflict_term(&right_id)],
};
insta::assert_snapshot!(
&materialize_conflict_string(store, &path, &conflict),
@ -150,10 +150,10 @@ line 5
// left modifies a line, right deletes the same line.
let conflict = Conflict {
removes: vec![file_conflict_part(&base_id)],
removes: vec![file_conflict_term(&base_id)],
adds: vec![
file_conflict_part(&modified_id),
file_conflict_part(&deleted_id),
file_conflict_term(&modified_id),
file_conflict_term(&deleted_id),
],
};
insta::assert_snapshot!(&materialize_conflict_string(store, &path, &conflict), @r###"
@ -172,10 +172,10 @@ line 5
// right modifies a line, left deletes the same line.
let conflict = Conflict {
removes: vec![file_conflict_part(&base_id)],
removes: vec![file_conflict_term(&base_id)],
adds: vec![
file_conflict_part(&deleted_id),
file_conflict_part(&modified_id),
file_conflict_term(&deleted_id),
file_conflict_term(&modified_id),
],
};
insta::assert_snapshot!(&materialize_conflict_string(store, &path, &conflict), @r###"
@ -194,8 +194,8 @@ line 5
// modify/delete conflict at the file level
let conflict = Conflict {
removes: vec![file_conflict_part(&base_id)],
adds: vec![file_conflict_part(&modified_id)],
removes: vec![file_conflict_term(&base_id)],
adds: vec![file_conflict_term(&modified_id)],
};
insta::assert_snapshot!(&materialize_conflict_string(store, &path, &conflict), @r###"
<<<<<<<
@ -381,10 +381,10 @@ fn test_update_conflict_from_content() {
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_part(&base_file_id)],
removes: vec![file_conflict_term(&base_file_id)],
adds: vec![
file_conflict_part(&left_file_id),
file_conflict_part(&right_file_id),
file_conflict_term(&left_file_id),
file_conflict_term(&right_file_id),
],
};
let conflict_id = store.write_conflict(&path, &conflict).unwrap();
@ -424,10 +424,10 @@ fn test_update_conflict_from_content() {
assert_eq!(
new_conflict,
Conflict {
removes: vec![file_conflict_part(&new_base_file_id)],
removes: vec![file_conflict_term(&new_base_file_id)],
adds: vec![
file_conflict_part(&new_left_file_id),
file_conflict_part(&new_right_file_id)
file_conflict_term(&new_left_file_id),
file_conflict_term(&new_right_file_id)
]
}
)

44
lib/tests/test_merge_trees.rs
View file

@ -14,7 +14,7 @@
use assert_matches::assert_matches;
use itertools::Itertools;
use jujutsu_lib::backend::{ConflictPart, TreeValue};
use jujutsu_lib::backend::{ConflictTerm, TreeValue};
use jujutsu_lib::repo::Repo;
use jujutsu_lib::repo_path::{RepoPath, RepoPathComponent};
use jujutsu_lib::rewrite::rebase_commit;
@ -126,10 +126,10 @@ fn test_same_type(use_git: bool) {
assert_eq!(
conflict.adds,
vec![
ConflictPart {
ConflictTerm {
value: side1_tree.value(&component).cloned().unwrap()
},
ConflictPart {
ConflictTerm {
value: side2_tree.value(&component).cloned().unwrap()
}
]
@ -146,13 +146,13 @@ fn test_same_type(use_git: bool) {
.unwrap();
assert_eq!(
conflict.removes,
vec![ConflictPart {
vec![ConflictTerm {
value: base_tree.value(&component).cloned().unwrap()
}]
);
assert_eq!(
conflict.adds,
vec![ConflictPart {
vec![ConflictTerm {
value: side2_tree.value(&component).cloned().unwrap()
}]
);
@ -167,13 +167,13 @@ fn test_same_type(use_git: bool) {
.unwrap();
assert_eq!(
conflict.removes,
vec![ConflictPart {
vec![ConflictTerm {
value: base_tree.value(&component).cloned().unwrap()
}]
);
assert_eq!(
conflict.adds,
vec![ConflictPart {
vec![ConflictTerm {
value: side1_tree.value(&component).cloned().unwrap()
}]
);
@ -188,17 +188,17 @@ fn test_same_type(use_git: bool) {
.unwrap();
assert_eq!(
conflict.removes,
vec![ConflictPart {
vec![ConflictTerm {
value: base_tree.value(&component).cloned().unwrap()
}]
);
assert_eq!(
conflict.adds,
vec![
ConflictPart {
ConflictTerm {
value: side1_tree.value(&component).cloned().unwrap()
},
ConflictPart {
ConflictTerm {
value: side2_tree.value(&component).cloned().unwrap()
}
]
@ -406,17 +406,17 @@ fn test_types(use_git: bool) {
.unwrap();
assert_eq!(
conflict.removes,
vec![ConflictPart {
vec![ConflictTerm {
value: base_tree.value(&component).cloned().unwrap()
}]
);
assert_eq!(
conflict.adds,
vec![
ConflictPart {
ConflictTerm {
value: side1_tree.value(&component).cloned().unwrap()
},
ConflictPart {
ConflictTerm {
value: side2_tree.value(&component).cloned().unwrap()
},
]
@ -432,17 +432,17 @@ fn test_types(use_git: bool) {
.unwrap();
assert_eq!(
conflict.removes,
vec![ConflictPart {
vec![ConflictTerm {
value: base_tree.value(&component).cloned().unwrap()
}]
);
assert_eq!(
conflict.adds,
vec![
ConflictPart {
ConflictTerm {
value: side1_tree.value(&component).cloned().unwrap()
},
ConflictPart {
ConflictTerm {
value: side2_tree.value(&component).cloned().unwrap()
},
]
@ -507,17 +507,17 @@ fn test_simplify_conflict(use_git: bool) {
.unwrap();
assert_eq!(
conflict.removes,
vec![ConflictPart {
vec![ConflictTerm {
value: base_tree.value(&component).cloned().unwrap()
}]
);
assert_eq!(
conflict.adds,
vec![
ConflictPart {
ConflictTerm {
value: branch_tree.value(&component).cloned().unwrap()
},
ConflictPart {
ConflictTerm {
value: upstream2_tree.value(&component).cloned().unwrap()
},
]
@ -531,17 +531,17 @@ fn test_simplify_conflict(use_git: bool) {
let conflict = store.read_conflict(&path, id).unwrap();
assert_eq!(
conflict.removes,
vec![ConflictPart {
vec![ConflictTerm {
value: base_tree.value(&component).cloned().unwrap()
}]
);
assert_eq!(
conflict.adds,
vec![
ConflictPart {
ConflictTerm {
value: upstream2_tree.value(&component).cloned().unwrap()
},
ConflictPart {
ConflictTerm {
value: branch_tree.value(&component).cloned().unwrap()
},
]

8
lib/tests/test_working_copy.rs
View file

@ -21,7 +21,7 @@ use std::os::unix::net::UnixListener;
use std::sync::Arc;
use itertools::Itertools;
use jujutsu_lib::backend::{Conflict, ConflictPart, TreeValue};
use jujutsu_lib::backend::{Conflict, ConflictTerm, TreeValue};
use jujutsu_lib::gitignore::GitIgnoreFile;
#[cfg(unix)]
use jujutsu_lib::op_store::OperationId;
@ -121,20 +121,20 @@ fn test_checkout_file_transitions(use_git: bool) {
let left_file_id = testutils::write_file(store, path, "left file contents");
let right_file_id = testutils::write_file(store, path, "right file contents");
let conflict = Conflict {
removes: vec![ConflictPart {
removes: vec![ConflictTerm {
value: TreeValue::File {
id: base_file_id,
executable: false,
},
}],
adds: vec![
ConflictPart {
ConflictTerm {
value: TreeValue::File {
id: left_file_id,
executable: false,
},
},
ConflictPart {
ConflictTerm {
value: TreeValue::File {
id: right_file_id,
executable: false,