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

backend: pass in path when reading/writing conflicts as well

Browse source 
We do it for all the other kinds of objects already. It's useful to
have the path for backends that store objects by path (we don't have
any such backends yet). I think the reason I didn't do it from the
beginning was because we had separate `RepoPath` types for files and
directories back then.
This commit is contained in:
Martin von Zweigbergk 2022-03-31 09:21:50 -07:00 committed by Martin von Zweigbergk
parent fd5bc7966c
commit f16d2a237b
11 changed files with 65 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
lib/src/backend.rs
View file

@ -393,9 +393,7 @@ pub trait Backend: Send + Sync + Debug {
fn write_commit(&self, contents: &Commit) -> BackendResult<CommitId>;
// TODO: Pass in the paths here too even though they are unused, just like for
// files and trees?
fn read_conflict(&self, id: &ConflictId) -> BackendResult<Conflict>;
fn read_conflict(&self, path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict>;
fn write_conflict(&self, contents: &Conflict) -> BackendResult<ConflictId>;
fn write_conflict(&self, path: &RepoPath, contents: &Conflict) -> BackendResult<ConflictId>;
}

4
lib/src/conflicts.rs
View file

@ -321,7 +321,7 @@ pub fn update_conflict_from_content(
conflict_id: &ConflictId,
content: &[u8],
) -> BackendResult<Option<ConflictId>> {
let mut conflict = store.read_conflict(conflict_id)?;
let mut conflict = store.read_conflict(path, conflict_id)?;
// 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
@ -379,7 +379,7 @@ pub fn update_conflict_from_content(
panic!("Found conflict markers in merge of non-files");
}
}
let new_conflict_id = store.write_conflict(&conflict)?;
let new_conflict_id = store.write_conflict(path, &conflict)?;
Ok(Some(new_conflict_id))
} else {
Ok(None)

4
lib/src/git_backend.rs
View file

@ -434,7 +434,7 @@ impl Backend for GitBackend {
Ok(id)
}
fn read_conflict(&self, id: &ConflictId) -> BackendResult<Conflict> {
fn read_conflict(&self, _path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict> {
let mut file = self.read_file(
&RepoPath::from_internal_string("unused"),
&FileId::new(id.to_bytes()),
@ -448,7 +448,7 @@ impl Backend for GitBackend {
})
}
fn write_conflict(&self, conflict: &Conflict) -> BackendResult<ConflictId> {
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),

4
lib/src/local_backend.rs
View file

@ -215,7 +215,7 @@ impl Backend for LocalBackend {
Ok(id)
}
fn read_conflict(&self, id: &ConflictId) -> BackendResult<Conflict> {
fn read_conflict(&self, _path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict> {
let path = self.conflict_path(id);
let mut file = File::open(path).map_err(not_found_to_backend_error)?;
@ -223,7 +223,7 @@ impl Backend for LocalBackend {
Ok(conflict_from_proto(&proto))
}
fn write_conflict(&self, conflict: &Conflict) -> BackendResult<ConflictId> {
fn write_conflict(&self, _path: &RepoPath, conflict: &Conflict) -> BackendResult<ConflictId> {
let temp_file = NamedTempFile::new_in(&self.path)?;
let proto = conflict_to_proto(conflict);

12
lib/src/store.rs
View file

@ -192,12 +192,16 @@ impl Store {
self.backend.write_symlink(path, contents)
}
pub fn read_conflict(&self, id: &ConflictId) -> BackendResult<Conflict> {
self.backend.read_conflict(id)
pub fn read_conflict(&self, path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict> {
self.backend.read_conflict(path, id)
}
pub fn write_conflict(&self, contents: &Conflict) -> BackendResult<ConflictId> {
self.backend.write_conflict(contents)
pub fn write_conflict(
&self,
path: &RepoPath,
contents: &Conflict,
) -> BackendResult<ConflictId> {
self.backend.write_conflict(path, contents)
}
pub fn tree_builder(self: &Arc<Self>, base_tree_id: TreeId) -> TreeBuilder {

24
lib/src/tree.rs
View file

@ -603,7 +603,8 @@ fn merge_tree_value(
value: side2.clone(),
});
}
let conflict = simplify_conflict(store, &conflict)?;
let filename = dir.join(basename);
let conflict = simplify_conflict(store, &filename, &conflict)?;
if conflict.adds.is_empty() {
// If there are no values to add, then the path doesn't exist
return Ok(None);
@ -612,14 +613,13 @@ fn merge_tree_value(
// A single add means that the current state is that state.
return Ok(Some(conflict.adds[0].value.clone()));
}
let filename = dir.join(basename);
if let Some((merged_content, executable)) =
try_resolve_file_conflict(store, &filename, &conflict)?
{
let id = store.write_file(&filename, &mut merged_content.as_slice())?;
Some(TreeValue::Normal { id, executable })
} else {
let conflict_id = store.write_conflict(&conflict)?;
let conflict_id = store.write_conflict(&filename, &conflict)?;
Some(TreeValue::Conflict(conflict_id))
}
}
@ -703,10 +703,14 @@ fn try_resolve_file_conflict(
}
}
fn conflict_part_to_conflict(store: &Store, part: &ConflictPart) -> Result<Conflict, BackendError> {
fn conflict_part_to_conflict(
store: &Store,
path: &RepoPath,
part: &ConflictPart,
) -> Result<Conflict, BackendError> {
match &part.value {
TreeValue::Conflict(id) => {
let conflict = store.read_conflict(id)?;
let conflict = store.read_conflict(path, id)?;
Ok(conflict)
}
other => Ok(Conflict {
@ -718,7 +722,11 @@ fn conflict_part_to_conflict(store: &Store, part: &ConflictPart) -> Result<Confl
}
}
fn simplify_conflict(store: &Store, conflict: &Conflict) -> Result<Conflict, BackendError> {
fn simplify_conflict(
store: &Store,
path: &RepoPath,
conflict: &Conflict,
) -> Result<Conflict, BackendError> {
// Important cases to simplify:
//
// D
@ -756,7 +764,7 @@ fn simplify_conflict(store: &Store, conflict: &Conflict) -> Result<Conflict, Bac
for part in &conflict.adds {
match part.value {
TreeValue::Conflict(_) => {
let conflict = conflict_part_to_conflict(store, part)?;
let conflict = conflict_part_to_conflict(store, path, part)?;
new_removes.extend_from_slice(&conflict.removes);
new_adds.extend_from_slice(&conflict.adds);
}
@ -768,7 +776,7 @@ fn simplify_conflict(store: &Store, conflict: &Conflict) -> Result<Conflict, Bac
for part in &conflict.removes {
match part.value {
TreeValue::Conflict(_) => {
let conflict = conflict_part_to_conflict(store, part)?;
let conflict = conflict_part_to_conflict(store, path, part)?;
new_removes.extend_from_slice(&conflict.adds);
new_adds.extend_from_slice(&conflict.removes);
}

2
lib/src/working_copy.rs
View file

@ -512,7 +512,7 @@ impl TreeState {
fn write_conflict(&self, disk_path: &Path, path: &RepoPath, id: &ConflictId) -> FileState {
create_parent_dirs(disk_path);
let conflict = self.store.read_conflict(id).unwrap();
let conflict = self.store.read_conflict(path, id).unwrap();
// TODO: Check that we're not overwriting an un-ignored file here (which might
// be created by a concurrent process).
let mut file = OpenOptions::new()

4
lib/tests/test_conflicts.rs
View file

@ -452,7 +452,7 @@ fn test_update_conflict_from_content() {
},
],
};
let conflict_id = store.write_conflict(&conflict).unwrap();
let conflict_id = store.write_conflict(&path, &conflict).unwrap();
// If the content is unchanged compared to the materialized value, we get the
// old conflict id back.
@ -475,7 +475,7 @@ fn test_update_conflict_from_content() {
let result = update_conflict_from_content(store, &path, &conflict_id, b"resolved 1\nline 2\n<<<<<<<\n-------\n+++++++\n-line 3\n+left 3\n+++++++\nright 3\n>>>>>>>\n").unwrap();
assert_ne!(result, None);
assert_ne!(result, Some(conflict_id));
let new_conflict = store.read_conflict(&result.unwrap()).unwrap();
let new_conflict = store.read_conflict(&path, &result.unwrap()).unwrap();
// 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");

35
lib/tests/test_merge_trees.rs
View file

@ -119,7 +119,9 @@ fn test_same_type(use_git: bool) {
// Check the conflicting cases
match merged_tree.value(&RepoPathComponent::from("_ab")).unwrap() {
TreeValue::Conflict(id) => {
let conflict = store.read_conflict(id).unwrap();
let conflict = store
.read_conflict(&RepoPath::from_internal_string("_ab"), id)
.unwrap();
assert_eq!(
conflict.adds,
vec![
@ -143,7 +145,9 @@ fn test_same_type(use_git: bool) {
};
match merged_tree.value(&RepoPathComponent::from("a_b")).unwrap() {
TreeValue::Conflict(id) => {
let conflict = store.read_conflict(id).unwrap();
let conflict = store
.read_conflict(&RepoPath::from_internal_string("a_b"), id)
.unwrap();
assert_eq!(
conflict.removes,
vec![ConflictPart {
@ -167,7 +171,9 @@ fn test_same_type(use_git: bool) {
};
match merged_tree.value(&RepoPathComponent::from("ab_")).unwrap() {
TreeValue::Conflict(id) => {
let conflict = store.read_conflict(id).unwrap();
let conflict = store
.read_conflict(&RepoPath::from_internal_string("ab_"), id)
.unwrap();
assert_eq!(
conflict.removes,
vec![ConflictPart {
@ -191,7 +197,9 @@ fn test_same_type(use_git: bool) {
};
match merged_tree.value(&RepoPathComponent::from("abc")).unwrap() {
TreeValue::Conflict(id) => {
let conflict = store.read_conflict(id).unwrap();
let conflict = store
.read_conflict(&RepoPath::from_internal_string("abc"), id)
.unwrap();
assert_eq!(
conflict.removes,
vec![ConflictPart {
@ -373,7 +381,12 @@ fn test_types(use_git: bool) {
.unwrap()
{
TreeValue::Conflict(id) => {
let conflict = store.read_conflict(id).unwrap();
let conflict = store
.read_conflict(
&RepoPath::from_internal_string("normal_executable_symlink"),
id,
)
.unwrap();
assert_eq!(
conflict.removes,
vec![ConflictPart {
@ -408,7 +421,9 @@ fn test_types(use_git: bool) {
.unwrap()
{
TreeValue::Conflict(id) => {
let conflict = store.read_conflict(id).unwrap();
let conflict = store
.read_conflict(&RepoPath::from_internal_string("tree_normal_symlink"), id)
.unwrap();
assert_eq!(
conflict.removes,
vec![ConflictPart {
@ -496,7 +511,9 @@ fn test_simplify_conflict(use_git: bool) {
.unwrap()
{
TreeValue::Conflict(id) => {
let conflict = store.read_conflict(id).unwrap();
let conflict = store
.read_conflict(&RepoPath::from_internal_string("file"), id)
.unwrap();
assert_eq!(
conflict.removes,
vec![ConflictPart {
@ -532,7 +549,9 @@ fn test_simplify_conflict(use_git: bool) {
.unwrap()
{
TreeValue::Conflict(id) => {
let conflict = store.read_conflict(id).unwrap();
let conflict = store
.read_conflict(&RepoPath::from_internal_string("file"), id)
.unwrap();
assert_eq!(
conflict.removes,
vec![ConflictPart {

2
lib/tests/test_working_copy.rs
View file

@ -125,7 +125,7 @@ fn test_checkout_file_transitions(use_git: bool) {
},
],
};
let conflict_id = store.write_conflict(&conflict).unwrap();
let conflict_id = store.write_conflict(path, &conflict).unwrap();
TreeValue::Conflict(conflict_id)
}
Kind::Symlink => {

4
src/commands.rs
View file

@ -2120,7 +2120,7 @@ fn diff_content(
Ok(format!("Git submodule checked out at {}", id.hex()).into_bytes())
}
TreeValue::Conflict(id) => {
let conflict = repo.store().read_conflict(id).unwrap();
let conflict = repo.store().read_conflict(path, id).unwrap();
let mut content = vec![];
conflicts::materialize_conflict(repo.store(), path, &conflict, &mut content).unwrap();
Ok(content)
@ -2271,7 +2271,7 @@ fn git_diff_part(
TreeValue::Conflict(id) => {
mode = "100644".to_string();
hash = id.hex();
let conflict = repo.store().read_conflict(id).unwrap();
let conflict = repo.store().read_conflict(path, id).unwrap();
conflicts::materialize_conflict(repo.store(), path, &conflict, &mut content).unwrap();
}
}