From ea32c0cb9ed363f08f81f2d3bade01c61881d5da Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 11 Nov 2023 09:16:40 +0900 Subject: [PATCH] git_backend: pass UserSettings to GitBackend constructors --- cli/examples/custom-backend/main.rs | 13 ++++---- cli/examples/custom-working-copy/main.rs | 5 +-- lib/src/git_backend.rs | 40 +++++++++++++++++++----- lib/src/repo.rs | 2 +- lib/src/workspace.rs | 16 ++++++---- lib/tests/test_git.rs | 9 ++++-- lib/testutils/src/lib.rs | 12 ++++--- 7 files changed, 67 insertions(+), 30 deletions(-) diff --git a/cli/examples/custom-backend/main.rs b/cli/examples/custom-backend/main.rs index 39c06e2ac..bd55d5f31 100644 --- a/cli/examples/custom-backend/main.rs +++ b/cli/examples/custom-backend/main.rs @@ -26,6 +26,7 @@ use jj_lib::backend::{ use jj_lib::git_backend::GitBackend; use jj_lib::repo::StoreFactories; use jj_lib::repo_path::RepoPath; +use jj_lib::settings::UserSettings; use jj_lib::workspace::Workspace; #[derive(clap::Parser, Clone, Debug)] @@ -40,7 +41,7 @@ fn create_store_factories() -> StoreFactories { // must match `Backend::name()`. store_factories.add_backend( "jit", - Box::new(|_settings, store_path| Ok(Box::new(JitBackend::load(store_path)?))), + Box::new(|settings, store_path| Ok(Box::new(JitBackend::load(settings, store_path)?))), ); store_factories } @@ -57,7 +58,7 @@ fn run_custom_command( Workspace::init_with_backend( command_helper.settings(), wc_path, - &|_settings, store_path| Ok(Box::new(JitBackend::init(store_path)?)), + &|settings, store_path| Ok(Box::new(JitBackend::init(settings, store_path)?)), )?; Ok(()) } @@ -78,13 +79,13 @@ struct JitBackend { } impl JitBackend { - fn init(store_path: &Path) -> Result { - let inner = GitBackend::init_internal(store_path)?; + fn init(settings: &UserSettings, store_path: &Path) -> Result { + let inner = GitBackend::init_internal(settings, store_path)?; Ok(JitBackend { inner }) } - fn load(store_path: &Path) -> Result { - let inner = GitBackend::load(store_path)?; + fn load(settings: &UserSettings, store_path: &Path) -> Result { + let inner = GitBackend::load(settings, store_path)?; Ok(JitBackend { inner }) } } diff --git a/cli/examples/custom-working-copy/main.rs b/cli/examples/custom-working-copy/main.rs index 8bb41d293..68c2f1937 100644 --- a/cli/examples/custom-working-copy/main.rs +++ b/cli/examples/custom-working-copy/main.rs @@ -49,8 +49,9 @@ fn run_custom_command( match command { CustomCommands::InitConflicts => { let wc_path = command_helper.cwd(); - let backend_initializer = |_settings: &UserSettings, store_path: &Path| { - let backend: Box = Box::new(GitBackend::init_internal(store_path)?); + let backend_initializer = |settings: &UserSettings, store_path: &Path| { + let backend: Box = + Box::new(GitBackend::init_internal(settings, store_path)?); Ok(backend) }; Workspace::init_with_factories( diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index 1ad9bcef1..daec187d7 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -35,6 +35,7 @@ use crate::file_util::{IoResultExt as _, PathError}; use crate::lock::FileLock; use crate::merge::{Merge, MergeBuilder}; use crate::repo_path::{RepoPath, RepoPathComponent}; +use crate::settings::UserSettings; use crate::stacked_table::{ MutableTable, ReadonlyTable, TableSegment, TableStore, TableStoreError, }; @@ -125,7 +126,10 @@ impl GitBackend { } } - pub fn init_internal(store_path: &Path) -> Result> { + pub fn init_internal( + _settings: &UserSettings, + store_path: &Path, + ) -> Result> { let git_repo_path = Path::new("git"); let git_repo = gix::ThreadSafeRepository::init( store_path.join(git_repo_path), @@ -139,6 +143,7 @@ impl GitBackend { /// Initializes backend by creating a new Git repo at the specified /// workspace path. The workspace directory must exist. pub fn init_colocated( + _settings: &UserSettings, store_path: &Path, workspace_root: &Path, ) -> Result> { @@ -160,6 +165,7 @@ impl GitBackend { /// Initializes backend with an existing Git repo at the specified path. pub fn init_external( + _settings: &UserSettings, store_path: &Path, git_repo_path: &Path, ) -> Result> { @@ -207,7 +213,10 @@ impl GitBackend { Ok(GitBackend::new(git_repo, extra_metadata_store)) } - pub fn load(store_path: &Path) -> Result> { + pub fn load( + _settings: &UserSettings, + store_path: &Path, + ) -> Result> { let git_repo_path = { let target_path = store_path.join("git_target"); let git_repo_path_str = fs::read_to_string(&target_path) @@ -1051,6 +1060,7 @@ mod tests { #[test_case(false; "legacy tree format")] #[test_case(true; "tree-level conflict format")] fn read_plain_git_commit(uses_tree_conflict_format: bool) { + let settings = user_settings(); let temp_dir = testutils::new_temp_dir(); let store_path = temp_dir.path(); let git_repo_path = temp_dir.path().join("git"); @@ -1110,7 +1120,7 @@ mod tests { .unwrap(); let commit_id2 = CommitId::from_bytes(git_commit_id2.as_bytes()); - let backend = GitBackend::init_external(store_path, &git_repo_path).unwrap(); + let backend = GitBackend::init_external(&settings, store_path, &git_repo_path).unwrap(); // Import the head commit and its ancestors backend @@ -1212,6 +1222,7 @@ mod tests { #[test] fn read_git_commit_without_importing() { + let settings = user_settings(); let temp_dir = testutils::new_temp_dir(); let store_path = temp_dir.path(); let git_repo_path = temp_dir.path().join("git"); @@ -1231,7 +1242,7 @@ mod tests { ) .unwrap(); - let backend = GitBackend::init_external(store_path, &git_repo_path).unwrap(); + let backend = GitBackend::init_external(&settings, store_path, &git_repo_path).unwrap(); // read_commit() without import_head_commits() works as of now. This might be // changed later. @@ -1298,12 +1309,13 @@ mod tests { /// Test that parents get written correctly #[test] fn git_commit_parents() { + let settings = user_settings(); let temp_dir = testutils::new_temp_dir(); let store_path = temp_dir.path(); let git_repo_path = temp_dir.path().join("git"); let git_repo = git2::Repository::init(&git_repo_path).unwrap(); - let backend = GitBackend::init_external(store_path, &git_repo_path).unwrap(); + let backend = GitBackend::init_external(&settings, store_path, &git_repo_path).unwrap(); let mut commit = Commit { parents: vec![], predecessors: vec![], @@ -1361,12 +1373,13 @@ mod tests { #[test] fn write_tree_conflicts() { + let settings = user_settings(); let temp_dir = testutils::new_temp_dir(); let store_path = temp_dir.path(); let git_repo_path = temp_dir.path().join("git"); let git_repo = git2::Repository::init(&git_repo_path).unwrap(); - let backend = GitBackend::init_external(store_path, &git_repo_path).unwrap(); + let backend = GitBackend::init_external(&settings, store_path, &git_repo_path).unwrap(); let create_tree = |i| { let blob_id = git_repo.blob(b"content {i}").unwrap(); let mut tree_builder = git_repo.treebuilder(None).unwrap(); @@ -1450,8 +1463,9 @@ mod tests { #[test] fn commit_has_ref() { + let settings = user_settings(); let temp_dir = testutils::new_temp_dir(); - let backend = GitBackend::init_internal(temp_dir.path()).unwrap(); + let backend = GitBackend::init_internal(&settings, temp_dir.path()).unwrap(); let signature = Signature { name: "Someone".to_string(), email: "someone@example.com".to_string(), @@ -1482,8 +1496,9 @@ mod tests { #[test] fn overlapping_git_commit_id() { + let settings = user_settings(); let temp_dir = testutils::new_temp_dir(); - let backend = GitBackend::init_internal(temp_dir.path()).unwrap(); + let backend = GitBackend::init_internal(&settings, temp_dir.path()).unwrap(); let mut commit1 = Commit { parents: vec![backend.root_commit_id().clone()], predecessors: vec![], @@ -1533,4 +1548,13 @@ mod tests { }, } } + + // Not using testutils::user_settings() because there is a dependency cycle + // 'jj_lib (1) -> testutils -> jj_lib (2)' which creates another distinct + // UserSettings type. testutils returns jj_lib (2)'s UserSettings, whereas + // our UserSettings type comes from jj_lib (1). + fn user_settings() -> UserSettings { + let config = config::Config::default(); + UserSettings::from_config(config) + } } diff --git a/lib/src/repo.rs b/lib/src/repo.rs index b3672fa4f..5a14378e3 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -376,7 +376,7 @@ impl Default for StoreFactories { ); factories.add_backend( GitBackend::name(), - Box::new(|_settings, store_path| Ok(Box::new(GitBackend::load(store_path)?))), + Box::new(|settings, store_path| Ok(Box::new(GitBackend::load(settings, store_path)?))), ); // OpStores diff --git a/lib/src/workspace.rs b/lib/src/workspace.rs index 5665180a2..36b11bfed 100644 --- a/lib/src/workspace.rs +++ b/lib/src/workspace.rs @@ -156,7 +156,7 @@ impl Workspace { workspace_root: &Path, ) -> Result<(Self, Arc), WorkspaceInitError> { let backend_initializer: &'static BackendInitializer = - &|_settings, store_path| Ok(Box::new(GitBackend::init_internal(store_path)?)); + &|settings, store_path| Ok(Box::new(GitBackend::init_internal(settings, store_path)?)); Self::init_with_backend(user_settings, workspace_root, backend_initializer) } @@ -168,7 +168,7 @@ impl Workspace { ) -> Result<(Self, Arc), WorkspaceInitError> { let backend_initializer = { let workspace_root = workspace_root.to_owned(); - move |_settings: &UserSettings, store_path: &Path| -> Result, _> { + move |settings: &UserSettings, store_path: &Path| -> Result, _> { // TODO: Clean up path normalization. store_path is canonicalized by // ReadonlyRepo::init(). workspace_root will be canonicalized by // Workspace::new(), but it's not yet here. @@ -178,8 +178,11 @@ impl Workspace { } else { workspace_root.to_owned() }; - let backend = - GitBackend::init_colocated(store_path, &store_relative_workspace_root)?; + let backend = GitBackend::init_colocated( + settings, + store_path, + &store_relative_workspace_root, + )?; Ok(Box::new(backend)) } }; @@ -195,7 +198,7 @@ impl Workspace { let backend_initializer = { let workspace_root = workspace_root.to_owned(); let git_repo_path = git_repo_path.to_owned(); - move |_settings: &UserSettings, store_path: &Path| -> Result, _> { + move |settings: &UserSettings, store_path: &Path| -> Result, _> { // If the git repo is inside the workspace, use a relative path to it so the // whole workspace can be moved without breaking. // TODO: Clean up path normalization. store_path is canonicalized by @@ -210,7 +213,8 @@ impl Workspace { } _ => git_repo_path.to_owned(), }; - let backend = GitBackend::init_external(store_path, &store_relative_git_repo_path)?; + let backend = + GitBackend::init_external(settings, store_path, &store_relative_git_repo_path)?; Ok(Box::new(backend)) } }; diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 59c4f6f8e..440522b3d 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -1119,8 +1119,9 @@ impl GitRepoData { let repo = ReadonlyRepo::init( &settings, &jj_repo_dir, - &move |_settings, store_path| { + &move |settings, store_path| { Ok(Box::new(GitBackend::init_external( + settings, store_path, &git_repo_dir, )?)) @@ -1969,8 +1970,9 @@ fn test_init() { let repo = &ReadonlyRepo::init( &settings, &jj_repo_dir, - &move |_settings, store_path| { + &move |settings, store_path| { Ok(Box::new(GitBackend::init_external( + settings, store_path, &git_repo_dir, )?)) @@ -2293,8 +2295,9 @@ fn set_up_push_repos(settings: &UserSettings, temp_dir: &TempDir) -> PushTestSet let jj_repo = ReadonlyRepo::init( settings, &jj_repo_dir, - &move |_settings, store_path| { + &move |settings, store_path| { Ok(Box::new(GitBackend::init_external( + settings, store_path, &clone_repo_dir, )?)) diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index 2b0ffa678..f129785a1 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -110,9 +110,13 @@ pub enum TestRepoBackend { } impl TestRepoBackend { - fn init_backend(&self, store_path: &Path) -> Result, BackendInitError> { + fn init_backend( + &self, + settings: &UserSettings, + store_path: &Path, + ) -> Result, BackendInitError> { match self { - TestRepoBackend::Git => Ok(Box::new(GitBackend::init_internal(store_path)?)), + TestRepoBackend::Git => Ok(Box::new(GitBackend::init_internal(settings, store_path)?)), TestRepoBackend::Local => Ok(Box::new(LocalBackend::init(store_path))), TestRepoBackend::Test => Ok(Box::new(TestBackend::init(store_path))), } @@ -134,7 +138,7 @@ impl TestRepo { let repo = ReadonlyRepo::init( &settings, &repo_dir, - &move |_settings, store_path| backend.init_backend(store_path), + &move |settings, store_path| backend.init_backend(settings, store_path), ReadonlyRepo::default_op_store_initializer(), ReadonlyRepo::default_op_heads_store_initializer(), ReadonlyRepo::default_index_store_initializer(), @@ -179,7 +183,7 @@ impl TestWorkspace { let (workspace, repo) = Workspace::init_with_backend( settings, &workspace_root, - &move |_settings, store_path| backend.init_backend(store_path), + &move |settings, store_path| backend.init_backend(settings, store_path), ) .unwrap();