diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 72fd8fd24..0e1286e5d 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -126,12 +126,6 @@ pub enum RepoInitError { DestinationExists(PathBuf), } -#[derive(Error, Debug, PartialEq)] -pub enum RepoLoadError { - #[error("There is no Jujutsu repo in {0}")] - NoRepoHere(PathBuf), -} - impl ReadonlyRepo { pub fn init_local( settings: &UserSettings, @@ -239,11 +233,8 @@ impl ReadonlyRepo { }) } - pub fn load( - user_settings: &UserSettings, - wc_path: PathBuf, - ) -> Result, RepoLoadError> { - Ok(RepoLoader::init(user_settings, wc_path)?.load_at_head()) + pub fn load(user_settings: &UserSettings, repo_path: PathBuf) -> Arc { + RepoLoader::init(user_settings, repo_path).load_at_head() } pub fn loader(&self) -> RepoLoader { @@ -351,26 +342,8 @@ pub struct RepoLoader { index_store: Arc, } -fn find_repo_dir(mut wc_dir: &Path) -> Option { - loop { - let repo_path = wc_dir.join(".jj"); - if repo_path.is_dir() { - return Some(repo_path); - } - if let Some(wc_dir_parent) = wc_dir.parent() { - wc_dir = wc_dir_parent; - } else { - return None; - } - } -} - impl RepoLoader { - pub fn init( - user_settings: &UserSettings, - wc_path: PathBuf, - ) -> Result { - let repo_path = find_repo_dir(&wc_path).ok_or(RepoLoadError::NoRepoHere(wc_path))?; + pub fn init(user_settings: &UserSettings, repo_path: PathBuf) -> Self { let wc_path = repo_path.parent().unwrap().to_owned(); let store_path = repo_path.join("store"); if store_path.is_file() { @@ -398,7 +371,7 @@ impl RepoLoader { let op_store: Arc = Arc::new(SimpleOpStore::load(repo_path.join("op_store"))); let op_heads_store = Arc::new(OpHeadsStore::load(repo_path.join("op_heads"))); let index_store = Arc::new(IndexStore::load(repo_path.join("index"))); - Ok(RepoLoader { + Self { wc_path, repo_path, repo_settings, @@ -406,7 +379,7 @@ impl RepoLoader { 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 66bc7ce66..72f8e95f4 100644 --- a/lib/src/workspace.rs +++ b/lib/src/workspace.rs @@ -12,13 +12,21 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::Arc; -use crate::repo::{ReadonlyRepo, RepoInitError, RepoLoadError, RepoLoader}; +use thiserror::Error; + +use crate::repo::{ReadonlyRepo, RepoInitError, RepoLoader}; use crate::settings::UserSettings; use crate::working_copy::WorkingCopy; +#[derive(Error, Debug, PartialEq)] +pub enum WorkspaceLoadError { + #[error("There is no Jujutsu repo in {0}")] + NoWorkspaceHere(PathBuf), +} + /// Represents a workspace, i.e. what's typically the .jj/ directory and its /// parent. pub struct Workspace { @@ -64,11 +72,12 @@ impl Workspace { pub fn load( user_settings: &UserSettings, - workspace_root: PathBuf, - ) -> Result { - // TODO: Move the find_repo_dir() call from RepoLoader::init() to here - let repo_loader = RepoLoader::init(user_settings, workspace_root)?; - let workspace_root = repo_loader.working_copy_path().clone(); + workspace_path: PathBuf, + ) -> Result { + let repo_path = find_repo_dir(&workspace_path) + .ok_or(WorkspaceLoadError::NoWorkspaceHere(workspace_path))?; + let workspace_root = repo_path.parent().unwrap().to_owned(); + let repo_loader = RepoLoader::init(user_settings, repo_path); Ok(Self::from_repo_loader(workspace_root, repo_loader)) } @@ -106,3 +115,17 @@ impl Workspace { &mut self.working_copy } } + +fn find_repo_dir(mut workspace_root: &Path) -> Option { + loop { + let repo_path = workspace_root.join(".jj"); + if repo_path.is_dir() { + return Some(repo_path); + } + if let Some(wc_dir_parent) = workspace_root.parent() { + workspace_root = wc_dir_parent; + } else { + return None; + } + } +} diff --git a/lib/tests/test_bad_locking.rs b/lib/tests/test_bad_locking.rs index f7ec0dad2..d7dd41a2a 100644 --- a/lib/tests/test_bad_locking.rs +++ b/lib/tests/test_bad_locking.rs @@ -107,7 +107,7 @@ fn test_bad_locking_children(use_git: bool) { // Simulate a write of a commit that happens on one machine let machine1_root = TempDir::new().unwrap().into_path(); copy_directory(workspace_root, &machine1_root); - let machine1_repo = ReadonlyRepo::load(&settings, machine1_root.clone()).unwrap(); + let machine1_repo = ReadonlyRepo::load(&settings, machine1_root.join(".jj")); let mut machine1_tx = machine1_repo.start_transaction("test"); let child1 = testutils::create_random_commit(&settings, &machine1_repo) .set_parents(vec![initial.id().clone()]) @@ -117,7 +117,7 @@ fn test_bad_locking_children(use_git: bool) { // Simulate a write of a commit that happens on another machine let machine2_root = TempDir::new().unwrap().into_path(); copy_directory(workspace_root, &machine2_root); - let machine2_repo = ReadonlyRepo::load(&settings, machine2_root.clone()).unwrap(); + let machine2_repo = ReadonlyRepo::load(&settings, machine2_root.join(".jj")); let mut machine2_tx = machine2_repo.start_transaction("test"); let child2 = testutils::create_random_commit(&settings, &machine2_repo) .set_parents(vec![initial.id().clone()]) @@ -128,7 +128,7 @@ fn test_bad_locking_children(use_git: bool) { // both machines let merged_path = TempDir::new().unwrap().into_path(); merge_directories(&machine1_root, workspace_root, &machine2_root, &merged_path); - let merged_repo = ReadonlyRepo::load(&settings, merged_path).unwrap(); + let merged_repo = ReadonlyRepo::load(&settings, merged_path.join(".jj")); assert!(merged_repo.view().heads().contains(child1.id())); assert!(merged_repo.view().heads().contains(child2.id())); let op_id = merged_repo.op_id().clone(); @@ -167,10 +167,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.repo_path().clone()).unwrap(); + let reloaded_repo = ReadonlyRepo::load(&settings, repo.repo_path().clone()); assert_eq!(reloaded_repo.op_id(), &op_id); // Reload once more to make sure that the .jj/op_heads/ directory was updated // correctly. - let reloaded_repo = ReadonlyRepo::load(&settings, repo.repo_path().clone()).unwrap(); + let reloaded_repo = ReadonlyRepo::load(&settings, repo.repo_path().clone()); assert_eq!(reloaded_repo.op_id(), &op_id); } diff --git a/lib/tests/test_commit_concurrent.rs b/lib/tests/test_commit_concurrent.rs index d4e6dbb55..9b5c16151 100644 --- a/lib/tests/test_commit_concurrent.rs +++ b/lib/tests/test_commit_concurrent.rs @@ -84,7 +84,7 @@ fn test_commit_parallel_instances(use_git: bool) { let mut threads = vec![]; for _ in 0..num_threads { let settings = settings.clone(); - let repo = ReadonlyRepo::load(&settings, repo.repo_path().clone()).unwrap(); + let repo = ReadonlyRepo::load(&settings, repo.repo_path().clone()); let handle = thread::spawn(move || { let mut tx = repo.start_transaction("test"); testutils::create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo()); @@ -97,7 +97,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.repo_path().clone()).unwrap(); + let repo = ReadonlyRepo::load(&settings, repo.repo_path().clone()); assert_eq!(repo.view().heads().len(), num_threads + 1); // One operation for initializing the repo (containing the root id and the diff --git a/lib/tests/test_index.rs b/lib/tests/test_index.rs index 8512f70ab..ca628277f 100644 --- a/lib/tests/test_index.rs +++ b/lib/tests/test_index.rs @@ -262,7 +262,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.repo_path().clone()).unwrap(); + let repo = ReadonlyRepo::load(&settings, repo.repo_path().clone()); let index = repo.index(); // There should be the root commit and the working copy commit, plus // 3 more @@ -309,7 +309,7 @@ fn test_index_commits_incremental(use_git: bool) { let commit_c = child_commit(&settings, &repo, &commit_b).write_to_repo(tx.mut_repo()); tx.commit(); - let repo = ReadonlyRepo::load(&settings, repo.repo_path().clone()).unwrap(); + let repo = ReadonlyRepo::load(&settings, repo.repo_path().clone()); let index = repo.index(); // There should be the root commit and the working copy commit, plus // 3 more @@ -354,7 +354,7 @@ fn test_index_commits_incremental_empty_transaction(use_git: bool) { repo.start_transaction("test").commit(); - let repo = ReadonlyRepo::load(&settings, repo.repo_path().clone()).unwrap(); + let repo = ReadonlyRepo::load(&settings, repo.repo_path().clone()); let index = repo.index(); // There should be the root commit and the working copy commit, plus // 1 more diff --git a/lib/tests/test_load_repo.rs b/lib/tests/test_load_repo.rs index ce5116b2e..3456b1da9 100644 --- a/lib/tests/test_load_repo.rs +++ b/lib/tests/test_load_repo.rs @@ -12,37 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use jujutsu_lib::repo::{ReadonlyRepo, RepoLoadError, RepoLoader}; +use jujutsu_lib::repo::RepoLoader; use jujutsu_lib::testutils; use test_case::test_case; -#[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))); -} - -#[test_case(false ; "local backend")] -#[test_case(true ; "git backend")] -fn test_load_from_subdir(use_git: bool) { - let settings = testutils::user_settings(); - let test_workspace = testutils::init_repo(&settings, use_git); - let repo = &test_workspace.repo; - let workspace_root = test_workspace.workspace.workspace_root().clone(); - - let subdir = workspace_root.join("dir").join("subdir"); - std::fs::create_dir_all(subdir.clone()).unwrap(); - let same_repo = ReadonlyRepo::load(&settings, subdir); - assert!(same_repo.is_ok()); - let same_repo = same_repo.unwrap(); - assert_eq!(same_repo.repo_path(), repo.repo_path()); - assert_eq!(same_repo.working_copy_path(), repo.working_copy_path()); -} - #[test_case(false ; "local backend")] #[test_case(true ; "git backend")] fn test_load_at_operation(use_git: bool) { @@ -60,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().clone()).unwrap(); + let loader = RepoLoader::init(&settings, repo.repo_path().clone()); let head_repo = loader.load_at_head(); 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().clone()).unwrap(); + let loader = RepoLoader::init(&settings, repo.repo_path().clone()); let old_repo = loader.load_at(repo.operation()); assert!(old_repo.view().heads().contains(commit.id())); } diff --git a/lib/tests/test_workspace.rs b/lib/tests/test_workspace.rs new file mode 100644 index 000000000..cf5392dcb --- /dev/null +++ b/lib/tests/test_workspace.rs @@ -0,0 +1,46 @@ +// 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 jujutsu_lib::testutils; +use jujutsu_lib::workspace::{Workspace, WorkspaceLoadError}; +use test_case::test_case; + +#[test] +fn test_load_bad_path() { + let settings = testutils::user_settings(); + let temp_dir = tempfile::tempdir().unwrap(); + let workspace_root = temp_dir.path().to_owned(); + // We haven't created a repo in the workspace_root, so it should fail to load. + let result = Workspace::load(&settings, workspace_root.clone()); + assert_eq!( + result.err(), + Some(WorkspaceLoadError::NoWorkspaceHere(workspace_root)) + ); +} + +#[test_case(false ; "local backend")] +#[test_case(true ; "git backend")] +fn test_load_from_subdir(use_git: bool) { + let settings = testutils::user_settings(); + let test_workspace = testutils::init_repo(&settings, use_git); + let workspace = &test_workspace.workspace; + + let subdir = workspace.workspace_root().join("dir").join("subdir"); + std::fs::create_dir_all(subdir.clone()).unwrap(); + let same_workspace = Workspace::load(&settings, subdir); + assert!(same_workspace.is_ok()); + let same_workspace = same_workspace.unwrap(); + assert_eq!(same_workspace.repo_path(), workspace.repo_path()); + assert_eq!(same_workspace.workspace_root(), workspace.workspace_root()); +} diff --git a/src/commands.rs b/src/commands.rs index 2ed5eed26..d91b85d3b 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -45,7 +45,7 @@ use jujutsu_lib::op_heads_store::OpHeadsStore; use jujutsu_lib::op_store::{OpStore, OpStoreError, OperationId, RefTarget}; use jujutsu_lib::operation::Operation; use jujutsu_lib::refs::{classify_branch_push_action, BranchPushAction}; -use jujutsu_lib::repo::{MutableRepo, ReadonlyRepo, RepoInitError, RepoLoadError, RepoRef}; +use jujutsu_lib::repo::{MutableRepo, ReadonlyRepo, RepoInitError, RepoRef}; use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::revset::{RevsetError, RevsetExpression, RevsetParseError}; use jujutsu_lib::revset_graph_iterator::RevsetGraphEdgeType; @@ -55,7 +55,7 @@ use jujutsu_lib::store::Store; use jujutsu_lib::transaction::Transaction; use jujutsu_lib::tree::TreeDiffIterator; use jujutsu_lib::working_copy::{CheckoutStats, WorkingCopy}; -use jujutsu_lib::workspace::Workspace; +use jujutsu_lib::workspace::{Workspace, WorkspaceLoadError}; use jujutsu_lib::{conflicts, diff, files, git, revset, tree}; use maplit::{hashmap, hashset}; use pest::Parser; @@ -154,7 +154,7 @@ impl<'args> CommandHelper<'args> { let wc_path = ui.cwd().join(wc_path_str); let workspace = match Workspace::load(ui.settings(), wc_path) { Ok(workspace) => workspace, - Err(RepoLoadError::NoRepoHere(wc_path)) => { + Err(WorkspaceLoadError::NoWorkspaceHere(wc_path)) => { let mut message = format!("There is no jj repo in \"{}\"", wc_path_str); let git_dir = wc_path.join(".git"); if git_dir.is_dir() {