workspace: turn WorkspaceLoader into a trait

Like #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

@@ -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<RevsetExtensions>,
     commit_template_extensions: Vec<Arc<dyn CommitTemplateLanguageExtension>>,
     operation_template_extensions: Vec<Arc<dyn OperationTemplateLanguageExtension>>,
-    maybe_workspace_loader: Result<WorkspaceLoader, CommandError>,
+    maybe_workspace_loader: Result<Box<dyn WorkspaceLoader>, 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<config::Config>,
     store_factories: StoreFactories,
     working_copy_factories: WorkingCopyFactories,
+    workspace_loader_factory: Box<dyn WorkspaceLoaderFactory>,
     revset_extensions: RevsetExtensions,
     commit_template_extensions: Vec<Arc<dyn CommitTemplateLanguageExtension>>,
     operation_template_extensions: Vec<Arc<dyn OperationTemplateLanguageExtension>>,
@@ -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<dyn WorkspaceLoaderFactory>,
+    ) -> Self {
+        self.workspace_loader_factory = workspace_loader_factory;
+        self
+    }
+
     pub fn add_symbol_resolver_extension(
         mut self,
         symbol_resolver: Box<dyn SymbolResolverExtension>,
@@ -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)

lib/src/workspace.rs

@@ -376,7 +376,7 @@ impl Workspace {
         store_factories: &StoreFactories,
         working_copy_factories: &WorkingCopyFactories,
     ) -> Result<Self, WorkspaceLoadError> {
-        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,17 +460,78 @@ impl<'a> LockedWorkspace<'a> {
     }
 }
 
+// Factory trait to build WorkspaceLoaders given the workspace root.
+pub trait WorkspaceLoaderFactory {
+    fn create(&self, workspace_root: &Path)
+        -> Result<Box<dyn WorkspaceLoader>, 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<Workspace, WorkspaceLoadError>;
+
+    // Returns the type identifier for the WorkingCopy trait in this Workspace.
+    fn get_working_copy_type(&self) -> Result<String, StoreLoadError>;
+
+    // Loads the WorkingCopy trait for this Workspace.
+    fn load_working_copy(
+        &self,
+        store: &Arc<Store>,
+        working_copy_factory: &dyn WorkingCopyFactory,
+    ) -> Result<Box<dyn WorkingCopy>, WorkspaceLoadError>;
+}
+
+pub struct DefaultWorkspaceLoaderFactory;
+
+impl WorkspaceLoaderFactory for DefaultWorkspaceLoaderFactory {
+    fn create(
+        &self,
+        workspace_root: &Path,
+    ) -> Result<Box<dyn WorkspaceLoader>, 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,
 }
 
 pub type WorkingCopyFactories = HashMap<String, Box<dyn WorkingCopyFactory>>;
 
-impl WorkspaceLoader {
-    pub fn init(workspace_root: &Path) -> Result<Self, WorkspaceLoadError> {
+impl DefaultWorkspaceLoader {
+    pub fn new(workspace_root: &Path) -> Result<Self, WorkspaceLoadError> {
         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<Workspace, WorkspaceLoadError> {
         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<String, StoreLoadError> {
+        read_store_type("working copy", self.working_copy_state_path.join("type"))
     }
 
     fn load_working_copy(
         &self,
         store: &Arc<Store>,
-        working_copy_factories: &WorkingCopyFactories,
+        working_copy_factory: &dyn WorkingCopyFactory,
     ) -> Result<Box<dyn WorkingCopy>, 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)
+        )?)
     }
 }