From 735d4b89b082f1138a9482c6444364ec72d17830 Mon Sep 17 00:00:00 2001 From: Clayton Carter Date: Sun, 18 Sep 2022 15:21:30 -0400 Subject: [PATCH] temp(move --fixup): Try to get on-disk rebase working --- .../src/core/rewrite/execute.rs | 17 +- git-branchless-lib/src/core/rewrite/plan.rs | 7 +- git-branchless/tests/command/test_move.rs | 173 ++++++++---------- 3 files changed, 95 insertions(+), 102 deletions(-) diff --git a/git-branchless-lib/src/core/rewrite/execute.rs b/git-branchless-lib/src/core/rewrite/execute.rs index 37bc611c8..a377bfe39 100644 --- a/git-branchless-lib/src/core/rewrite/execute.rs +++ b/git-branchless-lib/src/core/rewrite/execute.rs @@ -609,7 +609,9 @@ mod in_memory { Some(MaybeZeroOid::NonZero(rewritten_oid)) => repo .find_commit_or_fail(*rewritten_oid) .wrap_err("Finding commit to apply")?, - Some(MaybeZeroOid::Zero) => todo!("FIXME fixing up deleted commit"), + Some(MaybeZeroOid::Zero) => { + todo!("FIXME fixing up deleted commit: {:#?}", command) + } None => repo .find_commit_or_fail(*commit_to_fixup_oid) .wrap_err("Finding commit to apply")?, @@ -700,6 +702,7 @@ mod in_memory { *commit_to_fixup_oid, MaybeZeroOid::NonZero(rebased_commit_oid), ); + // FIXME can this be removed if we use RebaseCommand::SkipUpstreamBlahBlah? rewritten_oids.insert( *fixup_commit_oid, match head_oid { @@ -1051,7 +1054,17 @@ mod on_disk { ) .wrap_err_with(|| format!("Writing head-name to: {:?}", &head_name_file_path))?; - save_original_head_info(repo, &head_info)?; + // Do not save head info if head is being fixed up. We want to + // figure out which commit to checkout later. + if !rebase_plan.commands.iter().any(|command| match command { + RebaseCommand::FixUp { + commit_to_fixup_oid, + fixup_commit_oid: _, + } => commit_to_fixup_oid == &head_oid, + _ => false, + }) { + save_original_head_info(repo, &head_info)?; + } // Dummy `head` file. We will `reset` to the appropriate commit as soon as // we start the rebase. diff --git a/git-branchless-lib/src/core/rewrite/plan.rs b/git-branchless-lib/src/core/rewrite/plan.rs index f40637f56..d9998d9a6 100644 --- a/git-branchless-lib/src/core/rewrite/plan.rs +++ b/git-branchless-lib/src/core/rewrite/plan.rs @@ -841,7 +841,12 @@ impl<'a> RebasePlanBuilder<'a> { Some(commit_to_fixup_oid) => { acc.push(RebaseCommand::FixUp { commit_to_fixup_oid: *commit_to_fixup_oid, - fixup_commit_oid: commit_oid, // or original_commit_oid? + fixup_commit_oid: commit_oid, + }); + // FIXME is this right? It seems to help for on disk + // rebase w/ fixup, but not for multiple fixups + acc.push(RebaseCommand::SkipUpstreamAppliedCommit { + commit_oid: *commit_to_fixup_oid, }); } None => acc.push(RebaseCommand::Pick { diff --git a/git-branchless/tests/command/test_move.rs b/git-branchless/tests/command/test_move.rs index d6cd04836..08a33c328 100644 --- a/git-branchless/tests/command/test_move.rs +++ b/git-branchless/tests/command/test_move.rs @@ -4575,7 +4575,8 @@ fn test_move_fixup_head_into_parent() -> eyre::Result<()> { ])?; insta::assert_snapshot!(stdout, @r###" Attempting rebase in-memory... - [1/1] Amended as: 68aa706 create test2.txt + [1/2] Amended as: 68aa706 create test2.txt + [2/2] Skipped commit (was already applied upstream): 96d1c37 create test2.txt branchless: processing 2 rewritten commits branchless: running command: checkout 68aa7060ef38274a33c9a51dc6964de6e84fbd43 O f777ecc (master) create initial.txt @@ -4622,29 +4623,20 @@ fn test_move_fixup_parent_into_head() -> eyre::Result<()> { "###); // --on-disk - // FIXME on disk is not working :( - // { - // let git = git.duplicate_repo()?; - // git.run(&[ - // "move", - // "--on-disk", - // "--fixup", - // "--debug-dump-rebase-plan", - // "-x", - // &test2_oid.to_string(), - // "-d", - // &"HEAD".to_string(), - // ])?; - - // let (stdout, _stderr) = git.run(&["smartlog"])?; - // insta::assert_snapshot!(stdout, @r###" - // O f777ecc (master) create initial.txt - // | - // o 62fc20d create test1.txt - // | - // @ 68aa706 create test3.txt - // "###); - // } + { + let git = git.duplicate_repo()?; + let (_stdout, _stderr) = + git.run(&["move", "--on-disk", "--fixup", "-x", &test2_oid.to_string()])?; + + let (stdout, _stderr) = git.run(&["smartlog"])?; + insta::assert_snapshot!(stdout, @r###" + O f777ecc (master) create initial.txt + | + o 62fc20d create test1.txt + | + @ 2565373 create test3.txt + "###); + } // --in-memory { @@ -4660,8 +4652,9 @@ fn test_move_fixup_parent_into_head() -> eyre::Result<()> { // insta::assert_snapshot!(stderr, @r###""###); insta::assert_snapshot!(stdout, @r###" Attempting rebase in-memory... - [1/2] Committed as: 4838e49 create test3.txt - [2/2] Amended as: 2565373 create test3.txt + [1/3] Committed as: 4838e49 create test3.txt + [2/3] Amended as: 2565373 create test3.txt + [3/3] Skipped commit (was already applied upstream): 70deb1e create test3.txt branchless: processing 2 rewritten commits branchless: running command: checkout 2565373efecda4dda6274ad5107dbff6be05f0a3 O f777ecc (master) create initial.txt @@ -4704,29 +4697,25 @@ fn test_move_fixup_multiple_into_ancestor() -> eyre::Result<()> { "###); // --on-disk - // FIXME on disk is not working :( - // { - // let git = git.duplicate_repo()?; - // git.run(&[ - // "move", - // "--on-disk", - // "--fixup", - // "--debug-dump-rebase-plan", - // "-x", - // &test2_oid.to_string(), - // "-d", - // &"HEAD".to_string(), - // ])?; - - // let (stdout, _stderr) = git.run(&["smartlog"])?; - // insta::assert_snapshot!(stdout, @r###" - // O f777ecc (master) create initial.txt - // | - // o 62fc20d create test1.txt - // | - // @ 68aa706 create test3.txt - // "###); - // } + { + let git = git.duplicate_repo()?; + git.run(&[ + "move", + "--on-disk", + "--fixup", + "-x", + "HEAD+HEAD~", + "-d", + &test1_oid.to_string(), + ])?; + + let (stdout, _stderr) = git.run(&["smartlog"])?; + insta::assert_snapshot!(stdout, @r###" + O f777ecc (master) create initial.txt + | + @ 9ede80c create test1.txt + "###); + } // --in-memory { @@ -4785,29 +4774,16 @@ fn test_move_fixup_multiple_ancestors_into_head() -> eyre::Result<()> { "###); // --on-disk - // FIXME on disk is not working :( - // { - // let git = git.duplicate_repo()?; - // git.run(&[ - // "move", - // "--on-disk", - // "--fixup", - // "--debug-dump-rebase-plan", - // "-x", - // &test2_oid.to_string(), - // "-d", - // &"HEAD".to_string(), - // ])?; - - // let (stdout, _stderr) = git.run(&["smartlog"])?; - // insta::assert_snapshot!(stdout, @r###" - // O f777ecc (master) create initial.txt - // | - // o 62fc20d create test1.txt - // | - // @ 68aa706 create test3.txt - // "###); - // } + { + let git = git.duplicate_repo()?; + git.run(&["move", "--on-disk", "--fixup", "-x", "HEAD~+HEAD~2"])?; + let (stdout, _stderr) = git.run(&["smartlog"])?; + insta::assert_snapshot!(stdout, @r###" + O f777ecc (master) create initial.txt + | + @ dbc9c34 create test3.txt + "###); + } // --in-memory { @@ -4873,33 +4849,32 @@ fn test_move_fixup_complicated() -> eyre::Result<()> { "###); // --on-disk - // FIXME on disk is not working :( - // { - // let git = git.duplicate_repo()?; - // git.run(&[ - // "move", - // "--on-disk", - // "--fixup", - // "-x", - // &format!("{}+{}", test2_oid, test6_oid), - // "-d", - // &test4_oid.to_string(), - // ])?; - // let (stdout, _stderr) = git.run(&["smartlog"])?; - // insta::assert_snapshot!(stdout, @r###" - // O f777ecc (master) create initial.txt - // | - // o 62fc20d create test1.txt - // | - // o 4838e49 create test3.txt - // |\ - // | o 9866190 create test4.txt - // | | - // | o a84ae82 create test5.txt - // | - // @ dd38f3d create test7.txt - // "###); - // } + { + let git = git.duplicate_repo()?; + git.run(&[ + "move", + "--on-disk", + "--fixup", + "-x", + &format!("{}+{}", test2_oid, test6_oid), + "-d", + &test4_oid.to_string(), + ])?; + let (stdout, _stderr) = git.run(&["smartlog"])?; + insta::assert_snapshot!(stdout, @r###" + O f777ecc (master) create initial.txt + | + o 62fc20d create test1.txt + | + o 4838e49 create test3.txt + |\ + | o 9866190 create test4.txt + | | + | o a84ae82 create test5.txt + | + @ dd38f3d create test7.txt + "###); + } // --in-memory {