transaction: add accessors for view and evolution directly on transaction

lib/src/git.rs

@@ -30,7 +30,7 @@ pub fn import_refs(
 ) -> Result<(), GitImportError> {
     let store = tx.store().clone();
     let git_refs = git_repo.references()?;
-    let existing_git_refs: Vec<_> = tx.as_repo_ref().view().git_refs().keys().cloned().collect();
+    let existing_git_refs: Vec<_> = tx.view().git_refs().keys().cloned().collect();
     // TODO: Store the id of the previous import and read it back here, so we can
     // merge the views instead of overwriting.
     for existing_git_ref in existing_git_refs {

lib/src/transaction.rs

@@ -15,6 +15,7 @@
 use crate::commit::Commit;
 use crate::commit_builder::CommitBuilder;
 use crate::conflicts;
+use crate::evolution::MutableEvolution;
 use crate::op_store;
 use crate::operation::Operation;
 use crate::repo::{MutableRepo, ReadonlyRepo, RepoRef};
@@ -22,6 +23,7 @@ use crate::settings::UserSettings;
 use crate::store;
 use crate::store::{CommitId, Timestamp};
 use crate::store_wrapper::StoreWrapper;
+use crate::view::MutableView;
 use std::sync::Arc;
 
 pub struct Transaction<'r> {
@@ -57,6 +59,14 @@ impl<'r> Transaction<'r> {
         Arc::get_mut(self.repo.as_mut().unwrap()).unwrap()
     }
 
+    pub fn view(&self) -> &MutableView {
+        self.repo.as_ref().unwrap().view()
+    }
+
+    pub fn evolution(&self) -> &MutableEvolution {
+        self.repo.as_ref().unwrap().evolution()
+    }
+
     pub fn write_commit(&mut self, commit: store::Commit) -> Commit {
         let commit = self.store().write_commit(commit);
         self.add_head(&commit);
@@ -64,15 +74,11 @@ impl<'r> Transaction<'r> {
     }
 
     pub fn check_out(&mut self, settings: &UserSettings, commit: &Commit) -> Commit {
-        let current_checkout_id = self.as_repo_ref().view().checkout().clone();
+        let current_checkout_id = self.view().checkout().clone();
         let current_checkout = self.store().get_commit(&current_checkout_id).unwrap();
         assert!(current_checkout.is_open(), "current checkout is closed");
         if current_checkout.is_empty()
-            && !(current_checkout.is_pruned()
+            && !(current_checkout.is_pruned() || self.evolution().is_obsolete(&current_checkout_id))
-                || self
-                    .as_repo_ref()
-                    .evolution()
-                    .is_obsolete(&current_checkout_id))
         {
             // Prune the checkout we're leaving if it's empty.
             // TODO: Also prune it if the only changes are conflicts that got materialized.

lib/tests/test_evolution.rs

@@ -37,8 +37,8 @@ fn test_obsolete_and_orphan(use_git: bool) {
 
     // A commit without successors should not be obsolete and not an orphan.
     let original = child_commit(&settings, &repo, &root_commit).write_to_transaction(&mut tx);
-    assert!(!tx.as_repo_ref().evolution().is_obsolete(original.id()));
+    assert!(!tx.evolution().is_obsolete(original.id()));
-    assert!(!tx.as_repo_ref().evolution().is_orphan(original.id()));
+    assert!(!tx.evolution().is_orphan(original.id()));
 
     // A commit with a successor with a different change_id should not be obsolete.
     let child = child_commit(&settings, &repo, &original).write_to_transaction(&mut tx);
@@ -46,30 +46,30 @@ fn test_obsolete_and_orphan(use_git: bool) {
     let cherry_picked = child_commit(&settings, &repo, &root_commit)
         .set_predecessors(vec![original.id().clone()])
         .write_to_transaction(&mut tx);
-    assert!(!tx.as_repo_ref().evolution().is_obsolete(original.id()));
+    assert!(!tx.evolution().is_obsolete(original.id()));
-    assert!(!tx.as_repo_ref().evolution().is_orphan(original.id()));
+    assert!(!tx.evolution().is_orphan(original.id()));
-    assert!(!tx.as_repo_ref().evolution().is_obsolete(child.id()));
+    assert!(!tx.evolution().is_obsolete(child.id()));
-    assert!(!tx.as_repo_ref().evolution().is_orphan(child.id()));
+    assert!(!tx.evolution().is_orphan(child.id()));
 
     // A commit with a successor with the same change_id should be obsolete.
     let rewritten = child_commit(&settings, &repo, &root_commit)
         .set_predecessors(vec![original.id().clone()])
         .set_change_id(original.change_id().clone())
         .write_to_transaction(&mut tx);
-    assert!(tx.as_repo_ref().evolution().is_obsolete(original.id()));
+    assert!(tx.evolution().is_obsolete(original.id()));
-    assert!(!tx.as_repo_ref().evolution().is_obsolete(child.id()));
+    assert!(!tx.evolution().is_obsolete(child.id()));
-    assert!(tx.as_repo_ref().evolution().is_orphan(child.id()));
+    assert!(tx.evolution().is_orphan(child.id()));
-    assert!(tx.as_repo_ref().evolution().is_orphan(grandchild.id()));
+    assert!(tx.evolution().is_orphan(grandchild.id()));
-    assert!(!tx.as_repo_ref().evolution().is_obsolete(cherry_picked.id()));
+    assert!(!tx.evolution().is_obsolete(cherry_picked.id()));
-    assert!(!tx.as_repo_ref().evolution().is_orphan(cherry_picked.id()));
+    assert!(!tx.evolution().is_orphan(cherry_picked.id()));
-    assert!(!tx.as_repo_ref().evolution().is_obsolete(rewritten.id()));
+    assert!(!tx.evolution().is_obsolete(rewritten.id()));
-    assert!(!tx.as_repo_ref().evolution().is_orphan(rewritten.id()));
+    assert!(!tx.evolution().is_orphan(rewritten.id()));
 
     // It should no longer be obsolete if we remove the successor.
     tx.remove_head(&rewritten);
-    assert!(!tx.as_repo_ref().evolution().is_obsolete(original.id()));
+    assert!(!tx.evolution().is_obsolete(original.id()));
-    assert!(!tx.as_repo_ref().evolution().is_orphan(child.id()));
+    assert!(!tx.evolution().is_orphan(child.id()));
-    assert!(!tx.as_repo_ref().evolution().is_orphan(grandchild.id()));
+    assert!(!tx.evolution().is_orphan(grandchild.id()));
     tx.discard();
 }
 
@@ -83,10 +83,7 @@ fn test_divergent(use_git: bool) {
 
     // A single commit should not be divergent
     let original = child_commit(&settings, &repo, &root_commit).write_to_transaction(&mut tx);
-    assert!(!tx
+    assert!(!tx.evolution().is_divergent(original.change_id()));
-        .as_repo_ref()
-        .evolution()
-        .is_divergent(original.change_id()));
 
     // Commits with the same change id are divergent, including the original commit
     // (it's the change that's divergent)
@@ -98,10 +95,7 @@ fn test_divergent(use_git: bool) {
         .set_predecessors(vec![original.id().clone()])
         .set_change_id(original.change_id().clone())
         .write_to_transaction(&mut tx);
-    assert!(tx
+    assert!(tx.evolution().is_divergent(original.change_id()));
-        .as_repo_ref()
-        .evolution()
-        .is_divergent(original.change_id()));
     tx.discard();
 }
 
@@ -127,10 +121,7 @@ fn test_divergent_pruned(use_git: bool) {
         .set_change_id(original.change_id().clone())
         .set_pruned(true)
         .write_to_transaction(&mut tx);
-    assert!(tx
+    assert!(tx.evolution().is_divergent(original.change_id()));
-        .as_repo_ref()
-        .evolution()
-        .is_divergent(original.change_id()));
     tx.discard();
 }
 
@@ -150,18 +141,9 @@ fn test_divergent_duplicate(use_git: bool) {
     let cherry_picked2 = child_commit(&settings, &repo, &root_commit)
         .set_predecessors(vec![original.id().clone()])
         .write_to_transaction(&mut tx);
-    assert!(!tx
+    assert!(!tx.evolution().is_divergent(original.change_id()));
-        .as_repo_ref()
+    assert!(!tx.evolution().is_divergent(cherry_picked1.change_id()));
-        .evolution()
+    assert!(!tx.evolution().is_divergent(cherry_picked2.change_id()));
-        .is_divergent(original.change_id()));
-    assert!(!tx
-        .as_repo_ref()
-        .evolution()
-        .is_divergent(cherry_picked1.change_id()));
-    assert!(!tx
-        .as_repo_ref()
-        .evolution()
-        .is_divergent(cherry_picked2.change_id()));
     tx.discard();
 }
 
@@ -182,7 +164,7 @@ fn test_new_parent_rewritten(use_git: bool) {
         .set_change_id(original.change_id().clone())
         .write_to_transaction(&mut tx);
     assert_eq!(
-        tx.as_repo_ref().evolution().new_parent(original.id()),
+        tx.evolution().new_parent(original.id()),
         vec![rewritten.id().clone()].into_iter().collect()
     );
     tx.discard();
@@ -202,7 +184,7 @@ fn test_new_parent_cherry_picked(use_git: bool) {
         .set_predecessors(vec![original.id().clone()])
         .write_to_transaction(&mut tx);
     assert_eq!(
-        tx.as_repo_ref().evolution().new_parent(original.id()),
+        tx.evolution().new_parent(original.id()),
         vec![original.id().clone()].into_iter().collect()
     );
     tx.discard();
@@ -226,7 +208,7 @@ fn test_new_parent_is_pruned(use_git: bool) {
         .set_change_id(original.change_id().clone())
         .write_to_transaction(&mut tx);
     assert_eq!(
-        tx.as_repo_ref().evolution().new_parent(original.id()),
+        tx.evolution().new_parent(original.id()),
         vec![new_parent.id().clone()].into_iter().collect()
     );
     tx.discard();
@@ -255,7 +237,7 @@ fn test_new_parent_divergent(use_git: bool) {
         .set_change_id(original.change_id().clone())
         .write_to_transaction(&mut tx);
     assert_eq!(
-        tx.as_repo_ref().evolution().new_parent(original.id()),
+        tx.evolution().new_parent(original.id()),
         vec![
             rewritten1.id().clone(),
             rewritten2.id().clone(),
@@ -296,7 +278,7 @@ fn test_new_parent_divergent_one_not_pruned(use_git: bool) {
         .set_pruned(true)
         .write_to_transaction(&mut tx);
     assert_eq!(
-        tx.as_repo_ref().evolution().new_parent(original.id()),
+        tx.evolution().new_parent(original.id()),
         vec![
             rewritten1.id().clone(),
             parent2.id().clone(),
@@ -339,7 +321,7 @@ fn test_new_parent_divergent_all_pruned(use_git: bool) {
         .set_pruned(true)
         .write_to_transaction(&mut tx);
     assert_eq!(
-        tx.as_repo_ref().evolution().new_parent(original.id()),
+        tx.evolution().new_parent(original.id()),
         vec![
             parent1.id().clone(),
             parent2.id().clone(),
@@ -375,7 +357,7 @@ fn test_new_parent_split(use_git: bool) {
         .set_predecessors(vec![original.id().clone()])
         .write_to_transaction(&mut tx);
     assert_eq!(
-        tx.as_repo_ref().evolution().new_parent(original.id()),
+        tx.evolution().new_parent(original.id()),
         vec![rewritten3.id().clone()].into_iter().collect()
     );
     tx.discard();
@@ -409,7 +391,7 @@ fn test_new_parent_split_pruned_descendant(use_git: bool) {
         .set_predecessors(vec![original.id().clone()])
         .write_to_transaction(&mut tx);
     assert_eq!(
-        tx.as_repo_ref().evolution().new_parent(original.id()),
+        tx.evolution().new_parent(original.id()),
         vec![rewritten2.id().clone()].into_iter().collect()
     );
     tx.discard();
@@ -443,7 +425,7 @@ fn test_new_parent_split_forked(use_git: bool) {
         .set_predecessors(vec![original.id().clone()])
         .write_to_transaction(&mut tx);
     assert_eq!(
-        tx.as_repo_ref().evolution().new_parent(original.id()),
+        tx.evolution().new_parent(original.id()),
         vec![rewritten2.id().clone(), rewritten3.id().clone()]
             .into_iter()
             .collect()
@@ -479,7 +461,7 @@ fn test_new_parent_split_forked_pruned(use_git: bool) {
         .set_predecessors(vec![original.id().clone()])
         .write_to_transaction(&mut tx);
     assert_eq!(
-        tx.as_repo_ref().evolution().new_parent(original.id()),
+        tx.evolution().new_parent(original.id()),
         vec![rewritten3.id().clone()].into_iter().collect()
     );
     tx.discard();

lib/tests/test_git.rs

@@ -69,7 +69,7 @@ fn test_import_refs() {
     let mut tx = repo.start_transaction("test");
     let heads_before: HashSet<_> = repo.view().heads().clone();
     jujube_lib::git::import_refs(&mut tx, &git_repo).unwrap_or_default();
-    let view = tx.as_repo_ref().view();
+    let view = tx.view();
     let expected_heads: HashSet<_> = heads_before
         .union(&hashset!(
             commit_id(&commit3),
@@ -125,7 +125,7 @@ fn test_import_refs_reimport() {
     let mut tx = repo.start_transaction("test");
     jujube_lib::git::import_refs(&mut tx, &git_repo).unwrap_or_default();
 
-    let view = tx.as_repo_ref().view();
+    let view = tx.view();
     // TODO: commit3 and commit4 should probably be removed
     let expected_heads: HashSet<_> = heads_before
         .union(&hashset!(
@@ -277,9 +277,8 @@ fn test_import_refs_empty_git_repo() {
     let heads_before = repo.view().heads().clone();
     let mut tx = repo.start_transaction("test");
     jujube_lib::git::import_refs(&mut tx, &git_repo).unwrap_or_default();
-    let view = tx.as_repo_ref().view();
+    assert_eq!(*tx.view().heads(), heads_before);
-    assert_eq!(*view.heads(), heads_before);
+    assert_eq!(tx.view().git_refs().len(), 0);
-    assert_eq!(view.git_refs().len(), 0);
     tx.discard();
 }
 
@@ -323,11 +322,7 @@ fn test_fetch_success() {
     // The new commit is visible after git::fetch().
     let mut tx = jj_repo.start_transaction("test");
     git::fetch(&mut tx, &clone_git_repo, "origin").unwrap();
-    assert!(tx
+    assert!(tx.view().heads().contains(&commit_id(&new_git_commit)));
-        .as_repo_ref()
-        .view()
-        .heads()
-        .contains(&commit_id(&new_git_commit)));
 
     tx.discard();
 }

lib/tests/test_operations.rs

@@ -130,8 +130,8 @@ fn test_isolation(use_git: bool) {
     assert_heads(tx1.as_repo_ref(), vec![&wc_id, initial.id()]);
     assert_heads(tx2.as_repo_ref(), vec![&wc_id, initial.id()]);
     assert!(!repo.evolution().is_obsolete(initial.id()));
-    assert!(!tx1.as_repo_ref().evolution().is_obsolete(initial.id()));
+    assert!(!tx1.evolution().is_obsolete(initial.id()));
-    assert!(!tx2.as_repo_ref().evolution().is_obsolete(initial.id()));
+    assert!(!tx2.evolution().is_obsolete(initial.id()));
 
     let rewrite1 = CommitBuilder::for_rewrite_from(&settings, repo.store(), &initial)
         .set_description("rewrite1".to_string())
@@ -146,8 +146,8 @@ fn test_isolation(use_git: bool) {
     assert_heads(tx1.as_repo_ref(), vec![&wc_id, initial.id(), rewrite1.id()]);
     assert_heads(tx2.as_repo_ref(), vec![&wc_id, initial.id(), rewrite2.id()]);
     assert!(!repo.evolution().is_obsolete(initial.id()));
-    assert!(tx1.as_repo_ref().evolution().is_obsolete(initial.id()));
+    assert!(tx1.evolution().is_obsolete(initial.id()));
-    assert!(tx2.as_repo_ref().evolution().is_obsolete(initial.id()));
+    assert!(tx2.evolution().is_obsolete(initial.id()));
 
     // The base repo and tx2 don't see the commits from tx1.
     tx1.commit();

lib/tests/test_transaction.rs

@@ -202,7 +202,7 @@ fn test_checkout_previous_not_empty(use_git: bool) {
         .set_open(true)
         .write_to_transaction(&mut tx);
     tx.check_out(&settings, &new_checkout);
-    assert!(!tx.as_repo_ref().evolution().is_obsolete(old_checkout.id()));
+    assert!(!tx.evolution().is_obsolete(old_checkout.id()));
     tx.discard();
 }
 
@@ -231,7 +231,7 @@ fn test_checkout_previous_empty(use_git: bool) {
         .set_open(true)
         .write_to_transaction(&mut tx);
     tx.check_out(&settings, &new_checkout);
-    assert!(tx.as_repo_ref().evolution().is_obsolete(old_checkout.id()));
+    assert!(tx.evolution().is_obsolete(old_checkout.id()));
     tx.discard();
 }
 
@@ -262,7 +262,7 @@ fn test_checkout_previous_empty_and_obsolete(use_git: bool) {
         .set_open(true)
         .write_to_transaction(&mut tx);
     tx.check_out(&settings, &new_checkout);
-    let successors = tx.as_repo_ref().evolution().successors(old_checkout.id());
+    let successors = tx.evolution().successors(old_checkout.id());
     assert_eq!(successors.len(), 1);
     assert_eq!(successors.iter().next().unwrap(), successor.id());
     tx.discard();
@@ -290,11 +290,7 @@ fn test_checkout_previous_empty_and_pruned(use_git: bool) {
         .set_open(true)
         .write_to_transaction(&mut tx);
     tx.check_out(&settings, &new_checkout);
-    assert!(tx
+    assert!(tx.evolution().successors(old_checkout.id()).is_empty());
-        .as_repo_ref()
-        .evolution()
-        .successors(old_checkout.id())
-        .is_empty());
     tx.discard();
 }
 
@@ -314,9 +310,9 @@ fn test_add_head_success(use_git: bool) {
     tx.discard();
 
     let mut tx = repo.start_transaction("test");
-    assert!(!tx.as_repo_ref().view().heads().contains(new_commit.id()));
+    assert!(!tx.view().heads().contains(new_commit.id()));
     tx.add_head(&new_commit);
-    assert!(tx.as_repo_ref().view().heads().contains(new_commit.id()));
+    assert!(tx.view().heads().contains(new_commit.id()));
     tx.commit();
     Arc::get_mut(&mut repo).unwrap().reload();
     assert!(repo.view().heads().contains(new_commit.id()));
@@ -343,7 +339,7 @@ fn test_add_head_ancestor(use_git: bool) {
 
     let mut tx = repo.start_transaction("test");
     tx.add_head(&commit1);
-    assert!(!tx.as_repo_ref().view().heads().contains(commit1.id()));
+    assert!(!tx.view().heads().contains(commit1.id()));
     tx.discard();
 }
 
@@ -367,9 +363,9 @@ fn test_remove_head(use_git: bool) {
     Arc::get_mut(&mut repo).unwrap().reload();
 
     let mut tx = repo.start_transaction("test");
-    assert!(tx.as_repo_ref().view().heads().contains(commit3.id()));
+    assert!(tx.view().heads().contains(commit3.id()));
     tx.remove_head(&commit3);
-    let heads = tx.as_repo_ref().view().heads().clone();
+    let heads = tx.view().heads().clone();
     assert!(!heads.contains(commit3.id()));
     assert!(!heads.contains(commit2.id()));
     assert!(!heads.contains(commit1.id()));
@@ -403,10 +399,10 @@ fn test_remove_head_ancestor_git_ref(use_git: bool) {
     Arc::get_mut(&mut repo).unwrap().reload();
 
     let mut tx = repo.start_transaction("test");
-    let heads = tx.as_repo_ref().view().heads().clone();
+    let heads = tx.view().heads().clone();
     assert!(heads.contains(commit3.id()));
     tx.remove_head(&commit3);
-    let heads = tx.as_repo_ref().view().heads().clone();
+    let heads = tx.view().heads().clone();
     assert!(!heads.contains(commit3.id()));
     assert!(!heads.contains(commit2.id()));
     assert!(heads.contains(commit1.id()));
@@ -433,17 +429,9 @@ fn test_add_public_head(use_git: bool) {
     Arc::get_mut(&mut repo).unwrap().reload();
 
     let mut tx = repo.start_transaction("test");
-    assert!(!tx
+    assert!(!tx.view().public_heads().contains(commit1.id()));
-        .as_repo_ref()
-        .view()
-        .public_heads()
-        .contains(commit1.id()));
     tx.add_public_head(&commit1);
-    assert!(tx
+    assert!(tx.view().public_heads().contains(commit1.id()));
-        .as_repo_ref()
-        .view()
-        .public_heads()
-        .contains(commit1.id()));
     tx.commit();
     Arc::get_mut(&mut repo).unwrap().reload();
     assert!(repo.view().public_heads().contains(commit1.id()));
@@ -467,17 +455,9 @@ fn test_add_public_head_ancestor(use_git: bool) {
     Arc::get_mut(&mut repo).unwrap().reload();
 
     let mut tx = repo.start_transaction("test");
-    assert!(!tx
+    assert!(!tx.view().public_heads().contains(commit1.id()));
-        .as_repo_ref()
-        .view()
-        .public_heads()
-        .contains(commit1.id()));
     tx.add_public_head(&commit1);
-    assert!(!tx
+    assert!(!tx.view().public_heads().contains(commit1.id()));
-        .as_repo_ref()
-        .view()
-        .public_heads()
-        .contains(commit1.id()));
     tx.commit();
     Arc::get_mut(&mut repo).unwrap().reload();
     assert!(!repo.view().public_heads().contains(commit1.id()));
@@ -498,17 +478,9 @@ fn test_remove_public_head(use_git: bool) {
     Arc::get_mut(&mut repo).unwrap().reload();
 
     let mut tx = repo.start_transaction("test");
-    assert!(tx
+    assert!(tx.view().public_heads().contains(commit1.id()));
-        .as_repo_ref()
-        .view()
-        .public_heads()
-        .contains(commit1.id()));
     tx.remove_public_head(&commit1);
-    assert!(!tx
+    assert!(!tx.view().public_heads().contains(commit1.id()));
-        .as_repo_ref()
-        .view()
-        .public_heads()
-        .contains(commit1.id()));
     tx.commit();
     Arc::get_mut(&mut repo).unwrap().reload();
     assert!(!repo.view().public_heads().contains(commit1.id()));

lib/tests/test_view.rs

@@ -45,7 +45,7 @@ fn test_heads_fork(use_git: bool) {
 
     let wc = repo.working_copy_locked();
     assert_eq!(
-        *tx.as_repo_ref().view().heads(),
+        *tx.view().heads(),
         hashset! {
             wc.current_commit_id(),
             child1.id().clone(),
@@ -77,7 +77,7 @@ fn test_heads_merge(use_git: bool) {
 
     let wc = repo.working_copy_locked();
     assert_eq!(
-        *tx.as_repo_ref().view().heads(),
+        *tx.view().heads(),
         hashset! {wc.current_commit_id(), merge.id().clone()}
     );
     tx.discard();

src/commands.rs

@@ -256,8 +256,7 @@ fn update_working_copy(
 
 fn update_checkout_after_rewrite(ui: &mut Ui, tx: &mut Transaction) {
     // TODO: Perhaps this method should be in Transaction.
-    let repo = tx.as_repo_ref();
+    let new_checkout_candidates = tx.evolution().new_parent(tx.view().checkout());
-    let new_checkout_candidates = repo.evolution().new_parent(repo.view().checkout());
     if new_checkout_candidates.is_empty() {
         return;
     }
@@ -278,7 +277,7 @@ fn update_checkout_after_rewrite(ui: &mut Ui, tx: &mut Transaction) {
         );
     }
     let new_checkout = new_checkout_candidates.iter().min().unwrap();
-    let new_commit = repo.store().get_commit(new_checkout).unwrap();
+    let new_commit = tx.store().get_commit(new_checkout).unwrap();
     tx.check_out(ui.settings(), &new_commit);
 }
 
@@ -1337,7 +1336,7 @@ fn cmd_new(
     );
     let mut tx = repo.start_transaction("new empty commit");
     let new_commit = commit_builder.write_to_transaction(&mut tx);
-    if tx.as_repo_ref().view().checkout() == parent.id() {
+    if tx.view().checkout() == parent.id() {
         tx.check_out(ui.settings(), &new_commit);
     }
     tx.commit();