[PATCH] headers: Preserve 'change-id' header in rebase / cherry-pick.

[Matt Stark <msta@google.com>]

In the discussions on
https://lore.kernel.org/git/Z_OGMb-1oV0Ex05e@pks.im/T/#m038be849b9b4020c16c562d810cf77bad91a2c87,
it seems to be that:
* There is consensus that a `change-id` header provides good value
* There is not consenus on what precise format that should take

This commit, rather than attempting to standardize the format, simply
preserves the change-id header in whatever format it used previously.

If we so choose, we can later decide on a standardized format, but since
git only preserves existing headers, this should not create backwards
incompatibility.

Signed-off-by: Matt Stark <msta@google.com>
---
 sequencer.c                           | 39 ++++++++++++++++++++++-----
 t/t3400-rebase.sh                     | 20 ++++++++++++++
 t/t3501-revert-cherry-pick.sh         | 15 +++++++++++
 t/t7501-commit-basic-functionality.sh | 15 +++++++++++
 4 files changed, 83 insertions(+), 6 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index b7d8dca47f..093d47d42a 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1530,12 +1530,12 @@ static int try_to_commit(struct repository *r,
  struct strbuf *msg, const char *author,
  const char *reflog_action,
  struct replay_opts *opts, unsigned int flags,
- struct object_id *oid)
+ struct object_id *oid,
+ struct commit_extra_header *extra)
 {
  struct object_id tree;
  struct commit *current_head = NULL;
  struct commit_list *parents = NULL;
- struct commit_extra_header *extra = NULL;
  struct strbuf err = STRBUF_INIT;
  struct strbuf commit_msg = STRBUF_INIT;
  char *amend_author = NULL;
@@ -1721,7 +1721,8 @@ static int do_commit(struct repository *r,
       const char *msg_file, const char *author,
       const char *reflog_action,
       struct replay_opts *opts, unsigned int flags,
-      struct object_id *oid)
+      struct object_id *oid,
+      struct commit_extra_header *extra_headers)
 {
  int res = 1;

@@ -1735,7 +1736,7 @@ static int do_commit(struct repository *r,
     msg_file);

  res = try_to_commit(r, msg_file ? &sb : NULL,
-     author, reflog_action, opts, flags, &oid);
+     author, reflog_action, opts, flags, &oid, extra_headers);
  strbuf_release(&sb);
  if (!res) {
  refs_delete_ref(get_main_ref_store(r), "",
@@ -2511,10 +2512,36 @@ static int do_pick_commit(struct repository *r,
  oid_to_hex(&commit->object.oid), msg.subject);
  } /* else allow == 0 and there's nothing special to do */
  if (!opts->no_commit && !drop_commit) {
- if (author || command == TODO_REVERT || (flags & AMEND_MSG))
+ if (author || command == TODO_REVERT || (flags & AMEND_MSG)) {
+ struct commit_extra_header *extra_headers = NULL;
+ if (commit) {
+ unsigned long size;
+ const char *buffer = repo_get_commit_buffer(r, commit, &size);
+ size_t out_len;
+ // The Gerrit, GitButler, and Jujutsu projects all have a concept of
+ // a "change id", and it behaves in a similar way between the three
+ // tools. The change id is conceptually associated with a commit.
+ // It follows a commit as its rewritten (e.g. by amending and
+ // rebasing).
+ // While git doesn't add this header itself, and currently has no plans
+ // to do so, there is consensus that if the header is added by another
+ // tool, git should at least preserve it.
+ const char *header_value = find_commit_header(buffer, "change-id", &out_len);
+ if (header_value) {
+ extra_headers = xmalloc(sizeof(*extra_headers));
+ *extra_headers = (struct commit_extra_header){
+ .next = NULL,
+ .key = xstrdup("change-id"),
+ .value = xmemdupz(header_value, out_len),
+ .len = out_len
+ };
+ }
+ repo_unuse_commit_buffer(r, commit, buffer);
+ }
  res = do_commit(r, msg_file, author, reflog_action,
  opts, flags,
- commit? &commit->object.oid : NULL);
+ commit ? &commit->object.oid : NULL, extra_headers);
+ }
  else
  res = error(_("unable to parse commit author"));
  *check_todo = !!(flags & EDIT_MSG);
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index c0c00fbb7b..6b5d6fe56f 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -474,4 +474,24 @@ test_expect_success 'git rebase --update-ref with
core.commentChar and branch on
  test_grep "% Ref refs/heads/topic2 checked out at" actual
 '

+test_expect_success 'rebase preserves change-id header' '
+ test_commit "source-for-rebase" file-rebase content-rebase &&
+ git cat-file commit HEAD >commit_obj &&
+ awk "/^committer / { print; print \"change-id my-change-id\"; next
}1" commit_obj >commit_obj_mod &&
+ new_commit=$(git hash-object -t commit -w commit_obj_mod) &&
+ git branch -f source-branch $new_commit &&
+
+ git checkout -b target-branch HEAD^ &&
+ echo "unrelated" >file-unrelated &&
+ git add file-unrelated &&
+ git commit -m "unrelated" &&
+
+ git checkout source-branch &&
+ git rebase target-branch &&
+
+ git cat-file commit HEAD >result_obj &&
+ grep "^change-id my-change-id$" result_obj
+'
+
 test_done
+
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 8025a28cfd..0ada99f216 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -256,4 +256,19 @@ test_expect_success 'cherry-pick is unaware of
--reference (for now)' '
  grep "^usage: git cherry-pick" actual
 '

+test_expect_success 'cherry-pick preserves change-id header' '
+ test_commit "source-for-cherry" file-cherry content-cherry &&
+ git cat-file commit HEAD >commit_obj &&
+ awk "/^committer / { print; print \"change-id my-change-id\"; next
}1" commit_obj >commit_obj_mod &&
+ new_commit=$(git hash-object -t commit -w commit_obj_mod) &&
+ git branch -f source-branch $new_commit &&
+
+ git checkout -b target-branch HEAD^ &&
+ git cherry-pick source-branch &&
+
+ git cat-file commit HEAD >result_obj &&
+ grep "^change-id my-change-id$" result_obj
+'
+
 test_done
+
diff --git a/t/t7501-commit-basic-functionality.sh
b/t/t7501-commit-basic-functionality.sh
index a37509f004..e25dd9dc6f 100755
--- a/t/t7501-commit-basic-functionality.sh
+++ b/t/t7501-commit-basic-functionality.sh
@@ -793,4 +793,19 @@ test_expect_success '--dry-run --short' '
  git commit --dry-run --short
 '

+test_expect_success 'amend preserves change-id header' '
+ test_commit "source-for-amend" file-amend content-amend &&
+ git cat-file commit HEAD >commit_obj &&
+ awk "/^committer / { print; print \"change-id my-change-id\"; next
}1" commit_obj >commit_obj_mod &&
+ new_commit=$(git hash-object -t commit -w commit_obj_mod) &&
+ git reset --hard $new_commit &&
+
+ echo "amended content" >>file-amend &&
+ git add file-amend &&
+ git commit --amend --no-edit &&
+
+ git cat-file commit HEAD >result_obj &&
+ grep "^change-id my-change-id$" result_obj
+'
+
 test_done
-- 
2.53.0.1213.gd9a14994de-goog