backend: Inline gix::Repository::commit_as to prepare for signing

Additional bonus is that this allows us to avoid creating keep refs for the
intermediate commits in the data race preventing loop.

lib/src/git_backend.rs

@@ -23,8 +23,11 @@ use std::{fs, str};
 
 use async_trait::async_trait;
 use gix::objs::CommitRefIter;
+use gix::refs::Target;
+use gix::refs::transaction::{RefEdit, Change, LogChange, RefLog, PreviousValue};
 use itertools::Itertools;
 use prost::Message;
+use smallvec::SmallVec;
 use thiserror::Error;
 
 use crate::backend::{
@@ -557,6 +560,43 @@ fn prevent_gc(git_repo: &gix::Repository, id: &CommitId) -> Result<(), BackendEr
     Ok(())
 }
 
+/// This is an inlined part of gix::Repository::commit_as_inner (called from
+/// commit_as). It's different from prevent_gc because we update the reflog
+/// correctly as we're (should be) making a real commit here.
+fn prevent_gc_for_full_commit(
+    git_repo: &gix::Repository,
+    git_id: gix::Id,
+    commit: &gix::objs::Commit,
+) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
+    git_repo.edit_reference(RefEdit {
+        change: Change::Update {
+            log: LogChange {
+                mode: RefLog::AndReference,
+                force_create_reflog: false,
+                message: gix::reference::log::message(
+                    "commit",
+                    commit.message.as_ref(),
+                    commit.parents.len(),
+                ),
+            },
+            expected: match commit.parents.first().map(|p| Target::Peeled(*p)) {
+                Some(previous) => {
+                    // if reference == "HEAD" { // no_gc_ref is never HEAD
+                    //     PreviousValue::MustExistAndMatch(previous)
+                    // } else {
+                    PreviousValue::ExistingMustMatch(previous)
+                    // }
+                }
+                None => PreviousValue::MustNotExist,
+            },
+            new: Target::Peeled(git_id.detach()),
+        },
+        name: create_no_gc_ref().try_into()?,
+        deref: true,
+    })?;
+    Ok(())
+}
+
 fn validate_git_object_id(id: &impl ObjectId) -> Result<gix::ObjectId, BackendError> {
     if id.as_bytes().len() != HASH_LENGTH {
         return Err(BackendError::InvalidHashLength {
@@ -941,40 +981,60 @@ impl Backend for GitBackend {
                 parents.push(validate_git_object_id(parent_id)?);
             }
         }
+        let parents: SmallVec<[gix::ObjectId; 1]> = parents.into();
         let extras = serialize_extras(&contents);
+
         // If two writers write commits of the same id with different metadata, they
         // will both succeed and the metadata entries will be "merged" later. Since
         // metadata entry is keyed by the commit id, one of the entries would be lost.
         // To prevent such race condition locally, we extend the scope covered by the
         // table lock. This is still racy if multiple machines are involved and the
         // repository is rsync-ed.
         let (table, table_lock) = self.read_extra_metadata_table_locked()?;
-        let id = loop {
-            let git_id = locked_repo
-                .commit_as(
-                    committer,
-                    author,
-                    create_no_gc_ref(),
-                    message,
-                    git_tree_id,
-                    parents.iter().copied(),
-                )
-                .map_err(|err| BackendError::WriteObject {
-                    object_type: "commit",
-                    source: Box::new(err),
-                })?;
-            let id = CommitId::from_bytes(git_id.as_bytes());
-            match table.get_value(id.as_bytes()) {
+        let (git_id, commit) = loop {
+            let commit = gix::objs::Commit {
+                message: message.to_owned().into(),
+                tree: git_tree_id,
+                author: author.into(),
+                committer: committer.into(),
+                encoding: None,
+                parents: parents.clone(),
+                extra_headers: Default::default(),
+            };
+
+            // todo sign commits here
+
+            let git_id =
+                locked_repo
+                    .write_object(&commit)
+                    .map_err(|err| BackendError::WriteObject {
+                        object_type: "commit",
+                        source: Box::new(err),
+                    })?;
+
+            match table.get_value(git_id.as_bytes()) {
                 Some(existing_extras) if existing_extras != extras => {
                     // It's possible a commit already exists with the same commit id but different
                     // change id. Adjust the timestamp until this is no longer the case.
                     committer.time.seconds -= 1;
                 }
                 _ => {
-                    break id;
+                    break (git_id, commit);
                 }
             }
         };
+
+        // Everything up to this point had no permanent effect on the repo except
+        // GC-able objects
+        prevent_gc_for_full_commit(&locked_repo, git_id, &commit).map_err(|err| {
+            BackendError::WriteObject {
+                object_type: "commit",
+                source: err,
+            }
+        })?;
+
+        let id = CommitId::from_bytes(git_id.as_bytes());
+
         // Update the signature to match the one that was actually written to the object
         // store
         contents.committer.timestamp.timestamp = MillisSinceEpoch(committer.time.seconds * 1000);