repo: move merge with operation to transaction (#111)

It's the transaction's job to create a new operation, and that's where
the knowledge of parent operations is. By moving the logic for merging
in another operation there, we can make it contiuously update its set
of parent operations. That removes the risk of forgetting to add the
merged-in operation as a parent. It also makes it easier to reuse the
function from the CLI so we can inform the user about the process
(which is what I was investigating when I noticed that this cleanup
was possible).

lib/src/repo.rs

@@ -26,7 +26,7 @@ use thiserror::Error;
 use crate::backend::{BackendError, ChangeId, CommitId};
 use crate::commit::Commit;
 use crate::commit_builder::CommitBuilder;
-use crate::dag_walk::{closest_common_node, topo_order_reverse};
+use crate::dag_walk::topo_order_reverse;
 use crate::index::{IndexRef, MutableIndex, ReadonlyIndex};
 use crate::index_store::IndexStore;
 use crate::op_heads_store::{LockedOpHeads, OpHeads, OpHeadsStore};
@@ -392,22 +392,9 @@ impl RepoLoader {
         op_heads.sort_by_key(|op| op.store_operation().metadata.end_time.timestamp.clone());
         let base_repo = self.load_at(&op_heads[0]);
         let mut tx = base_repo.start_transaction("resolve concurrent operations");
-        let merged_repo = tx.mut_repo();
+        for other_op_head in op_heads.into_iter().skip(1) {
-        let neighbors_fn = |op: &Operation| op.parents();
+            tx.merge_operation(user_settings, other_op_head);
-        for (i, other_op_head) in op_heads.iter().enumerate().skip(1) {
-            let ancestor_op = closest_common_node(
-                op_heads[0..i].to_vec(),
-                vec![other_op_head.clone()],
-                &neighbors_fn,
-                &|op: &Operation| op.id().clone(),
-            )
-            .unwrap();
-            let base_repo = self.load_at(&ancestor_op);
-            let other_repo = self.load_at(other_op_head);
-            merged_repo.merge(&base_repo, &other_repo);
-            merged_repo.rebase_descendants(user_settings);
         }
-        tx.set_parent_ops(op_heads);
         tx.write()
     }
 

lib/src/transaction.rs

@@ -16,11 +16,13 @@ use std::collections::HashMap;
 use std::sync::Arc;
 
 use crate::backend::Timestamp;
+use crate::dag_walk::closest_common_node;
 use crate::index::ReadonlyIndex;
 use crate::op_store;
 use crate::op_store::OperationMetadata;
 use crate::operation::Operation;
 use crate::repo::{MutableRepo, ReadonlyRepo, RepoLoader};
+use crate::settings::UserSettings;
 use crate::view::View;
 
 pub struct Transaction {
@@ -47,10 +49,6 @@ impl Transaction {
         self.repo.as_ref().unwrap().base_repo()
     }
 
-    pub fn set_parent_ops(&mut self, parent_ops: Vec<Operation>) {
-        self.parent_ops = parent_ops;
-    }
-
     pub fn set_tag(&mut self, key: String, value: String) {
         self.tags.insert(key, value);
     }
@@ -59,6 +57,23 @@ impl Transaction {
         self.repo.as_mut().unwrap()
     }
 
+    pub fn merge_operation(&mut self, user_settings: &UserSettings, other_op: Operation) {
+        let ancestor_op = closest_common_node(
+            self.parent_ops.clone(),
+            vec![other_op.clone()],
+            &|op: &Operation| op.parents(),
+            &|op: &Operation| op.id().clone(),
+        )
+        .unwrap();
+        let repo_loader = self.base_repo().loader();
+        let base_repo = repo_loader.load_at(&ancestor_op);
+        let other_repo = repo_loader.load_at(&other_op);
+        self.parent_ops.push(other_op);
+        let merged_repo = self.mut_repo();
+        merged_repo.merge(&base_repo, &other_repo);
+        merged_repo.rebase_descendants(user_settings);
+    }
+
     /// Writes the transaction to the operation store and publishes it.
     pub fn commit(self) -> Arc<ReadonlyRepo> {
         self.write().publish()