ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

cleanup: replace some PathBuf args by &Path

Browse source 
In many of these places, we don't need an owned value, so using a
reference means we don't force the caller to clone the value. I really
doubt it will have any noticeable impact on performance (I think these
are all once-per-repo paths); it's just a little simpler this way.
This commit is contained in:
Martin von Zweigbergk 2022-09-23 14:03:43 -07:00 committed by Martin von Zweigbergk
parent 48a7378d9f
commit ea5aa0a96d
15 changed files with 82 additions and 86 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
lib/src/git_backend.rs
View file

@ -15,7 +15,7 @@
use std::fmt::{Debug, Error, Formatter}; use std::fmt::{Debug, Error, Formatter};
use std::fs::File; use std::fs::File;
use std::io::{Cursor, Read, Write}; use std::io::{Cursor, Read, Write};
use std::path::PathBuf; use std::path::Path;
use std::sync::Mutex; use std::sync::Mutex;
use git2::Oid; use git2::Oid;
@ -64,7 +64,7 @@ impl GitBackend {
} }
} }
pub fn init_internal(store_path: PathBuf) -> Self { pub fn init_internal(store_path: &Path) -> Self {
let git_repo = git2::Repository::init_bare(&store_path.join("git")).unwrap(); let git_repo = git2::Repository::init_bare(&store_path.join("git")).unwrap();
let extra_path = store_path.join("extra"); let extra_path = store_path.join("extra");
std::fs::create_dir(&extra_path).unwrap(); std::fs::create_dir(&extra_path).unwrap();
@ -74,7 +74,7 @@ impl GitBackend {
GitBackend::new(git_repo, extra_metadata_store) GitBackend::new(git_repo, extra_metadata_store)
} }
pub fn init_external(store_path: PathBuf, git_repo_path: PathBuf) -> Self { pub fn init_external(store_path: &Path, git_repo_path: &Path) -> Self {
let extra_path = store_path.join("extra"); let extra_path = store_path.join("extra");
std::fs::create_dir(&extra_path).unwrap(); std::fs::create_dir(&extra_path).unwrap();
let mut git_target_file = File::create(store_path.join("git_target")).unwrap(); let mut git_target_file = File::create(store_path.join("git_target")).unwrap();
@ -86,7 +86,7 @@ impl GitBackend {
GitBackend::new(repo, extra_metadata_store) GitBackend::new(repo, extra_metadata_store)
} }
pub fn load(store_path: PathBuf) -> Self { pub fn load(store_path: &Path) -> Self {
let mut git_target_file = File::open(store_path.join("git_target")).unwrap(); let mut git_target_file = File::open(store_path.join("git_target")).unwrap();
let mut buf = Vec::new(); let mut buf = Vec::new();
git_target_file.read_to_end(&mut buf).unwrap(); git_target_file.read_to_end(&mut buf).unwrap();
@ -536,7 +536,7 @@ mod tests {
#[test] #[test]
fn read_plain_git_commit() { fn read_plain_git_commit() {
let temp_dir = testutils::new_temp_dir(); let temp_dir = testutils::new_temp_dir();
let store_path = temp_dir.path().to_path_buf(); let store_path = temp_dir.path();
let git_repo_path = temp_dir.path().join("git"); let git_repo_path = temp_dir.path().join("git");
let git_repo = git2::Repository::init(&git_repo_path).unwrap(); let git_repo = git2::Repository::init(&git_repo_path).unwrap();
@ -581,7 +581,7 @@ mod tests {
// Check that the git commit above got the hash we expect // Check that the git commit above got the hash we expect
assert_eq!(git_commit_id.as_bytes(), commit_id.as_bytes()); assert_eq!(git_commit_id.as_bytes(), commit_id.as_bytes());
let store = GitBackend::init_external(store_path, git_repo_path); let store = GitBackend::init_external(store_path, &git_repo_path);
let commit = store.read_commit(&commit_id).unwrap(); let commit = store.read_commit(&commit_id).unwrap();
assert_eq!(&commit.change_id, &change_id); assert_eq!(&commit.change_id, &change_id);
assert_eq!(commit.parents, vec![CommitId::from_bytes(&[0; 20])]); assert_eq!(commit.parents, vec![CommitId::from_bytes(&[0; 20])]);
@ -647,7 +647,7 @@ mod tests {
#[test] #[test]
fn commit_has_ref() { fn commit_has_ref() {
let temp_dir = testutils::new_temp_dir(); let temp_dir = testutils::new_temp_dir();
let store = GitBackend::init_internal(temp_dir.path().to_path_buf()); let store = GitBackend::init_internal(temp_dir.path());
let signature = Signature { let signature = Signature {
name: "Someone".to_string(), name: "Someone".to_string(),
email: "someone@example.com".to_string(), email: "someone@example.com".to_string(),
@ -683,7 +683,7 @@ mod tests {
#[test] #[test]
fn overlapping_git_commit_id() { fn overlapping_git_commit_id() {
let temp_dir = testutils::new_temp_dir(); let temp_dir = testutils::new_temp_dir();
let store = GitBackend::init_internal(temp_dir.path().to_path_buf()); let store = GitBackend::init_internal(temp_dir.path());
let signature = Signature { let signature = Signature {
name: "Someone".to_string(), name: "Someone".to_string(),
email: "someone@example.com".to_string(), email: "someone@example.com".to_string(),

8
lib/src/local_backend.rs
View file

@ -16,7 +16,7 @@ use std::fmt::Debug;
use std::fs; use std::fs;
use std::fs::File; use std::fs::File;
use std::io::{ErrorKind, Read, Write}; use std::io::{ErrorKind, Read, Write};
use std::path::PathBuf; use std::path::{Path, PathBuf};
use blake2::{Blake2b512, Digest}; use blake2::{Blake2b512, Digest};
use protobuf::{Message, MessageField}; use protobuf::{Message, MessageField};
@ -56,7 +56,7 @@ pub struct LocalBackend {
} }
impl LocalBackend { impl LocalBackend {
pub fn init(store_path: PathBuf) -> Self { pub fn init(store_path: &Path) -> Self {
fs::create_dir(store_path.join("commits")).unwrap(); fs::create_dir(store_path.join("commits")).unwrap();
fs::create_dir(store_path.join("trees")).unwrap(); fs::create_dir(store_path.join("trees")).unwrap();
fs::create_dir(store_path.join("files")).unwrap(); fs::create_dir(store_path.join("files")).unwrap();
@ -70,11 +70,11 @@ impl LocalBackend {
backend backend
} }
pub fn load(store_path: PathBuf) -> Self { pub fn load(store_path: &Path) -> Self {
let root_commit_id = CommitId::from_bytes(&[0; 64]); let root_commit_id = CommitId::from_bytes(&[0; 64]);
let empty_tree_id = TreeId::from_hex("786a02f742015903c6c6fd852552d272912f4740e15847618a86e217f71f5419d25e1031afee585313896444934eb04b903a685b1448b755d56f701afe9be2ce"); let empty_tree_id = TreeId::from_hex("786a02f742015903c6c6fd852552d272912f4740e15847618a86e217f71f5419d25e1031afee585313896444934eb04b903a685b1448b755d56f701afe9be2ce");
LocalBackend { LocalBackend {
path: store_path, path: store_path.to_path_buf(),
root_commit_id, root_commit_id,
empty_tree_id, empty_tree_id,
} }

28
lib/src/repo.rs
View file

@ -128,14 +128,14 @@ impl Debug for ReadonlyRepo {
} }
impl ReadonlyRepo { impl ReadonlyRepo {
pub fn init_local(settings: &UserSettings, repo_path: PathBuf) -> Arc<ReadonlyRepo> { pub fn init_local(settings: &UserSettings, repo_path: &Path) -> Arc<ReadonlyRepo> {
Self::init(settings, repo_path, |store_path| { Self::init(settings, repo_path, |store_path| {
Box::new(LocalBackend::init(store_path)) Box::new(LocalBackend::init(store_path))
}) })
} }
/// Initializes a repo with a new Git backend in .jj/git/ (bare Git repo) /// Initializes a repo with a new Git backend in .jj/git/ (bare Git repo)
pub fn init_internal_git(settings: &UserSettings, repo_path: PathBuf) -> Arc<ReadonlyRepo> { pub fn init_internal_git(settings: &UserSettings, repo_path: &Path) -> Arc<ReadonlyRepo> {
Self::init(settings, repo_path, |store_path| { Self::init(settings, repo_path, |store_path| {
Box::new(GitBackend::init_internal(store_path)) Box::new(GitBackend::init_internal(store_path))
}) })
@ -144,11 +144,11 @@ impl ReadonlyRepo {
/// Initializes a repo with an existing Git backend at the specified path /// Initializes a repo with an existing Git backend at the specified path
pub fn init_external_git( pub fn init_external_git(
settings: &UserSettings, settings: &UserSettings,
repo_path: PathBuf, repo_path: &Path,
git_repo_path: PathBuf, git_repo_path: &Path,
) -> Arc<ReadonlyRepo> { ) -> Arc<ReadonlyRepo> {
Self::init(settings, repo_path, |store_path| { Self::init(settings, repo_path, |store_path| {
Box::new(GitBackend::init_external(store_path, git_repo_path.clone())) Box::new(GitBackend::init_external(store_path, git_repo_path))
}) })
} }
@ -161,12 +161,12 @@ impl ReadonlyRepo {
pub fn init( pub fn init(
user_settings: &UserSettings, user_settings: &UserSettings,
repo_path: PathBuf, repo_path: &Path,
backend_factory: impl FnOnce(PathBuf) -> Box<dyn Backend>, backend_factory: impl FnOnce(&Path) -> Box<dyn Backend>,
) -> Arc<ReadonlyRepo> { ) -> Arc<ReadonlyRepo> {
let repo_path = repo_path.canonicalize().unwrap(); let repo_path = repo_path.canonicalize().unwrap();
ReadonlyRepo::init_repo_dir(&repo_path); ReadonlyRepo::init_repo_dir(&repo_path);
let store = Store::new(backend_factory(repo_path.join("store"))); let store = Store::new(backend_factory(&repo_path.join("store")));
let repo_settings = user_settings.with_repo(&repo_path).unwrap(); let repo_settings = user_settings.with_repo(&repo_path).unwrap();
let op_store: Arc<dyn OpStore> = Arc::new(SimpleOpStore::init(repo_path.join("op_store"))); let op_store: Arc<dyn OpStore> = Arc::new(SimpleOpStore::init(repo_path.join("op_store")));
let mut root_view = op_store::View::default(); let mut root_view = op_store::View::default();
@ -194,7 +194,7 @@ impl ReadonlyRepo {
pub fn load_at_head( pub fn load_at_head(
user_settings: &UserSettings, user_settings: &UserSettings,
repo_path: PathBuf, repo_path: &Path,
) -> Result<Arc<ReadonlyRepo>, BackendError> { ) -> Result<Arc<ReadonlyRepo>, BackendError> {
RepoLoader::init(user_settings, repo_path) RepoLoader::init(user_settings, repo_path)
.load_at_head() .load_at_head()
@ -339,21 +339,21 @@ pub struct RepoLoader {
} }
impl RepoLoader { impl RepoLoader {
pub fn init(user_settings: &UserSettings, repo_path: PathBuf) -> Self { pub fn init(user_settings: &UserSettings, repo_path: &Path) -> Self {
let store_path = repo_path.join("store"); let store_path = repo_path.join("store");
let git_target_path = store_path.join("git_target"); let git_target_path = store_path.join("git_target");
let backend: Box<dyn Backend> = if git_target_path.is_file() { let backend: Box<dyn Backend> = if git_target_path.is_file() {
Box::new(GitBackend::load(store_path)) Box::new(GitBackend::load(&store_path))
} else { } else {
Box::new(LocalBackend::load(store_path)) Box::new(LocalBackend::load(&store_path))
}; };
let store = Store::new(backend); let store = Store::new(backend);
let repo_settings = user_settings.with_repo(&repo_path).unwrap(); let repo_settings = user_settings.with_repo(repo_path).unwrap();
let op_store: Arc<dyn OpStore> = Arc::new(SimpleOpStore::load(repo_path.join("op_store"))); let op_store: Arc<dyn OpStore> = Arc::new(SimpleOpStore::load(repo_path.join("op_store")));
let op_heads_store = Arc::new(OpHeadsStore::load(repo_path.join("op_heads"))); let op_heads_store = Arc::new(OpHeadsStore::load(repo_path.join("op_heads")));
let index_store = Arc::new(IndexStore::load(repo_path.join("index"))); let index_store = Arc::new(IndexStore::load(repo_path.join("index")));
Self { Self {
repo_path, repo_path: repo_path.to_path_buf(),
repo_settings, repo_settings,
store, store,
op_store, op_store,

10
lib/src/testutils.rs
View file

@ -77,11 +77,11 @@ impl TestRepo {
let repo = if use_git { let repo = if use_git {
let git_path = temp_dir.path().join("git-repo"); let git_path = temp_dir.path().join("git-repo");
git2::Repository::init(&git_path).unwrap(); git2::Repository::init(&git_path).unwrap();
ReadonlyRepo::init(&settings, repo_dir, |store_path| { ReadonlyRepo::init(&settings, &repo_dir, |store_path| {
Box::new(GitBackend::init_external(store_path, git_path.clone())) Box::new(GitBackend::init_external(store_path, &git_path))
}) })
} else { } else {
ReadonlyRepo::init(&settings, repo_dir, |store_path| { ReadonlyRepo::init(&settings, &repo_dir, |store_path| {
Box::new(LocalBackend::init(store_path)) Box::new(LocalBackend::init(store_path))
}) })
}; };
@ -109,9 +109,9 @@ impl TestWorkspace {
let (workspace, repo) = if use_git { let (workspace, repo) = if use_git {
let git_path = temp_dir.path().join("git-repo"); let git_path = temp_dir.path().join("git-repo");
git2::Repository::init(&git_path).unwrap(); git2::Repository::init(&git_path).unwrap();
Workspace::init_external_git(settings, workspace_root, git_path).unwrap() Workspace::init_external_git(settings, &workspace_root, &git_path).unwrap()
} else { } else {
Workspace::init_local(settings, workspace_root).unwrap() Workspace::init_local(settings, &workspace_root).unwrap()
}; };
Self { Self {

44
lib/src/workspace.rs
View file

@ -90,11 +90,7 @@ fn init_working_copy(
} }
impl Workspace { impl Workspace {
fn new( fn new(workspace_root: &Path, working_copy: WorkingCopy, repo_loader: RepoLoader) -> Workspace {
workspace_root: PathBuf,
working_copy: WorkingCopy,
repo_loader: RepoLoader,
) -> Workspace {
let workspace_root = workspace_root.canonicalize().unwrap(); let workspace_root = workspace_root.canonicalize().unwrap();
Workspace { Workspace {
workspace_root, workspace_root,
@ -105,7 +101,7 @@ impl Workspace {
pub fn init_local( pub fn init_local(
user_settings: &UserSettings, user_settings: &UserSettings,
workspace_root: PathBuf, workspace_root: &Path,
) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> { ) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> {
Self::init_with_backend(user_settings, workspace_root, |store_path| { Self::init_with_backend(user_settings, workspace_root, |store_path| {
Box::new(LocalBackend::init(store_path)) Box::new(LocalBackend::init(store_path))
@ -114,7 +110,7 @@ impl Workspace {
pub fn init_internal_git( pub fn init_internal_git(
user_settings: &UserSettings, user_settings: &UserSettings,
workspace_root: PathBuf, workspace_root: &Path,
) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> { ) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> {
Self::init_with_backend(user_settings, workspace_root, |store_path| { Self::init_with_backend(user_settings, workspace_root, |store_path| {
Box::new(GitBackend::init_internal(store_path)) Box::new(GitBackend::init_internal(store_path))
@ -123,27 +119,27 @@ impl Workspace {
pub fn init_external_git( pub fn init_external_git(
user_settings: &UserSettings, user_settings: &UserSettings,
workspace_root: PathBuf, workspace_root: &Path,
git_repo_path: PathBuf, git_repo_path: &Path,
) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> { ) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> {
Self::init_with_backend(user_settings, workspace_root, |store_path| { Self::init_with_backend(user_settings, workspace_root, |store_path| {
Box::new(GitBackend::init_external(store_path, git_repo_path.clone())) Box::new(GitBackend::init_external(store_path, git_repo_path))
}) })
} }
fn init_with_backend( fn init_with_backend(
user_settings: &UserSettings, user_settings: &UserSettings,
workspace_root: PathBuf, workspace_root: &Path,
backend_factory: impl FnOnce(PathBuf) -> Box<dyn Backend>, backend_factory: impl FnOnce(&Path) -> Box<dyn Backend>,
) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> { ) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> {
let jj_dir = create_jj_dir(&workspace_root)?; let jj_dir = create_jj_dir(workspace_root)?;
let repo_dir = jj_dir.join("repo"); let repo_dir = jj_dir.join("repo");
std::fs::create_dir(&repo_dir).unwrap(); std::fs::create_dir(&repo_dir).unwrap();
let repo = ReadonlyRepo::init(user_settings, repo_dir, backend_factory); let repo = ReadonlyRepo::init(user_settings, &repo_dir, backend_factory);
let (working_copy, repo) = init_working_copy( let (working_copy, repo) = init_working_copy(
user_settings, user_settings,
&repo, &repo,
&workspace_root, workspace_root,
&jj_dir, &jj_dir,
WorkspaceId::default(), WorkspaceId::default(),
); );
@ -154,11 +150,11 @@ impl Workspace {
pub fn init_workspace_with_existing_repo( pub fn init_workspace_with_existing_repo(
user_settings: &UserSettings, user_settings: &UserSettings,
workspace_root: PathBuf, workspace_root: &Path,
repo: &Arc<ReadonlyRepo>, repo: &Arc<ReadonlyRepo>,
workspace_id: WorkspaceId, workspace_id: WorkspaceId,
) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> { ) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> {
let jj_dir = create_jj_dir(&workspace_root)?; let jj_dir = create_jj_dir(workspace_root)?;
let repo_dir = repo.repo_path().canonicalize().unwrap(); let repo_dir = repo.repo_path().canonicalize().unwrap();
let mut repo_file = File::create(jj_dir.join("repo")).unwrap(); let mut repo_file = File::create(jj_dir.join("repo")).unwrap();
@ -166,19 +162,19 @@ impl Workspace {
.write_all(repo_dir.to_str().unwrap().as_bytes()) .write_all(repo_dir.to_str().unwrap().as_bytes())
.unwrap(); .unwrap();
let repo_loader = RepoLoader::init(user_settings, repo_dir); let repo_loader = RepoLoader::init(user_settings, &repo_dir);
let (working_copy, repo) = let (working_copy, repo) =
init_working_copy(user_settings, repo, &workspace_root, &jj_dir, workspace_id); init_working_copy(user_settings, repo, workspace_root, &jj_dir, workspace_id);
let workspace = Workspace::new(workspace_root, working_copy, repo_loader); let workspace = Workspace::new(workspace_root, working_copy, repo_loader);
Ok((workspace, repo)) Ok((workspace, repo))
} }
pub fn load( pub fn load(
user_settings: &UserSettings, user_settings: &UserSettings,
workspace_path: PathBuf, workspace_path: &Path,
) -> Result<Self, WorkspaceLoadError> { ) -> Result<Self, WorkspaceLoadError> {
let jj_dir = find_jj_dir(&workspace_path) let jj_dir = find_jj_dir(workspace_path)
.ok_or(WorkspaceLoadError::NoWorkspaceHere(workspace_path))?; .ok_or_else(|| WorkspaceLoadError::NoWorkspaceHere(workspace_path.to_owned()))?;
let workspace_root = jj_dir.parent().unwrap().to_owned(); let workspace_root = jj_dir.parent().unwrap().to_owned();
let mut repo_dir = jj_dir.join("repo"); let mut repo_dir = jj_dir.join("repo");
// If .jj/repo is a file, then we interpret its contents as a relative path to // If .jj/repo is a file, then we interpret its contents as a relative path to
@ -193,14 +189,14 @@ impl Workspace {
return Err(WorkspaceLoadError::RepoDoesNotExist(repo_dir)); return Err(WorkspaceLoadError::RepoDoesNotExist(repo_dir));
} }
} }
let repo_loader = RepoLoader::init(user_settings, repo_dir); let repo_loader = RepoLoader::init(user_settings, &repo_dir);
let working_copy_state_path = jj_dir.join("working_copy"); let working_copy_state_path = jj_dir.join("working_copy");
let working_copy = WorkingCopy::load( let working_copy = WorkingCopy::load(
repo_loader.store().clone(), repo_loader.store().clone(),
workspace_root.clone(), workspace_root.clone(),
working_copy_state_path, working_copy_state_path,
); );
Ok(Workspace::new(workspace_root, working_copy, repo_loader)) Ok(Workspace::new(&workspace_root, working_copy, repo_loader))
} }
pub fn workspace_root(&self) -> &PathBuf { pub fn workspace_root(&self) -> &PathBuf {

10
lib/tests/test_bad_locking.rs
View file

@ -108,7 +108,7 @@ fn test_bad_locking_children(use_git: bool) {
// Simulate a write of a commit that happens on one machine // Simulate a write of a commit that happens on one machine
let machine1_root = testutils::new_temp_dir(); let machine1_root = testutils::new_temp_dir();
copy_directory(workspace_root, machine1_root.path()); copy_directory(workspace_root, machine1_root.path());
let machine1_workspace = Workspace::load(&settings, machine1_root.path().to_owned()).unwrap(); let machine1_workspace = Workspace::load(&settings, machine1_root.path()).unwrap();
let machine1_repo = machine1_workspace let machine1_repo = machine1_workspace
.repo_loader() .repo_loader()
.load_at_head() .load_at_head()
@ -123,7 +123,7 @@ fn test_bad_locking_children(use_git: bool) {
// Simulate a write of a commit that happens on another machine // Simulate a write of a commit that happens on another machine
let machine2_root = testutils::new_temp_dir(); let machine2_root = testutils::new_temp_dir();
copy_directory(workspace_root, machine2_root.path()); copy_directory(workspace_root, machine2_root.path());
let machine2_workspace = Workspace::load(&settings, machine2_root.path().to_owned()).unwrap(); let machine2_workspace = Workspace::load(&settings, machine2_root.path()).unwrap();
let machine2_repo = machine2_workspace let machine2_repo = machine2_workspace
.repo_loader() .repo_loader()
.load_at_head() .load_at_head()
@ -144,7 +144,7 @@ fn test_bad_locking_children(use_git: bool) {
machine2_root.path(), machine2_root.path(),
merged_path.path(), merged_path.path(),
); );
let merged_workspace = Workspace::load(&settings, merged_path.path().to_owned()).unwrap(); let merged_workspace = Workspace::load(&settings, merged_path.path()).unwrap();
let merged_repo = merged_workspace let merged_repo = merged_workspace
.repo_loader() .repo_loader()
.load_at_head() .load_at_head()
@ -188,10 +188,10 @@ fn test_bad_locking_interrupted(use_git: bool) {
copy_directory(backup_path.path(), &op_heads_dir); copy_directory(backup_path.path(), &op_heads_dir);
// Reload the repo and check that only the new head is present. // Reload the repo and check that only the new head is present.
let reloaded_repo = ReadonlyRepo::load_at_head(&settings, repo.repo_path().clone()).unwrap(); let reloaded_repo = ReadonlyRepo::load_at_head(&settings, repo.repo_path()).unwrap();
assert_eq!(reloaded_repo.op_id(), &op_id); assert_eq!(reloaded_repo.op_id(), &op_id);
// Reload once more to make sure that the .jj/op_heads/ directory was updated // Reload once more to make sure that the .jj/op_heads/ directory was updated
// correctly. // correctly.
let reloaded_repo = ReadonlyRepo::load_at_head(&settings, repo.repo_path().clone()).unwrap(); let reloaded_repo = ReadonlyRepo::load_at_head(&settings, repo.repo_path()).unwrap();
assert_eq!(reloaded_repo.op_id(), &op_id); assert_eq!(reloaded_repo.op_id(), &op_id);
} }

4
lib/tests/test_commit_concurrent.rs
View file

@ -85,7 +85,7 @@ fn test_commit_parallel_instances(use_git: bool) {
let mut threads = vec![]; let mut threads = vec![];
for _ in 0..num_threads { for _ in 0..num_threads {
let settings = settings.clone(); let settings = settings.clone();
let repo = ReadonlyRepo::load_at_head(&settings, repo.repo_path().clone()).unwrap(); let repo = ReadonlyRepo::load_at_head(&settings, repo.repo_path()).unwrap();
let handle = thread::spawn(move || { let handle = thread::spawn(move || {
let mut tx = repo.start_transaction("test"); let mut tx = repo.start_transaction("test");
testutils::create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo()); testutils::create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo());
@ -98,7 +98,7 @@ fn test_commit_parallel_instances(use_git: bool) {
} }
// One commit per thread plus the commit from the initial checkout on top of the // One commit per thread plus the commit from the initial checkout on top of the
// root commit // root commit
let repo = ReadonlyRepo::load_at_head(&settings, repo.repo_path().clone()).unwrap(); let repo = ReadonlyRepo::load_at_head(&settings, repo.repo_path()).unwrap();
assert_eq!(repo.view().heads().len(), num_threads + 1); assert_eq!(repo.view().heads().len(), num_threads + 1);
// One addition operation for initializing the repo, one for checking out the // One addition operation for initializing the repo, one for checking out the

6
lib/tests/test_git.rs
View file

@ -284,7 +284,7 @@ impl GitRepoData {
git2::Repository::clone(origin_repo_dir.to_str().unwrap(), &git_repo_dir).unwrap(); git2::Repository::clone(origin_repo_dir.to_str().unwrap(), &git_repo_dir).unwrap();
let jj_repo_dir = temp_dir.path().join("jj"); let jj_repo_dir = temp_dir.path().join("jj");
std::fs::create_dir(&jj_repo_dir).unwrap(); std::fs::create_dir(&jj_repo_dir).unwrap();
let repo = ReadonlyRepo::init_external_git(&settings, jj_repo_dir, git_repo_dir); let repo = ReadonlyRepo::init_external_git(&settings, &jj_repo_dir, &git_repo_dir);
Self { Self {
settings, settings,
_temp_dir: temp_dir, _temp_dir: temp_dir,
@ -520,7 +520,7 @@ fn test_init() {
let initial_git_commit = empty_git_commit(&git_repo, "refs/heads/main", &[]); let initial_git_commit = empty_git_commit(&git_repo, "refs/heads/main", &[]);
let initial_commit_id = commit_id(&initial_git_commit); let initial_commit_id = commit_id(&initial_git_commit);
std::fs::create_dir(&jj_repo_dir).unwrap(); std::fs::create_dir(&jj_repo_dir).unwrap();
let repo = ReadonlyRepo::init_external_git(&settings, jj_repo_dir, git_repo_dir); let repo = ReadonlyRepo::init_external_git(&settings, &jj_repo_dir, &git_repo_dir);
// The refs were *not* imported -- it's the caller's responsibility to import // The refs were *not* imported -- it's the caller's responsibility to import
// any refs they care about. // any refs they care about.
assert!(!repo.view().heads().contains(&initial_commit_id)); assert!(!repo.view().heads().contains(&initial_commit_id));
@ -682,7 +682,7 @@ fn set_up_push_repos(settings: &UserSettings, temp_dir: &TempDir) -> PushTestSet
let initial_commit_id = commit_id(&initial_git_commit); let initial_commit_id = commit_id(&initial_git_commit);
git2::Repository::clone(source_repo_dir.to_str().unwrap(), &clone_repo_dir).unwrap(); git2::Repository::clone(source_repo_dir.to_str().unwrap(), &clone_repo_dir).unwrap();
std::fs::create_dir(&jj_repo_dir).unwrap(); std::fs::create_dir(&jj_repo_dir).unwrap();
let jj_repo = ReadonlyRepo::init_external_git(settings, jj_repo_dir, clone_repo_dir); let jj_repo = ReadonlyRepo::init_external_git(settings, &jj_repo_dir, &clone_repo_dir);
let mut tx = jj_repo.start_transaction("test"); let mut tx = jj_repo.start_transaction("test");
let new_commit = testutils::create_random_commit(settings, &jj_repo) let new_commit = testutils::create_random_commit(settings, &jj_repo)
.set_parents(vec![initial_commit_id]) .set_parents(vec![initial_commit_id])

6
lib/tests/test_index.rs
View file

@ -256,7 +256,7 @@ fn test_index_commits_previous_operations(use_git: bool) {
std::fs::remove_dir_all(&index_operations_dir).unwrap(); std::fs::remove_dir_all(&index_operations_dir).unwrap();
std::fs::create_dir(&index_operations_dir).unwrap(); std::fs::create_dir(&index_operations_dir).unwrap();
let repo = ReadonlyRepo::load_at_head(&settings, repo.repo_path().clone()).unwrap(); let repo = ReadonlyRepo::load_at_head(&settings, repo.repo_path()).unwrap();
let index = repo.index(); let index = repo.index();
// There should be the root commit, plus 3 more // There should be the root commit, plus 3 more
assert_eq!(index.num_commits(), 1 + 3); assert_eq!(index.num_commits(), 1 + 3);
@ -301,7 +301,7 @@ fn test_index_commits_incremental(use_git: bool) {
let commit_c = child_commit(&settings, &repo, &commit_b).write_to_repo(tx.mut_repo()); let commit_c = child_commit(&settings, &repo, &commit_b).write_to_repo(tx.mut_repo());
tx.commit(); tx.commit();
let repo = ReadonlyRepo::load_at_head(&settings, repo.repo_path().clone()).unwrap(); let repo = ReadonlyRepo::load_at_head(&settings, repo.repo_path()).unwrap();
let index = repo.index(); let index = repo.index();
// There should be the root commit, plus 3 more // There should be the root commit, plus 3 more
assert_eq!(index.num_commits(), 1 + 3); assert_eq!(index.num_commits(), 1 + 3);
@ -344,7 +344,7 @@ fn test_index_commits_incremental_empty_transaction(use_git: bool) {
repo.start_transaction("test").commit(); repo.start_transaction("test").commit();
let repo = ReadonlyRepo::load_at_head(&settings, repo.repo_path().clone()).unwrap(); let repo = ReadonlyRepo::load_at_head(&settings, repo.repo_path()).unwrap();
let index = repo.index(); let index = repo.index();
// There should be the root commit, plus 1 more // There should be the root commit, plus 1 more
assert_eq!(index.num_commits(), 1 + 1); assert_eq!(index.num_commits(), 1 + 1);

6
lib/tests/test_init.rs
View file

@ -32,7 +32,7 @@ fn test_init_local() {
let settings = testutils::user_settings(); let settings = testutils::user_settings();
let temp_dir = testutils::new_temp_dir(); let temp_dir = testutils::new_temp_dir();
let (canonical, uncanonical) = canonicalize(temp_dir.path()); let (canonical, uncanonical) = canonicalize(temp_dir.path());
let (workspace, repo) = Workspace::init_local(&settings, uncanonical).unwrap(); let (workspace, repo) = Workspace::init_local(&settings, &uncanonical).unwrap();
assert!(repo.store().git_repo().is_none()); assert!(repo.store().git_repo().is_none());
assert_eq!(repo.repo_path(), &canonical.join(".jj").join("repo")); assert_eq!(repo.repo_path(), &canonical.join(".jj").join("repo"));
assert_eq!(workspace.workspace_root(), &canonical); assert_eq!(workspace.workspace_root(), &canonical);
@ -47,7 +47,7 @@ fn test_init_internal_git() {
let settings = testutils::user_settings(); let settings = testutils::user_settings();
let temp_dir = testutils::new_temp_dir(); let temp_dir = testutils::new_temp_dir();
let (canonical, uncanonical) = canonicalize(temp_dir.path()); let (canonical, uncanonical) = canonicalize(temp_dir.path());
let (workspace, repo) = Workspace::init_internal_git(&settings, uncanonical).unwrap(); let (workspace, repo) = Workspace::init_internal_git(&settings, &uncanonical).unwrap();
assert!(repo.store().git_repo().is_some()); assert!(repo.store().git_repo().is_some());
assert_eq!(repo.repo_path(), &canonical.join(".jj").join("repo")); assert_eq!(repo.repo_path(), &canonical.join(".jj").join("repo"));
assert_eq!(workspace.workspace_root(), &canonical); assert_eq!(workspace.workspace_root(), &canonical);
@ -66,7 +66,7 @@ fn test_init_external_git() {
git2::Repository::init(&git_repo_path).unwrap(); git2::Repository::init(&git_repo_path).unwrap();
std::fs::create_dir(&uncanonical.join("jj")).unwrap(); std::fs::create_dir(&uncanonical.join("jj")).unwrap();
let (workspace, repo) = let (workspace, repo) =
Workspace::init_external_git(&settings, uncanonical.join("jj"), git_repo_path).unwrap(); Workspace::init_external_git(&settings, &uncanonical.join("jj"), &git_repo_path).unwrap();
assert!(repo.store().git_repo().is_some()); assert!(repo.store().git_repo().is_some());
assert_eq!( assert_eq!(
repo.repo_path(), repo.repo_path(),

4
lib/tests/test_load_repo.rs
View file

@ -34,13 +34,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 // If we load the repo at head, we should not see the commit since it was
// removed // removed
let loader = RepoLoader::init(&settings, repo.repo_path().clone()); let loader = RepoLoader::init(&settings, repo.repo_path());
let head_repo = loader.load_at_head().resolve(&settings).unwrap(); let head_repo = loader.load_at_head().resolve(&settings).unwrap();
assert!(!head_repo.view().heads().contains(commit.id())); assert!(!head_repo.view().heads().contains(commit.id()));
// If we load the repo at the previous operation, we should see the commit since // If we load the repo at the previous operation, we should see the commit since
// it has not been removed yet // it has not been removed yet
let loader = RepoLoader::init(&settings, repo.repo_path().clone()); let loader = RepoLoader::init(&settings, repo.repo_path());
let old_repo = loader.load_at(repo.operation()); let old_repo = loader.load_at(repo.operation());
assert!(old_repo.view().heads().contains(commit.id())); assert!(old_repo.view().heads().contains(commit.id()));
} }

6
lib/tests/test_working_copy_concurrent.rs
View file

@ -57,7 +57,7 @@ fn test_concurrent_checkout(use_git: bool) {
// Check out tree2 from another process (simulated by another workspace // Check out tree2 from another process (simulated by another workspace
// instance) // instance)
let mut workspace2 = Workspace::load(&settings, workspace1_root.clone()).unwrap(); let mut workspace2 = Workspace::load(&settings, &workspace1_root).unwrap();
workspace2 workspace2
.working_copy_mut() .working_copy_mut()
.check_out(repo1.op_id().clone(), Some(&tree_id1), &tree2) .check_out(repo1.op_id().clone(), Some(&tree_id1), &tree2)
@ -70,7 +70,7 @@ fn test_concurrent_checkout(use_git: bool) {
); );
// Check that the tree2 is still checked out on disk. // Check that the tree2 is still checked out on disk.
let workspace3 = Workspace::load(&settings, workspace1_root).unwrap(); let workspace3 = Workspace::load(&settings, &workspace1_root).unwrap();
assert_eq!(workspace3.working_copy().current_tree_id(), tree_id2); assert_eq!(workspace3.working_copy().current_tree_id(), tree_id2);
} }
@ -112,7 +112,7 @@ fn test_checkout_parallel(use_git: bool) {
let settings = settings.clone(); let settings = settings.clone();
let workspace_root = workspace_root.clone(); let workspace_root = workspace_root.clone();
let handle = thread::spawn(move || { let handle = thread::spawn(move || {
let mut workspace = Workspace::load(&settings, workspace_root).unwrap(); let mut workspace = Workspace::load(&settings, &workspace_root).unwrap();
let tree = workspace let tree = workspace
.repo_loader() .repo_loader()
.store() .store()

8
lib/tests/test_workspace.rs
View file

@ -24,7 +24,7 @@ fn test_load_bad_path() {
let temp_dir = testutils::new_temp_dir(); let temp_dir = testutils::new_temp_dir();
let workspace_root = temp_dir.path().to_owned(); let workspace_root = temp_dir.path().to_owned();
// We haven't created a repo in the workspace_root, so it should fail to load. // We haven't created a repo in the workspace_root, so it should fail to load.
let result = Workspace::load(&settings, workspace_root.clone()); let result = Workspace::load(&settings, &workspace_root);
assert_eq!( assert_eq!(
result.err(), result.err(),
Some(WorkspaceLoadError::NoWorkspaceHere(workspace_root)) Some(WorkspaceLoadError::NoWorkspaceHere(workspace_root))
@ -40,7 +40,7 @@ fn test_load_from_subdir(use_git: bool) {
let subdir = workspace.workspace_root().join("dir").join("subdir"); let subdir = workspace.workspace_root().join("dir").join("subdir");
std::fs::create_dir_all(subdir.clone()).unwrap(); std::fs::create_dir_all(subdir.clone()).unwrap();
let same_workspace = Workspace::load(&settings, subdir); let same_workspace = Workspace::load(&settings, &subdir);
assert!(same_workspace.is_ok()); assert!(same_workspace.is_ok());
let same_workspace = same_workspace.unwrap(); let same_workspace = same_workspace.unwrap();
assert_eq!(same_workspace.repo_path(), workspace.repo_path()); assert_eq!(same_workspace.repo_path(), workspace.repo_path());
@ -59,7 +59,7 @@ fn test_init_additional_workspace(use_git: bool) {
std::fs::create_dir(&ws2_root).unwrap(); std::fs::create_dir(&ws2_root).unwrap();
let (ws2, repo) = Workspace::init_workspace_with_existing_repo( let (ws2, repo) = Workspace::init_workspace_with_existing_repo(
&settings, &settings,
ws2_root.clone(), &ws2_root,
&test_workspace.repo, &test_workspace.repo,
ws2_id.clone(), ws2_id.clone(),
) )
@ -78,7 +78,7 @@ fn test_init_additional_workspace(use_git: bool) {
workspace.repo_path().canonicalize().unwrap() workspace.repo_path().canonicalize().unwrap()
); );
assert_eq!(*ws2.workspace_root(), ws2_root.canonicalize().unwrap()); assert_eq!(*ws2.workspace_root(), ws2_root.canonicalize().unwrap());
let same_workspace = Workspace::load(&settings, ws2_root); let same_workspace = Workspace::load(&settings, &ws2_root);
assert!(same_workspace.is_ok()); assert!(same_workspace.is_ok());
let same_workspace = same_workspace.unwrap(); let same_workspace = same_workspace.unwrap();
assert_eq!(same_workspace.workspace_id(), ws2_id); assert_eq!(same_workspace.workspace_id(), ws2_id);

2
src/cli_util.rs
View file

@ -201,7 +201,7 @@ impl<'help> CommandHelper<'help> {
pub fn workspace_helper(&self, ui: &mut Ui) -> Result<WorkspaceCommandHelper, CommandError> { pub fn workspace_helper(&self, ui: &mut Ui) -> Result<WorkspaceCommandHelper, CommandError> {
let wc_path_str = self.global_args.repository.as_deref().unwrap_or("."); let wc_path_str = self.global_args.repository.as_deref().unwrap_or(".");
let wc_path = ui.cwd().join(wc_path_str); let wc_path = ui.cwd().join(wc_path_str);
let workspace = match Workspace::load(ui.settings(), wc_path) { let workspace = match Workspace::load(ui.settings(), &wc_path) {
Ok(workspace) => workspace, Ok(workspace) => workspace,
Err(WorkspaceLoadError::NoWorkspaceHere(wc_path)) => { Err(WorkspaceLoadError::NoWorkspaceHere(wc_path)) => {
let mut message = format!("There is no jj repo in \"{}\"", wc_path_str); let mut message = format!("There is no jj repo in \"{}\"", wc_path_str);

10
src/commands.rs
View file

@ -1122,7 +1122,7 @@ fn cmd_init(ui: &mut Ui, command: &CommandHelper, args: &InitArgs) -> Result<(),
.join(relative_path); .join(relative_path);
} }
let (workspace, repo) = let (workspace, repo) =
Workspace::init_external_git(ui.settings(), wc_path.clone(), git_store_path)?; Workspace::init_external_git(ui.settings(), &wc_path, &git_store_path)?;
let git_repo = repo.store().git_repo().unwrap(); let git_repo = repo.store().git_repo().unwrap();
let mut workspace_command = command.for_loaded_repo(ui, workspace, repo)?; let mut workspace_command = command.for_loaded_repo(ui, workspace, repo)?;
if workspace_command.working_copy_shared_with_git() { if workspace_command.working_copy_shared_with_git() {
@ -1143,9 +1143,9 @@ fn cmd_init(ui: &mut Ui, command: &CommandHelper, args: &InitArgs) -> Result<(),
} }
} }
} else if args.git { } else if args.git {
Workspace::init_internal_git(ui.settings(), wc_path.clone())?; Workspace::init_internal_git(ui.settings(), &wc_path)?;
} else { } else {
Workspace::init_local(ui.settings(), wc_path.clone())?; Workspace::init_local(ui.settings(), &wc_path)?;
}; };
let cwd = ui.cwd().canonicalize().unwrap(); let cwd = ui.cwd().canonicalize().unwrap();
let relative_wc_path = ui::relative_path(&cwd, &wc_path); let relative_wc_path = ui::relative_path(&cwd, &wc_path);
@ -3883,7 +3883,7 @@ fn cmd_workspace_add(
} }
let (new_workspace, repo) = Workspace::init_workspace_with_existing_repo( let (new_workspace, repo) = Workspace::init_workspace_with_existing_repo(
ui.settings(), ui.settings(),
destination_path.clone(), &destination_path,
repo, repo,
workspace_id, workspace_id,
)?; )?;
@ -4194,7 +4194,7 @@ fn do_git_clone(
source: &str, source: &str,
wc_path: &Path, wc_path: &Path,
) -> Result<(WorkspaceCommandHelper, Option<String>), CommandError> { ) -> Result<(WorkspaceCommandHelper, Option<String>), CommandError> {
let (workspace, repo) = Workspace::init_internal_git(ui.settings(), wc_path.to_path_buf())?; let (workspace, repo) = Workspace::init_internal_git(ui.settings(), wc_path)?;
let git_repo = get_git_repo(repo.store())?; let git_repo = get_git_repo(repo.store())?;
writeln!(ui, r#"Fetching into new repo in "{}""#, wc_path.display())?; writeln!(ui, r#"Fetching into new repo in "{}""#, wc_path.display())?;
let mut workspace_command = command.for_loaded_repo(ui, workspace, repo)?; let mut workspace_command = command.for_loaded_repo(ui, workspace, repo)?;