Merge pull request #13056 from gitbutlerapp/jt/multcommit

gitbutler-edit-mode: allow commit graph modification

Author: jonathantanmy2

crates/but-rebase/src/graph_rebase/mod.rs

@@ -31,7 +31,7 @@ pub struct Pick {
     ///
     /// If this is Some, the commit WILL NOT be picked onto the parents the
     /// graph implies but instead on to the parents listed here.
-    pub(crate) preserved_parents: Option<Vec<gix::ObjectId>>,
+    pub preserved_parents: Option<Vec<gix::ObjectId>>,
     /// If set to false, a rebase will fail if this commit results in a
     /// conflicted state.
     pub conflictable: bool,

crates/gitbutler-edit-mode/src/lib.rs

@@ -11,7 +11,7 @@ use but_ctx::{
     access::{RepoExclusive, RepoShared},
 };
 use but_oxidize::{ObjectIdExt as _, OidExt, gix_to_git2_index};
-use but_rebase::graph_rebase::{Editor, Step};
+use but_rebase::graph_rebase::{Editor, Pick, Step};
 use git2::build::CheckoutBuilder;
 use gitbutler_cherry_pick::{ConflictedTreeKey, GixRepositoryExt as _, RepositoryExt as _};
 use gitbutler_commit::commit_ext::CommitExt;
@@ -258,9 +258,35 @@ pub(crate) fn save_and_return_to_workspace(ctx: &Context, perm: &mut RepoExclusi
 
     let old_workspace = WorkspaceState::create(ctx, perm.read_permission())?;
 
+    let head_commit = repo.head_commit()?;
+    let decoded_head_commit = head_commit.decode()?;
     // Write out all the changes, including unstaged changes to a tree for re-committing
     #[expect(deprecated)]
     let tree_id = repo.create_wd_tree(0)?;
+    let new_commit_oid = if decoded_head_commit.tree() == tree_id {
+        head_commit.id
+    } else {
+        let commit = gix::objs::Commit::try_from(decoded_head_commit.clone())?;
+        let extra_headers: Vec<(BString, BString)> = Headers::try_from_commit(&commit)
+            .map(|commit_headers| {
+                let headers = Headers {
+                    conflicted: None,
+                    ..commit_headers
+                };
+                (&headers).into()
+            })
+            .unwrap_or_default();
+        but_rebase::commit::create(
+            repo,
+            gix::objs::Commit {
+                tree: tree_id,
+                extra_headers,
+                ..commit
+            },
+            but_rebase::commit::DateMode::CommitterUpdateAuthorKeep,
+            true,
+        )?
+    };
 
     let workspace_commit = repo
         .find_reference(WORKSPACE_BRANCH_REF)?
@@ -274,29 +300,15 @@ pub(crate) fn save_and_return_to_workspace(ctx: &Context, perm: &mut RepoExclusi
     )?
     .into_workspace()?;
     let mut editor = Editor::create(&mut workspace, &mut meta, repo)?;
-    let (target_selector, commit) = editor.find_selectable_commit(edit_mode_metadata.commit_oid)?;
+    let (target_selector, _commit) =
+        editor.find_selectable_commit(edit_mode_metadata.commit_oid)?;
 
-    let extra_headers: Vec<(BString, BString)> = Headers::try_from_commit(&commit.inner)
-        .map(|commit_headers| {
-            let headers = Headers {
-                conflicted: None,
-                ..commit_headers
-            };
-            (&headers).into()
-        })
-        .unwrap_or_default();
-    let new_commit_oid = but_rebase::commit::create(
-        repo,
-        gix::objs::Commit {
-            tree: tree_id,
-            extra_headers,
-            ..commit.inner
-        },
-        but_rebase::commit::DateMode::CommitterUpdateAuthorKeep,
-        true,
-    )?;
+    let mut pick = Pick::new_pick(new_commit_oid);
+    // Do not replace new_commit_oid's parents with the parents of
+    // edit_mode_metadata.commit_oid
+    pick.preserved_parents = Some(decoded_head_commit.parents().collect());
 
-    editor.replace(target_selector, Step::new_pick(new_commit_oid))?;
+    editor.replace(target_selector, Step::Pick(pick))?;
     let outcome = editor.rebase()?;
     // HEAD is EDIT_BRANCH_REF and we do not need to re-checkout it (we
     // are checking out WORKSPACE_BRANCH_REF after this). As for needing to

crates/gitbutler-edit-mode/tests/edit_mode.rs

@@ -1,12 +1,12 @@
 use anyhow::{Context as _, Result, anyhow};
-use bstr::ByteSlice as _;
+use bstr::{BString, ByteSlice as _};
 use but_core::{
     RefMetadata, RepositoryExt,
     ref_metadata::{StackId, WorkspaceCommitRelation, WorkspaceStack, WorkspaceStackBranch},
 };
 use but_ctx::Context;
 use but_meta::{VirtualBranchesTomlMetadata, virtual_branches_legacy_types::Target};
-use but_testsupport::{gix_testtools, open_repo};
+use but_testsupport::{gix_testtools, open_repo, visualize_commit_graph};
 use git2::build::CheckoutBuilder;
 use gitbutler_edit_mode::commands::{
     abort_and_return_to_workspace, enter_edit_mode, save_and_return_to_workspace,
@@ -98,6 +98,115 @@ fn basic_leaving_edit_mode() -> Result<()> {
     Ok(())
 }
 
+#[test]
+fn multiple_commits_created_during_edit_mode() -> Result<()> {
+    let (mut ctx, _tempdir) = command_ctx("conficted_entries_get_written_when_leaving_edit_mode")?;
+    let repo = ctx.repo.get()?;
+
+    let foobar = repo.head_commit()?.decode()?.parents().next().unwrap();
+
+    let worktree_dir = repo.workdir().unwrap().to_owned();
+    drop(repo);
+    let stack_id = stack_id(&ctx)?;
+    enter_edit_mode(&mut ctx, foobar, stack_id)?;
+
+    let repo = ctx.repo.get()?;
+    let commit = gix::objs::Commit::try_from(repo.find_commit(foobar)?.decode()?)?;
+    let first_id = repo.write_object(gix::objs::Commit {
+        message: BString::from(b"first commit added"),
+        parents: [foobar].into(),
+        ..commit.clone()
+    })?;
+    let second_id = repo.write_object(gix::objs::Commit {
+        message: BString::from(b"second commit added"),
+        parents: [first_id.detach()].into(),
+        ..commit
+    })?;
+    repo.edit_reference(gix::refs::transaction::RefEdit {
+        change: gix::refs::transaction::Change::Update {
+            log: gix::refs::transaction::LogChange {
+                mode: gix::refs::transaction::RefLog::AndReference,
+                force_create_reflog: false,
+                message: b"arbitrary message".into(),
+            },
+            expected: gix::refs::transaction::PreviousValue::Any,
+            new: gix::refs::Target::Object(second_id.detach()),
+        },
+        name: "HEAD".try_into().unwrap(),
+        deref: true,
+    })?;
+    drop(repo);
+
+    std::fs::write(worktree_dir.join("file"), "edited during edit mode\n")?;
+
+    save_and_return_to_workspace(&mut ctx)?;
+
+    let repo = &*ctx.repo.get()?;
+    insta::assert_snapshot!(visualize_commit_graph(repo, "refs/heads/gitbutler/workspace")?, @r"
+    * 59ab552 (HEAD -> gitbutler/workspace) GitButler Workspace Commit
+    * f6d3539 (branchy) second commit added
+    * d39dd61 first commit added
+    * 26804c3 foobar
+    * 7950f06 (origin/main, origin/HEAD, main, gitbutler/target) init
+    ");
+    // As usual, any uncommitted changes (to "file" in this test) is applied
+    // onto the HEAD commit at the time of exiting edit mode.
+    let blob = repo.rev_parse_single(b"HEAD^{/second}:file")?.object()?;
+    insta::assert_snapshot!(blob.data.as_bstr(), @"edited during edit mode");
+    // Rebase also happens correctly.
+    let blob = repo.rev_parse_single(b"HEAD:file")?.object()?;
+    insta::assert_snapshot!(blob.data.as_bstr(), @"edited during edit mode");
+
+    Ok(())
+}
+
+#[test]
+fn apply_commit_on_itself() -> Result<()> {
+    let (mut ctx, _tempdir) = command_ctx("conficted_entries_get_written_when_leaving_edit_mode")?;
+    let repo = ctx.repo.get()?;
+
+    let foobar = repo.head_commit()?.decode()?.parents().next().unwrap();
+
+    drop(repo);
+    let stack_id = stack_id(&ctx)?;
+    enter_edit_mode(&mut ctx, foobar, stack_id)?;
+
+    let repo = ctx.repo.get()?;
+    // Set HEAD to gitbutler/workspace, detached, to see what happens when we
+    // try to apply itself on itself.
+    let workspace_commit_id = repo
+        .find_reference("refs/heads/gitbutler/workspace")?
+        .peel_to_commit()?
+        .id;
+    repo.edit_reference(gix::refs::transaction::RefEdit {
+        change: gix::refs::transaction::Change::Update {
+            log: gix::refs::transaction::LogChange {
+                mode: gix::refs::transaction::RefLog::AndReference,
+                force_create_reflog: false,
+                message: b"arbitrary message".into(),
+            },
+            expected: gix::refs::transaction::PreviousValue::Any,
+            new: gix::refs::Target::Object(workspace_commit_id),
+        },
+        name: "HEAD".try_into().unwrap(),
+        deref: true,
+    })?;
+    drop(repo);
+
+    save_and_return_to_workspace(&mut ctx)?;
+
+    let repo = &*ctx.repo.get()?;
+    // It works.
+    insta::assert_snapshot!(visualize_commit_graph(repo, "refs/heads/gitbutler/workspace")?, @r"
+    * 85cd48c (HEAD -> gitbutler/workspace, gitbutler/edit) GitButler Workspace Commit
+    * 6eb9642 (branchy) GitButler Workspace Commit
+    * 26804c3 foobar
+    * 7950f06 (origin/main, origin/HEAD, main, gitbutler/target) init
+    ");
+
+    Ok(())
+}
+
 // Fixture:
 // * xxx (HEAD -> gitbutler/workspace) GitButler Workspace Commit
 // * xxx foobar