From 00fb670c9cac564ef6a0230008e81b2ecd076606 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Thu, 17 Dec 2020 22:59:39 -0800 Subject: [PATCH] index: make Index::load() return Arc instead of Index This removes one level of indirection, which is nice because it was visible to the callers. The `Index` struct is now empty. The next step is obviously to delete it (and perhaps rename `IndexFile` to `Index` or `ReadonlyIndex`). --- lib/src/index.rs | 16 +++------------- lib/src/repo.rs | 14 +++++--------- lib/tests/test_index.rs | 16 ++++++++-------- src/commands.rs | 12 ++++++------ 4 files changed, 22 insertions(+), 36 deletions(-) diff --git a/lib/src/index.rs b/lib/src/index.rs index ed19493cc..f017d80de 100644 --- a/lib/src/index.rs +++ b/lib/src/index.rs @@ -1103,9 +1103,7 @@ impl IndexFile { } } -pub struct Index { - index_file: Arc, -} +pub struct Index; impl Index { pub fn init(dir: PathBuf) { @@ -1117,7 +1115,7 @@ impl Index { Index::init(dir); } - pub fn load(repo: &ReadonlyRepo, dir: PathBuf, op_id: OperationId) -> Index { + pub fn load(repo: &ReadonlyRepo, dir: PathBuf, op_id: OperationId) -> Arc { let op_id_hex = op_id.hex(); let op_id_file = dir.join("operations").join(&op_id_hex); let index_file = if op_id_file.exists() { @@ -1128,15 +1126,7 @@ impl Index { IndexFile::index(repo.store(), &dir, &op).unwrap() }; - Index { - index_file: Arc::new(index_file), - } - } - - // TODO: Maybe just call this data() or something? We should also hide the - // IndexFile type from the API. - pub fn index_file(&self) -> Arc { - self.index_file.clone() + Arc::new(index_file) } } diff --git a/lib/src/repo.rs b/lib/src/repo.rs index c89386db5..683a783ff 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -24,7 +24,7 @@ use thiserror::Error; use crate::commit_builder::{new_change_id, signature}; use crate::evolution::{Evolution, ReadonlyEvolution}; use crate::git_store::GitStore; -use crate::index::Index; +use crate::index::{Index, IndexFile}; use crate::local_store::LocalStore; use crate::operation::Operation; use crate::settings::{RepoSettings, UserSettings}; @@ -65,7 +65,7 @@ pub struct ReadonlyRepo { wc_path: PathBuf, store: Arc, settings: RepoSettings, - index: Mutex>>, + index: Mutex>>, working_copy: Arc>, view: ReadonlyView, evolution: Option>, @@ -217,20 +217,16 @@ impl ReadonlyRepo { &self.wc_path } - pub fn index(&self) -> Arc { + pub fn index(&self) -> Arc { let mut locked_index = self.index.lock().unwrap(); if locked_index.is_none() { let op_id = self.view.base_op_head_id().clone(); - locked_index.replace(Arc::new(Index::load( - self, - self.repo_path.join("index"), - op_id, - ))); + locked_index.replace(Index::load(self, self.repo_path.join("index"), op_id)); } locked_index.as_ref().unwrap().clone() } - pub fn reindex(&mut self) -> Arc { + pub fn reindex(&mut self) -> Arc { Index::reinit(self.repo_path.join("index")); { let mut locked_index = self.index.lock().unwrap(); diff --git a/lib/tests/test_index.rs b/lib/tests/test_index.rs index 359713c7a..248f1d45c 100644 --- a/lib/tests/test_index.rs +++ b/lib/tests/test_index.rs @@ -38,7 +38,7 @@ fn test_index_commits_empty_repo(use_git: bool) { let settings = testutils::user_settings(); let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); - let index = repo.index().index_file(); + let index = repo.index(); let index = index.as_composite(); // There should be the root commit and the working copy commit assert_eq!(index.num_commits(), 2); @@ -87,7 +87,7 @@ fn test_index_commits_standard_cases(use_git: bool) { tx.commit(); Arc::get_mut(&mut repo).unwrap().reload(); - let index = repo.index().index_file(); + let index = repo.index(); let index = index.as_composite(); // There should be the root commit and the working copy commit, plus // 8 more @@ -160,7 +160,7 @@ fn test_index_commits_criss_cross(use_git: bool) { tx.commit(); Arc::get_mut(&mut repo).unwrap().reload(); - let index = repo.index().index_file(); + let index = repo.index(); let index = index.as_composite(); // There should the root commit and the working copy commit, plus 2 for each // generation @@ -276,7 +276,7 @@ fn test_index_commits_previous_operations(use_git: bool) { std::fs::create_dir(&index_operations_dir).unwrap(); let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()); - let index = repo.index().index_file(); + let index = repo.index(); let index = index.as_composite(); // There should be the root commit and the working copy commit, plus // 3 more @@ -312,7 +312,7 @@ fn test_index_commits_incremental(use_git: bool) { child_commit(&settings, &repo, &root_commit).write_to_new_transaction(&repo, "test"); Arc::get_mut(&mut repo).unwrap().reload(); - let index = repo.index().index_file(); + let index = repo.index(); let index = index.as_composite(); // There should be the root commit and the working copy commit, plus // 1 more @@ -324,7 +324,7 @@ fn test_index_commits_incremental(use_git: bool) { tx.commit(); let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()); - let index = repo.index().index_file(); + let index = repo.index(); let index = index.as_composite(); // There should be the root commit and the working copy commit, plus // 3 more @@ -362,7 +362,7 @@ fn test_index_commits_incremental_empty_transaction(use_git: bool) { child_commit(&settings, &repo, &root_commit).write_to_new_transaction(&repo, "test"); Arc::get_mut(&mut repo).unwrap().reload(); - let index = repo.index().index_file(); + let index = repo.index(); let index = index.as_composite(); // There should be the root commit and the working copy commit, plus // 1 more @@ -371,7 +371,7 @@ fn test_index_commits_incremental_empty_transaction(use_git: bool) { repo.start_transaction("test").commit(); let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()); - let index = repo.index().index_file(); + let index = repo.index(); let index = index.as_composite(); // There should be the root commit and the working copy commit, plus // 1 more diff --git a/src/commands.rs b/src/commands.rs index 91cf0e764..60bde508c 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -81,7 +81,7 @@ fn resolve_commit_id_prefix( repo: &ReadonlyRepo, prefix: &HexPrefix, ) -> Result { - let index = repo.index().index_file(); + let index = repo.index(); match index.as_composite().resolve_prefix(prefix) { PrefixResolution::NoMatch => Err(CommandError::UserError(String::from("No such commit"))), PrefixResolution::AmbiguousMatch => { @@ -1564,7 +1564,7 @@ fn cmd_debug( writeln!(ui, "{:?}", parse); } else if let Some(_reindex_matches) = sub_matches.subcommand_matches("index") { let repo = get_repo(ui, &matches)?; - let index = repo.index().index_file(); + let index = repo.index(); let stats = index.as_composite().stats(); writeln!(ui, "Number of commits: {}", stats.num_commits); writeln!(ui, "Number of merges: {}", stats.num_merges); @@ -1583,7 +1583,7 @@ fn cmd_debug( writeln!( ui, "Finished indexing {:?} commits.", - index.index_file().as_composite().num_commits() + index.as_composite().num_commits() ); } else { panic!("unhandled command: {:#?}", matches); @@ -1619,7 +1619,7 @@ fn cmd_bench( mut_repo, command_matches.value_of("descendant").unwrap(), )?; - let index = repo.index().index_file(); + let index = repo.index(); let index = index.as_composite(); let routine = || index.is_ancestor(ancestor_commit.id(), descendant_commit.id()); writeln!(ui, "Result: {:?}", routine()); @@ -1633,7 +1633,7 @@ fn cmd_bench( resolve_single_rev(ui, mut_repo, command_matches.value_of("unwanted").unwrap())?; let wanted_commit = resolve_single_rev(ui, mut_repo, command_matches.value_of("wanted").unwrap())?; - let index = repo.index().index_file(); + let index = repo.index(); let index = index.as_composite(); let routine = || { index @@ -1650,7 +1650,7 @@ fn cmd_bench( } else if let Some(command_matches) = sub_matches.subcommand_matches("resolveprefix") { let repo = get_repo(ui, &matches)?; let prefix = HexPrefix::new(command_matches.value_of("prefix").unwrap().to_string()); - let index = repo.index().index_file(); + let index = repo.index(); let index = index.as_composite(); let routine = || index.resolve_prefix(&prefix); writeln!(ui, "Result: {:?}", routine());