rebase: support --trailer

Add a new --trailer=<trailer> option to git rebase to append trailer
lines to each rewritten commit message (merge backend only).

Because the apply backend does not provide a commit-message filter,
reject --trailer when --apply is in effect and require the merge backend
instead.

This option implies --force-rebase so that fast-forwarded commits are
also rewritten. Validate trailer arguments early to avoid starting an
interactive rebase with invalid input.

Add integration tests covering error paths and trailer insertion across
non-interactive and interactive rebases.

Signed-off-by: Li Chen <me@linux.beauty>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc
index e177808004fa8159f93ce3fd548d6ee9ac5cd16e..f6c22d1598978a88cbc690e9012c12197b9ea3ab 100644 (file)
--- a/Documentation/git-rebase.adoc
+++ b/Documentation/git-rebase.adoc
@@ -497,6 +497,13 @@ See also INCOMPATIBLE OPTIONS below.
 +
 See also INCOMPATIBLE OPTIONS below.
 
+--trailer=<trailer>::
+       Append the given trailer to every rebased commit message, processed
+       via linkgit:git-interpret-trailers[1]. This option implies
+       `--force-rebase`.
++
+See also INCOMPATIBLE OPTIONS below.
+
 -i::
 --interactive::
        Make a list of the commits which are about to be rebased.  Let the
@@ -653,6 +660,7 @@ are incompatible with the following options:
  * --[no-]reapply-cherry-picks when used without --keep-base
  * --update-refs
  * --root when used without --onto
+ * --trailer
 
 In addition, the following pairs of options are incompatible:
 

diff --git a/builtin/rebase.c b/builtin/rebase.c
index c487e1090779c2c6ee3a1464662a47178f354817..fe25d2ad4bc7ccb49ff8714f333bf20fd1f4ffea 100644 (file)
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -36,6 +36,7 @@
 #include "reset.h"
 #include "trace2.h"
 #include "hook.h"
+#include "trailer.h"
 
 static char const * const builtin_rebase_usage[] = {
        N_("git rebase [-i] [options] [--exec <cmd>] "
@@ -113,6 +114,7 @@ struct rebase_options {
        enum action action;
        char *reflog_action;
        int signoff;
+       struct strvec trailer_args;
        int allow_rerere_autoupdate;
        int keep_empty;
        int autosquash;
@@ -143,6 +145,7 @@ struct rebase_options {
                .flags = REBASE_NO_QUIET,               \
                .git_am_opts = STRVEC_INIT,             \
                .exec = STRING_LIST_INIT_NODUP,         \
+               .trailer_args = STRVEC_INIT,            \
                .git_format_patch_opt = STRBUF_INIT,    \
                .fork_point = -1,                       \
                .reapply_cherry_picks = -1,             \
@@ -166,6 +169,7 @@ static void rebase_options_release(struct rebase_options *opts)
        free(opts->strategy);
        string_list_clear(&opts->strategy_opts, 0);
        strbuf_release(&opts->git_format_patch_opt);
+       strvec_clear(&opts->trailer_args);
 }
 
 static struct replay_opts get_replay_opts(const struct rebase_options *opts)
@@ -177,6 +181,10 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)
        sequencer_init_config(&replay);
 
        replay.signoff = opts->signoff;
+
+       for (size_t i = 0; i < opts->trailer_args.nr; i++)
+               strvec_push(&replay.trailer_args, opts->trailer_args.v[i]);
+
        replay.allow_ff = !(opts->flags & REBASE_FORCE);
        if (opts->allow_rerere_autoupdate)
                replay.allow_rerere_auto = opts->allow_rerere_autoupdate;
@@ -1132,6 +1140,8 @@ int cmd_rebase(int argc,
                        .flags = PARSE_OPT_NOARG,
                        .defval = REBASE_DIFFSTAT,
                },
+               OPT_STRVEC(0, "trailer", &options.trailer_args, N_("trailer"),
+                          N_("add custom trailer(s)")),
                OPT_BOOL(0, "signoff", &options.signoff,
                         N_("add a Signed-off-by trailer to each commit")),
                OPT_BOOL(0, "committer-date-is-author-date",
@@ -1285,6 +1295,12 @@ int cmd_rebase(int argc,
                             builtin_rebase_options,
                             builtin_rebase_usage, 0);
 
+       if (options.trailer_args.nr) {
+               if (validate_trailer_args(&options.trailer_args))
+                       die(NULL);
+               options.flags |= REBASE_FORCE;
+       }
+
        if (preserve_merges_selected)
                die(_("--preserve-merges was replaced by --rebase-merges\n"
                        "Note: Your `pull.rebase` configuration may also be set to 'preserve',\n"
@@ -1542,6 +1558,9 @@ int cmd_rebase(int argc,
        if (options.root && !options.onto_name)
                imply_merge(&options, "--root without --onto");
 
+       if (options.trailer_args.nr)
+               imply_merge(&options, "--trailer");
+
        if (isatty(2) && options.flags & REBASE_NO_QUIET)
                strbuf_addstr(&options.git_format_patch_opt, " --progress");
 

diff --git a/sequencer.c b/sequencer.c
index a3eb39bb25248a819d3a0c15ba8af24ffbf68208..a2d72ce8b3b3c55ce9baaf5ce496c8ec1ff8f23d 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -209,6 +209,7 @@ static GIT_PATH_FUNC(rebase_path_reschedule_failed_exec, "rebase-merge/reschedul
 static GIT_PATH_FUNC(rebase_path_no_reschedule_failed_exec, "rebase-merge/no-reschedule-failed-exec")
 static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, "rebase-merge/drop_redundant_commits")
 static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redundant_commits")
+static GIT_PATH_FUNC(rebase_path_trailer, "rebase-merge/trailer")
 
 /*
  * A 'struct replay_ctx' represents the private state of the sequencer.
@@ -420,6 +421,7 @@ void replay_opts_release(struct replay_opts *opts)
        if (opts->revs)
                release_revisions(opts->revs);
        free(opts->revs);
+       strvec_clear(&opts->trailer_args);
        replay_ctx_release(ctx);
        free(opts->ctx);
 }
@@ -2019,12 +2021,15 @@ static int append_squash_message(struct strbuf *buf, const char *body,
        if (is_fixup_flag(command, flag) && !seen_squash(ctx)) {
                /*
                 * We're replacing the commit message so we need to
-                * append the Signed-off-by: trailer if the user
-                * requested '--signoff'.
+                * append any trailers if the user requested
+                * '--signoff' or '--trailer'.
                 */
                if (opts->signoff)
                        append_signoff(buf, 0, 0);
 
+               if (opts->trailer_args.nr)
+                       amend_strbuf_with_trailers(buf, &opts->trailer_args);
+
                if ((command == TODO_FIXUP) &&
                    (flag & TODO_REPLACE_FIXUP_MSG) &&
                    (file_exists(rebase_path_fixup_msg()) ||
@@ -2443,6 +2448,9 @@ static int do_pick_commit(struct repository *r,
        if (opts->signoff && !is_fixup(command))
                append_signoff(&ctx->message, 0, 0);
 
+       if (opts->trailer_args.nr && !is_fixup(command))
+               amend_strbuf_with_trailers(&ctx->message, &opts->trailer_args);
+
        if (is_rebase_i(opts) && write_author_script(msg.message) < 0)
                res = -1;
        else if (!opts->strategy ||
@@ -3172,6 +3180,33 @@ static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)
        parse_strategy_opts(opts, buf->buf);
 }
 
+static int read_trailers(struct replay_opts *opts, struct strbuf *buf)
+{
+       ssize_t len;
+
+       strbuf_reset(buf);
+       len = strbuf_read_file(buf, rebase_path_trailer(), 0);
+       if (len > 0) {
+               char *p = buf->buf, *nl;
+
+               trailer_config_init();
+
+               while ((nl = strchr(p, '\n'))) {
+                       *nl = '\0';
+                       if (!*p)
+                               return error(_("trailers file contains empty line"));
+                       strvec_push(&opts->trailer_args, p);
+                       p = nl + 1;
+               }
+       } else if (!len) {
+               return error(_("trailers file is empty"));
+       } else if (errno != ENOENT) {
+               return error(_("cannot read trailers files"));
+       }
+
+       return 0;
+}
+
 static int read_populate_opts(struct replay_opts *opts)
 {
        struct replay_ctx *ctx = opts->ctx;
@@ -3233,6 +3268,11 @@ static int read_populate_opts(struct replay_opts *opts)
                        opts->keep_redundant_commits = 1;
 
                read_strategy_opts(opts, &buf);
+
+               if (read_trailers(opts, &buf)) {
+                       ret = -1;
+                       goto done_rebase_i;
+               }
                strbuf_reset(&buf);
 
                if (read_oneliner(&ctx->current_fixups,
@@ -3328,6 +3368,14 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
                write_file(rebase_path_reschedule_failed_exec(), "%s", "");
        else
                write_file(rebase_path_no_reschedule_failed_exec(), "%s", "");
+       if (opts->trailer_args.nr) {
+               struct strbuf buf = STRBUF_INIT;
+
+               for (size_t i = 0; i < opts->trailer_args.nr; i++)
+                       strbuf_addf(&buf, "%s\n", opts->trailer_args.v[i]);
+               write_file(rebase_path_trailer(), "%s", buf.buf);
+               strbuf_release(&buf);
+       }
 
        return 0;
 }

diff --git a/sequencer.h b/sequencer.h
index 719684c8a9fb2e5ab10464e3edf1726097fe33c8..bea20da085f0a2a62655160bf99845b1f57547d0 100644 (file)
--- a/sequencer.h
+++ b/sequencer.h
@@ -57,6 +57,8 @@ struct replay_opts {
        int ignore_date;
        int commit_use_reference;
 
+       struct strvec trailer_args;
+
        int mainline;
 
        char *gpg_sign;
@@ -84,6 +86,7 @@ struct replay_opts {
 #define REPLAY_OPTS_INIT {                     \
        .edit = -1,                             \
        .action = -1,                           \
+       .trailer_args = STRVEC_INIT,            \
        .xopts = STRVEC_INIT,                   \
        .ctx = replay_ctx_new(),                \
 }

diff --git a/t/meson.build b/t/meson.build
index f80e366cff73f3330de163d877d5102592566cfa..1f6f8ac20b6c4882c9fa467e636317fb20827fc5 100644 (file)
--- a/t/meson.build
+++ b/t/meson.build
@@ -388,6 +388,7 @@ integration_tests = [
   't3436-rebase-more-options.sh',
   't3437-rebase-fixup-options.sh',
   't3438-rebase-broken-files.sh',
+  't3440-rebase-trailer.sh',
   't3450-history.sh',
   't3451-history-reword.sh',
   't3500-cherry.sh',

diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
new file mode 100755 (executable)
index 0000000..8b47579
--- /dev/null
+++ b/t/t3440-rebase-trailer.sh
@@ -0,0 +1,147 @@
+#!/bin/sh
+#
+
+test_description='git rebase --trailer integration tests
+We verify that --trailer works with the merge backend,
+and that it is rejected early when the apply backend is requested.'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh # test_commit_message, helpers
+
+REVIEWED_BY_TRAILER="Reviewed-by: Dev <dev@example.com>"
+SP=" "
+
+test_expect_success 'setup repo with a small history' '
+       git commit --allow-empty -m "Initial empty commit" &&
+       test_commit first file a &&
+       test_commit second file &&
+       git checkout -b conflict-branch first &&
+       test_commit file-2 file-2 &&
+       test_commit conflict file &&
+       test_commit third file &&
+       git checkout main
+'
+
+test_expect_success 'apply backend is rejected with --trailer' '
+       git checkout -B apply-backend third &&
+       test_expect_code 128 \
+               git rebase --apply --trailer "$REVIEWED_BY_TRAILER" HEAD^ 2>err &&
+       test_grep "fatal: --trailer requires the merge backend" err
+'
+
+test_expect_success 'reject empty --trailer argument' '
+       git checkout -B empty-trailer third &&
+       test_expect_code 128 git rebase --trailer "" HEAD^ 2>err &&
+       test_grep "empty --trailer" err
+'
+
+test_expect_success 'reject trailer with missing key before separator' '
+       git checkout -B missing-key third &&
+       test_expect_code 128 git rebase --trailer ": no-key" HEAD^ 2>err &&
+       test_grep "missing key before separator" err
+'
+
+test_expect_success 'allow trailer with missing value after separator' '
+       git checkout -B missing-value third &&
+       git rebase --trailer "Acked-by:" HEAD^ &&
+       test_commit_message HEAD <<-EOF
+       third
+
+       Acked-by:${SP}
+       EOF
+'
+
+test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last' '
+       git checkout -B replace-policy third &&
+       git -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add \
+               rebase --trailer "Bug: 123" --trailer "Bug: 456" HEAD^ &&
+       test_commit_message HEAD <<-EOF
+       third
+
+       Bug: 456
+       EOF
+'
+
+test_expect_success 'multiple Signed-off-by trailers all preserved' '
+       git checkout -B multiple-signoff third &&
+       git rebase --trailer "Signed-off-by: Dev A <a@example.com>" \
+               --trailer "Signed-off-by: Dev B <b@example.com>" HEAD^ &&
+       test_commit_message HEAD <<-EOF
+       third
+
+       Signed-off-by: Dev A <a@example.com>
+       Signed-off-by: Dev B <b@example.com>
+       EOF
+'
+
+test_expect_success 'rebase --trailer adds trailer after conflicts' '
+       git checkout -B trailer-conflict third &&
+       test_commit fourth file &&
+       test_must_fail git rebase --trailer "$REVIEWED_BY_TRAILER" second &&
+       git checkout --theirs file &&
+       git add file &&
+       git rebase --continue &&
+       test_commit_message HEAD <<-EOF &&
+       fourth
+
+       $REVIEWED_BY_TRAILER
+       EOF
+       test_commit_message HEAD^ <<-EOF
+       third
+
+       $REVIEWED_BY_TRAILER
+       EOF
+'
+
+test_expect_success '--trailer handles fixup commands in todo list' '
+       git checkout -B fixup-trailer third &&
+       test_commit fixup-base base &&
+       test_commit fixup-second second &&
+       cat >todo <<-\EOF &&
+       pick fixup-base fixup-base
+       fixup fixup-second fixup-second
+       EOF
+       (
+               set_replace_editor todo &&
+               git rebase -i --trailer "$REVIEWED_BY_TRAILER" HEAD~2
+       ) &&
+       test_commit_message HEAD <<-EOF &&
+       fixup-base
+
+       $REVIEWED_BY_TRAILER
+       EOF
+       git reset --hard fixup-second &&
+       cat >todo <<-\EOF &&
+       pick fixup-base fixup-base
+       fixup -C fixup-second fixup-second
+       EOF
+       (
+               set_replace_editor todo &&
+               git rebase -i --trailer "$REVIEWED_BY_TRAILER" HEAD~2
+       ) &&
+       test_commit_message HEAD <<-EOF
+       fixup-second
+
+       $REVIEWED_BY_TRAILER
+       EOF
+'
+
+test_expect_success 'rebase --root honors trailer.<name>.key' '
+       git checkout -B root-trailer first &&
+       git -c trailer.review.key=Reviewed-by rebase --root \
+               --trailer=review="Dev <dev@example.com>" &&
+       test_commit_message HEAD <<-EOF &&
+       first
+
+       Reviewed-by: Dev <dev@example.com>
+       EOF
+       test_commit_message HEAD^ <<-EOF
+       Initial empty commit
+
+       Reviewed-by: Dev <dev@example.com>
+       EOF
+'
+test_done