Fix round-tripping commits that contain extra headers (#1389)

* Ignore .t.err files

* Test round tripping commits w/ custom headers

#1377

* Fix handling of commits with custom headers

* Fix proxy version test

* Lean more on existing traits

- Fmt for Oid instead of hex::encode
- Into instead of as_bstr()
- ByteSlice instead of pointer deref and as_bstr()

* Simplify unsigning when rewriting commits

* Remove unnecessary `use`

* Avoid `use`ing gix_object and fully qualify instead

* Replace proxy test with (currently broken) filter test

* Fix filter-based test for custom headers

Avoid navigating `--reverse`'s behaviour by just applying two inverse
filters explicitly.

* Show Rust backtrace on crashes when running tests

* Remove comment that was for my benefit only

* Reword comment

* Add multiline header to test case

josh-core/Cargo.toml

@@ -12,8 +12,10 @@ edition = "2021"
 [dependencies]
 backtrace = "0.3.72"
 bitvec = "1.0.1"
+bstr = "1.9.0"
 git-version = "0.3.9"
 git2 = { workspace = true }
+gix-object = "0.42.2"
 glob = "0.3.1"
 hex = "0.4.3"
 indoc = "2.0.5"

josh-core/src/history.rs

@@ -196,7 +196,7 @@ pub struct RewriteData<'a> {
     pub message: Option<String>,
 }
 
-// takes everything from base except it's tree and replaces it with the tree
+// takes everything from base except its tree and replaces it with the tree
 // given
 pub fn rewrite_commit(
     repo: &git2::Repository,
@@ -205,40 +205,49 @@ pub fn rewrite_commit(
     rewrite_data: RewriteData,
     unsign: bool,
 ) -> JoshResult<git2::Oid> {
-    let message = rewrite_data
-        .message
-        .unwrap_or(base.message_raw().unwrap_or("no message").to_string());
-    let tree = &rewrite_data.tree;
-
-    let a = base.author();
-    let new_a = if let Some((author, email)) = rewrite_data.author {
-        git2::Signature::new(&author, &email, &a.when())?
-    } else {
-        a
-    };
+    let odb = repo.odb()?;
+    let odb_commit = odb.read(base.id())?;
+    assert!(odb_commit.kind() == git2::ObjectType::Commit);
+
+    // gix_object uses byte strings for Oids, but in hex representation, not raw bytes. Its `Format` implementation
+    // writes out hex-encoded bytes. Because CommitRef's reference lifetimes we have to this, before creating CommitRef
+    let tree_id = format!("{}", rewrite_data.tree.id());
+    let parent_ids = parents
+        .iter()
+        .map(|x| format!("{}", x.id()))
+        .collect::<Vec<_>>();
 
-    let c = base.committer();
-    let new_c = if let Some((committer, email)) = rewrite_data.committer {
-        git2::Signature::new(&committer, &email, &c.when())?
-    } else {
-        c
-    };
+    let mut commit = gix_object::CommitRef::from_bytes(odb_commit.data())?;
+
+    commit.tree = tree_id.as_bytes().into();
+
+    commit.parents.clear();
+    commit
+        .parents
+        .extend(parent_ids.iter().map(|x| x.as_bytes().into()));
 
-    let b = repo.commit_create_buffer(&new_a, &new_c, &message, tree, parents)?;
-
-    if let (false, Ok((sig, _))) = (unsign, repo.extract_signature(&base.id(), None)) {
-        // Re-create the object with the original signature (which of course does not match any
-        // more, but this is needed to guarantee perfect round-trips).
-        let b = b
-            .as_str()
-            .ok_or_else(|| josh_error("non-UTF-8 signed commit"))?;
-        let sig = sig
-            .as_str()
-            .ok_or_else(|| josh_error("non-UTF-8 signature"))?;
-        return Ok(repo.commit_signed(b, sig, None)?);
+    if let Some(ref msg) = rewrite_data.message {
+        commit.message = msg.as_bytes().into();
     }
 
-    return Ok(repo.odb()?.write(git2::ObjectType::Commit, &b)?);
+    if let Some((ref author, ref email)) = rewrite_data.author {
+        commit.author.name = author.as_bytes().into();
+        commit.author.email = email.as_bytes().into();
+    }
+
+    if let Some((ref author, ref email)) = rewrite_data.committer {
+        commit.committer.name = author.as_bytes().into();
+        commit.committer.email = email.as_bytes().into();
+    }
+
+    commit
+        .extra_headers
+        .retain(|(k, _)| *k != "gpgsig".as_bytes() || !unsign);
+
+    let mut b = vec![];
+    gix_object::WriteTo::write_to(&commit, &mut b)?;
+
+    return Ok(odb.write(git2::ObjectType::Commit, &b)?);
 }
 
 // Given an OID of an unfiltered commit and a filter,

run-tests.sh

@@ -38,4 +38,5 @@ export GIT_CONFIG_GLOBAL=${CONFIG_FILE}
 git config --global init.defaultBranch master
 
 cargo fmt
+export RUST_BACKTRACE=1
 python3 -m prysk "$@"

tests/.gitignore

@@ -0,0 +1 @@
+*.t.err

tests/filter/roundtrip_custom_header.t

@@ -0,0 +1,132 @@
+  $ export TESTTMP=${PWD}
+
+  $ cd ${TESTTMP}
+  $ git init -q repo 1>/dev/null
+  $ cd repo
+
+  $ echo "hello world" > hw.txt
+  $ git add .
+  $ git commit -m initial
+  [master (root-commit) 7d7c929] initial
+   1 file changed, 1 insertion(+)
+   create mode 100644 hw.txt
+
+  $ mkdir subdir
+  $ echo "hello moon" > subdir/hw.txt
+  $ git add .
+  $ git commit -m second
+  [master bab39f4] second
+   1 file changed, 1 insertion(+)
+   create mode 100644 subdir/hw.txt
+
+  $ git diff ${EMPTY_TREE}..refs/heads/master
+  diff --git a/hw.txt b/hw.txt
+  new file mode 100644
+  index 0000000..3b18e51
+  --- /dev/null
+  +++ b/hw.txt
+  @@ -0,0 +1 @@
+  +hello world
+  diff --git a/subdir/hw.txt b/subdir/hw.txt
+  new file mode 100644
+  index 0000000..1b95c6e
+  --- /dev/null
+  +++ b/subdir/hw.txt
+  @@ -0,0 +1 @@
+  +hello moon
+
+Write a custom header into the commit (h/t https://github.com/Byron/gitoxide/blob/68cbea8gix/tests/fixtures/make_pre_epoch_repo.sh#L12-L27)
+  $ git cat-file -p @ | tee commit.txt
+  tree 15e3a4de9f0b90057746be6658b0f321f4bcc470
+  parent 7d7c9293be5483ccd1a24bdf33ad52cf07cda738
+  author Josh <[email protected]> 1112911993 +0000
+  committer Josh <[email protected]> 1112911993 +0000
+
+  second
+
+  $ patch -p1 <<EOF
+  > diff --git a/commit.txt b/commit.txt
+  > index 1758866..fe1998a 100644
+  > --- a/commit.txt
+  > +++ b/commit.txt
+  > @@ -2,5 +2,9 @@ tree 15e3a4de9f0b90057746be6658b0f321f4bcc470
+  >  parent 7d7c9293be5483ccd1a24bdf33ad52cf07cda738
+  >  author Josh <[email protected]> 1112911993 +0000
+  >  committer Josh <[email protected]> 1112911993 +0000
+  > +custom-header with
+  > + multiline
+  > + value
+  > +another-header such that it sorts before custom-header
+  >  
+  >  second
+  > EOF
+  patching file commit.txt
+  $ new_commit=$(git hash-object --literally -w -t commit commit.txt)
+  $ echo $new_commit
+  f2fd7b23a4a2318d534d122615a6e75196c3e3c4
+  $ git update-ref refs/heads/master $new_commit
+
+  $ josh-filter --update refs/heads/filtered ':prefix=pre'
+
+  $ git diff ${EMPTY_TREE}..refs/heads/filtered
+  diff --git a/pre/hw.txt b/pre/hw.txt
+  new file mode 100644
+  index 0000000..3b18e51
+  --- /dev/null
+  +++ b/pre/hw.txt
+  @@ -0,0 +1 @@
+  +hello world
+  diff --git a/pre/subdir/hw.txt b/pre/subdir/hw.txt
+  new file mode 100644
+  index 0000000..1b95c6e
+  --- /dev/null
+  +++ b/pre/subdir/hw.txt
+  @@ -0,0 +1 @@
+  +hello moon
+
+  $ git cat-file -p refs/heads/filtered
+  tree 6876aad1a2259b9d4c7c24e0e3ff908d3d580404
+  parent 73007fa33b8628d6560b78e37191c07c9e001d3b
+  author Josh <[email protected]> 1112911993 +0000
+  committer Josh <[email protected]> 1112911993 +0000
+  custom-header with
+   multiline
+   value
+  another-header such that it sorts before custom-header
+
+  second
+
+  $ josh-filter --update refs/heads/re-filtered ':/pre' refs/heads/filtered 
+
+  $ git show refs/heads/re-filtered
+  commit f2fd7b23a4a2318d534d122615a6e75196c3e3c4
+  Author: Josh <[email protected]>
+  Date:   Thu Apr 7 22:13:13 2005 +0000
+
+      second
+
+  diff --git a/subdir/hw.txt b/subdir/hw.txt
+  new file mode 100644
+  index 0000000..1b95c6e
+  --- /dev/null
+  +++ b/subdir/hw.txt
+  @@ -0,0 +1 @@
+  +hello moon
+
+  $ git cat-file -p refs/heads/re-filtered
+  tree 15e3a4de9f0b90057746be6658b0f321f4bcc470
+  parent 7d7c9293be5483ccd1a24bdf33ad52cf07cda738
+  author Josh <[email protected]> 1112911993 +0000
+  committer Josh <[email protected]> 1112911993 +0000
+  custom-header with
+   multiline
+   value
+  another-header such that it sorts before custom-header
+
+  second
+
+  $ git log --oneline --all --decorate
+  63982dc (filtered) second
+  f2fd7b2 (HEAD -> master, re-filtered) second
+  73007fa initial
+  7d7c929 initial

tests/proxy/get_version.t

@@ -2,7 +2,7 @@
   $ cd ${TESTTMP}
 
   $ curl -s http://localhost:8002/version
-  Version: r*.*.* (glob)
+  Version: v*.*.* (glob)
 
   $ bash ${TESTDIR}/destroy_test_env.sh
   .