From 2a6d0b600736b761136d77e6b4c5367c168b8b6a Mon Sep 17 00:00:00 2001
From: Jun Wu <quark@meta.com>
Date: Thu, 24 Oct 2024 10:50:22 -0700
Subject: [PATCH] format-util: drop uninteresting extras when writing git
 commit

Summary:
Drop redundant or noisy fields so we match the typical Git commit expectation
that in most cases there are no extras.

This avoids writing `author_date`, `rebase_source` out that changes some commit
hashes in tests so some broken tests get fixed.

Reviewed By: muirdm

Differential Revision: D64909107

fbshipit-source-id: b21d2631219c23bb4cf9126aa225abded4f09b91
---
 eden/scm/lib/util/format-util/Cargo.toml      |  1 +
 eden/scm/lib/util/format-util/TARGETS         |  1 +
 .../util/format-util/src/git_commit_fields.rs | 38 +++++++++++++++++--
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/eden/scm/lib/util/format-util/Cargo.toml b/eden/scm/lib/util/format-util/Cargo.toml
index a8a6a06573d7f..3265a3238f74c 100644
--- a/eden/scm/lib/util/format-util/Cargo.toml
+++ b/eden/scm/lib/util/format-util/Cargo.toml
@@ -13,4 +13,5 @@ once_cell = "1.12"
 serde = { version = "1.0.185", features = ["derive", "rc"] }
 sha1 = "0.10.5"
 storemodel = { version = "0.1.0", path = "../../storemodel" }
+tracing = { version = "0.1.40", features = ["attributes", "valuable"] }
 types = { version = "0.1.0", path = "../../types" }
diff --git a/eden/scm/lib/util/format-util/TARGETS b/eden/scm/lib/util/format-util/TARGETS
index 108def83d877a..ba5e40cdb0427 100644
--- a/eden/scm/lib/util/format-util/TARGETS
+++ b/eden/scm/lib/util/format-util/TARGETS
@@ -12,6 +12,7 @@ rust_library(
         "fbsource//third-party/rust:once_cell",
         "fbsource//third-party/rust:serde",
         "fbsource//third-party/rust:sha1",
+        "fbsource//third-party/rust:tracing",
         "//eden/scm/lib/minibytes:minibytes",
         "//eden/scm/lib/storemodel:storemodel",
         "//eden/scm/lib/types:types",
diff --git a/eden/scm/lib/util/format-util/src/git_commit_fields.rs b/eden/scm/lib/util/format-util/src/git_commit_fields.rs
index a4b48e3baec55..7da504b9fe046 100644
--- a/eden/scm/lib/util/format-util/src/git_commit_fields.rs
+++ b/eden/scm/lib/util/format-util/src/git_commit_fields.rs
@@ -265,15 +265,38 @@ fn write_message(message: &str, out: &mut String) -> Result<()> {
     Ok(())
 }
 
+// redundant with actual fields: author*, committer*
+// not interesting for git: *source, mut*, branch
+const SORTED_IGNORED_EXTRA_NAMES: &[&str] = &[
+    "amend_source",
+    "author",
+    "author_date",
+    "branch",
+    "committer",
+    "committer_date",
+    "histedit_source",
+    "intermediate-source",
+    "mutdate",
+    "mutop",
+    "mutpred",
+    "mutsplit",
+    "mutuser",
+    "rebase_source",
+    "source",
+    "transplant_source",
+];
+
 fn write_extra(name: &str, value: &str, out: &mut String) -> Result<()> {
-    if name == "committer" || name == "committer_date" || name == "branch" {
-        // "committer" was written before; "branch" is useless - just skip it.
+    if SORTED_IGNORED_EXTRA_NAMES.binary_search(&name).is_ok() {
+        tracing::trace!(name, value, "ignored commit extra field");
         return Ok(());
     }
-    let bad_extra_names = ["author", "parent", "tree"];
+
+    let bad_extra_names = ["parent", "tree"];
     ensure!(
         !name.contains("\n") && !name.contains(" ") && bad_extra_names.iter().all(|&n| n != name),
-        "invalid extra name"
+        "invalid extra: {:?}",
+        name
     );
     out.push_str(name);
     let empty = write_multi_line(value, " ", out)?;
@@ -424,4 +447,11 @@ m
         // Test root_tree() after "fields" being calculated.
         assert!(fields.root_tree().unwrap().is_null());
     }
+
+    #[test]
+    fn test_sorted_ignored_extras_are_actually_sorted() {
+        let mut v = SORTED_IGNORED_EXTRA_NAMES.to_vec();
+        v.sort_unstable();
+        assert_eq!(v, SORTED_IGNORED_EXTRA_NAMES);
+    }
 }