From 8bb92fa6fab22fd967845eab4b2f9f88b1b72e55 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Thu, 18 Apr 2024 23:36:01 -0700
Subject: [PATCH] working_copy: allow `load_working_copy()` to return error

It's reasonable for a `WorkingCopy` implementation to want to return
an error. `LocalWorkingCopyFactory` doesn't because it loads all data
lazily. The VFS-based one at Google wants to be able to return an
error, however.
---
 cli/examples/custom-working-copy/main.rs | 6 +++---
 cli/src/cli_util.rs                      | 1 +
 lib/src/local_working_copy.rs            | 8 ++++++--
 lib/src/working_copy.rs                  | 2 +-
 lib/src/workspace.rs                     | 9 ++++++---
 5 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/cli/examples/custom-working-copy/main.rs b/cli/examples/custom-working-copy/main.rs
index cb5ae1d6d0..eaba135c69 100644
--- a/cli/examples/custom-working-copy/main.rs
+++ b/cli/examples/custom-working-copy/main.rs
@@ -191,12 +191,12 @@ impl WorkingCopyFactory for ConflictsWorkingCopyFactory {
         store: Arc<Store>,
         working_copy_path: PathBuf,
         state_path: PathBuf,
-    ) -> Box<dyn WorkingCopy> {
-        Box::new(ConflictsWorkingCopy::load(
+    ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError> {
+        Ok(Box::new(ConflictsWorkingCopy::load(
             store,
             working_copy_path,
             state_path,
-        ))
+        )))
     }
 }
 
diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs
index 9e7001f2a5..1d40c9dce1 100644
--- a/cli/src/cli_util.rs
+++ b/cli/src/cli_util.rs
@@ -1504,6 +1504,7 @@ jj git init --colocate",
             err @ SignInitError::UnknownBackend(_),
         )) => user_error(err),
         WorkspaceLoadError::StoreLoadError(err) => internal_error(err),
+        WorkspaceLoadError::WorkingCopyState(err) => internal_error(err),
         WorkspaceLoadError::NonUnicodePath | WorkspaceLoadError::Path(_) => user_error(err),
     }
 }
diff --git a/lib/src/local_working_copy.rs b/lib/src/local_working_copy.rs
index 09244b214c..1392166dd8 100644
--- a/lib/src/local_working_copy.rs
+++ b/lib/src/local_working_copy.rs
@@ -1706,8 +1706,12 @@ impl WorkingCopyFactory for LocalWorkingCopyFactory {
         store: Arc<Store>,
         working_copy_path: PathBuf,
         state_path: PathBuf,
-    ) -> Box<dyn WorkingCopy> {
-        Box::new(LocalWorkingCopy::load(store, working_copy_path, state_path))
+    ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError> {
+        Ok(Box::new(LocalWorkingCopy::load(
+            store,
+            working_copy_path,
+            state_path,
+        )))
     }
 }
 
diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs
index 072895e50b..5760487e5a 100644
--- a/lib/src/working_copy.rs
+++ b/lib/src/working_copy.rs
@@ -81,7 +81,7 @@ pub trait WorkingCopyFactory {
         store: Arc<Store>,
         working_copy_path: PathBuf,
         state_path: PathBuf,
-    ) -> Box<dyn WorkingCopy>;
+    ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError>;
 }
 
 /// A working copy that's being modified.
diff --git a/lib/src/workspace.rs b/lib/src/workspace.rs
index 42bcb95f56..6b3e6e4b29 100644
--- a/lib/src/workspace.rs
+++ b/lib/src/workspace.rs
@@ -72,6 +72,8 @@ pub enum WorkspaceLoadError {
     #[error("Repo path could not be interpreted as Unicode text")]
     NonUnicodePath,
     #[error(transparent)]
+    WorkingCopyState(#[from] WorkingCopyStateError),
+    #[error(transparent)]
     Path(#[from] PathError),
 }
 
@@ -503,13 +505,14 @@ impl WorkspaceLoader {
         &self,
         store: &Arc<Store>,
         working_copy_factories: &WorkingCopyFactories,
-    ) -> Result<Box<dyn WorkingCopy>, StoreLoadError> {
+    ) -> Result<Box<dyn WorkingCopy>, WorkspaceLoadError> {
         let working_copy_factory = self.get_working_copy_factory(working_copy_factories)?;
-        Ok(working_copy_factory.load_working_copy(
+        let working_copy = working_copy_factory.load_working_copy(
             store.clone(),
             self.workspace_root.to_owned(),
             self.working_copy_state_path.to_owned(),
-        ))
+        )?;
+        Ok(working_copy)
     }
 }