feat(split): add --detach flag

Author: claytonrcarter

git-branchless-opts/src/lib.rs

@@ -662,6 +662,10 @@ pub enum Command {
         #[clap(value_parser, required = true)]
         files: Vec<String>,
 
+        /// Restack any descendents onto the split commit, not the extracted commit.
+        #[clap(action, short = 'd', long = "detach")]
+        detach: bool,
+
         /// Options for resolving revset expressions.
         #[clap(flatten)]
         resolve_revset_options: ResolveRevsetOptions,

git-branchless/src/commands/mod.rs

@@ -181,18 +181,27 @@ fn command_main(ctx: CommandContext, opts: Opts) -> EyreExitOr<()> {
         },
 
         Command::Split {
+            detach,
             files,
             resolve_revset_options,
             revset,
             move_options,
-        } => split::split(
-            &effects,
-            revset,
-            &resolve_revset_options,
-            files,
-            &move_options,
-            &git_run_info,
-        )?,
+        } => {
+            let split_mode = match detach {
+                true => split::SplitMode::DetachAfter,
+                false => split::SplitMode::InsertAfter,
+            };
+
+            split::split(
+                &effects,
+                revset,
+                &resolve_revset_options,
+                files,
+                split_mode,
+                &move_options,
+                &git_run_info,
+            )?
+        }
 
         Command::Submit(args) => git_branchless_submit::command_main(ctx, args)?,
 

git-branchless/src/commands/split.rs

@@ -34,13 +34,21 @@ use lib::{
 };
 use tracing::instrument;
 
+#[derive(Debug, PartialEq)]
+/// What should `split` do with the extracted changes?
+pub enum SplitMode {
+    DetachAfter,
+    InsertAfter,
+}
+
 /// Split a commit and restack its descendants.
 #[instrument]
 pub fn split(
     effects: &Effects,
     revset: Revset,
     resolve_revset_options: &ResolveRevsetOptions,
     files_to_extract: Vec<String>,
+    split_mode: SplitMode,
     move_options: &MoveOptions,
     git_run_info: &GitRunInfo,
 ) -> EyreExitOr<()> {
@@ -117,15 +125,19 @@ pub fn split(
     let target_commit = repo.find_commit_or_fail(target_oid)?;
     let target_tree = target_commit.get_tree()?;
     let parent_commits = target_commit.get_parents();
-    let (parent_tree, mut remainder_tree) = match parent_commits.as_slice() {
+    let (parent_tree, mut remainder_tree) = match (&split_mode, parent_commits.as_slice()) {
         // split the commit by removing the changes from the target, and then
         // cherry picking the orignal target as the "extracted" commit
-        [only_parent] => (only_parent.get_tree()?, target_commit.get_tree()?),
+        (SplitMode::InsertAfter, [only_parent]) | (SplitMode::DetachAfter, [only_parent]) => {
+            (only_parent.get_tree()?, target_commit.get_tree()?)
+        }
 
         // no parent: use an empty tree for comparison
-        [] => (make_empty_tree(&repo)?, target_commit.get_tree()?),
+        (SplitMode::InsertAfter, []) | (SplitMode::DetachAfter, []) => {
+            (make_empty_tree(&repo)?, target_commit.get_tree()?)
+        }
 
-        [..] => {
+        (_, [..]) => {
             writeln!(
                 effects.get_error_stream(),
                 "Cannot split merge commit {}.",
@@ -204,14 +216,18 @@ pub fn split(
         };
 
         let target_entry = target_tree.get_path(path)?;
-        let temp_tree_oid = match (parent_entry, target_entry) {
+        let temp_tree_oid = match (parent_entry, target_entry, &split_mode) {
             // added => remove from remainder commit
-            (None, Some(_)) => remainder_tree.remove(&repo, path)?,
+            (None, Some(_), SplitMode::InsertAfter) | (None, Some(_), SplitMode::DetachAfter) => {
+                remainder_tree.remove(&repo, path)?
+            }
 
             // deleted or modified => replace w/ parent content in split commit
-            (Some(parent_entry), _) => remainder_tree.add_or_replace(&repo, path, &parent_entry)?,
+            (Some(parent_entry), _, _) => {
+                remainder_tree.add_or_replace(&repo, path, &parent_entry)?
+            }
 
-            (None, _) => {
+            (None, _, _) => {
                 if path.exists() {
                     writeln!(
                         effects.get_error_stream(),
@@ -263,35 +279,37 @@ pub fn split(
         new_commit_oid: MaybeZeroOid::NonZero(remainder_commit_oid),
     }])?;
 
-    let extracted_commit_oid = {
-        let extracted_tree = repo.cherry_pick_fast(
-            &target_commit,
-            &remainder_commit,
-            &CherryPickFastOptions {
-                reuse_parent_tree_if_possible: true,
-            },
-        )?;
-        let extracted_commit_oid = repo.create_commit(
-            None,
-            &target_commit.get_author(),
-            &target_commit.get_committer(),
-            format!("temp(split): {message}").as_str(),
-            &extracted_tree,
-            vec![&remainder_commit],
-        )?;
+    let extracted_commit_oid = match split_mode {
+        SplitMode::InsertAfter | SplitMode::DetachAfter => {
+            let extracted_tree = repo.cherry_pick_fast(
+                &target_commit,
+                &remainder_commit,
+                &CherryPickFastOptions {
+                    reuse_parent_tree_if_possible: true,
+                },
+            )?;
+            let extracted_commit_oid = repo.create_commit(
+                None,
+                &target_commit.get_author(),
+                &target_commit.get_committer(),
+                format!("temp(split): {message}").as_str(),
+                &extracted_tree,
+                vec![&remainder_commit],
+            )?;
 
-        // see git-branchless/src/commands/amend.rs:172
-        // TODO maybe this should happen after we've confirmed the rebase has succeeded
-        mark_commit_reachable(&repo, extracted_commit_oid)
-            .wrap_err("Marking commit as reachable for GC purposes.")?;
+            // see git-branchless/src/commands/amend.rs:172
+            // TODO maybe this should happen after we've confirmed the rebase has succeeded?
+            mark_commit_reachable(&repo, extracted_commit_oid)
+                .wrap_err("Marking commit as reachable for GC purposes.")?;
 
-        event_log_db.add_events(vec![Event::CommitEvent {
-            timestamp: now.duration_since(UNIX_EPOCH)?.as_secs_f64(),
-            event_tx_id,
-            commit_oid: extracted_commit_oid,
-        }])?;
+            event_log_db.add_events(vec![Event::CommitEvent {
+                timestamp: now.duration_since(UNIX_EPOCH)?.as_secs_f64(),
+                event_tx_id,
+                commit_oid: extracted_commit_oid,
+            }])?;
 
-        Some(extracted_commit_oid)
+            Some(extracted_commit_oid)
+        }
     };
 
     // push the new commits into the dag for the rebase planner
@@ -338,23 +356,25 @@ pub fn split(
         rewritten_oids: Vec<(NonZeroOid, MaybeZeroOid)>,
     }
 
-    let cleanup = match (target_state, extracted_commit_oid) {
+    let cleanup = match (target_state, &split_mode, extracted_commit_oid) {
         // branch @ split commit checked out: extend branch to include extracted
         // commit; branch will stay checked out w/o any explicit checkout
-        (TargetState::CurrentBranch, Some(extracted_commit_oid)) => CleanUp {
-            checkout_target: None,
-            rewritten_oids: vec![(target_oid, MaybeZeroOid::NonZero(extracted_commit_oid))],
-        },
+        (TargetState::CurrentBranch, SplitMode::InsertAfter, Some(extracted_commit_oid)) => {
+            CleanUp {
+                checkout_target: None,
+                rewritten_oids: vec![(target_oid, MaybeZeroOid::NonZero(extracted_commit_oid))],
+            }
+        }
 
         // commit to split checked out as detached HEAD, don't extend any
         // branches, but explicitly check out the newly split commit
-        (TargetState::DetachedHead, _) => CleanUp {
+        (TargetState::DetachedHead, _, _) => CleanUp {
             checkout_target: Some(CheckoutTarget::Oid(remainder_commit_oid)),
             rewritten_oids: vec![(target_oid, MaybeZeroOid::NonZero(remainder_commit_oid))],
         },
 
         // some other commit or branch was checked out, default behavior is fine
-        (TargetState::CurrentBranch, _) | (TargetState::Other, _) => CleanUp {
+        (TargetState::CurrentBranch, _, _) | (TargetState::Other, _, _) => CleanUp {
             checkout_target: None,
             rewritten_oids: vec![(target_oid, MaybeZeroOid::NonZero(remainder_commit_oid))],
         },
@@ -393,9 +413,18 @@ pub fn split(
     let mut builder = RebasePlanBuilder::new(&dag, permissions);
     let children = dag.query_children(CommitSet::from(target_oid))?;
     for child in dag.commit_set_to_vec(&children)? {
-        match extracted_commit_oid {
-            None => builder.move_subtree(child, vec![remainder_commit_oid])?,
-            Some(extracted_commit_oid) => {
+        match (&split_mode, extracted_commit_oid) {
+            (_, None) => builder.move_subtree(child, vec![remainder_commit_oid])?,
+            (_, Some(extracted_commit_oid)) => {
+                builder.move_subtree(child, vec![extracted_commit_oid])?
+            }
+        }
+
+        match (&split_mode, extracted_commit_oid) {
+            (_, None) | (SplitMode::DetachAfter, Some(_)) => {
+                builder.move_subtree(child, vec![remainder_commit_oid])?
+            }
+            (_, Some(extracted_commit_oid)) => {
                 builder.move_subtree(child, vec![extracted_commit_oid])?
             }
         }

git-branchless/tests/test_split.rs

@@ -439,6 +439,69 @@ fn test_split_restacks_descendents() -> eyre::Result<()> {
     Ok(())
 }
 
+#[test]
+fn test_split_detach() -> eyre::Result<()> {
+    let git = make_git()?;
+    git.init_repo()?;
+    git.detach_head()?;
+
+    git.write_file_txt("test1", "contents1")?;
+    git.write_file_txt("test2", "contents2")?;
+    git.write_file_txt("test3", "contents3")?;
+    git.run(&["add", "."])?;
+    git.run(&["commit", "-m", "first commit"])?;
+
+    git.commit_file("test3", 1)?;
+
+    {
+        let (stdout, _stderr) = git.branchless("smartlog", &[])?;
+        insta::assert_snapshot!(stdout, @r###"
+        O f777ecc (master) create initial.txt
+        |
+        o e48cdc5 first commit
+        |
+        @ 3d220e0 create test3.txt
+        "###);
+    }
+
+    {
+        let (stdout, _stderr) = git.branchless("split", &["HEAD~", "test2.txt", "--detach"])?;
+        insta::assert_snapshot!(&stdout, @r###"
+            Attempting rebase in-memory...
+            [1/1] Committed as: f88fbe5 create test3.txt
+            branchless: processing 1 rewritten commit
+            branchless: running command: <git-executable> checkout f88fbe5901493ffe1c669cdb8aa5f056dc0bb605
+            In-memory rebase succeeded.
+            O f777ecc (master) create initial.txt
+            |
+            o 2932db7 first commit
+            |\
+            | o 01523cc temp(split): test2.txt (+1)
+            |
+            @ f88fbe5 create test3.txt
+        "###);
+    }
+
+    {
+        let (stdout, _stderr) = git.run(&["show", "--pretty=format:", "--stat", "HEAD~"])?;
+        insta::assert_snapshot!(&stdout, @"
+            test1.txt | 1 +
+            test3.txt | 1 +
+            2 files changed, 2 insertions(+)
+        ");
+
+        let (split_commit, _stderr) = git.run(&["query", "--raw", "exactly(siblings(HEAD), 1)"])?;
+        let (stdout, _stderr) =
+            git.run(&["show", "--pretty=format:", "--stat", split_commit.trim()])?;
+        insta::assert_snapshot!(&stdout, @"
+            test2.txt | 1 +
+            1 file changed, 1 insertion(+)
+        ");
+    }
+
+    Ok(())
+}
+
 #[test]
 fn test_split_undo_works() -> eyre::Result<()> {
     let git = make_git()?;