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

DescendantRebaser: also update checkout (#32)

Browse source 
This is similar to how a recent change taught `DescendantRebaser` to
update branches pointing to rewritten commits. Now we also update the
checkout if it pointed to a rewritten commit.
This commit is contained in:
Martin von Zweigbergk 2021-10-03 12:01:14 -07:00
parent f33117b2f8
commit fcb79ef0a6
3 changed files with 136 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
lib/src/rewrite.rs
View file

@ -214,8 +214,8 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
fn new_parents(&self, old_ids: &[CommitId]) -> Vec<CommitId> {
let mut new_ids = vec![];
for old_id in old_ids {
if let Some(replacements) = self.new_parents.get(old_id) {
new_ids.extend(replacements.clone());
if let Some(new_parent_ids) = self.new_parents.get(old_id) {
new_ids.extend(new_parent_ids.clone());
} else if let Some(new_parent_id) = self.rebased.get(old_id) {
new_ids.push(new_parent_id.clone());
} else {
@ -239,7 +239,18 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
)
}
fn update_branches(&mut self, old_commit_id: CommitId, new_commit_ids: Vec<CommitId>) {
fn update_branches_and_checkout(
&mut self,
old_commit_id: CommitId,
new_commit_ids: Vec<CommitId>,
) {
if *self.mut_repo.view().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(self.settings, &new_commit);
}
if let Some(branch_names) = self.branches.get(&old_commit_id) {
let view = self.mut_repo.view();
let mut branch_updates = vec![];
@ -285,21 +296,23 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
// (i.e. it's part of the input for this rebase). We don't need
// to rebase it, but we still want to update branches pointing
// to the old commit.
self.update_branches(old_commit_id, new_parent_ids);
self.update_branches_and_checkout(old_commit_id, new_parent_ids);
continue;
}
if let Some(new_commit_ids) = self.divergent.get(&old_commit_id).cloned() {
if let Some(divergent_ids) = self.divergent.get(&old_commit_id).cloned() {
// Leave divergent commits in place. Don't update `new_parents` since we don't
// want to rebase descendants either.
self.update_branches(old_commit_id, new_commit_ids);
self.update_branches_and_checkout(old_commit_id, divergent_ids);
continue;
}
let old_commit = self.mut_repo.store().get_commit(&old_commit_id).unwrap();
let old_parent_ids = old_commit.parent_ids();
let new_parent_ids = self.new_parents(&old_parent_ids);
if self.to_skip.contains(&old_commit_id) {
// Update the `replacements` map so descendants are rebased correctly.
self.new_parents.insert(old_commit_id, new_parent_ids);
// Update the `new_parents` map so descendants are rebased correctly.
self.new_parents
.insert(old_commit_id.clone(), new_parent_ids.clone());
self.update_branches_and_checkout(old_commit_id, new_parent_ids);
continue;
} else if new_parent_ids == old_parent_ids {
// The commit is already in place.
@ -320,7 +333,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
.map(|new_parent_id| self.mut_repo.store().get_commit(new_parent_id).unwrap())
.collect_vec();
let new_commit = rebase_commit(self.settings, self.mut_repo, &old_commit, &new_parents);
self.update_branches(old_commit_id.clone(), vec![new_commit.id().clone()]);
self.update_branches_and_checkout(old_commit_id.clone(), vec![new_commit.id().clone()]);
self.rebased.insert(old_commit_id, new_commit.id().clone());
return Some(RebasedDescendant {
old_commit,

112
lib/tests/test_rewrite.rs
View file

@ -17,7 +17,7 @@ use jujutsu_lib::op_store::RefTarget;
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::rewrite::DescendantRebaser;
use jujutsu_lib::testutils;
use jujutsu_lib::testutils::{assert_rebased, CommitGraphBuilder};
use jujutsu_lib::testutils::{assert_rebased, create_random_commit, CommitGraphBuilder};
use maplit::{hashmap, hashset};
use test_case::test_case;
@ -836,3 +836,113 @@ fn test_rebase_descendants_rewrite_resolves_branch_conflict() {
//
// Now we hide B and make A visible instead. When that diff is applied to the
// branch, the branch state becomes empty and is thus deleted.
#[test_case(false ; "local backend")]
#[test_case(true ; "git backend")]
fn test_rebase_descendants_update_checkout_open(use_git: bool) {
let settings = testutils::user_settings();
let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
// Checked-out, open commit B was replaced by open commit C. C should become
// checked out.
//
// C B
// |/
// A
let mut tx = repo.start_transaction("test");
let commit_a = create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo());
let commit_b = create_random_commit(&settings, &repo)
.set_parents(vec![commit_a.id().clone()])
.set_open(true)
.write_to_repo(tx.mut_repo());
tx.mut_repo().set_checkout(commit_b.id().clone());
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let commit_c = CommitBuilder::for_rewrite_from(&settings, repo.store(), &commit_b)
.set_description("C".to_string())
.write_to_repo(tx.mut_repo());
tx.mut_repo()
.create_descendant_rebaser(&settings)
.rebase_all();
let repo = tx.commit();
assert_eq!(repo.view().checkout(), commit_c.id());
}
#[test_case(false ; "local backend")]
#[test_case(true ; "git backend")]
fn test_rebase_descendants_update_checkout_closed(use_git: bool) {
let settings = testutils::user_settings();
let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
// Checked-out, open commit B was replaced by closed commit C. A child of C
// should become checked out.
//
// C B
// |/
// A
let mut tx = repo.start_transaction("test");
let commit_a = create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo());
let commit_b = create_random_commit(&settings, &repo)
.set_parents(vec![commit_a.id().clone()])
.set_open(true)
.write_to_repo(tx.mut_repo());
tx.mut_repo().set_checkout(commit_b.id().clone());
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let commit_c = CommitBuilder::for_rewrite_from(&settings, repo.store(), &commit_b)
.set_description("C".to_string())
.set_open(false)
.write_to_repo(tx.mut_repo());
tx.mut_repo()
.create_descendant_rebaser(&settings)
.rebase_all();
let repo = tx.commit();
let checkout = repo.store().get_commit(repo.view().checkout()).unwrap();
assert!(checkout.is_open());
assert_eq!(checkout.parent_ids(), vec![commit_c.id().clone()]);
}
#[test_case(false ; "local backend")]
#[test_case(true ; "git backend")]
fn test_rebase_descendants_update_checkout_abandoned_merge(use_git: bool) {
let settings = testutils::user_settings();
let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
// Checked-out, open merge commit D was abandoned. A parent commit should become
// checked out.
//
// D
// |\
// B C
// |/
// A
let mut tx = repo.start_transaction("test");
let commit_a = create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo());
let commit_b = create_random_commit(&settings, &repo)
.set_parents(vec![commit_a.id().clone()])
.write_to_repo(tx.mut_repo());
let commit_c = create_random_commit(&settings, &repo)
.set_parents(vec![commit_a.id().clone()])
.write_to_repo(tx.mut_repo());
let commit_d = create_random_commit(&settings, &repo)
.set_parents(vec![commit_b.id().clone(), commit_c.id().clone()])
.set_open(true)
.write_to_repo(tx.mut_repo());
tx.mut_repo().set_checkout(commit_d.id().clone());
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
tx.mut_repo().record_abandoned_commit(commit_d.id().clone());
tx.mut_repo()
.create_descendant_rebaser(&settings)
.rebase_all();
let repo = tx.commit();
let checkout = repo.store().get_commit(repo.view().checkout()).unwrap();
assert!(checkout.is_open());
assert_eq!(checkout.parent_ids(), vec![commit_b.id().clone()]);
}

51
src/commands.rs
View file

@ -199,12 +199,9 @@ struct RepoCommandHelper {
repo: Arc<ReadonlyRepo>,
may_update_working_copy: bool,
working_copy_committed: bool,
// Whether to evolve orphans when the transaction
// Whether to rebase descendants when the transaction
// finishes. This should generally be true for commands that rewrite commits.
rebase_descendants: bool,
// Whether the checkout should be updated to an appropriate successor when the transaction
// finishes. This should generally be true for commands that rewrite commits.
auto_update_checkout: bool,
}
impl RepoCommandHelper {
@ -222,7 +219,6 @@ impl RepoCommandHelper {
may_update_working_copy,
working_copy_committed: false,
rebase_descendants: true,
auto_update_checkout: true,
})
}
@ -231,11 +227,6 @@ impl RepoCommandHelper {
self
}
fn auto_update_checkout(mut self, value: bool) -> Self {
self.auto_update_checkout = value;
self
}
fn repo(&self) -> &Arc<ReadonlyRepo> {
&self.repo
}
@ -431,9 +422,6 @@ impl RepoCommandHelper {
writeln!(ui, "Rebased {} descendant commits", num_rebased)?;
}
}
if self.auto_update_checkout {
update_checkout_after_rewrite(ui, mut_repo)?;
}
self.repo = tx.commit();
update_working_copy(ui, &self.repo, &self.repo.working_copy_locked())
}
@ -598,36 +586,6 @@ fn rebase_descendants(settings: &UserSettings, mut_repo: &mut MutableRepo) -> i3
num_rebased
}
fn update_checkout_after_rewrite(ui: &mut Ui, mut_repo: &mut MutableRepo) -> io::Result<()> {
// TODO: Perhaps this method should be in MutableRepo.
let new_checkout_candidates = mut_repo
.evolution()
.new_parent(mut_repo.as_repo_ref(), mut_repo.view().checkout());
if new_checkout_candidates.is_empty() {
return Ok(());
}
// Filter out heads that already existed.
// TODO: Filter out *commits* that already existed (so we get updated to an
// appropriate new non-head)
let old_heads = mut_repo.base_repo().view().heads().clone();
let new_checkout_candidates: HashSet<_> = new_checkout_candidates
.difference(&old_heads)
.cloned()
.collect();
if new_checkout_candidates.is_empty() {
return Ok(());
}
if new_checkout_candidates.len() > 1 {
ui.write(
"There are several candidates for updating the checkout to -- picking arbitrarily\n",
)?;
}
let new_checkout = new_checkout_candidates.iter().min().unwrap();
let new_commit = mut_repo.store().get_commit(new_checkout).unwrap();
mut_repo.check_out(ui.settings(), &new_commit);
Ok(())
}
fn get_app<'a, 'b>() -> App<'a, 'b> {
let init_command = SubCommand::with_name("init")
.about("Create a new repo in the given directory")
@ -1384,7 +1342,7 @@ fn cmd_checkout(
command: &CommandHelper,
sub_matches: &ArgMatches,
) -> Result<(), CommandError> {
let mut repo_command = command.repo_helper(ui)?.auto_update_checkout(false);
let mut repo_command = command.repo_helper(ui)?;
let new_commit = repo_command.resolve_revision_arg(ui, sub_matches)?;
repo_command.commit_working_copy(ui)?;
let mut tx =
@ -2652,10 +2610,7 @@ fn cmd_branch(
command: &CommandHelper,
sub_matches: &ArgMatches,
) -> Result<(), CommandError> {
let mut repo_command = command
.repo_helper(ui)?
.auto_update_checkout(false)
.rebase_descendants(false);
let mut repo_command = command.repo_helper(ui)?.rebase_descendants(false);
let branch_name = sub_matches.value_of("name").unwrap();
if sub_matches.is_present("delete") {
let mut tx = repo_command.start_transaction(&format!("delete branch {}", branch_name));