git_backend: propagate load() error to caller

#1794

examples/custom-backend/main.rs

@@ -39,7 +39,7 @@ fn create_store_factories() -> StoreFactories {
     // must match `Backend::name()`.
     store_factories.add_backend(
         "jit",
-        Box::new(|store_path| Box::new(JitBackend::load(store_path))),
+        Box::new(|store_path| Ok(Box::new(JitBackend::load(store_path)?))),
     );
     store_factories
 }
@@ -80,10 +80,9 @@ impl JitBackend {
         Ok(JitBackend { inner })
     }
 
-    fn load(store_path: &Path) -> Self {
-        JitBackend {
-            inner: GitBackend::load(store_path),
-        }
+    fn load(store_path: &Path) -> BackendResult<Self> {
+        let inner = GitBackend::load(store_path)?;
+        Ok(JitBackend { inner })
     }
 }
 

lib/src/git_backend.rs

@@ -56,6 +56,20 @@ impl From<GitBackendInitError> for BackendError {
     }
 }
 
+#[derive(Debug, Error)]
+pub enum GitBackendLoadError {
+    #[error("Failed to open git repository: {0}")]
+    OpenRepository(#[source] git2::Error),
+    #[error(transparent)]
+    Path(#[from] PathError),
+}
+
+impl From<GitBackendLoadError> for BackendError {
+    fn from(err: GitBackendLoadError) -> Self {
+        BackendError::Other(err.to_string())
+    }
+}
+
 pub struct GitBackend {
     repo: Mutex<git2::Repository>,
     root_commit_id: CommitId,
@@ -106,12 +120,17 @@ impl GitBackend {
         Ok(GitBackend::new(repo, extra_metadata_store))
     }
 
-    pub fn load(store_path: &Path) -> Self {
-        let git_repo_path_str = fs::read_to_string(store_path.join("git_target")).unwrap();
-        let git_repo_path = store_path.join(git_repo_path_str).canonicalize().unwrap();
-        let repo = git2::Repository::open(git_repo_path).unwrap();
+    pub fn load(store_path: &Path) -> Result<Self, GitBackendLoadError> {
+        let git_repo_path = {
+            let target_path = store_path.join("git_target");
+            let git_repo_path_str = fs::read_to_string(&target_path).context(&target_path)?;
+            let git_repo_path = store_path.join(git_repo_path_str);
+            git_repo_path.canonicalize().context(&git_repo_path)?
+        };
+        let repo =
+            git2::Repository::open(git_repo_path).map_err(GitBackendLoadError::OpenRepository)?;
         let extra_metadata_store = TableStore::load(store_path.join("extra"), HASH_LENGTH);
-        GitBackend::new(repo, extra_metadata_store)
+        Ok(GitBackend::new(repo, extra_metadata_store))
     }
 
     pub fn git_repo(&self) -> MutexGuard<'_, git2::Repository> {

lib/src/repo.rs

@@ -332,7 +332,7 @@ impl Repo for ReadonlyRepo {
     }
 }
 
-type BackendFactory = Box<dyn Fn(&Path) -> Box<dyn Backend>>;
+type BackendFactory = Box<dyn Fn(&Path) -> BackendResult<Box<dyn Backend>>>;
 type OpStoreFactory = Box<dyn Fn(&Path) -> Box<dyn OpStore>>;
 type OpHeadsStoreFactory = Box<dyn Fn(&Path) -> Box<dyn OpHeadsStore>>;
 type IndexStoreFactory = Box<dyn Fn(&Path) -> Box<dyn IndexStore>>;
@@ -353,11 +353,11 @@ impl Default for StoreFactories {
         // Backends
         factories.add_backend(
             "local",
-            Box::new(|store_path| Box::new(LocalBackend::load(store_path))),
+            Box::new(|store_path| Ok(Box::new(LocalBackend::load(store_path)))),
         );
         factories.add_backend(
             "git",
-            Box::new(|store_path| Box::new(GitBackend::load(store_path))),
+            Box::new(|store_path| Ok(Box::new(GitBackend::load(store_path)?))),
         );
 
         // OpStores
@@ -400,6 +400,9 @@ pub enum StoreLoadError {
         store: &'static str,
         source: io::Error,
     },
+    // TODO: might be better to introduce BackendLoadError type
+    #[error(transparent)]
+    Backend(#[from] BackendError),
 }
 
 impl StoreFactories {
@@ -448,7 +451,7 @@ impl StoreFactories {
                 store_type: backend_type.to_string(),
             }
         })?;
-        Ok(backend_factory(store_path))
+        Ok(backend_factory(store_path)?)
     }
 
     pub fn add_op_store(&mut self, name: &str, factory: OpStoreFactory) {

src/cli_util.rs

@@ -1399,11 +1399,11 @@ jj init --git-repo=.",
                 "This version of the jj binary doesn't support this type of repo: {err}"
             ))
         }
-        WorkspaceLoadError::StoreLoadError(err @ StoreLoadError::ReadError { .. }) => {
-            CommandError::InternalError(format!(
-                "The repository appears broken or inaccessible: {err}"
-            ))
-        }
+        WorkspaceLoadError::StoreLoadError(
+            err @ (StoreLoadError::ReadError { .. } | StoreLoadError::Backend(_)),
+        ) => CommandError::InternalError(format!(
+            "The repository appears broken or inaccessible: {err}"
+        )),
     }
 }
 

tests/test_global_opts.rs

@@ -215,12 +215,15 @@ fn test_broken_repo_structure() {
     let test_env = TestEnvironment::default();
     test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
     let repo_path = test_env.env_root().join("repo");
+    let store_path = repo_path.join(".jj").join("repo").join("store");
+    let store_type_path = store_path.join("type");
 
-    let store_type_path = repo_path
-        .join(".jj")
-        .join("repo")
-        .join("store")
-        .join("type");
+    // Test the error message when the git repository can't be located.
+    std::fs::remove_file(store_path.join("git_target")).unwrap();
+    let stderr = test_env.jj_cmd_internal_error(&repo_path, &["log"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Internal error: The repository appears broken or inaccessible: Error: Cannot access $TEST_ENV/repo/.jj/repo/store/git_target
+    "###);
 
     // Test the error message when the commit backend is of unknown type.
     std::fs::write(&store_type_path, "unknown").unwrap();