move commit-creation (with signature support) into but-rebase

There it can naturally be used by `but-workspace` and its
commit-creation.

Author: Byron

Cargo.lock

@@ -68,7 +68,7 @@ gitbutler-hunk-dependency = { path = "crates/gitbutler-hunk-dependency" }
 gitbutler-settings = { path = "crates/gitbutler-settings" }
 gitbutler-workspace = { path = "crates/gitbutler-workspace" }
 but-debugging = { path = "crates/but-debugging" }
-but-rebase = { path = "crates/but-debugging" }
+but-rebase = { path = "crates/but-rebase" }
 but-core = { path = "crates/but-core" }
 but-workspace = { path = "crates/but-workspace" }
 but-hunk-dependency = { path = "crates/but-hunk-dependency" }

Cargo.toml

@@ -11,10 +11,14 @@ doctest = false
 [dependencies]
 gix.workspace = true
 anyhow.workspace = true
+tracing.workspace = true
+but-core.workspace = true
 gitbutler-repo.workspace = true
 gitbutler-oxidize.workspace = true
+gitbutler-error.workspace = true
 bstr.workspace = true
 git2.workspace = true
+tempfile.workspace = true
 
 [dev-dependencies]
 gix-testtools.workspace = true

crates/but-rebase/Cargo.toml

@@ -4,48 +4,21 @@ use but_core::cmd::prepare_with_shell_on_windows;
 use but_core::{GitConfigSettings, RepositoryExt};
 use gitbutler_error::error::Code;
 use gix::objs::WriteTo;
-use gix::refs::transaction::{Change, LogChange, PreviousValue, RefLog};
-use gix::refs::Target;
 use std::borrow::Cow;
 use std::io::Write;
 use std::path::Path;
 use std::process::Stdio;
 
-/// Create a commit exactly as specified, and sign it depending on Git and GitButler specific Git configuration.
-#[allow(clippy::too_many_arguments)]
-pub fn create_commit(
-    repo: &gix::Repository,
-    update_ref: Option<gix::refs::FullName>,
-    author: gix::actor::Signature,
-    committer: gix::actor::Signature,
-    message: &str,
-    tree: gix::ObjectId,
-    parents: impl IntoIterator<Item = impl Into<gix::ObjectId>>,
-    commit_headers: Option<but_core::commit::HeadersV2>,
-) -> anyhow::Result<(gix::ObjectId, Option<gix::refs::transaction::RefEdit>)> {
-    let commit = gix::objs::Commit {
-        message: message.into(),
-        tree,
-        author,
-        committer,
-        encoding: None,
-        parents: parents.into_iter().map(Into::into).collect(),
-        extra_headers: commit_headers.unwrap_or_default().into(),
-    };
-    create_given_commit(repo, update_ref, commit)
-}
-
-/// Use the given commit and possibly sign it, replacing a possibly existing signature,
+/// Use the given `commit` and possibly sign it, replacing a possibly existing signature,
 /// or removing the signature if GitButler is not configured to keep it.
 ///
 /// Signatures will be removed automatically if signing is disabled to prevent an amended commit
-/// to use the old signature. They will also be added or replace existing signatures.
+/// to use the old signature.
 #[allow(clippy::too_many_arguments)]
-pub fn create_given_commit(
+pub fn create(
     repo: &gix::Repository,
-    update_ref: Option<gix::refs::FullName>,
     mut commit: gix::objs::Commit,
-) -> anyhow::Result<(gix::ObjectId, Option<gix::refs::transaction::RefEdit>)> {
+) -> anyhow::Result<gix::ObjectId> {
     if let Some(pos) = commit
         .extra_headers()
         .find_pos(gix::objs::commit::SIGNATURE_FIELD_NAME)
@@ -74,44 +47,7 @@ pub fn create_given_commit(
         }
     }
 
-    let new_commit_id = repo.write_object(&commit)?.detach();
-    let refedit = if let Some(refname) = update_ref {
-        // TODO:(ST) should this be something more like what Git does (also in terms of reflog message)?
-        //       Probably should support making a commit in full with `gix`.
-        let log_message = commit.message;
-        let edit = update_reference(repo, refname, new_commit_id, log_message)?;
-        Some(edit)
-    } else {
-        None
-    };
-    Ok((new_commit_id, refedit))
-}
-
-fn update_reference(
-    repo: &gix::Repository,
-    name: gix::refs::FullName,
-    target: gix::ObjectId,
-    log_message: BString,
-) -> anyhow::Result<gix::refs::transaction::RefEdit> {
-    let mut edits = repo.edit_reference(gix::refs::transaction::RefEdit {
-        change: Change::Update {
-            log: LogChange {
-                mode: RefLog::AndReference,
-                force_create_reflog: false,
-                message: log_message,
-            },
-            expected: PreviousValue::Any,
-            new: Target::Object(target),
-        },
-        name,
-        deref: false,
-    })?;
-    debug_assert_eq!(
-        edits.len(),
-        1,
-        "only one reference can be created, splits aren't possible"
-    );
-    Ok(edits.pop().expect("exactly one edit"))
+    Ok(repo.write_object(&commit)?.detach())
 }
 
 fn sign_buffer(repo: &gix::Repository, buffer: &[u8]) -> anyhow::Result<BString> {

crates/but-workspace/src/commit_engine/plumbing.rs crates/but-rebase/src/commit.rs

@@ -6,6 +6,9 @@ use bstr::{BString, ByteSlice};
 use gitbutler_oxidize::{ObjectIdExt, OidExt};
 use gitbutler_repo::rebase::{cherry_rebase_group, merge_commits};
 
+///
+pub mod commit;
+
 /// An instruction for [`RebaseBuilder::rebase()`].
 #[derive(Debug)]
 pub enum RebaseStep {
@@ -171,64 +174,69 @@ fn rebase(
     let git2_repo = git2::Repository::open(repo.path())?;
     let mut references = vec![];
     // Start with the base commit
-    let mut head = base;
+    let (mut cursor, mut last_seen_commit) = (base, base);
     // Running cherry_rebase_group for each step individually
     for step in steps {
         match step {
             RebaseStep::Pick {
                 commit_id,
                 new_message,
             } => {
-                let mut new_head =
+                last_seen_commit = commit_id;
+                let mut new_commit =
                     cherry_rebase_group(&git2_repo, base.to_git2(), &[commit_id.to_git2()], true)?
                         .to_gix();
                 if let Some(new_message) = new_message {
-                    new_head = reword_commit(repo, new_head, new_message.clone())?;
+                    new_commit = reword_commit(repo, new_commit, new_message.clone())?;
                 }
                 // Update the base for the next loop iteration
-                head = new_head;
+                cursor = new_commit;
             }
             RebaseStep::Merge {
                 commit_id,
                 new_message,
             } => {
-                head = merge_commits(repo, head, commit_id, &new_message.to_str_lossy())?;
+                last_seen_commit = commit_id;
+                cursor = merge_commits(repo, cursor, commit_id, &new_message.to_str_lossy())?;
             }
             RebaseStep::Fixup {
                 commit_id,
                 new_message,
             } => {
+                last_seen_commit = commit_id;
                 // This time, the base is the parent of the last commit
-                let base_commit = repo.find_commit(head)?;
+                let base_commit = repo.find_commit(cursor)?;
 
                 // First cherry-pick the target oid on top of base_commit
-                let new_head =
-                    cherry_rebase_group(&git2_repo, head.to_git2(), &[commit_id.to_git2()], true)?
-                        .to_gix();
+                let new_commit = cherry_rebase_group(
+                    &git2_repo,
+                    cursor.to_git2(),
+                    &[commit_id.to_git2()],
+                    true,
+                )?
+                .to_gix();
 
                 // Now, lets pretend the base didn't exist by swapping parent with the parent of the base
-                let commit = repo.find_commit(new_head)?;
+                let commit = repo.find_commit(new_commit)?;
                 let mut new_commit = commit.decode()?.to_owned();
                 new_commit.parents = base_commit.parent_ids().map(|id| id.detach()).collect();
-                // Optionally reword the commit
                 if let Some(new_message) = new_message {
                     new_commit.message = new_message;
                 }
-                let new_head = repo.write_object(new_commit)?.detach();
-                // Update the base for the next loop iteration
-                head = new_head;
+                cursor = commit::create(repo, new_commit)?;
             }
             RebaseStep::Reference { name: refname } => {
                 references.push(ReferenceSpec {
                     refname: refname.clone(),
-                    oid: head,
+                    commit_id: cursor,
+                    previous_commit_id: last_seen_commit,
                 });
             }
         }
     }
 
     Ok(RebaseOutput {
-        new_head: head,
+        top_commit: cursor,
         references,
     })
 }
@@ -240,7 +248,7 @@ fn reword_commit(
 ) -> Result<gix::ObjectId> {
     let mut new_commit = repo.find_commit(oid)?.decode()?.to_owned();
     new_commit.message = new_message;
-    Ok(repo.write_object(new_commit)?.detach())
+    Ok(commit::create(repo, new_commit)?)
 }
 
 /// A reference that is an output of a rebase operation.
@@ -249,13 +257,16 @@ pub struct ReferenceSpec {
     /// A literal reference, useful only to the caller.
     pub refname: BString,
     /// The commit it now points to.
-    pub oid: gix::ObjectId,
+    pub commit_id: gix::ObjectId,
+    /// The commit it previously pointed to (as per pick-list).
+    /// Useful for reference-transactions that validate the current value before changing it to the new one.
+    pub previous_commit_id: gix::ObjectId,
 }
 
 /// The output of the [rebase](RebaseBuilder::rebase()) operation.
 pub struct RebaseOutput {
-    /// The oid of the last commit in the rebase operation, i.e. the new head.
-    pub new_head: gix::ObjectId,
-    /// The list of references that should be created, ordered from the least recent to the most recent.
+    /// The id of the most recently created commit in the rebase operation.
+    pub top_commit: gix::ObjectId,
+    /// The list of references along with their new locations, ordered from the least recent to the most recent.
     pub references: Vec<ReferenceSpec>,
 }

crates/but-rebase/src/lib.rs

@@ -13,26 +13,17 @@ anyhow.workspace = true
 bstr.workspace = true
 git2.workspace = true
 but-core.workspace = true
+but-rebase.workspace = true
 gitbutler-id.workspace = true
-gix = { workspace = true, features = [
-    "dirwalk",
-    "credentials",
-    "parallel",
-    "serde",
-    "status",
-    "revision"
-] }
+gix = { workspace = true, features = [] }
 gitbutler-stack.workspace = true
-gitbutler-error.workspace = true
 gitbutler-command-context.workspace = true
 gitbutler-oxidize.workspace = true
 gitbutler-commit.workspace = true
 gitbutler-repo.workspace = true
 serde = { workspace = true, features = ["std"] }
 gitbutler-serde.workspace = true
 itertools = "0.14"
-tracing.workspace = true
-tempfile.workspace = true
 url = { version = "2.5.4", features = ["serde"] }
 md5 = "0.7.0"
 

crates/but-workspace/Cargo.toml

@@ -15,8 +15,6 @@ use std::io::Read;
 /// Types for use in the frontend with serialization support.
 pub mod ui;
 
-mod plumbing;
-
 /// The place to apply the [change-specifications](DiffSpec) to.
 ///
 /// Note that any commit this instance points to will be the basis to apply all changes to.
@@ -253,8 +251,8 @@ pub fn create_commit(
                 parent_commit_id: _,
             } => {
                 let (author, committer) = repo.commit_signatures()?;
-                let (new_commit, _ref_edit) = plumbing::create_commit(
-                    repo, None, author, committer, &message, new_tree, parents, None,
+                let new_commit = create_possibly_signed_commit(
+                    repo, author, committer, &message, new_tree, parents, None,
                 )?;
                 Some(new_commit)
             }
@@ -266,8 +264,7 @@ pub fn create_commit(
                     .decode()?
                     .to_owned();
                 commit.tree = new_tree;
-                let (new_commit, _ref_edit) = plumbing::create_given_commit(repo, None, commit)?;
-                Some(new_commit)
+                Some(but_rebase::commit::create(repo, commit)?)
             }
         }
     } else {
@@ -491,3 +488,25 @@ fn apply_worktree_changes<'repo>(
 fn has_zero_based_line_numbers(hunk_header: &HunkHeader) -> bool {
     hunk_header.new_start == 0 || hunk_header.old_start == 0
 }
+/// Create a commit exactly as specified, and sign it depending on Git and GitButler specific Git configuration.
+#[allow(clippy::too_many_arguments)]
+fn create_possibly_signed_commit(
+    repo: &gix::Repository,
+    author: gix::actor::Signature,
+    committer: gix::actor::Signature,
+    message: &str,
+    tree: gix::ObjectId,
+    parents: impl IntoIterator<Item = impl Into<gix::ObjectId>>,
+    commit_headers: Option<but_core::commit::HeadersV2>,
+) -> anyhow::Result<gix::ObjectId> {
+    let commit = gix::objs::Commit {
+        message: message.into(),
+        tree,
+        author,
+        committer,
+        encoding: None,
+        parents: parents.into_iter().map(Into::into).collect(),
+        extra_headers: commit_headers.unwrap_or_default().into(),
+    };
+    but_rebase::commit::create(repo, commit)
+}

crates/but-workspace/src/commit_engine/mod.rs

@@ -1,7 +1,7 @@
 use but_core::TreeStatus;
 use but_workspace::commit_engine::{Destination, DiffSpec};
 use gix::prelude::ObjectIdExt;
-use gix_testtools::Creation;
+use gix_testtools::{tempfile, Creation};
 
 pub const CONTEXT_LINES: u32 = 0;
 

crates/but-workspace/tests/workspace/commit_engine/utils.rs

@@ -7,7 +7,7 @@ publish = false
 
 [dependencies]
 serde = { workspace = true, features = ["std"] }
-tracing = "0.1.41"
+tracing.workspace = true
 git2.workspace = true
 anyhow.workspace = true
 toml.workspace = true