fix: assure Repository::commit_as() also uses the committer for ref…

…logs (#1829)

Previously it would retrieve the configured committer, or trigger an error
if there was none despite the commiter being provided to `commit_as()`.

This als adds `Repository::edit_references_as(committer)` to allow passing
a given committer.

gix/src/repository/config/mod.rs

@@ -12,8 +12,8 @@ impl crate::Repository {
     /// Return a mutable snapshot of the configuration as seen upon opening the repository, starting a transaction.
     /// When the returned instance is dropped, it is applied in full, even if the reason for the drop is an error.
     ///
-    /// Note that changes to the configuration are in-memory only and are observed only the this instance
-    /// of the [`Repository`][crate::Repository].
+    /// Note that changes to the configuration are in-memory only and are observed only this instance
+    /// of the [`Repository`](crate::Repository).
     pub fn config_snapshot_mut(&mut self) -> config::SnapshotMut<'_> {
         let config = self.config.resolved.as_ref().clone();
         config::SnapshotMut {

gix/src/repository/object.rs

@@ -254,7 +254,7 @@ impl crate::Repository {
         self.tag_reference(name, tag_id, constraint).map_err(Into::into)
     }
 
-    /// Similar to [`commit(…)`][crate::Repository::commit()], but allows to create the commit with `committer` and `author` specified.
+    /// Similar to [`commit(…)`](crate::Repository::commit()), but allows to create the commit with `committer` and `author` specified.
     ///
     /// This forces setting the commit time and author time by hand. Note that typically, committer and author are the same.
     pub fn commit_as<'a, 'c, Name, E>(
@@ -307,28 +307,35 @@ impl crate::Repository {
         };
 
         let commit_id = self.write_object(&commit)?;
-        self.edit_reference(RefEdit {
-            change: Change::Update {
-                log: LogChange {
-                    mode: RefLog::AndReference,
-                    force_create_reflog: false,
-                    message: crate::reference::log::message("commit", commit.message.as_ref(), commit.parents.len()),
-                },
-                expected: match commit.parents.first().map(|p| Target::Object(*p)) {
-                    Some(previous) => {
-                        if reference.as_bstr() == "HEAD" {
-                            PreviousValue::MustExistAndMatch(previous)
-                        } else {
-                            PreviousValue::ExistingMustMatch(previous)
+        self.edit_references_as(
+            Some(RefEdit {
+                change: Change::Update {
+                    log: LogChange {
+                        mode: RefLog::AndReference,
+                        force_create_reflog: false,
+                        message: crate::reference::log::message(
+                            "commit",
+                            commit.message.as_ref(),
+                            commit.parents.len(),
+                        ),
+                    },
+                    expected: match commit.parents.first().map(|p| Target::Object(*p)) {
+                        Some(previous) => {
+                            if reference.as_bstr() == "HEAD" {
+                                PreviousValue::MustExistAndMatch(previous)
+                            } else {
+                                PreviousValue::ExistingMustMatch(previous)
+                            }
                         }
-                    }
-                    None => PreviousValue::MustNotExist,
+                        None => PreviousValue::MustNotExist,
+                    },
+                    new: Target::Object(commit_id.inner),
                 },
-                new: Target::Object(commit_id.inner),
-            },
-            name: reference,
-            deref: true,
-        })?;
+                name: reference,
+                deref: true,
+            }),
+            Some(committer),
+        )?;
         Ok(commit_id)
     }
 

gix/src/repository/reference.rs

@@ -10,7 +10,7 @@ use crate::{bstr::BString, ext::ReferenceExt, reference, Reference};
 impl crate::Repository {
     /// Create a lightweight tag with given `name` (and without `refs/tags/` prefix) pointing to the given `target`, and return it as reference.
     ///
-    /// It will be created with `constraint` which is most commonly to [only create it][PreviousValue::MustNotExist]
+    /// It will be created with `constraint` which is most commonly to [only create it](PreviousValue::MustNotExist)
     /// or to [force overwriting a possibly existing tag](PreviousValue::Any).
     pub fn tag_reference(
         &self,
@@ -135,25 +135,37 @@ impl crate::Repository {
 
     /// Edit one or more references as described by their `edits`.
     /// Note that one can set the committer name for use in the ref-log by temporarily
-    /// [overriding the git-config][crate::Repository::config_snapshot_mut()].
+    /// [overriding the git-config](crate::Repository::config_snapshot_mut()), or use
+    /// [`edit_references_as(committer)`](Self::edit_references_as()) for convenience.
     ///
     /// Returns all reference edits, which might be more than where provided due the splitting of symbolic references, and
     /// whose previous (_old_) values are the ones seen on in storage after the reference was locked.
     pub fn edit_references(
         &self,
         edits: impl IntoIterator<Item = RefEdit>,
+    ) -> Result<Vec<RefEdit>, reference::edit::Error> {
+        self.edit_references_as(edits, self.committer().transpose()?)
+    }
+
+    /// A way to apply reference `edits` similar to [edit_references(…)](Self::edit_references()), but set a specific
+    /// `commiter` for use in the reflog. It can be `None` if it's the purpose `edits` are configured to not update the
+    /// reference log, or cause a failure otherwise.
+    pub fn edit_references_as(
+        &self,
+        edits: impl IntoIterator<Item = RefEdit>,
+        committer: Option<gix_actor::SignatureRef<'_>>,
     ) -> Result<Vec<RefEdit>, reference::edit::Error> {
         let (file_lock_fail, packed_refs_lock_fail) = self.config.lock_timeout()?;
         self.refs
             .transaction()
             .prepare(edits, file_lock_fail, packed_refs_lock_fail)?
-            .commit(self.committer().transpose()?)
+            .commit(committer)
             .map_err(Into::into)
     }
 
     /// Return the repository head, an abstraction to help dealing with the `HEAD` reference.
     ///
-    /// The `HEAD` reference can be in various states, for more information, the documentation of [`Head`][crate::Head].
+    /// The `HEAD` reference can be in various states, for more information, the documentation of [`Head`](crate::Head).
     pub fn head(&self) -> Result<crate::Head<'_>, reference::find::existing::Error> {
         let head = self.find_reference("HEAD")?;
         Ok(match head.inner.target {
@@ -184,7 +196,7 @@ impl crate::Repository {
 
     /// Return the name to the symbolic reference `HEAD` points to, or `None` if the head is detached.
     ///
-    /// The difference to [`head_ref()`][Self::head_ref()] is that the latter requires the reference to exist,
+    /// The difference to [`head_ref()`](Self::head_ref()) is that the latter requires the reference to exist,
     /// whereas here we merely return a the name of the possibly unborn reference.
     pub fn head_name(&self) -> Result<Option<FullName>, reference::find::existing::Error> {
         Ok(self.head()?.referent_name().map(std::borrow::ToOwned::to_owned))
@@ -228,7 +240,7 @@ impl crate::Repository {
     /// Find the reference with the given partial or full `name`, like `main`, `HEAD`, `heads/branch` or `origin/other`,
     /// or return an error if it wasn't found.
     ///
-    /// Consider [`try_find_reference(…)`][crate::Repository::try_find_reference()] if the reference might not exist
+    /// Consider [`try_find_reference(…)`](crate::Repository::try_find_reference()) if the reference might not exist
     /// without that being considered an error.
     pub fn find_reference<'a, Name, E>(&self, name: Name) -> Result<Reference<'_>, reference::find::existing::Error>
     where
@@ -249,7 +261,7 @@ impl crate::Repository {
 
     /// Return a platform for iterating references.
     ///
-    /// Common kinds of iteration are [all][crate::reference::iter::Platform::all()] or [prefixed][crate::reference::iter::Platform::prefixed()]
+    /// Common kinds of iteration are [all](crate::reference::iter::Platform::all()) or [prefixed](crate::reference::iter::Platform::prefixed())
     /// references.
     pub fn references(&self) -> Result<reference::iter::Platform<'_>, reference::iter::Error> {
         Ok(reference::iter::Platform {
@@ -261,7 +273,7 @@ impl crate::Repository {
     /// Try to find the reference named `name`, like `main`, `heads/branch`, `HEAD` or `origin/other`, and return it.
     ///
     /// Otherwise return `None` if the reference wasn't found.
-    /// If the reference is expected to exist, use [`find_reference()`][crate::Repository::find_reference()].
+    /// If the reference is expected to exist, use [`find_reference()`](crate::Repository::find_reference()).
     pub fn try_find_reference<'a, Name, E>(&self, name: Name) -> Result<Option<Reference<'_>>, reference::find::Error>
     where
         Name: TryInto<&'a PartialNameRef, Error = E>,

gix/tests/gix/repository/object.rs

@@ -536,16 +536,14 @@ mod tag {
 mod commit_as {
     use gix_testtools::tempfile;
 
-    use crate::util::restricted_and_git;
-
     #[test]
     fn specify_committer_and_author() -> crate::Result {
         let tmp = tempfile::tempdir()?;
         let repo = gix::ThreadSafeRepository::init_opts(
             &tmp,
             gix::create::Kind::WithWorktree,
             Default::default(),
-            restricted_and_git(),
+            gix::open::Options::isolated(),
         )?
         .to_thread_local();
         let empty_tree = repo.empty_tree();