{"thread":{"id":"65068","subject":"[PATCH v7 0/5] rebase: support --trailer","startedAt":"2026-02-24T07:06:20Z","lastAt":"2026-03-09T10:36:57Z","messageCount":32,"participants":["Li Chen","Junio C Hamano","Phillip Wood","Kristoffer Haugsbakk","phillip.wood123@gmail.com"],"isPatch":true,"patchVersion":7,"patchTotal":5},"messages":[{"id":"536922","messageId":"20260224070552.148591-1-me@linux.beauty","threadId":"65068","inReplyTo":null,"subject":"[PATCH v7 0/5] rebase: support --trailer","fromName":"Li Chen","fromEmail":"me@linux.beauty","sentAt":"2026-02-24T07:05:46Z","receivedAt":"2026-02-24T07:06:20Z","isPatch":true,"sender":{"key":"me@linux.beauty","avatar":"https://avatars.githubusercontent.com/u/37442588?v=4"},"body":"Apologies for the long delay in sending v7.\n\nv7 is based on origin/master at v2.53.0-154-g7c02d39fc2.\n\nThis series routes trailer insertion through an in-process path, removing the\nfork/exec to builtin/interpret-trailers.\n\nThe first four commits refactor trailer rewriting in builtin/interpret-trailers\nand trailer.c so callers can reuse a single in-process helper (used by git\ninterpret-trailers, git commit and git tag). The final commit adds git rebase\n--trailer, currently supported with the merge backend only (rejecting apply-only\nscenarios and validating input early).\n\nv7:\nRebased onto origin/master at v2.53.0-154-g7c02d39fc2.\nSplit out a new patch to parse --trailer with OPT_STRVEC in git commit and git\ntag.\nUse strbuf_write() in interpret-trailers when emitting buffered output.\nRestore --in-place rewriting semantics via tempfile+rename.\nDrop wrapper.c/h and validate trailer args via validate_trailer_args().\nDrop redundant rebase basic-state save/restore for --trailer arguments.\n\nv6: squash all fix commits and split refactor step from the original patch\nbased on Phillip's suggestion and code [4].\nv5: fix all Kristoffer's review comments from v4[3] in place and without new\npatches.\nv4: fix all reviewer comments in v3. [2], and add patch 1~8 & 10~29 to fix\nreview comments.\nv3: merges the remaining trailer paths into one in-process helper, dropping the\nduplicate code, as pointed by Junio and Phillip [1]\nv2: fix issues pointed by Phillip\nRFC link:\nhttps://lore.kernel.org/git/196a5ac1393.f5b4db7d187309.2451613571977217927@linux.beauty/\n\nComments very very welcome!\n\n[1]: https://lore.kernel.org/git/xmqq8qlzkukw.fsf@gitster.g/\n[2]: https://lore.kernel.org/git/20250803150059.402017-1-me@linux.beauty/\n[3]: https://lore.kernel.org/git/20251014122452.1851103-1-me@linux.beauty/\n[4]: https://lore.kernel.org/git/7d12b046-365f-441c-af8e-8a39d61efbbd@gmail.com/\n\nLi Chen (5):\n interpret-trailers: factor trailer rewriting\n trailer: move process_trailers to trailer.h\n trailer: append trailers without fork/exec\n commit, tag: parse --trailer with OPT_STRVEC\n rebase: support --trailer\n\n Documentation/git-rebase.adoc | 7 ++\n builtin/commit.c | 3 +-\n builtin/interpret-trailers.c | 47 ++-------\n builtin/rebase.c | 18 ++++\n builtin/tag.c | 4 +-\n sequencer.c | 28 ++++++\n sequencer.h | 3 +\n t/meson.build | 1 +\n t/t3440-rebase-trailer.sh | 147 ++++++++++++++++++++++++++++\n trailer.c | 179 ++++++++++++++++++++++++++++++++--\n trailer.h | 30 +++++-\n 11 files changed, 414 insertions(+), 53 deletions(-)\n create mode 100755 t/t3440-rebase-trailer.sh\n\n-- \n2.52.0\n"},{"id":"536923","messageId":"20260224070552.148591-2-me@linux.beauty","threadId":"65068","inReplyTo":"20260224070552.148591-1-me@linux.beauty","subject":"[PATCH v7 1/5] interpret-trailers: factor trailer rewriting","fromName":"Li Chen","fromEmail":"me@linux.beauty","sentAt":"2026-02-24T07:05:47Z","receivedAt":"2026-02-24T07:06:30Z","isPatch":true,"sender":{"key":"me@linux.beauty","avatar":"https://avatars.githubusercontent.com/u/37442588?v=4"},"body":"Extract the trailer rewriting logic into a helper that appends to an\noutput strbuf.\n\nUpdate interpret_trailers() to handle file I/O only: read input once,\ncall the helper, and write the buffered result.\n\nThis separation makes it easier to move the helper into trailer.c in the\nnext commit.\n\nSigned-off-by: Li Chen \n---\nv7:\nUse strbuf_write() when emitting buffered output.\n\n builtin/interpret-trailers.c | 53 ++++++++++++++++++++----------------\n 1 file changed, 30 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex 41b0750e5a..69f9d67ec0 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -136,32 +136,21 @@ static void read_input_file(struct strbuf *sb, const char *file)\n \tstrbuf_complete_line(sb);\n }\n \n-static void interpret_trailers(const struct process_trailer_options *opts,\n-\t\t\t struct list_head *new_trailer_head,\n-\t\t\t const char *file)\n+static void process_trailers(const struct process_trailer_options *opts,\n+\t\t\t struct list_head *new_trailer_head,\n+\t\t\t struct strbuf *input, struct strbuf *out)\n {\n \tLIST_HEAD(head);\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tstruct strbuf trailer_block_sb = STRBUF_INIT;\n \tstruct trailer_block *trailer_block;\n-\tFILE *outfile = stdout;\n \n-\ttrailer_config_init();\n-\n-\tread_input_file(&sb, file);\n-\n-\tif (opts->in_place)\n-\t\toutfile = create_in_place_tempfile(file);\n-\n-\ttrailer_block = parse_trailers(opts, sb.buf, &head);\n+\ttrailer_block = parse_trailers(opts, input->buf, &head);\n \n \t/* Print the lines before the trailer block */\n \tif (!opts->only_trailers)\n-\t\tfwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);\n+\t\tstrbuf_add(out, input->buf, trailer_block_start(trailer_block));\n \n \tif (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))\n-\t\tfprintf(outfile, \"\\n\");\n-\n+\t\tstrbuf_addch(out, '\\n');\n \n \tif (!opts->only_input) {\n \t\tLIST_HEAD(config_head);\n@@ -173,22 +162,40 @@ static void interpret_trailers(const struct process_trailer_options *opts,\n \t}\n \n \t/* Print trailer block. */\n-\tformat_trailers(opts, &head, &trailer_block_sb);\n+\tformat_trailers(opts, &head, out);\n \tfree_trailers(&head);\n-\tfwrite(trailer_block_sb.buf, 1, trailer_block_sb.len, outfile);\n-\tstrbuf_release(&trailer_block_sb);\n \n \t/* Print the lines after the trailer block as is. */\n \tif (!opts->only_trailers)\n-\t\tfwrite(sb.buf + trailer_block_end(trailer_block), 1,\n-\t\t sb.len - trailer_block_end(trailer_block), outfile);\n+\t\tstrbuf_add(out, input->buf + trailer_block_end(trailer_block),\n+\t\t\t input->len - trailer_block_end(trailer_block));\n \ttrailer_block_release(trailer_block);\n+}\n+\n+static void interpret_trailers(const struct process_trailer_options *opts,\n+\t\t\t struct list_head *new_trailer_head,\n+\t\t\t const char *file)\n+{\n+\tstruct strbuf input = STRBUF_INIT;\n+\tstruct strbuf out = STRBUF_INIT;\n+\tFILE *outfile = stdout;\n+\n+\ttrailer_config_init();\n+\n+\tread_input_file(&input, file);\n+\n+\tif (opts->in_place)\n+\t\toutfile = create_in_place_tempfile(file);\n+\n+\tprocess_trailers(opts, new_trailer_head, &input, &out);\n \n+\tstrbuf_write(&out, outfile);\n \tif (opts->in_place)\n \t\tif (rename_tempfile(&trailers_tempfile, file))\n \t\t\tdie_errno(_(\"could not rename temporary file to %s\"), file);\n \n-\tstrbuf_release(&sb);\n+\tstrbuf_release(&input);\n+\tstrbuf_release(&out);\n }\n \n int cmd_interpret_trailers(int argc,\n-- \n2.52.0\n"},{"id":"536924","messageId":"20260224070552.148591-3-me@linux.beauty","threadId":"65068","inReplyTo":"20260224070552.148591-1-me@linux.beauty","subject":"[PATCH v7 2/5] trailer: move process_trailers to trailer.h","fromName":"Li Chen","fromEmail":"me@linux.beauty","sentAt":"2026-02-24T07:05:48Z","receivedAt":"2026-02-24T07:06:40Z","isPatch":true,"sender":{"key":"me@linux.beauty","avatar":"https://avatars.githubusercontent.com/u/37442588?v=4"},"body":"Move process_trailers() from builtin/interpret-trailers.c into trailer.c\nand expose it via trailer.h.\n\nThis lets other call sites reuse the same trailer rewriting logic.\n\nSigned-off-by: Li Chen \n---\nv7:\nRename the input parameter from sb to input.\n\n builtin/interpret-trailers.c | 36 ------------------------------------\n trailer.c | 36 ++++++++++++++++++++++++++++++++++++\n trailer.h | 3 +++\n 3 files changed, 39 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex 69f9d67ec0..1354109e0f 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -136,42 +136,6 @@ static void read_input_file(struct strbuf *sb, const char *file)\n \tstrbuf_complete_line(sb);\n }\n \n-static void process_trailers(const struct process_trailer_options *opts,\n-\t\t\t struct list_head *new_trailer_head,\n-\t\t\t struct strbuf *input, struct strbuf *out)\n-{\n-\tLIST_HEAD(head);\n-\tstruct trailer_block *trailer_block;\n-\n-\ttrailer_block = parse_trailers(opts, input->buf, &head);\n-\n-\t/* Print the lines before the trailer block */\n-\tif (!opts->only_trailers)\n-\t\tstrbuf_add(out, input->buf, trailer_block_start(trailer_block));\n-\n-\tif (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))\n-\t\tstrbuf_addch(out, '\\n');\n-\n-\tif (!opts->only_input) {\n-\t\tLIST_HEAD(config_head);\n-\t\tLIST_HEAD(arg_head);\n-\t\tparse_trailers_from_config(&config_head);\n-\t\tparse_trailers_from_command_line_args(&arg_head, new_trailer_head);\n-\t\tlist_splice(&config_head, &arg_head);\n-\t\tprocess_trailers_lists(&head, &arg_head);\n-\t}\n-\n-\t/* Print trailer block. */\n-\tformat_trailers(opts, &head, out);\n-\tfree_trailers(&head);\n-\n-\t/* Print the lines after the trailer block as is. */\n-\tif (!opts->only_trailers)\n-\t\tstrbuf_add(out, input->buf + trailer_block_end(trailer_block),\n-\t\t\t input->len - trailer_block_end(trailer_block));\n-\ttrailer_block_release(trailer_block);\n-}\n-\n static void interpret_trailers(const struct process_trailer_options *opts,\n \t\t\t struct list_head *new_trailer_head,\n \t\t\t const char *file)\ndiff --git a/trailer.c b/trailer.c\nindex 911a81ed99..0c9200506d 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -1235,3 +1235,39 @@ int amend_file_with_trailers(const char *path, const struct strvec *trailer_args\n \tstrvec_pushv(&run_trailer.args, trailer_args->v);\n \treturn run_command(&run_trailer);\n }\n+\n+void process_trailers(const struct process_trailer_options *opts,\n+\t\t struct list_head *new_trailer_head,\n+\t\t struct strbuf *input, struct strbuf *out)\n+{\n+\tLIST_HEAD(head);\n+\tstruct trailer_block *trailer_block;\n+\n+\ttrailer_block = parse_trailers(opts, input->buf, &head);\n+\n+\t/* Print the lines before the trailer block */\n+\tif (!opts->only_trailers)\n+\t\tstrbuf_add(out, input->buf, trailer_block_start(trailer_block));\n+\n+\tif (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))\n+\t\tstrbuf_addch(out, '\\n');\n+\n+\tif (!opts->only_input) {\n+\t\tLIST_HEAD(config_head);\n+\t\tLIST_HEAD(arg_head);\n+\t\tparse_trailers_from_config(&config_head);\n+\t\tparse_trailers_from_command_line_args(&arg_head, new_trailer_head);\n+\t\tlist_splice(&config_head, &arg_head);\n+\t\tprocess_trailers_lists(&head, &arg_head);\n+\t}\n+\n+\t/* Print trailer block. */\n+\tformat_trailers(opts, &head, out);\n+\tfree_trailers(&head);\n+\n+\t/* Print the lines after the trailer block as is. */\n+\tif (!opts->only_trailers)\n+\t\tstrbuf_add(out, input->buf + trailer_block_end(trailer_block),\n+\t\t\t input->len - trailer_block_end(trailer_block));\n+\ttrailer_block_release(trailer_block);\n+}\ndiff --git a/trailer.h b/trailer.h\nindex 4740549586..531fa1a13f 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -202,4 +202,7 @@ 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+void process_trailers(const struct process_trailer_options *opts,\n+\t\t struct list_head *new_trailer_head,\n+\t\t struct strbuf *input, struct strbuf *out);\n #endif /* TRAILER_H */\n-- \n2.52.0\n"},{"id":"536925","messageId":"20260224070552.148591-4-me@linux.beauty","threadId":"65068","inReplyTo":"20260224070552.148591-1-me@linux.beauty","subject":"[PATCH v7 3/5] trailer: append trailers without fork/exec","fromName":"Li Chen","fromEmail":"me@linux.beauty","sentAt":"2026-02-24T07:05:49Z","receivedAt":"2026-02-24T07:06:51Z","isPatch":true,"sender":{"key":"me@linux.beauty","avatar":"https://avatars.githubusercontent.com/u/37442588?v=4"},"body":"Introduce amend_strbuf_with_trailers() to apply trailer additions to a\nmessage buffer via process_trailers(), avoiding the need to run git\ninterpret-trailers as a child process.\n\nUpdate amend_file_with_trailers() to use the in-process helper and\nrewrite the target file via tempfile+rename, preserving the previous\nin-place semantics.\n\nKeep existing callers unchanged by continuing to accept argv-style\n--trailer= entries and stripping the prefix before feeding the\nin-process implementation.\n\nSigned-off-by: Li Chen \n---\nv7:\nDrop wrapper.c/h and validate trailer args via validate_trailer_args().\nRewrite the target file via tempfile+rename to preserve --in-place semantics.\n\n builtin/interpret-trailers.c | 4 +-\n trailer.c | 162 +++++++++++++++++++++++++++++++++--\n trailer.h | 27 +++++-\n 3 files changed, 179 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex 1354109e0f..d4aff68746 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -144,8 +144,6 @@ static void interpret_trailers(const struct process_trailer_options *opts,\n \tstruct strbuf out = STRBUF_INIT;\n \tFILE *outfile = stdout;\n \n-\ttrailer_config_init();\n-\n \tread_input_file(&input, file);\n \n \tif (opts->in_place)\n@@ -203,6 +201,8 @@ int cmd_interpret_trailers(int argc,\n \t\t\tgit_interpret_trailers_usage,\n \t\t\toptions);\n \n+\ttrailer_config_init();\n+\n \tif (argc) {\n \t\tint i;\n \t\tfor (i = 0; i < argc; i++)\ndiff --git a/trailer.c b/trailer.c\nindex 0c9200506d..8e87d185d9 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -7,8 +7,11 @@\n #include \"string-list.h\"\n #include \"run-command.h\"\n #include \"commit.h\"\n+#include \"strvec.h\"\n+#include \"tempfile.h\"\n #include \"trailer.h\"\n #include \"list.h\"\n+\n /*\n * Copyright (c) 2013, 2014 Christian Couder \n */\n@@ -772,6 +775,30 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,\n \tfree(cl_separators);\n }\n \n+void validate_trailer_args(const struct strvec *cli_args)\n+{\n+\tchar *cl_separators;\n+\n+\ttrailer_config_init();\n+\n+\tcl_separators = xstrfmt(\"=%s\", separators);\n+\n+\tfor (size_t i = 0; i < cli_args->nr; i++) {\n+\t\tconst char *txt = cli_args->v[i];\n+\t\tssize_t separator_pos;\n+\n+\t\tif (!*txt)\n+\t\t\tdie(_(\"empty --trailer argument\"));\n+\n+\t\tseparator_pos = find_separator(txt, cl_separators);\n+\t\tif (separator_pos == 0)\n+\t\t\tdie(_(\"invalid trailer '%s': missing key before separator\"),\n+\t\t\t txt);\n+\t}\n+\n+\tfree(cl_separators);\n+}\n+\n static const char *next_line(const char *str)\n {\n \tconst char *nl = strchrnul(str, '\\n');\n@@ -1224,16 +1251,133 @@ 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 void new_trailer_items_clear(struct list_head *items)\n+{\n+\twhile (!list_empty(items)) {\n+\t\tstruct new_trailer_item *item =\n+\t\t\tlist_first_entry(items, struct new_trailer_item, list);\n+\t\tlist_del(&item->list);\n+\t\tfree(item);\n+\t}\n+}\n+\n+void amend_strbuf_with_trailers(struct strbuf *buf,\n+\t\t\t\tconst struct strvec *trailer_args)\n+{\n+\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n+\tLIST_HEAD(new_trailer_head);\n+\tstruct strbuf out = STRBUF_INIT;\n+\tsize_t i;\n+\n+\topts.no_divider = 1;\n+\n+\tfor (i = 0; i < trailer_args->nr; i++) {\n+\t\tconst char *text = trailer_args->v[i];\n+\t\tstruct new_trailer_item *item;\n+\n+\t\tif (!*text)\n+\t\t\tdie(_(\"empty --trailer argument\"));\n+\t\titem = xcalloc(1, sizeof(*item));\n+\t\titem->text = text;\n+\t\tlist_add_tail(&item->list, &new_trailer_head);\n+\t}\n+\n+\ttrailer_config_init();\n+\tprocess_trailers(&opts, &new_trailer_head, buf, &out);\n+\n+\tstrbuf_swap(buf, &out);\n+\tstrbuf_release(&out);\n+\n+\tnew_trailer_items_clear(&new_trailer_head);\n+}\n+\n+static int write_file_in_place(const char *path, const struct strbuf *buf)\n+{\n+\tstruct stat st;\n+\tstruct strbuf filename_template = STRBUF_INIT;\n+\tconst char *tail;\n+\tstruct tempfile *tempfile;\n+\tFILE *outfile;\n+\n+\tif (stat(path, &st))\n+\t\treturn error_errno(_(\"could not stat %s\"), path);\n+\tif (!S_ISREG(st.st_mode))\n+\t\treturn error(_(\"file %s is not a regular file\"), path);\n+\tif (!(st.st_mode & S_IWUSR))\n+\t\treturn error(_(\"file %s is not writable by user\"), path);\n+\n+\t/* Create temporary file in the same directory as the original */\n+\ttail = strrchr(path, '/');\n+\tif (tail)\n+\t\tstrbuf_add(&filename_template, path, tail - path + 1);\n+\tstrbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n+\n+\ttempfile = mks_tempfile_sm(filename_template.buf, 0, st.st_mode);\n+\tstrbuf_release(&filename_template);\n+\tif (!tempfile)\n+\t\treturn error_errno(_(\"could not create temporary file\"));\n+\n+\toutfile = fdopen_tempfile(tempfile, \"w\");\n+\tif (!outfile) {\n+\t\tint saved_errno = errno;\n+\t\tdelete_tempfile(&tempfile);\n+\t\terrno = saved_errno;\n+\t\treturn error_errno(_(\"could not open temporary file\"));\n+\t}\n+\n+\tif (buf->len && fwrite(buf->buf, 1, buf->len, outfile) < buf->len) {\n+\t\tint saved_errno = errno;\n+\t\tdelete_tempfile(&tempfile);\n+\t\terrno = saved_errno;\n+\t\treturn error_errno(_(\"could not write to temporary file\"));\n+\t}\n+\n+\tif (rename_tempfile(&tempfile, path))\n+\t\treturn error_errno(_(\"could not rename temporary file to %s\"), path);\n+\n+\treturn 0;\n+}\n+\n+int amend_file_with_trailers(const char *path,\n+\t\t\t const struct strvec *trailer_args)\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+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strvec stripped_trailer_args = STRVEC_INIT;\n+\tint ret = 0;\n+\tsize_t i;\n+\n+\tif (!trailer_args)\n+\t\tBUG(\"amend_file_with_trailers called with NULL trailer_args\");\n+\tif (!trailer_args->nr)\n+\t\treturn 0;\n+\n+\tfor (i = 0; i < trailer_args->nr; i++) {\n+\t\tconst char *txt = trailer_args->v[i];\n+\n+\t\t/*\n+\t\t * Historically amend_file_with_trailers() passed its arguments\n+\t\t * to \"git interpret-trailers\", which expected argv entries in\n+\t\t * \"--trailer=\" form. Continue to accept those for\n+\t\t * existing callers, but pass only the value portion to the\n+\t\t * in-process implementation.\n+\t\t */\n+\t\tskip_prefix(txt, \"--trailer=\", &txt);\n+\t\tif (!*txt)\n+\t\t\tdie(_(\"empty --trailer argument\"));\n+\t\tstrvec_push(&stripped_trailer_args, txt);\n+\t}\n+\n+\tif (strbuf_read_file(&buf, path, 0) < 0)\n+\t\tret = error_errno(_(\"could not read '%s'\"), path);\n+\telse\n+\t\tamend_strbuf_with_trailers(&buf, &stripped_trailer_args);\n+\n+\tif (!ret)\n+\t\tret = write_file_in_place(path, &buf);\n+\n+\tstrvec_clear(&stripped_trailer_args);\n+\tstrbuf_release(&buf);\n+\treturn ret;\n }\n \n void process_trailers(const struct process_trailer_options *opts,\ndiff --git a/trailer.h b/trailer.h\nindex 531fa1a13f..d05dab050b 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -68,6 +68,8 @@ void parse_trailers_from_config(struct list_head *config_head);\n void parse_trailers_from_command_line_args(struct list_head *arg_head,\n \t\t\t\t\t struct list_head *new_trailer_head);\n \n+void validate_trailer_args(const struct strvec *cli_args);\n+\n void process_trailers_lists(struct list_head *head,\n \t\t\t struct list_head *arg_head);\n \n@@ -196,12 +198,31 @@ int trailer_iterator_advance(struct trailer_iterator *iter);\n void trailer_iterator_release(struct trailer_iterator *iter);\n \n /*\n- * Augment a file to add trailers to it by running git-interpret-trailers.\n- * This calls run_command() and its return value is the same (i.e. 0 for\n- * success, various non-zero for other errors). See run-command.h.\n+ * Append trailers specified in trailer_args to buf in-place.\n+ *\n+ * Each element of trailer_args should be in the same format as the value\n+ * accepted by --trailer= (i.e., without the --trailer= prefix).\n+ */\n+void amend_strbuf_with_trailers(struct strbuf *buf,\n+\t\t\t\tconst struct strvec *trailer_args);\n+\n+/*\n+ * Augment a file by appending trailers specified in trailer_args.\n+ *\n+ * Each element of trailer_args should be an argv-style --trailer=\n+ * option (i.e., including the --trailer= prefix).\n+ *\n+ * Returns 0 on success or a non-zero error code on failure.\n */\n int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);\n \n+/*\n+ * Rewrite the contents of input by processing its trailer block according to\n+ * opts and (optionally) appending trailers from new_trailer_head.\n+ *\n+ * The rewritten message is appended to out (callers should strbuf_reset()\n+ * first if needed).\n+ */\n void process_trailers(const struct process_trailer_options *opts,\n \t\t struct list_head *new_trailer_head,\n \t\t struct strbuf *input, struct strbuf *out);\n-- \n2.52.0\n"},{"id":"536926","messageId":"20260224070552.148591-5-me@linux.beauty","threadId":"65068","inReplyTo":"20260224070552.148591-1-me@linux.beauty","subject":"[PATCH v7 4/5] commit, tag: parse --trailer with OPT_STRVEC","fromName":"Li Chen","fromEmail":"me@linux.beauty","sentAt":"2026-02-24T07:05:50Z","receivedAt":"2026-02-24T07:07:01Z","isPatch":true,"sender":{"key":"me@linux.beauty","avatar":"https://avatars.githubusercontent.com/u/37442588?v=4"},"body":"Now that amend_file_with_trailers() expects raw trailer lines, do not\nstore argv-style \"--trailer=\" strings in git commit and git\ntag.\n\nParse --trailer using OPT_STRVEC so trailer_args contains only the\ntrailer value, and drop the temporary prefix stripping in\namend_file_with_trailers().\n\nSigned-off-by: Li Chen \n---\nv7:\nNew patch.\n\n builtin/commit.c | 3 ++-\n builtin/tag.c | 4 ++--\n trailer.c | 21 +--------------------\n trailer.h | 4 ++--\n 4 files changed, 7 insertions(+), 25 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 9e3a09d532..d9983230de 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1720,7 +1720,8 @@ int cmd_commit(int argc,\n \t\tOPT_STRING(0, \"fixup\", &fixup_message, N_(\"[(amend|reword):]commit\"), N_(\"use autosquash formatted message to fixup or amend/reword specified commit\")),\n \t\tOPT_STRING(0, \"squash\", &squash_message, N_(\"commit\"), N_(\"use autosquash formatted message to squash specified commit\")),\n \t\tOPT_BOOL(0, \"reset-author\", &renew_authorship, N_(\"the commit is authored by me now (used with -C/-c/--amend)\")),\n-\t\tOPT_PASSTHRU_ARGV(0, \"trailer\", &trailer_args, N_(\"trailer\"), N_(\"add custom trailer(s)\"), PARSE_OPT_NONEG),\n+\t\tOPT_STRVEC(0, \"trailer\", &trailer_args, N_(\"trailer\"),\n+\t\t\t N_(\"add custom trailer(s)\")),\n \t\tOPT_BOOL('s', \"signoff\", &signoff, N_(\"add a Signed-off-by trailer\")),\n \t\tOPT_FILENAME('t', \"template\", &template_file, N_(\"use specified template file\")),\n \t\tOPT_BOOL('e', \"edit\", &edit_flag, N_(\"force edit of commit\")),\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex aeb04c487f..15aee1b03a 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -499,8 +499,8 @@ int cmd_tag(int argc,\n \t\tOPT_CALLBACK_F('m', \"message\", &msg, N_(\"message\"),\n \t\t\t N_(\"tag message\"), PARSE_OPT_NONEG, parse_msg_arg),\n \t\tOPT_FILENAME('F', \"file\", &msgfile, N_(\"read message from file\")),\n-\t\tOPT_PASSTHRU_ARGV(0, \"trailer\", &trailer_args, N_(\"trailer\"),\n-\t\t\t\t N_(\"add custom trailer(s)\"), PARSE_OPT_NONEG),\n+\t\tOPT_STRVEC(0, \"trailer\", &trailer_args, N_(\"trailer\"),\n+\t\t\t N_(\"add custom trailer(s)\")),\n \t\tOPT_BOOL('e', \"edit\", &edit_flag, N_(\"force edit of tag message\")),\n \t\tOPT_BOOL('s', \"sign\", &opt.sign, N_(\"annotated and GPG-signed tag\")),\n \t\tOPT_CLEANUP(&cleanup_arg),\ndiff --git a/trailer.c b/trailer.c\nindex 8e87d185d9..e85c6c9fbe 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -1342,40 +1342,21 @@ int amend_file_with_trailers(const char *path,\n \t\t\t const struct strvec *trailer_args)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n-\tstruct strvec stripped_trailer_args = STRVEC_INIT;\n \tint ret = 0;\n-\tsize_t i;\n \n \tif (!trailer_args)\n \t\tBUG(\"amend_file_with_trailers called with NULL trailer_args\");\n \tif (!trailer_args->nr)\n \t\treturn 0;\n \n-\tfor (i = 0; i < trailer_args->nr; i++) {\n-\t\tconst char *txt = trailer_args->v[i];\n-\n-\t\t/*\n-\t\t * Historically amend_file_with_trailers() passed its arguments\n-\t\t * to \"git interpret-trailers\", which expected argv entries in\n-\t\t * \"--trailer=\" form. Continue to accept those for\n-\t\t * existing callers, but pass only the value portion to the\n-\t\t * in-process implementation.\n-\t\t */\n-\t\tskip_prefix(txt, \"--trailer=\", &txt);\n-\t\tif (!*txt)\n-\t\t\tdie(_(\"empty --trailer argument\"));\n-\t\tstrvec_push(&stripped_trailer_args, txt);\n-\t}\n-\n \tif (strbuf_read_file(&buf, path, 0) < 0)\n \t\tret = error_errno(_(\"could not read '%s'\"), path);\n \telse\n-\t\tamend_strbuf_with_trailers(&buf, &stripped_trailer_args);\n+\t\tamend_strbuf_with_trailers(&buf, trailer_args);\n \n \tif (!ret)\n \t\tret = write_file_in_place(path, &buf);\n \n-\tstrvec_clear(&stripped_trailer_args);\n \tstrbuf_release(&buf);\n \treturn ret;\n }\ndiff --git a/trailer.h b/trailer.h\nindex d05dab050b..e5bd355aad 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -209,8 +209,8 @@ void amend_strbuf_with_trailers(struct strbuf *buf,\n /*\n * Augment a file by appending trailers specified in trailer_args.\n *\n- * Each element of trailer_args should be an argv-style --trailer=\n- * option (i.e., including the --trailer= prefix).\n+ * Each element of trailer_args should be in the same format as the value\n+ * accepted by --trailer= (i.e., without the --trailer= prefix).\n *\n * Returns 0 on success or a non-zero error code on failure.\n */\n-- \n2.52.0\n"},{"id":"536927","messageId":"20260224070552.148591-6-me@linux.beauty","threadId":"65068","inReplyTo":"20260224070552.148591-1-me@linux.beauty","subject":"[PATCH v7 5/5] rebase: support --trailer","fromName":"Li Chen","fromEmail":"me@linux.beauty","sentAt":"2026-02-24T07:05:51Z","receivedAt":"2026-02-24T07:07:12Z","isPatch":true,"sender":{"key":"me@linux.beauty","avatar":"https://avatars.githubusercontent.com/u/37442588?v=4"},"body":"Add a new --trailer= option to git rebase to append trailer\nlines to each rewritten commit message (merge backend only).\n\nBecause the apply backend does not provide a commit-message filter,\nreject --trailer when --apply is in effect and require the merge backend\ninstead.\n\nThis option implies --force-rebase so that fast-forwarded commits are\nalso rewritten. Validate trailer arguments early to avoid starting an\ninteractive rebase with invalid input.\n\nAdd integration tests covering error paths and trailer insertion across\nnon-interactive and interactive rebases.\n\nSigned-off-by: Li Chen \n---\nv7:\nValidate trailer args via validate_trailer_args().\nDrop redundant rebase basic-state save/restore for --trailer arguments.\nFix Documentation/git-rebase.adoc formatting for the new option.\n\n Documentation/git-rebase.adoc | 7 ++\n builtin/rebase.c | 18 +++++\n sequencer.c | 28 +++++++\n sequencer.h | 3 +\n t/meson.build | 1 +\n t/t3440-rebase-trailer.sh | 147 ++++++++++++++++++++++++++++++++++\n 6 files changed, 204 insertions(+)\n create mode 100755 t/t3440-rebase-trailer.sh\n\ndiff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc\nindex e177808004..908717991a 100644\n--- a/Documentation/git-rebase.adoc\n+++ b/Documentation/git-rebase.adoc\n@@ -497,6 +497,13 @@ See also INCOMPATIBLE OPTIONS below.\n +\n See also INCOMPATIBLE OPTIONS below.\n \n+--trailer=::\n+\tAppend the given trailer to every rebased commit message, processed\n+\tvia linkgit:git-interpret-trailers[1]. This option implies\n+\t`--force-rebase` so that fast-forwarded commits are also rewritten.\n++\n+See also INCOMPATIBLE OPTIONS below.\n+\n -i::\n --interactive::\n \tMake a list of the commits which are about to be rebased. Let the\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex c487e10907..3200506c89 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -36,6 +36,7 @@\n #include \"reset.h\"\n #include \"trace2.h\"\n #include \"hook.h\"\n+#include \"trailer.h\"\n \n static char const * const builtin_rebase_usage[] = {\n \tN_(\"git rebase [-i] [options] [--exec ] \"\n@@ -113,6 +114,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 +145,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,\t\t\\\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 +169,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 +181,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@@ -1132,6 +1140,8 @@ 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+\t\t\t 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@@ -1285,6 +1295,11 @@ int cmd_rebase(int argc,\n \t\t\t builtin_rebase_options,\n \t\t\t builtin_rebase_usage, 0);\n \n+\tif (options.trailer_args.nr) {\n+\t\tvalidate_trailer_args(&options.trailer_args);\n+\t\toptions.flags |= REBASE_FORCE;\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@@ -1542,6 +1557,9 @@ int cmd_rebase(int argc,\n \tif (options.root && !options.onto_name)\n \t\timply_merge(&options, \"--root without --onto\");\n \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 a3eb39bb25..a60c2a0cde 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -209,6 +209,7 @@ static GIT_PATH_FUNC(rebase_path_reschedule_failed_exec, \"rebase-merge/reschedul\n static GIT_PATH_FUNC(rebase_path_no_reschedule_failed_exec, \"rebase-merge/no-reschedule-failed-exec\")\n static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, \"rebase-merge/drop_redundant_commits\")\n static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, \"rebase-merge/keep_redundant_commits\")\n+static GIT_PATH_FUNC(rebase_path_trailer, \"rebase-merge/trailer\")\n \n /*\n * A 'struct replay_ctx' represents the private state of the sequencer.\n@@ -420,6 +421,7 @@ void replay_opts_release(struct replay_opts *opts)\n \tif (opts->revs)\n \t\trelease_revisions(opts->revs);\n \tfree(opts->revs);\n+\tstrvec_clear(&opts->trailer_args);\n \treplay_ctx_release(ctx);\n \tfree(opts->ctx);\n }\n@@ -2025,6 +2027,9 @@ static int append_squash_message(struct strbuf *buf, const char *body,\n \t\tif (opts->signoff)\n \t\t\tappend_signoff(buf, 0, 0);\n \n+\t\tif (opts->trailer_args.nr)\n+\t\t\tamend_strbuf_with_trailers(buf, &opts->trailer_args);\n+\n \t\tif ((command == TODO_FIXUP) &&\n \t\t (flag & TODO_REPLACE_FIXUP_MSG) &&\n \t\t (file_exists(rebase_path_fixup_msg()) ||\n@@ -2443,6 +2448,9 @@ static int do_pick_commit(struct repository *r,\n \tif (opts->signoff && !is_fixup(command))\n \t\tappend_signoff(&ctx->message, 0, 0);\n \n+\tif (opts->trailer_args.nr && !is_fixup(command))\n+\t\tamend_strbuf_with_trailers(&ctx->message, &opts->trailer_args);\n+\n \tif (is_rebase_i(opts) && write_author_script(msg.message) < 0)\n \t\tres = -1;\n \telse if (!opts->strategy ||\n@@ -3234,6 +3242,18 @@ static int read_populate_opts(struct replay_opts *opts)\n \n \t\tread_strategy_opts(opts, &buf);\n \t\tstrbuf_reset(&buf);\n+\t\tif (strbuf_read_file(&buf, rebase_path_trailer(), 0) >= 0) {\n+\t\t\tchar *p = buf.buf, *nl;\n+\n+\t\t\twhile ((nl = strchr(p, '\\n'))) {\n+\t\t\t\t*nl = '\\0';\n+\t\t\t\tif (!*p)\n+\t\t\t\t\tBUG(\"rebase-merge/trailer has an empty line\");\n+\t\t\t\tstrvec_push(&opts->trailer_args, p);\n+\t\t\t\tp = nl + 1;\n+\t\t\t}\n+\t\t\tstrbuf_reset(&buf);\n+\t\t}\n \n \t\tif (read_oneliner(&ctx->current_fixups,\n \t\t\t\t rebase_path_current_fixups(),\n@@ -3328,6 +3348,14 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,\n \t\twrite_file(rebase_path_reschedule_failed_exec(), \"%s\", \"\");\n \telse\n \t\twrite_file(rebase_path_no_reschedule_failed_exec(), \"%s\", \"\");\n+\tif (opts->trailer_args.nr) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\n+\t\tfor (size_t i = 0; i < opts->trailer_args.nr; i++)\n+\t\t\tstrbuf_addf(&buf, \"%s\\n\", opts->trailer_args.v[i]);\n+\t\twrite_file(rebase_path_trailer(), \"%s\", buf.buf);\n+\t\tstrbuf_release(&buf);\n+\t}\n \n \treturn 0;\n }\ndiff --git a/sequencer.h b/sequencer.h\nindex 719684c8a9..bea20da085 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -57,6 +57,8 @@ struct replay_opts {\n \tint ignore_date;\n \tint commit_use_reference;\n \n+\tstruct strvec trailer_args;\n+\n \tint mainline;\n \n \tchar *gpg_sign;\n@@ -84,6 +86,7 @@ struct replay_opts {\n #define REPLAY_OPTS_INIT {\t\t\t\\\n \t.edit = -1,\t\t\t\t\\\n \t.action = -1,\t\t\t\t\\\n+\t.trailer_args = STRVEC_INIT,\t\t\\\n \t.xopts = STRVEC_INIT,\t\t\t\\\n \t.ctx = replay_ctx_new(),\t\t\\\n }\ndiff --git a/t/meson.build b/t/meson.build\nindex f80e366cff..1f6f8ac20b 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -388,6 +388,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 't3450-history.sh',\n 't3451-history-reword.sh',\n 't3500-cherry.sh',\ndiff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh\nnew file mode 100755\nindex 0000000000..8b47579566\n--- /dev/null\n+++ b/t/t3440-rebase-trailer.sh\n@@ -0,0 +1,147 @@\n+#!/bin/sh\n+#\n+\n+test_description='git rebase --trailer integration tests\n+We verify that --trailer works with the merge backend,\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+REVIEWED_BY_TRAILER=\"Reviewed-by: Dev \"\n+SP=\" \"\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+\tgit checkout main\n+'\n+\n+test_expect_success 'apply backend is rejected with --trailer' '\n+\tgit checkout -B apply-backend third &&\n+\ttest_expect_code 128 \\\n+\t\tgit rebase --apply --trailer \"$REVIEWED_BY_TRAILER\" HEAD^ 2>err &&\n+\ttest_grep \"fatal: --trailer requires the merge backend\" err\n+'\n+\n+test_expect_success 'reject empty --trailer argument' '\n+\tgit checkout -B empty-trailer third &&\n+\ttest_expect_code 128 git rebase --trailer \"\" HEAD^ 2>err &&\n+\ttest_grep \"empty --trailer\" err\n+'\n+\n+test_expect_success 'reject trailer with missing key before separator' '\n+\tgit checkout -B missing-key third &&\n+\ttest_expect_code 128 git rebase --trailer \": no-key\" HEAD^ 2>err &&\n+\ttest_grep \"missing key before separator\" err\n+'\n+\n+test_expect_success 'allow trailer with missing value after separator' '\n+\tgit checkout -B missing-value third &&\n+\tgit rebase --trailer \"Acked-by:\" HEAD^ &&\n+\ttest_commit_message HEAD <<-EOF\n+\tthird\n+\n+\tAcked-by:${SP}\n+\tEOF\n+'\n+\n+test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last' '\n+\tgit checkout -B replace-policy third &&\n+\tgit -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add \\\n+\t\trebase --trailer \"Bug: 123\" --trailer \"Bug: 456\" HEAD^ &&\n+\ttest_commit_message HEAD <<-EOF\n+\tthird\n+\n+\tBug: 456\n+\tEOF\n+'\n+\n+test_expect_success 'multiple Signed-off-by trailers all preserved' '\n+\tgit checkout -B multiple-signoff third &&\n+\tgit rebase --trailer \"Signed-off-by: Dev A \" \\\n+\t\t--trailer \"Signed-off-by: Dev B \" HEAD^ &&\n+\ttest_commit_message HEAD <<-EOF\n+\tthird\n+\n+\tSigned-off-by: Dev A \n+\tSigned-off-by: Dev B \n+\tEOF\n+'\n+\n+test_expect_success 'rebase --trailer adds trailer after conflicts' '\n+\tgit checkout -B trailer-conflict third &&\n+\ttest_commit fourth file &&\n+\ttest_must_fail git rebase --trailer \"$REVIEWED_BY_TRAILER\" second &&\n+\tgit checkout --theirs file &&\n+\tgit add file &&\n+\tgit rebase --continue &&\n+\ttest_commit_message HEAD <<-EOF &&\n+\tfourth\n+\n+\t$REVIEWED_BY_TRAILER\n+\tEOF\n+\ttest_commit_message HEAD^ <<-EOF\n+\tthird\n+\n+\t$REVIEWED_BY_TRAILER\n+\tEOF\n+'\n+\n+test_expect_success '--trailer handles fixup commands in todo list' '\n+\tgit checkout -B fixup-trailer third &&\n+\ttest_commit fixup-base base &&\n+\ttest_commit fixup-second second &&\n+\tcat >todo <<-\\EOF &&\n+\tpick fixup-base fixup-base\n+\tfixup fixup-second fixup-second\n+\tEOF\n+\t(\n+\t\tset_replace_editor todo &&\n+\t\tgit rebase -i --trailer \"$REVIEWED_BY_TRAILER\" HEAD~2\n+\t) &&\n+\ttest_commit_message HEAD <<-EOF &&\n+\tfixup-base\n+\n+\t$REVIEWED_BY_TRAILER\n+\tEOF\n+\tgit reset --hard fixup-second &&\n+\tcat >todo <<-\\EOF &&\n+\tpick fixup-base fixup-base\n+\tfixup -C fixup-second fixup-second\n+\tEOF\n+\t(\n+\t\tset_replace_editor todo &&\n+\t\tgit rebase -i --trailer \"$REVIEWED_BY_TRAILER\" HEAD~2\n+\t) &&\n+\ttest_commit_message HEAD <<-EOF\n+\tfixup-second\n+\n+\t$REVIEWED_BY_TRAILER\n+\tEOF\n+'\n+\n+test_expect_success 'rebase --root honors trailer..key' '\n+\tgit checkout -B root-trailer first &&\n+\tgit -c trailer.review.key=Reviewed-by rebase --root \\\n+\t\t--trailer=review=\"Dev \" &&\n+\ttest_commit_message HEAD <<-EOF &&\n+\tfirst\n+\n+\tReviewed-by: Dev \n+\tEOF\n+\ttest_commit_message HEAD^ <<-EOF\n+\tInitial empty commit\n+\n+\tReviewed-by: Dev \n+\tEOF\n+'\n+test_done\n-- \n2.52.0\n"},{"id":"537207","messageId":"xmqqecm74fcd.fsf@gitster.g","threadId":"65068","inReplyTo":"20260224070552.148591-1-me@linux.beauty","subject":"Re: [PATCH v7 0/5] rebase: support --trailer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-26T16:52:34Z","receivedAt":"2026-02-26T16:52:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Li Chen writes:\n\n > Apologies for the long delay in sending v7.\n>\n> v7 is based on origin/master at v2.53.0-154-g7c02d39fc2.\n>\n> This series routes trailer insertion through an in-process path, removing the\n> fork/exec to builtin/interpret-trailers.\n>\n> The first four commits refactor trailer rewriting in builtin/interpret-trailers\n> and trailer.c so callers can reuse a single in-process helper (used by git\n> interpret-trailers, git commit and git tag). The final commit adds git rebase\n> --trailer, currently supported with the merge backend only (rejecting apply-only\n> scenarios and validating input early).\n>\n> v7:\n> Rebased onto origin/master at v2.53.0-154-g7c02d39fc2.\n> Split out a new patch to parse --trailer with OPT_STRVEC in git commit and git\n> tag.\n> Use strbuf_write() in interpret-trailers when emitting buffered output.\n> Restore --in-place rewriting semantics via tempfile+rename.\n> Drop wrapper.c/h and validate trailer args via validate_trailer_args().\n> Drop redundant rebase basic-state save/restore for --trailer arguments.\n> ...\n> Comments very very welcome!\n\nYes indeed. The discussion thread for v6 saw quite a bit of\nactivity, but this one is quiet. Is everybody happy with this\niteration?\n\nThanks.\n"},{"id":"537216","messageId":"83cac4b2-c59d-4ab2-9dfc-2f615ed3959d@gmail.com","threadId":"65068","inReplyTo":"xmqqecm74fcd.fsf@gitster.g","subject":"Re: [PATCH v7 0/5] rebase: support --trailer","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-26T18:15:19Z","receivedAt":"2026-02-26T18:15:13Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 26/02/2026 16:52, Junio C Hamano wrote:\n> Li Chen writes:\n> \n> > Apologies for the long delay in sending v7.\n>>\n>> v7 is based on origin/master at v2.53.0-154-g7c02d39fc2.\n>>\n>> This series routes trailer insertion through an in-process path, removing the\n>> fork/exec to builtin/interpret-trailers.\n>>\n>> The first four commits refactor trailer rewriting in builtin/interpret-trailers\n>> and trailer.c so callers can reuse a single in-process helper (used by git\n>> interpret-trailers, git commit and git tag). The final commit adds git rebase\n>> --trailer, currently supported with the merge backend only (rejecting apply-only\n>> scenarios and validating input early).\n>>\n>> v7:\n>> Rebased onto origin/master at v2.53.0-154-g7c02d39fc2.\n>> Split out a new patch to parse --trailer with OPT_STRVEC in git commit and git\n>> tag.\n>> Use strbuf_write() in interpret-trailers when emitting buffered output.\n>> Restore --in-place rewriting semantics via tempfile+rename.\n>> Drop wrapper.c/h and validate trailer args via validate_trailer_args().\n>> Drop redundant rebase basic-state save/restore for --trailer arguments.\n>> ...\n>> Comments very very welcome!\n> \n> Yes indeed. The discussion thread for v6 saw quite a bit of\n> activity, but this one is quiet. Is everybody happy with this\n> iteration?\n\nI've not had chance to read this version yet, I'm planning to do so next \nweek.\n\nThanks\n\nPhillip\n"},{"id":"537235","messageId":"b3b12e9c-bd4e-4585-aad9-24e2df6579a1@app.fastmail.com","threadId":"65068","inReplyTo":"20260224070552.148591-1-me@linux.beauty","subject":"Re: [PATCH v7 0/5] rebase: support --trailer","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-02-26T21:12:59Z","receivedAt":"2026-02-26T21:13:20Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Tue, Feb 24, 2026, at 08:05, Li Chen wrote:\n> Apologies for the long delay in sending v7.\n>\n> v7 is based on origin/master at v2.53.0-154-g7c02d39fc2.\n>\n> This series routes trailer insertion through an in-process path, removing the\n> fork/exec to builtin/interpret-trailers.\n>\n>[snip]\n\nThis round solves the documentation problem I was complaining about in\nthe previous round.\n\nThanks. I will make use of this option. :)\n"},{"id":"537528","messageId":"56b1b6b6-94c2-4a2f-b473-9b4d09d6f52e@gmail.com","threadId":"65068","inReplyTo":"20260224070552.148591-2-me@linux.beauty","subject":"Re: [PATCH v7 1/5] interpret-trailers: factor trailer rewriting","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-02T14:56:04Z","receivedAt":"2026-03-02T14:56:10Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Li\n\nOn 24/02/2026 07:05, Li Chen wrote:\n> Extract the trailer rewriting logic into a helper that appends to an\n> output strbuf.\n> \n> Update interpret_trailers() to handle file I/O only: read input once,\n> call the helper, and write the buffered result.\n> \n> This separation makes it easier to move the helper into trailer.c in the\n> next commit.\n\nThis is still missing my sign off c.f. \nhttps://lore.kernel.org/f5152523-f7ff-4dee-a685-fb0b74cd6a56@gmail.com\n\n> Signed-off-by: Li Chen \n> ---\n> v7:\n> Use strbuf_write() when emitting buffered output.\n\nAlso renamed \"sb\" to \"input\"\n\nApart from the missing sign off this all looks good.\n\nThanks\n\nPhillip\n\n> \n> builtin/interpret-trailers.c | 53 ++++++++++++++++++++----------------\n> 1 file changed, 30 insertions(+), 23 deletions(-)\n> \n> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\n> index 41b0750e5a..69f9d67ec0 100644\n> --- a/builtin/interpret-trailers.c\n> +++ b/builtin/interpret-trailers.c\n> @@ -136,32 +136,21 @@ static void read_input_file(struct strbuf *sb, const char *file)\n> \tstrbuf_complete_line(sb);\n> }\n> \n> -static void interpret_trailers(const struct process_trailer_options *opts,\n> -\t\t\t struct list_head *new_trailer_head,\n> -\t\t\t const char *file)\n> +static void process_trailers(const struct process_trailer_options *opts,\n> +\t\t\t struct list_head *new_trailer_head,\n> +\t\t\t struct strbuf *input, struct strbuf *out)\n> {\n> \tLIST_HEAD(head);\n> -\tstruct strbuf sb = STRBUF_INIT;\n> -\tstruct strbuf trailer_block_sb = STRBUF_INIT;\n> \tstruct trailer_block *trailer_block;\n> -\tFILE *outfile = stdout;\n> \n> -\ttrailer_config_init();\n> -\n> -\tread_input_file(&sb, file);\n> -\n> -\tif (opts->in_place)\n> -\t\toutfile = create_in_place_tempfile(file);\n> -\n> -\ttrailer_block = parse_trailers(opts, sb.buf, &head);\n> +\ttrailer_block = parse_trailers(opts, input->buf, &head);\n> \n> \t/* Print the lines before the trailer block */\n> \tif (!opts->only_trailers)\n> -\t\tfwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);\n> +\t\tstrbuf_add(out, input->buf, trailer_block_start(trailer_block));\n> \n> \tif (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))\n> -\t\tfprintf(outfile, \"\\n\");\n> -\n> +\t\tstrbuf_addch(out, '\\n');\n> \n> \tif (!opts->only_input) {\n> \t\tLIST_HEAD(config_head);\n> @@ -173,22 +162,40 @@ static void interpret_trailers(const struct process_trailer_options *opts,\n> \t}\n> \n> \t/* Print trailer block. */\n> -\tformat_trailers(opts, &head, &trailer_block_sb);\n> +\tformat_trailers(opts, &head, out);\n> \tfree_trailers(&head);\n> -\tfwrite(trailer_block_sb.buf, 1, trailer_block_sb.len, outfile);\n> -\tstrbuf_release(&trailer_block_sb);\n> \n> \t/* Print the lines after the trailer block as is. */\n> \tif (!opts->only_trailers)\n> -\t\tfwrite(sb.buf + trailer_block_end(trailer_block), 1,\n> -\t\t sb.len - trailer_block_end(trailer_block), outfile);\n> +\t\tstrbuf_add(out, input->buf + trailer_block_end(trailer_block),\n> +\t\t\t input->len - trailer_block_end(trailer_block));\n> \ttrailer_block_release(trailer_block);\n> +}\n> +\n> +static void interpret_trailers(const struct process_trailer_options *opts,\n> +\t\t\t struct list_head *new_trailer_head,\n> +\t\t\t const char *file)\n> +{\n> +\tstruct strbuf input = STRBUF_INIT;\n> +\tstruct strbuf out = STRBUF_INIT;\n> +\tFILE *outfile = stdout;\n> +\n> +\ttrailer_config_init();\n> +\n> +\tread_input_file(&input, file);\n> +\n> +\tif (opts->in_place)\n> +\t\toutfile = create_in_place_tempfile(file);\n> +\n> +\tprocess_trailers(opts, new_trailer_head, &input, &out);\n> \n> +\tstrbuf_write(&out, outfile);\n> \tif (opts->in_place)\n> \t\tif (rename_tempfile(&trailers_tempfile, file))\n> \t\t\tdie_errno(_(\"could not rename temporary file to %s\"), file);\n> \n> -\tstrbuf_release(&sb);\n> +\tstrbuf_release(&input);\n> +\tstrbuf_release(&out);\n> }\n> \n> int cmd_interpret_trailers(int argc,\n\n"},{"id":"537529","messageId":"db8eda43-5b2b-41eb-91bf-1b5b4da817de@gmail.com","threadId":"65068","inReplyTo":"20260224070552.148591-3-me@linux.beauty","subject":"Re: [PATCH v7 2/5] trailer: move process_trailers to trailer.h","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-02T14:56:15Z","receivedAt":"2026-03-02T14:56:19Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 24/02/2026 07:05, Li Chen wrote:\n> Move process_trailers() from builtin/interpret-trailers.c into trailer.c\n> and expose it via trailer.h.\n> \n> This lets other call sites reuse the same trailer rewriting logic.\n\nThis looks good, thanks\n\nPhillip\n\n> Signed-off-by: Li Chen \n> ---\n> v7:\n> Rename the input parameter from sb to input.\n> \n> builtin/interpret-trailers.c | 36 ------------------------------------\n> trailer.c | 36 ++++++++++++++++++++++++++++++++++++\n> trailer.h | 3 +++\n> 3 files changed, 39 insertions(+), 36 deletions(-)\n> \n> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\n> index 69f9d67ec0..1354109e0f 100644\n> --- a/builtin/interpret-trailers.c\n> +++ b/builtin/interpret-trailers.c\n> @@ -136,42 +136,6 @@ static void read_input_file(struct strbuf *sb, const char *file)\n> \tstrbuf_complete_line(sb);\n> }\n> \n> -static void process_trailers(const struct process_trailer_options *opts,\n> -\t\t\t struct list_head *new_trailer_head,\n> -\t\t\t struct strbuf *input, struct strbuf *out)\n> -{\n> -\tLIST_HEAD(head);\n> -\tstruct trailer_block *trailer_block;\n> -\n> -\ttrailer_block = parse_trailers(opts, input->buf, &head);\n> -\n> -\t/* Print the lines before the trailer block */\n> -\tif (!opts->only_trailers)\n> -\t\tstrbuf_add(out, input->buf, trailer_block_start(trailer_block));\n> -\n> -\tif (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))\n> -\t\tstrbuf_addch(out, '\\n');\n> -\n> -\tif (!opts->only_input) {\n> -\t\tLIST_HEAD(config_head);\n> -\t\tLIST_HEAD(arg_head);\n> -\t\tparse_trailers_from_config(&config_head);\n> -\t\tparse_trailers_from_command_line_args(&arg_head, new_trailer_head);\n> -\t\tlist_splice(&config_head, &arg_head);\n> -\t\tprocess_trailers_lists(&head, &arg_head);\n> -\t}\n> -\n> -\t/* Print trailer block. */\n> -\tformat_trailers(opts, &head, out);\n> -\tfree_trailers(&head);\n> -\n> -\t/* Print the lines after the trailer block as is. */\n> -\tif (!opts->only_trailers)\n> -\t\tstrbuf_add(out, input->buf + trailer_block_end(trailer_block),\n> -\t\t\t input->len - trailer_block_end(trailer_block));\n> -\ttrailer_block_release(trailer_block);\n> -}\n> -\n> static void interpret_trailers(const struct process_trailer_options *opts,\n> \t\t\t struct list_head *new_trailer_head,\n> \t\t\t const char *file)\n> diff --git a/trailer.c b/trailer.c\n> index 911a81ed99..0c9200506d 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -1235,3 +1235,39 @@ int amend_file_with_trailers(const char *path, const struct strvec *trailer_args\n> \tstrvec_pushv(&run_trailer.args, trailer_args->v);\n> \treturn run_command(&run_trailer);\n> }\n> +\n> +void process_trailers(const struct process_trailer_options *opts,\n> +\t\t struct list_head *new_trailer_head,\n> +\t\t struct strbuf *input, struct strbuf *out)\n> +{\n> +\tLIST_HEAD(head);\n> +\tstruct trailer_block *trailer_block;\n> +\n> +\ttrailer_block = parse_trailers(opts, input->buf, &head);\n> +\n> +\t/* Print the lines before the trailer block */\n> +\tif (!opts->only_trailers)\n> +\t\tstrbuf_add(out, input->buf, trailer_block_start(trailer_block));\n> +\n> +\tif (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))\n> +\t\tstrbuf_addch(out, '\\n');\n> +\n> +\tif (!opts->only_input) {\n> +\t\tLIST_HEAD(config_head);\n> +\t\tLIST_HEAD(arg_head);\n> +\t\tparse_trailers_from_config(&config_head);\n> +\t\tparse_trailers_from_command_line_args(&arg_head, new_trailer_head);\n> +\t\tlist_splice(&config_head, &arg_head);\n> +\t\tprocess_trailers_lists(&head, &arg_head);\n> +\t}\n> +\n> +\t/* Print trailer block. */\n> +\tformat_trailers(opts, &head, out);\n> +\tfree_trailers(&head);\n> +\n> +\t/* Print the lines after the trailer block as is. */\n> +\tif (!opts->only_trailers)\n> +\t\tstrbuf_add(out, input->buf + trailer_block_end(trailer_block),\n> +\t\t\t input->len - trailer_block_end(trailer_block));\n> +\ttrailer_block_release(trailer_block);\n> +}\n> diff --git a/trailer.h b/trailer.h\n> index 4740549586..531fa1a13f 100644\n> --- a/trailer.h\n> +++ b/trailer.h\n> @@ -202,4 +202,7 @@ 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> +void process_trailers(const struct process_trailer_options *opts,\n> +\t\t struct list_head *new_trailer_head,\n> +\t\t struct strbuf *input, struct strbuf *out);\n> #endif /* TRAILER_H */\n\n"},{"id":"537530","messageId":"e555e29f-57e3-4192-98f3-380181c81fd2@gmail.com","threadId":"65068","inReplyTo":"20260224070552.148591-4-me@linux.beauty","subject":"Re: [PATCH v7 3/5] trailer: append trailers without fork/exec","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-02T14:56:27Z","receivedAt":"2026-03-02T14:56:32Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Li\n\nOn 24/02/2026 07:05, Li Chen wrote:\n> Introduce amend_strbuf_with_trailers() to apply trailer additions to a\n> message buffer via process_trailers(), avoiding the need to run git\n> interpret-trailers as a child process.\n> \n> Update amend_file_with_trailers() to use the in-process helper and\n> rewrite the target file via tempfile+rename, preserving the previous\n> in-place semantics.\n> \n> Keep existing callers unchanged by continuing to accept argv-style\n> --trailer= entries and stripping the prefix before feeding the\n> in-process implementation.\n\nThis looks pretty good, there are a few rough edges in the tempfile \nhandling and a couple of unrelated changes but the basics are sound.\n\n> Signed-off-by: Li Chen \n> ---\n> v7:\n> Drop wrapper.c/h and validate trailer args via validate_trailer_args().\n> Rewrite the target file via tempfile+rename to preserve --in-place semantics.\n> \n> builtin/interpret-trailers.c | 4 +-\n> trailer.c | 162 +++++++++++++++++++++++++++++++++--\n> trailer.h | 27 +++++-\n> 3 files changed, 179 insertions(+), 14 deletions(-)\n> \n> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\n> index 1354109e0f..d4aff68746 100644\n> --- a/builtin/interpret-trailers.c\n> +++ b/builtin/interpret-trailers.c\n> @@ -144,8 +144,6 @@ static void interpret_trailers(const struct process_trailer_options *opts,\n> \tstruct strbuf out = STRBUF_INIT;\n> \tFILE *outfile = stdout;\n> \n> -\ttrailer_config_init();\n> -\n> \tread_input_file(&input, file);\n> \n> \tif (opts->in_place)\n> @@ -203,6 +201,8 @@ int cmd_interpret_trailers(int argc,\n> \t\t\tgit_interpret_trailers_usage,\n> \t\t\toptions);\n> \n> +\ttrailer_config_init();\n> +\n\nThe changes above are unrelated to everything else in this patch.\n\n> \tif (argc) {\n> \t\tint i;\n> \t\tfor (i = 0; i < argc; i++)\n> diff --git a/trailer.c b/trailer.c\n> index 0c9200506d..8e87d185d9 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -772,6 +775,30 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,\n> \tfree(cl_separators);\n> }\n> \n> +void validate_trailer_args(const struct strvec *cli_args)\n> +{\n> +\tchar *cl_separators;\n> +\n> +\ttrailer_config_init();\n> +\n> +\tcl_separators = xstrfmt(\"=%s\", separators);\n> +\n> +\tfor (size_t i = 0; i < cli_args->nr; i++) {\n> +\t\tconst char *txt = cli_args->v[i];\n> +\t\tssize_t separator_pos;\n> +\n> +\t\tif (!*txt)\n> +\t\t\tdie(_(\"empty --trailer argument\"));\n> +\n> +\t\tseparator_pos = find_separator(txt, cl_separators);\n> +\t\tif (separator_pos == 0)\n> +\t\t\tdie(_(\"invalid trailer '%s': missing key before separator\"),\n> +\t\t\t txt);\n> +\t}\n> +\n> +\tfree(cl_separators);\n> +}\n\nThis function is never called, is it left over by a mistake, or should \nit be called by amend_file_with_trailers()?\n\n> static const char *next_line(const char *str)\n> {\n> \tconst char *nl = strchrnul(str, '\\n');\n> @@ -1224,16 +1251,133 @@ 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 void new_trailer_items_clear(struct list_head *items)\n> +{\n> +\twhile (!list_empty(items)) {\n> +\t\tstruct new_trailer_item *item =\n> +\t\t\tlist_first_entry(items, struct new_trailer_item, list);\n> +\t\tlist_del(&item->list);\n> +\t\tfree(item);\n> +\t}\n> +}\n\nIsn't this just doing what free_trailers() does c.f. \nhttps://lore.kernel.org/git/ef12ada7-13ae-4df0-a823-6f428c797223@gmail.com/\n\n> +void amend_strbuf_with_trailers(struct strbuf *buf,\n> +\t\t\t\tconst struct strvec *trailer_args)\n> +{\n> +\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n> +\tLIST_HEAD(new_trailer_head);\n> +\tstruct strbuf out = STRBUF_INIT;\n> +\tsize_t i;\n> +\n> +\topts.no_divider = 1;\n> +\n> +\tfor (i = 0; i < trailer_args->nr; i++) {\n> +\t\tconst char *text = trailer_args->v[i];\n> +\t\tstruct new_trailer_item *item;\n> +\n> +\t\tif (!*text)\n> +\t\t\tdie(_(\"empty --trailer argument\"));\n\nThe existing callers of amend_file_with_trailers() are not expecting it \nto die so we should return an error here so they cleanup before exiting.\n\n> +\t\titem = xcalloc(1, sizeof(*item));\n> +\t\titem->text = text;\n> +\t\tlist_add_tail(&item->list, &new_trailer_head);\n> +\t}\n> +\n> +\ttrailer_config_init();\n> +\tprocess_trailers(&opts, &new_trailer_head, buf, &out);\n> +\n> +\tstrbuf_swap(buf, &out);\n> +\tstrbuf_release(&out);\n> +\n> +\tnew_trailer_items_clear(&new_trailer_head);\n> +}\n> +\n> +static int write_file_in_place(const char *path, const struct strbuf *buf)\n> +{\n> +\tstruct stat st;\n> +\tstruct strbuf filename_template = STRBUF_INIT;\n> +\tconst char *tail;\n> +\tstruct tempfile *tempfile;\n> +\tFILE *outfile;\n> +\n> +\tif (stat(path, &st))\n> +\t\treturn error_errno(_(\"could not stat %s\"), path);\n> +\tif (!S_ISREG(st.st_mode))\n> +\t\treturn error(_(\"file %s is not a regular file\"), path);\n> +\tif (!(st.st_mode & S_IWUSR))\n> +\t\treturn error(_(\"file %s is not writable by user\"), path);\n\nThis is copied from create_in_place_tempfile() replacing die() with \nerror() - ok\n\n> +\t/* Create temporary file in the same directory as the original */\n> +\ttail = strrchr(path, '/');\n\nThis is also copied from create_in_place_tempfile() but it would be \nbetter to use find_last_dir_sep()\n\n> +\tif (tail)\n> +\t\tstrbuf_add(&filename_template, path, tail - path + 1);\n> +\tstrbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n> +\n> +\ttempfile = mks_tempfile_sm(filename_template.buf, 0, st.st_mode);\n\nThis could be mks_tempfile_m() like create_in_place_tempfile() as we \ndon't have a suffix to add.\n\n> +\tstrbuf_release(&filename_template);\n> +\tif (!tempfile)\n> +\t\treturn error_errno(_(\"could not create temporary file\"));\n> +\n> +\toutfile = fdopen_tempfile(tempfile, \"w\");\n> +\tif (!outfile) {\n> +\t\tint saved_errno = errno;\n> +\t\tdelete_tempfile(&tempfile);\n> +\t\terrno = saved_errno;\n> +\t\treturn error_errno(_(\"could not open temporary file\"));\n> +\t}\n> +\n> +\tif (buf->len && fwrite(buf->buf, 1, buf->len, outfile) < buf->len) {\n> +\t\tint saved_errno = errno;\n> +\t\tdelete_tempfile(&tempfile);\n> +\t\terrno = saved_errno;\n> +\t\treturn error_errno(_(\"could not write to temporary file\"));\n> +\t}\n\nAs all we're doing is writing the contents of buf we can use \nwrite_in_full() here and avoid the call to fdopen_tempfile() above. Also \nnote that the documentation in tempfile.h says\n\n * If the program exits before `rename_tempfile()` or\n * `delete_tempfile()` is called, an `atexit(3)` handler will close\n * and remove the temporary file.\n\nso we don't need to call delete_tempfile() if the write fails.\n\nWe could probably share this code with builtin/interpret-trailers.c (it \nno-longer needs to call fdopen_tempfile() either) which would make it \nmore obvious that we're basically copying it here.\n\n> +\tif (rename_tempfile(&tempfile, path))\n> +\t\treturn error_errno(_(\"could not rename temporary file to %s\"), path);\n> +\n> +\treturn 0;\n> +}\n> +\n> +int amend_file_with_trailers(const char *path,\n> +\t\t\t const struct strvec *trailer_args)\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> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tstruct strvec stripped_trailer_args = STRVEC_INIT;\n> +\tint ret = 0;\n> +\tsize_t i;\n> +\n> +\tif (!trailer_args)\n> +\t\tBUG(\"amend_file_with_trailers called with NULL trailer_args\");\n> +\tif (!trailer_args->nr)\n> +\t\treturn 0;\n> +\n> +\tfor (i = 0; i < trailer_args->nr; i++) {\n> +\t\tconst char *txt = trailer_args->v[i];\n> +\n> +\t\t/*\n> +\t\t * Historically amend_file_with_trailers() passed its arguments\n> +\t\t * to \"git interpret-trailers\", which expected argv entries in\n> +\t\t * \"--trailer=\" form. Continue to accept those for\n> +\t\t * existing callers, but pass only the value portion to the\n> +\t\t * in-process implementation.\n> +\t\t */\n> +\t\tskip_prefix(txt, \"--trailer=\", &txt);\n> +\t\tif (!*txt)\n> +\t\t\tdie(_(\"empty --trailer argument\"));\n\nIt seems a shame to die() here when the rest of the function uses \nerror() especially as the existing callers of amend_file_with_trailers() \nare not expecting it to die() and may want to perform some cleanup \nbefore exiting.\n\n> +\t\tstrvec_push(&stripped_trailer_args, txt);\n> +\t}\n> +\n> +\tif (strbuf_read_file(&buf, path, 0) < 0)\n> +\t\tret = error_errno(_(\"could not read '%s'\"), path);\n> +\telse\n> +\t\tamend_strbuf_with_trailers(&buf, &stripped_trailer_args);\n> +\n> +\tif (!ret)\n> +\t\tret = write_file_in_place(path, &buf);\n> +\n> +\tstrvec_clear(&stripped_trailer_args);\n> +\tstrbuf_release(&buf);\n> +\treturn ret;\n> }\n\n> [...] \n> +/*\n> + * Rewrite the contents of input by processing its trailer block according to\n> + * opts and (optionally) appending trailers from new_trailer_head.\n> + *\n> + * The rewritten message is appended to out (callers should strbuf_reset()\n> + * first if needed).\n> + */\nThis comment should be part of the previous commit that adds \nprocess_trailers().\n\n> void process_trailers(const struct process_trailer_options *opts,\n> \t\t struct list_head *new_trailer_head,\n> \t\t struct strbuf *input, struct strbuf *out);\n\nThanks\n\nPhillip\n\n"},{"id":"537531","messageId":"dffc34a6-3a89-4535-9caf-95625a221797@gmail.com","threadId":"65068","inReplyTo":"20260224070552.148591-5-me@linux.beauty","subject":"Re: [PATCH v7 4/5] commit, tag: parse --trailer with OPT_STRVEC","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-02T14:56:33Z","receivedAt":"2026-03-02T14:56:37Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Li\n\nOn 24/02/2026 07:05, Li Chen wrote:\n> Now that amend_file_with_trailers() expects raw trailer lines, do not\n> store argv-style \"--trailer=\" strings in git commit and git\n> tag.\n> \n> Parse --trailer using OPT_STRVEC so trailer_args contains only the\n> trailer value, and drop the temporary prefix stripping in\n> amend_file_with_trailers().\n\nThis looks good. I'll stop here for today and look at the last patch \ntomorrow\n\nThanks\n\nPhillip\n\n> Signed-off-by: Li Chen \n> ---\n> v7:\n> New patch.\n> \n> builtin/commit.c | 3 ++-\n> builtin/tag.c | 4 ++--\n> trailer.c | 21 +--------------------\n> trailer.h | 4 ++--\n> 4 files changed, 7 insertions(+), 25 deletions(-)\n> \n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 9e3a09d532..d9983230de 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1720,7 +1720,8 @@ int cmd_commit(int argc,\n> \t\tOPT_STRING(0, \"fixup\", &fixup_message, N_(\"[(amend|reword):]commit\"), N_(\"use autosquash formatted message to fixup or amend/reword specified commit\")),\n> \t\tOPT_STRING(0, \"squash\", &squash_message, N_(\"commit\"), N_(\"use autosquash formatted message to squash specified commit\")),\n> \t\tOPT_BOOL(0, \"reset-author\", &renew_authorship, N_(\"the commit is authored by me now (used with -C/-c/--amend)\")),\n> -\t\tOPT_PASSTHRU_ARGV(0, \"trailer\", &trailer_args, N_(\"trailer\"), N_(\"add custom trailer(s)\"), PARSE_OPT_NONEG),\n> +\t\tOPT_STRVEC(0, \"trailer\", &trailer_args, N_(\"trailer\"),\n> +\t\t\t N_(\"add custom trailer(s)\")),\n> \t\tOPT_BOOL('s', \"signoff\", &signoff, N_(\"add a Signed-off-by trailer\")),\n> \t\tOPT_FILENAME('t', \"template\", &template_file, N_(\"use specified template file\")),\n> \t\tOPT_BOOL('e', \"edit\", &edit_flag, N_(\"force edit of commit\")),\n> diff --git a/builtin/tag.c b/builtin/tag.c\n> index aeb04c487f..15aee1b03a 100644\n> --- a/builtin/tag.c\n> +++ b/builtin/tag.c\n> @@ -499,8 +499,8 @@ int cmd_tag(int argc,\n> \t\tOPT_CALLBACK_F('m', \"message\", &msg, N_(\"message\"),\n> \t\t\t N_(\"tag message\"), PARSE_OPT_NONEG, parse_msg_arg),\n> \t\tOPT_FILENAME('F', \"file\", &msgfile, N_(\"read message from file\")),\n> -\t\tOPT_PASSTHRU_ARGV(0, \"trailer\", &trailer_args, N_(\"trailer\"),\n> -\t\t\t\t N_(\"add custom trailer(s)\"), PARSE_OPT_NONEG),\n> +\t\tOPT_STRVEC(0, \"trailer\", &trailer_args, N_(\"trailer\"),\n> +\t\t\t N_(\"add custom trailer(s)\")),\n> \t\tOPT_BOOL('e', \"edit\", &edit_flag, N_(\"force edit of tag message\")),\n> \t\tOPT_BOOL('s', \"sign\", &opt.sign, N_(\"annotated and GPG-signed tag\")),\n> \t\tOPT_CLEANUP(&cleanup_arg),\n> diff --git a/trailer.c b/trailer.c\n> index 8e87d185d9..e85c6c9fbe 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -1342,40 +1342,21 @@ int amend_file_with_trailers(const char *path,\n> \t\t\t const struct strvec *trailer_args)\n> {\n> \tstruct strbuf buf = STRBUF_INIT;\n> -\tstruct strvec stripped_trailer_args = STRVEC_INIT;\n> \tint ret = 0;\n> -\tsize_t i;\n> \n> \tif (!trailer_args)\n> \t\tBUG(\"amend_file_with_trailers called with NULL trailer_args\");\n> \tif (!trailer_args->nr)\n> \t\treturn 0;\n> \n> -\tfor (i = 0; i < trailer_args->nr; i++) {\n> -\t\tconst char *txt = trailer_args->v[i];\n> -\n> -\t\t/*\n> -\t\t * Historically amend_file_with_trailers() passed its arguments\n> -\t\t * to \"git interpret-trailers\", which expected argv entries in\n> -\t\t * \"--trailer=\" form. Continue to accept those for\n> -\t\t * existing callers, but pass only the value portion to the\n> -\t\t * in-process implementation.\n> -\t\t */\n> -\t\tskip_prefix(txt, \"--trailer=\", &txt);\n> -\t\tif (!*txt)\n> -\t\t\tdie(_(\"empty --trailer argument\"));\n> -\t\tstrvec_push(&stripped_trailer_args, txt);\n> -\t}\n> -\n> \tif (strbuf_read_file(&buf, path, 0) < 0)\n> \t\tret = error_errno(_(\"could not read '%s'\"), path);\n> \telse\n> -\t\tamend_strbuf_with_trailers(&buf, &stripped_trailer_args);\n> +\t\tamend_strbuf_with_trailers(&buf, trailer_args);\n> \n> \tif (!ret)\n> \t\tret = write_file_in_place(path, &buf);\n> \n> -\tstrvec_clear(&stripped_trailer_args);\n> \tstrbuf_release(&buf);\n> \treturn ret;\n> }\n> diff --git a/trailer.h b/trailer.h\n> index d05dab050b..e5bd355aad 100644\n> --- a/trailer.h\n> +++ b/trailer.h\n> @@ -209,8 +209,8 @@ void amend_strbuf_with_trailers(struct strbuf *buf,\n> /*\n> * Augment a file by appending trailers specified in trailer_args.\n> *\n> - * Each element of trailer_args should be an argv-style --trailer=\n> - * option (i.e., including the --trailer= prefix).\n> + * Each element of trailer_args should be in the same format as the value\n> + * accepted by --trailer= (i.e., without the --trailer= prefix).\n> *\n> * Returns 0 on success or a non-zero error code on failure.\n> */\n\n"},{"id":"537532","messageId":"19caf109a99.6c639fbf1370819.2681969272338930783@linux.beauty","threadId":"65068","inReplyTo":"56b1b6b6-94c2-4a2f-b473-9b4d09d6f52e@gmail.com","subject":"Re: [PATCH v7 1/5] interpret-trailers: factor trailer rewriting","fromName":"Li Chen","fromEmail":"me@linux.beauty","sentAt":"2026-03-02T15:00:26Z","receivedAt":"2026-03-02T15:00:42Z","isPatch":true,"sender":{"key":"me@linux.beauty","avatar":"https://avatars.githubusercontent.com/u/37442588?v=4"},"body":"Hi Phillip,\n\n ---- On Mon, 02 Mar 2026 22:56:04 +0800 Phillip Wood wrote --- \n > Hi Li\n > \n > On 24/02/2026 07:05, Li Chen wrote:\n > > Extract the trailer rewriting logic into a helper that appends to an\n > > output strbuf.\n > > \n > > Update interpret_trailers() to handle file I/O only: read input once,\n > > call the helper, and write the buffered result.\n > > \n > > This separation makes it easier to move the helper into trailer.c in the\n > > next commit.\n > \n > This is still missing my sign off c.f. \n > https://lore.kernel.org/f5152523-f7ff-4dee-a685-fb0b74cd6a56@gmail.com\n > \n > > Signed-off-by: Li Chen \n > > ---\n > > v7:\n > > Use strbuf_write() when emitting buffered output.\n > \n > Also renamed \"sb\" to \"input\"\n > \n > Apart from the missing sign off this all looks good.\n \nI sincerely apologize for that! I did check it during development, but somehow lost\ntrack in the end. I will definitely include your sign-off in the next version!\n\nRegards,\nLi\n"},{"id":"537682","messageId":"824809c3-72ac-43fb-8a93-4f48e0727e6a@gmail.com","threadId":"65068","inReplyTo":"20260224070552.148591-6-me@linux.beauty","subject":"Re: [PATCH v7 5/5] rebase: support --trailer","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-03T15:05:10Z","receivedAt":"2026-03-03T15:05:17Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Li\n\nOn 24/02/2026 07:05, Li Chen wrote:\n> \n> diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc\n> index e177808004..908717991a 100644\n> --- a/Documentation/git-rebase.adoc\n> +++ b/Documentation/git-rebase.adoc\n> @@ -497,6 +497,13 @@ See also INCOMPATIBLE OPTIONS below.\n> +\n> See also INCOMPATIBLE OPTIONS below.\n> \n> +--trailer=::\n> +\tAppend the given trailer to every rebased commit message, processed\n> +\tvia linkgit:git-interpret-trailers[1]. This option implies\n> +\t`--force-rebase` so that fast-forwarded commits are also rewritten.\n\nIf the commits are rewritten then they're not fast-forwarded, I just say \n\"This option implies `--force-rebase`.\" as all the other options that \nimply `--force-rebase` do.\n\n> +See also INCOMPATIBLE OPTIONS below.\n\nThat section needs updating to include `--trailer`\n\n> diff --git a/sequencer.c b/sequencer.c\n> index a3eb39bb25..a60c2a0cde 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> [...]\n> @@ -2025,6 +2027,9 @@ static int append_squash_message(struct strbuf *buf, const char *body,\n> \t\tif (opts->signoff)\n> \t\t\tappend_signoff(buf, 0, 0);\n> \n> +\t\tif (opts->trailer_args.nr)\n> +\t\t\tamend_strbuf_with_trailers(buf, &opts->trailer_args);\n\nI wonder if it would be better to add the trailers before the signoff so \nthat \"git rebase --signoff --trailer='Reviewed-by: ...'\" adds the \n\"Reviewed-by:\" trailer before the \"Signed-off-by:\" trailer.\n\n> @@ -3234,6 +3242,18 @@ static int read_populate_opts(struct replay_opts *opts)\n> \n> \t\tread_strategy_opts(opts, &buf);\n> \t\tstrbuf_reset(&buf);\n> +\t\tif (strbuf_read_file(&buf, rebase_path_trailer(), 0) >= 0) {\n\nMost of the other options are read with read_oneliner() which warns if \nthere is a read error - we should do the same here. We should also warn \nif this file is empty as we only write it when trailer_args.nr > 0.\n\nThis is all looking pretty good\n\nThanks\n\nPhillip\n\n> +\t\t\tchar *p = buf.buf, *nl;\n> +\n> +\t\t\twhile ((nl = strchr(p, '\\n'))) {\n> +\t\t\t\t*nl = '\\0';\n> +\t\t\t\tif (!*p)\n> +\t\t\t\t\tBUG(\"rebase-merge/trailer has an empty line\");\n> +\t\t\t\tstrvec_push(&opts->trailer_args, p);\n> +\t\t\t\tp = nl + 1;\n> +\t\t\t}\n> +\t\t\tstrbuf_reset(&buf);\n> +\t\t}\n> \n> \t\tif (read_oneliner(&ctx->current_fixups,\n> \t\t\t\t rebase_path_current_fixups(),\n> @@ -3328,6 +3348,14 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,\n> \t\twrite_file(rebase_path_reschedule_failed_exec(), \"%s\", \"\");\n> \telse\n> \t\twrite_file(rebase_path_no_reschedule_failed_exec(), \"%s\", \"\");\n> +\tif (opts->trailer_args.nr) {\n> +\t\tstruct strbuf buf = STRBUF_INIT;\n> +\n> +\t\tfor (size_t i = 0; i < opts->trailer_args.nr; i++)\n> +\t\t\tstrbuf_addf(&buf, \"%s\\n\", opts->trailer_args.v[i]);\n> +\t\twrite_file(rebase_path_trailer(), \"%s\", buf.buf);\n> +\t\tstrbuf_release(&buf);\n> +\t}\n> \n> \treturn 0;\n> }\n> diff --git a/sequencer.h b/sequencer.h\n> index 719684c8a9..bea20da085 100644\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -57,6 +57,8 @@ struct replay_opts {\n> \tint ignore_date;\n> \tint commit_use_reference;\n> \n> +\tstruct strvec trailer_args;\n> +\n> \tint mainline;\n> \n> \tchar *gpg_sign;\n> @@ -84,6 +86,7 @@ struct replay_opts {\n> #define REPLAY_OPTS_INIT {\t\t\t\\\n> \t.edit = -1,\t\t\t\t\\\n> \t.action = -1,\t\t\t\t\\\n> +\t.trailer_args = STRVEC_INIT,\t\t\\\n> \t.xopts = STRVEC_INIT,\t\t\t\\\n> \t.ctx = replay_ctx_new(),\t\t\\\n> }\n> diff --git a/t/meson.build b/t/meson.build\n> index f80e366cff..1f6f8ac20b 100644\n> --- a/t/meson.build\n> +++ b/t/meson.build\n> @@ -388,6 +388,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> 't3450-history.sh',\n> 't3451-history-reword.sh',\n> 't3500-cherry.sh',\n> diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh\n> new file mode 100755\n> index 0000000000..8b47579566\n> --- /dev/null\n> +++ b/t/t3440-rebase-trailer.sh\n> @@ -0,0 +1,147 @@\n> +#!/bin/sh\n> +#\n> +\n> +test_description='git rebase --trailer integration tests\n> +We verify that --trailer works with the merge backend,\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> +REVIEWED_BY_TRAILER=\"Reviewed-by: Dev \"\n> +SP=\" \"\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> +\tgit checkout main\n> +'\n> +\n> +test_expect_success 'apply backend is rejected with --trailer' '\n> +\tgit checkout -B apply-backend third &&\n> +\ttest_expect_code 128 \\\n> +\t\tgit rebase --apply --trailer \"$REVIEWED_BY_TRAILER\" HEAD^ 2>err &&\n> +\ttest_grep \"fatal: --trailer requires the merge backend\" err\n> +'\n> +\n> +test_expect_success 'reject empty --trailer argument' '\n> +\tgit checkout -B empty-trailer third &&\n> +\ttest_expect_code 128 git rebase --trailer \"\" HEAD^ 2>err &&\n> +\ttest_grep \"empty --trailer\" err\n> +'\n> +\n> +test_expect_success 'reject trailer with missing key before separator' '\n> +\tgit checkout -B missing-key third &&\n> +\ttest_expect_code 128 git rebase --trailer \": no-key\" HEAD^ 2>err &&\n> +\ttest_grep \"missing key before separator\" err\n> +'\n> +\n> +test_expect_success 'allow trailer with missing value after separator' '\n> +\tgit checkout -B missing-value third &&\n> +\tgit rebase --trailer \"Acked-by:\" HEAD^ &&\n> +\ttest_commit_message HEAD <<-EOF\n> +\tthird\n> +\n> +\tAcked-by:${SP}\n> +\tEOF\n> +'\n> +\n> +test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last' '\n> +\tgit checkout -B replace-policy third &&\n> +\tgit -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add \\\n> +\t\trebase --trailer \"Bug: 123\" --trailer \"Bug: 456\" HEAD^ &&\n> +\ttest_commit_message HEAD <<-EOF\n> +\tthird\n> +\n> +\tBug: 456\n> +\tEOF\n> +'\n> +\n> +test_expect_success 'multiple Signed-off-by trailers all preserved' '\n> +\tgit checkout -B multiple-signoff third &&\n> +\tgit rebase --trailer \"Signed-off-by: Dev A \" \\\n> +\t\t--trailer \"Signed-off-by: Dev B \" HEAD^ &&\n> +\ttest_commit_message HEAD <<-EOF\n> +\tthird\n> +\n> +\tSigned-off-by: Dev A \n> +\tSigned-off-by: Dev B \n> +\tEOF\n> +'\n> +\n> +test_expect_success 'rebase --trailer adds trailer after conflicts' '\n> +\tgit checkout -B trailer-conflict third &&\n> +\ttest_commit fourth file &&\n> +\ttest_must_fail git rebase --trailer \"$REVIEWED_BY_TRAILER\" second &&\n> +\tgit checkout --theirs file &&\n> +\tgit add file &&\n> +\tgit rebase --continue &&\n> +\ttest_commit_message HEAD <<-EOF &&\n> +\tfourth\n> +\n> +\t$REVIEWED_BY_TRAILER\n> +\tEOF\n> +\ttest_commit_message HEAD^ <<-EOF\n> +\tthird\n> +\n> +\t$REVIEWED_BY_TRAILER\n> +\tEOF\n> +'\n> +\n> +test_expect_success '--trailer handles fixup commands in todo list' '\n> +\tgit checkout -B fixup-trailer third &&\n> +\ttest_commit fixup-base base &&\n> +\ttest_commit fixup-second second &&\n> +\tcat >todo <<-\\EOF &&\n> +\tpick fixup-base fixup-base\n> +\tfixup fixup-second fixup-second\n> +\tEOF\n> +\t(\n> +\t\tset_replace_editor todo &&\n> +\t\tgit rebase -i --trailer \"$REVIEWED_BY_TRAILER\" HEAD~2\n> +\t) &&\n> +\ttest_commit_message HEAD <<-EOF &&\n> +\tfixup-base\n> +\n> +\t$REVIEWED_BY_TRAILER\n> +\tEOF\n> +\tgit reset --hard fixup-second &&\n> +\tcat >todo <<-\\EOF &&\n> +\tpick fixup-base fixup-base\n> +\tfixup -C fixup-second fixup-second\n> +\tEOF\n> +\t(\n> +\t\tset_replace_editor todo &&\n> +\t\tgit rebase -i --trailer \"$REVIEWED_BY_TRAILER\" HEAD~2\n> +\t) &&\n> +\ttest_commit_message HEAD <<-EOF\n> +\tfixup-second\n> +\n> +\t$REVIEWED_BY_TRAILER\n> +\tEOF\n> +'\n> +\n> +test_expect_success 'rebase --root honors trailer..key' '\n> +\tgit checkout -B root-trailer first &&\n> +\tgit -c trailer.review.key=Reviewed-by rebase --root \\\n> +\t\t--trailer=review=\"Dev \" &&\n> +\ttest_commit_message HEAD <<-EOF &&\n> +\tfirst\n> +\n> +\tReviewed-by: Dev \n> +\tEOF\n> +\ttest_commit_message HEAD^ <<-EOF\n> +\tInitial empty commit\n> +\n> +\tReviewed-by: Dev \n> +\tEOF\n> +'\n> +test_done\n\n"},{"id":"537730","messageId":"22e1de8e-935d-4efa-9fa8-ef8d9b4ffc6a@app.fastmail.com","threadId":"65068","inReplyTo":"824809c3-72ac-43fb-8a93-4f48e0727e6a@gmail.com","subject":"Re: [PATCH v7 5/5] rebase: support --trailer","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-03-03T20:36:53Z","receivedAt":"2026-03-03T20:37:21Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Tue, Mar 3, 2026, at 16:05, Phillip Wood wrote:\n>>[snip]\n>> diff --git a/sequencer.c b/sequencer.c\n>> index a3eb39bb25..a60c2a0cde 100644\n>> --- a/sequencer.c\n>> +++ b/sequencer.c\n>> [...]\n>> @@ -2025,6 +2027,9 @@ static int append_squash_message(struct strbuf *buf, const char *body,\n>> \t\tif (opts->signoff)\n>> \t\t\tappend_signoff(buf, 0, 0);\n>>\n>> +\t\tif (opts->trailer_args.nr)\n>> +\t\t\tamend_strbuf_with_trailers(buf, &opts->trailer_args);\n>\n> I wonder if it would be better to add the trailers before the signoff so\n> that \"git rebase --signoff --trailer='Reviewed-by: ...'\" adds the\n> \"Reviewed-by:\" trailer before the \"Signed-off-by:\" trailer.\n\nWhy is that? Is that because that is the practice in this project (and\nmaybe others)?\n\nI would expect it to act like however `--trailer` already acts on\ngit-commit(1) and git-tag(1). I would have to test that.\n\nIn any case these `--signoff` options are considered a historical\nmistake now (since they special-case one key).\n\nThe logic for before/after and so on are supposed to be handled by the\ntrailer config, it seems. But last I looked that was only for same-key\ntrailers and duplicates. Not for logic like keeping your own signoff\nlast.\n\n>[snip]\n"},{"id":"537736","messageId":"xmqqqzq0boi8.fsf@gitster.g","threadId":"65068","inReplyTo":"22e1de8e-935d-4efa-9fa8-ef8d9b4ffc6a@app.fastmail.com","subject":"Re: [PATCH v7 5/5] rebase: support --trailer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-03T21:18:39Z","receivedAt":"2026-03-03T21:18:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristoffer Haugsbakk\" writes:\n\n>> I wonder if it would be better to add the trailers before the signoff so\n>> that \"git rebase --signoff --trailer='Reviewed-by: ...'\" adds the\n>> \"Reviewed-by:\" trailer before the \"Signed-off-by:\" trailer.\n>\n> Why is that? Is that because that is the practice in this project (and\n> maybe others)?\n\nI do not think it is a good idea for the above sample command, where\nwe can argue that the intent of the user is to have sign-off and\nthen reviewed-by, expressed in the order of options given.\n\nIf we want to control where the new trailers are added, perhaps we\nwould need to match the --where option interpret-trailers has and\nlet the configuration honored by that command take care of the\nordering.\n"},{"id":"537781","messageId":"dbaf6ea5-8a08-42c0-8184-16dcf40207dd@gmail.com","threadId":"65068","inReplyTo":"20260224070552.148591-1-me@linux.beauty","subject":"Re: [PATCH v7 0/5] rebase: support --trailer","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-04T14:29:42Z","receivedAt":"2026-03-04T14:29:50Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Li\n\nOn 24/02/2026 07:05, Li Chen wrote:\n> Apologies for the long delay in sending v7.\n> \n> v7 is based on origin/master at v2.53.0-154-g7c02d39fc2.\n> \n> This series routes trailer insertion through an in-process path, removing the\n> fork/exec to builtin/interpret-trailers.\n> \n> The first four commits refactor trailer rewriting in builtin/interpret-trailers\n> and trailer.c so callers can reuse a single in-process helper (used by git\n> interpret-trailers, git commit and git tag). The final commit adds git rebase\n> --trailer, currently supported with the merge backend only (rejecting apply-only\n> scenarios and validating input early).\n\nThis all looks pretty good. The main thing I think we want to change is\nsharing the code that creates the temporary file in patch 3. I've push a\nversion that does that and fixes my other small comments to\nhttps://github.com/phillipwood/git/commits/refs/heads/rebase-trailers-v8\nThe range diff is below. If you're happy I can post that as a hopefully\nfinal v8. Of course if you want to work on it yourself you're very\nwelcome to do that.\n\nThanks\n\nPhillip\n\n1: b2685e34c22 ! 1: 0d08b361995 interpret-trailers: factor trailer rewriting\n @@ Commit message\n This separation makes it easier to move the helper into trailer.c in the\n next commit.\n \n + Signed-off-by: Phillip Wood \n Signed-off-by: Li Chen \n \n ## builtin/interpret-trailers.c ##\n @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)\n-: ----------- > 2: 5a4d03ab375 interpret-trailers: refactor create_in_place_tempfile()\n2: 1bac3025045 ! 3: ab7e232a95d trailer: move process_trailers to trailer.h\n @@ Metadata\n Author: Li Chen \n \n ## Commit message ##\n - trailer: move process_trailers to trailer.h\n -\n - Move process_trailers() from builtin/interpret-trailers.c into trailer.c\n - and expose it via trailer.h.\n -\n - This lets other call sites reuse the same trailer rewriting logic.\n + trailer: libify a couple of functions\n +\n + Move create_in_place_tempfile() and process_trailers() from\n + builtin/interpret-trailers.c into trailer.c and expose it via trailer.h.\n +\n + This reverts most of ae0ec2e0e0b (trailer: move interpret_trailers()\n + to interpret-trailers.c, 2024-03-01) and lets other call sites reuse\n + the same trailer rewriting logic.\n \n Signed-off-by: Li Chen \n + Signed-off-by: Phillip Wood \n \n ## builtin/interpret-trailers.c ##\n +@@ builtin/interpret-trailers.c: static int parse_opt_parse(const struct option *opt, const char *arg,\n + \treturn 0;\n + }\n +\n +-\n +-static struct tempfile *create_in_place_tempfile(const char *file)\n +-{\n +-\tstruct tempfile *tempfile = NULL;\n +-\tstruct stat st;\n +-\tstruct strbuf filename_template = STRBUF_INIT;\n +-\tconst char *tail;\n +-\n +-\tif (stat(file, &st)) {\n +-\t\terror_errno(_(\"could not stat %s\"), file);\n +-\t\treturn NULL;\n +-\t}\n +-\tif (!S_ISREG(st.st_mode)) {\n +-\t\terror(_(\"file %s is not a regular file\"), file);\n +-\t\treturn NULL;\n +-\t}\n +-\tif (!(st.st_mode & S_IWUSR)) {\n +-\t\terror(_(\"file %s is not writable by user\"), file);\n +-\t\treturn NULL;\n +-\t}\n +-\t/* Create temporary file in the same directory as the original */\n +-\ttail = find_last_dir_sep(file);\n +-\tif (tail)\n +-\t\tstrbuf_add(&filename_template, file, tail - file + 1);\n +-\tstrbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n +-\n +-\ttempfile = mks_tempfile_m(filename_template.buf, st.st_mode);\n +-\n +-\tstrbuf_release(&filename_template);\n +-\n +-\treturn tempfile;\n +-}\n +-\n + static void read_input_file(struct strbuf *sb, const char *file)\n + {\n + \tif (file) {\n @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)\n \tstrbuf_complete_line(sb);\n }\n @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, con\n static void interpret_trailers(const struct process_trailer_options *opts,\n \t\t\t struct list_head *new_trailer_head,\n \t\t\t const char *file)\n +@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,\n + \tread_input_file(&input, file);\n +\n + \tif (opts->in_place) {\n +-\t\ttempfile = create_in_place_tempfile(file);\n ++\t\ttempfile = trailer_create_in_place_tempfile(file);\n + \t\tif (!tempfile)\n + \t\t\tdie(NULL);\n + \t\tfd = tempfile->fd;\n \n ## trailer.c ##\n +@@\n + #include \"commit.h\"\n + #include \"trailer.h\"\n + #include \"list.h\"\n ++#include \"tempfile.h\"\n ++\n + /*\n + * Copyright (c) 2013, 2014 Christian Couder \n + */\n +@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n + \tstrbuf_release(&iter->key);\n + }\n +\n ++struct tempfile *trailer_create_in_place_tempfile(const char *file)\n ++{\n ++\tstruct tempfile *tempfile = NULL;\n ++\tstruct stat st;\n ++\tstruct strbuf filename_template = STRBUF_INIT;\n ++\tconst char *tail;\n ++\n ++\tif (stat(file, &st)) {\n ++\t\terror_errno(_(\"could not stat %s\"), file);\n ++\t\treturn NULL;\n ++\t}\n ++\tif (!S_ISREG(st.st_mode)) {\n ++\t\terror(_(\"file %s is not a regular file\"), file);\n ++\t\treturn NULL;\n ++\t}\n ++\tif (!(st.st_mode & S_IWUSR)) {\n ++\t\terror(_(\"file %s is not writable by user\"), file);\n ++\t\treturn NULL;\n ++\t}\n ++\t/* Create temporary file in the same directory as the original */\n ++\ttail = find_last_dir_sep(file);\n ++\tif (tail)\n ++\t\tstrbuf_add(&filename_template, file, tail - file + 1);\n ++\tstrbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n ++\n ++\ttempfile = mks_tempfile_m(filename_template.buf, st.st_mode);\n ++\n ++\tstrbuf_release(&filename_template);\n ++\n ++\treturn tempfile;\n ++}\n ++\n + int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)\n + {\n + \tstruct child_process run_trailer = CHILD_PROCESS_INIT;\n @@ trailer.c: int amend_file_with_trailers(const char *path, const struct strvec *trailer_args\n \tstrvec_pushv(&run_trailer.args, trailer_args->v);\n \treturn run_command(&run_trailer);\n @@ trailer.h: 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 ++ * Create a tempfile \"\"git-interpret-trailers-XXXXXX\" in the same\n ++ * directory as file.\n ++ */\n ++struct tempfile *trailer_create_in_place_tempfile(const char *file);\n ++\n ++/*\n ++ * Rewrite the contents of input by processing its trailer block according to\n ++ * opts and (optionally) appending trailers from new_trailer_head.\n ++ *\n ++ * The rewritten message is appended to out (callers should strbuf_reset()\n ++ * first if needed).\n ++ */\n +void process_trailers(const struct process_trailer_options *opts,\n +\t\t struct list_head *new_trailer_head,\n +\t\t struct strbuf *input, struct strbuf *out);\n3: 3114f0dbb57 ! 4: 1f24917eb64 trailer: append trailers without fork/exec\n @@ Commit message\n \n Update amend_file_with_trailers() to use the in-process helper and\n rewrite the target file via tempfile+rename, preserving the previous\n - in-place semantics.\n + in-place semantics. As the trailers are no longer added in a separate\n + process and trailer_config_init() die()s on missing config values it\n + is called early on in cmd_commit() and cmd_tag() so that they die()\n + early before writing the message file. The trailer arguments are now\n + also sanity checked.\n \n Keep existing callers unchanged by continuing to accept argv-style\n --trailer= entries and stripping the prefix before feeding the\n in-process implementation.\n \n Signed-off-by: Li Chen \n + Signed-off-by: Phillip Wood \n -\n - ## builtin/interpret-trailers.c ##\n -@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,\n - \tstruct strbuf out = STRBUF_INIT;\n - \tFILE *outfile = stdout;\n -\n --\ttrailer_config_init();\n --\n - \tread_input_file(&input, file);\n -\n - \tif (opts->in_place)\n -@@ builtin/interpret-trailers.c: int cmd_interpret_trailers(int argc,\n - \t\t\tgit_interpret_trailers_usage,\n - \t\t\toptions);\n -\n -+\ttrailer_config_init();\n -+\n - \tif (argc) {\n - \t\tint i;\n - \t\tfor (i = 0; i < argc; i++)\n + Signed-off-by: Phillip Wood \n +\n + ## builtin/commit.c ##\n +@@ builtin/commit.c: int cmd_commit(int argc,\n + \targc = parse_and_validate_options(argc, argv, builtin_commit_options,\n + \t\t\t\t\t builtin_commit_usage,\n + \t\t\t\t\t prefix, current_head, &s);\n ++\tif (trailer_args.nr)\n ++\t\ttrailer_config_init();\n ++\n + \tif (verbose == -1)\n + \t\tverbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n +\n +\n + ## builtin/tag.c ##\n +@@ builtin/tag.c: int cmd_tag(int argc,\n + \tif (cmdmode == 'l')\n + \t\tsetup_auto_pager(\"tag\", 1);\n +\n ++\tif (trailer_args.nr)\n ++\t\ttrailer_config_init();\n ++\n + \tif (opt.sign == -1)\n + \t\topt.sign = cmdmode ? 0 : config_sign_tag > 0;\n +\n \n ## trailer.c ##\n @@\n #include \"string-list.h\"\n #include \"run-command.h\"\n #include \"commit.h\"\n +#include \"strvec.h\"\n -+#include \"tempfile.h\"\n #include \"trailer.h\"\n #include \"list.h\"\n -+\n - /*\n - * Copyright (c) 2013, 2014 Christian Couder \n - */\n + #include \"tempfile.h\"\n @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head,\n \tfree(cl_separators);\n }\n \n -+void validate_trailer_args(const struct strvec *cli_args)\n ++int validate_trailer_args(const struct strvec *cli_args)\n +{\n +\tchar *cl_separators;\n ++\tint ret = 0;\n +\n +\ttrailer_config_init();\n +\n @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head\n +\t\tconst char *txt = cli_args->v[i];\n +\t\tssize_t separator_pos;\n +\n -+\t\tif (!*txt)\n -+\t\t\tdie(_(\"empty --trailer argument\"));\n -+\n ++\t\tif (!*txt) {\n ++\t\t\tret = error(_(\"empty --trailer argument\"));\n ++\t\t\tgoto out;\n ++\t\t}\n +\t\tseparator_pos = find_separator(txt, cl_separators);\n -+\t\tif (separator_pos == 0)\n -+\t\t\tdie(_(\"invalid trailer '%s': missing key before separator\"),\n -+\t\t\t txt);\n ++\t\tif (separator_pos == 0) {\n ++\t\t\tret = error(_(\"invalid trailer '%s': missing key before separator\"),\n ++\t\t\t\t txt);\n ++\t\t\tgoto out;\n ++\t\t}\n +\t}\n -+\n ++out:\n +\tfree(cl_separators);\n ++\treturn ret;\n +}\n +\n static const char *next_line(const char *str)\n {\n \tconst char *nl = strchrnul(str, '\\n');\n -@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n - \tstrbuf_release(&iter->key);\n +@@ trailer.c: struct tempfile *trailer_create_in_place_tempfile(const char *file)\n + \treturn tempfile;\n }\n \n -int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)\n @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n -\t\t path, NULL);\n -\tstrvec_pushv(&run_trailer.args, trailer_args->v);\n -\treturn run_command(&run_trailer);\n -+static void new_trailer_items_clear(struct list_head *items)\n -+{\n -+\twhile (!list_empty(items)) {\n -+\t\tstruct new_trailer_item *item =\n -+\t\t\tlist_first_entry(items, struct new_trailer_item, list);\n -+\t\tlist_del(&item->list);\n -+\t\tfree(item);\n -+\t}\n -+}\n -+\n -+void amend_strbuf_with_trailers(struct strbuf *buf,\n ++int amend_strbuf_with_trailers(struct strbuf *buf,\n +\t\t\t\tconst struct strvec *trailer_args)\n +{\n +\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n +\tLIST_HEAD(new_trailer_head);\n +\tstruct strbuf out = STRBUF_INIT;\n +\tsize_t i;\n ++\tint ret = 0;\n +\n +\topts.no_divider = 1;\n +\n +\tfor (i = 0; i < trailer_args->nr; i++) {\n +\t\tconst char *text = trailer_args->v[i];\n +\t\tstruct new_trailer_item *item;\n +\n -+\t\tif (!*text)\n -+\t\t\tdie(_(\"empty --trailer argument\"));\n ++\t\tif (!*text) {\n ++\t\t\tret = error(_(\"empty --trailer argument\"));\n ++\t\t\tgoto out;\n ++\t\t}\n +\t\titem = xcalloc(1, sizeof(*item));\n -+\t\titem->text = text;\n ++\t\titem->text = xstrdup(text);\n +\t\tlist_add_tail(&item->list, &new_trailer_head);\n +\t}\n +\n -+\ttrailer_config_init();\n +\tprocess_trailers(&opts, &new_trailer_head, buf, &out);\n +\n +\tstrbuf_swap(buf, &out);\n ++out:\n +\tstrbuf_release(&out);\n ++\tfree_trailers(&new_trailer_head);\n +\n -+\tnew_trailer_items_clear(&new_trailer_head);\n ++\treturn ret;\n +}\n +\n +static int write_file_in_place(const char *path, const struct strbuf *buf)\n +{\n -+\tstruct stat st;\n -+\tstruct strbuf filename_template = STRBUF_INIT;\n -+\tconst char *tail;\n -+\tstruct tempfile *tempfile;\n -+\tFILE *outfile;\n -+\n -+\tif (stat(path, &st))\n -+\t\treturn error_errno(_(\"could not stat %s\"), path);\n -+\tif (!S_ISREG(st.st_mode))\n -+\t\treturn error(_(\"file %s is not a regular file\"), path);\n -+\tif (!(st.st_mode & S_IWUSR))\n -+\t\treturn error(_(\"file %s is not writable by user\"), path);\n -+\n -+\t/* Create temporary file in the same directory as the original */\n -+\ttail = strrchr(path, '/');\n -+\tif (tail)\n -+\t\tstrbuf_add(&filename_template, path, tail - path + 1);\n -+\tstrbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n -+\n -+\ttempfile = mks_tempfile_sm(filename_template.buf, 0, st.st_mode);\n -+\tstrbuf_release(&filename_template);\n ++\tstruct tempfile *tempfile = trailer_create_in_place_tempfile(path);\n +\tif (!tempfile)\n -+\t\treturn error_errno(_(\"could not create temporary file\"));\n -+\n -+\toutfile = fdopen_tempfile(tempfile, \"w\");\n -+\tif (!outfile) {\n -+\t\tint saved_errno = errno;\n -+\t\tdelete_tempfile(&tempfile);\n -+\t\terrno = saved_errno;\n -+\t\treturn error_errno(_(\"could not open temporary file\"));\n -+\t}\n -+\n -+\tif (buf->len && fwrite(buf->buf, 1, buf->len, outfile) < buf->len) {\n -+\t\tint saved_errno = errno;\n -+\t\tdelete_tempfile(&tempfile);\n -+\t\terrno = saved_errno;\n ++\t\treturn -1;\n ++\n ++\tif (write_in_full(tempfile->fd, buf->buf, buf->len) < 0)\n +\t\treturn error_errno(_(\"could not write to temporary file\"));\n -+\t}\n +\n +\tif (rename_tempfile(&tempfile, path))\n +\t\treturn error_errno(_(\"could not rename temporary file to %s\"), path);\n @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n +\t\t * in-process implementation.\n +\t\t */\n +\t\tskip_prefix(txt, \"--trailer=\", &txt);\n -+\t\tif (!*txt)\n -+\t\t\tdie(_(\"empty --trailer argument\"));\n ++\t\tif (!*txt) {\n ++\t\t\tret = error(_(\"empty --trailer argument\"));\n ++\t\t\tgoto out;\n ++\t\t}\n +\t\tstrvec_push(&stripped_trailer_args, txt);\n +\t}\n +\n ++\tif (validate_trailer_args(&stripped_trailer_args)) {\n ++\t\tret = -1;\n ++\t\tgoto out;\n ++\t}\n +\tif (strbuf_read_file(&buf, path, 0) < 0)\n +\t\tret = error_errno(_(\"could not read '%s'\"), path);\n +\telse\n +\t\tamend_strbuf_with_trailers(&buf, &stripped_trailer_args);\n +\n +\tif (!ret)\n +\t\tret = write_file_in_place(path, &buf);\n -+\n ++out:\n +\tstrvec_clear(&stripped_trailer_args);\n +\tstrbuf_release(&buf);\n +\treturn ret;\n @@ trailer.h: void parse_trailers_from_config(struct list_head *config_head);\n void parse_trailers_from_command_line_args(struct list_head *arg_head,\n \t\t\t\t\t struct list_head *new_trailer_head);\n \n -+void validate_trailer_args(const struct strvec *cli_args);\n ++int validate_trailer_args(const struct strvec *cli_args);\n +\n void process_trailers_lists(struct list_head *head,\n \t\t\t struct list_head *arg_head);\n @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);\n + * Each element of trailer_args should be in the same format as the value\n + * accepted by --trailer= (i.e., without the --trailer= prefix).\n + */\n -+void amend_strbuf_with_trailers(struct strbuf *buf,\n ++int amend_strbuf_with_trailers(struct strbuf *buf,\n +\t\t\t\tconst struct strvec *trailer_args);\n +\n +/*\n @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);\n */\n int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);\n \n -+/*\n -+ * Rewrite the contents of input by processing its trailer block according to\n -+ * opts and (optionally) appending trailers from new_trailer_head.\n -+ *\n -+ * The rewritten message is appended to out (callers should strbuf_reset()\n -+ * first if needed).\n -+ */\n - void process_trailers(const struct process_trailer_options *opts,\n - \t\t struct list_head *new_trailer_head,\n - \t\t struct strbuf *input, struct strbuf *out);\n4: 147595a9317 ! 5: 3c1fa9e8579 commit, tag: parse --trailer with OPT_STRVEC\n @@ Commit message\n amend_file_with_trailers().\n \n Signed-off-by: Li Chen \n \n ## builtin/commit.c ##\n @@ builtin/commit.c: int cmd_commit(int argc,\n @@ trailer.c: int amend_file_with_trailers(const char *path,\n -\t\t * in-process implementation.\n -\t\t */\n -\t\tskip_prefix(txt, \"--trailer=\", &txt);\n --\t\tif (!*txt)\n --\t\t\tdie(_(\"empty --trailer argument\"));\n +-\t\tif (!*txt) {\n +-\t\t\tret = error(_(\"empty --trailer argument\"));\n +-\t\t\tgoto out;\n +-\t\t}\n -\t\tstrvec_push(&stripped_trailer_args, txt);\n -\t}\n -\n +-\tif (validate_trailer_args(&stripped_trailer_args)) {\n ++\tif (validate_trailer_args(trailer_args)) {\n + \t\tret = -1;\n + \t\tgoto out;\n + \t}\n \tif (strbuf_read_file(&buf, path, 0) < 0)\n \t\tret = error_errno(_(\"could not read '%s'\"), path);\n \telse\n @@ trailer.c: int amend_file_with_trailers(const char *path,\n \n \tif (!ret)\n \t\tret = write_file_in_place(path, &buf);\n -\n + out:\n -\tstrvec_clear(&stripped_trailer_args);\n \tstrbuf_release(&buf);\n \treturn ret;\n }\n \n ## trailer.h ##\n -@@ trailer.h: void amend_strbuf_with_trailers(struct strbuf *buf,\n +@@ trailer.h: int amend_strbuf_with_trailers(struct strbuf *buf,\n /*\n * Augment a file by appending trailers specified in trailer_args.\n *\n5: 864cf5f8eb6 ! 6: 99654d80547 rebase: support --trailer\n @@ Commit message\n non-interactive and interactive rebases.\n \n Signed-off-by: Li Chen \n + Signed-off-by: Phillip Wood \n \n ## Documentation/git-rebase.adoc ##\n @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.\n @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.\n +--trailer=::\n +\tAppend the given trailer to every rebased commit message, processed\n +\tvia linkgit:git-interpret-trailers[1]. This option implies\n -+\t`--force-rebase` so that fast-forwarded commits are also rewritten.\n ++\t`--force-rebase`.\n ++\n +See also INCOMPATIBLE OPTIONS below.\n +\n -i::\n --interactive::\n \tMake a list of the commits which are about to be rebased. Let the\n +@@ Documentation/git-rebase.adoc: are incompatible with the following options:\n + * --[no-]reapply-cherry-picks when used without --keep-base\n + * --update-refs\n + * --root when used without --onto\n ++ * --trailer\n +\n + In addition, the following pairs of options are incompatible:\n +\n \n ## builtin/rebase.c ##\n @@\n @@ builtin/rebase.c: int cmd_rebase(int argc,\n \t\t\t builtin_rebase_usage, 0);\n \n +\tif (options.trailer_args.nr) {\n -+\t\tvalidate_trailer_args(&options.trailer_args);\n ++\t\tif (validate_trailer_args(&options.trailer_args))\n ++\t\t\tdie(NULL);\n +\t\toptions.flags |= REBASE_FORCE;\n +\t}\n +\n @@ sequencer.c: void replay_opts_release(struct replay_opts *opts)\n \tfree(opts->ctx);\n }\n @@ sequencer.c: static int append_squash_message(struct strbuf *buf, const char *body,\n + \tif (is_fixup_flag(command, flag) && !seen_squash(ctx)) {\n + \t\t/*\n + \t\t * We're replacing the commit message so we need to\n +-\t\t * append the Signed-off-by: trailer if the user\n +-\t\t * requested '--signoff'.\n ++\t\t * append any trailers if the user requested\n ++\t\t * '--signoff' or '--trailer'.\n + \t\t */\n \t\tif (opts->signoff)\n \t\t\tappend_signoff(buf, 0, 0);\n \n @@ sequencer.c: static int do_pick_commit(struct repository *r,\n \tif (is_rebase_i(opts) && write_author_script(msg.message) < 0)\n \t\tres = -1;\n \telse if (!opts->strategy ||\n +@@ sequencer.c: static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)\n + \tparse_strategy_opts(opts, buf->buf);\n + }\n +\n ++static int read_trailers(struct replay_opts *opts, struct strbuf *buf)\n ++{\n ++\tssize_t len;\n ++\n ++\tstrbuf_reset(buf);\n ++\tlen = strbuf_read_file(buf, rebase_path_trailer(), 0);\n ++\tif (len > 0) {\n ++\t\tchar *p = buf->buf, *nl;\n ++\n ++\t\ttrailer_config_init();\n ++\n ++\t\twhile ((nl = strchr(p, '\\n'))) {\n ++\t\t\t*nl = '\\0';\n ++\t\t\tif (!*p)\n ++\t\t\t\treturn error(_(\"trailers file contains empty line\"));\n ++\t\t\tstrvec_push(&opts->trailer_args, p);\n ++\t\t\tp = nl + 1;\n ++\t\t}\n ++\t} else if (!len) {\n ++\t\treturn error(_(\"trailers file is empty\"));\n ++\t} else if (errno != ENOENT) {\n ++\t\treturn error(_(\"cannot read trailers files\"));\n ++\t}\n ++\n ++\treturn 0;\n ++}\n ++\n + static int read_populate_opts(struct replay_opts *opts)\n + {\n + \tstruct replay_ctx *ctx = opts->ctx;\n @@ sequencer.c: static int read_populate_opts(struct replay_opts *opts)\n + \t\t\topts->keep_redundant_commits = 1;\n \n \t\tread_strategy_opts(opts, &buf);\n ++\n ++\t\tif (read_trailers(opts, &buf)) {\n ++\t\t\tret = -1;\n ++\t\t\tgoto done_rebase_i;\n ++\t\t}\n \t\tstrbuf_reset(&buf);\n -+\t\tif (strbuf_read_file(&buf, rebase_path_trailer(), 0) >= 0) {\n -+\t\t\tchar *p = buf.buf, *nl;\n -+\n -+\t\t\twhile ((nl = strchr(p, '\\n'))) {\n -+\t\t\t\t*nl = '\\0';\n -+\t\t\t\tif (!*p)\n -+\t\t\t\t\tBUG(\"rebase-merge/trailer has an empty line\");\n -+\t\t\t\tstrvec_push(&opts->trailer_args, p);\n -+\t\t\t\tp = nl + 1;\n -+\t\t\t}\n -+\t\t\tstrbuf_reset(&buf);\n -+\t\t}\n \n \t\tif (read_oneliner(&ctx->current_fixups,\n - \t\t\t\t rebase_path_current_fixups(),\n @@ sequencer.c: int write_basic_state(struct replay_opts *opts, const char *head_name,\n \t\twrite_file(rebase_path_reschedule_failed_exec(), \"%s\", \"\");\n \telse\n\n"},{"id":"537786","messageId":"ac09b00d-0566-405e-b7ac-b091448e0f4b@gmail.com","threadId":"65068","inReplyTo":"xmqqqzq0boi8.fsf@gitster.g","subject":"Re: [PATCH v7 5/5] rebase: support --trailer","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-04T15:53:07Z","receivedAt":"2026-03-04T15:53:16Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 03/03/2026 21:18, Junio C Hamano wrote:\n> \"Kristoffer Haugsbakk\" writes:\n> \n>>> I wonder if it would be better to add the trailers before the signoff so\n>>> that \"git rebase --signoff --trailer='Reviewed-by: ...'\" adds the\n>>> \"Reviewed-by:\" trailer before the \"Signed-off-by:\" trailer.\n>>\n>> Why is that? Is that because that is the practice in this project (and\n>> maybe others)?\n> \n> I do not think it is a good idea for the above sample command, where\n> we can argue that the intent of the user is to have sign-off and\n> then reviewed-by, expressed in the order of options given.\n> \n> If we want to control where the new trailers are added, perhaps we\n> would need to match the --where option interpret-trailers has and\n> let the configuration honored by that command take care of the\n> ordering.\n\nLet's leave it as it is for now as that matches what \"git commit \n--signoff --trailer=...\" does and no one has complained about that.\n\nThanks\n\nPhillip\n\n"},{"id":"537794","messageId":"xmqqseaf8q7v.fsf@gitster.g","threadId":"65068","inReplyTo":"ac09b00d-0566-405e-b7ac-b091448e0f4b@gmail.com","subject":"Re: [PATCH v7 5/5] rebase: support --trailer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-04T17:22:12Z","receivedAt":"2026-03-04T17:22:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> On 03/03/2026 21:18, Junio C Hamano wrote:\n>> \"Kristoffer Haugsbakk\" writes:\n>> \n>>>> I wonder if it would be better to add the trailers before the signoff so\n>>>> that \"git rebase --signoff --trailer='Reviewed-by: ...'\" adds the\n>>>> \"Reviewed-by:\" trailer before the \"Signed-off-by:\" trailer.\n>>>\n>>> Why is that? Is that because that is the practice in this project (and\n>>> maybe others)?\n>> \n>> I do not think it is a good idea for the above sample command, where\n>> we can argue that the intent of the user is to have sign-off and\n>> then reviewed-by, expressed in the order of options given.\n>> \n>> If we want to control where the new trailers are added, perhaps we\n>> would need to match the --where option interpret-trailers has and\n>> let the configuration honored by that command take care of the\n>> ordering.\n>\n> Let's leave it as it is for now as that matches what \"git commit \n> --signoff --trailer=...\" does and no one has complained about that.\n\nFair enough.\n"},{"id":"537954","messageId":"19cbe42b2cd.bb6e3883730656.6495265672263159010@linux.beauty","threadId":"65068","inReplyTo":"dbaf6ea5-8a08-42c0-8184-16dcf40207dd@gmail.com","subject":"Re: [PATCH v7 0/5] rebase: support --trailer","fromName":"Li Chen","fromEmail":"me@linux.beauty","sentAt":"2026-03-05T13:49:28Z","receivedAt":"2026-03-05T13:49:40Z","isPatch":true,"sender":{"key":"me@linux.beauty","avatar":"https://avatars.githubusercontent.com/u/37442588?v=4"},"body":"Hi Phillip,\n\n ---- On Wed, 04 Mar 2026 22:29:42 +0800 Phillip Wood wrote --- \n > Hi Li\n > \n > On 24/02/2026 07:05, Li Chen wrote:\n > > Apologies for the long delay in sending v7.\n > > \n > > v7 is based on origin/master at v2.53.0-154-g7c02d39fc2.\n > > \n > > This series routes trailer insertion through an in-process path, removing the\n > > fork/exec to builtin/interpret-trailers.\n > > \n > > The first four commits refactor trailer rewriting in builtin/interpret-trailers\n > > and trailer.c so callers can reuse a single in-process helper (used by git\n > > interpret-trailers, git commit and git tag). The final commit adds git rebase\n > > --trailer, currently supported with the merge backend only (rejecting apply-only\n > > scenarios and validating input early).\n > \n > This all looks pretty good. The main thing I think we want to change is\n > sharing the code that creates the temporary file in patch 3. I've push a\n > version that does that and fixes my other small comments to\n > https://github.com/phillipwood/git/commits/refs/heads/rebase-trailers-v8\n > The range diff is below. If you're happy I can post that as a hopefully\n > final v8. Of course if you want to work on it yourself you're very\n > welcome to do that.\n\nThese changes all look good to me! Thank you very much for your patience and work! Please help \npost this as v8.\n\nRegards,\nLi\n\n > \n > 1: b2685e34c22 ! 1: 0d08b361995 interpret-trailers: factor trailer rewriting\n > @@ Commit message\n > This separation makes it easier to move the helper into trailer.c in the\n > next commit.\n > \n > + Signed-off-by: Phillip Wood \n > Signed-off-by: Li Chen \n > \n > ## builtin/interpret-trailers.c ##\n > @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)\n > -: ----------- > 2: 5a4d03ab375 interpret-trailers: refactor create_in_place_tempfile()\n > 2: 1bac3025045 ! 3: ab7e232a95d trailer: move process_trailers to trailer.h\n > @@ Metadata\n > Author: Li Chen \n > \n > ## Commit message ##\n > - trailer: move process_trailers to trailer.h\n > -\n > - Move process_trailers() from builtin/interpret-trailers.c into trailer.c\n > - and expose it via trailer.h.\n > -\n > - This lets other call sites reuse the same trailer rewriting logic.\n > + trailer: libify a couple of functions\n > +\n > + Move create_in_place_tempfile() and process_trailers() from\n > + builtin/interpret-trailers.c into trailer.c and expose it via trailer.h.\n > +\n > + This reverts most of ae0ec2e0e0b (trailer: move interpret_trailers()\n > + to interpret-trailers.c, 2024-03-01) and lets other call sites reuse\n > + the same trailer rewriting logic.\n > \n > Signed-off-by: Li Chen \n > + Signed-off-by: Phillip Wood \n > \n > ## builtin/interpret-trailers.c ##\n > +@@ builtin/interpret-trailers.c: static int parse_opt_parse(const struct option *opt, const char *arg,\n > + return 0;\n > + }\n > +\n > +-\n > +-static struct tempfile *create_in_place_tempfile(const char *file)\n > +-{\n > +- struct tempfile *tempfile = NULL;\n > +- struct stat st;\n > +- struct strbuf filename_template = STRBUF_INIT;\n > +- const char *tail;\n > +-\n > +- if (stat(file, &st)) {\n > +- error_errno(_(\"could not stat %s\"), file);\n > +- return NULL;\n > +- }\n > +- if (!S_ISREG(st.st_mode)) {\n > +- error(_(\"file %s is not a regular file\"), file);\n > +- return NULL;\n > +- }\n > +- if (!(st.st_mode & S_IWUSR)) {\n > +- error(_(\"file %s is not writable by user\"), file);\n > +- return NULL;\n > +- }\n > +- /* Create temporary file in the same directory as the original */\n > +- tail = find_last_dir_sep(file);\n > +- if (tail)\n > +- strbuf_add(&filename_template, file, tail - file + 1);\n > +- strbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n > +-\n > +- tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);\n > +-\n > +- strbuf_release(&filename_template);\n > +-\n > +- return tempfile;\n > +-}\n > +-\n > + static void read_input_file(struct strbuf *sb, const char *file)\n > + {\n > + if (file) {\n > @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)\n > strbuf_complete_line(sb);\n > }\n > @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, con\n > static void interpret_trailers(const struct process_trailer_options *opts,\n > struct list_head *new_trailer_head,\n > const char *file)\n > +@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,\n > + read_input_file(&input, file);\n > +\n > + if (opts->in_place) {\n > +- tempfile = create_in_place_tempfile(file);\n > ++ tempfile = trailer_create_in_place_tempfile(file);\n > + if (!tempfile)\n > + die(NULL);\n > + fd = tempfile->fd;\n > \n > ## trailer.c ##\n > +@@\n > + #include \"commit.h\"\n > + #include \"trailer.h\"\n > + #include \"list.h\"\n > ++#include \"tempfile.h\"\n > ++\n > + /*\n > + * Copyright (c) 2013, 2014 Christian Couder \n > + */\n > +@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n > + strbuf_release(&iter->key);\n > + }\n > +\n > ++struct tempfile *trailer_create_in_place_tempfile(const char *file)\n > ++{\n > ++ struct tempfile *tempfile = NULL;\n > ++ struct stat st;\n > ++ struct strbuf filename_template = STRBUF_INIT;\n > ++ const char *tail;\n > ++\n > ++ if (stat(file, &st)) {\n > ++ error_errno(_(\"could not stat %s\"), file);\n > ++ return NULL;\n > ++ }\n > ++ if (!S_ISREG(st.st_mode)) {\n > ++ error(_(\"file %s is not a regular file\"), file);\n > ++ return NULL;\n > ++ }\n > ++ if (!(st.st_mode & S_IWUSR)) {\n > ++ error(_(\"file %s is not writable by user\"), file);\n > ++ return NULL;\n > ++ }\n > ++ /* Create temporary file in the same directory as the original */\n > ++ tail = find_last_dir_sep(file);\n > ++ if (tail)\n > ++ strbuf_add(&filename_template, file, tail - file + 1);\n > ++ strbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n > ++\n > ++ tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);\n > ++\n > ++ strbuf_release(&filename_template);\n > ++\n > ++ return tempfile;\n > ++}\n > ++\n > + int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)\n > + {\n > + struct child_process run_trailer = CHILD_PROCESS_INIT;\n > @@ trailer.c: int amend_file_with_trailers(const char *path, const struct strvec *trailer_args\n > strvec_pushv(&run_trailer.args, trailer_args->v);\n > return run_command(&run_trailer);\n > @@ trailer.h: 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 > ++ * Create a tempfile \"\"git-interpret-trailers-XXXXXX\" in the same\n > ++ * directory as file.\n > ++ */\n > ++struct tempfile *trailer_create_in_place_tempfile(const char *file);\n > ++\n > ++/*\n > ++ * Rewrite the contents of input by processing its trailer block according to\n > ++ * opts and (optionally) appending trailers from new_trailer_head.\n > ++ *\n > ++ * The rewritten message is appended to out (callers should strbuf_reset()\n > ++ * first if needed).\n > ++ */\n > +void process_trailers(const struct process_trailer_options *opts,\n > + struct list_head *new_trailer_head,\n > + struct strbuf *input, struct strbuf *out);\n > 3: 3114f0dbb57 ! 4: 1f24917eb64 trailer: append trailers without fork/exec\n > @@ Commit message\n > \n > Update amend_file_with_trailers() to use the in-process helper and\n > rewrite the target file via tempfile+rename, preserving the previous\n > - in-place semantics.\n > + in-place semantics. As the trailers are no longer added in a separate\n > + process and trailer_config_init() die()s on missing config values it\n > + is called early on in cmd_commit() and cmd_tag() so that they die()\n > + early before writing the message file. The trailer arguments are now\n > + also sanity checked.\n > \n > Keep existing callers unchanged by continuing to accept argv-style\n > --trailer= entries and stripping the prefix before feeding the\n > in-process implementation.\n > \n > Signed-off-by: Li Chen \n > + Signed-off-by: Phillip Wood \n > -\n > - ## builtin/interpret-trailers.c ##\n > -@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,\n > - struct strbuf out = STRBUF_INIT;\n > - FILE *outfile = stdout;\n > -\n > -- trailer_config_init();\n > --\n > - read_input_file(&input, file);\n > -\n > - if (opts->in_place)\n > -@@ builtin/interpret-trailers.c: int cmd_interpret_trailers(int argc,\n > - git_interpret_trailers_usage,\n > - options);\n > -\n > -+ trailer_config_init();\n > -+\n > - if (argc) {\n > - int i;\n > - for (i = 0; i < argc; i++)\n > + Signed-off-by: Phillip Wood \n > +\n > + ## builtin/commit.c ##\n > +@@ builtin/commit.c: int cmd_commit(int argc,\n > + argc = parse_and_validate_options(argc, argv, builtin_commit_options,\n > + builtin_commit_usage,\n > + prefix, current_head, &s);\n > ++ if (trailer_args.nr)\n > ++ trailer_config_init();\n > ++\n > + if (verbose == -1)\n > + verbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n > +\n > +\n > + ## builtin/tag.c ##\n > +@@ builtin/tag.c: int cmd_tag(int argc,\n > + if (cmdmode == 'l')\n > + setup_auto_pager(\"tag\", 1);\n > +\n > ++ if (trailer_args.nr)\n > ++ trailer_config_init();\n > ++\n > + if (opt.sign == -1)\n > + opt.sign = cmdmode ? 0 : config_sign_tag > 0;\n > +\n > \n > ## trailer.c ##\n > @@\n > #include \"string-list.h\"\n > #include \"run-command.h\"\n > #include \"commit.h\"\n > +#include \"strvec.h\"\n > -+#include \"tempfile.h\"\n > #include \"trailer.h\"\n > #include \"list.h\"\n > -+\n > - /*\n > - * Copyright (c) 2013, 2014 Christian Couder \n > - */\n > + #include \"tempfile.h\"\n > @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head,\n > free(cl_separators);\n > }\n > \n > -+void validate_trailer_args(const struct strvec *cli_args)\n > ++int validate_trailer_args(const struct strvec *cli_args)\n > +{\n > + char *cl_separators;\n > ++ int ret = 0;\n > +\n > + trailer_config_init();\n > +\n > @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head\n > + const char *txt = cli_args->v[i];\n > + ssize_t separator_pos;\n > +\n > -+ if (!*txt)\n > -+ die(_(\"empty --trailer argument\"));\n > -+\n > ++ if (!*txt) {\n > ++ ret = error(_(\"empty --trailer argument\"));\n > ++ goto out;\n > ++ }\n > + separator_pos = find_separator(txt, cl_separators);\n > -+ if (separator_pos == 0)\n > -+ die(_(\"invalid trailer '%s': missing key before separator\"),\n > -+ txt);\n > ++ if (separator_pos == 0) {\n > ++ ret = error(_(\"invalid trailer '%s': missing key before separator\"),\n > ++ txt);\n > ++ goto out;\n > ++ }\n > + }\n > -+\n > ++out:\n > + free(cl_separators);\n > ++ return ret;\n > +}\n > +\n > static const char *next_line(const char *str)\n > {\n > const char *nl = strchrnul(str, '\\n');\n > -@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n > - strbuf_release(&iter->key);\n > +@@ trailer.c: struct tempfile *trailer_create_in_place_tempfile(const char *file)\n > + return tempfile;\n > }\n > \n > -int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)\n > @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n > - path, NULL);\n > - strvec_pushv(&run_trailer.args, trailer_args->v);\n > - return run_command(&run_trailer);\n > -+static void new_trailer_items_clear(struct list_head *items)\n > -+{\n > -+ while (!list_empty(items)) {\n > -+ struct new_trailer_item *item =\n > -+ list_first_entry(items, struct new_trailer_item, list);\n > -+ list_del(&item->list);\n > -+ free(item);\n > -+ }\n > -+}\n > -+\n > -+void amend_strbuf_with_trailers(struct strbuf *buf,\n > ++int amend_strbuf_with_trailers(struct strbuf *buf,\n > + const struct strvec *trailer_args)\n > +{\n > + struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n > + LIST_HEAD(new_trailer_head);\n > + struct strbuf out = STRBUF_INIT;\n > + size_t i;\n > ++ int ret = 0;\n > +\n > + opts.no_divider = 1;\n > +\n > + for (i = 0; i < trailer_args->nr; i++) {\n > + const char *text = trailer_args->v[i];\n > + struct new_trailer_item *item;\n > +\n > -+ if (!*text)\n > -+ die(_(\"empty --trailer argument\"));\n > ++ if (!*text) {\n > ++ ret = error(_(\"empty --trailer argument\"));\n > ++ goto out;\n > ++ }\n > + item = xcalloc(1, sizeof(*item));\n > -+ item->text = text;\n > ++ item->text = xstrdup(text);\n > + list_add_tail(&item->list, &new_trailer_head);\n > + }\n > +\n > -+ trailer_config_init();\n > + process_trailers(&opts, &new_trailer_head, buf, &out);\n > +\n > + strbuf_swap(buf, &out);\n > ++out:\n > + strbuf_release(&out);\n > ++ free_trailers(&new_trailer_head);\n > +\n > -+ new_trailer_items_clear(&new_trailer_head);\n > ++ return ret;\n > +}\n > +\n > +static int write_file_in_place(const char *path, const struct strbuf *buf)\n > +{\n > -+ struct stat st;\n > -+ struct strbuf filename_template = STRBUF_INIT;\n > -+ const char *tail;\n > -+ struct tempfile *tempfile;\n > -+ FILE *outfile;\n > -+\n > -+ if (stat(path, &st))\n > -+ return error_errno(_(\"could not stat %s\"), path);\n > -+ if (!S_ISREG(st.st_mode))\n > -+ return error(_(\"file %s is not a regular file\"), path);\n > -+ if (!(st.st_mode & S_IWUSR))\n > -+ return error(_(\"file %s is not writable by user\"), path);\n > -+\n > -+ /* Create temporary file in the same directory as the original */\n > -+ tail = strrchr(path, '/');\n > -+ if (tail)\n > -+ strbuf_add(&filename_template, path, tail - path + 1);\n > -+ strbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n > -+\n > -+ tempfile = mks_tempfile_sm(filename_template.buf, 0, st.st_mode);\n > -+ strbuf_release(&filename_template);\n > ++ struct tempfile *tempfile = trailer_create_in_place_tempfile(path);\n > + if (!tempfile)\n > -+ return error_errno(_(\"could not create temporary file\"));\n > -+\n > -+ outfile = fdopen_tempfile(tempfile, \"w\");\n > -+ if (!outfile) {\n > -+ int saved_errno = errno;\n > -+ delete_tempfile(&tempfile);\n > -+ errno = saved_errno;\n > -+ return error_errno(_(\"could not open temporary file\"));\n > -+ }\n > -+\n > -+ if (buf->len && fwrite(buf->buf, 1, buf->len, outfile) < buf->len) {\n > -+ int saved_errno = errno;\n > -+ delete_tempfile(&tempfile);\n > -+ errno = saved_errno;\n > ++ return -1;\n > ++\n > ++ if (write_in_full(tempfile->fd, buf->buf, buf->len) < 0)\n > + return error_errno(_(\"could not write to temporary file\"));\n > -+ }\n > +\n > + if (rename_tempfile(&tempfile, path))\n > + return error_errno(_(\"could not rename temporary file to %s\"), path);\n > @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n > + * in-process implementation.\n > + */\n > + skip_prefix(txt, \"--trailer=\", &txt);\n > -+ if (!*txt)\n > -+ die(_(\"empty --trailer argument\"));\n > ++ if (!*txt) {\n > ++ ret = error(_(\"empty --trailer argument\"));\n > ++ goto out;\n > ++ }\n > + strvec_push(&stripped_trailer_args, txt);\n > + }\n > +\n > ++ if (validate_trailer_args(&stripped_trailer_args)) {\n > ++ ret = -1;\n > ++ goto out;\n > ++ }\n > + if (strbuf_read_file(&buf, path, 0) < 0)\n > + ret = error_errno(_(\"could not read '%s'\"), path);\n > + else\n > + amend_strbuf_with_trailers(&buf, &stripped_trailer_args);\n > +\n > + if (!ret)\n > + ret = write_file_in_place(path, &buf);\n > -+\n > ++out:\n > + strvec_clear(&stripped_trailer_args);\n > + strbuf_release(&buf);\n > + return ret;\n > @@ trailer.h: void parse_trailers_from_config(struct list_head *config_head);\n > void parse_trailers_from_command_line_args(struct list_head *arg_head,\n > struct list_head *new_trailer_head);\n > \n > -+void validate_trailer_args(const struct strvec *cli_args);\n > ++int validate_trailer_args(const struct strvec *cli_args);\n > +\n > void process_trailers_lists(struct list_head *head,\n > struct list_head *arg_head);\n > @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);\n > + * Each element of trailer_args should be in the same format as the value\n > + * accepted by --trailer= (i.e., without the --trailer= prefix).\n > + */\n > -+void amend_strbuf_with_trailers(struct strbuf *buf,\n > ++int amend_strbuf_with_trailers(struct strbuf *buf,\n > + const struct strvec *trailer_args);\n > +\n > +/*\n > @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);\n > */\n > int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);\n > \n > -+/*\n > -+ * Rewrite the contents of input by processing its trailer block according to\n > -+ * opts and (optionally) appending trailers from new_trailer_head.\n > -+ *\n > -+ * The rewritten message is appended to out (callers should strbuf_reset()\n > -+ * first if needed).\n > -+ */\n > - void process_trailers(const struct process_trailer_options *opts,\n > - struct list_head *new_trailer_head,\n > - struct strbuf *input, struct strbuf *out);\n > 4: 147595a9317 ! 5: 3c1fa9e8579 commit, tag: parse --trailer with OPT_STRVEC\n > @@ Commit message\n > amend_file_with_trailers().\n > \n > Signed-off-by: Li Chen \n > \n > ## builtin/commit.c ##\n > @@ builtin/commit.c: int cmd_commit(int argc,\n > @@ trailer.c: int amend_file_with_trailers(const char *path,\n > - * in-process implementation.\n > - */\n > - skip_prefix(txt, \"--trailer=\", &txt);\n > -- if (!*txt)\n > -- die(_(\"empty --trailer argument\"));\n > +- if (!*txt) {\n > +- ret = error(_(\"empty --trailer argument\"));\n > +- goto out;\n > +- }\n > - strvec_push(&stripped_trailer_args, txt);\n > - }\n > -\n > +- if (validate_trailer_args(&stripped_trailer_args)) {\n > ++ if (validate_trailer_args(trailer_args)) {\n > + ret = -1;\n > + goto out;\n > + }\n > if (strbuf_read_file(&buf, path, 0) < 0)\n > ret = error_errno(_(\"could not read '%s'\"), path);\n > else\n > @@ trailer.c: int amend_file_with_trailers(const char *path,\n > \n > if (!ret)\n > ret = write_file_in_place(path, &buf);\n > -\n > + out:\n > - strvec_clear(&stripped_trailer_args);\n > strbuf_release(&buf);\n > return ret;\n > }\n > \n > ## trailer.h ##\n > -@@ trailer.h: void amend_strbuf_with_trailers(struct strbuf *buf,\n > +@@ trailer.h: int amend_strbuf_with_trailers(struct strbuf *buf,\n > /*\n > * Augment a file by appending trailers specified in trailer_args.\n > *\n > 5: 864cf5f8eb6 ! 6: 99654d80547 rebase: support --trailer\n > @@ Commit message\n > non-interactive and interactive rebases.\n > \n > Signed-off-by: Li Chen \n > + Signed-off-by: Phillip Wood \n > \n > ## Documentation/git-rebase.adoc ##\n > @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.\n > @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.\n > +--trailer=::\n > + Append the given trailer to every rebased commit message, processed\n > + via linkgit:git-interpret-trailers[1]. This option implies\n > -+ `--force-rebase` so that fast-forwarded commits are also rewritten.\n > ++ `--force-rebase`.\n > ++\n > +See also INCOMPATIBLE OPTIONS below.\n > +\n > -i::\n > --interactive::\n > Make a list of the commits which are about to be rebased. Let the\n > +@@ Documentation/git-rebase.adoc: are incompatible with the following options:\n > + * --[no-]reapply-cherry-picks when used without --keep-base\n > + * --update-refs\n > + * --root when used without --onto\n > ++ * --trailer\n > +\n > + In addition, the following pairs of options are incompatible:\n > +\n > \n > ## builtin/rebase.c ##\n > @@\n > @@ builtin/rebase.c: int cmd_rebase(int argc,\n > builtin_rebase_usage, 0);\n > \n > + if (options.trailer_args.nr) {\n > -+ validate_trailer_args(&options.trailer_args);\n > ++ if (validate_trailer_args(&options.trailer_args))\n > ++ die(NULL);\n > + options.flags |= REBASE_FORCE;\n > + }\n > +\n > @@ sequencer.c: void replay_opts_release(struct replay_opts *opts)\n > free(opts->ctx);\n > }\n > @@ sequencer.c: static int append_squash_message(struct strbuf *buf, const char *body,\n > + if (is_fixup_flag(command, flag) && !seen_squash(ctx)) {\n > + /*\n > + * We're replacing the commit message so we need to\n > +- * append the Signed-off-by: trailer if the user\n > +- * requested '--signoff'.\n > ++ * append any trailers if the user requested\n > ++ * '--signoff' or '--trailer'.\n > + */\n > if (opts->signoff)\n > append_signoff(buf, 0, 0);\n > \n > @@ sequencer.c: static int do_pick_commit(struct repository *r,\n > if (is_rebase_i(opts) && write_author_script(msg.message) < 0)\n > res = -1;\n > else if (!opts->strategy ||\n > +@@ sequencer.c: static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)\n > + parse_strategy_opts(opts, buf->buf);\n > + }\n > +\n > ++static int read_trailers(struct replay_opts *opts, struct strbuf *buf)\n > ++{\n > ++ ssize_t len;\n > ++\n > ++ strbuf_reset(buf);\n > ++ len = strbuf_read_file(buf, rebase_path_trailer(), 0);\n > ++ if (len > 0) {\n > ++ char *p = buf->buf, *nl;\n > ++\n > ++ trailer_config_init();\n > ++\n > ++ while ((nl = strchr(p, '\\n'))) {\n > ++ *nl = '\\0';\n > ++ if (!*p)\n > ++ return error(_(\"trailers file contains empty line\"));\n > ++ strvec_push(&opts->trailer_args, p);\n > ++ p = nl + 1;\n > ++ }\n > ++ } else if (!len) {\n > ++ return error(_(\"trailers file is empty\"));\n > ++ } else if (errno != ENOENT) {\n > ++ return error(_(\"cannot read trailers files\"));\n > ++ }\n > ++\n > ++ return 0;\n > ++}\n > ++\n > + static int read_populate_opts(struct replay_opts *opts)\n > + {\n > + struct replay_ctx *ctx = opts->ctx;\n > @@ sequencer.c: static int read_populate_opts(struct replay_opts *opts)\n > + opts->keep_redundant_commits = 1;\n > \n > read_strategy_opts(opts, &buf);\n > ++\n > ++ if (read_trailers(opts, &buf)) {\n > ++ ret = -1;\n > ++ goto done_rebase_i;\n > ++ }\n > strbuf_reset(&buf);\n > -+ if (strbuf_read_file(&buf, rebase_path_trailer(), 0) >= 0) {\n > -+ char *p = buf.buf, *nl;\n > -+\n > -+ while ((nl = strchr(p, '\\n'))) {\n > -+ *nl = '\\0';\n > -+ if (!*p)\n > -+ BUG(\"rebase-merge/trailer has an empty line\");\n > -+ strvec_push(&opts->trailer_args, p);\n > -+ p = nl + 1;\n > -+ }\n > -+ strbuf_reset(&buf);\n > -+ }\n > \n > if (read_oneliner(&ctx->current_fixups,\n > - rebase_path_current_fixups(),\n > @@ sequencer.c: int write_basic_state(struct replay_opts *opts, const char *head_name,\n > write_file(rebase_path_reschedule_failed_exec(), \"%s\", \"\");\n > else\n > \n > \n\n"},{"id":"538079","messageId":"cover.1772808594.git.phillip.wood@dunelm.org.uk","threadId":"65068","inReplyTo":"20260224070552.148591-1-me@linux.beauty","subject":"[PATCH v8 0/6] rebase: support --trailer","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-06T14:53:26Z","receivedAt":"2026-03-06T14:54:17Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nThis series adds support for creating trailers when rebasing, along\nthe lines of \"git commit --trailer\". Patches 1-3 refactor and libify\nthe code to add trailers to a buffer, patches 4,5 update \"git commit\n--trailer\" and \"git tag --trailer\" to use the new code and the last\npatch adds support for \"git rebase --trailer\".\n\nThanks to Li for working on this series, the main change in this\nhopefully final iteration is that more code is shared between \"git\ninterpret-trailers\" and the builtin commands that add trailers via\na --trailer option, see the range-diff for patches 2-4.\n\nChanges since V7:\n\n - Patch 1: Added missing sign-off.\n - Patch 2: New, refactor create_in_place_tempfile() in preparation for\n moving it.\n - Patch 3: Move create_in_place_tempfile() in addition to\n process_trailers().\n - Patch 4: Use the libified create_in_place_tempfile(), initialize\n trailer config earlier and validate --trailer args for\n \"git commit\" and \"git tag\". Library code now calls error()\n rather than die()\n - Patch 5: Unchanged, apart from the effect of previous patches\n - Patch 6: Small doc fixes, initialize trailer_config() when continuing\n a rebase that was started with --trailer. Report errors when\n reading trailer state file.\n\nLi Chen (5):\n interpret-trailers: factor trailer rewriting\n trailer: libify a couple of functions\n trailer: append trailers without fork/exec\n commit, tag: parse --trailer with OPT_STRVEC\n rebase: support --trailer\n\nPhillip Wood (1):\n interpret-trailers: refactor create_in_place_tempfile()\n\n Documentation/git-rebase.adoc | 8 ++\n builtin/commit.c | 6 +-\n builtin/interpret-trailers.c | 91 ++++-------------\n builtin/rebase.c | 19 ++++\n builtin/tag.c | 7 +-\n sequencer.c | 52 +++++++++-\n sequencer.h | 3 +\n t/meson.build | 1 +\n t/t3440-rebase-trailer.sh | 147 +++++++++++++++++++++++++++\n trailer.c | 184 ++++++++++++++++++++++++++++++++--\n trailer.h | 36 ++++++-\n 11 files changed, 463 insertions(+), 91 deletions(-)\n create mode 100755 t/t3440-rebase-trailer.sh\n\nRange-diff:\n1: b2685e34c22 ! 1: 0d08b361995 interpret-trailers: factor trailer rewriting\n @@ Commit message\n This separation makes it easier to move the helper into trailer.c in the\n next commit.\n \n + Signed-off-by: Phillip Wood \n Signed-off-by: Li Chen \n \n ## builtin/interpret-trailers.c ##\n @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)\n-: ----------- > 2: 5a4d03ab375 interpret-trailers: refactor create_in_place_tempfile()\n2: 1bac3025045 ! 3: ab7e232a95d trailer: move process_trailers to trailer.h\n @@ Metadata\n Author: Li Chen \n \n ## Commit message ##\n - trailer: move process_trailers to trailer.h\n -\n - Move process_trailers() from builtin/interpret-trailers.c into trailer.c\n - and expose it via trailer.h.\n -\n - This lets other call sites reuse the same trailer rewriting logic.\n + trailer: libify a couple of functions\n +\n + Move create_in_place_tempfile() and process_trailers() from\n + builtin/interpret-trailers.c into trailer.c and expose it via trailer.h.\n +\n + This reverts most of ae0ec2e0e0b (trailer: move interpret_trailers()\n + to interpret-trailers.c, 2024-03-01) and lets other call sites reuse\n + the same trailer rewriting logic.\n \n Signed-off-by: Li Chen \n + Signed-off-by: Phillip Wood \n \n ## builtin/interpret-trailers.c ##\n +@@ builtin/interpret-trailers.c: static int parse_opt_parse(const struct option *opt, const char *arg,\n + \treturn 0;\n + }\n + \n +-\n +-static struct tempfile *create_in_place_tempfile(const char *file)\n +-{\n +-\tstruct tempfile *tempfile = NULL;\n +-\tstruct stat st;\n +-\tstruct strbuf filename_template = STRBUF_INIT;\n +-\tconst char *tail;\n +-\n +-\tif (stat(file, &st)) {\n +-\t\terror_errno(_(\"could not stat %s\"), file);\n +-\t\treturn NULL;\n +-\t}\n +-\tif (!S_ISREG(st.st_mode)) {\n +-\t\terror(_(\"file %s is not a regular file\"), file);\n +-\t\treturn NULL;\n +-\t}\n +-\tif (!(st.st_mode & S_IWUSR)) {\n +-\t\terror(_(\"file %s is not writable by user\"), file);\n +-\t\treturn NULL;\n +-\t}\n +-\t/* Create temporary file in the same directory as the original */\n +-\ttail = find_last_dir_sep(file);\n +-\tif (tail)\n +-\t\tstrbuf_add(&filename_template, file, tail - file + 1);\n +-\tstrbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n +-\n +-\ttempfile = mks_tempfile_m(filename_template.buf, st.st_mode);\n +-\n +-\tstrbuf_release(&filename_template);\n +-\n +-\treturn tempfile;\n +-}\n +-\n + static void read_input_file(struct strbuf *sb, const char *file)\n + {\n + \tif (file) {\n @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)\n \tstrbuf_complete_line(sb);\n }\n @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, con\n static void interpret_trailers(const struct process_trailer_options *opts,\n \t\t\t struct list_head *new_trailer_head,\n \t\t\t const char *file)\n +@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,\n + \tread_input_file(&input, file);\n + \n + \tif (opts->in_place) {\n +-\t\ttempfile = create_in_place_tempfile(file);\n ++\t\ttempfile = trailer_create_in_place_tempfile(file);\n + \t\tif (!tempfile)\n + \t\t\tdie(NULL);\n + \t\tfd = tempfile->fd;\n \n ## trailer.c ##\n +@@\n + #include \"commit.h\"\n + #include \"trailer.h\"\n + #include \"list.h\"\n ++#include \"tempfile.h\"\n ++\n + /*\n + * Copyright (c) 2013, 2014 Christian Couder \n + */\n +@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n + \tstrbuf_release(&iter->key);\n + }\n + \n ++struct tempfile *trailer_create_in_place_tempfile(const char *file)\n ++{\n ++\tstruct tempfile *tempfile = NULL;\n ++\tstruct stat st;\n ++\tstruct strbuf filename_template = STRBUF_INIT;\n ++\tconst char *tail;\n ++\n ++\tif (stat(file, &st)) {\n ++\t\terror_errno(_(\"could not stat %s\"), file);\n ++\t\treturn NULL;\n ++\t}\n ++\tif (!S_ISREG(st.st_mode)) {\n ++\t\terror(_(\"file %s is not a regular file\"), file);\n ++\t\treturn NULL;\n ++\t}\n ++\tif (!(st.st_mode & S_IWUSR)) {\n ++\t\terror(_(\"file %s is not writable by user\"), file);\n ++\t\treturn NULL;\n ++\t}\n ++\t/* Create temporary file in the same directory as the original */\n ++\ttail = find_last_dir_sep(file);\n ++\tif (tail)\n ++\t\tstrbuf_add(&filename_template, file, tail - file + 1);\n ++\tstrbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n ++\n ++\ttempfile = mks_tempfile_m(filename_template.buf, st.st_mode);\n ++\n ++\tstrbuf_release(&filename_template);\n ++\n ++\treturn tempfile;\n ++}\n ++\n + int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)\n + {\n + \tstruct child_process run_trailer = CHILD_PROCESS_INIT;\n @@ trailer.c: int amend_file_with_trailers(const char *path, const struct strvec *trailer_args\n \tstrvec_pushv(&run_trailer.args, trailer_args->v);\n \treturn run_command(&run_trailer);\n @@ trailer.h: 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 ++ * Create a tempfile \"\"git-interpret-trailers-XXXXXX\" in the same\n ++ * directory as file.\n ++ */\n ++struct tempfile *trailer_create_in_place_tempfile(const char *file);\n ++\n ++/*\n ++ * Rewrite the contents of input by processing its trailer block according to\n ++ * opts and (optionally) appending trailers from new_trailer_head.\n ++ *\n ++ * The rewritten message is appended to out (callers should strbuf_reset()\n ++ * first if needed).\n ++ */\n +void process_trailers(const struct process_trailer_options *opts,\n +\t\t struct list_head *new_trailer_head,\n +\t\t struct strbuf *input, struct strbuf *out);\n3: 3114f0dbb57 ! 4: 1f24917eb64 trailer: append trailers without fork/exec\n @@ Commit message\n \n Update amend_file_with_trailers() to use the in-process helper and\n rewrite the target file via tempfile+rename, preserving the previous\n - in-place semantics.\n + in-place semantics. As the trailers are no longer added in a separate\n + process and trailer_config_init() die()s on missing config values it\n + is called early on in cmd_commit() and cmd_tag() so that they die()\n + early before writing the message file. The trailer arguments are now\n + also sanity checked.\n \n Keep existing callers unchanged by continuing to accept argv-style\n --trailer= entries and stripping the prefix before feeding the\n in-process implementation.\n \n Signed-off-by: Li Chen \n -\n - ## builtin/interpret-trailers.c ##\n -@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,\n - \tstruct strbuf out = STRBUF_INIT;\n - \tFILE *outfile = stdout;\n - \n --\ttrailer_config_init();\n --\n - \tread_input_file(&input, file);\n - \n - \tif (opts->in_place)\n -@@ builtin/interpret-trailers.c: int cmd_interpret_trailers(int argc,\n - \t\t\tgit_interpret_trailers_usage,\n - \t\t\toptions);\n - \n -+\ttrailer_config_init();\n -+\n - \tif (argc) {\n - \t\tint i;\n - \t\tfor (i = 0; i < argc; i++)\n + Signed-off-by: Phillip Wood \n +\n + ## builtin/commit.c ##\n +@@ builtin/commit.c: int cmd_commit(int argc,\n + \targc = parse_and_validate_options(argc, argv, builtin_commit_options,\n + \t\t\t\t\t builtin_commit_usage,\n + \t\t\t\t\t prefix, current_head, &s);\n ++\tif (trailer_args.nr)\n ++\t\ttrailer_config_init();\n ++\n + \tif (verbose == -1)\n + \t\tverbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n + \n +\n + ## builtin/tag.c ##\n +@@ builtin/tag.c: int cmd_tag(int argc,\n + \tif (cmdmode == 'l')\n + \t\tsetup_auto_pager(\"tag\", 1);\n + \n ++\tif (trailer_args.nr)\n ++\t\ttrailer_config_init();\n ++\n + \tif (opt.sign == -1)\n + \t\topt.sign = cmdmode ? 0 : config_sign_tag > 0;\n + \n \n ## trailer.c ##\n @@\n #include \"string-list.h\"\n #include \"run-command.h\"\n #include \"commit.h\"\n +#include \"strvec.h\"\n -+#include \"tempfile.h\"\n #include \"trailer.h\"\n #include \"list.h\"\n -+\n - /*\n - * Copyright (c) 2013, 2014 Christian Couder \n - */\n + #include \"tempfile.h\"\n @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head,\n \tfree(cl_separators);\n }\n \n -+void validate_trailer_args(const struct strvec *cli_args)\n ++int validate_trailer_args(const struct strvec *cli_args)\n +{\n +\tchar *cl_separators;\n ++\tint ret = 0;\n +\n +\ttrailer_config_init();\n +\n @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head\n +\t\tconst char *txt = cli_args->v[i];\n +\t\tssize_t separator_pos;\n +\n -+\t\tif (!*txt)\n -+\t\t\tdie(_(\"empty --trailer argument\"));\n -+\n ++\t\tif (!*txt) {\n ++\t\t\tret = error(_(\"empty --trailer argument\"));\n ++\t\t\tgoto out;\n ++\t\t}\n +\t\tseparator_pos = find_separator(txt, cl_separators);\n -+\t\tif (separator_pos == 0)\n -+\t\t\tdie(_(\"invalid trailer '%s': missing key before separator\"),\n -+\t\t\t txt);\n ++\t\tif (separator_pos == 0) {\n ++\t\t\tret = error(_(\"invalid trailer '%s': missing key before separator\"),\n ++\t\t\t\t txt);\n ++\t\t\tgoto out;\n ++\t\t}\n +\t}\n -+\n ++out:\n +\tfree(cl_separators);\n ++\treturn ret;\n +}\n +\n static const char *next_line(const char *str)\n {\n \tconst char *nl = strchrnul(str, '\\n');\n -@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n - \tstrbuf_release(&iter->key);\n +@@ trailer.c: struct tempfile *trailer_create_in_place_tempfile(const char *file)\n + \treturn tempfile;\n }\n \n -int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)\n @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n -\t\t path, NULL);\n -\tstrvec_pushv(&run_trailer.args, trailer_args->v);\n -\treturn run_command(&run_trailer);\n -+static void new_trailer_items_clear(struct list_head *items)\n -+{\n -+\twhile (!list_empty(items)) {\n -+\t\tstruct new_trailer_item *item =\n -+\t\t\tlist_first_entry(items, struct new_trailer_item, list);\n -+\t\tlist_del(&item->list);\n -+\t\tfree(item);\n -+\t}\n -+}\n -+\n -+void amend_strbuf_with_trailers(struct strbuf *buf,\n ++int amend_strbuf_with_trailers(struct strbuf *buf,\n +\t\t\t\tconst struct strvec *trailer_args)\n +{\n +\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n +\tLIST_HEAD(new_trailer_head);\n +\tstruct strbuf out = STRBUF_INIT;\n +\tsize_t i;\n ++\tint ret = 0;\n +\n +\topts.no_divider = 1;\n +\n +\tfor (i = 0; i < trailer_args->nr; i++) {\n +\t\tconst char *text = trailer_args->v[i];\n +\t\tstruct new_trailer_item *item;\n +\n -+\t\tif (!*text)\n -+\t\t\tdie(_(\"empty --trailer argument\"));\n ++\t\tif (!*text) {\n ++\t\t\tret = error(_(\"empty --trailer argument\"));\n ++\t\t\tgoto out;\n ++\t\t}\n +\t\titem = xcalloc(1, sizeof(*item));\n -+\t\titem->text = text;\n ++\t\titem->text = xstrdup(text);\n +\t\tlist_add_tail(&item->list, &new_trailer_head);\n +\t}\n +\n -+\ttrailer_config_init();\n +\tprocess_trailers(&opts, &new_trailer_head, buf, &out);\n +\n +\tstrbuf_swap(buf, &out);\n ++out:\n +\tstrbuf_release(&out);\n ++\tfree_trailers(&new_trailer_head);\n +\n -+\tnew_trailer_items_clear(&new_trailer_head);\n ++\treturn ret;\n +}\n +\n +static int write_file_in_place(const char *path, const struct strbuf *buf)\n +{\n -+\tstruct stat st;\n -+\tstruct strbuf filename_template = STRBUF_INIT;\n -+\tconst char *tail;\n -+\tstruct tempfile *tempfile;\n -+\tFILE *outfile;\n -+\n -+\tif (stat(path, &st))\n -+\t\treturn error_errno(_(\"could not stat %s\"), path);\n -+\tif (!S_ISREG(st.st_mode))\n -+\t\treturn error(_(\"file %s is not a regular file\"), path);\n -+\tif (!(st.st_mode & S_IWUSR))\n -+\t\treturn error(_(\"file %s is not writable by user\"), path);\n -+\n -+\t/* Create temporary file in the same directory as the original */\n -+\ttail = strrchr(path, '/');\n -+\tif (tail)\n -+\t\tstrbuf_add(&filename_template, path, tail - path + 1);\n -+\tstrbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n -+\n -+\ttempfile = mks_tempfile_sm(filename_template.buf, 0, st.st_mode);\n -+\tstrbuf_release(&filename_template);\n ++\tstruct tempfile *tempfile = trailer_create_in_place_tempfile(path);\n +\tif (!tempfile)\n -+\t\treturn error_errno(_(\"could not create temporary file\"));\n -+\n -+\toutfile = fdopen_tempfile(tempfile, \"w\");\n -+\tif (!outfile) {\n -+\t\tint saved_errno = errno;\n -+\t\tdelete_tempfile(&tempfile);\n -+\t\terrno = saved_errno;\n -+\t\treturn error_errno(_(\"could not open temporary file\"));\n -+\t}\n -+\n -+\tif (buf->len && fwrite(buf->buf, 1, buf->len, outfile) < buf->len) {\n -+\t\tint saved_errno = errno;\n -+\t\tdelete_tempfile(&tempfile);\n -+\t\terrno = saved_errno;\n ++\t\treturn -1;\n ++\n ++\tif (write_in_full(tempfile->fd, buf->buf, buf->len) < 0)\n +\t\treturn error_errno(_(\"could not write to temporary file\"));\n -+\t}\n +\n +\tif (rename_tempfile(&tempfile, path))\n +\t\treturn error_errno(_(\"could not rename temporary file to %s\"), path);\n @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n +\t\t * in-process implementation.\n +\t\t */\n +\t\tskip_prefix(txt, \"--trailer=\", &txt);\n -+\t\tif (!*txt)\n -+\t\t\tdie(_(\"empty --trailer argument\"));\n ++\t\tif (!*txt) {\n ++\t\t\tret = error(_(\"empty --trailer argument\"));\n ++\t\t\tgoto out;\n ++\t\t}\n +\t\tstrvec_push(&stripped_trailer_args, txt);\n +\t}\n +\n ++\tif (validate_trailer_args(&stripped_trailer_args)) {\n ++\t\tret = -1;\n ++\t\tgoto out;\n ++\t}\n +\tif (strbuf_read_file(&buf, path, 0) < 0)\n +\t\tret = error_errno(_(\"could not read '%s'\"), path);\n +\telse\n +\t\tamend_strbuf_with_trailers(&buf, &stripped_trailer_args);\n +\n +\tif (!ret)\n +\t\tret = write_file_in_place(path, &buf);\n -+\n ++out:\n +\tstrvec_clear(&stripped_trailer_args);\n +\tstrbuf_release(&buf);\n +\treturn ret;\n @@ trailer.h: void parse_trailers_from_config(struct list_head *config_head);\n void parse_trailers_from_command_line_args(struct list_head *arg_head,\n \t\t\t\t\t struct list_head *new_trailer_head);\n \n -+void validate_trailer_args(const struct strvec *cli_args);\n ++int validate_trailer_args(const struct strvec *cli_args);\n +\n void process_trailers_lists(struct list_head *head,\n \t\t\t struct list_head *arg_head);\n @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);\n + * Each element of trailer_args should be in the same format as the value\n + * accepted by --trailer= (i.e., without the --trailer= prefix).\n + */\n -+void amend_strbuf_with_trailers(struct strbuf *buf,\n ++int amend_strbuf_with_trailers(struct strbuf *buf,\n +\t\t\t\tconst struct strvec *trailer_args);\n +\n +/*\n @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);\n */\n int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);\n \n -+/*\n -+ * Rewrite the contents of input by processing its trailer block according to\n -+ * opts and (optionally) appending trailers from new_trailer_head.\n -+ *\n -+ * The rewritten message is appended to out (callers should strbuf_reset()\n -+ * first if needed).\n -+ */\n - void process_trailers(const struct process_trailer_options *opts,\n - \t\t struct list_head *new_trailer_head,\n - \t\t struct strbuf *input, struct strbuf *out);\n4: 147595a9317 ! 5: 3c1fa9e8579 commit, tag: parse --trailer with OPT_STRVEC\n @@ Commit message\n amend_file_with_trailers().\n \n Signed-off-by: Li Chen \n \n ## builtin/commit.c ##\n @@ builtin/commit.c: int cmd_commit(int argc,\n @@ trailer.c: int amend_file_with_trailers(const char *path,\n -\t\t * in-process implementation.\n -\t\t */\n -\t\tskip_prefix(txt, \"--trailer=\", &txt);\n --\t\tif (!*txt)\n --\t\t\tdie(_(\"empty --trailer argument\"));\n +-\t\tif (!*txt) {\n +-\t\t\tret = error(_(\"empty --trailer argument\"));\n +-\t\t\tgoto out;\n +-\t\t}\n -\t\tstrvec_push(&stripped_trailer_args, txt);\n -\t}\n -\n +-\tif (validate_trailer_args(&stripped_trailer_args)) {\n ++\tif (validate_trailer_args(trailer_args)) {\n + \t\tret = -1;\n + \t\tgoto out;\n + \t}\n \tif (strbuf_read_file(&buf, path, 0) < 0)\n \t\tret = error_errno(_(\"could not read '%s'\"), path);\n \telse\n @@ trailer.c: int amend_file_with_trailers(const char *path,\n \n \tif (!ret)\n \t\tret = write_file_in_place(path, &buf);\n - \n + out:\n -\tstrvec_clear(&stripped_trailer_args);\n \tstrbuf_release(&buf);\n \treturn ret;\n }\n \n ## trailer.h ##\n -@@ trailer.h: void amend_strbuf_with_trailers(struct strbuf *buf,\n +@@ trailer.h: int amend_strbuf_with_trailers(struct strbuf *buf,\n /*\n * Augment a file by appending trailers specified in trailer_args.\n *\n5: 864cf5f8eb6 ! 6: 99654d80547 rebase: support --trailer\n @@ Commit message\n non-interactive and interactive rebases.\n \n Signed-off-by: Li Chen \n + Signed-off-by: Phillip Wood \n \n ## Documentation/git-rebase.adoc ##\n @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.\n @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.\n +--trailer=::\n +\tAppend the given trailer to every rebased commit message, processed\n +\tvia linkgit:git-interpret-trailers[1]. This option implies\n -+\t`--force-rebase` so that fast-forwarded commits are also rewritten.\n ++\t`--force-rebase`.\n ++\n +See also INCOMPATIBLE OPTIONS below.\n +\n -i::\n --interactive::\n \tMake a list of the commits which are about to be rebased. Let the\n +@@ Documentation/git-rebase.adoc: are incompatible with the following options:\n + * --[no-]reapply-cherry-picks when used without --keep-base\n + * --update-refs\n + * --root when used without --onto\n ++ * --trailer\n + \n + In addition, the following pairs of options are incompatible:\n + \n \n ## builtin/rebase.c ##\n @@\n @@ builtin/rebase.c: int cmd_rebase(int argc,\n \t\t\t builtin_rebase_usage, 0);\n \n +\tif (options.trailer_args.nr) {\n -+\t\tvalidate_trailer_args(&options.trailer_args);\n ++\t\tif (validate_trailer_args(&options.trailer_args))\n ++\t\t\tdie(NULL);\n +\t\toptions.flags |= REBASE_FORCE;\n +\t}\n +\n @@ sequencer.c: void replay_opts_release(struct replay_opts *opts)\n \tfree(opts->ctx);\n }\n @@ sequencer.c: static int append_squash_message(struct strbuf *buf, const char *body,\n + \tif (is_fixup_flag(command, flag) && !seen_squash(ctx)) {\n + \t\t/*\n + \t\t * We're replacing the commit message so we need to\n +-\t\t * append the Signed-off-by: trailer if the user\n +-\t\t * requested '--signoff'.\n ++\t\t * append any trailers if the user requested\n ++\t\t * '--signoff' or '--trailer'.\n + \t\t */\n \t\tif (opts->signoff)\n \t\t\tappend_signoff(buf, 0, 0);\n \n @@ sequencer.c: static int do_pick_commit(struct repository *r,\n \tif (is_rebase_i(opts) && write_author_script(msg.message) < 0)\n \t\tres = -1;\n \telse if (!opts->strategy ||\n +@@ sequencer.c: static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)\n + \tparse_strategy_opts(opts, buf->buf);\n + }\n + \n ++static int read_trailers(struct replay_opts *opts, struct strbuf *buf)\n ++{\n ++\tssize_t len;\n ++\n ++\tstrbuf_reset(buf);\n ++\tlen = strbuf_read_file(buf, rebase_path_trailer(), 0);\n ++\tif (len > 0) {\n ++\t\tchar *p = buf->buf, *nl;\n ++\n ++\t\ttrailer_config_init();\n ++\n ++\t\twhile ((nl = strchr(p, '\\n'))) {\n ++\t\t\t*nl = '\\0';\n ++\t\t\tif (!*p)\n ++\t\t\t\treturn error(_(\"trailers file contains empty line\"));\n ++\t\t\tstrvec_push(&opts->trailer_args, p);\n ++\t\t\tp = nl + 1;\n ++\t\t}\n ++\t} else if (!len) {\n ++\t\treturn error(_(\"trailers file is empty\"));\n ++\t} else if (errno != ENOENT) {\n ++\t\treturn error(_(\"cannot read trailers files\"));\n ++\t}\n ++\n ++\treturn 0;\n ++}\n ++\n + static int read_populate_opts(struct replay_opts *opts)\n + {\n + \tstruct replay_ctx *ctx = opts->ctx;\n @@ sequencer.c: static int read_populate_opts(struct replay_opts *opts)\n + \t\t\topts->keep_redundant_commits = 1;\n \n \t\tread_strategy_opts(opts, &buf);\n ++\n ++\t\tif (read_trailers(opts, &buf)) {\n ++\t\t\tret = -1;\n ++\t\t\tgoto done_rebase_i;\n ++\t\t}\n \t\tstrbuf_reset(&buf);\n -+\t\tif (strbuf_read_file(&buf, rebase_path_trailer(), 0) >= 0) {\n -+\t\t\tchar *p = buf.buf, *nl;\n -+\n -+\t\t\twhile ((nl = strchr(p, '\\n'))) {\n -+\t\t\t\t*nl = '\\0';\n -+\t\t\t\tif (!*p)\n -+\t\t\t\t\tBUG(\"rebase-merge/trailer has an empty line\");\n -+\t\t\t\tstrvec_push(&opts->trailer_args, p);\n -+\t\t\t\tp = nl + 1;\n -+\t\t\t}\n -+\t\t\tstrbuf_reset(&buf);\n -+\t\t}\n \n \t\tif (read_oneliner(&ctx->current_fixups,\n - \t\t\t\t rebase_path_current_fixups(),\n @@ sequencer.c: int write_basic_state(struct replay_opts *opts, const char *head_name,\n \t\twrite_file(rebase_path_reschedule_failed_exec(), \"%s\", \"\");\n \telse\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"538078","messageId":"0d08b361995f4d117b3c80a7e403c1a821d10a5f.1772808594.git.phillip.wood@dunelm.org.uk","threadId":"65068","inReplyTo":"cover.1772808594.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v8 1/6] interpret-trailers: factor trailer rewriting","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-06T14:53:27Z","receivedAt":"2026-03-06T14:54:18Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Li Chen \n\nExtract the trailer rewriting logic into a helper that appends to an\noutput strbuf.\n\nUpdate interpret_trailers() to handle file I/O only: read input once,\ncall the helper, and write the buffered result.\n\nThis separation makes it easier to move the helper into trailer.c in the\nnext commit.\n\nSigned-off-by: Phillip Wood \nSigned-off-by: Li Chen \n---\n builtin/interpret-trailers.c | 57 ++++++++++++++++++++----------------\n 1 file changed, 32 insertions(+), 25 deletions(-)\n\ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex 41b0750e5af..69f9d67ec0e 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -136,32 +136,21 @@ static void read_input_file(struct strbuf *sb, const char *file)\n \tstrbuf_complete_line(sb);\n }\n \n-static void interpret_trailers(const struct process_trailer_options *opts,\n-\t\t\t struct list_head *new_trailer_head,\n-\t\t\t const char *file)\n+static void process_trailers(const struct process_trailer_options *opts,\n+\t\t\t struct list_head *new_trailer_head,\n+\t\t\t struct strbuf *input, struct strbuf *out)\n {\n \tLIST_HEAD(head);\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tstruct strbuf trailer_block_sb = STRBUF_INIT;\n \tstruct trailer_block *trailer_block;\n-\tFILE *outfile = stdout;\n-\n-\ttrailer_config_init();\n-\n-\tread_input_file(&sb, file);\n-\n-\tif (opts->in_place)\n-\t\toutfile = create_in_place_tempfile(file);\n-\n-\ttrailer_block = parse_trailers(opts, sb.buf, &head);\n+\n+\ttrailer_block = parse_trailers(opts, input->buf, &head);\n \n \t/* Print the lines before the trailer block */\n \tif (!opts->only_trailers)\n-\t\tfwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);\n+\t\tstrbuf_add(out, input->buf, trailer_block_start(trailer_block));\n \n \tif (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))\n-\t\tfprintf(outfile, \"\\n\");\n-\n+\t\tstrbuf_addch(out, '\\n');\n \n \tif (!opts->only_input) {\n \t\tLIST_HEAD(config_head);\n@@ -173,22 +162,40 @@ static void interpret_trailers(const struct process_trailer_options *opts,\n \t}\n \n \t/* Print trailer block. */\n-\tformat_trailers(opts, &head, &trailer_block_sb);\n+\tformat_trailers(opts, &head, out);\n \tfree_trailers(&head);\n-\tfwrite(trailer_block_sb.buf, 1, trailer_block_sb.len, outfile);\n-\tstrbuf_release(&trailer_block_sb);\n \n \t/* Print the lines after the trailer block as is. */\n \tif (!opts->only_trailers)\n-\t\tfwrite(sb.buf + trailer_block_end(trailer_block), 1,\n-\t\t sb.len - trailer_block_end(trailer_block), outfile);\n+\t\tstrbuf_add(out, input->buf + trailer_block_end(trailer_block),\n+\t\t\t input->len - trailer_block_end(trailer_block));\n \ttrailer_block_release(trailer_block);\n-\n+}\n+\n+static void interpret_trailers(const struct process_trailer_options *opts,\n+\t\t\t struct list_head *new_trailer_head,\n+\t\t\t const char *file)\n+{\n+\tstruct strbuf input = STRBUF_INIT;\n+\tstruct strbuf out = STRBUF_INIT;\n+\tFILE *outfile = stdout;\n+\n+\ttrailer_config_init();\n+\n+\tread_input_file(&input, file);\n+\n+\tif (opts->in_place)\n+\t\toutfile = create_in_place_tempfile(file);\n+\n+\tprocess_trailers(opts, new_trailer_head, &input, &out);\n+\n+\tstrbuf_write(&out, outfile);\n \tif (opts->in_place)\n \t\tif (rename_tempfile(&trailers_tempfile, file))\n \t\t\tdie_errno(_(\"could not rename temporary file to %s\"), file);\n \n-\tstrbuf_release(&sb);\n+\tstrbuf_release(&input);\n+\tstrbuf_release(&out);\n }\n \n int cmd_interpret_trailers(int argc,\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"538080","messageId":"5a4d03ab375bbba84436796ca6871204f47521eb.1772808594.git.phillip.wood@dunelm.org.uk","threadId":"65068","inReplyTo":"cover.1772808594.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v8 2/6] interpret-trailers: refactor create_in_place_tempfile()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-06T14:53:28Z","receivedAt":"2026-03-06T14:54:19Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nRefactor create_in_place_tempfile() in preparation for moving it\nto tralier.c. Change the return type to return a `struct tempfile*`\ninstead of a `FILE*` so that we can remove the file scope tempfile\nvariable. Since 076aa2cbda5 (tempfile: auto-allocate tempfiles on\nheap, 2017-09-05) it has not been necessary to make tempfile varibales\nstatic so this is safe. Also use error() and return NULL in place of\ndie() so the caller can exit gracefully and use find_last_dir_sep()\nrather than strchr() to find the parent directory.\n\nSigned-off-by: Phillip Wood \n---\n builtin/interpret-trailers.c | 51 ++++++++++++++++++++----------------\n 1 file changed, 29 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex 69f9d67ec0e..033c2e46713 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -93,35 +93,37 @@ static int parse_opt_parse(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n-static struct tempfile *trailers_tempfile;\n \n-static FILE *create_in_place_tempfile(const char *file)\n+static struct tempfile *create_in_place_tempfile(const char *file)\n {\n+\tstruct tempfile *tempfile = NULL;\n \tstruct stat st;\n \tstruct strbuf filename_template = STRBUF_INIT;\n \tconst char *tail;\n-\tFILE *outfile;\n-\n-\tif (stat(file, &st))\n-\t\tdie_errno(_(\"could not stat %s\"), file);\n-\tif (!S_ISREG(st.st_mode))\n-\t\tdie(_(\"file %s is not a regular file\"), file);\n-\tif (!(st.st_mode & S_IWUSR))\n-\t\tdie(_(\"file %s is not writable by user\"), file);\n \n+\tif (stat(file, &st)) {\n+\t\terror_errno(_(\"could not stat %s\"), file);\n+\t\treturn NULL;\n+\t}\n+\tif (!S_ISREG(st.st_mode)) {\n+\t\terror(_(\"file %s is not a regular file\"), file);\n+\t\treturn NULL;\n+\t}\n+\tif (!(st.st_mode & S_IWUSR)) {\n+\t\terror(_(\"file %s is not writable by user\"), file);\n+\t\treturn NULL;\n+\t}\n \t/* Create temporary file in the same directory as the original */\n-\ttail = strrchr(file, '/');\n+\ttail = find_last_dir_sep(file);\n \tif (tail)\n \t\tstrbuf_add(&filename_template, file, tail - file + 1);\n \tstrbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n \n-\ttrailers_tempfile = xmks_tempfile_m(filename_template.buf, st.st_mode);\n+\ttempfile = mks_tempfile_m(filename_template.buf, st.st_mode);\n+\n \tstrbuf_release(&filename_template);\n-\toutfile = fdopen_tempfile(trailers_tempfile, \"w\");\n-\tif (!outfile)\n-\t\tdie_errno(_(\"could not open temporary file\"));\n \n-\treturn outfile;\n+\treturn tempfile;\n }\n \n static void read_input_file(struct strbuf *sb, const char *file)\n@@ -178,20 +180,25 @@ static void interpret_trailers(const struct process_trailer_options *opts,\n {\n \tstruct strbuf input = STRBUF_INIT;\n \tstruct strbuf out = STRBUF_INIT;\n-\tFILE *outfile = stdout;\n+\tstruct tempfile *tempfile = NULL;\n+\tint fd = 1;\n \n \ttrailer_config_init();\n \n \tread_input_file(&input, file);\n \n-\tif (opts->in_place)\n-\t\toutfile = create_in_place_tempfile(file);\n-\n+\tif (opts->in_place) {\n+\t\ttempfile = create_in_place_tempfile(file);\n+\t\tif (!tempfile)\n+\t\t\tdie(NULL);\n+\t\tfd = tempfile->fd;\n+\t}\n \tprocess_trailers(opts, new_trailer_head, &input, &out);\n \n-\tstrbuf_write(&out, outfile);\n+\tif (write_in_full(fd, out.buf, out.len) < 0)\n+\t\tdie_errno(_(\"could not write to temporary file '%s'\"), file);\n \tif (opts->in_place)\n-\t\tif (rename_tempfile(&trailers_tempfile, file))\n+\t\tif (rename_tempfile(&tempfile, file))\n \t\t\tdie_errno(_(\"could not rename temporary file to %s\"), file);\n \n \tstrbuf_release(&input);\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"538081","messageId":"ab7e232a95d1312dd57b57eea65de56f702daaf5.1772808594.git.phillip.wood@dunelm.org.uk","threadId":"65068","inReplyTo":"cover.1772808594.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v8 3/6] trailer: libify a couple of functions","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-06T14:53:29Z","receivedAt":"2026-03-06T14:54:20Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Li Chen \n\nMove create_in_place_tempfile() and process_trailers() from\nbuiltin/interpret-trailers.c into trailer.c and expose it via trailer.h.\n\nThis reverts most of ae0ec2e0e0b (trailer: move interpret_trailers()\nto interpret-trailers.c, 2024-03-01) and lets other call sites reuse\nthe same trailer rewriting logic.\n\nSigned-off-by: Li Chen \nSigned-off-by: Phillip Wood \n---\n builtin/interpret-trailers.c | 71 +-----------------------------------\n trailer.c | 70 +++++++++++++++++++++++++++++++++++\n trailer.h | 16 ++++++++\n 3 files changed, 87 insertions(+), 70 deletions(-)\n\ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex 033c2e46713..acaf42b2d93 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -93,39 +93,6 @@ static int parse_opt_parse(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n-\n-static struct tempfile *create_in_place_tempfile(const char *file)\n-{\n-\tstruct tempfile *tempfile = NULL;\n-\tstruct stat st;\n-\tstruct strbuf filename_template = STRBUF_INIT;\n-\tconst char *tail;\n-\n-\tif (stat(file, &st)) {\n-\t\terror_errno(_(\"could not stat %s\"), file);\n-\t\treturn NULL;\n-\t}\n-\tif (!S_ISREG(st.st_mode)) {\n-\t\terror(_(\"file %s is not a regular file\"), file);\n-\t\treturn NULL;\n-\t}\n-\tif (!(st.st_mode & S_IWUSR)) {\n-\t\terror(_(\"file %s is not writable by user\"), file);\n-\t\treturn NULL;\n-\t}\n-\t/* Create temporary file in the same directory as the original */\n-\ttail = find_last_dir_sep(file);\n-\tif (tail)\n-\t\tstrbuf_add(&filename_template, file, tail - file + 1);\n-\tstrbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n-\n-\ttempfile = mks_tempfile_m(filename_template.buf, st.st_mode);\n-\n-\tstrbuf_release(&filename_template);\n-\n-\treturn tempfile;\n-}\n-\n static void read_input_file(struct strbuf *sb, const char *file)\n {\n \tif (file) {\n@@ -138,42 +105,6 @@ static void read_input_file(struct strbuf *sb, const char *file)\n \tstrbuf_complete_line(sb);\n }\n \n-static void process_trailers(const struct process_trailer_options *opts,\n-\t\t\t struct list_head *new_trailer_head,\n-\t\t\t struct strbuf *input, struct strbuf *out)\n-{\n-\tLIST_HEAD(head);\n-\tstruct trailer_block *trailer_block;\n-\n-\ttrailer_block = parse_trailers(opts, input->buf, &head);\n-\n-\t/* Print the lines before the trailer block */\n-\tif (!opts->only_trailers)\n-\t\tstrbuf_add(out, input->buf, trailer_block_start(trailer_block));\n-\n-\tif (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))\n-\t\tstrbuf_addch(out, '\\n');\n-\n-\tif (!opts->only_input) {\n-\t\tLIST_HEAD(config_head);\n-\t\tLIST_HEAD(arg_head);\n-\t\tparse_trailers_from_config(&config_head);\n-\t\tparse_trailers_from_command_line_args(&arg_head, new_trailer_head);\n-\t\tlist_splice(&config_head, &arg_head);\n-\t\tprocess_trailers_lists(&head, &arg_head);\n-\t}\n-\n-\t/* Print trailer block. */\n-\tformat_trailers(opts, &head, out);\n-\tfree_trailers(&head);\n-\n-\t/* Print the lines after the trailer block as is. */\n-\tif (!opts->only_trailers)\n-\t\tstrbuf_add(out, input->buf + trailer_block_end(trailer_block),\n-\t\t\t input->len - trailer_block_end(trailer_block));\n-\ttrailer_block_release(trailer_block);\n-}\n-\n static void interpret_trailers(const struct process_trailer_options *opts,\n \t\t\t struct list_head *new_trailer_head,\n \t\t\t const char *file)\n@@ -188,7 +119,7 @@ static void interpret_trailers(const struct process_trailer_options *opts,\n \tread_input_file(&input, file);\n \n \tif (opts->in_place) {\n-\t\ttempfile = create_in_place_tempfile(file);\n+\t\ttempfile = trailer_create_in_place_tempfile(file);\n \t\tif (!tempfile)\n \t\t\tdie(NULL);\n \t\tfd = tempfile->fd;\ndiff --git a/trailer.c b/trailer.c\nindex 911a81ed993..163018483a5 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -9,6 +9,8 @@\n #include \"commit.h\"\n #include \"trailer.h\"\n #include \"list.h\"\n+#include \"tempfile.h\"\n+\n /*\n * Copyright (c) 2013, 2014 Christian Couder \n */\n@@ -1224,6 +1226,38 @@ void trailer_iterator_release(struct trailer_iterator *iter)\n \tstrbuf_release(&iter->key);\n }\n \n+struct tempfile *trailer_create_in_place_tempfile(const char *file)\n+{\n+\tstruct tempfile *tempfile = NULL;\n+\tstruct stat st;\n+\tstruct strbuf filename_template = STRBUF_INIT;\n+\tconst char *tail;\n+\n+\tif (stat(file, &st)) {\n+\t\terror_errno(_(\"could not stat %s\"), file);\n+\t\treturn NULL;\n+\t}\n+\tif (!S_ISREG(st.st_mode)) {\n+\t\terror(_(\"file %s is not a regular file\"), file);\n+\t\treturn NULL;\n+\t}\n+\tif (!(st.st_mode & S_IWUSR)) {\n+\t\terror(_(\"file %s is not writable by user\"), file);\n+\t\treturn NULL;\n+\t}\n+\t/* Create temporary file in the same directory as the original */\n+\ttail = find_last_dir_sep(file);\n+\tif (tail)\n+\t\tstrbuf_add(&filename_template, file, tail - file + 1);\n+\tstrbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n+\n+\ttempfile = mks_tempfile_m(filename_template.buf, st.st_mode);\n+\n+\tstrbuf_release(&filename_template);\n+\n+\treturn tempfile;\n+}\n+\n int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)\n {\n \tstruct child_process run_trailer = CHILD_PROCESS_INIT;\n@@ -1235,3 +1269,39 @@ int amend_file_with_trailers(const char *path, const struct strvec *trailer_args\n \tstrvec_pushv(&run_trailer.args, trailer_args->v);\n \treturn run_command(&run_trailer);\n }\n+\n+void process_trailers(const struct process_trailer_options *opts,\n+\t\t struct list_head *new_trailer_head,\n+\t\t struct strbuf *input, struct strbuf *out)\n+{\n+\tLIST_HEAD(head);\n+\tstruct trailer_block *trailer_block;\n+\n+\ttrailer_block = parse_trailers(opts, input->buf, &head);\n+\n+\t/* Print the lines before the trailer block */\n+\tif (!opts->only_trailers)\n+\t\tstrbuf_add(out, input->buf, trailer_block_start(trailer_block));\n+\n+\tif (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))\n+\t\tstrbuf_addch(out, '\\n');\n+\n+\tif (!opts->only_input) {\n+\t\tLIST_HEAD(config_head);\n+\t\tLIST_HEAD(arg_head);\n+\t\tparse_trailers_from_config(&config_head);\n+\t\tparse_trailers_from_command_line_args(&arg_head, new_trailer_head);\n+\t\tlist_splice(&config_head, &arg_head);\n+\t\tprocess_trailers_lists(&head, &arg_head);\n+\t}\n+\n+\t/* Print trailer block. */\n+\tformat_trailers(opts, &head, out);\n+\tfree_trailers(&head);\n+\n+\t/* Print the lines after the trailer block as is. */\n+\tif (!opts->only_trailers)\n+\t\tstrbuf_add(out, input->buf + trailer_block_end(trailer_block),\n+\t\t\t input->len - trailer_block_end(trailer_block));\n+\ttrailer_block_release(trailer_block);\n+}\ndiff --git a/trailer.h b/trailer.h\nindex 4740549586a..7fd2564e035 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -202,4 +202,20 @@ 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+ * Create a tempfile \"\"git-interpret-trailers-XXXXXX\" in the same\n+ * directory as file.\n+ */\n+struct tempfile *trailer_create_in_place_tempfile(const char *file);\n+\n+/*\n+ * Rewrite the contents of input by processing its trailer block according to\n+ * opts and (optionally) appending trailers from new_trailer_head.\n+ *\n+ * The rewritten message is appended to out (callers should strbuf_reset()\n+ * first if needed).\n+ */\n+void process_trailers(const struct process_trailer_options *opts,\n+\t\t struct list_head *new_trailer_head,\n+\t\t struct strbuf *input, struct strbuf *out);\n #endif /* TRAILER_H */\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"538082","messageId":"1f24917eb64091fd0a756ae5c5d26db1ca5d6e9a.1772808594.git.phillip.wood@dunelm.org.uk","threadId":"65068","inReplyTo":"cover.1772808594.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v8 4/6] trailer: append trailers without fork/exec","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-06T14:53:30Z","receivedAt":"2026-03-06T14:54:22Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Li Chen \n\nIntroduce amend_strbuf_with_trailers() to apply trailer additions to a\nmessage buffer via process_trailers(), avoiding the need to run git\ninterpret-trailers as a child process.\n\nUpdate amend_file_with_trailers() to use the in-process helper and\nrewrite the target file via tempfile+rename, preserving the previous\nin-place semantics. As the trailers are no longer added in a separate\nprocess and trailer_config_init() die()s on missing config values it\nis called early on in cmd_commit() and cmd_tag() so that they die()\nearly before writing the message file. The trailer arguments are now\nalso sanity checked.\n\nKeep existing callers unchanged by continuing to accept argv-style\n--trailer= entries and stripping the prefix before feeding the\nin-process implementation.\n\nSigned-off-by: Li Chen \nSigned-off-by: Phillip Wood \n---\n builtin/commit.c | 3 ++\n builtin/tag.c | 3 ++\n trailer.c | 135 +++++++++++++++++++++++++++++++++++++++++++----\n trailer.h | 20 +++++--\n 4 files changed, 148 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 9e3a09d532b..eb9013995c9 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1820,6 +1820,9 @@ int cmd_commit(int argc,\n \targc = parse_and_validate_options(argc, argv, builtin_commit_options,\n \t\t\t\t\t builtin_commit_usage,\n \t\t\t\t\t prefix, current_head, &s);\n+\tif (trailer_args.nr)\n+\t\ttrailer_config_init();\n+\n \tif (verbose == -1)\n \t\tverbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n \ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex aeb04c487fe..68b581a9c26 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -568,6 +568,9 @@ int cmd_tag(int argc,\n \tif (cmdmode == 'l')\n \t\tsetup_auto_pager(\"tag\", 1);\n \n+\tif (trailer_args.nr)\n+\t\ttrailer_config_init();\n+\n \tif (opt.sign == -1)\n \t\topt.sign = cmdmode ? 0 : config_sign_tag > 0;\n \ndiff --git a/trailer.c b/trailer.c\nindex 163018483a5..5eab4fa549d 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -7,6 +7,7 @@\n #include \"string-list.h\"\n #include \"run-command.h\"\n #include \"commit.h\"\n+#include \"strvec.h\"\n #include \"trailer.h\"\n #include \"list.h\"\n #include \"tempfile.h\"\n@@ -774,6 +775,35 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,\n \tfree(cl_separators);\n }\n \n+int validate_trailer_args(const struct strvec *cli_args)\n+{\n+\tchar *cl_separators;\n+\tint ret = 0;\n+\n+\ttrailer_config_init();\n+\n+\tcl_separators = xstrfmt(\"=%s\", separators);\n+\n+\tfor (size_t i = 0; i < cli_args->nr; i++) {\n+\t\tconst char *txt = cli_args->v[i];\n+\t\tssize_t separator_pos;\n+\n+\t\tif (!*txt) {\n+\t\t\tret = error(_(\"empty --trailer argument\"));\n+\t\t\tgoto out;\n+\t\t}\n+\t\tseparator_pos = find_separator(txt, cl_separators);\n+\t\tif (separator_pos == 0) {\n+\t\t\tret = error(_(\"invalid trailer '%s': missing key before separator\"),\n+\t\t\t\t txt);\n+\t\t\tgoto out;\n+\t\t}\n+\t}\n+out:\n+\tfree(cl_separators);\n+\treturn ret;\n+}\n+\n static const char *next_line(const char *str)\n {\n \tconst char *nl = strchrnul(str, '\\n');\n@@ -1258,16 +1288,101 @@ struct tempfile *trailer_create_in_place_tempfile(const char *file)\n \treturn tempfile;\n }\n \n-int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)\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+int amend_strbuf_with_trailers(struct strbuf *buf,\n+\t\t\t\tconst struct strvec *trailer_args)\n+{\n+\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n+\tLIST_HEAD(new_trailer_head);\n+\tstruct strbuf out = STRBUF_INIT;\n+\tsize_t i;\n+\tint ret = 0;\n+\n+\topts.no_divider = 1;\n+\n+\tfor (i = 0; i < trailer_args->nr; i++) {\n+\t\tconst char *text = trailer_args->v[i];\n+\t\tstruct new_trailer_item *item;\n+\n+\t\tif (!*text) {\n+\t\t\tret = error(_(\"empty --trailer argument\"));\n+\t\t\tgoto out;\n+\t\t}\n+\t\titem = xcalloc(1, sizeof(*item));\n+\t\titem->text = xstrdup(text);\n+\t\tlist_add_tail(&item->list, &new_trailer_head);\n+\t}\n+\n+\tprocess_trailers(&opts, &new_trailer_head, buf, &out);\n+\n+\tstrbuf_swap(buf, &out);\n+out:\n+\tstrbuf_release(&out);\n+\tfree_trailers(&new_trailer_head);\n+\n+\treturn ret;\n+}\n+\n+static int write_file_in_place(const char *path, const struct strbuf *buf)\n+{\n+\tstruct tempfile *tempfile = trailer_create_in_place_tempfile(path);\n+\tif (!tempfile)\n+\t\treturn -1;\n+\n+\tif (write_in_full(tempfile->fd, buf->buf, buf->len) < 0)\n+\t\treturn error_errno(_(\"could not write to temporary file\"));\n+\n+\tif (rename_tempfile(&tempfile, path))\n+\t\treturn error_errno(_(\"could not rename temporary file to %s\"), path);\n+\n+\treturn 0;\n+}\n+\n+int amend_file_with_trailers(const char *path,\n+\t\t\t const struct strvec *trailer_args)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strvec stripped_trailer_args = STRVEC_INIT;\n+\tint ret = 0;\n+\tsize_t i;\n+\n+\tif (!trailer_args)\n+\t\tBUG(\"amend_file_with_trailers called with NULL trailer_args\");\n+\tif (!trailer_args->nr)\n+\t\treturn 0;\n+\n+\tfor (i = 0; i < trailer_args->nr; i++) {\n+\t\tconst char *txt = trailer_args->v[i];\n+\n+\t\t/*\n+\t\t * Historically amend_file_with_trailers() passed its arguments\n+\t\t * to \"git interpret-trailers\", which expected argv entries in\n+\t\t * \"--trailer=\" form. Continue to accept those for\n+\t\t * existing callers, but pass only the value portion to the\n+\t\t * in-process implementation.\n+\t\t */\n+\t\tskip_prefix(txt, \"--trailer=\", &txt);\n+\t\tif (!*txt) {\n+\t\t\tret = error(_(\"empty --trailer argument\"));\n+\t\t\tgoto out;\n+\t\t}\n+\t\tstrvec_push(&stripped_trailer_args, txt);\n+\t}\n+\n+\tif (validate_trailer_args(&stripped_trailer_args)) {\n+\t\tret = -1;\n+\t\tgoto out;\n+\t}\n+\tif (strbuf_read_file(&buf, path, 0) < 0)\n+\t\tret = error_errno(_(\"could not read '%s'\"), path);\n+\telse\n+\t\tamend_strbuf_with_trailers(&buf, &stripped_trailer_args);\n+\n+\tif (!ret)\n+\t\tret = write_file_in_place(path, &buf);\n+out:\n+\tstrvec_clear(&stripped_trailer_args);\n+\tstrbuf_release(&buf);\n+\treturn ret;\n }\n \n void process_trailers(const struct process_trailer_options *opts,\ndiff --git a/trailer.h b/trailer.h\nindex 7fd2564e035..3c5d9a6e199 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -68,6 +68,8 @@ void parse_trailers_from_config(struct list_head *config_head);\n void parse_trailers_from_command_line_args(struct list_head *arg_head,\n \t\t\t\t\t struct list_head *new_trailer_head);\n \n+int validate_trailer_args(const struct strvec *cli_args);\n+\n void process_trailers_lists(struct list_head *head,\n \t\t\t struct list_head *arg_head);\n \n@@ -196,9 +198,21 @@ int trailer_iterator_advance(struct trailer_iterator *iter);\n void trailer_iterator_release(struct trailer_iterator *iter);\n \n /*\n- * Augment a file to add trailers to it by running git-interpret-trailers.\n- * This calls run_command() and its return value is the same (i.e. 0 for\n- * success, various non-zero for other errors). See run-command.h.\n+ * Append trailers specified in trailer_args to buf in-place.\n+ *\n+ * Each element of trailer_args should be in the same format as the value\n+ * accepted by --trailer= (i.e., without the --trailer= prefix).\n+ */\n+int amend_strbuf_with_trailers(struct strbuf *buf,\n+\t\t\t\tconst struct strvec *trailer_args);\n+\n+/*\n+ * Augment a file by appending trailers specified in trailer_args.\n+ *\n+ * Each element of trailer_args should be an argv-style --trailer=\n+ * option (i.e., including the --trailer= prefix).\n+ *\n+ * Returns 0 on success or a non-zero error code on failure.\n */\n int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);\n \n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"538083","messageId":"3c1fa9e85791953fd30176f6afaae80c2de8cef2.1772808594.git.phillip.wood@dunelm.org.uk","threadId":"65068","inReplyTo":"cover.1772808594.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v8 5/6] commit, tag: parse --trailer with OPT_STRVEC","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-06T14:53:31Z","receivedAt":"2026-03-06T14:54:23Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Li Chen \n\nNow that amend_file_with_trailers() expects raw trailer lines, do not\nstore argv-style \"--trailer=\" strings in git commit and git\ntag.\n\nParse --trailer using OPT_STRVEC so trailer_args contains only the\ntrailer value, and drop the temporary prefix stripping in\namend_file_with_trailers().\n\nSigned-off-by: Li Chen \n---\n builtin/commit.c | 3 ++-\n builtin/tag.c | 4 ++--\n trailer.c | 25 ++-----------------------\n trailer.h | 4 ++--\n 4 files changed, 8 insertions(+), 28 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex eb9013995c9..3d25c1856ce 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1720,7 +1720,8 @@ int cmd_commit(int argc,\n \t\tOPT_STRING(0, \"fixup\", &fixup_message, N_(\"[(amend|reword):]commit\"), N_(\"use autosquash formatted message to fixup or amend/reword specified commit\")),\n \t\tOPT_STRING(0, \"squash\", &squash_message, N_(\"commit\"), N_(\"use autosquash formatted message to squash specified commit\")),\n \t\tOPT_BOOL(0, \"reset-author\", &renew_authorship, N_(\"the commit is authored by me now (used with -C/-c/--amend)\")),\n-\t\tOPT_PASSTHRU_ARGV(0, \"trailer\", &trailer_args, N_(\"trailer\"), N_(\"add custom trailer(s)\"), PARSE_OPT_NONEG),\n+\t\tOPT_STRVEC(0, \"trailer\", &trailer_args, N_(\"trailer\"),\n+\t\t\t N_(\"add custom trailer(s)\")),\n \t\tOPT_BOOL('s', \"signoff\", &signoff, N_(\"add a Signed-off-by trailer\")),\n \t\tOPT_FILENAME('t', \"template\", &template_file, N_(\"use specified template file\")),\n \t\tOPT_BOOL('e', \"edit\", &edit_flag, N_(\"force edit of commit\")),\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 68b581a9c26..e0f05f94fdb 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -499,8 +499,8 @@ int cmd_tag(int argc,\n \t\tOPT_CALLBACK_F('m', \"message\", &msg, N_(\"message\"),\n \t\t\t N_(\"tag message\"), PARSE_OPT_NONEG, parse_msg_arg),\n \t\tOPT_FILENAME('F', \"file\", &msgfile, N_(\"read message from file\")),\n-\t\tOPT_PASSTHRU_ARGV(0, \"trailer\", &trailer_args, N_(\"trailer\"),\n-\t\t\t\t N_(\"add custom trailer(s)\"), PARSE_OPT_NONEG),\n+\t\tOPT_STRVEC(0, \"trailer\", &trailer_args, N_(\"trailer\"),\n+\t\t\t N_(\"add custom trailer(s)\")),\n \t\tOPT_BOOL('e', \"edit\", &edit_flag, N_(\"force edit of tag message\")),\n \t\tOPT_BOOL('s', \"sign\", &opt.sign, N_(\"annotated and GPG-signed tag\")),\n \t\tOPT_CLEANUP(&cleanup_arg),\ndiff --git a/trailer.c b/trailer.c\nindex 5eab4fa549d..ca8abd18826 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -1341,46 +1341,25 @@ int amend_file_with_trailers(const char *path,\n \t\t\t const struct strvec *trailer_args)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n-\tstruct strvec stripped_trailer_args = STRVEC_INIT;\n \tint ret = 0;\n-\tsize_t i;\n \n \tif (!trailer_args)\n \t\tBUG(\"amend_file_with_trailers called with NULL trailer_args\");\n \tif (!trailer_args->nr)\n \t\treturn 0;\n \n-\tfor (i = 0; i < trailer_args->nr; i++) {\n-\t\tconst char *txt = trailer_args->v[i];\n-\n-\t\t/*\n-\t\t * Historically amend_file_with_trailers() passed its arguments\n-\t\t * to \"git interpret-trailers\", which expected argv entries in\n-\t\t * \"--trailer=\" form. Continue to accept those for\n-\t\t * existing callers, but pass only the value portion to the\n-\t\t * in-process implementation.\n-\t\t */\n-\t\tskip_prefix(txt, \"--trailer=\", &txt);\n-\t\tif (!*txt) {\n-\t\t\tret = error(_(\"empty --trailer argument\"));\n-\t\t\tgoto out;\n-\t\t}\n-\t\tstrvec_push(&stripped_trailer_args, txt);\n-\t}\n-\n-\tif (validate_trailer_args(&stripped_trailer_args)) {\n+\tif (validate_trailer_args(trailer_args)) {\n \t\tret = -1;\n \t\tgoto out;\n \t}\n \tif (strbuf_read_file(&buf, path, 0) < 0)\n \t\tret = error_errno(_(\"could not read '%s'\"), path);\n \telse\n-\t\tamend_strbuf_with_trailers(&buf, &stripped_trailer_args);\n+\t\tamend_strbuf_with_trailers(&buf, trailer_args);\n \n \tif (!ret)\n \t\tret = write_file_in_place(path, &buf);\n out:\n-\tstrvec_clear(&stripped_trailer_args);\n \tstrbuf_release(&buf);\n \treturn ret;\n }\ndiff --git a/trailer.h b/trailer.h\nindex 3c5d9a6e199..b49338858c4 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -209,8 +209,8 @@ int amend_strbuf_with_trailers(struct strbuf *buf,\n /*\n * Augment a file by appending trailers specified in trailer_args.\n *\n- * Each element of trailer_args should be an argv-style --trailer=\n- * option (i.e., including the --trailer= prefix).\n+ * Each element of trailer_args should be in the same format as the value\n+ * accepted by --trailer= (i.e., without the --trailer= prefix).\n *\n * Returns 0 on success or a non-zero error code on failure.\n */\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"538084","messageId":"99654d805471188695c0ccf400a485db16e51dd4.1772808594.git.phillip.wood@dunelm.org.uk","threadId":"65068","inReplyTo":"cover.1772808594.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v8 6/6] rebase: support --trailer","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-06T14:53:32Z","receivedAt":"2026-03-06T14:54:24Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Li Chen \n\nAdd a new --trailer= option to git rebase to append trailer\nlines to each rewritten commit message (merge backend only).\n\nBecause the apply backend does not provide a commit-message filter,\nreject --trailer when --apply is in effect and require the merge backend\ninstead.\n\nThis option implies --force-rebase so that fast-forwarded commits are\nalso rewritten. Validate trailer arguments early to avoid starting an\ninteractive rebase with invalid input.\n\nAdd integration tests covering error paths and trailer insertion across\nnon-interactive and interactive rebases.\n\nSigned-off-by: Li Chen \nSigned-off-by: Phillip Wood \n---\n Documentation/git-rebase.adoc | 8 ++\n builtin/rebase.c | 19 +++++\n sequencer.c | 52 +++++++++++-\n sequencer.h | 3 +\n t/meson.build | 1 +\n t/t3440-rebase-trailer.sh | 147 ++++++++++++++++++++++++++++++++++\n 6 files changed, 228 insertions(+), 2 deletions(-)\n create mode 100755 t/t3440-rebase-trailer.sh\n\ndiff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc\nindex e177808004f..f6c22d15989 100644\n--- a/Documentation/git-rebase.adoc\n+++ b/Documentation/git-rebase.adoc\n@@ -497,6 +497,13 @@ See also INCOMPATIBLE OPTIONS below.\n +\n See also INCOMPATIBLE OPTIONS below.\n \n+--trailer=::\n+\tAppend the given trailer to every rebased commit message, processed\n+\tvia linkgit:git-interpret-trailers[1]. This option implies\n+\t`--force-rebase`.\n++\n+See also INCOMPATIBLE OPTIONS below.\n+\n -i::\n --interactive::\n \tMake a list of the commits which are about to be rebased. Let the\n@@ -653,6 +660,7 @@ are incompatible with the following options:\n * --[no-]reapply-cherry-picks when used without --keep-base\n * --update-refs\n * --root when used without --onto\n+ * --trailer\n \n In addition, the following pairs of options are incompatible:\n \ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex c487e109077..fe25d2ad4bc 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -36,6 +36,7 @@\n #include \"reset.h\"\n #include \"trace2.h\"\n #include \"hook.h\"\n+#include \"trailer.h\"\n \n static char const * const builtin_rebase_usage[] = {\n \tN_(\"git rebase [-i] [options] [--exec ] \"\n@@ -113,6 +114,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 +145,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,\t\t\\\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 +169,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 +181,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@@ -1132,6 +1140,8 @@ 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+\t\t\t 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@@ -1285,6 +1295,12 @@ int cmd_rebase(int argc,\n \t\t\t builtin_rebase_options,\n \t\t\t builtin_rebase_usage, 0);\n \n+\tif (options.trailer_args.nr) {\n+\t\tif (validate_trailer_args(&options.trailer_args))\n+\t\t\tdie(NULL);\n+\t\toptions.flags |= REBASE_FORCE;\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@@ -1542,6 +1558,9 @@ int cmd_rebase(int argc,\n \tif (options.root && !options.onto_name)\n \t\timply_merge(&options, \"--root without --onto\");\n \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 a3eb39bb252..a2d72ce8b3b 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -209,6 +209,7 @@ static GIT_PATH_FUNC(rebase_path_reschedule_failed_exec, \"rebase-merge/reschedul\n static GIT_PATH_FUNC(rebase_path_no_reschedule_failed_exec, \"rebase-merge/no-reschedule-failed-exec\")\n static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, \"rebase-merge/drop_redundant_commits\")\n static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, \"rebase-merge/keep_redundant_commits\")\n+static GIT_PATH_FUNC(rebase_path_trailer, \"rebase-merge/trailer\")\n \n /*\n * A 'struct replay_ctx' represents the private state of the sequencer.\n@@ -420,6 +421,7 @@ void replay_opts_release(struct replay_opts *opts)\n \tif (opts->revs)\n \t\trelease_revisions(opts->revs);\n \tfree(opts->revs);\n+\tstrvec_clear(&opts->trailer_args);\n \treplay_ctx_release(ctx);\n \tfree(opts->ctx);\n }\n@@ -2019,12 +2021,15 @@ static int append_squash_message(struct strbuf *buf, const char *body,\n \tif (is_fixup_flag(command, flag) && !seen_squash(ctx)) {\n \t\t/*\n \t\t * We're replacing the commit message so we need to\n-\t\t * append the Signed-off-by: trailer if the user\n-\t\t * requested '--signoff'.\n+\t\t * append any trailers if the user requested\n+\t\t * '--signoff' or '--trailer'.\n \t\t */\n \t\tif (opts->signoff)\n \t\t\tappend_signoff(buf, 0, 0);\n \n+\t\tif (opts->trailer_args.nr)\n+\t\t\tamend_strbuf_with_trailers(buf, &opts->trailer_args);\n+\n \t\tif ((command == TODO_FIXUP) &&\n \t\t (flag & TODO_REPLACE_FIXUP_MSG) &&\n \t\t (file_exists(rebase_path_fixup_msg()) ||\n@@ -2443,6 +2448,9 @@ static int do_pick_commit(struct repository *r,\n \tif (opts->signoff && !is_fixup(command))\n \t\tappend_signoff(&ctx->message, 0, 0);\n \n+\tif (opts->trailer_args.nr && !is_fixup(command))\n+\t\tamend_strbuf_with_trailers(&ctx->message, &opts->trailer_args);\n+\n \tif (is_rebase_i(opts) && write_author_script(msg.message) < 0)\n \t\tres = -1;\n \telse if (!opts->strategy ||\n@@ -3172,6 +3180,33 @@ static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)\n \tparse_strategy_opts(opts, buf->buf);\n }\n \n+static int read_trailers(struct replay_opts *opts, struct strbuf *buf)\n+{\n+\tssize_t len;\n+\n+\tstrbuf_reset(buf);\n+\tlen = strbuf_read_file(buf, rebase_path_trailer(), 0);\n+\tif (len > 0) {\n+\t\tchar *p = buf->buf, *nl;\n+\n+\t\ttrailer_config_init();\n+\n+\t\twhile ((nl = strchr(p, '\\n'))) {\n+\t\t\t*nl = '\\0';\n+\t\t\tif (!*p)\n+\t\t\t\treturn error(_(\"trailers file contains empty line\"));\n+\t\t\tstrvec_push(&opts->trailer_args, p);\n+\t\t\tp = nl + 1;\n+\t\t}\n+\t} else if (!len) {\n+\t\treturn error(_(\"trailers file is empty\"));\n+\t} else if (errno != ENOENT) {\n+\t\treturn error(_(\"cannot read trailers files\"));\n+\t}\n+\n+\treturn 0;\n+}\n+\n static int read_populate_opts(struct replay_opts *opts)\n {\n \tstruct replay_ctx *ctx = opts->ctx;\n@@ -3233,6 +3268,11 @@ static int read_populate_opts(struct replay_opts *opts)\n \t\t\topts->keep_redundant_commits = 1;\n \n \t\tread_strategy_opts(opts, &buf);\n+\n+\t\tif (read_trailers(opts, &buf)) {\n+\t\t\tret = -1;\n+\t\t\tgoto done_rebase_i;\n+\t\t}\n \t\tstrbuf_reset(&buf);\n \n \t\tif (read_oneliner(&ctx->current_fixups,\n@@ -3328,6 +3368,14 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,\n \t\twrite_file(rebase_path_reschedule_failed_exec(), \"%s\", \"\");\n \telse\n \t\twrite_file(rebase_path_no_reschedule_failed_exec(), \"%s\", \"\");\n+\tif (opts->trailer_args.nr) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\n+\t\tfor (size_t i = 0; i < opts->trailer_args.nr; i++)\n+\t\t\tstrbuf_addf(&buf, \"%s\\n\", opts->trailer_args.v[i]);\n+\t\twrite_file(rebase_path_trailer(), \"%s\", buf.buf);\n+\t\tstrbuf_release(&buf);\n+\t}\n \n \treturn 0;\n }\ndiff --git a/sequencer.h b/sequencer.h\nindex 719684c8a9f..bea20da085f 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -57,6 +57,8 @@ struct replay_opts {\n \tint ignore_date;\n \tint commit_use_reference;\n \n+\tstruct strvec trailer_args;\n+\n \tint mainline;\n \n \tchar *gpg_sign;\n@@ -84,6 +86,7 @@ struct replay_opts {\n #define REPLAY_OPTS_INIT {\t\t\t\\\n \t.edit = -1,\t\t\t\t\\\n \t.action = -1,\t\t\t\t\\\n+\t.trailer_args = STRVEC_INIT,\t\t\\\n \t.xopts = STRVEC_INIT,\t\t\t\\\n \t.ctx = replay_ctx_new(),\t\t\\\n }\ndiff --git a/t/meson.build b/t/meson.build\nindex 6d91470ebc1..3e0bb631af8 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -390,6 +390,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 't3450-history.sh',\n 't3451-history-reword.sh',\n 't3500-cherry.sh',\ndiff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh\nnew file mode 100755\nindex 00000000000..8b475795660\n--- /dev/null\n+++ b/t/t3440-rebase-trailer.sh\n@@ -0,0 +1,147 @@\n+#!/bin/sh\n+#\n+\n+test_description='git rebase --trailer integration tests\n+We verify that --trailer works with the merge backend,\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+REVIEWED_BY_TRAILER=\"Reviewed-by: Dev \"\n+SP=\" \"\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+\tgit checkout main\n+'\n+\n+test_expect_success 'apply backend is rejected with --trailer' '\n+\tgit checkout -B apply-backend third &&\n+\ttest_expect_code 128 \\\n+\t\tgit rebase --apply --trailer \"$REVIEWED_BY_TRAILER\" HEAD^ 2>err &&\n+\ttest_grep \"fatal: --trailer requires the merge backend\" err\n+'\n+\n+test_expect_success 'reject empty --trailer argument' '\n+\tgit checkout -B empty-trailer third &&\n+\ttest_expect_code 128 git rebase --trailer \"\" HEAD^ 2>err &&\n+\ttest_grep \"empty --trailer\" err\n+'\n+\n+test_expect_success 'reject trailer with missing key before separator' '\n+\tgit checkout -B missing-key third &&\n+\ttest_expect_code 128 git rebase --trailer \": no-key\" HEAD^ 2>err &&\n+\ttest_grep \"missing key before separator\" err\n+'\n+\n+test_expect_success 'allow trailer with missing value after separator' '\n+\tgit checkout -B missing-value third &&\n+\tgit rebase --trailer \"Acked-by:\" HEAD^ &&\n+\ttest_commit_message HEAD <<-EOF\n+\tthird\n+\n+\tAcked-by:${SP}\n+\tEOF\n+'\n+\n+test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last' '\n+\tgit checkout -B replace-policy third &&\n+\tgit -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add \\\n+\t\trebase --trailer \"Bug: 123\" --trailer \"Bug: 456\" HEAD^ &&\n+\ttest_commit_message HEAD <<-EOF\n+\tthird\n+\n+\tBug: 456\n+\tEOF\n+'\n+\n+test_expect_success 'multiple Signed-off-by trailers all preserved' '\n+\tgit checkout -B multiple-signoff third &&\n+\tgit rebase --trailer \"Signed-off-by: Dev A \" \\\n+\t\t--trailer \"Signed-off-by: Dev B \" HEAD^ &&\n+\ttest_commit_message HEAD <<-EOF\n+\tthird\n+\n+\tSigned-off-by: Dev A \n+\tSigned-off-by: Dev B \n+\tEOF\n+'\n+\n+test_expect_success 'rebase --trailer adds trailer after conflicts' '\n+\tgit checkout -B trailer-conflict third &&\n+\ttest_commit fourth file &&\n+\ttest_must_fail git rebase --trailer \"$REVIEWED_BY_TRAILER\" second &&\n+\tgit checkout --theirs file &&\n+\tgit add file &&\n+\tgit rebase --continue &&\n+\ttest_commit_message HEAD <<-EOF &&\n+\tfourth\n+\n+\t$REVIEWED_BY_TRAILER\n+\tEOF\n+\ttest_commit_message HEAD^ <<-EOF\n+\tthird\n+\n+\t$REVIEWED_BY_TRAILER\n+\tEOF\n+'\n+\n+test_expect_success '--trailer handles fixup commands in todo list' '\n+\tgit checkout -B fixup-trailer third &&\n+\ttest_commit fixup-base base &&\n+\ttest_commit fixup-second second &&\n+\tcat >todo <<-\\EOF &&\n+\tpick fixup-base fixup-base\n+\tfixup fixup-second fixup-second\n+\tEOF\n+\t(\n+\t\tset_replace_editor todo &&\n+\t\tgit rebase -i --trailer \"$REVIEWED_BY_TRAILER\" HEAD~2\n+\t) &&\n+\ttest_commit_message HEAD <<-EOF &&\n+\tfixup-base\n+\n+\t$REVIEWED_BY_TRAILER\n+\tEOF\n+\tgit reset --hard fixup-second &&\n+\tcat >todo <<-\\EOF &&\n+\tpick fixup-base fixup-base\n+\tfixup -C fixup-second fixup-second\n+\tEOF\n+\t(\n+\t\tset_replace_editor todo &&\n+\t\tgit rebase -i --trailer \"$REVIEWED_BY_TRAILER\" HEAD~2\n+\t) &&\n+\ttest_commit_message HEAD <<-EOF\n+\tfixup-second\n+\n+\t$REVIEWED_BY_TRAILER\n+\tEOF\n+'\n+\n+test_expect_success 'rebase --root honors trailer..key' '\n+\tgit checkout -B root-trailer first &&\n+\tgit -c trailer.review.key=Reviewed-by rebase --root \\\n+\t\t--trailer=review=\"Dev \" &&\n+\ttest_commit_message HEAD <<-EOF &&\n+\tfirst\n+\n+\tReviewed-by: Dev \n+\tEOF\n+\ttest_commit_message HEAD^ <<-EOF\n+\tInitial empty commit\n+\n+\tReviewed-by: Dev \n+\tEOF\n+'\n+test_done\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"538085","messageId":"8a37a357-5a3b-4ca1-9949-6b2f28d3f208@gmail.com","threadId":"65068","inReplyTo":"19cbe42b2cd.bb6e3883730656.6495265672263159010@linux.beauty","subject":"Re: [PATCH v7 0/5] rebase: support --trailer","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-06T14:55:48Z","receivedAt":"2026-03-06T14:55:59Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Li\n\nOn 05/03/2026 13:49, Li Chen wrote:\n> >\n> > This all looks pretty good. The main thing I think we want to change is\n> > sharing the code that creates the temporary file in patch 3. I've push a\n> > version that does that and fixes my other small comments to\n> > https://github.com/phillipwood/git/commits/refs/heads/rebase-trailers-v8\n> > The range diff is below. If you're happy I can post that as a hopefully\n> > final v8. Of course if you want to work on it yourself you're very\n> > welcome to do that.\n> \n> These changes all look good to me! Thank you very much for your patience and work! Please help\n> post this as v8.\n\nI've just posted v8, thanks for all your work on these patches\n\nPhillip\n\n> Regards,\n> Li\n> \n> >\n> > 1: b2685e34c22 ! 1: 0d08b361995 interpret-trailers: factor trailer rewriting\n> > @@ Commit message\n> > This separation makes it easier to move the helper into trailer.c in the\n> > next commit.\n> >\n> > + Signed-off-by: Phillip Wood \n> > Signed-off-by: Li Chen \n> >\n> > ## builtin/interpret-trailers.c ##\n> > @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)\n> > -: ----------- > 2: 5a4d03ab375 interpret-trailers: refactor create_in_place_tempfile()\n> > 2: 1bac3025045 ! 3: ab7e232a95d trailer: move process_trailers to trailer.h\n> > @@ Metadata\n> > Author: Li Chen \n> >\n> > ## Commit message ##\n> > - trailer: move process_trailers to trailer.h\n> > -\n> > - Move process_trailers() from builtin/interpret-trailers.c into trailer.c\n> > - and expose it via trailer.h.\n> > -\n> > - This lets other call sites reuse the same trailer rewriting logic.\n> > + trailer: libify a couple of functions\n> > +\n> > + Move create_in_place_tempfile() and process_trailers() from\n> > + builtin/interpret-trailers.c into trailer.c and expose it via trailer.h.\n> > +\n> > + This reverts most of ae0ec2e0e0b (trailer: move interpret_trailers()\n> > + to interpret-trailers.c, 2024-03-01) and lets other call sites reuse\n> > + the same trailer rewriting logic.\n> >\n> > Signed-off-by: Li Chen \n> > + Signed-off-by: Phillip Wood \n> >\n> > ## builtin/interpret-trailers.c ##\n> > +@@ builtin/interpret-trailers.c: static int parse_opt_parse(const struct option *opt, const char *arg,\n> > + return 0;\n> > + }\n> > +\n> > +-\n> > +-static struct tempfile *create_in_place_tempfile(const char *file)\n> > +-{\n> > +- struct tempfile *tempfile = NULL;\n> > +- struct stat st;\n> > +- struct strbuf filename_template = STRBUF_INIT;\n> > +- const char *tail;\n> > +-\n> > +- if (stat(file, &st)) {\n> > +- error_errno(_(\"could not stat %s\"), file);\n> > +- return NULL;\n> > +- }\n> > +- if (!S_ISREG(st.st_mode)) {\n> > +- error(_(\"file %s is not a regular file\"), file);\n> > +- return NULL;\n> > +- }\n> > +- if (!(st.st_mode & S_IWUSR)) {\n> > +- error(_(\"file %s is not writable by user\"), file);\n> > +- return NULL;\n> > +- }\n> > +- /* Create temporary file in the same directory as the original */\n> > +- tail = find_last_dir_sep(file);\n> > +- if (tail)\n> > +- strbuf_add(&filename_template, file, tail - file + 1);\n> > +- strbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n> > +-\n> > +- tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);\n> > +-\n> > +- strbuf_release(&filename_template);\n> > +-\n> > +- return tempfile;\n> > +-}\n> > +-\n> > + static void read_input_file(struct strbuf *sb, const char *file)\n> > + {\n> > + if (file) {\n> > @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)\n> > strbuf_complete_line(sb);\n> > }\n> > @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, con\n> > static void interpret_trailers(const struct process_trailer_options *opts,\n> > struct list_head *new_trailer_head,\n> > const char *file)\n> > +@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,\n> > + read_input_file(&input, file);\n> > +\n> > + if (opts->in_place) {\n> > +- tempfile = create_in_place_tempfile(file);\n> > ++ tempfile = trailer_create_in_place_tempfile(file);\n> > + if (!tempfile)\n> > + die(NULL);\n> > + fd = tempfile->fd;\n> >\n> > ## trailer.c ##\n> > +@@\n> > + #include \"commit.h\"\n> > + #include \"trailer.h\"\n> > + #include \"list.h\"\n> > ++#include \"tempfile.h\"\n> > ++\n> > + /*\n> > + * Copyright (c) 2013, 2014 Christian Couder \n> > + */\n> > +@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n> > + strbuf_release(&iter->key);\n> > + }\n> > +\n> > ++struct tempfile *trailer_create_in_place_tempfile(const char *file)\n> > ++{\n> > ++ struct tempfile *tempfile = NULL;\n> > ++ struct stat st;\n> > ++ struct strbuf filename_template = STRBUF_INIT;\n> > ++ const char *tail;\n> > ++\n> > ++ if (stat(file, &st)) {\n> > ++ error_errno(_(\"could not stat %s\"), file);\n> > ++ return NULL;\n> > ++ }\n> > ++ if (!S_ISREG(st.st_mode)) {\n> > ++ error(_(\"file %s is not a regular file\"), file);\n> > ++ return NULL;\n> > ++ }\n> > ++ if (!(st.st_mode & S_IWUSR)) {\n> > ++ error(_(\"file %s is not writable by user\"), file);\n> > ++ return NULL;\n> > ++ }\n> > ++ /* Create temporary file in the same directory as the original */\n> > ++ tail = find_last_dir_sep(file);\n> > ++ if (tail)\n> > ++ strbuf_add(&filename_template, file, tail - file + 1);\n> > ++ strbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n> > ++\n> > ++ tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);\n> > ++\n> > ++ strbuf_release(&filename_template);\n> > ++\n> > ++ return tempfile;\n> > ++}\n> > ++\n> > + int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)\n> > + {\n> > + struct child_process run_trailer = CHILD_PROCESS_INIT;\n> > @@ trailer.c: int amend_file_with_trailers(const char *path, const struct strvec *trailer_args\n> > strvec_pushv(&run_trailer.args, trailer_args->v);\n> > return run_command(&run_trailer);\n> > @@ trailer.h: 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> > ++ * Create a tempfile \"\"git-interpret-trailers-XXXXXX\" in the same\n> > ++ * directory as file.\n> > ++ */\n> > ++struct tempfile *trailer_create_in_place_tempfile(const char *file);\n> > ++\n> > ++/*\n> > ++ * Rewrite the contents of input by processing its trailer block according to\n> > ++ * opts and (optionally) appending trailers from new_trailer_head.\n> > ++ *\n> > ++ * The rewritten message is appended to out (callers should strbuf_reset()\n> > ++ * first if needed).\n> > ++ */\n> > +void process_trailers(const struct process_trailer_options *opts,\n> > + struct list_head *new_trailer_head,\n> > + struct strbuf *input, struct strbuf *out);\n> > 3: 3114f0dbb57 ! 4: 1f24917eb64 trailer: append trailers without fork/exec\n> > @@ Commit message\n> >\n> > Update amend_file_with_trailers() to use the in-process helper and\n> > rewrite the target file via tempfile+rename, preserving the previous\n> > - in-place semantics.\n> > + in-place semantics. As the trailers are no longer added in a separate\n> > + process and trailer_config_init() die()s on missing config values it\n> > + is called early on in cmd_commit() and cmd_tag() so that they die()\n> > + early before writing the message file. The trailer arguments are now\n> > + also sanity checked.\n> >\n> > Keep existing callers unchanged by continuing to accept argv-style\n> > --trailer= entries and stripping the prefix before feeding the\n> > in-process implementation.\n> >\n> > Signed-off-by: Li Chen \n> > + Signed-off-by: Phillip Wood \n> > -\n> > - ## builtin/interpret-trailers.c ##\n> > -@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,\n> > - struct strbuf out = STRBUF_INIT;\n> > - FILE *outfile = stdout;\n> > -\n> > -- trailer_config_init();\n> > --\n> > - read_input_file(&input, file);\n> > -\n> > - if (opts->in_place)\n> > -@@ builtin/interpret-trailers.c: int cmd_interpret_trailers(int argc,\n> > - git_interpret_trailers_usage,\n> > - options);\n> > -\n> > -+ trailer_config_init();\n> > -+\n> > - if (argc) {\n> > - int i;\n> > - for (i = 0; i < argc; i++)\n> > + Signed-off-by: Phillip Wood \n> > +\n> > + ## builtin/commit.c ##\n> > +@@ builtin/commit.c: int cmd_commit(int argc,\n> > + argc = parse_and_validate_options(argc, argv, builtin_commit_options,\n> > + builtin_commit_usage,\n> > + prefix, current_head, &s);\n> > ++ if (trailer_args.nr)\n> > ++ trailer_config_init();\n> > ++\n> > + if (verbose == -1)\n> > + verbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n> > +\n> > +\n> > + ## builtin/tag.c ##\n> > +@@ builtin/tag.c: int cmd_tag(int argc,\n> > + if (cmdmode == 'l')\n> > + setup_auto_pager(\"tag\", 1);\n> > +\n> > ++ if (trailer_args.nr)\n> > ++ trailer_config_init();\n> > ++\n> > + if (opt.sign == -1)\n> > + opt.sign = cmdmode ? 0 : config_sign_tag > 0;\n> > +\n> >\n> > ## trailer.c ##\n> > @@\n> > #include \"string-list.h\"\n> > #include \"run-command.h\"\n> > #include \"commit.h\"\n> > +#include \"strvec.h\"\n> > -+#include \"tempfile.h\"\n> > #include \"trailer.h\"\n> > #include \"list.h\"\n> > -+\n> > - /*\n> > - * Copyright (c) 2013, 2014 Christian Couder \n> > - */\n> > + #include \"tempfile.h\"\n> > @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head,\n> > free(cl_separators);\n> > }\n> >\n> > -+void validate_trailer_args(const struct strvec *cli_args)\n> > ++int validate_trailer_args(const struct strvec *cli_args)\n> > +{\n> > + char *cl_separators;\n> > ++ int ret = 0;\n> > +\n> > + trailer_config_init();\n> > +\n> > @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head\n> > + const char *txt = cli_args->v[i];\n> > + ssize_t separator_pos;\n> > +\n> > -+ if (!*txt)\n> > -+ die(_(\"empty --trailer argument\"));\n> > -+\n> > ++ if (!*txt) {\n> > ++ ret = error(_(\"empty --trailer argument\"));\n> > ++ goto out;\n> > ++ }\n> > + separator_pos = find_separator(txt, cl_separators);\n> > -+ if (separator_pos == 0)\n> > -+ die(_(\"invalid trailer '%s': missing key before separator\"),\n> > -+ txt);\n> > ++ if (separator_pos == 0) {\n> > ++ ret = error(_(\"invalid trailer '%s': missing key before separator\"),\n> > ++ txt);\n> > ++ goto out;\n> > ++ }\n> > + }\n> > -+\n> > ++out:\n> > + free(cl_separators);\n> > ++ return ret;\n> > +}\n> > +\n> > static const char *next_line(const char *str)\n> > {\n> > const char *nl = strchrnul(str, '\\n');\n> > -@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n> > - strbuf_release(&iter->key);\n> > +@@ trailer.c: struct tempfile *trailer_create_in_place_tempfile(const char *file)\n> > + return tempfile;\n> > }\n> >\n> > -int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)\n> > @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n> > - path, NULL);\n> > - strvec_pushv(&run_trailer.args, trailer_args->v);\n> > - return run_command(&run_trailer);\n> > -+static void new_trailer_items_clear(struct list_head *items)\n> > -+{\n> > -+ while (!list_empty(items)) {\n> > -+ struct new_trailer_item *item =\n> > -+ list_first_entry(items, struct new_trailer_item, list);\n> > -+ list_del(&item->list);\n> > -+ free(item);\n> > -+ }\n> > -+}\n> > -+\n> > -+void amend_strbuf_with_trailers(struct strbuf *buf,\n> > ++int amend_strbuf_with_trailers(struct strbuf *buf,\n> > + const struct strvec *trailer_args)\n> > +{\n> > + struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n> > + LIST_HEAD(new_trailer_head);\n> > + struct strbuf out = STRBUF_INIT;\n> > + size_t i;\n> > ++ int ret = 0;\n> > +\n> > + opts.no_divider = 1;\n> > +\n> > + for (i = 0; i < trailer_args->nr; i++) {\n> > + const char *text = trailer_args->v[i];\n> > + struct new_trailer_item *item;\n> > +\n> > -+ if (!*text)\n> > -+ die(_(\"empty --trailer argument\"));\n> > ++ if (!*text) {\n> > ++ ret = error(_(\"empty --trailer argument\"));\n> > ++ goto out;\n> > ++ }\n> > + item = xcalloc(1, sizeof(*item));\n> > -+ item->text = text;\n> > ++ item->text = xstrdup(text);\n> > + list_add_tail(&item->list, &new_trailer_head);\n> > + }\n> > +\n> > -+ trailer_config_init();\n> > + process_trailers(&opts, &new_trailer_head, buf, &out);\n> > +\n> > + strbuf_swap(buf, &out);\n> > ++out:\n> > + strbuf_release(&out);\n> > ++ free_trailers(&new_trailer_head);\n> > +\n> > -+ new_trailer_items_clear(&new_trailer_head);\n> > ++ return ret;\n> > +}\n> > +\n> > +static int write_file_in_place(const char *path, const struct strbuf *buf)\n> > +{\n> > -+ struct stat st;\n> > -+ struct strbuf filename_template = STRBUF_INIT;\n> > -+ const char *tail;\n> > -+ struct tempfile *tempfile;\n> > -+ FILE *outfile;\n> > -+\n> > -+ if (stat(path, &st))\n> > -+ return error_errno(_(\"could not stat %s\"), path);\n> > -+ if (!S_ISREG(st.st_mode))\n> > -+ return error(_(\"file %s is not a regular file\"), path);\n> > -+ if (!(st.st_mode & S_IWUSR))\n> > -+ return error(_(\"file %s is not writable by user\"), path);\n> > -+\n> > -+ /* Create temporary file in the same directory as the original */\n> > -+ tail = strrchr(path, '/');\n> > -+ if (tail)\n> > -+ strbuf_add(&filename_template, path, tail - path + 1);\n> > -+ strbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n> > -+\n> > -+ tempfile = mks_tempfile_sm(filename_template.buf, 0, st.st_mode);\n> > -+ strbuf_release(&filename_template);\n> > ++ struct tempfile *tempfile = trailer_create_in_place_tempfile(path);\n> > + if (!tempfile)\n> > -+ return error_errno(_(\"could not create temporary file\"));\n> > -+\n> > -+ outfile = fdopen_tempfile(tempfile, \"w\");\n> > -+ if (!outfile) {\n> > -+ int saved_errno = errno;\n> > -+ delete_tempfile(&tempfile);\n> > -+ errno = saved_errno;\n> > -+ return error_errno(_(\"could not open temporary file\"));\n> > -+ }\n> > -+\n> > -+ if (buf->len && fwrite(buf->buf, 1, buf->len, outfile) < buf->len) {\n> > -+ int saved_errno = errno;\n> > -+ delete_tempfile(&tempfile);\n> > -+ errno = saved_errno;\n> > ++ return -1;\n> > ++\n> > ++ if (write_in_full(tempfile->fd, buf->buf, buf->len) < 0)\n> > + return error_errno(_(\"could not write to temporary file\"));\n> > -+ }\n> > +\n> > + if (rename_tempfile(&tempfile, path))\n> > + return error_errno(_(\"could not rename temporary file to %s\"), path);\n> > @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)\n> > + * in-process implementation.\n> > + */\n> > + skip_prefix(txt, \"--trailer=\", &txt);\n> > -+ if (!*txt)\n> > -+ die(_(\"empty --trailer argument\"));\n> > ++ if (!*txt) {\n> > ++ ret = error(_(\"empty --trailer argument\"));\n> > ++ goto out;\n> > ++ }\n> > + strvec_push(&stripped_trailer_args, txt);\n> > + }\n> > +\n> > ++ if (validate_trailer_args(&stripped_trailer_args)) {\n> > ++ ret = -1;\n> > ++ goto out;\n> > ++ }\n> > + if (strbuf_read_file(&buf, path, 0) < 0)\n> > + ret = error_errno(_(\"could not read '%s'\"), path);\n> > + else\n> > + amend_strbuf_with_trailers(&buf, &stripped_trailer_args);\n> > +\n> > + if (!ret)\n> > + ret = write_file_in_place(path, &buf);\n> > -+\n> > ++out:\n> > + strvec_clear(&stripped_trailer_args);\n> > + strbuf_release(&buf);\n> > + return ret;\n> > @@ trailer.h: void parse_trailers_from_config(struct list_head *config_head);\n> > void parse_trailers_from_command_line_args(struct list_head *arg_head,\n> > struct list_head *new_trailer_head);\n> >\n> > -+void validate_trailer_args(const struct strvec *cli_args);\n> > ++int validate_trailer_args(const struct strvec *cli_args);\n> > +\n> > void process_trailers_lists(struct list_head *head,\n> > struct list_head *arg_head);\n> > @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);\n> > + * Each element of trailer_args should be in the same format as the value\n> > + * accepted by --trailer= (i.e., without the --trailer= prefix).\n> > + */\n> > -+void amend_strbuf_with_trailers(struct strbuf *buf,\n> > ++int amend_strbuf_with_trailers(struct strbuf *buf,\n> > + const struct strvec *trailer_args);\n> > +\n> > +/*\n> > @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);\n> > */\n> > int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);\n> >\n> > -+/*\n> > -+ * Rewrite the contents of input by processing its trailer block according to\n> > -+ * opts and (optionally) appending trailers from new_trailer_head.\n> > -+ *\n> > -+ * The rewritten message is appended to out (callers should strbuf_reset()\n> > -+ * first if needed).\n> > -+ */\n> > - void process_trailers(const struct process_trailer_options *opts,\n> > - struct list_head *new_trailer_head,\n> > - struct strbuf *input, struct strbuf *out);\n> > 4: 147595a9317 ! 5: 3c1fa9e8579 commit, tag: parse --trailer with OPT_STRVEC\n> > @@ Commit message\n> > amend_file_with_trailers().\n> >\n> > Signed-off-by: Li Chen \n> >\n> > ## builtin/commit.c ##\n> > @@ builtin/commit.c: int cmd_commit(int argc,\n> > @@ trailer.c: int amend_file_with_trailers(const char *path,\n> > - * in-process implementation.\n> > - */\n> > - skip_prefix(txt, \"--trailer=\", &txt);\n> > -- if (!*txt)\n> > -- die(_(\"empty --trailer argument\"));\n> > +- if (!*txt) {\n> > +- ret = error(_(\"empty --trailer argument\"));\n> > +- goto out;\n> > +- }\n> > - strvec_push(&stripped_trailer_args, txt);\n> > - }\n> > -\n> > +- if (validate_trailer_args(&stripped_trailer_args)) {\n> > ++ if (validate_trailer_args(trailer_args)) {\n> > + ret = -1;\n> > + goto out;\n> > + }\n> > if (strbuf_read_file(&buf, path, 0) < 0)\n> > ret = error_errno(_(\"could not read '%s'\"), path);\n> > else\n> > @@ trailer.c: int amend_file_with_trailers(const char *path,\n> >\n> > if (!ret)\n> > ret = write_file_in_place(path, &buf);\n> > -\n> > + out:\n> > - strvec_clear(&stripped_trailer_args);\n> > strbuf_release(&buf);\n> > return ret;\n> > }\n> >\n> > ## trailer.h ##\n> > -@@ trailer.h: void amend_strbuf_with_trailers(struct strbuf *buf,\n> > +@@ trailer.h: int amend_strbuf_with_trailers(struct strbuf *buf,\n> > /*\n> > * Augment a file by appending trailers specified in trailer_args.\n> > *\n> > 5: 864cf5f8eb6 ! 6: 99654d80547 rebase: support --trailer\n> > @@ Commit message\n> > non-interactive and interactive rebases.\n> >\n> > Signed-off-by: Li Chen \n> > + Signed-off-by: Phillip Wood \n> >\n> > ## Documentation/git-rebase.adoc ##\n> > @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.\n> > @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.\n> > +--trailer=::\n> > + Append the given trailer to every rebased commit message, processed\n> > + via linkgit:git-interpret-trailers[1]. This option implies\n> > -+ `--force-rebase` so that fast-forwarded commits are also rewritten.\n> > ++ `--force-rebase`.\n> > ++\n> > +See also INCOMPATIBLE OPTIONS below.\n> > +\n> > -i::\n> > --interactive::\n> > Make a list of the commits which are about to be rebased. Let the\n> > +@@ Documentation/git-rebase.adoc: are incompatible with the following options:\n> > + * --[no-]reapply-cherry-picks when used without --keep-base\n> > + * --update-refs\n> > + * --root when used without --onto\n> > ++ * --trailer\n> > +\n> > + In addition, the following pairs of options are incompatible:\n> > +\n> >\n> > ## builtin/rebase.c ##\n> > @@\n> > @@ builtin/rebase.c: int cmd_rebase(int argc,\n> > builtin_rebase_usage, 0);\n> >\n> > + if (options.trailer_args.nr) {\n> > -+ validate_trailer_args(&options.trailer_args);\n> > ++ if (validate_trailer_args(&options.trailer_args))\n> > ++ die(NULL);\n> > + options.flags |= REBASE_FORCE;\n> > + }\n> > +\n> > @@ sequencer.c: void replay_opts_release(struct replay_opts *opts)\n> > free(opts->ctx);\n> > }\n> > @@ sequencer.c: static int append_squash_message(struct strbuf *buf, const char *body,\n> > + if (is_fixup_flag(command, flag) && !seen_squash(ctx)) {\n> > + /*\n> > + * We're replacing the commit message so we need to\n> > +- * append the Signed-off-by: trailer if the user\n> > +- * requested '--signoff'.\n> > ++ * append any trailers if the user requested\n> > ++ * '--signoff' or '--trailer'.\n> > + */\n> > if (opts->signoff)\n> > append_signoff(buf, 0, 0);\n> >\n> > @@ sequencer.c: static int do_pick_commit(struct repository *r,\n> > if (is_rebase_i(opts) && write_author_script(msg.message) < 0)\n> > res = -1;\n> > else if (!opts->strategy ||\n> > +@@ sequencer.c: static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)\n> > + parse_strategy_opts(opts, buf->buf);\n> > + }\n> > +\n> > ++static int read_trailers(struct replay_opts *opts, struct strbuf *buf)\n> > ++{\n> > ++ ssize_t len;\n> > ++\n> > ++ strbuf_reset(buf);\n> > ++ len = strbuf_read_file(buf, rebase_path_trailer(), 0);\n> > ++ if (len > 0) {\n> > ++ char *p = buf->buf, *nl;\n> > ++\n> > ++ trailer_config_init();\n> > ++\n> > ++ while ((nl = strchr(p, '\\n'))) {\n> > ++ *nl = '\\0';\n> > ++ if (!*p)\n> > ++ return error(_(\"trailers file contains empty line\"));\n> > ++ strvec_push(&opts->trailer_args, p);\n> > ++ p = nl + 1;\n> > ++ }\n> > ++ } else if (!len) {\n> > ++ return error(_(\"trailers file is empty\"));\n> > ++ } else if (errno != ENOENT) {\n> > ++ return error(_(\"cannot read trailers files\"));\n> > ++ }\n> > ++\n> > ++ return 0;\n> > ++}\n> > ++\n> > + static int read_populate_opts(struct replay_opts *opts)\n> > + {\n> > + struct replay_ctx *ctx = opts->ctx;\n> > @@ sequencer.c: static int read_populate_opts(struct replay_opts *opts)\n> > + opts->keep_redundant_commits = 1;\n> >\n> > read_strategy_opts(opts, &buf);\n> > ++\n> > ++ if (read_trailers(opts, &buf)) {\n> > ++ ret = -1;\n> > ++ goto done_rebase_i;\n> > ++ }\n> > strbuf_reset(&buf);\n> > -+ if (strbuf_read_file(&buf, rebase_path_trailer(), 0) >= 0) {\n> > -+ char *p = buf.buf, *nl;\n> > -+\n> > -+ while ((nl = strchr(p, '\\n'))) {\n> > -+ *nl = '\\0';\n> > -+ if (!*p)\n> > -+ BUG(\"rebase-merge/trailer has an empty line\");\n> > -+ strvec_push(&opts->trailer_args, p);\n> > -+ p = nl + 1;\n> > -+ }\n> > -+ strbuf_reset(&buf);\n> > -+ }\n> >\n> > if (read_oneliner(&ctx->current_fixups,\n> > - rebase_path_current_fixups(),\n> > @@ sequencer.c: int write_basic_state(struct replay_opts *opts, const char *head_name,\n> > write_file(rebase_path_reschedule_failed_exec(), \"%s\", \"\");\n> > else\n> >\n> >\n> \n\n"},{"id":"538112","messageId":"xmqqo6l0zn2p.fsf@gitster.g","threadId":"65068","inReplyTo":"0d08b361995f4d117b3c80a7e403c1a821d10a5f.1772808594.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH v8 1/6] interpret-trailers: factor trailer rewriting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-06T21:04:46Z","receivedAt":"2026-03-06T21:04:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> From: Li Chen \n>\n> Extract the trailer rewriting logic into a helper that appends to an\n> output strbuf.\n>\n> Update interpret_trailers() to handle file I/O only: read input once,\n> call the helper, and write the buffered result.\n>\n> This separation makes it easier to move the helper into trailer.c in the\n> next commit.\n>\n> Signed-off-by: Phillip Wood \n> Signed-off-by: Li Chen \n> ---\n\nIf \"Li wrote it and signed it off, Phillip is relaying\" is what is\nhappening, the sign-offs are in reverse order. If \"Li incorporated\nwhat Phillip wrote earlier in his larger work\", then the sign-offs\nmay be fine, but it would be necessary to hint that that is what\nhappened in the proposed log message. I cannot quite tell which is\nthe case.\n\n> builtin/interpret-trailers.c | 57 ++++++++++++++++++++----------------\n> 1 file changed, 32 insertions(+), 25 deletions(-)\n>\n> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\n> index 41b0750e5af..69f9d67ec0e 100644\n> --- a/builtin/interpret-trailers.c\n> +++ b/builtin/interpret-trailers.c\n> @@ -136,32 +136,21 @@ static void read_input_file(struct strbuf *sb, const char *file)\n> \tstrbuf_complete_line(sb);\n> }\n> \n> -static void interpret_trailers(const struct process_trailer_options *opts,\n> -\t\t\t struct list_head *new_trailer_head,\n> -\t\t\t const char *file)\n> +static void process_trailers(const struct process_trailer_options *opts,\n> +\t\t\t struct list_head *new_trailer_head,\n> +\t\t\t struct strbuf *input, struct strbuf *out)\n> {\n> \tLIST_HEAD(head);\n> -\tstruct strbuf sb = STRBUF_INIT;\n> -\tstruct strbuf trailer_block_sb = STRBUF_INIT;\n> \tstruct trailer_block *trailer_block;\n> -\tFILE *outfile = stdout;\n> -\n> -\ttrailer_config_init();\n> -\n> -\tread_input_file(&sb, file);\n> -\n> -\tif (opts->in_place)\n> -\t\toutfile = create_in_place_tempfile(file);\n> -\n> -\ttrailer_block = parse_trailers(opts, sb.buf, &head);\n> +\n> +\ttrailer_block = parse_trailers(opts, input->buf, &head);\n> \n> \t/* Print the lines before the trailer block */\n> \tif (!opts->only_trailers)\n> -\t\tfwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);\n> +\t\tstrbuf_add(out, input->buf, trailer_block_start(trailer_block));\n> \n> \tif (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))\n> -\t\tfprintf(outfile, \"\\n\");\n> -\n> +\t\tstrbuf_addch(out, '\\n');\n> \n> \tif (!opts->only_input) {\n> \t\tLIST_HEAD(config_head);\n> @@ -173,22 +162,40 @@ static void interpret_trailers(const struct process_trailer_options *opts,\n> \t}\n> \n> \t/* Print trailer block. */\n> -\tformat_trailers(opts, &head, &trailer_block_sb);\n> +\tformat_trailers(opts, &head, out);\n> \tfree_trailers(&head);\n> -\tfwrite(trailer_block_sb.buf, 1, trailer_block_sb.len, outfile);\n> -\tstrbuf_release(&trailer_block_sb);\n> \n> \t/* Print the lines after the trailer block as is. */\n> \tif (!opts->only_trailers)\n> -\t\tfwrite(sb.buf + trailer_block_end(trailer_block), 1,\n> -\t\t sb.len - trailer_block_end(trailer_block), outfile);\n> +\t\tstrbuf_add(out, input->buf + trailer_block_end(trailer_block),\n> +\t\t\t input->len - trailer_block_end(trailer_block));\n> \ttrailer_block_release(trailer_block);\n> -\n> +}\n> +\n> +static void interpret_trailers(const struct process_trailer_options *opts,\n> +\t\t\t struct list_head *new_trailer_head,\n> +\t\t\t const char *file)\n> +{\n> +\tstruct strbuf input = STRBUF_INIT;\n> +\tstruct strbuf out = STRBUF_INIT;\n> +\tFILE *outfile = stdout;\n> +\n> +\ttrailer_config_init();\n> +\n> +\tread_input_file(&input, file);\n> +\n> +\tif (opts->in_place)\n> +\t\toutfile = create_in_place_tempfile(file);\n> +\n> +\tprocess_trailers(opts, new_trailer_head, &input, &out);\n> +\n> +\tstrbuf_write(&out, outfile);\n> \tif (opts->in_place)\n> \t\tif (rename_tempfile(&trailers_tempfile, file))\n> \t\t\tdie_errno(_(\"could not rename temporary file to %s\"), file);\n> \n> -\tstrbuf_release(&sb);\n> +\tstrbuf_release(&input);\n> +\tstrbuf_release(&out);\n> }\n> \n> int cmd_interpret_trailers(int argc,\n"},{"id":"538113","messageId":"xmqqjyvozn13.fsf@gitster.g","threadId":"65068","inReplyTo":"5a4d03ab375bbba84436796ca6871204f47521eb.1772808594.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH v8 2/6] interpret-trailers: refactor create_in_place_tempfile()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-06T21:05:44Z","receivedAt":"2026-03-06T21:05:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> From: Phillip Wood \n>\n> Refactor create_in_place_tempfile() in preparation for moving it\n> to tralier.c. Change the return type to return a `struct tempfile*`\n> instead of a `FILE*` so that we can remove the file scope tempfile\n> variable. Since 076aa2cbda5 (tempfile: auto-allocate tempfiles on\n> heap, 2017-09-05) it has not been necessary to make tempfile varibales\n> static so this is safe. Also use error() and return NULL in place of\n> die() so the caller can exit gracefully and use find_last_dir_sep()\n> rather than strchr() to find the parent directory.\n>\n> Signed-off-by: Phillip Wood \n> ---\n> builtin/interpret-trailers.c | 51 ++++++++++++++++++++----------------\n> 1 file changed, 29 insertions(+), 22 deletions(-)\n\nYes, this organization is much nicer. Thanks for cleaning it up.\n\n>\n> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\n> index 69f9d67ec0e..033c2e46713 100644\n> --- a/builtin/interpret-trailers.c\n> +++ b/builtin/interpret-trailers.c\n> @@ -93,35 +93,37 @@ static int parse_opt_parse(const struct option *opt, const char *arg,\n> \treturn 0;\n> }\n> \n> -static struct tempfile *trailers_tempfile;\n> \n> -static FILE *create_in_place_tempfile(const char *file)\n> +static struct tempfile *create_in_place_tempfile(const char *file)\n> {\n> +\tstruct tempfile *tempfile = NULL;\n> \tstruct stat st;\n> \tstruct strbuf filename_template = STRBUF_INIT;\n> \tconst char *tail;\n> -\tFILE *outfile;\n> -\n> -\tif (stat(file, &st))\n> -\t\tdie_errno(_(\"could not stat %s\"), file);\n> -\tif (!S_ISREG(st.st_mode))\n> -\t\tdie(_(\"file %s is not a regular file\"), file);\n> -\tif (!(st.st_mode & S_IWUSR))\n> -\t\tdie(_(\"file %s is not writable by user\"), file);\n> \n> +\tif (stat(file, &st)) {\n> +\t\terror_errno(_(\"could not stat %s\"), file);\n> +\t\treturn NULL;\n> +\t}\n> +\tif (!S_ISREG(st.st_mode)) {\n> +\t\terror(_(\"file %s is not a regular file\"), file);\n> +\t\treturn NULL;\n> +\t}\n> +\tif (!(st.st_mode & S_IWUSR)) {\n> +\t\terror(_(\"file %s is not writable by user\"), file);\n> +\t\treturn NULL;\n> +\t}\n> \t/* Create temporary file in the same directory as the original */\n> -\ttail = strrchr(file, '/');\n> +\ttail = find_last_dir_sep(file);\n> \tif (tail)\n> \t\tstrbuf_add(&filename_template, file, tail - file + 1);\n> \tstrbuf_addstr(&filename_template, \"git-interpret-trailers-XXXXXX\");\n> \n> -\ttrailers_tempfile = xmks_tempfile_m(filename_template.buf, st.st_mode);\n> +\ttempfile = mks_tempfile_m(filename_template.buf, st.st_mode);\n> +\n> \tstrbuf_release(&filename_template);\n> -\toutfile = fdopen_tempfile(trailers_tempfile, \"w\");\n> -\tif (!outfile)\n> -\t\tdie_errno(_(\"could not open temporary file\"));\n> \n> -\treturn outfile;\n> +\treturn tempfile;\n> }\n> \n> static void read_input_file(struct strbuf *sb, const char *file)\n> @@ -178,20 +180,25 @@ static void interpret_trailers(const struct process_trailer_options *opts,\n> {\n> \tstruct strbuf input = STRBUF_INIT;\n> \tstruct strbuf out = STRBUF_INIT;\n> -\tFILE *outfile = stdout;\n> +\tstruct tempfile *tempfile = NULL;\n> +\tint fd = 1;\n> \n> \ttrailer_config_init();\n> \n> \tread_input_file(&input, file);\n> \n> -\tif (opts->in_place)\n> -\t\toutfile = create_in_place_tempfile(file);\n> -\n> +\tif (opts->in_place) {\n> +\t\ttempfile = create_in_place_tempfile(file);\n> +\t\tif (!tempfile)\n> +\t\t\tdie(NULL);\n> +\t\tfd = tempfile->fd;\n> +\t}\n> \tprocess_trailers(opts, new_trailer_head, &input, &out);\n> \n> -\tstrbuf_write(&out, outfile);\n> +\tif (write_in_full(fd, out.buf, out.len) < 0)\n> +\t\tdie_errno(_(\"could not write to temporary file '%s'\"), file);\n> \tif (opts->in_place)\n> -\t\tif (rename_tempfile(&trailers_tempfile, file))\n> +\t\tif (rename_tempfile(&tempfile, file))\n> \t\t\tdie_errno(_(\"could not rename temporary file to %s\"), file);\n> \n> \tstrbuf_release(&input);\n"},{"id":"538259","messageId":"de777714-49f1-41d4-a645-b173ca0e2b31@gmail.com","threadId":"65068","inReplyTo":"xmqqo6l0zn2p.fsf@gitster.g","subject":"Re: [PATCH v8 1/6] interpret-trailers: factor trailer rewriting","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-09T10:36:53Z","receivedAt":"2026-03-09T10:36:57Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 06/03/2026 21:04, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> From: Li Chen \n>>\n>> Extract the trailer rewriting logic into a helper that appends to an\n>> output strbuf.\n>>\n>> Update interpret_trailers() to handle file I/O only: read input once,\n>> call the helper, and write the buffered result.\n>>\n>> This separation makes it easier to move the helper into trailer.c in the\n>> next commit.\n>>\n>> Signed-off-by: Phillip Wood \n>> Signed-off-by: Li Chen \n>> ---\n> \n> If \"Li wrote it and signed it off, Phillip is relaying\" is what is\n> happening, the sign-offs are in reverse order. If \"Li incorporated\n> what Phillip wrote earlier in his larger work\", then the sign-offs\n> may be fine, but it would be necessary to hint that that is what\n> happened in the proposed log message. I cannot quite tell which is\n> the case.\n\nIt's the latter, I posted a diff[1] and Li based this patch on that diff\n\nThanks\n\nPhillip\n\n[1] \nhttps://lore.kernel.org/git/7d12b046-365f-441c-af8e-8a39d61efbbd@gmail.com/\n>> builtin/interpret-trailers.c | 57 ++++++++++++++++++++----------------\n>> 1 file changed, 32 insertions(+), 25 deletions(-)\n>>\n>> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\n>> index 41b0750e5af..69f9d67ec0e 100644\n>> --- a/builtin/interpret-trailers.c\n>> +++ b/builtin/interpret-trailers.c\n>> @@ -136,32 +136,21 @@ static void read_input_file(struct strbuf *sb, const char *file)\n>> \tstrbuf_complete_line(sb);\n>> }\n>> \n>> -static void interpret_trailers(const struct process_trailer_options *opts,\n>> -\t\t\t struct list_head *new_trailer_head,\n>> -\t\t\t const char *file)\n>> +static void process_trailers(const struct process_trailer_options *opts,\n>> +\t\t\t struct list_head *new_trailer_head,\n>> +\t\t\t struct strbuf *input, struct strbuf *out)\n>> {\n>> \tLIST_HEAD(head);\n>> -\tstruct strbuf sb = STRBUF_INIT;\n>> -\tstruct strbuf trailer_block_sb = STRBUF_INIT;\n>> \tstruct trailer_block *trailer_block;\n>> -\tFILE *outfile = stdout;\n>> -\n>> -\ttrailer_config_init();\n>> -\n>> -\tread_input_file(&sb, file);\n>> -\n>> -\tif (opts->in_place)\n>> -\t\toutfile = create_in_place_tempfile(file);\n>> -\n>> -\ttrailer_block = parse_trailers(opts, sb.buf, &head);\n>> +\n>> +\ttrailer_block = parse_trailers(opts, input->buf, &head);\n>> \n>> \t/* Print the lines before the trailer block */\n>> \tif (!opts->only_trailers)\n>> -\t\tfwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);\n>> +\t\tstrbuf_add(out, input->buf, trailer_block_start(trailer_block));\n>> \n>> \tif (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))\n>> -\t\tfprintf(outfile, \"\\n\");\n>> -\n>> +\t\tstrbuf_addch(out, '\\n');\n>> \n>> \tif (!opts->only_input) {\n>> \t\tLIST_HEAD(config_head);\n>> @@ -173,22 +162,40 @@ static void interpret_trailers(const struct process_trailer_options *opts,\n>> \t}\n>> \n>> \t/* Print trailer block. */\n>> -\tformat_trailers(opts, &head, &trailer_block_sb);\n>> +\tformat_trailers(opts, &head, out);\n>> \tfree_trailers(&head);\n>> -\tfwrite(trailer_block_sb.buf, 1, trailer_block_sb.len, outfile);\n>> -\tstrbuf_release(&trailer_block_sb);\n>> \n>> \t/* Print the lines after the trailer block as is. */\n>> \tif (!opts->only_trailers)\n>> -\t\tfwrite(sb.buf + trailer_block_end(trailer_block), 1,\n>> -\t\t sb.len - trailer_block_end(trailer_block), outfile);\n>> +\t\tstrbuf_add(out, input->buf + trailer_block_end(trailer_block),\n>> +\t\t\t input->len - trailer_block_end(trailer_block));\n>> \ttrailer_block_release(trailer_block);\n>> -\n>> +}\n>> +\n>> +static void interpret_trailers(const struct process_trailer_options *opts,\n>> +\t\t\t struct list_head *new_trailer_head,\n>> +\t\t\t const char *file)\n>> +{\n>> +\tstruct strbuf input = STRBUF_INIT;\n>> +\tstruct strbuf out = STRBUF_INIT;\n>> +\tFILE *outfile = stdout;\n>> +\n>> +\ttrailer_config_init();\n>> +\n>> +\tread_input_file(&input, file);\n>> +\n>> +\tif (opts->in_place)\n>> +\t\toutfile = create_in_place_tempfile(file);\n>> +\n>> +\tprocess_trailers(opts, new_trailer_head, &input, &out);\n>> +\n>> +\tstrbuf_write(&out, outfile);\n>> \tif (opts->in_place)\n>> \t\tif (rename_tempfile(&trailers_tempfile, file))\n>> \t\t\tdie_errno(_(\"could not rename temporary file to %s\"), file);\n>> \n>> -\tstrbuf_release(&sb);\n>> +\tstrbuf_release(&input);\n>> +\tstrbuf_release(&out);\n>> }\n>> \n>> int cmd_interpret_trailers(int argc,\n\n"}]}