From f963af3f162dfd7c8ade593697993b76d8154e1b Mon Sep 17 00:00:00 2001 From: dploch Date: Thu, 29 Aug 2024 16:57:58 -0400 Subject: [PATCH] workspace: turn WorkspaceLoader into a trait Like https://github.com/martinvonz/jj/pull/4189, this allows extensions the ability to load the repo in an environment where the local filesystem is not accessible. This change allows such extensions to exist at the CLI layer where jj is invoked as a subprocess, rather than a library (common in testing). --- cli/src/cli_util.rs | 32 +++++++++---- lib/src/workspace.rs | 107 +++++++++++++++++++++++++++++++------------ 2 files changed, 102 insertions(+), 37 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 0bb8f4c97..9ab9ea234 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -113,11 +113,14 @@ use jj_lib::working_copy::SnapshotOptions; use jj_lib::working_copy::WorkingCopy; use jj_lib::working_copy::WorkingCopyFactory; use jj_lib::workspace::default_working_copy_factories; +use jj_lib::workspace::get_working_copy_factory; +use jj_lib::workspace::DefaultWorkspaceLoaderFactory; use jj_lib::workspace::LockedWorkspace; use jj_lib::workspace::WorkingCopyFactories; use jj_lib::workspace::Workspace; use jj_lib::workspace::WorkspaceLoadError; use jj_lib::workspace::WorkspaceLoader; +use jj_lib::workspace::WorkspaceLoaderFactory; use once_cell::unsync::OnceCell; use tracing::instrument; use tracing_chrome::ChromeLayerBuilder; @@ -263,7 +266,7 @@ pub struct CommandHelper { revset_extensions: Arc, commit_template_extensions: Vec>, operation_template_extensions: Vec>, - maybe_workspace_loader: Result, + maybe_workspace_loader: Result, CommandError>, store_factories: StoreFactories, working_copy_factories: WorkingCopyFactories, } @@ -341,8 +344,8 @@ impl CommandHelper { &self.operation_template_extensions } - pub fn workspace_loader(&self) -> Result<&WorkspaceLoader, CommandError> { - self.maybe_workspace_loader.as_ref().map_err(Clone::clone) + pub fn workspace_loader(&self) -> Result<&dyn WorkspaceLoader, CommandError> { + self.maybe_workspace_loader.as_deref().map_err(Clone::clone) } /// Loads workspace and repo, then snapshots the working copy if allowed. @@ -370,9 +373,8 @@ impl CommandHelper { let loader = self.workspace_loader()?; // We convert StoreLoadError -> WorkspaceLoadError -> CommandError - let factory: Result<_, WorkspaceLoadError> = loader - .get_working_copy_factory(&self.working_copy_factories) - .map_err(|e| e.into()); + let factory: Result<_, WorkspaceLoadError> = + get_working_copy_factory(loader, &self.working_copy_factories).map_err(|e| e.into()); let factory = factory .map_err(|err| map_workspace_load_error(err, self.global_args.repository.as_deref()))?; Ok(factory) @@ -2824,6 +2826,7 @@ pub struct CliRunner { extra_configs: Vec, store_factories: StoreFactories, working_copy_factories: WorkingCopyFactories, + workspace_loader_factory: Box, revset_extensions: RevsetExtensions, commit_template_extensions: Vec>, operation_template_extensions: Vec>, @@ -2848,6 +2851,7 @@ impl CliRunner { extra_configs: vec![], store_factories: StoreFactories::default(), working_copy_factories: default_working_copy_factories(), + workspace_loader_factory: Box::new(DefaultWorkspaceLoaderFactory), revset_extensions: Default::default(), commit_template_extensions: vec![], operation_template_extensions: vec![], @@ -2896,6 +2900,14 @@ impl CliRunner { self } + pub fn set_workspace_loader_factory( + mut self, + workspace_loader_factory: Box, + ) -> Self { + self.workspace_loader_factory = workspace_loader_factory; + self + } + pub fn add_symbol_resolver_extension( mut self, symbol_resolver: Box, @@ -2990,7 +3002,9 @@ impl CliRunner { // Use cwd-relative workspace configs to resolve default command and // aliases. WorkspaceLoader::init() won't do any heavy lifting other // than the path resolution. - let maybe_cwd_workspace_loader = WorkspaceLoader::init(find_workspace_dir(&cwd)) + let maybe_cwd_workspace_loader = self + .workspace_loader_factory + .create(find_workspace_dir(&cwd)) .map_err(|err| map_workspace_load_error(err, None)); layered_configs.read_user_config()?; let mut repo_config_path = None; @@ -3024,7 +3038,9 @@ impl CliRunner { let maybe_workspace_loader = if let Some(path) = &args.global_args.repository { // Invalid -R path is an error. No need to proceed. - let loader = WorkspaceLoader::init(&cwd.join(path)) + let loader = self + .workspace_loader_factory + .create(&cwd.join(path)) .map_err(|err| map_workspace_load_error(err, Some(path)))?; layered_configs.read_repo_config(loader.repo_path())?; Ok(loader) diff --git a/lib/src/workspace.rs b/lib/src/workspace.rs index e3fdf12d5..048958faa 100644 --- a/lib/src/workspace.rs +++ b/lib/src/workspace.rs @@ -376,7 +376,7 @@ impl Workspace { store_factories: &StoreFactories, working_copy_factories: &WorkingCopyFactories, ) -> Result { - let loader = WorkspaceLoader::init(workspace_path)?; + let loader = DefaultWorkspaceLoader::new(workspace_path)?; let workspace = loader.load(user_settings, store_factories, working_copy_factories)?; Ok(workspace) } @@ -460,8 +460,69 @@ impl<'a> LockedWorkspace<'a> { } } +// Factory trait to build WorkspaceLoaders given the workspace root. +pub trait WorkspaceLoaderFactory { + fn create(&self, workspace_root: &Path) + -> Result, WorkspaceLoadError>; +} + +pub fn get_working_copy_factory<'a>( + workspace_loader: &dyn WorkspaceLoader, + working_copy_factories: &'a WorkingCopyFactories, +) -> Result<&'a dyn WorkingCopyFactory, StoreLoadError> { + let working_copy_type = workspace_loader.get_working_copy_type()?; + + if let Some(factory) = working_copy_factories.get(&working_copy_type) { + Ok(factory.as_ref()) + } else { + Err(StoreLoadError::UnsupportedType { + store: "working copy", + store_type: working_copy_type.to_string(), + }) + } +} + +// Loader assigned to a specific workspace root that knows how to load a +// Workspace object for that path. +pub trait WorkspaceLoader { + // The root of the Workspace to be loaded. + fn workspace_root(&self) -> &Path; + + // The path to the repo/ dir for this Workspace. + fn repo_path(&self) -> &Path; + + // Loads the specified Workspace with the provided factories. + fn load( + &self, + user_settings: &UserSettings, + store_factories: &StoreFactories, + working_copy_factories: &WorkingCopyFactories, + ) -> Result; + + // Returns the type identifier for the WorkingCopy trait in this Workspace. + fn get_working_copy_type(&self) -> Result; + + // Loads the WorkingCopy trait for this Workspace. + fn load_working_copy( + &self, + store: &Arc, + working_copy_factory: &dyn WorkingCopyFactory, + ) -> Result, WorkspaceLoadError>; +} + +pub struct DefaultWorkspaceLoaderFactory; + +impl WorkspaceLoaderFactory for DefaultWorkspaceLoaderFactory { + fn create( + &self, + workspace_root: &Path, + ) -> Result, WorkspaceLoadError> { + Ok(Box::new(DefaultWorkspaceLoader::new(workspace_root)?)) + } +} + #[derive(Clone, Debug)] -pub struct WorkspaceLoader { +struct DefaultWorkspaceLoader { workspace_root: PathBuf, repo_dir: PathBuf, working_copy_state_path: PathBuf, @@ -469,8 +530,8 @@ pub struct WorkspaceLoader { pub type WorkingCopyFactories = HashMap>; -impl WorkspaceLoader { - pub fn init(workspace_root: &Path) -> Result { +impl DefaultWorkspaceLoader { + pub fn new(workspace_root: &Path) -> Result { let jj_dir = workspace_root.join(".jj"); if !jj_dir.is_dir() { return Err(WorkspaceLoadError::NoWorkspaceHere( @@ -493,62 +554,50 @@ impl WorkspaceLoader { } } let working_copy_state_path = jj_dir.join("working_copy"); - Ok(WorkspaceLoader { + Ok(Self { workspace_root: workspace_root.to_owned(), repo_dir, working_copy_state_path, }) } +} - pub fn workspace_root(&self) -> &Path { +impl WorkspaceLoader for DefaultWorkspaceLoader { + fn workspace_root(&self) -> &Path { &self.workspace_root } - pub fn repo_path(&self) -> &Path { + fn repo_path(&self) -> &Path { &self.repo_dir } - pub fn load( + fn load( &self, user_settings: &UserSettings, store_factories: &StoreFactories, working_copy_factories: &WorkingCopyFactories, ) -> Result { let repo_loader = RepoLoader::init(user_settings, &self.repo_dir, store_factories)?; - let working_copy = self.load_working_copy(repo_loader.store(), working_copy_factories)?; + let working_copy_factory = get_working_copy_factory(self, working_copy_factories)?; + let working_copy = self.load_working_copy(repo_loader.store(), working_copy_factory)?; let workspace = Workspace::new(&self.workspace_root, working_copy, repo_loader)?; Ok(workspace) } - pub fn get_working_copy_factory<'a>( - &self, - working_copy_factories: &'a WorkingCopyFactories, - ) -> Result<&'a dyn WorkingCopyFactory, StoreLoadError> { - let working_copy_type = - read_store_type("working copy", self.working_copy_state_path.join("type"))?; - - if let Some(factory) = working_copy_factories.get(&working_copy_type) { - Ok(factory.as_ref()) - } else { - Err(StoreLoadError::UnsupportedType { - store: "working copy", - store_type: working_copy_type.to_string(), - }) - } + fn get_working_copy_type(&self) -> Result { + read_store_type("working copy", self.working_copy_state_path.join("type")) } fn load_working_copy( &self, store: &Arc, - working_copy_factories: &WorkingCopyFactories, + working_copy_factory: &dyn WorkingCopyFactory, ) -> Result, WorkspaceLoadError> { - let working_copy_factory = self.get_working_copy_factory(working_copy_factories)?; - let working_copy = working_copy_factory.load_working_copy( + Ok(working_copy_factory.load_working_copy( store.clone(), self.workspace_root.to_owned(), self.working_copy_state_path.to_owned(), - )?; - Ok(working_copy) + )?) } }