diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 5c691b1ad..769ada7c0 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -330,6 +330,20 @@ impl Default for StoreFactories { } } +#[derive(Debug, Error)] +pub enum StoreLoadError { + #[error("Unsupported {store} backend type '{store_type}'")] + UnsupportedType { + store: &'static str, + store_type: String, + }, + #[error("Failed to read {store} backend type: {source}")] + ReadError { + store: &'static str, + source: io::Error, + }, +} + impl StoreFactories { pub fn empty() -> Self { StoreFactories { @@ -343,7 +357,7 @@ impl StoreFactories { self.backend_factories.insert(name.to_string(), factory); } - pub fn load_backend(&self, store_path: &Path) -> Box { + pub fn load_backend(&self, store_path: &Path) -> Result, StoreLoadError> { // For compatibility with existing repos. TODO: Delete in 0.8+. if store_path.join("backend").is_file() { fs::rename(store_path.join("backend"), store_path.join("type")) @@ -361,22 +375,27 @@ impl StoreFactories { fs::write(store_path.join("type"), &inferred_type).unwrap(); inferred_type } - Err(_) => { - panic!("Failed to read backend type"); + Err(err) => { + return Err(StoreLoadError::ReadError { + store: "commit", + source: err, + }); } }; - let backend_factory = self - .backend_factories - .get(&backend_type) - .expect("Unexpected backend type"); - backend_factory(store_path) + let backend_factory = self.backend_factories.get(&backend_type).ok_or_else(|| { + StoreLoadError::UnsupportedType { + store: "commit", + store_type: backend_type.to_string(), + } + })?; + Ok(backend_factory(store_path)) } pub fn add_op_store(&mut self, name: &str, factory: OpStoreFactory) { self.op_store_factories.insert(name.to_string(), factory); } - pub fn load_op_store(&self, store_path: &Path) -> Box { + pub fn load_op_store(&self, store_path: &Path) -> Result, StoreLoadError> { let op_store_type = match fs::read_to_string(store_path.join("type")) { Ok(content) => content, Err(err) if err.kind() == ErrorKind::NotFound => { @@ -385,15 +404,20 @@ impl StoreFactories { fs::write(store_path.join("type"), &default_type).unwrap(); default_type } - Err(_) => { - panic!("Failed to read op_store type"); + Err(err) => { + return Err(StoreLoadError::ReadError { + store: "operation", + source: err, + }); } }; - let op_store_factory = self - .op_store_factories - .get(&op_store_type) - .expect("Unexpected op_store type"); - op_store_factory(store_path) + let op_store_factory = self.op_store_factories.get(&op_store_type).ok_or_else(|| { + StoreLoadError::UnsupportedType { + store: "operation", + store_type: op_store_type.to_string(), + } + })?; + Ok(op_store_factory(store_path)) } pub fn add_op_heads_store(&mut self, name: &str, factory: OpHeadsStoreFactory) { @@ -401,7 +425,10 @@ impl StoreFactories { .insert(name.to_string(), factory); } - pub fn load_op_heads_store(&self, store_path: &Path) -> Box { + pub fn load_op_heads_store( + &self, + store_path: &Path, + ) -> Result, StoreLoadError> { let op_heads_store_type = match fs::read_to_string(store_path.join("type")) { Ok(content) => content, Err(err) if err.kind() == ErrorKind::NotFound => { @@ -410,15 +437,21 @@ impl StoreFactories { fs::write(store_path.join("type"), &default_type).unwrap(); default_type } - Err(_) => { - panic!("Failed to read op_heads_store type"); + Err(err) => { + return Err(StoreLoadError::ReadError { + store: "operation heads", + source: err, + }); } }; let op_heads_store_factory = self .op_heads_store_factories .get(&op_heads_store_type) - .expect("Unexpected op_heads_store type"); - op_heads_store_factory(store_path) + .ok_or_else(|| StoreLoadError::UnsupportedType { + store: "operation heads", + store_type: op_heads_store_type.to_string(), + })?; + Ok(op_heads_store_factory(store_path)) } } @@ -437,21 +470,21 @@ impl RepoLoader { user_settings: &UserSettings, repo_path: &Path, store_factories: &StoreFactories, - ) -> Self { - let store = Store::new(store_factories.load_backend(&repo_path.join("store"))); + ) -> Result { + let store = Store::new(store_factories.load_backend(&repo_path.join("store"))?); let repo_settings = user_settings.with_repo(repo_path).unwrap(); - let op_store = Arc::from(store_factories.load_op_store(&repo_path.join("op_store"))); + let op_store = Arc::from(store_factories.load_op_store(&repo_path.join("op_store"))?); let op_heads_store = - Arc::from(store_factories.load_op_heads_store(&repo_path.join("op_heads"))); + Arc::from(store_factories.load_op_heads_store(&repo_path.join("op_heads"))?); let index_store = Arc::new(IndexStore::load(repo_path.join("index"))); - Self { + Ok(Self { repo_path: repo_path.to_path_buf(), repo_settings, store, op_store, op_heads_store, index_store, - } + }) } pub fn repo_path(&self) -> &PathBuf { diff --git a/lib/src/workspace.rs b/lib/src/workspace.rs index 3f4e263d1..53718468e 100644 --- a/lib/src/workspace.rs +++ b/lib/src/workspace.rs @@ -27,6 +27,7 @@ use crate::op_store::{self, OpStore, OperationMetadata, WorkspaceId}; use crate::operation::Operation; use crate::repo::{ CheckOutCommitError, IoResultExt, PathError, ReadonlyRepo, Repo, RepoLoader, StoreFactories, + StoreLoadError, }; use crate::settings::UserSettings; use crate::working_copy::WorkingCopy; @@ -49,6 +50,8 @@ pub enum WorkspaceLoadError { RepoDoesNotExist(PathBuf), #[error("There is no Jujutsu repo in {0}")] NoWorkspaceHere(PathBuf), + #[error("Cannot read the repo: {0}")] + StoreLoadError(#[from] StoreLoadError), #[error("Repo path could not be interpreted as Unicode text")] NonUnicodePath, #[error(transparent)] @@ -233,7 +236,8 @@ impl Workspace { store_factories: &StoreFactories, ) -> Result { let loader = WorkspaceLoader::init(workspace_path)?; - Ok(loader.load(user_settings, store_factories)?) + let workspace = loader.load(user_settings, store_factories)?; + Ok(workspace) } pub fn workspace_root(&self) -> &PathBuf { @@ -314,7 +318,7 @@ impl WorkspaceLoader { user_settings: &UserSettings, store_factories: &StoreFactories, ) -> Result { - let repo_loader = RepoLoader::init(user_settings, &self.repo_dir, store_factories); + let repo_loader = RepoLoader::init(user_settings, &self.repo_dir, store_factories)?; let working_copy = WorkingCopy::load( repo_loader.store().clone(), self.workspace_root.clone(), diff --git a/lib/tests/test_load_repo.rs b/lib/tests/test_load_repo.rs index c94c9fd21..ea98faec9 100644 --- a/lib/tests/test_load_repo.rs +++ b/lib/tests/test_load_repo.rs @@ -33,13 +33,13 @@ fn test_load_at_operation(use_git: bool) { // If we load the repo at head, we should not see the commit since it was // removed - let loader = RepoLoader::init(&settings, repo.repo_path(), &StoreFactories::default()); + let loader = RepoLoader::init(&settings, repo.repo_path(), &StoreFactories::default()).unwrap(); let head_repo = loader.load_at_head(&settings).unwrap(); assert!(!head_repo.view().heads().contains(commit.id())); // If we load the repo at the previous operation, we should see the commit since // it has not been removed yet - let loader = RepoLoader::init(&settings, repo.repo_path(), &StoreFactories::default()); + let loader = RepoLoader::init(&settings, repo.repo_path(), &StoreFactories::default()).unwrap(); let old_repo = loader.load_at(repo.operation()); assert!(old_repo.view().heads().contains(commit.id())); } diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index c31fa3956..c8d752f29 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -153,6 +153,7 @@ impl TestWorkspace { pub fn load_repo_at_head(settings: &UserSettings, repo_path: &Path) -> Arc { RepoLoader::init(settings, repo_path, &StoreFactories::default()) + .unwrap() .load_at_head(settings) .unwrap() } diff --git a/src/cli_util.rs b/src/cli_util.rs index c32d48b75..c49131788 100644 --- a/src/cli_util.rs +++ b/src/cli_util.rs @@ -39,7 +39,7 @@ use jujutsu_lib::op_store::{OpStore, OpStoreError, OperationId, RefTarget, Works use jujutsu_lib::operation::Operation; use jujutsu_lib::repo::{ CheckOutCommitError, EditCommitError, MutableRepo, ReadonlyRepo, Repo, RepoLoader, - RewriteRootCommit, StoreFactories, + RewriteRootCommit, StoreFactories, StoreLoadError, }; use jujutsu_lib::repo_path::{FsPathParseError, RepoPath}; use jujutsu_lib::revset::{ @@ -1203,6 +1203,16 @@ jj init --git-repo=.", )), WorkspaceLoadError::Path(e) => user_error(format!("{}: {}", e, e.error)), WorkspaceLoadError::NonUnicodePath => user_error(err.to_string()), + WorkspaceLoadError::StoreLoadError(err @ StoreLoadError::UnsupportedType { .. }) => { + CommandError::InternalError(format!( + "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}" + )) + } } } diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 63fad0ad1..ff129bc54 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -103,6 +103,14 @@ impl TestEnvironment { self.normalize_output(&get_stderr_string(&assert)) } + /// Run a `jj` command, check that it failed with code 255, and return its + /// stderr + #[must_use] + pub fn jj_cmd_internal_error(&self, current_dir: &Path, args: &[&str]) -> String { + let assert = self.jj_cmd(current_dir, args).assert().code(255).stdout(""); + self.normalize_output(&get_stderr_string(&assert)) + } + pub fn env_root(&self) -> &Path { &self.env_root } diff --git a/tests/test_global_opts.rs b/tests/test_global_opts.rs index 016252cf8..05c8e96b4 100644 --- a/tests/test_global_opts.rs +++ b/tests/test_global_opts.rs @@ -14,6 +14,8 @@ use std::ffi::OsString; +use itertools::Itertools; + use crate::common::{get_stderr_string, TestEnvironment}; pub mod common; @@ -172,6 +174,35 @@ fn test_no_workspace_directory() { "###); } +#[test] +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_type_path = repo_path + .join(".jj") + .join("repo") + .join("store") + .join("type"); + + // Test the error message when the commit backend is of unknown type. + std::fs::write(&store_type_path, "unknown").unwrap(); + let stderr = test_env.jj_cmd_internal_error(&repo_path, &["log"]); + insta::assert_snapshot!(stderr, @r###" + Internal error: This version of the jj binary doesn't support this type of repo: Unsupported commit backend type 'unknown' + "###); + + // Test the error message when the file indicating the commit backend type + // cannot be read. + std::fs::remove_file(&store_type_path).unwrap(); + std::fs::create_dir(&store_type_path).unwrap(); + let stderr = test_env.jj_cmd_internal_error(&repo_path, &["log"]); + // Trim off the OS-specific error message. + let stderr = stderr.split(':').take(3).join(":"); + insta::assert_snapshot!(stderr, @"Internal error: The repository appears broken or inaccessible: Failed to read commit backend type"); +} + #[test] fn test_color_config() { let mut test_env = TestEnvironment::default();