diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 35ae07a78..436a46669 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -543,10 +543,6 @@ impl MutableRepo { rebaser.rebased().len() } - pub fn get_checkout(&mut self) -> CommitId { - self.view.borrow().checkout().clone() - } - pub fn set_checkout(&mut self, workspace_id: WorkspaceId, commit_id: CommitId) { self.view_mut().set_checkout(workspace_id, commit_id); } diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index 240e5228f..2f6c26aba 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -20,7 +20,7 @@ use crate::backend::CommitId; use crate::commit::Commit; use crate::commit_builder::CommitBuilder; use crate::dag_walk; -use crate::op_store::{RefTarget, WorkspaceId}; +use crate::op_store::RefTarget; use crate::repo::{MutableRepo, RepoRef}; use crate::repo_path::RepoPath; use crate::revset::RevsetExpression; @@ -291,15 +291,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { } fn update_references(&mut self, old_commit_id: CommitId, new_commit_ids: Vec) { - // TODO: Either make this update the checkout in all workspaces or pass in a - // particular workspace ID to this function - if self.mut_repo.get_checkout() == old_commit_id { - // We arbitrarily pick a new checkout among the candidates. - let new_commit_id = new_commit_ids[0].clone(); - let new_commit = self.mut_repo.store().get_commit(&new_commit_id).unwrap(); - self.mut_repo - .check_out(WorkspaceId::default(), self.settings, &new_commit); - } + self.update_checkouts(&old_commit_id, &new_commit_ids); if let Some(branch_names) = self.branches.get(&old_commit_id) { let mut branch_updates = vec![]; @@ -343,7 +335,36 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { } } - // TODO: Perhaps the interface since it's not just about rebasing commits. + fn update_checkouts(&mut self, old_commit_id: &CommitId, new_commit_ids: &[CommitId]) { + let mut workspaces_to_update = vec![]; + for (workspace_id, checkout_id) in self.mut_repo.view().checkouts() { + if checkout_id == old_commit_id { + workspaces_to_update.push(workspace_id.clone()); + } + } + + if workspaces_to_update.is_empty() { + return; + } + + // If several workspaces had the same old commit checked out, we want them all + // to have the same commit checked out afterwards as well, so we avoid + // calling MutableRepo::check_out() multiple times, since that might + // otherwise create a separate new commit for each workspace. + // We arbitrarily pick a new checkout among the candidates. + let new_commit_id = new_commit_ids[0].clone(); + let new_commit = self.mut_repo.store().get_commit(&new_commit_id).unwrap(); + let new_checkout_commit = + self.mut_repo + .check_out(workspaces_to_update[0].clone(), self.settings, &new_commit); + for workspace_id in workspaces_to_update.into_iter().skip(1) { + self.mut_repo + .check_out(workspace_id, self.settings, &new_checkout_commit); + } + } + + // TODO: Perhaps change the interface since it's not just about rebasing + // commits. pub fn rebase_next(&mut self) -> Option { while let Some(old_commit_id) = self.to_visit.pop() { if let Some(new_parent_ids) = self.new_parents.get(&old_commit_id).cloned() { diff --git a/lib/tests/test_rewrite.rs b/lib/tests/test_rewrite.rs index 43d087706..17105c5a3 100644 --- a/lib/tests/test_rewrite.rs +++ b/lib/tests/test_rewrite.rs @@ -1226,8 +1226,15 @@ fn test_rebase_descendants_update_checkout_open(use_git: bool) { .set_parents(vec![commit_a.id().clone()]) .set_open(true) .write_to_repo(tx.mut_repo()); + let ws1_id = WorkspaceId::new("ws1".to_string()); + let ws2_id = WorkspaceId::new("ws2".to_string()); + let ws3_id = WorkspaceId::new("ws3".to_string()); tx.mut_repo() - .set_checkout(WorkspaceId::default(), commit_b.id().clone()); + .set_checkout(ws1_id.clone(), commit_b.id().clone()); + tx.mut_repo() + .set_checkout(ws2_id.clone(), commit_b.id().clone()); + tx.mut_repo() + .set_checkout(ws3_id.clone(), commit_a.id().clone()); let repo = tx.commit(); let mut tx = repo.start_transaction("test"); @@ -1237,7 +1244,11 @@ fn test_rebase_descendants_update_checkout_open(use_git: bool) { tx.mut_repo().rebase_descendants(&settings); let repo = tx.commit(); - assert_eq!(repo.view().checkout(), commit_c.id()); + // Workspaces 1 and 2 had B checked out, so they get updated to C. Workspace 3 + // had A checked out, so it doesn't get updated. + assert_eq!(repo.view().get_checkout(&ws1_id), Some(commit_c.id())); + assert_eq!(repo.view().get_checkout(&ws2_id), Some(commit_c.id())); + assert_eq!(repo.view().get_checkout(&ws3_id), Some(commit_a.id())); } #[test_case(false ; "local backend")] @@ -1259,8 +1270,15 @@ fn test_rebase_descendants_update_checkout_closed(use_git: bool) { .set_parents(vec![commit_a.id().clone()]) .set_open(true) .write_to_repo(tx.mut_repo()); + let ws1_id = WorkspaceId::new("ws1".to_string()); + let ws2_id = WorkspaceId::new("ws2".to_string()); + let ws3_id = WorkspaceId::new("ws3".to_string()); tx.mut_repo() - .set_checkout(WorkspaceId::default(), commit_b.id().clone()); + .set_checkout(ws1_id.clone(), commit_b.id().clone()); + tx.mut_repo() + .set_checkout(ws2_id.clone(), commit_b.id().clone()); + tx.mut_repo() + .set_checkout(ws3_id.clone(), commit_a.id().clone()); let repo = tx.commit(); let mut tx = repo.start_transaction("test"); @@ -1271,9 +1289,19 @@ fn test_rebase_descendants_update_checkout_closed(use_git: bool) { tx.mut_repo().rebase_descendants(&settings); let repo = tx.commit(); - let checkout = repo.store().get_commit(repo.view().checkout()).unwrap(); + // Workspaces 1 and 2 had B checked out, so they get updated to the same new + // commit on top of C. Workspace 3 had A checked out, so it doesn't get updated. + assert_eq!( + repo.view().get_checkout(&ws1_id), + repo.view().get_checkout(&ws2_id) + ); + let checkout = repo + .store() + .get_commit(repo.view().get_checkout(&ws1_id).unwrap()) + .unwrap(); assert!(checkout.is_open()); assert_eq!(checkout.parent_ids(), vec![commit_c.id().clone()]); + assert_eq!(repo.view().get_checkout(&ws3_id), Some(commit_a.id())); } #[test_case(false ; "local backend")]