From 25b922cd0ba642de5d81d1356905f73bc1afc5b6 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Tue, 12 Jul 2022 22:04:04 -0700 Subject: [PATCH] store: reduce duplication in `Workspace` and `Repo` init code There is a bit of duplicated across the three functions for creating a `Workspace` and `Repo`. This patch reduces that duplication by passing in a closure. In addition to reducing duplication, this is a step towards making it easier to add new backends. --- lib/src/repo.rs | 37 +++++++++++++++------------------ lib/src/store.rs | 17 +-------------- lib/src/testutils.rs | 10 +++++++-- lib/src/workspace.rs | 49 ++++++++++++++++++-------------------------- 4 files changed, 46 insertions(+), 67 deletions(-) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 2a205959d..08e691201 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -23,12 +23,14 @@ use std::sync::{Arc, Mutex}; use itertools::Itertools; use thiserror::Error; -use crate::backend::{BackendError, ChangeId, CommitId}; +use crate::backend::{Backend, BackendError, ChangeId, CommitId}; use crate::commit::Commit; use crate::commit_builder::CommitBuilder; use crate::dag_walk::topo_order_reverse; +use crate::git_backend::GitBackend; use crate::index::{IndexRef, MutableIndex, ReadonlyIndex}; use crate::index_store::IndexStore; +use crate::local_backend::LocalBackend; use crate::op_heads_store::{LockedOpHeads, OpHeads, OpHeadsStore}; use crate::op_store::{BranchTarget, OpStore, OperationId, RefTarget, WorkspaceId}; use crate::operation::Operation; @@ -127,18 +129,16 @@ impl Debug for ReadonlyRepo { impl ReadonlyRepo { pub fn init_local(settings: &UserSettings, repo_path: PathBuf) -> Arc { - let repo_path = repo_path.canonicalize().unwrap(); - ReadonlyRepo::init_repo_dir(&repo_path); - let store = Store::init_local(repo_path.join("store")); - ReadonlyRepo::init(settings, repo_path, store) + Self::init(settings, repo_path, |store_path| { + Box::new(LocalBackend::init(store_path)) + }) } /// Initializes a repo with a new Git backend in .jj/git/ (bare Git repo) pub fn init_internal_git(settings: &UserSettings, repo_path: PathBuf) -> Arc { - let repo_path = repo_path.canonicalize().unwrap(); - ReadonlyRepo::init_repo_dir(&repo_path); - let store = Store::init_internal_git(repo_path.join("store")); - ReadonlyRepo::init(settings, repo_path, store) + Self::init(settings, repo_path, |store_path| { + Box::new(GitBackend::init_internal(store_path)) + }) } /// Initializes a repo with an existing Git backend at the specified path @@ -147,10 +147,9 @@ impl ReadonlyRepo { repo_path: PathBuf, git_repo_path: PathBuf, ) -> Arc { - let repo_path = repo_path.canonicalize().unwrap(); - ReadonlyRepo::init_repo_dir(&repo_path); - let store = Store::init_external_git(repo_path.join("store"), git_repo_path); - ReadonlyRepo::init(settings, repo_path, store) + Self::init(settings, repo_path, |store_path| { + Box::new(GitBackend::init_external(store_path, git_repo_path.clone())) + }) } fn init_repo_dir(repo_path: &Path) { @@ -160,15 +159,16 @@ impl ReadonlyRepo { fs::create_dir(repo_path.join("index")).unwrap(); } - fn init( + pub fn init( user_settings: &UserSettings, repo_path: PathBuf, - store: Arc, + backend_factory: impl FnOnce(PathBuf) -> Box, ) -> Arc { + let repo_path = repo_path.canonicalize().unwrap(); + ReadonlyRepo::init_repo_dir(&repo_path); + let store = Store::new(backend_factory(repo_path.join("store"))); let repo_settings = user_settings.with_repo(&repo_path).unwrap(); - let op_store: Arc = Arc::new(SimpleOpStore::init(repo_path.join("op_store"))); - let mut root_view = op_store::View::default(); root_view.head_ids.insert(store.root_commit_id().clone()); root_view @@ -177,11 +177,8 @@ impl ReadonlyRepo { let (op_heads_store, init_op) = OpHeadsStore::init(repo_path.join("op_heads"), &op_store, &root_view); let op_heads_store = Arc::new(op_heads_store); - let index_store = Arc::new(IndexStore::init(repo_path.join("index"))); - let view = View::new(root_view); - Arc::new(ReadonlyRepo { repo_path, store, diff --git a/lib/src/store.rs b/lib/src/store.rs index 3e7a0f53a..1552f1b27 100644 --- a/lib/src/store.rs +++ b/lib/src/store.rs @@ -40,7 +40,7 @@ pub struct Store { } impl Store { - fn new(backend: Box) -> Arc { + pub fn new(backend: Box) -> Arc { let root_commit_id = CommitId::new(vec![0; backend.hash_length()]); Arc::new(Store { backend, @@ -50,21 +50,6 @@ impl Store { }) } - pub fn init_local(store_path: PathBuf) -> Arc { - Store::new(Box::new(LocalBackend::init(store_path))) - } - - pub fn init_internal_git(store_path: PathBuf) -> Arc { - Store::new(Box::new(GitBackend::init_internal(store_path))) - } - - pub fn init_external_git(store_path: PathBuf, git_repo_path: PathBuf) -> Arc { - Store::new(Box::new(GitBackend::init_external( - store_path, - git_repo_path, - ))) - } - pub fn load_store(store_path: PathBuf) -> Arc { let git_target_path = store_path.join("git_target"); let backend: Box = if git_target_path.is_file() { diff --git a/lib/src/testutils.rs b/lib/src/testutils.rs index eb9c3005b..d2b89d030 100644 --- a/lib/src/testutils.rs +++ b/lib/src/testutils.rs @@ -24,6 +24,8 @@ use tempfile::TempDir; use crate::backend::{FileId, TreeId, TreeValue}; use crate::commit::Commit; use crate::commit_builder::CommitBuilder; +use crate::git_backend::GitBackend; +use crate::local_backend::LocalBackend; use crate::repo::{MutableRepo, ReadonlyRepo}; use crate::repo_path::RepoPath; use crate::rewrite::RebasedDescendant; @@ -68,9 +70,13 @@ impl TestRepo { let repo = if use_git { let git_path = temp_dir.path().join("git-repo"); git2::Repository::init(&git_path).unwrap(); - ReadonlyRepo::init_external_git(&settings, repo_dir, git_path) + ReadonlyRepo::init(&settings, repo_dir, |store_path| { + Box::new(GitBackend::init_external(store_path, git_path.clone())) + }) } else { - ReadonlyRepo::init_local(&settings, repo_dir) + ReadonlyRepo::init(&settings, repo_dir, |store_path| { + Box::new(LocalBackend::init(store_path)) + }) }; Self { diff --git a/lib/src/workspace.rs b/lib/src/workspace.rs index 1b402b61d..e5a5425ee 100644 --- a/lib/src/workspace.rs +++ b/lib/src/workspace.rs @@ -19,6 +19,9 @@ use std::sync::Arc; use thiserror::Error; +use crate::backend::Backend; +use crate::git_backend::GitBackend; +use crate::local_backend::LocalBackend; use crate::op_store::WorkspaceId; use crate::repo::{ReadonlyRepo, RepoLoader}; use crate::settings::UserSettings; @@ -104,51 +107,39 @@ impl Workspace { user_settings: &UserSettings, workspace_root: PathBuf, ) -> Result<(Self, Arc), WorkspaceInitError> { - let jj_dir = create_jj_dir(&workspace_root)?; - let repo_dir = jj_dir.join("repo"); - std::fs::create_dir(&repo_dir).unwrap(); - let repo = ReadonlyRepo::init_local(user_settings, repo_dir); - let (working_copy, repo) = init_working_copy( - user_settings, - &repo, - &workspace_root, - &jj_dir, - WorkspaceId::default(), - ); - let repo_loader = repo.loader(); - let workspace = Self::new(workspace_root, working_copy, repo_loader); - Ok((workspace, repo)) + Self::init_with_backend(user_settings, workspace_root, |store_path| { + Box::new(LocalBackend::init(store_path)) + }) } pub fn init_internal_git( user_settings: &UserSettings, workspace_root: PathBuf, ) -> Result<(Self, Arc), WorkspaceInitError> { - let jj_dir = create_jj_dir(&workspace_root)?; - let repo_dir = jj_dir.join("repo"); - std::fs::create_dir(&repo_dir).unwrap(); - let repo = ReadonlyRepo::init_internal_git(user_settings, repo_dir); - let (working_copy, repo) = init_working_copy( - user_settings, - &repo, - &workspace_root, - &jj_dir, - WorkspaceId::default(), - ); - let repo_loader = repo.loader(); - let workspace = Workspace::new(workspace_root, working_copy, repo_loader); - Ok((workspace, repo)) + Self::init_with_backend(user_settings, workspace_root, |store_path| { + Box::new(GitBackend::init_internal(store_path)) + }) } pub fn init_external_git( user_settings: &UserSettings, workspace_root: PathBuf, git_repo_path: PathBuf, + ) -> Result<(Self, Arc), WorkspaceInitError> { + Self::init_with_backend(user_settings, workspace_root, |store_path| { + Box::new(GitBackend::init_external(store_path, git_repo_path.clone())) + }) + } + + fn init_with_backend( + user_settings: &UserSettings, + workspace_root: PathBuf, + backend_factory: impl FnOnce(PathBuf) -> Box, ) -> Result<(Self, Arc), WorkspaceInitError> { let jj_dir = create_jj_dir(&workspace_root)?; let repo_dir = jj_dir.join("repo"); std::fs::create_dir(&repo_dir).unwrap(); - let repo = ReadonlyRepo::init_external_git(user_settings, repo_dir, git_repo_path); + let repo = ReadonlyRepo::init(user_settings, repo_dir, backend_factory); let (working_copy, repo) = init_working_copy( user_settings, &repo,