From 3b6524787fcce190b1e35e73a77ef64973c95ddd 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. --- cli/src/cli_util.rs | 32 ++++++++++---- lib/src/workspace.rs | 99 +++++++++++++++++++++++++++++++------------- 2 files changed, 94 insertions(+), 37 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 0bb8f4c97ce..9ab9ea23432 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 e3fdf12d57a..a93de545614 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,61 @@ impl<'a> LockedWorkspace<'a> { } } +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(), + }) + } +} + +pub trait WorkspaceLoader { + fn workspace_root(&self) -> &Path; + + fn repo_path(&self) -> &Path; + + fn load( + &self, + user_settings: &UserSettings, + store_factories: &StoreFactories, + working_copy_factories: &WorkingCopyFactories, + ) -> Result; + + fn get_working_copy_type(&self) -> Result; + + 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 +522,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 +546,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) + )?) } }