diff --git a/lib/src/repo.rs b/lib/src/repo.rs index ac2f63d95..a6836b59a 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -81,6 +81,12 @@ impl Debug for ReadonlyRepo { } } +#[derive(Error, Debug, PartialEq)] +pub enum RepoLoadError { + #[error("There is no Jujube repo in {0}")] + NoRepoHere(PathBuf), +} + impl ReadonlyRepo { pub fn init_local(settings: &UserSettings, wc_path: PathBuf) -> Arc { let repo_path = wc_path.join(".jj"); @@ -185,8 +191,15 @@ impl ReadonlyRepo { repo } - pub fn load(user_settings: &UserSettings, wc_path: PathBuf) -> Arc { + pub fn load( + user_settings: &UserSettings, + wc_path: PathBuf, + ) -> Result, RepoLoadError> { let repo_path = wc_path.join(".jj"); + // TODO: Check if ancestor directory has a .jj/ + if !repo_path.is_dir() { + return Err(RepoLoadError::NoRepoHere(wc_path)); + } let store_path = repo_path.join("store"); let store: Box; if store_path.is_dir() { @@ -225,7 +238,7 @@ impl ReadonlyRepo { let static_lifetime_repo: &'static ReadonlyRepo = unsafe { std::mem::transmute(repo_ref) }; let evolution = ReadonlyEvolution::new(static_lifetime_repo); ReadonlyRepo::init_cycles(&mut repo, evolution); - repo + Ok(repo) } fn init_cycles(mut repo: &mut Arc, evolution: ReadonlyEvolution<'static>) { diff --git a/lib/tests/test_bad_locking.rs b/lib/tests/test_bad_locking.rs index 2a1c9e4dd..0edefc257 100644 --- a/lib/tests/test_bad_locking.rs +++ b/lib/tests/test_bad_locking.rs @@ -106,7 +106,7 @@ fn test_bad_locking_children(use_git: bool) { // Simulate a write of a commit that happens on one machine let machine1_path = TempDir::new().unwrap().into_path(); copy_directory(repo.working_copy_path(), &machine1_path); - let machine1_repo = ReadonlyRepo::load(&settings, machine1_path); + let machine1_repo = ReadonlyRepo::load(&settings, machine1_path).unwrap(); let child1 = testutils::create_random_commit(&settings, &machine1_repo) .set_parents(vec![initial.id().clone()]) .write_to_new_transaction(&machine1_repo, "test"); @@ -114,7 +114,7 @@ fn test_bad_locking_children(use_git: bool) { // Simulate a write of a commit that happens on another machine let machine2_path = TempDir::new().unwrap().into_path(); copy_directory(repo.working_copy_path(), &machine2_path); - let machine2_repo = ReadonlyRepo::load(&settings, machine2_path); + let machine2_repo = ReadonlyRepo::load(&settings, machine2_path).unwrap(); let child2 = testutils::create_random_commit(&settings, &machine2_repo) .set_parents(vec![initial.id().clone()]) .write_to_new_transaction(&machine2_repo, "test"); @@ -128,7 +128,7 @@ fn test_bad_locking_children(use_git: bool) { machine2_repo.working_copy_path(), &merged_path, ); - let merged_repo = ReadonlyRepo::load(&settings, merged_path); + let merged_repo = ReadonlyRepo::load(&settings, merged_path).unwrap(); let heads: HashSet<_> = merged_repo.view().heads().cloned().collect(); assert!(heads.contains(child1.id())); assert!(heads.contains(child2.id())); @@ -170,10 +170,10 @@ fn test_bad_locking_interrupted(use_git: bool) { copy_directory(&backup_path, &op_heads_dir); // Reload the repo and check that only the new head is present. - let reloaded_repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()); + let reloaded_repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()).unwrap(); assert_eq!(reloaded_repo.view().base_op_head_id(), &op_head_id); // Reload once more to make sure that the view/op_heads/ directory was updated // correctly. - let reloaded_repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()); + let reloaded_repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()).unwrap(); assert_eq!(reloaded_repo.view().base_op_head_id(), &op_head_id); } diff --git a/lib/tests/test_commit_concurrent.rs b/lib/tests/test_commit_concurrent.rs index 39554c54b..2b2a90b37 100644 --- a/lib/tests/test_commit_concurrent.rs +++ b/lib/tests/test_commit_concurrent.rs @@ -81,7 +81,7 @@ fn test_commit_parallel_instances(use_git: bool) { let mut threads = vec![]; for _ in 0..100 { let settings = settings.clone(); - let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()); + let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()).unwrap(); let handle = thread::spawn(move || { testutils::create_random_commit(&settings, &repo) .write_to_new_transaction(&repo, "test"); @@ -93,7 +93,7 @@ fn test_commit_parallel_instances(use_git: bool) { } // One commit per thread plus the commit from the initial checkout on top of the // root commit - let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()); + let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()).unwrap(); assert_eq!(repo.view().heads().count(), 101); // One operation for initializing the repo (containing the root id and the diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index d84b5daa4..51625be31 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -144,7 +144,7 @@ fn test_fetch_success() { let new_git_commit = empty_git_commit(&git_repo, "refs/heads/main", &[&initial_git_commit]); // The new commit is not visible before git::fetch(). - let jj_repo = ReadonlyRepo::load(&settings, jj_repo_dir.clone()); + let jj_repo = ReadonlyRepo::load(&settings, jj_repo_dir.clone()).unwrap(); let heads: HashSet<_> = jj_repo.view().heads().cloned().collect(); assert!(!heads.contains(&commit_id(&new_git_commit))); diff --git a/lib/tests/test_index.rs b/lib/tests/test_index.rs index 97f34e125..2a7eea411 100644 --- a/lib/tests/test_index.rs +++ b/lib/tests/test_index.rs @@ -275,7 +275,7 @@ fn test_index_commits_previous_operations(use_git: bool) { std::fs::remove_dir_all(&index_operations_dir).unwrap(); std::fs::create_dir(&index_operations_dir).unwrap(); - let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()); + let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()).unwrap(); let index = repo.index(); let index = index.as_composite(); // There should be the root commit and the working copy commit, plus @@ -323,7 +323,7 @@ fn test_index_commits_incremental(use_git: bool) { let commit_c = child_commit(&settings, &repo, &commit_b).write_to_transaction(&mut tx); tx.commit(); - let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()); + let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()).unwrap(); let index = repo.index(); let index = index.as_composite(); // There should be the root commit and the working copy commit, plus @@ -370,7 +370,7 @@ fn test_index_commits_incremental_empty_transaction(use_git: bool) { repo.start_transaction("test").commit(); - let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()); + let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()).unwrap(); let index = repo.index(); let index = index.as_composite(); // There should be the root commit and the working copy commit, plus diff --git a/lib/tests/test_load_repo.rs b/lib/tests/test_load_repo.rs new file mode 100644 index 000000000..db5f189f9 --- /dev/null +++ b/lib/tests/test_load_repo.rs @@ -0,0 +1,26 @@ +// Copyright 2021 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use jujube_lib::repo::{ReadonlyRepo, RepoLoadError}; +use jujube_lib::testutils; + +#[test] +fn test_load_bad_path() { + let settings = testutils::user_settings(); + let temp_dir = tempfile::tempdir().unwrap(); + let wc_path = temp_dir.path().to_owned(); + // We haven't created a repo in the wc_path, so it should fail to load. + let result = ReadonlyRepo::load(&settings, wc_path.clone()); + assert_eq!(result.err(), Some(RepoLoadError::NoRepoHere(wc_path))); +} diff --git a/lib/tests/test_working_copy_concurrent.rs b/lib/tests/test_working_copy_concurrent.rs index 59d768a53..d2d622400 100644 --- a/lib/tests/test_working_copy_concurrent.rs +++ b/lib/tests/test_working_copy_concurrent.rs @@ -47,7 +47,7 @@ fn test_concurrent_checkout(use_git: bool) { wc1.check_out(commit1).unwrap(); // Check out commit2 from another process (simulated by another repo instance) - let repo2 = ReadonlyRepo::load(&settings, repo1.working_copy_path().clone()); + let repo2 = ReadonlyRepo::load(&settings, repo1.working_copy_path().clone()).unwrap(); repo2 .working_copy_locked() .check_out(commit2.clone()) @@ -60,7 +60,7 @@ fn test_concurrent_checkout(use_git: bool) { ); // Check that the commit2 is still checked out on disk. - let repo3 = ReadonlyRepo::load(&settings, repo1.working_copy_path().clone()); + let repo3 = ReadonlyRepo::load(&settings, repo1.working_copy_path().clone()).unwrap(); assert_eq!( repo3.working_copy_locked().current_tree_id(), commit2.tree().id().clone() @@ -80,7 +80,7 @@ fn test_concurrent_commit(use_git: bool) { let commit1 = wc1.current_commit(); // Commit from another process (simulated by another repo instance) - let mut repo2 = ReadonlyRepo::load(&settings, repo1.working_copy_path().clone()); + let mut repo2 = ReadonlyRepo::load(&settings, repo1.working_copy_path().clone()).unwrap(); testutils::write_working_copy_file(&repo2, &FileRepoPath::from("file2"), "contents2"); let owned_wc2 = repo2.working_copy().clone(); let wc2 = owned_wc2.lock().unwrap(); @@ -132,7 +132,7 @@ fn test_checkout_parallel(use_git: bool) { let settings = settings.clone(); let working_copy_path = repo.working_copy_path().clone(); let handle = thread::spawn(move || { - let mut repo = ReadonlyRepo::load(&settings, working_copy_path); + let mut repo = ReadonlyRepo::load(&settings, working_copy_path).unwrap(); let owned_wc = repo.working_copy().clone(); let wc = owned_wc.lock().unwrap(); let commit = repo.store().get_commit(&commit_id).unwrap(); diff --git a/src/commands.rs b/src/commands.rs index c9e11f1b3..effd05286 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -41,7 +41,7 @@ use jujube_lib::files; use jujube_lib::files::DiffLine; use jujube_lib::git; use jujube_lib::op_store::{OpStoreError, OperationId}; -use jujube_lib::repo::{ReadonlyRepo, Repo}; +use jujube_lib::repo::{ReadonlyRepo, Repo, RepoLoadError}; use jujube_lib::repo_path::RepoPath; use jujube_lib::rewrite::{back_out_commit, merge_commit_trees, rebase_commit}; use jujube_lib::settings::UserSettings; @@ -82,10 +82,16 @@ impl From for CommandError { } } +impl From for CommandError { + fn from(err: RepoLoadError) -> Self { + CommandError::UserError(format!("Failed to load repo: {}", err)) + } +} + fn get_repo(ui: &Ui, matches: &ArgMatches) -> Result, CommandError> { let repo_path_str = matches.value_of("repository").unwrap(); let repo_path = ui.cwd().join(repo_path_str); - let mut repo = ReadonlyRepo::load(ui.settings(), repo_path); + let mut repo = ReadonlyRepo::load(ui.settings(), repo_path)?; if let Some(op_str) = matches.value_of("at_op") { let op = resolve_single_op(&repo, op_str)?; Arc::get_mut(&mut repo).unwrap().reload_at(&op);