From 37151e0ff9fae89678b60c771279c171e97c0f58 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sun, 26 Feb 2023 12:38:53 -0800 Subject: [PATCH] index: load store based on type recorded in .jj/repo/index/type This is another step towards allowing a custom `jj` binary to have its own index type. We're going to have a server-backed index implementation at Google, for example. --- lib/src/index_store.rs | 6 ++++ lib/src/repo.rs | 62 ++++++++++++++++++++++++++++++++++++---- lib/src/transaction.rs | 1 - lib/src/workspace.rs | 4 +++ lib/tests/test_git.rs | 3 ++ lib/tests/test_index.rs | 26 +++++++++++++++++ lib/testutils/src/lib.rs | 2 ++ 7 files changed, 97 insertions(+), 7 deletions(-) diff --git a/lib/src/index_store.rs b/lib/src/index_store.rs index 2b42d8ce7..3e05bfadc 100644 --- a/lib/src/index_store.rs +++ b/lib/src/index_store.rs @@ -40,6 +40,8 @@ pub enum IndexWriteError { } pub trait IndexStore: Send + Sync + Debug { + fn name(&self) -> &str; + fn get_index_at_op(&self, op: &Operation, store: &Arc) -> Arc; fn write_index( @@ -167,6 +169,10 @@ impl DefaultIndexStore { } impl IndexStore for DefaultIndexStore { + fn name(&self) -> &str { + "default" + } + fn get_index_at_op(&self, op: &Operation, store: &Arc) -> Arc { let op_id_hex = op.id().hex(); let op_id_file = self.dir.join("operations").join(op_id_hex); diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 36faf7ef9..9f0f24453 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -80,7 +80,7 @@ pub struct ReadonlyRepo { op_heads_store: Arc, operation: Operation, settings: RepoSettings, - index_store: Arc, + index_store: Arc, index: OnceCell>, // TODO: This should eventually become part of the index and not be stored fully in memory. change_id_index: OnceCell, @@ -108,12 +108,17 @@ impl ReadonlyRepo { } } + pub fn default_index_store_factory() -> impl FnOnce(&Path) -> Box { + |store_path| Box::new(DefaultIndexStore::init(store_path)) + } + pub fn init( user_settings: &UserSettings, repo_path: &Path, backend_factory: impl FnOnce(&Path) -> Box, op_store_factory: impl FnOnce(&Path) -> Box, op_heads_store_factory: impl FnOnce(&Path) -> Box, + index_store_factory: impl FnOnce(&Path) -> Box, ) -> Result, PathError> { let repo_path = repo_path.canonicalize().context(repo_path)?; @@ -158,7 +163,10 @@ impl ReadonlyRepo { let index_path = repo_path.join("index"); fs::create_dir(&index_path).context(&index_path)?; - let index_store = Arc::new(DefaultIndexStore::init(&index_path)); + let index_store = index_store_factory(&index_path); + let index_type_path = index_path.join("type"); + fs::write(&index_type_path, index_store.name()).context(&index_type_path)?; + let index_store = Arc::from(index_store); let view = View::new(root_view); Ok(Arc::new(ReadonlyRepo { @@ -224,7 +232,7 @@ impl ReadonlyRepo { &self.op_heads_store } - pub fn index_store(&self) -> &Arc { + pub fn index_store(&self) -> &Arc { &self.index_store } @@ -288,11 +296,13 @@ impl Repo for Arc { type BackendFactory = Box Box>; type OpStoreFactory = Box Box>; type OpHeadsStoreFactory = Box Box>; +type IndexStoreFactory = Box Box>; pub struct StoreFactories { backend_factories: HashMap, op_store_factories: HashMap, op_heads_store_factories: HashMap, + index_store_factories: HashMap, } impl Default for StoreFactories { @@ -321,6 +331,12 @@ impl Default for StoreFactories { Box::new(|store_path| Box::new(SimpleOpHeadsStore::load(store_path))), ); + // Index + factories.add_index_store( + "default", + Box::new(|store_path| Box::new(DefaultIndexStore::load(store_path))), + ); + factories } } @@ -345,6 +361,7 @@ impl StoreFactories { backend_factories: HashMap::new(), op_store_factories: HashMap::new(), op_heads_store_factories: HashMap::new(), + index_store_factories: HashMap::new(), } } @@ -448,6 +465,39 @@ impl StoreFactories { })?; Ok(op_heads_store_factory(store_path)) } + + pub fn add_index_store(&mut self, name: &str, factory: IndexStoreFactory) { + self.index_store_factories.insert(name.to_string(), factory); + } + + pub fn load_index_store( + &self, + store_path: &Path, + ) -> Result, StoreLoadError> { + let index_store_type = match fs::read_to_string(store_path.join("type")) { + Ok(content) => content, + Err(err) if err.kind() == ErrorKind::NotFound => { + // For compatibility with existing repos. TODO: Delete in 0.9+ + let default_type = String::from("default"); + fs::write(store_path.join("type"), &default_type).unwrap(); + default_type + } + Err(err) => { + return Err(StoreLoadError::ReadError { + store: "index", + source: err, + }); + } + }; + let index_store_factory = self + .index_store_factories + .get(&index_store_type) + .ok_or_else(|| StoreLoadError::UnsupportedType { + store: "index", + store_type: index_store_type.to_string(), + })?; + Ok(index_store_factory(store_path)) + } } #[derive(Clone)] @@ -457,7 +507,7 @@ pub struct RepoLoader { store: Arc, op_store: Arc, op_heads_store: Arc, - index_store: Arc, + index_store: Arc, } impl RepoLoader { @@ -471,7 +521,7 @@ impl RepoLoader { let op_store = Arc::from(store_factories.load_op_store(&repo_path.join("op_store"))?); let op_heads_store = Arc::from(store_factories.load_op_heads_store(&repo_path.join("op_heads"))?); - let index_store = Arc::new(DefaultIndexStore::load(&repo_path.join("index"))); + let index_store = Arc::from(store_factories.load_index_store(&repo_path.join("index"))?); Ok(Self { repo_path: repo_path.to_path_buf(), repo_settings, @@ -490,7 +540,7 @@ impl RepoLoader { &self.store } - pub fn index_store(&self) -> &Arc { + pub fn index_store(&self) -> &Arc { &self.index_store } diff --git a/lib/src/transaction.rs b/lib/src/transaction.rs index 49dd2b04b..afff05994 100644 --- a/lib/src/transaction.rs +++ b/lib/src/transaction.rs @@ -17,7 +17,6 @@ use std::sync::Arc; use crate::backend::Timestamp; use crate::dag_walk::closest_common_node; use crate::index::ReadonlyIndex; -use crate::index_store::IndexStore; use crate::op_store; use crate::op_store::OperationMetadata; use crate::operation::Operation; diff --git a/lib/src/workspace.rs b/lib/src/workspace.rs index c8f6eed57..cc6154b47 100644 --- a/lib/src/workspace.rs +++ b/lib/src/workspace.rs @@ -21,6 +21,7 @@ use thiserror::Error; use crate::backend::Backend; use crate::git_backend::GitBackend; +use crate::index_store::IndexStore; use crate::local_backend::LocalBackend; use crate::op_heads_store::OpHeadsStore; use crate::op_store::{OpStore, WorkspaceId}; @@ -161,6 +162,7 @@ impl Workspace { backend_factory: impl FnOnce(&Path) -> Box, op_store_factory: impl FnOnce(&Path) -> Box, op_heads_store_factory: impl FnOnce(&Path) -> Box, + index_store_factory: impl FnOnce(&Path) -> Box, ) -> Result<(Self, Arc), WorkspaceInitError> { let jj_dir = create_jj_dir(workspace_root)?; let repo_dir = jj_dir.join("repo"); @@ -171,6 +173,7 @@ impl Workspace { backend_factory, op_store_factory, op_heads_store_factory, + index_store_factory, )?; let (working_copy, repo) = init_working_copy( user_settings, @@ -195,6 +198,7 @@ impl Workspace { backend_factory, ReadonlyRepo::default_op_store_factory(), ReadonlyRepo::default_op_heads_store_factory(), + ReadonlyRepo::default_index_store_factory(), ) } diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index ced3aa40d..fa6e117fc 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -524,6 +524,7 @@ impl GitRepoData { |store_path| Box::new(GitBackend::init_external(store_path, &git_repo_dir)), ReadonlyRepo::default_op_store_factory(), ReadonlyRepo::default_op_heads_store_factory(), + ReadonlyRepo::default_index_store_factory(), ) .unwrap(); Self { @@ -1089,6 +1090,7 @@ fn test_init() { |store_path| Box::new(GitBackend::init_external(store_path, &git_repo_dir)), ReadonlyRepo::default_op_store_factory(), ReadonlyRepo::default_op_heads_store_factory(), + ReadonlyRepo::default_index_store_factory(), ) .unwrap(); // The refs were *not* imported -- it's the caller's responsibility to import @@ -1347,6 +1349,7 @@ fn set_up_push_repos(settings: &UserSettings, temp_dir: &TempDir) -> PushTestSet |store_path| Box::new(GitBackend::init_external(store_path, &clone_repo_dir)), ReadonlyRepo::default_op_store_factory(), ReadonlyRepo::default_op_heads_store_factory(), + ReadonlyRepo::default_index_store_factory(), ) .unwrap(); let mut tx = jj_repo.start_transaction(settings, "test"); diff --git a/lib/tests/test_index.rs b/lib/tests/test_index.rs index 6281f6470..5a18b08b1 100644 --- a/lib/tests/test_index.rs +++ b/lib/tests/test_index.rs @@ -517,3 +517,29 @@ fn test_index_commits_incremental_squashed(use_git: bool) { let repo = create_n_commits(&settings, &repo, 10); assert_eq!(commits_by_level(&repo), vec![71, 20]); } + +/// Test that .jj/repo/index/type is created when the repo is created, and that +/// it is created when an old repo is loaded. +#[test_case(false ; "local backend")] +#[test_case(true ; "git backend")] +fn test_index_store_type(use_git: bool) { + let settings = testutils::user_settings(); + let test_repo = TestRepo::init(use_git); + let repo = &test_repo.repo; + + assert_eq!(repo.index().num_commits(), 1); + let index_store_type_path = repo.repo_path().join("index").join("type"); + assert_eq!( + std::fs::read_to_string(&index_store_type_path).unwrap(), + "default" + ); + // Remove the file to simulate an old repo. Loading the repo should result in + // the file being created. + std::fs::remove_file(&index_store_type_path).unwrap(); + let repo = load_repo_at_head(&settings, repo.repo_path()); + assert_eq!( + std::fs::read_to_string(&index_store_type_path).unwrap(), + "default" + ); + assert_eq!(repo.index().num_commits(), 1); +} diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index c8d752f29..7b9986989 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -98,6 +98,7 @@ impl TestRepo { |store_path| Box::new(GitBackend::init_external(store_path, &git_path)), ReadonlyRepo::default_op_store_factory(), ReadonlyRepo::default_op_heads_store_factory(), + ReadonlyRepo::default_index_store_factory(), ) .unwrap() } else { @@ -107,6 +108,7 @@ impl TestRepo { |store_path| Box::new(LocalBackend::init(store_path)), ReadonlyRepo::default_op_store_factory(), ReadonlyRepo::default_op_heads_store_factory(), + ReadonlyRepo::default_index_store_factory(), ) .unwrap() };