{"thread":{"id":"63616","subject":"[PATCH v2 0/2] rebase: support --trailer","startedAt":"2025-06-10T12:35:17Z","lastAt":"2025-06-12T05:40:18Z","messageCount":7,"participants":["Li Chen","Junio C Hamano","phillip.wood123@gmail.com"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"520034","messageId":"20250610123459.278582-1-me@linux.beauty","threadId":"63616","inReplyTo":null,"subject":"[PATCH v2 0/2] rebase: support --trailer","fromName":"Li Chen","fromEmail":"me@linux.beauty","sentAt":"2025-06-10T12:34:57Z","receivedAt":"2025-06-10T12:35:17Z","isPatch":true,"sender":{"key":"me@linux.beauty","avatar":"https://avatars.githubusercontent.com/u/37442588?v=4"},"body":"From: Li Chen <chenl311@chinatelecom.cn>\n\nThis two-patch series teaches git rebase a new\n--trailer <text> option and, as a prerequisite, moves all trailer\nhandling out of the external interpret-trailers helper and into the\nbuiltin code path, as suggested by Phillip Wood.\n\nPatch 1 switches trailer.c to an in-memory implementation\n(amend_strbuf_with_trailers()). It removes every fork/exec.\n\nPatch 2 builds on that helper to implement\ngit rebase --trailer. When the option is given we:\nforce the merge backend (apply/am backend lacks a message filter),\nautomatically enable --force-rebase so that fast-forwarded\ncommits are rewritten, and append the requested trailer(s) to every\nrewritten commit.\nState is stored in $state_dir/trailer so an interrupted rebase can\nresume safely. A dedicated test-suite (t3440) exercises plain,\nconflict, --root, invalid-input scenarios and etc.\n\nAll t/*.sh testcases have run successfully.\nGithub CI tests have all been past: https://github.com/FirstLoveLife/git/actions/runs/15558597606/job/43805117563\n(except for debian built test because of network issue)\n\nRFC link: https://lore.kernel.org/git/196a5ac1393.f5b4db7d187309.2451613571977217927@linux.beauty/\n\nThe review comments from Phillip in the RFC series have been fixed. \nThanks a lot for Phillip's great review!\n\nComments welcome!\n\nLi Chen (2):\n  trailer: append trailers in-process and drop the fork to\n    `interpret-trailers`\n  rebase: support --trailer\n\n Documentation/git-rebase.adoc |   7 ++\n builtin/rebase.c              |  98 +++++++++++++++++++++++\n sequencer.c                   |  13 +++\n sequencer.h                   |   3 +\n t/meson.build                 |   1 +\n t/t3440-rebase-trailer.sh     |  95 ++++++++++++++++++++++\n trailer.c                     | 147 +++++++++++++++++++++++++++++++---\n trailer.h                     |  17 ++++\n 8 files changed, 372 insertions(+), 9 deletions(-)\n create mode 100755 t/t3440-rebase-trailer.sh\n\n-- \n2.49.0\n\n"},{"id":"520035","messageId":"20250610123459.278582-2-me@linux.beauty","threadId":"63616","inReplyTo":"20250610123459.278582-1-me@linux.beauty","subject":"[PATCH v2 1/2] trailer: append trailers in-process and drop the fork to `interpret-trailers`","fromName":"Li Chen","fromEmail":"me@linux.beauty","sentAt":"2025-06-10T12:34:58Z","receivedAt":"2025-06-10T12:35:33Z","isPatch":true,"sender":{"key":"me@linux.beauty","avatar":"https://avatars.githubusercontent.com/u/37442588?v=4"},"body":"From: Li Chen <chenl311@chinatelecom.cn>\n\nThe built-in commands that support `--trailer` (tag, commit --amend,\nrebase --trailer, etc.) have always appended trailers by spawning an\nexternal `git interpret-trailers --in-place` process.  That extra fork\nis negligible for a single commit, but it scales poorly when hundreds or\nthousands of commits are rewritten (e.g. an interactive rebase or a\nhistory-wide `filter-repo`).  This patch moves the heavy lifting fully\nin-process:\n\n* `amend_strbuf_with_trailers()`\n  – parses the existing message,\n  – merges trailers from the command line and config,\n  – formats the final trailer block, and\n  – rewrites the supplied `struct strbuf`\n  without ever leaving the current process.\n\n* `amend_file_with_trailers()` becomes a thin wrapper that reads a file\n  into a `strbuf`, calls the new helper, and writes the result back.\n\n* `builtin/rebase.c` now calls `amend_file_with_trailers()` instead of\n  executing `interpret-trailers`.\n\nEdge-cases that used to be handled implicitly by the external helper are\nnow replicated:\n\n  * `trailer.ifexists=replace` and other per-key policies are honoured by\n    running the existing `parse_trailers_from_command_line_args()` logic\n    on the raw CLI list, so the same *replace / add / ignore* semantics\n    are preserved.\n\n  * A message that contains an RFC-2822 style `---` separator but no\n    prior trailer block gets its trailers appended **after** the\n    separator, matching the external command’s output.\n\n  * When `git commit --verbose` leaves a diff after the message (comment\n    lines beginning with `#`), trailers are inserted before that comment\n    section so they survive the subsequent strip-space cleanup.\n\n  * Ordering remains identical to the old path: CLI trailers first,\n    followed by trailers introduced via `trailer.<key>.*` config.\n\nSigned-off-by: Li Chen <chenl311@chinatelecom.cn>\n---\n trailer.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++++----\n trailer.h |  17 +++++++\n 2 files changed, 155 insertions(+), 9 deletions(-)\n\ndiff --git a/trailer.c b/trailer.c\nindex 310cf582dc..70b04736a8 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -1224,14 +1224,143 @@ void trailer_iterator_release(struct trailer_iterator *iter)\n \tstrbuf_release(&iter->key);\n }\n \n-int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)\n+static size_t first_comment_pos(const struct strbuf *buf)\n {\n-\tstruct child_process run_trailer = CHILD_PROCESS_INIT;\n-\n-\trun_trailer.git_cmd = 1;\n-\tstrvec_pushl(&run_trailer.args, \"interpret-trailers\",\n-\t\t     \"--in-place\", \"--no-divider\",\n-\t\t     path, NULL);\n-\tstrvec_pushv(&run_trailer.args, trailer_args->v);\n-\treturn run_command(&run_trailer);\n+\tconst char *p = buf->buf;\n+\tconst char *end = buf->buf + buf->len;\n+\n+\twhile (p < end) {\n+\t\tconst char *line = p;\n+\t\tconst char *nl = memchr(p, '\\n', end - p);\n+\t\tsize_t len = nl ? (size_t)(nl - p) : (size_t)(end - p);\n+\n+\t\t/* skip leading whitespace */\n+\t\tsize_t i = 0;\n+\t\twhile (i < len && isspace((unsigned char)line[i]))\n+\t\t\ti++;\n+\n+\t\tif (i < len && line[i] == '#')\n+\t\t\treturn (size_t)(line - buf->buf); /* comment starts here */\n+\n+\t\tif (!nl)              /* last line without newline */\n+\t\t\tbreak;\n+\t\tp = nl + 1;\n+\t}\n+\treturn buf->len;          /* no comment line found */\n+}\n+\n+int amend_strbuf_with_trailers(struct strbuf *buf,\n+\t\t\t\t\t\t\t   const struct strvec *trailer_args)\n+{\n+\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n+\tstruct strbuf trailers_sb = STRBUF_INIT;\n+\tstruct strbuf out         = STRBUF_INIT;\n+\tsize_t i;\n+\t/* 1. parse message ------------------------------------------------- */\n+\tLIST_HEAD(orig_head);   /* existing trailers, if any          */\n+\tLIST_HEAD(cfg_head);    /* config trailers                    */\n+\tLIST_HEAD(cli_raw);     /* new_trailer_item from CLI          */\n+\tLIST_HEAD(cli_head);    /* arg_item after helper              */\n+\tLIST_HEAD(all_new);     /* merged list                        */\n+\tstruct trailer_block *blk = parse_trailers(&opts, buf->buf, &orig_head);\n+\tbool had_trailer_before = !list_empty(&orig_head);\n+\n+\t/* 2. CLI trailers -------------------------------------------------- */\n+\tif (trailer_args && trailer_args->nr) {\n+\t\tfor (i = 0; i < trailer_args->nr; i++) {\n+\t\t\t\tconst char *arg  = trailer_args->v[i];\n+\t\t\t\tconst char *text;\n+\t\t\t\tstruct new_trailer_item *ni;\n+\n+\t\t\t\tif (!skip_prefix(arg, \"--trailer=\", &text))\n+\t\t\t\t\t\ttext = arg;\n+\n+\t\t\t\tif (!*text)\n+\t\t\t\t\t\tcontinue;\n+\n+\t\t\t\tni = xcalloc(1, sizeof(*ni));\n+\t\t\t\tINIT_LIST_HEAD(&ni->list);\n+\t\t\t\tni->text = (char *)text;\n+\t\t\t\tlist_add_tail(&ni->list, &cli_raw);\n+\t\t}\n+\t\tparse_trailers_from_command_line_args(&cli_head, &cli_raw);\n+\n+\t\twhile (!list_empty(&cli_raw)) {\n+\t\t\t\tstruct new_trailer_item *ni =\n+\t\t\t\t\t\tlist_first_entry(&cli_raw, struct new_trailer_item, list);\n+\t\t\t\tlist_del(&ni->list);\n+\t\t\t\tfree(ni);\n+\t\t}\n+\t}\n+\n+\t/* 3. config trailers ---------------------------------------------- */\n+\tparse_trailers_from_config(&cfg_head);\n+\n+\t/* 4. merge lists --------------------------------------------------- */\n+\tlist_splice(&cli_head, &all_new);\n+\tlist_splice(&cfg_head, &all_new);\n+\n+\t/* 5. apply --------------------------------------------------------- */\n+\tprocess_trailers_lists(&orig_head, &all_new);\n+\n+\t/* 6. format updated trailer block --------------------------------- */\n+\tformat_trailers(&opts, &orig_head, &trailers_sb);\n+\n+\t/* 7. decide insertion point --------------------------------------- */\n+\tif (had_trailer_before) {\n+\t\t/* insert at existing trailer block */\n+\t\tstrbuf_add(&out, buf->buf, trailer_block_start(blk));\n+\t\tif (!blank_line_before_trailer_block(blk))\n+\t\t\tstrbuf_addch(&out, '\\n');\n+\t\tstrbuf_addbuf(&out, &trailers_sb);\n+\t\tstrbuf_add(&out,\n+\t\t\t\t   buf->buf + trailer_block_end(blk),\n+\t\t\t\t   buf->len - trailer_block_end(blk));\n+\t} else {\n+\t\t/* insert before first comment (git --verbose) if any */\n+\t\tsize_t cpos = first_comment_pos(buf);\n+\n+\t\t/* copy body up to comment (or whole buf if none) */\n+\t\tstrbuf_add(&out, buf->buf, cpos);\n+\n+\t\t/* ensure single blank line separating body & trailers */\n+\t\tif (!out.len || out.buf[out.len - 1] != '\\n')\n+\t\t\tstrbuf_addch(&out, '\\n');\n+\t\tif (out.len >= 2 && out.buf[out.len - 2] != '\\n')\n+\t\t\tstrbuf_addch(&out, '\\n');\n+\n+\t\tstrbuf_addbuf(&out, &trailers_sb);\n+\n+\t\t/* copy remaining comment lines (if any) */\n+\t\tstrbuf_add(&out, buf->buf + cpos, buf->len - cpos);\n+\t}\n+\n+\tstrbuf_swap(buf, &out);\n+\n+\t/* 8. cleanup ------------------------------------------------------- */\n+\tstrbuf_release(&out);\n+\tstrbuf_release(&trailers_sb);\n+\tfree_trailers(&orig_head);\n+\ttrailer_block_release(blk);\n+\treturn 0;\n+}\n+\n+int amend_file_with_trailers(const char *path,\n+\t\t\t\t\t\t\t const struct strvec *trailer_args)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tif (!trailer_args || !trailer_args->nr)\n+\t\treturn 0;\n+\n+\tif (strbuf_read_file(&buf, path, 0) < 0)\n+\t\treturn error_errno(\"could not read '%s'\", path);\n+\n+\tif (amend_strbuf_with_trailers(&buf, trailer_args))\n+\t\tdie(\"failed to append trailers\");\n+\n+\t/* `write_file_buf()` aborts on error internally */\n+\twrite_file_buf(path, buf.buf, buf.len);\n+\tstrbuf_release(&buf);\n+\treturn 0;\n }\ndiff --git a/trailer.h b/trailer.h\nindex 4740549586..17986d5dd0 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -202,4 +202,21 @@ void trailer_iterator_release(struct trailer_iterator *iter);\n  */\n int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);\n \n+/*\n+ * In‑memory variant of amend_file_with_trailers(): instead of rewriting a file\n+ * on disk (and therefore spawning a separate `git interpret‑trailers` process)\n+ * we operate directly on a struct strbuf that already contains the commit\n+ * message.  The rules for positioning, deduplication, and formatting are the\n+ * same as those implemented by the builtin `interpret‑trailers` command.\n+ *\n+ *  - @buf          : the message to be amended.  On success, its contents are\n+ *                    replaced with the new message that has the trailers\n+ *                    inserted.\n+ *  - @trailer_args : the list of trailer strings exactly as would be passed on\n+ *                    the command‑line via repeated `--trailer` options.\n+ *\n+ * Returns 0 on success, <0 on error.\n+ */\n+int amend_strbuf_with_trailers(struct strbuf *buf,\n+                               const struct strvec *trailer_args);\n #endif /* TRAILER_H */\n-- \n2.49.0\n\n"},{"id":"520036","messageId":"20250610123459.278582-3-me@linux.beauty","threadId":"63616","inReplyTo":"20250610123459.278582-1-me@linux.beauty","subject":"[PATCH v2 2/2] rebase: support --trailer","fromName":"Li Chen","fromEmail":"me@linux.beauty","sentAt":"2025-06-10T12:34:59Z","receivedAt":"2025-06-10T12:35:44Z","isPatch":true,"sender":{"key":"me@linux.beauty","avatar":"https://avatars.githubusercontent.com/u/37442588?v=4"},"body":"From: Li Chen <chenl311@chinatelecom.cn>\n\nImplement a new `--trailer <text>` option for `git rebase`\n(support merge backend only now), which appends arbitrary\ntrailer lines to each rebased commit message. Reject early\nif used with the apply backend (git am) since it lacks\nmessage‑filter/trailer hook. Automatically set REBASE_FORCE when\nany trailer is supplied.\n\nAnd reject invalid input before user edit the interactive file,\nas suggested by Phillip Wood.\n\nSigned-off-by: Li Chen <chenl311@chinatelecom.cn>\n---\n Documentation/git-rebase.adoc |  7 +++\n builtin/rebase.c              | 98 +++++++++++++++++++++++++++++++++++\n sequencer.c                   | 13 +++++\n sequencer.h                   |  3 ++\n t/meson.build                 |  1 +\n t/t3440-rebase-trailer.sh     | 95 +++++++++++++++++++++++++++++++++\n 6 files changed, 217 insertions(+)\n create mode 100755 t/t3440-rebase-trailer.sh\n\ndiff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc\nindex 956d3048f5..df8fb97526 100644\n--- a/Documentation/git-rebase.adoc\n+++ b/Documentation/git-rebase.adoc\n@@ -521,6 +521,13 @@ See also INCOMPATIBLE OPTIONS below.\n \tthat if `--interactive` is given then only commits marked to be\n \tpicked, edited or reworded will have the trailer added.\n +\n+--trailer <trailer>::\n+       Append the given trailer line(s) to every rebased commit\n+       message, processed via linkgit:git-interpret-trailers[1].\n+       When this option is present *rebase automatically enables*\n+       `--force-rebase` so that fast‑forwarded commits are also\n+       rewritten.\n+\n See also INCOMPATIBLE OPTIONS below.\n \n -i::\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 2e8c4ee678..1e21caace7 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -36,6 +36,8 @@\n #include \"reset.h\"\n #include \"trace2.h\"\n #include \"hook.h\"\n+#include \"trailer.h\"\n+#include \"parse-options.h\"\n \n static char const * const builtin_rebase_usage[] = {\n \tN_(\"git rebase [-i] [options] [--exec <cmd>] \"\n@@ -113,6 +115,7 @@ struct rebase_options {\n \tenum action action;\n \tchar *reflog_action;\n \tint signoff;\n+\tstruct strvec trailer_args;\n \tint allow_rerere_autoupdate;\n \tint keep_empty;\n \tint autosquash;\n@@ -143,6 +146,7 @@ struct rebase_options {\n \t\t.flags = REBASE_NO_QUIET, \t\t\\\n \t\t.git_am_opts = STRVEC_INIT,\t\t\\\n \t\t.exec = STRING_LIST_INIT_NODUP,\t\t\\\n+\t\t.trailer_args = STRVEC_INIT,  \\\n \t\t.git_format_patch_opt = STRBUF_INIT,\t\\\n \t\t.fork_point = -1,\t\t\t\\\n \t\t.reapply_cherry_picks = -1,             \\\n@@ -166,6 +170,7 @@ static void rebase_options_release(struct rebase_options *opts)\n \tfree(opts->strategy);\n \tstring_list_clear(&opts->strategy_opts, 0);\n \tstrbuf_release(&opts->git_format_patch_opt);\n+\tstrvec_clear(&opts->trailer_args);\n }\n \n static struct replay_opts get_replay_opts(const struct rebase_options *opts)\n@@ -177,6 +182,10 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)\n \tsequencer_init_config(&replay);\n \n \treplay.signoff = opts->signoff;\n+\n+\tfor (size_t i = 0; i < opts->trailer_args.nr; i++)\n+\t\tstrvec_push(&replay.trailer_args, opts->trailer_args.v[i]);\n+\n \treplay.allow_ff = !(opts->flags & REBASE_FORCE);\n \tif (opts->allow_rerere_autoupdate)\n \t\treplay.allow_rerere_auto = opts->allow_rerere_autoupdate;\n@@ -435,6 +444,8 @@ static int read_basic_state(struct rebase_options *opts)\n \tstruct strbuf head_name = STRBUF_INIT;\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct object_id oid;\n+\tconst char trailer_state_name[] = \"trailer\";\n+\tconst char *path = state_dir_path(trailer_state_name, opts);\n \n \tif (!read_oneliner(&head_name, state_dir_path(\"head-name\", opts),\n \t\t\t   READ_ONELINER_WARN_MISSING) ||\n@@ -503,11 +514,31 @@ static int read_basic_state(struct rebase_options *opts)\n \n \tstrbuf_release(&buf);\n \n+\tif (strbuf_read_file(&buf, path, 0) >= 0) {\n+\t\tconst char *p = buf.buf, *end = buf.buf + buf.len;\n+\n+\t\twhile (p < end) {\n+\t\t\tchar *nl = memchr(p, '\\n', end - p);\n+\t\t\tif (!nl)\n+\t\t\t\tdie(\"nl shouldn't be NULL\");\n+\t\t\t*nl = '\\0';\n+\n+\t\t\tif (*p)\n+\t\t\t\tstrvec_push(&opts->trailer_args, p);\n+\n+\t\t\tp = nl + 1;\n+\t\t}\n+\t\tstrbuf_release(&buf);\n+\t}\n+\tstrbuf_release(&buf);\n+\n \treturn 0;\n }\n \n static int rebase_write_basic_state(struct rebase_options *opts)\n {\n+\tconst char trailer_state_name[] = \"trailer\";\n+\n \twrite_file(state_dir_path(\"head-name\", opts), \"%s\",\n \t\t   opts->head_name ? opts->head_name : \"detached HEAD\");\n \twrite_file(state_dir_path(\"onto\", opts), \"%s\",\n@@ -529,6 +560,22 @@ static int rebase_write_basic_state(struct rebase_options *opts)\n \tif (opts->signoff)\n \t\twrite_file(state_dir_path(\"signoff\", opts), \"--signoff\");\n \n+    /*\n+     * save opts->trailer_args into state_dir/trailer\n+     */\n+    if (opts->trailer_args.nr) {\n+            struct strbuf buf = STRBUF_INIT;\n+            size_t i;\n+\n+            for (i = 0; i < opts->trailer_args.nr; i++) {\n+                    strbuf_addstr(&buf, opts->trailer_args.v[i]);\n+                    strbuf_addch(&buf, '\\n');\n+            }\n+            write_file(state_dir_path(trailer_state_name, opts),\n+                       \"%s\", buf.buf);\n+            strbuf_release(&buf);\n+    }\n+\n \treturn 0;\n }\n \n@@ -1085,6 +1132,37 @@ static int check_exec_cmd(const char *cmd)\n \treturn 0;\n }\n \n+static int validate_trailer_args_after_config(const struct strvec *cli_args,\n+\t\t\t\t       struct strbuf *err)\n+{\n+\tsize_t i;\n+\n+\tfor (i = 0; i < cli_args->nr; i++) {\n+\t\tconst char *raw = cli_args->v[i];\n+\t\tconst char *txt; // Key[:=]Val\n+\t\tconst char *sep;\n+\n+\t\tif (!skip_prefix(raw, \"--trailer=\", &txt))\n+\t\t\ttxt = raw;\n+\n+\t\tif (!*txt) {\n+\t\t\tstrbuf_addstr(err, _(\"empty --trailer argument\"));\n+\t\t\treturn -1;\n+\t\t}\n+\n+\t\tsep = strpbrk(txt, \":=\");\n+\n+\t\t/* there must be key bfore seperator */\n+\t\tif (sep && sep == txt) {\n+\t\t\tstrbuf_addf(err,\n+\t\t\t\t    _(\"invalid trailer '%s': missing key before separator\"),\n+\t\t\t\t    txt);\n+\t\t\treturn -1;\n+\t\t}\n+\t}\n+\treturn 0;\n+}\n+\n int cmd_rebase(int argc,\n \t       const char **argv,\n \t       const char *prefix,\n@@ -1132,6 +1210,7 @@ int cmd_rebase(int argc,\n \t\t\t.flags = PARSE_OPT_NOARG,\n \t\t\t.defval = REBASE_DIFFSTAT,\n \t\t},\n+\t\tOPT_STRVEC(0, \"trailer\", &options.trailer_args, N_(\"trailer\"), N_(\"add custom trailer(s)\")),\n \t\tOPT_BOOL(0, \"signoff\", &options.signoff,\n \t\t\t N_(\"add a Signed-off-by trailer to each commit\")),\n \t\tOPT_BOOL(0, \"committer-date-is-author-date\",\n@@ -1282,6 +1361,17 @@ int cmd_rebase(int argc,\n \t\t\t     builtin_rebase_options,\n \t\t\t     builtin_rebase_usage, 0);\n \n+    /* if add --trailer，force rebase */\n+\tif (options.trailer_args.nr) {\n+        struct strbuf err = STRBUF_INIT;\n+\n+\t\tif (validate_trailer_args_after_config(&options.trailer_args, &err))\n+\t\t\tdie(\"%s\", err.buf);\n+\n+        options.flags |= REBASE_FORCE;\n+        strbuf_release(&err);\n+\t}\n+\n \tif (preserve_merges_selected)\n \t\tdie(_(\"--preserve-merges was replaced by --rebase-merges\\n\"\n \t\t\t\"Note: Your `pull.rebase` configuration may also be set to 'preserve',\\n\"\n@@ -1539,6 +1629,14 @@ int cmd_rebase(int argc,\n \tif (options.root && !options.onto_name)\n \t\timply_merge(&options, \"--root without --onto\");\n \n+\t/*\n+\t * The apply‑based backend (git am) cannot append trailers because\n+\t * it lacks a message‑filter facility.  Reject early, before any\n+\t * state (index, HEAD, etc.) is modified.\n+\t */\n+\tif (options.trailer_args.nr)\n+\t\timply_merge(&options, \"--trailer\");\n+\n \tif (isatty(2) && options.flags & REBASE_NO_QUIET)\n \t\tstrbuf_addstr(&options.git_format_patch_opt, \" --progress\");\n \ndiff --git a/sequencer.c b/sequencer.c\nindex 1ee0abbd45..504642e4c7 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -422,6 +422,7 @@ void replay_opts_release(struct replay_opts *opts)\n \tfree(opts->revs);\n \treplay_ctx_release(ctx);\n \tfree(opts->ctx);\n+\tstrvec_clear(&opts->trailer_args);\n }\n \n int sequencer_remove_state(struct replay_opts *opts)\n@@ -2527,6 +2528,18 @@ static int do_pick_commit(struct repository *r,\n \t\t\t_(\"dropping %s %s -- patch contents already upstream\\n\"),\n \t\t\toid_to_hex(&commit->object.oid), msg.subject);\n \t} /* else allow == 0 and there's nothing special to do */\n+\n+    if (!res && opts->trailer_args.nr && !drop_commit) {\n+            const char *trailer_file =\n+                    msg_file ? msg_file : git_path_merge_msg(r);\n+\n+            if (amend_file_with_trailers(trailer_file,\n+                                         &opts->trailer_args)) {\n+                    res = error(_(\"unable to add trailers to commit message\"));\n+                    goto leave;\n+            }\n+    }\n+\n \tif (!opts->no_commit && !drop_commit) {\n \t\tif (author || command == TODO_REVERT || (flags & AMEND_MSG))\n \t\t\tres = do_commit(r, msg_file, author, reflog_action,\ndiff --git a/sequencer.h b/sequencer.h\nindex 304ba4b4d3..28f2da6375 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -4,6 +4,7 @@\n #include \"strbuf.h\"\n #include \"strvec.h\"\n #include \"wt-status.h\"\n+#include <stddef.h>\n \n struct commit;\n struct index_state;\n@@ -44,6 +45,7 @@ struct replay_opts {\n \tint record_origin;\n \tint no_commit;\n \tint signoff;\n+\tstruct strvec trailer_args;\n \tint allow_ff;\n \tint allow_rerere_auto;\n \tint allow_empty;\n@@ -86,6 +88,7 @@ struct replay_opts {\n \t.action = -1,\t\t\t\t\\\n \t.xopts = STRVEC_INIT,\t\t\t\\\n \t.ctx = replay_ctx_new(),\t\t\\\n+\t.trailer_args = STRVEC_INIT, \\\n }\n \n /*\ndiff --git a/t/meson.build b/t/meson.build\nindex d052fc3e23..14ba1d8d75 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -392,6 +392,7 @@ integration_tests = [\n   't3436-rebase-more-options.sh',\n   't3437-rebase-fixup-options.sh',\n   't3438-rebase-broken-files.sh',\n+  't3440-rebase-trailer.sh',\n   't3500-cherry.sh',\n   't3501-revert-cherry-pick.sh',\n   't3502-cherry-pick-merge.sh',\ndiff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh\nnew file mode 100755\nindex 0000000000..a580449628\n--- /dev/null\n+++ b/t/t3440-rebase-trailer.sh\n@@ -0,0 +1,95 @@\n+#!/bin/sh\n+#\n+\n+test_description='git rebase --trailer integration tests\n+We verify that --trailer on the merge/interactive/exec/root backends,\n+and that it is rejected early when the apply backend is requested.'\n+\n+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n+\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-rebase.sh # test_commit_message, helpers\n+\n+create_expect() {\n+\tcat >\"$1\" <<-EOF\n+\t\t$2\n+\n+\t\tReviewed-by: Dev <dev@example.com>\n+\tEOF\n+}\n+\n+test_expect_success 'setup repo with a small history' '\n+\tgit commit --allow-empty -m \"Initial empty commit\" &&\n+\ttest_commit first file a &&\n+\ttest_commit second file &&\n+\tgit checkout -b conflict-branch first &&\n+\ttest_commit file-2 file-2 &&\n+\ttest_commit conflict file &&\n+\ttest_commit third file &&\n+\tident=\"$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" &&\n+\tcreate_expect initial-signed  \"Initial empty commit\" &&\n+\tcreate_expect first-signed    \"first\"                 &&\n+\tcreate_expect second-signed   \"second\"                &&\n+\tcreate_expect file2-signed    \"file-2\"                &&\n+\tcreate_expect third-signed    \"third\"                 &&\n+\tcreate_expect conflict-signed \"conflict\"\n+'\n+\n+test_expect_success 'apply backend is rejected with --trailer' '\n+\tgit reset --hard third &&\n+\thead_before=$(git rev-parse HEAD) &&\n+    test_expect_code 128 \\\n+    \t\tgit rebase --apply --trailer \"Reviewed-by: Dev <dev@example.com>\" \\\n+    \t\t\tHEAD^ 2>err &&\n+\ttest_grep \"requires the merge backend\" err &&\n+\ttest_cmp_rev HEAD $head_before\n+'\n+\n+test_expect_success 'reject empty --trailer argument' '\n+        git reset --hard third &&\n+        test_expect_code 128 git rebase -m --trailer \"\" HEAD^ 2>err &&\n+        test_grep \"empty --trailer\" err\n+'\n+\n+test_expect_success 'reject trailer with missing key before separator' '\n+        git reset --hard third &&\n+        test_expect_code 128 git rebase -m --trailer \": no-key\" HEAD^ 2>err &&\n+        test_grep \"missing key before separator\" err\n+'\n+\n+test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last' '\n+        git reset --hard third &&\n+        git -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add rebase -m --trailer \"Bug: 123\" --trailer \"Bug: 456\" HEAD~1 &&\n+        git cat-file commit HEAD | grep \"^Bug: 456\" &&\n+        git cat-file commit HEAD | grep -v \"^Bug: 123\"\n+'\n+\n+test_expect_success 'multiple Signed-off-by trailers all preserved' '\n+        git reset --hard third &&\n+        git rebase -m \\\n+            --trailer \"Signed-off-by: Dev A <a@ex.com>\" \\\n+            --trailer \"Signed-off-by: Dev B <b@ex.com>\" HEAD~1 &&\n+        git cat-file commit HEAD | grep -c \"^Signed-off-by:\" >count &&\n+        test \"$(cat count)\" = 2   # two new commits\n+'\n+\n+test_expect_success 'rebase -m --trailer adds trailer after conflicts' '\n+\tgit reset --hard third &&\n+\ttest_must_fail git rebase -m \\\n+\t\t--trailer \"Reviewed-by: Dev <dev@example.com>\" \\\n+\t\tsecond third &&\n+\tgit checkout --theirs file &&\n+\tgit add file &&\n+    git rebase --continue &&\n+\ttest_commit_message HEAD~2 file2-signed\n+'\n+\n+test_expect_success 'rebase --root --trailer updates every commit' '\n+\tgit checkout first &&\n+\tgit rebase --root --keep-empty \\\n+\t\t--trailer \"Reviewed-by: Dev <dev@example.com>\" &&\n+\ttest_commit_message HEAD   first-signed &&\n+\ttest_commit_message HEAD^  initial-signed\n+'\n+test_done\n-- \n2.49.0\n\n"},{"id":"520063","messageId":"xmqq8qlzkukw.fsf@gitster.g","threadId":"63616","inReplyTo":"20250610123459.278582-2-me@linux.beauty","subject":"Re: [PATCH v2 1/2] trailer: append trailers in-process and drop the fork to `interpret-trailers`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-11T00:09:19Z","receivedAt":"2025-06-11T00:09:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Li Chen <me@linux.beauty> writes:\n\n> * `amend_strbuf_with_trailers()`\n>   – parses the existing message,\n>   – merges trailers from the command line and config,\n>   – formats the final trailer block, and\n>   – rewrites the supplied `struct strbuf`\n>   without ever leaving the current process.\n\nIf such a helper function exists, shouldn't \"interpret-trailers\" be\nable to lose quite a lot of lines, at least nearly as many as the\nnew lines introduced to this new function, by making it call this\nfunction as well?  And that would ensure that the internal call can\nsafely replace the external call and produce exactly the same\noutput?  If so, that can be a pure refactoring patch that can become\na commit on its own, I presume?\n\n> * `amend_file_with_trailers()` becomes a thin wrapper that reads a file\n>   into a `strbuf`, calls the new helper, and writes the result back.\n> * `builtin/rebase.c` now calls `amend_file_with_trailers()` instead of\n>   executing `interpret-trailers`.\n\nAnd then these two changes can become a separate patch on top?\n\n> -int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)\n\nThis lone \"removed line\" can be avoided if the patch did not touch\nthis line ...\n\n> +static size_t first_comment_pos(const struct strbuf *buf)\n>  {\n> -\tstruct child_process run_trailer = CHILD_PROCESS_INIT;\n> -\n> -\trun_trailer.git_cmd = 1;\n> -\tstrvec_pushl(&run_trailer.args, \"interpret-trailers\",\n> -\t\t     \"--in-place\", \"--no-divider\",\n> -\t\t     path, NULL);\n> -\tstrvec_pushv(&run_trailer.args, trailer_args->v);\n> -\treturn run_command(&run_trailer);\n> +\tconst char *p = buf->buf;\n> +\tconst char *end = buf->buf + buf->len;\n> +\n> +\twhile (p < end) {\n> +\t\tconst char *line = p;\n> +\t\tconst char *nl = memchr(p, '\\n', end - p);\n> +\t\tsize_t len = nl ? (size_t)(nl - p) : (size_t)(end - p);\n> +\n> +\t\t/* skip leading whitespace */\n> +\t\tsize_t i = 0;\n> +\t\twhile (i < len && isspace((unsigned char)line[i]))\n> +\t\t\ti++;\n> +\n> +\t\tif (i < len && line[i] == '#')\n> +\t\t\treturn (size_t)(line - buf->buf); /* comment starts here */\n> +\n> +\t\tif (!nl)              /* last line without newline */\n> +\t\t\tbreak;\n> +\t\tp = nl + 1;\n> +\t}\n> +\treturn buf->len;          /* no comment line found */\n> +}\n> +\n> +int amend_strbuf_with_trailers(struct strbuf *buf,\n> +\t\t\t\t\t\t\t   const struct strvec *trailer_args)\n\n... like this?\n"},{"id":"520080","messageId":"407631ab-1184-41b1-8043-fd91d860e845@gmail.com","threadId":"63616","inReplyTo":"xmqq8qlzkukw.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] trailer: append trailers in-process and drop the fork to `interpret-trailers`","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-06-11T09:15:26Z","receivedAt":"2025-06-11T09:15:33Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 11/06/2025 01:09, Junio C Hamano wrote:\n> Li Chen <me@linux.beauty> writes:\n> \n>> * `amend_strbuf_with_trailers()`\n>>    – parses the existing message,\n>>    – merges trailers from the command line and config,\n>>    – formats the final trailer block, and\n>>    – rewrites the supplied `struct strbuf`\n>>    without ever leaving the current process.\n> \n> If such a helper function exists, shouldn't \"interpret-trailers\" be\n> able to lose quite a lot of lines, at least nearly as many as the\n> new lines introduced to this new function, by making it call this\n> function as well?  And that would ensure that the internal call can\n> safely replace the external call and produce exactly the same\n> output?  If so, that can be a pure refactoring patch that can become\n> a commit on its own, I presume?\n\nExactly - I was expecting to see a refactoring of interpret_trailers() \nin builtin/interpret-trailers.c that moved most of the function body \ninto a new function in trailer.c that added the trailers to an strbuf. \nThis seems to be a parallel implementation which doesn't sound like the \nbest plan.\n\nI'm going to be off the list for a couple of weeks, I'll take a more \ndetailed look at this series when I'm back\n\nBest Wishes\n\nPhillip\n\n> \n>> * `amend_file_with_trailers()` becomes a thin wrapper that reads a file\n>>    into a `strbuf`, calls the new helper, and writes the result back.\n>> * `builtin/rebase.c` now calls `amend_file_with_trailers()` instead of\n>>    executing `interpret-trailers`.\n> \n> And then these two changes can become a separate patch on top?\n> \n>> -int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)\n> \n> This lone \"removed line\" can be avoided if the patch did not touch\n> this line ...\n> \n>> +static size_t first_comment_pos(const struct strbuf *buf)\n>>   {\n>> -\tstruct child_process run_trailer = CHILD_PROCESS_INIT;\n>> -\n>> -\trun_trailer.git_cmd = 1;\n>> -\tstrvec_pushl(&run_trailer.args, \"interpret-trailers\",\n>> -\t\t     \"--in-place\", \"--no-divider\",\n>> -\t\t     path, NULL);\n>> -\tstrvec_pushv(&run_trailer.args, trailer_args->v);\n>> -\treturn run_command(&run_trailer);\n>> +\tconst char *p = buf->buf;\n>> +\tconst char *end = buf->buf + buf->len;\n>> +\n>> +\twhile (p < end) {\n>> +\t\tconst char *line = p;\n>> +\t\tconst char *nl = memchr(p, '\\n', end - p);\n>> +\t\tsize_t len = nl ? (size_t)(nl - p) : (size_t)(end - p);\n>> +\n>> +\t\t/* skip leading whitespace */\n>> +\t\tsize_t i = 0;\n>> +\t\twhile (i < len && isspace((unsigned char)line[i]))\n>> +\t\t\ti++;\n>> +\n>> +\t\tif (i < len && line[i] == '#')\n>> +\t\t\treturn (size_t)(line - buf->buf); /* comment starts here */\n>> +\n>> +\t\tif (!nl)              /* last line without newline */\n>> +\t\t\tbreak;\n>> +\t\tp = nl + 1;\n>> +\t}\n>> +\treturn buf->len;          /* no comment line found */\n>> +}\n>> +\n>> +int amend_strbuf_with_trailers(struct strbuf *buf,\n>> +\t\t\t\t\t\t\t   const struct strvec *trailer_args)\n> \n> ... like this?\n\n"},{"id":"520107","messageId":"xmqqa56ejnrk.fsf@gitster.g","threadId":"63616","inReplyTo":"407631ab-1184-41b1-8043-fd91d860e845@gmail.com","subject":"Re: [PATCH v2 1/2] trailer: append trailers in-process and drop the fork to `interpret-trailers`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-11T15:34:07Z","receivedAt":"2025-06-11T15:34:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"phillip.wood123@gmail.com writes:\n\n> Exactly - I was expecting to see a refactoring of interpret_trailers()\n> in builtin/interpret-trailers.c that moved most of the function body\n> into a new function in trailer.c that added the trailers to an\n> strbuf. This seems to be a parallel implementation which doesn't sound\n> like the best plan.\n>\n> I'm going to be off the list for a couple of weeks, I'll take a more\n> detailed look at this series when I'm back\n\nThanks.\n"},{"id":"520164","messageId":"19762a71ab1.d16b43db461922.5615394257517989238@linux.beauty","threadId":"63616","inReplyTo":"xmqqa56ejnrk.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] trailer: append trailers in-process and drop the fork to `interpret-trailers`","fromName":"Li Chen","fromEmail":"me@linux.beauty","sentAt":"2025-06-12T05:40:07Z","receivedAt":"2025-06-12T05:40:18Z","isPatch":true,"sender":{"key":"me@linux.beauty","avatar":"https://avatars.githubusercontent.com/u/37442588?v=4"},"body":"Hi Junio & Phillip,\n\n\n ---- On Wed, 11 Jun 2025 23:34:07 +0800  Junio C Hamano <gitster@pobox.com> wrote --- \n > phillip.wood123@gmail.com writes:\n > \n > > Exactly - I was expecting to see a refactoring of interpret_trailers()\n > > in builtin/interpret-trailers.c that moved most of the function body\n > > into a new function in trailer.c that added the trailers to an\n > > strbuf. This seems to be a parallel implementation which doesn't sound\n > > like the best plan.\n > >\n > > I'm going to be off the list for a couple of weeks, I'll take a more\n > > detailed look at this series when I'm back\n\nThanks for your suggestion; I will refactor it in the next version.\n\nRegards,\nLi\n"}]}