From ca114d6d7e61c5f43fc7907453a6421999d72da5 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Wed, 15 Sep 2021 08:54:55 -0700 Subject: [PATCH] rewrite: add support for rebasing descendants of multiple rewritten commits I plan to use this for rebasing descendants of rewritten remote branches (on fetch). --- Cargo.lock | 1 + Cargo.toml | 1 + lib/src/rewrite.rs | 5 +- lib/tests/test_rewrite.rs | 168 ++++++++++++++++++++++++++++++++++---- src/commands.rs | 11 ++- 5 files changed, 162 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 56b93bf87..73bfecdfa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -531,6 +531,7 @@ dependencies = [ "indoc", "itertools 0.10.1", "jujutsu-lib", + "maplit", "pest", "pest_derive", "protobuf", diff --git a/Cargo.toml b/Cargo.toml index 7da8b7504..c001e6f9d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ hex = "0.4.2" indoc = "1.0.3" itertools = "0.10.1" jujutsu-lib = { version = "=0.2.0", path = "lib"} +maplit = "1.0.2" pest = "2.1.3" pest_derive = "2.1.0" protobuf = { version = "2.22.1", features = ["with-bytes"] } diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index b611bb7e9..711c54e23 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -15,7 +15,6 @@ use std::collections::{HashMap, HashSet}; use itertools::Itertools; -use maplit::hashmap; use crate::backend::CommitId; use crate::commit::Commit; @@ -120,10 +119,8 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { pub fn new( settings: &'settings UserSettings, mut_repo: &'repo mut MutableRepo, - old_parent_id: CommitId, - new_parent_ids: Vec, + replacements: HashMap>, ) -> DescendantRebaser<'settings, 'repo> { - let replacements = hashmap! { old_parent_id => new_parent_ids}; let old_commits_expression = RevsetExpression::commits(replacements.keys().cloned().collect()); let new_commits_expression = diff --git a/lib/tests/test_rewrite.rs b/lib/tests/test_rewrite.rs index b0ae16bc5..f571a9b83 100644 --- a/lib/tests/test_rewrite.rs +++ b/lib/tests/test_rewrite.rs @@ -19,6 +19,7 @@ use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::rewrite::{DescendantRebaser, RebasedDescendant}; use jujutsu_lib::testutils; use jujutsu_lib::testutils::CommitGraphBuilder; +use maplit::hashmap; use test_case::test_case; fn assert_in_place(rebased: Option, expected_old_commit: &Commit) { @@ -83,8 +84,9 @@ fn test_rebase_descendants_sideways(use_git: bool) { let mut rebaser = DescendantRebaser::new( &settings, tx.mut_repo(), - commit2.id().clone(), - vec![commit6.id().clone()], + hashmap! { + commit2.id().clone() => vec![commit6.id().clone()] + }, ); let new_commit3 = assert_rebased(rebaser.rebase_next(), &commit3, &[commit6.id().clone()]); assert_rebased(rebaser.rebase_next(), &commit4, &[new_commit3.id().clone()]); @@ -126,8 +128,10 @@ fn test_rebase_descendants_forward(use_git: bool) { let mut rebaser = DescendantRebaser::new( &settings, tx.mut_repo(), - commit2.id().clone(), - vec![commit6.id().clone()], + hashmap! { + commit2.id().clone() => + vec![commit6.id().clone()] + }, ); assert_rebased(rebaser.rebase_next(), &commit3, &[commit6.id().clone()]); assert_ancestor(rebaser.rebase_next(), &commit4); @@ -162,8 +166,9 @@ fn test_rebase_descendants_backward(use_git: bool) { let mut rebaser = DescendantRebaser::new( &settings, tx.mut_repo(), - commit3.id().clone(), - vec![commit2.id().clone()], + hashmap! { + commit3.id().clone() => vec![commit2.id().clone()] + }, ); assert_rebased(rebaser.rebase_next(), &commit4, &[commit2.id().clone()]); assert!(rebaser.rebase_next().is_none()); @@ -200,8 +205,9 @@ fn test_rebase_descendants_internal_merge(use_git: bool) { let mut rebaser = DescendantRebaser::new( &settings, tx.mut_repo(), - commit2.id().clone(), - vec![commit6.id().clone()], + hashmap! { + commit2.id().clone() => vec![commit6.id().clone()] + }, ); let new_commit3 = assert_rebased(rebaser.rebase_next(), &commit3, &[commit6.id().clone()]); let new_commit4 = assert_rebased(rebaser.rebase_next(), &commit4, &[commit6.id().clone()]); @@ -245,8 +251,9 @@ fn test_rebase_descendants_external_merge(use_git: bool) { let mut rebaser = DescendantRebaser::new( &settings, tx.mut_repo(), - commit3.id().clone(), - vec![commit6.id().clone()], + hashmap! { + commit3.id().clone() => vec![commit6.id().clone()] + }, ); assert_rebased( rebaser.rebase_next(), @@ -283,8 +290,9 @@ fn test_rebase_descendants_degenerate_merge(use_git: bool) { let mut rebaser = DescendantRebaser::new( &settings, tx.mut_repo(), - commit2.id().clone(), - vec![commit1.id().clone()], + hashmap! { + commit2.id().clone() => vec![commit1.id().clone()] + }, ); assert_rebased(rebaser.rebase_next(), &commit4, &[commit3.id().clone()]); assert!(rebaser.rebase_next().is_none()); @@ -321,8 +329,9 @@ fn test_rebase_descendants_widen_merge(use_git: bool) { let mut rebaser = DescendantRebaser::new( &settings, tx.mut_repo(), - commit5.id().clone(), - vec![commit2.id().clone(), commit3.id().clone()], + hashmap! { + commit5.id().clone() => vec![commit2.id().clone(), commit3.id().clone()] + }, ); assert_rebased( rebaser.rebase_next(), @@ -339,6 +348,132 @@ fn test_rebase_descendants_widen_merge(use_git: bool) { tx.discard(); } +#[test_case(false ; "local backend")] +#[test_case(true ; "git backend")] +fn test_rebase_descendants_multiple_sideways(use_git: bool) { + let settings = testutils::user_settings(); + let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); + + // Commit 2 and commit 4 were both replaced by commit 6. Commit 3 and commit 5 + // should get rebased onto it. + // + // 3 5 + // 2 4 6 + // | |/ + // |/ + // 1 + let mut tx = repo.start_transaction("test"); + let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo()); + let commit1 = graph_builder.initial_commit(); + let commit2 = graph_builder.commit_with_parents(&[&commit1]); + let commit3 = graph_builder.commit_with_parents(&[&commit2]); + let commit4 = graph_builder.commit_with_parents(&[&commit1]); + let commit5 = graph_builder.commit_with_parents(&[&commit4]); + let commit6 = graph_builder.commit_with_parents(&[&commit1]); + + let mut rebaser = DescendantRebaser::new( + &settings, + tx.mut_repo(), + hashmap! { + commit2.id().clone() => vec![commit6.id().clone()], + commit4.id().clone() => vec![commit6.id().clone()], + }, + ); + assert_rebased(rebaser.rebase_next(), &commit3, &[commit6.id().clone()]); + assert_rebased(rebaser.rebase_next(), &commit5, &[commit6.id().clone()]); + assert!(rebaser.rebase_next().is_none()); + assert_eq!(rebaser.rebased().len(), 2); + + tx.discard(); +} + +#[test_case(false ; "local backend")] +#[test_case(true ; "git backend")] +fn test_rebase_descendants_multiple_swap(use_git: bool) { + let settings = testutils::user_settings(); + let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); + + // Commit 2 was replaced by commit 4 and commit 4 was replaced by commit 2. + // Commit 3 and commit 5 should swap places. + // + // 3 5 + // 2 4 + // |/ + // 1 + let mut tx = repo.start_transaction("test"); + let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo()); + let commit1 = graph_builder.initial_commit(); + let commit2 = graph_builder.commit_with_parents(&[&commit1]); + let commit3 = graph_builder.commit_with_parents(&[&commit2]); + let commit4 = graph_builder.commit_with_parents(&[&commit1]); + let commit5 = graph_builder.commit_with_parents(&[&commit4]); + + let mut rebaser = DescendantRebaser::new( + &settings, + tx.mut_repo(), + hashmap! { + commit2.id().clone() => vec![commit4.id().clone()], + commit4.id().clone() => vec![commit2.id().clone()], + }, + ); + assert_rebased(rebaser.rebase_next(), &commit3, &[commit4.id().clone()]); + assert_rebased(rebaser.rebase_next(), &commit5, &[commit2.id().clone()]); + assert!(rebaser.rebase_next().is_none()); + assert_eq!(rebaser.rebased().len(), 2); + + tx.discard(); +} + +#[test_case(false ; "local backend")] +#[test_case(true ; "git backend")] +fn test_rebase_descendants_multiple_forward_and_backward(use_git: bool) { + let settings = testutils::user_settings(); + let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); + + // Commit 2 was replaced by commit 4 and commit 6 was replaced by commit 3. + // Commit 7 should be rebased onto commit 3. Commit 8 should be rebased onto + // commit 4. Commits 3-4 should be left alone since they're ancestors of 4. + // Commit 5 should be left alone since its already in place (as a descendant of + // 4). + // + // 7 + // 6 + // 5 + // 4 + // 3 8 + // |/ + // 2 + // 1 + let mut tx = repo.start_transaction("test"); + let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo()); + let commit1 = graph_builder.initial_commit(); + let commit2 = graph_builder.commit_with_parents(&[&commit1]); + let commit3 = graph_builder.commit_with_parents(&[&commit2]); + let commit4 = graph_builder.commit_with_parents(&[&commit3]); + let commit5 = graph_builder.commit_with_parents(&[&commit4]); + let commit6 = graph_builder.commit_with_parents(&[&commit5]); + let commit7 = graph_builder.commit_with_parents(&[&commit6]); + let commit8 = graph_builder.commit_with_parents(&[&commit2]); + + let mut rebaser = DescendantRebaser::new( + &settings, + tx.mut_repo(), + hashmap! { + commit2.id().clone() => vec![commit4.id().clone()], + commit6.id().clone() => vec![commit3.id().clone()], + }, + ); + assert_ancestor(rebaser.rebase_next(), &commit3); + assert_ancestor(rebaser.rebase_next(), &commit4); + assert_in_place(rebaser.rebase_next(), &commit5); + assert_rebased(rebaser.rebase_next(), &commit7, &[commit3.id().clone()]); + assert_rebased(rebaser.rebase_next(), &commit8, &[commit4.id().clone()]); + assert!(rebaser.rebase_next().is_none()); + assert_eq!(rebaser.rebased().len(), 2); + + tx.discard(); +} + #[test_case(false ; "local backend")] #[test_case(true ; "git backend")] fn test_rebase_descendants_contents(use_git: bool) { @@ -377,8 +512,9 @@ fn test_rebase_descendants_contents(use_git: bool) { let mut rebaser = DescendantRebaser::new( &settings, tx.mut_repo(), - commit2.id().clone(), - vec![commit4.id().clone()], + hashmap! { + commit2.id().clone() => vec![commit4.id().clone()] + }, ); rebaser.rebase_all(); let rebased = rebaser.rebased(); diff --git a/src/commands.rs b/src/commands.rs index f946b51bf..687ca6355 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -59,6 +59,7 @@ use jujutsu_lib::transaction::Transaction; use jujutsu_lib::tree::{Diff, DiffSummary}; use jujutsu_lib::working_copy::{CheckoutStats, WorkingCopy}; use jujutsu_lib::{conflicts, files, git, revset}; +use maplit::hashmap; use pest::Parser; use self::chrono::{FixedOffset, TimeZone, Utc}; @@ -2656,8 +2657,9 @@ fn cmd_rebase( let mut rebaser = DescendantRebaser::new( ui.settings(), tx.mut_repo(), - old_commit.id().clone(), - vec![new_commit.id().clone()], + hashmap! { + old_commit.id().clone() => vec![new_commit.id().clone()] + }, ); rebaser.rebase_all(); let num_rebased = rebaser.rebased().len() + 1; @@ -2673,8 +2675,9 @@ fn cmd_rebase( let mut rebaser = DescendantRebaser::new( ui.settings(), tx.mut_repo(), - old_commit.id().clone(), - old_commit.parent_ids(), + hashmap! { + old_commit.id().clone() => old_commit.parent_ids() + }, ); rebaser.rebase_all(); let num_rebased = rebaser.rebased().len();