From 4734eb6493ade314038f462c2d1bf495e43561cc Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Fri, 18 Dec 2020 23:50:10 -0800 Subject: [PATCH] working_copy: let WorkingCopy and TreeState have the working copy path I don't know why I didn't do it this way from the beginning. --- lib/src/repo.rs | 14 +++- lib/src/working_copy.rs | 99 ++++++++++++++--------- lib/tests/test_working_copy.rs | 4 +- lib/tests/test_working_copy_concurrent.rs | 10 +-- src/commands.rs | 2 +- 5 files changed, 79 insertions(+), 50 deletions(-) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index d69afa909..20e9d85d8 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -118,7 +118,11 @@ impl ReadonlyRepo { let store = StoreWrapper::new(store); fs::create_dir(repo_path.join("working_copy")).unwrap(); - let working_copy = WorkingCopy::init(store.clone(), repo_path.join("working_copy")); + let working_copy = WorkingCopy::init( + store.clone(), + wc_path.clone(), + repo_path.join("working_copy"), + ); fs::create_dir(repo_path.join("view")).unwrap(); let signature = signature(user_settings); @@ -161,7 +165,7 @@ impl ReadonlyRepo { ReadonlyRepo::init_cycles(&mut repo, evolution); repo.working_copy_locked() - .check_out(&repo, checkout_commit) + .check_out(checkout_commit) .expect("failed to check out root commit"); repo } @@ -184,7 +188,11 @@ impl ReadonlyRepo { } let store = StoreWrapper::new(store); let repo_settings = user_settings.with_repo(&repo_path).unwrap(); - let working_copy = WorkingCopy::load(store.clone(), repo_path.join("working_copy")); + let working_copy = WorkingCopy::load( + store.clone(), + wc_path.clone(), + repo_path.join("working_copy"), + ); let view = ReadonlyView::load(store.clone(), repo_path.join("view")); let repo = ReadonlyRepo { repo_path, diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index bceb31f32..0f157b6b1 100644 --- a/lib/src/working_copy.rs +++ b/lib/src/working_copy.rs @@ -72,7 +72,8 @@ impl FileState { struct TreeState { store: Arc, - path: PathBuf, + working_copy_path: PathBuf, + state_path: PathBuf, tree_id: TreeId, file_states: BTreeMap, read_time: MillisSinceEpoch, @@ -152,40 +153,53 @@ impl TreeState { &self.file_states } - pub fn init(store: Arc, path: PathBuf) -> TreeState { - let mut wc = TreeState::empty(store, path); + pub fn init( + store: Arc, + working_copy_path: PathBuf, + state_path: PathBuf, + ) -> TreeState { + let mut wc = TreeState::empty(store, working_copy_path, state_path); wc.save(); wc } - fn empty(store: Arc, path: PathBuf) -> TreeState { + fn empty( + store: Arc, + working_copy_path: PathBuf, + state_path: PathBuf, + ) -> TreeState { let tree_id = store.empty_tree_id().clone(); TreeState { store, - path, + working_copy_path, + state_path, tree_id, file_states: BTreeMap::new(), read_time: MillisSinceEpoch(0), } } - pub fn load(store: Arc, path: PathBuf) -> TreeState { - let maybe_file = File::open(path.join("tree_state")); + pub fn load( + store: Arc, + working_copy_path: PathBuf, + state_path: PathBuf, + ) -> TreeState { + let maybe_file = File::open(state_path.join("tree_state")); let file = match maybe_file { Err(ref err) if err.kind() == std::io::ErrorKind::NotFound => { - return TreeState::init(store, path); + return TreeState::init(store, working_copy_path, state_path); } result => result.unwrap(), }; - let mut wc = TreeState::empty(store, path); + let mut wc = TreeState::empty(store, working_copy_path, state_path); wc.read(file); wc } fn update_read_time(&mut self) { let own_file_state = self - .file_state(&self.path.join("tree_state")) + .file_state(&self.state_path.join("tree_state")) .unwrap_or_else(FileState::null); self.read_time = own_file_state.mtime; } @@ -207,12 +221,14 @@ impl TreeState { .insert(file.to_internal_string(), file_state_to_proto(file_state)); } - let mut temp_file = NamedTempFile::new_in(&self.path).unwrap(); + let mut temp_file = NamedTempFile::new_in(&self.state_path).unwrap(); // update read time while we still have the file open for writes, so we know // there is no unknown data in it self.update_read_time(); proto.write_to_writer(temp_file.as_file_mut()).unwrap(); - temp_file.persist(self.path.join("tree_state")).unwrap(); + temp_file + .persist(self.state_path.join("tree_state")) + .unwrap(); } fn file_state(&self, path: &PathBuf) -> Option { @@ -255,8 +271,8 @@ impl TreeState { // Look for changes to the working copy. If there are any changes, create // a new tree from it and return it, and also update the dirstate on disk. // TODO: respect ignores - pub fn write_tree(&mut self, working_copy_path: PathBuf) -> &TreeId { - let mut work = vec![(DirRepoPath::root(), working_copy_path)]; + pub fn write_tree(&mut self) -> &TreeId { + let mut work = vec![(DirRepoPath::root(), self.working_copy_path.clone())]; let mut tree_builder = self.store.tree_builder(self.tree_id.clone()); let mut deleted_files: HashSet<&FileRepoPath> = self.file_states.keys().collect(); let mut modified_files = BTreeMap::new(); @@ -484,7 +500,8 @@ impl TreeState { pub struct WorkingCopy { store: Arc, - path: PathBuf, + working_copy_path: PathBuf, + state_path: PathBuf, commit_id: RefCell>, tree_state: RefCell>, // cached commit @@ -492,29 +509,39 @@ pub struct WorkingCopy { } impl WorkingCopy { - pub fn init(store: Arc, path: PathBuf) -> WorkingCopy { + pub fn init( + store: Arc, + working_copy_path: PathBuf, + state_path: PathBuf, + ) -> WorkingCopy { // Leave the commit_id empty so a subsequent call to check out the root revision // will have an effect. let proto = protos::working_copy::Checkout::new(); let mut file = OpenOptions::new() .create_new(true) .write(true) - .open(path.join("checkout")) + .open(state_path.join("checkout")) .unwrap(); proto.write_to_writer(&mut file).unwrap(); WorkingCopy { store, - path, + working_copy_path, + state_path, commit_id: RefCell::new(None), tree_state: RefCell::new(None), commit: RefCell::new(None), } } - pub fn load(store: Arc, path: PathBuf) -> WorkingCopy { + pub fn load( + store: Arc, + working_copy_path: PathBuf, + state_path: PathBuf, + ) -> WorkingCopy { WorkingCopy { store, - path, + working_copy_path, + state_path, commit_id: RefCell::new(None), tree_state: RefCell::new(None), commit: RefCell::new(None), @@ -522,13 +549,13 @@ impl WorkingCopy { } fn write_proto(&self, proto: protos::working_copy::Checkout) { - let mut temp_file = NamedTempFile::new_in(&self.path).unwrap(); + let mut temp_file = NamedTempFile::new_in(&self.state_path).unwrap(); proto.write_to_writer(temp_file.as_file_mut()).unwrap(); - temp_file.persist(self.path.join("checkout")).unwrap(); + temp_file.persist(self.state_path.join("checkout")).unwrap(); } fn read_proto(&self) -> protos::working_copy::Checkout { - let mut file = File::open(self.path.join("checkout")).unwrap(); + let mut file = File::open(self.state_path.join("checkout")).unwrap(); protobuf::parse_from_reader(&mut file).unwrap() } @@ -565,8 +592,11 @@ impl WorkingCopy { fn tree_state(&self) -> RefMut> { if self.tree_state.borrow().is_none() { - self.tree_state - .replace(Some(TreeState::load(self.store.clone(), self.path.clone()))); + self.tree_state.replace(Some(TreeState::load( + self.store.clone(), + self.working_copy_path.clone(), + self.state_path.clone(), + ))); } self.tree_state.borrow_mut() } @@ -589,13 +619,9 @@ impl WorkingCopy { self.write_proto(proto); } - pub fn check_out( - &self, - repo: &ReadonlyRepo, - commit: Commit, - ) -> Result { + pub fn check_out(&self, commit: Commit) -> Result { assert!(commit.is_open()); - let lock_path = self.path.join("working_copy.lock"); + let lock_path = self.state_path.join("working_copy.lock"); let _lock = FileLock::lock(lock_path); // TODO: Write a "pending_checkout" file with the old and new TreeIds so we can @@ -620,7 +646,7 @@ impl WorkingCopy { .tree_state() .as_mut() .unwrap() - .check_out(commit.tree().id().clone(), repo.working_copy_path())?; + .check_out(commit.tree().id().clone(), &self.working_copy_path)?; self.commit_id.replace(Some(commit.id().clone())); self.commit.replace(Some(commit)); @@ -631,7 +657,7 @@ impl WorkingCopy { } pub fn commit(&self, settings: &UserSettings, repo: &mut ReadonlyRepo) -> Commit { - let lock_path = self.path.join("working_copy.lock"); + let lock_path = self.state_path.join("working_copy.lock"); let _lock = FileLock::lock(lock_path); // Check if the current checkout has changed on disk after we read it. It's fine @@ -643,12 +669,7 @@ impl WorkingCopy { .replace(Some(CommitId(current_proto.commit_id))); let current_commit = self.current_commit(); - let new_tree_id = self - .tree_state() - .as_mut() - .unwrap() - .write_tree(repo.working_copy_path().clone()) - .clone(); + let new_tree_id = self.tree_state().as_mut().unwrap().write_tree().clone(); if &new_tree_id != current_commit.tree().id() { let mut tx = repo.start_transaction("commit working copy"); let commit = CommitBuilder::for_rewrite_from(settings, repo.store(), ¤t_commit) diff --git a/lib/tests/test_working_copy.rs b/lib/tests/test_working_copy.rs index d55a3b564..3cbd35e4d 100644 --- a/lib/tests/test_working_copy.rs +++ b/lib/tests/test_working_copy.rs @@ -173,9 +173,9 @@ fn test_checkout_file_transitions(use_git: bool) { let owned_wc = repo.working_copy().clone(); let wc = owned_wc.lock().unwrap(); - wc.check_out(&repo, left_commit).unwrap(); + wc.check_out(left_commit).unwrap(); wc.commit(&settings, Arc::get_mut(&mut repo).unwrap()); - wc.check_out(&repo, right_commit.clone()).unwrap(); + wc.check_out(right_commit.clone()).unwrap(); // Check that the working copy is clean. let after_commit = wc.commit(&settings, Arc::get_mut(&mut repo).unwrap()); diff --git a/lib/tests/test_working_copy_concurrent.rs b/lib/tests/test_working_copy_concurrent.rs index 18ca637fb..1cab65881 100644 --- a/lib/tests/test_working_copy_concurrent.rs +++ b/lib/tests/test_working_copy_concurrent.rs @@ -44,18 +44,18 @@ fn test_concurrent_checkout(use_git: bool) { // Check out commit1 let wc1 = repo1.working_copy_locked(); - wc1.check_out(&repo1, commit1).unwrap(); + 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()); repo2 .working_copy_locked() - .check_out(&repo2, commit2.clone()) + .check_out(commit2.clone()) .unwrap(); // Checking out another commit (via the first repo instance) should now fail. assert_eq!( - wc1.check_out(&repo1, commit3), + wc1.check_out(commit3), Err(CheckoutError::ConcurrentCheckout) ); @@ -121,7 +121,7 @@ fn test_checkout_parallel(use_git: bool) { let commit = CommitBuilder::for_new_commit(&settings, store, tree.id().clone()) .set_open(true) .write_to_transaction(&mut tx); - repo.working_copy_locked().check_out(&repo, commit).unwrap(); + repo.working_copy_locked().check_out(commit).unwrap(); tx.commit(); let mut threads = vec![]; @@ -136,7 +136,7 @@ fn test_checkout_parallel(use_git: bool) { let owned_wc = repo.working_copy().clone(); let wc = owned_wc.lock().unwrap(); let commit = repo.store().get_commit(&commit_id).unwrap(); - let stats = wc.check_out(&repo, commit).unwrap(); + let stats = wc.check_out(commit).unwrap(); assert_eq!(stats.updated_files, 0); assert_eq!(stats.added_files, 1); assert_eq!(stats.removed_files, 1); diff --git a/src/commands.rs b/src/commands.rs index 60bde508c..4315f8bb5 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -210,7 +210,7 @@ fn update_working_copy( ui.write("\n"); // TODO: CheckoutError::ConcurrentCheckout should probably just result in a // warning for most commands (but be an error for the checkout command) - let stats = wc.check_out(repo, new_commit.clone()).map_err(|err| { + let stats = wc.check_out(new_commit.clone()).map_err(|err| { CommandError::InternalError(format!( "Failed to check out commit {}: {}", new_commit.id().hex(),