From 8a067282c89caae09e47311dd27cac24027c9c6e Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Tue, 14 Feb 2023 16:00:27 -0800 Subject: [PATCH] repo: make `ReadonlyRepo::index()` return a `&dyn Index` This is just a little preparation for extracting a `Repo` trait that's implemented by both `ReadonlyRepo` and `MutableRepo`. The `index()` function in that trait will of course have to return the same type in both implementations, and that type will be `&dyn Index`. --- lib/src/repo.rs | 14 +++--- lib/tests/test_index.rs | 4 +- lib/tests/test_refs.rs | 59 ++++++++++++------------- lib/tests/test_revset_graph_iterator.rs | 1 - src/commands/branch.rs | 1 - src/commands/git.rs | 1 - src/commands/mod.rs | 2 +- 7 files changed, 41 insertions(+), 41 deletions(-) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index de75514ba..9b0512626 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -81,7 +81,7 @@ impl<'a> RepoRef<'a> { pub fn index(&self) -> &'a dyn Index { match self { - RepoRef::Readonly(repo) => repo.index().as_ref(), + RepoRef::Readonly(repo) => repo.index(), RepoRef::Mutable(repo) => repo.index(), } } @@ -265,13 +265,17 @@ impl ReadonlyRepo { &self.view } - pub fn index(&self) -> &Arc { + pub fn readonly_index(&self) -> &Arc { self.index.get_or_init(|| { self.index_store .get_index_at_op(&self.operation, &self.store) }) } + pub fn index(&self) -> &dyn Index { + self.readonly_index().as_ref() + } + fn change_id_index(&self) -> &ChangeIdIndex { self.change_id_index.get_or_init(|| { let heads = self.view().heads().iter().cloned().collect_vec(); @@ -321,7 +325,7 @@ impl ReadonlyRepo { user_settings: &UserSettings, description: &str, ) -> Transaction { - let mut_repo = MutableRepo::new(self.clone(), self.index().clone(), &self.view); + let mut_repo = MutableRepo::new(self.clone(), self.readonly_index().clone(), &self.view); Transaction::new(mut_repo, user_settings, description) } @@ -975,8 +979,8 @@ impl MutableRepo { // merging the view. Merging in base_repo's index isn't typically // necessary, but it can be if base_repo is ahead of either self or other_repo // (e.g. because we're undoing an operation that hasn't been published). - self.index.merge_in(base_repo.index()); - self.index.merge_in(other_repo.index()); + self.index.merge_in(base_repo.readonly_index()); + self.index.merge_in(other_repo.readonly_index()); self.view.ensure_clean(|v| self.enforce_view_invariants(v)); self.merge_view(&base_repo.view, &other_repo.view); diff --git a/lib/tests/test_index.rs b/lib/tests/test_index.rs index 8d3b5f956..34afa9ff8 100644 --- a/lib/tests/test_index.rs +++ b/lib/tests/test_index.rs @@ -17,7 +17,7 @@ use std::sync::Arc; use jujutsu_lib::backend::CommitId; use jujutsu_lib::commit::Commit; use jujutsu_lib::commit_builder::CommitBuilder; -use jujutsu_lib::index::{Index, ReadonlyIndex}; +use jujutsu_lib::index::{Index}; use jujutsu_lib::repo::{MutableRepo, ReadonlyRepo, StoreFactories}; use jujutsu_lib::settings::UserSettings; use test_case::test_case; @@ -32,7 +32,7 @@ fn child_commit<'repo>( } // Helper just to reduce line wrapping -fn generation_number(index: &Arc, commit_id: &CommitId) -> u32 { +fn generation_number(index: &dyn Index, commit_id: &CommitId) -> u32 { index.entry_by_id(commit_id).unwrap().generation_number() } diff --git a/lib/tests/test_refs.rs b/lib/tests/test_refs.rs index 20d3d4d7f..f4937235a 100644 --- a/lib/tests/test_refs.rs +++ b/lib/tests/test_refs.rs @@ -50,83 +50,82 @@ fn test_merge_ref_targets() { let _target7 = RefTarget::Normal(commit7.id().clone()); let index = repo.index(); - let index_ref = index.as_ref(); // Left moved forward assert_eq!( - merge_ref_targets(index_ref, Some(&target3), Some(&target1), Some(&target1)), + merge_ref_targets(index, Some(&target3), Some(&target1), Some(&target1)), Some(target3.clone()) ); // Right moved forward assert_eq!( - merge_ref_targets(index_ref, Some(&target1), Some(&target1), Some(&target3)), + merge_ref_targets(index, Some(&target1), Some(&target1), Some(&target3)), Some(target3.clone()) ); // Left moved backward assert_eq!( - merge_ref_targets(index_ref, Some(&target1), Some(&target3), Some(&target3)), + merge_ref_targets(index, Some(&target1), Some(&target3), Some(&target3)), Some(target1.clone()) ); // Right moved backward assert_eq!( - merge_ref_targets(index_ref, Some(&target3), Some(&target3), Some(&target1)), + merge_ref_targets(index, Some(&target3), Some(&target3), Some(&target1)), Some(target1.clone()) ); // Left moved sideways assert_eq!( - merge_ref_targets(index_ref, Some(&target4), Some(&target3), Some(&target3)), + merge_ref_targets(index, Some(&target4), Some(&target3), Some(&target3)), Some(target4.clone()) ); // Right moved sideways assert_eq!( - merge_ref_targets(index_ref, Some(&target3), Some(&target3), Some(&target4)), + merge_ref_targets(index, Some(&target3), Some(&target3), Some(&target4)), Some(target4.clone()) ); // Both added same target assert_eq!( - merge_ref_targets(index_ref, Some(&target3), None, Some(&target3)), + merge_ref_targets(index, Some(&target3), None, Some(&target3)), Some(target3.clone()) ); // Left added target, right added descendant target assert_eq!( - merge_ref_targets(index_ref, Some(&target2), None, Some(&target3)), + merge_ref_targets(index, Some(&target2), None, Some(&target3)), Some(target3.clone()) ); // Right added target, left added descendant target assert_eq!( - merge_ref_targets(index_ref, Some(&target3), None, Some(&target2)), + merge_ref_targets(index, Some(&target3), None, Some(&target2)), Some(target3.clone()) ); // Both moved forward to same target assert_eq!( - merge_ref_targets(index_ref, Some(&target3), Some(&target1), Some(&target3)), + merge_ref_targets(index, Some(&target3), Some(&target1), Some(&target3)), Some(target3.clone()) ); // Both moved forward, left moved further assert_eq!( - merge_ref_targets(index_ref, Some(&target3), Some(&target1), Some(&target2)), + merge_ref_targets(index, Some(&target3), Some(&target1), Some(&target2)), Some(target3.clone()) ); // Both moved forward, right moved further assert_eq!( - merge_ref_targets(index_ref, Some(&target2), Some(&target1), Some(&target3)), + merge_ref_targets(index, Some(&target2), Some(&target1), Some(&target3)), Some(target3.clone()) ); // Left and right moved forward to divergent targets assert_eq!( - merge_ref_targets(index_ref, Some(&target3), Some(&target1), Some(&target4)), + merge_ref_targets(index, Some(&target3), Some(&target1), Some(&target4)), Some(RefTarget::Conflict { removes: vec![commit1.id().clone()], adds: vec![commit3.id().clone(), commit4.id().clone()] @@ -135,7 +134,7 @@ fn test_merge_ref_targets() { // Left moved back, right moved forward assert_eq!( - merge_ref_targets(index_ref, Some(&target1), Some(&target2), Some(&target3)), + merge_ref_targets(index, Some(&target1), Some(&target2), Some(&target3)), Some(RefTarget::Conflict { removes: vec![commit2.id().clone()], adds: vec![commit1.id().clone(), commit3.id().clone()] @@ -144,7 +143,7 @@ fn test_merge_ref_targets() { // Right moved back, left moved forward assert_eq!( - merge_ref_targets(index_ref, Some(&target3), Some(&target2), Some(&target1)), + merge_ref_targets(index, Some(&target3), Some(&target2), Some(&target1)), Some(RefTarget::Conflict { removes: vec![commit2.id().clone()], adds: vec![commit3.id().clone(), commit1.id().clone()] @@ -153,19 +152,19 @@ fn test_merge_ref_targets() { // Left removed assert_eq!( - merge_ref_targets(index_ref, None, Some(&target3), Some(&target3)), + merge_ref_targets(index, None, Some(&target3), Some(&target3)), None ); // Right removed assert_eq!( - merge_ref_targets(index_ref, Some(&target3), Some(&target3), None), + merge_ref_targets(index, Some(&target3), Some(&target3), None), None ); // Left removed, right moved forward assert_eq!( - merge_ref_targets(index_ref, None, Some(&target1), Some(&target3)), + merge_ref_targets(index, None, Some(&target1), Some(&target3)), Some(RefTarget::Conflict { removes: vec![commit1.id().clone()], adds: vec![commit3.id().clone()] @@ -174,7 +173,7 @@ fn test_merge_ref_targets() { // Right removed, left moved forward assert_eq!( - merge_ref_targets(index_ref, Some(&target3), Some(&target1), None), + merge_ref_targets(index, Some(&target3), Some(&target1), None), Some(RefTarget::Conflict { removes: vec![commit1.id().clone()], adds: vec![commit3.id().clone()] @@ -184,7 +183,7 @@ fn test_merge_ref_targets() { // Left became conflicted, right moved forward assert_eq!( merge_ref_targets( - index_ref, + index, Some(&RefTarget::Conflict { removes: vec![commit2.id().clone()], adds: vec![commit3.id().clone(), commit4.id().clone()] @@ -202,7 +201,7 @@ fn test_merge_ref_targets() { // Right became conflicted, left moved forward assert_eq!( merge_ref_targets( - index_ref, + index, Some(&target3), Some(&target1), Some(&RefTarget::Conflict { @@ -219,7 +218,7 @@ fn test_merge_ref_targets() { // Existing conflict on left, right moves an "add" sideways assert_eq!( merge_ref_targets( - index_ref, + index, Some(&RefTarget::Conflict { removes: vec![commit2.id().clone()], adds: vec![commit3.id().clone(), commit4.id().clone()] @@ -236,7 +235,7 @@ fn test_merge_ref_targets() { // Existing conflict on right, left moves an "add" sideways assert_eq!( merge_ref_targets( - index_ref, + index, Some(&target5), Some(&target3), Some(&RefTarget::Conflict { @@ -254,7 +253,7 @@ fn test_merge_ref_targets() { // divergence assert_eq!( merge_ref_targets( - index_ref, + index, Some(&RefTarget::Conflict { removes: vec![commit2.id().clone()], adds: vec![commit3.id().clone(), commit4.id().clone()] @@ -272,7 +271,7 @@ fn test_merge_ref_targets() { // divergence assert_eq!( merge_ref_targets( - index_ref, + index, Some(&target1), Some(&target3), Some(&RefTarget::Conflict { @@ -289,7 +288,7 @@ fn test_merge_ref_targets() { // Existing conflict on left, right undoes one side of conflict assert_eq!( merge_ref_targets( - index_ref, + index, Some(&RefTarget::Conflict { removes: vec![commit2.id().clone()], adds: vec![commit3.id().clone(), commit4.id().clone()] @@ -303,7 +302,7 @@ fn test_merge_ref_targets() { // Existing conflict on right, left undoes one side of conflict assert_eq!( merge_ref_targets( - index_ref, + index, Some(&target2), Some(&target3), Some(&RefTarget::Conflict { @@ -317,7 +316,7 @@ fn test_merge_ref_targets() { // Existing conflict on left, right makes unrelated update assert_eq!( merge_ref_targets( - index_ref, + index, Some(&RefTarget::Conflict { removes: vec![commit2.id().clone()], adds: vec![commit3.id().clone(), commit4.id().clone()] @@ -338,7 +337,7 @@ fn test_merge_ref_targets() { // Existing conflict on right, left makes unrelated update assert_eq!( merge_ref_targets( - index_ref, + index, Some(&target6), Some(&target5), Some(&RefTarget::Conflict { diff --git a/lib/tests/test_revset_graph_iterator.rs b/lib/tests/test_revset_graph_iterator.rs index 6ab11b4fe..e706cc981 100644 --- a/lib/tests/test_revset_graph_iterator.rs +++ b/lib/tests/test_revset_graph_iterator.rs @@ -13,7 +13,6 @@ // limitations under the License. use itertools::Itertools; -use jujutsu_lib::index::Index; use jujutsu_lib::revset::revset_for_commits; use jujutsu_lib::revset_graph_iterator::RevsetGraphEdge; use test_case::test_case; diff --git a/src/commands/branch.rs b/src/commands/branch.rs index 858420bd0..af456085e 100644 --- a/src/commands/branch.rs +++ b/src/commands/branch.rs @@ -3,7 +3,6 @@ use std::collections::BTreeSet; use clap::builder::NonEmptyStringValueParser; use itertools::Itertools; use jujutsu_lib::backend::{CommitId, ObjectId}; -use jujutsu_lib::index::Index; use jujutsu_lib::op_store::RefTarget; use jujutsu_lib::repo::RepoRef; use jujutsu_lib::view::View; diff --git a/src/commands/git.rs b/src/commands/git.rs index 056750cc7..1c33a1dad 100644 --- a/src/commands/git.rs +++ b/src/commands/git.rs @@ -10,7 +10,6 @@ use clap::{ArgGroup, Subcommand}; use itertools::Itertools; use jujutsu_lib::backend::ObjectId; use jujutsu_lib::git::{self, GitFetchError, GitRefUpdate}; -use jujutsu_lib::index::Index; use jujutsu_lib::op_store::{BranchTarget, RefTarget}; use jujutsu_lib::refs::{classify_branch_push_action, BranchPushAction, BranchPushUpdate}; use jujutsu_lib::repo::RepoRef; diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 857a4addd..bc7897a30 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -1818,7 +1818,7 @@ fn cmd_duplicate( let mut tx = workspace_command .start_transaction(&format!("duplicating {} commit(s)", to_duplicate.len())); - let index = tx.base_repo().index().clone(); + let index = tx.base_repo().readonly_index().clone(); let store = tx.base_repo().store().clone(); let mut_repo = tx.mut_repo();