[PATCH v7 0/5] rebase: support --trailer

[PATCH v7 0/5] rebase: support --trailer

[Li Chen]

Apologies for the long delay in sending v7.

v7 is based on origin/master at v2.53.0-154-g7c02d39fc2.

This series routes trailer insertion through an in-process path, removing the
fork/exec to builtin/interpret-trailers.

The first four commits refactor trailer rewriting in builtin/interpret-trailers
and trailer.c so callers can reuse a single in-process helper (used by git
interpret-trailers, git commit and git tag). The final commit adds git rebase
--trailer, currently supported with the merge backend only (rejecting apply-only
scenarios and validating input early).

v7:
Rebased onto origin/master at v2.53.0-154-g7c02d39fc2.
Split out a new patch to parse --trailer with OPT_STRVEC in git commit and git
tag.
Use strbuf_write() in interpret-trailers when emitting buffered output.
Restore --in-place rewriting semantics via tempfile+rename.
Drop wrapper.c/h and validate trailer args via validate_trailer_args().
Drop redundant rebase basic-state save/restore for --trailer arguments.

v6: squash all fix commits and split refactor step from the original patch
based on Phillip's suggestion and code [4].
v5: fix all Kristoffer's review comments from v4[3] in place and without new
patches.
v4: fix all reviewer comments in v3. [2], and add patch 1~8 & 10~29 to fix
review comments.
v3: merges the remaining trailer paths into one in-process helper, dropping the
duplicate code, as pointed by Junio and Phillip [1]
v2: fix issues pointed by Phillip
RFC link:
https://lore.kernel.org/git/196a5ac1393.f5b4db7d187309.2451613571977217927@linux.beauty/

Comments very very welcome!

[1]: https://lore.kernel.org/git/xmqq8qlzkukw.fsf@gitster.g/
[2]: https://lore.kernel.org/git/20250803150059.402017-1-me@linux.beauty/
[3]: https://lore.kernel.org/git/20251014122452.1851103-1-me@linux.beauty/
[4]: https://lore.kernel.org/git/7d12b046-365f-441c-af8e-8a39d61efbbd@gmail.com/

Li Chen (5):
  interpret-trailers: factor trailer rewriting
  trailer: move process_trailers to trailer.h
  trailer: append trailers without fork/exec
  commit, tag: parse --trailer with OPT_STRVEC
  rebase: support --trailer

 Documentation/git-rebase.adoc |   7 ++
 builtin/commit.c              |   3 +-
 builtin/interpret-trailers.c  |  47 ++-------
 builtin/rebase.c              |  18 ++++
 builtin/tag.c                 |   4 +-
 sequencer.c                   |  28 ++++++
 sequencer.h                   |   3 +
 t/meson.build                 |   1 +
 t/t3440-rebase-trailer.sh     | 147 ++++++++++++++++++++++++++++
 trailer.c                     | 179 ++++++++++++++++++++++++++++++++--
 trailer.h                     |  30 +++++-
 11 files changed, 414 insertions(+), 53 deletions(-)
 create mode 100755 t/t3440-rebase-trailer.sh

-- 
2.52.0

[PATCH v7 1/5] interpret-trailers: factor trailer rewriting

[Li Chen]

Extract the trailer rewriting logic into a helper that appends to an
output strbuf.

Update interpret_trailers() to handle file I/O only: read input once,
call the helper, and write the buffered result.

This separation makes it easier to move the helper into trailer.c in the
next commit.

Signed-off-by: Li Chen <me@linux.beauty>
---
v7:
Use strbuf_write() when emitting buffered output.

 builtin/interpret-trailers.c | 53 ++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 23 deletions(-)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 41b0750e5a..69f9d67ec0 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -136,32 +136,21 @@ static void read_input_file(struct strbuf *sb, const char *file)
 	strbuf_complete_line(sb);
 }
 
-static void interpret_trailers(const struct process_trailer_options *opts,
-			       struct list_head *new_trailer_head,
-			       const char *file)
+static void process_trailers(const struct process_trailer_options *opts,
+			     struct list_head *new_trailer_head,
+			     struct strbuf *input, struct strbuf *out)
 {
 	LIST_HEAD(head);
-	struct strbuf sb = STRBUF_INIT;
-	struct strbuf trailer_block_sb = STRBUF_INIT;
 	struct trailer_block *trailer_block;
-	FILE *outfile = stdout;
 
-	trailer_config_init();
-
-	read_input_file(&sb, file);
-
-	if (opts->in_place)
-		outfile = create_in_place_tempfile(file);
-
-	trailer_block = parse_trailers(opts, sb.buf, &head);
+	trailer_block = parse_trailers(opts, input->buf, &head);
 
 	/* Print the lines before the trailer block */
 	if (!opts->only_trailers)
-		fwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);
+		strbuf_add(out, input->buf, trailer_block_start(trailer_block));
 
 	if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
-		fprintf(outfile, "\n");
-
+		strbuf_addch(out, '\n');
 
 	if (!opts->only_input) {
 		LIST_HEAD(config_head);
@@ -173,22 +162,40 @@ static void interpret_trailers(const struct process_trailer_options *opts,
 	}
 
 	/* Print trailer block. */
-	format_trailers(opts, &head, &trailer_block_sb);
+	format_trailers(opts, &head, out);
 	free_trailers(&head);
-	fwrite(trailer_block_sb.buf, 1, trailer_block_sb.len, outfile);
-	strbuf_release(&trailer_block_sb);
 
 	/* Print the lines after the trailer block as is. */
 	if (!opts->only_trailers)
-		fwrite(sb.buf + trailer_block_end(trailer_block), 1,
-		       sb.len - trailer_block_end(trailer_block), outfile);
+		strbuf_add(out, input->buf + trailer_block_end(trailer_block),
+			   input->len - trailer_block_end(trailer_block));
 	trailer_block_release(trailer_block);
+}
+
+static void interpret_trailers(const struct process_trailer_options *opts,
+			       struct list_head *new_trailer_head,
+			       const char *file)
+{
+	struct strbuf input = STRBUF_INIT;
+	struct strbuf out = STRBUF_INIT;
+	FILE *outfile = stdout;
+
+	trailer_config_init();
+
+	read_input_file(&input, file);
+
+	if (opts->in_place)
+		outfile = create_in_place_tempfile(file);
+
+	process_trailers(opts, new_trailer_head, &input, &out);
 
+	strbuf_write(&out, outfile);
 	if (opts->in_place)
 		if (rename_tempfile(&trailers_tempfile, file))
 			die_errno(_("could not rename temporary file to %s"), file);
 
-	strbuf_release(&sb);
+	strbuf_release(&input);
+	strbuf_release(&out);
 }
 
 int cmd_interpret_trailers(int argc,
-- 
2.52.0

[PATCH v7 2/5] trailer: move process_trailers to trailer.h

[Li Chen]

Move process_trailers() from builtin/interpret-trailers.c into trailer.c
and expose it via trailer.h.

This lets other call sites reuse the same trailer rewriting logic.

Signed-off-by: Li Chen <me@linux.beauty>
---
v7:
Rename the input parameter from sb to input.

 builtin/interpret-trailers.c | 36 ------------------------------------
 trailer.c                    | 36 ++++++++++++++++++++++++++++++++++++
 trailer.h                    |  3 +++
 3 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 69f9d67ec0..1354109e0f 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -136,42 +136,6 @@ static void read_input_file(struct strbuf *sb, const char *file)
 	strbuf_complete_line(sb);
 }
 
-static void process_trailers(const struct process_trailer_options *opts,
-			     struct list_head *new_trailer_head,
-			     struct strbuf *input, struct strbuf *out)
-{
-	LIST_HEAD(head);
-	struct trailer_block *trailer_block;
-
-	trailer_block = parse_trailers(opts, input->buf, &head);
-
-	/* Print the lines before the trailer block */
-	if (!opts->only_trailers)
-		strbuf_add(out, input->buf, trailer_block_start(trailer_block));
-
-	if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
-		strbuf_addch(out, '\n');
-
-	if (!opts->only_input) {
-		LIST_HEAD(config_head);
-		LIST_HEAD(arg_head);
-		parse_trailers_from_config(&config_head);
-		parse_trailers_from_command_line_args(&arg_head, new_trailer_head);
-		list_splice(&config_head, &arg_head);
-		process_trailers_lists(&head, &arg_head);
-	}
-
-	/* Print trailer block. */
-	format_trailers(opts, &head, out);
-	free_trailers(&head);
-
-	/* Print the lines after the trailer block as is. */
-	if (!opts->only_trailers)
-		strbuf_add(out, input->buf + trailer_block_end(trailer_block),
-			   input->len - trailer_block_end(trailer_block));
-	trailer_block_release(trailer_block);
-}
-
 static void interpret_trailers(const struct process_trailer_options *opts,
 			       struct list_head *new_trailer_head,
 			       const char *file)
diff --git a/trailer.c b/trailer.c
index 911a81ed99..0c9200506d 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1235,3 +1235,39 @@ int amend_file_with_trailers(const char *path, const struct strvec *trailer_args
 	strvec_pushv(&run_trailer.args, trailer_args->v);
 	return run_command(&run_trailer);
 }
+
+void process_trailers(const struct process_trailer_options *opts,
+		      struct list_head *new_trailer_head,
+		      struct strbuf *input, struct strbuf *out)
+{
+	LIST_HEAD(head);
+	struct trailer_block *trailer_block;
+
+	trailer_block = parse_trailers(opts, input->buf, &head);
+
+	/* Print the lines before the trailer block */
+	if (!opts->only_trailers)
+		strbuf_add(out, input->buf, trailer_block_start(trailer_block));
+
+	if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
+		strbuf_addch(out, '\n');
+
+	if (!opts->only_input) {
+		LIST_HEAD(config_head);
+		LIST_HEAD(arg_head);
+		parse_trailers_from_config(&config_head);
+		parse_trailers_from_command_line_args(&arg_head, new_trailer_head);
+		list_splice(&config_head, &arg_head);
+		process_trailers_lists(&head, &arg_head);
+	}
+
+	/* Print trailer block. */
+	format_trailers(opts, &head, out);
+	free_trailers(&head);
+
+	/* Print the lines after the trailer block as is. */
+	if (!opts->only_trailers)
+		strbuf_add(out, input->buf + trailer_block_end(trailer_block),
+			   input->len - trailer_block_end(trailer_block));
+	trailer_block_release(trailer_block);
+}
diff --git a/trailer.h b/trailer.h
index 4740549586..531fa1a13f 100644
--- a/trailer.h
+++ b/trailer.h
@@ -202,4 +202,7 @@ void trailer_iterator_release(struct trailer_iterator *iter);
  */
 int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
 
+void process_trailers(const struct process_trailer_options *opts,
+		      struct list_head *new_trailer_head,
+		      struct strbuf *input, struct strbuf *out);
 #endif /* TRAILER_H */
-- 
2.52.0

[PATCH v7 3/5] trailer: append trailers without fork/exec

[Li Chen]

Introduce amend_strbuf_with_trailers() to apply trailer additions to a
message buffer via process_trailers(), avoiding the need to run git
interpret-trailers as a child process.

Update amend_file_with_trailers() to use the in-process helper and
rewrite the target file via tempfile+rename, preserving the previous
in-place semantics.

Keep existing callers unchanged by continuing to accept argv-style
--trailer=<trailer> entries and stripping the prefix before feeding the
in-process implementation.

Signed-off-by: Li Chen <me@linux.beauty>
---
v7:
Drop wrapper.c/h and validate trailer args via validate_trailer_args().
Rewrite the target file via tempfile+rename to preserve --in-place semantics.

 builtin/interpret-trailers.c |   4 +-
 trailer.c                    | 162 +++++++++++++++++++++++++++++++++--
 trailer.h                    |  27 +++++-
 3 files changed, 179 insertions(+), 14 deletions(-)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 1354109e0f..d4aff68746 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -144,8 +144,6 @@ static void interpret_trailers(const struct process_trailer_options *opts,
 	struct strbuf out = STRBUF_INIT;
 	FILE *outfile = stdout;
 
-	trailer_config_init();
-
 	read_input_file(&input, file);
 
 	if (opts->in_place)
@@ -203,6 +201,8 @@ int cmd_interpret_trailers(int argc,
 			git_interpret_trailers_usage,
 			options);
 
+	trailer_config_init();
+
 	if (argc) {
 		int i;
 		for (i = 0; i < argc; i++)
diff --git a/trailer.c b/trailer.c
index 0c9200506d..8e87d185d9 100644
--- a/trailer.c
+++ b/trailer.c
@@ -7,8 +7,11 @@
 #include "string-list.h"
 #include "run-command.h"
 #include "commit.h"
+#include "strvec.h"
+#include "tempfile.h"
 #include "trailer.h"
 #include "list.h"
+
 /*
  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
  */
@@ -772,6 +775,30 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,
 	free(cl_separators);
 }
 
+void validate_trailer_args(const struct strvec *cli_args)
+{
+	char *cl_separators;
+
+	trailer_config_init();
+
+	cl_separators = xstrfmt("=%s", separators);
+
+	for (size_t i = 0; i < cli_args->nr; i++) {
+		const char *txt = cli_args->v[i];
+		ssize_t separator_pos;
+
+		if (!*txt)
+			die(_("empty --trailer argument"));
+
+		separator_pos = find_separator(txt, cl_separators);
+		if (separator_pos == 0)
+			die(_("invalid trailer '%s': missing key before separator"),
+			    txt);
+	}
+
+	free(cl_separators);
+}
+
 static const char *next_line(const char *str)
 {
 	const char *nl = strchrnul(str, '\n');
@@ -1224,16 +1251,133 @@ void trailer_iterator_release(struct trailer_iterator *iter)
 	strbuf_release(&iter->key);
 }
 
-int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)
+static void new_trailer_items_clear(struct list_head *items)
+{
+	while (!list_empty(items)) {
+		struct new_trailer_item *item =
+			list_first_entry(items, struct new_trailer_item, list);
+		list_del(&item->list);
+		free(item);
+	}
+}
+
+void amend_strbuf_with_trailers(struct strbuf *buf,
+				const struct strvec *trailer_args)
+{
+	struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
+	LIST_HEAD(new_trailer_head);
+	struct strbuf out = STRBUF_INIT;
+	size_t i;
+
+	opts.no_divider = 1;
+
+	for (i = 0; i < trailer_args->nr; i++) {
+		const char *text = trailer_args->v[i];
+		struct new_trailer_item *item;
+
+		if (!*text)
+			die(_("empty --trailer argument"));
+		item = xcalloc(1, sizeof(*item));
+		item->text = text;
+		list_add_tail(&item->list, &new_trailer_head);
+	}
+
+	trailer_config_init();
+	process_trailers(&opts, &new_trailer_head, buf, &out);
+
+	strbuf_swap(buf, &out);
+	strbuf_release(&out);
+
+	new_trailer_items_clear(&new_trailer_head);
+}
+
+static int write_file_in_place(const char *path, const struct strbuf *buf)
+{
+	struct stat st;
+	struct strbuf filename_template = STRBUF_INIT;
+	const char *tail;
+	struct tempfile *tempfile;
+	FILE *outfile;
+
+	if (stat(path, &st))
+		return error_errno(_("could not stat %s"), path);
+	if (!S_ISREG(st.st_mode))
+		return error(_("file %s is not a regular file"), path);
+	if (!(st.st_mode & S_IWUSR))
+		return error(_("file %s is not writable by user"), path);
+
+	/* Create temporary file in the same directory as the original */
+	tail = strrchr(path, '/');
+	if (tail)
+		strbuf_add(&filename_template, path, tail - path + 1);
+	strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
+
+	tempfile = mks_tempfile_sm(filename_template.buf, 0, st.st_mode);
+	strbuf_release(&filename_template);
+	if (!tempfile)
+		return error_errno(_("could not create temporary file"));
+
+	outfile = fdopen_tempfile(tempfile, "w");
+	if (!outfile) {
+		int saved_errno = errno;
+		delete_tempfile(&tempfile);
+		errno = saved_errno;
+		return error_errno(_("could not open temporary file"));
+	}
+
+	if (buf->len && fwrite(buf->buf, 1, buf->len, outfile) < buf->len) {
+		int saved_errno = errno;
+		delete_tempfile(&tempfile);
+		errno = saved_errno;
+		return error_errno(_("could not write to temporary file"));
+	}
+
+	if (rename_tempfile(&tempfile, path))
+		return error_errno(_("could not rename temporary file to %s"), path);
+
+	return 0;
+}
+
+int amend_file_with_trailers(const char *path,
+			     const struct strvec *trailer_args)
 {
-	struct child_process run_trailer = CHILD_PROCESS_INIT;
-
-	run_trailer.git_cmd = 1;
-	strvec_pushl(&run_trailer.args, "interpret-trailers",
-		     "--in-place", "--no-divider",
-		     path, NULL);
-	strvec_pushv(&run_trailer.args, trailer_args->v);
-	return run_command(&run_trailer);
+	struct strbuf buf = STRBUF_INIT;
+	struct strvec stripped_trailer_args = STRVEC_INIT;
+	int ret = 0;
+	size_t i;
+
+	if (!trailer_args)
+		BUG("amend_file_with_trailers called with NULL trailer_args");
+	if (!trailer_args->nr)
+		return 0;
+
+	for (i = 0; i < trailer_args->nr; i++) {
+		const char *txt = trailer_args->v[i];
+
+		/*
+		 * Historically amend_file_with_trailers() passed its arguments
+		 * to "git interpret-trailers", which expected argv entries in
+		 * "--trailer=<trailer>" form. Continue to accept those for
+		 * existing callers, but pass only the value portion to the
+		 * in-process implementation.
+		 */
+		skip_prefix(txt, "--trailer=", &txt);
+		if (!*txt)
+			die(_("empty --trailer argument"));
+		strvec_push(&stripped_trailer_args, txt);
+	}
+
+	if (strbuf_read_file(&buf, path, 0) < 0)
+		ret = error_errno(_("could not read '%s'"), path);
+	else
+		amend_strbuf_with_trailers(&buf, &stripped_trailer_args);
+
+	if (!ret)
+		ret = write_file_in_place(path, &buf);
+
+	strvec_clear(&stripped_trailer_args);
+	strbuf_release(&buf);
+	return ret;
 }
 
 void process_trailers(const struct process_trailer_options *opts,
diff --git a/trailer.h b/trailer.h
index 531fa1a13f..d05dab050b 100644
--- a/trailer.h
+++ b/trailer.h
@@ -68,6 +68,8 @@ void parse_trailers_from_config(struct list_head *config_head);
 void parse_trailers_from_command_line_args(struct list_head *arg_head,
 					   struct list_head *new_trailer_head);
 
+void validate_trailer_args(const struct strvec *cli_args);
+
 void process_trailers_lists(struct list_head *head,
 			    struct list_head *arg_head);
 
@@ -196,12 +198,31 @@ int trailer_iterator_advance(struct trailer_iterator *iter);
 void trailer_iterator_release(struct trailer_iterator *iter);
 
 /*
- * Augment a file to add trailers to it by running git-interpret-trailers.
- * This calls run_command() and its return value is the same (i.e. 0 for
- * success, various non-zero for other errors). See run-command.h.
+ * Append trailers specified in trailer_args to buf in-place.
+ *
+ * Each element of trailer_args should be in the same format as the value
+ * accepted by --trailer=<trailer> (i.e., without the --trailer= prefix).
+ */
+void amend_strbuf_with_trailers(struct strbuf *buf,
+				const struct strvec *trailer_args);
+
+/*
+ * Augment a file by appending trailers specified in trailer_args.
+ *
+ * Each element of trailer_args should be an argv-style --trailer=<trailer>
+ * option (i.e., including the --trailer= prefix).
+ *
+ * Returns 0 on success or a non-zero error code on failure.
  */
 int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
 
+/*
+ * Rewrite the contents of input by processing its trailer block according to
+ * opts and (optionally) appending trailers from new_trailer_head.
+ *
+ * The rewritten message is appended to out (callers should strbuf_reset()
+ * first if needed).
+ */
 void process_trailers(const struct process_trailer_options *opts,
 		      struct list_head *new_trailer_head,
 		      struct strbuf *input, struct strbuf *out);
-- 
2.52.0

[PATCH v7 4/5] commit, tag: parse --trailer with OPT_STRVEC

[Li Chen]

Now that amend_file_with_trailers() expects raw trailer lines, do not
store argv-style "--trailer=<trailer>" strings in git commit and git
tag.

Parse --trailer using OPT_STRVEC so trailer_args contains only the
trailer value, and drop the temporary prefix stripping in
amend_file_with_trailers().

Signed-off-by: Li Chen <me@linux.beauty>
---
v7:
New patch.

 builtin/commit.c |  3 ++-
 builtin/tag.c    |  4 ++--
 trailer.c        | 21 +--------------------
 trailer.h        |  4 ++--
 4 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 9e3a09d532..d9983230de 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1720,7 +1720,8 @@ int cmd_commit(int argc,
 		OPT_STRING(0, "fixup", &fixup_message, N_("[(amend|reword):]commit"), N_("use autosquash formatted message to fixup or amend/reword specified commit")),
 		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
 		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
-		OPT_PASSTHRU_ARGV(0, "trailer", &trailer_args, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG),
+		OPT_STRVEC(0, "trailer", &trailer_args, N_("trailer"),
+			   N_("add custom trailer(s)")),
 		OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
 		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
 		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
diff --git a/builtin/tag.c b/builtin/tag.c
index aeb04c487f..15aee1b03a 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -499,8 +499,8 @@ int cmd_tag(int argc,
 		OPT_CALLBACK_F('m', "message", &msg, N_("message"),
 			       N_("tag message"), PARSE_OPT_NONEG, parse_msg_arg),
 		OPT_FILENAME('F', "file", &msgfile, N_("read message from file")),
-		OPT_PASSTHRU_ARGV(0, "trailer", &trailer_args, N_("trailer"),
-				  N_("add custom trailer(s)"), PARSE_OPT_NONEG),
+		OPT_STRVEC(0, "trailer", &trailer_args, N_("trailer"),
+			   N_("add custom trailer(s)")),
 		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of tag message")),
 		OPT_BOOL('s', "sign", &opt.sign, N_("annotated and GPG-signed tag")),
 		OPT_CLEANUP(&cleanup_arg),
diff --git a/trailer.c b/trailer.c
index 8e87d185d9..e85c6c9fbe 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1342,40 +1342,21 @@ int amend_file_with_trailers(const char *path,
 			     const struct strvec *trailer_args)
 {
 	struct strbuf buf = STRBUF_INIT;
-	struct strvec stripped_trailer_args = STRVEC_INIT;
 	int ret = 0;
-	size_t i;
 
 	if (!trailer_args)
 		BUG("amend_file_with_trailers called with NULL trailer_args");
 	if (!trailer_args->nr)
 		return 0;
 
-	for (i = 0; i < trailer_args->nr; i++) {
-		const char *txt = trailer_args->v[i];
-
-		/*
-		 * Historically amend_file_with_trailers() passed its arguments
-		 * to "git interpret-trailers", which expected argv entries in
-		 * "--trailer=<trailer>" form. Continue to accept those for
-		 * existing callers, but pass only the value portion to the
-		 * in-process implementation.
-		 */
-		skip_prefix(txt, "--trailer=", &txt);
-		if (!*txt)
-			die(_("empty --trailer argument"));
-		strvec_push(&stripped_trailer_args, txt);
-	}
-
 	if (strbuf_read_file(&buf, path, 0) < 0)
 		ret = error_errno(_("could not read '%s'"), path);
 	else
-		amend_strbuf_with_trailers(&buf, &stripped_trailer_args);
+		amend_strbuf_with_trailers(&buf, trailer_args);
 
 	if (!ret)
 		ret = write_file_in_place(path, &buf);
 
-	strvec_clear(&stripped_trailer_args);
 	strbuf_release(&buf);
 	return ret;
 }
diff --git a/trailer.h b/trailer.h
index d05dab050b..e5bd355aad 100644
--- a/trailer.h
+++ b/trailer.h
@@ -209,8 +209,8 @@ void amend_strbuf_with_trailers(struct strbuf *buf,
 /*
  * Augment a file by appending trailers specified in trailer_args.
  *
- * Each element of trailer_args should be an argv-style --trailer=<trailer>
- * option (i.e., including the --trailer= prefix).
+ * Each element of trailer_args should be in the same format as the value
+ * accepted by --trailer=<trailer> (i.e., without the --trailer= prefix).
  *
  * Returns 0 on success or a non-zero error code on failure.
  */
-- 
2.52.0

[PATCH v7 5/5] rebase: support --trailer

[Li Chen]

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

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

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

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

Signed-off-by: Li Chen <me@linux.beauty>
---
v7:
Validate trailer args via validate_trailer_args().
Drop redundant rebase basic-state save/restore for --trailer arguments.
Fix Documentation/git-rebase.adoc formatting for the new option.

 Documentation/git-rebase.adoc |   7 ++
 builtin/rebase.c              |  18 +++++
 sequencer.c                   |  28 +++++++
 sequencer.h                   |   3 +
 t/meson.build                 |   1 +
 t/t3440-rebase-trailer.sh     | 147 ++++++++++++++++++++++++++++++++++
 6 files changed, 204 insertions(+)
 create mode 100755 t/t3440-rebase-trailer.sh

diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc
index e177808004..908717991a 100644
--- a/Documentation/git-rebase.adoc
+++ b/Documentation/git-rebase.adoc
@@ -497,6 +497,13 @@ See also INCOMPATIBLE OPTIONS below.
 +
 See also INCOMPATIBLE OPTIONS below.
 
+--trailer=<trailer>::
+	Append the given trailer to every rebased commit message, processed
+	via linkgit:git-interpret-trailers[1]. This option implies
+	`--force-rebase` so that fast-forwarded commits are also rewritten.
++
+See also INCOMPATIBLE OPTIONS below.
+
 -i::
 --interactive::
 	Make a list of the commits which are about to be rebased.  Let the
diff --git a/builtin/rebase.c b/builtin/rebase.c
index c487e10907..3200506c89 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -36,6 +36,7 @@
 #include "reset.h"
 #include "trace2.h"
 #include "hook.h"
+#include "trailer.h"
 
 static char const * const builtin_rebase_usage[] = {
 	N_("git rebase [-i] [options] [--exec <cmd>] "
@@ -113,6 +114,7 @@ struct rebase_options {
 	enum action action;
 	char *reflog_action;
 	int signoff;
+	struct strvec trailer_args;
 	int allow_rerere_autoupdate;
 	int keep_empty;
 	int autosquash;
@@ -143,6 +145,7 @@ struct rebase_options {
 		.flags = REBASE_NO_QUIET, 		\
 		.git_am_opts = STRVEC_INIT,		\
 		.exec = STRING_LIST_INIT_NODUP,		\
+		.trailer_args = STRVEC_INIT,		\
 		.git_format_patch_opt = STRBUF_INIT,	\
 		.fork_point = -1,			\
 		.reapply_cherry_picks = -1,             \
@@ -166,6 +169,7 @@ static void rebase_options_release(struct rebase_options *opts)
 	free(opts->strategy);
 	string_list_clear(&opts->strategy_opts, 0);
 	strbuf_release(&opts->git_format_patch_opt);
+	strvec_clear(&opts->trailer_args);
 }
 
 static struct replay_opts get_replay_opts(const struct rebase_options *opts)
@@ -177,6 +181,10 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)
 	sequencer_init_config(&replay);
 
 	replay.signoff = opts->signoff;
+
+	for (size_t i = 0; i < opts->trailer_args.nr; i++)
+		strvec_push(&replay.trailer_args, opts->trailer_args.v[i]);
+
 	replay.allow_ff = !(opts->flags & REBASE_FORCE);
 	if (opts->allow_rerere_autoupdate)
 		replay.allow_rerere_auto = opts->allow_rerere_autoupdate;
@@ -1132,6 +1140,8 @@ int cmd_rebase(int argc,
 			.flags = PARSE_OPT_NOARG,
 			.defval = REBASE_DIFFSTAT,
 		},
+		OPT_STRVEC(0, "trailer", &options.trailer_args, N_("trailer"),
+			   N_("add custom trailer(s)")),
 		OPT_BOOL(0, "signoff", &options.signoff,
 			 N_("add a Signed-off-by trailer to each commit")),
 		OPT_BOOL(0, "committer-date-is-author-date",
@@ -1285,6 +1295,11 @@ int cmd_rebase(int argc,
 			     builtin_rebase_options,
 			     builtin_rebase_usage, 0);
 
+	if (options.trailer_args.nr) {
+		validate_trailer_args(&options.trailer_args);
+		options.flags |= REBASE_FORCE;
+	}
+
 	if (preserve_merges_selected)
 		die(_("--preserve-merges was replaced by --rebase-merges\n"
 			"Note: Your `pull.rebase` configuration may also be set to 'preserve',\n"
@@ -1542,6 +1557,9 @@ int cmd_rebase(int argc,
 	if (options.root && !options.onto_name)
 		imply_merge(&options, "--root without --onto");
 
+	if (options.trailer_args.nr)
+		imply_merge(&options, "--trailer");
+
 	if (isatty(2) && options.flags & REBASE_NO_QUIET)
 		strbuf_addstr(&options.git_format_patch_opt, " --progress");
 
diff --git a/sequencer.c b/sequencer.c
index a3eb39bb25..a60c2a0cde 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -209,6 +209,7 @@ static GIT_PATH_FUNC(rebase_path_reschedule_failed_exec, "rebase-merge/reschedul
 static GIT_PATH_FUNC(rebase_path_no_reschedule_failed_exec, "rebase-merge/no-reschedule-failed-exec")
 static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, "rebase-merge/drop_redundant_commits")
 static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redundant_commits")
+static GIT_PATH_FUNC(rebase_path_trailer, "rebase-merge/trailer")
 
 /*
  * A 'struct replay_ctx' represents the private state of the sequencer.
@@ -420,6 +421,7 @@ void replay_opts_release(struct replay_opts *opts)
 	if (opts->revs)
 		release_revisions(opts->revs);
 	free(opts->revs);
+	strvec_clear(&opts->trailer_args);
 	replay_ctx_release(ctx);
 	free(opts->ctx);
 }
@@ -2025,6 +2027,9 @@ static int append_squash_message(struct strbuf *buf, const char *body,
 		if (opts->signoff)
 			append_signoff(buf, 0, 0);
 
+		if (opts->trailer_args.nr)
+			amend_strbuf_with_trailers(buf, &opts->trailer_args);
+
 		if ((command == TODO_FIXUP) &&
 		    (flag & TODO_REPLACE_FIXUP_MSG) &&
 		    (file_exists(rebase_path_fixup_msg()) ||
@@ -2443,6 +2448,9 @@ static int do_pick_commit(struct repository *r,
 	if (opts->signoff && !is_fixup(command))
 		append_signoff(&ctx->message, 0, 0);
 
+	if (opts->trailer_args.nr && !is_fixup(command))
+		amend_strbuf_with_trailers(&ctx->message, &opts->trailer_args);
+
 	if (is_rebase_i(opts) && write_author_script(msg.message) < 0)
 		res = -1;
 	else if (!opts->strategy ||
@@ -3234,6 +3242,18 @@ static int read_populate_opts(struct replay_opts *opts)
 
 		read_strategy_opts(opts, &buf);
 		strbuf_reset(&buf);
+		if (strbuf_read_file(&buf, rebase_path_trailer(), 0) >= 0) {
+			char *p = buf.buf, *nl;
+
+			while ((nl = strchr(p, '\n'))) {
+				*nl = '\0';
+				if (!*p)
+					BUG("rebase-merge/trailer has an empty line");
+				strvec_push(&opts->trailer_args, p);
+				p = nl + 1;
+			}
+			strbuf_reset(&buf);
+		}
 
 		if (read_oneliner(&ctx->current_fixups,
 				  rebase_path_current_fixups(),
@@ -3328,6 +3348,14 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
 		write_file(rebase_path_reschedule_failed_exec(), "%s", "");
 	else
 		write_file(rebase_path_no_reschedule_failed_exec(), "%s", "");
+	if (opts->trailer_args.nr) {
+		struct strbuf buf = STRBUF_INIT;
+
+		for (size_t i = 0; i < opts->trailer_args.nr; i++)
+			strbuf_addf(&buf, "%s\n", opts->trailer_args.v[i]);
+		write_file(rebase_path_trailer(), "%s", buf.buf);
+		strbuf_release(&buf);
+	}
 
 	return 0;
 }
diff --git a/sequencer.h b/sequencer.h
index 719684c8a9..bea20da085 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -57,6 +57,8 @@ struct replay_opts {
 	int ignore_date;
 	int commit_use_reference;
 
+	struct strvec trailer_args;
+
 	int mainline;
 
 	char *gpg_sign;
@@ -84,6 +86,7 @@ struct replay_opts {
 #define REPLAY_OPTS_INIT {			\
 	.edit = -1,				\
 	.action = -1,				\
+	.trailer_args = STRVEC_INIT,		\
 	.xopts = STRVEC_INIT,			\
 	.ctx = replay_ctx_new(),		\
 }
diff --git a/t/meson.build b/t/meson.build
index f80e366cff..1f6f8ac20b 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -388,6 +388,7 @@ integration_tests = [
   't3436-rebase-more-options.sh',
   't3437-rebase-fixup-options.sh',
   't3438-rebase-broken-files.sh',
+  't3440-rebase-trailer.sh',
   't3450-history.sh',
   't3451-history-reword.sh',
   't3500-cherry.sh',
diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
new file mode 100755
index 0000000000..8b47579566
--- /dev/null
+++ b/t/t3440-rebase-trailer.sh
@@ -0,0 +1,147 @@
+#!/bin/sh
+#
+
+test_description='git rebase --trailer integration tests
+We verify that --trailer works with the merge backend,
+and that it is rejected early when the apply backend is requested.'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh # test_commit_message, helpers
+
+REVIEWED_BY_TRAILER="Reviewed-by: Dev <dev@example.com>"
+SP=" "
+
+test_expect_success 'setup repo with a small history' '
+	git commit --allow-empty -m "Initial empty commit" &&
+	test_commit first file a &&
+	test_commit second file &&
+	git checkout -b conflict-branch first &&
+	test_commit file-2 file-2 &&
+	test_commit conflict file &&
+	test_commit third file &&
+	git checkout main
+'
+
+test_expect_success 'apply backend is rejected with --trailer' '
+	git checkout -B apply-backend third &&
+	test_expect_code 128 \
+		git rebase --apply --trailer "$REVIEWED_BY_TRAILER" HEAD^ 2>err &&
+	test_grep "fatal: --trailer requires the merge backend" err
+'
+
+test_expect_success 'reject empty --trailer argument' '
+	git checkout -B empty-trailer third &&
+	test_expect_code 128 git rebase --trailer "" HEAD^ 2>err &&
+	test_grep "empty --trailer" err
+'
+
+test_expect_success 'reject trailer with missing key before separator' '
+	git checkout -B missing-key third &&
+	test_expect_code 128 git rebase --trailer ": no-key" HEAD^ 2>err &&
+	test_grep "missing key before separator" err
+'
+
+test_expect_success 'allow trailer with missing value after separator' '
+	git checkout -B missing-value third &&
+	git rebase --trailer "Acked-by:" HEAD^ &&
+	test_commit_message HEAD <<-EOF
+	third
+
+	Acked-by:${SP}
+	EOF
+'
+
+test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last' '
+	git checkout -B replace-policy third &&
+	git -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add \
+		rebase --trailer "Bug: 123" --trailer "Bug: 456" HEAD^ &&
+	test_commit_message HEAD <<-EOF
+	third
+
+	Bug: 456
+	EOF
+'
+
+test_expect_success 'multiple Signed-off-by trailers all preserved' '
+	git checkout -B multiple-signoff third &&
+	git rebase --trailer "Signed-off-by: Dev A <a@example.com>" \
+		--trailer "Signed-off-by: Dev B <b@example.com>" HEAD^ &&
+	test_commit_message HEAD <<-EOF
+	third
+
+	Signed-off-by: Dev A <a@example.com>
+	Signed-off-by: Dev B <b@example.com>
+	EOF
+'
+
+test_expect_success 'rebase --trailer adds trailer after conflicts' '
+	git checkout -B trailer-conflict third &&
+	test_commit fourth file &&
+	test_must_fail git rebase --trailer "$REVIEWED_BY_TRAILER" second &&
+	git checkout --theirs file &&
+	git add file &&
+	git rebase --continue &&
+	test_commit_message HEAD <<-EOF &&
+	fourth
+
+	$REVIEWED_BY_TRAILER
+	EOF
+	test_commit_message HEAD^ <<-EOF
+	third
+
+	$REVIEWED_BY_TRAILER
+	EOF
+'
+
+test_expect_success '--trailer handles fixup commands in todo list' '
+	git checkout -B fixup-trailer third &&
+	test_commit fixup-base base &&
+	test_commit fixup-second second &&
+	cat >todo <<-\EOF &&
+	pick fixup-base fixup-base
+	fixup fixup-second fixup-second
+	EOF
+	(
+		set_replace_editor todo &&
+		git rebase -i --trailer "$REVIEWED_BY_TRAILER" HEAD~2
+	) &&
+	test_commit_message HEAD <<-EOF &&
+	fixup-base
+
+	$REVIEWED_BY_TRAILER
+	EOF
+	git reset --hard fixup-second &&
+	cat >todo <<-\EOF &&
+	pick fixup-base fixup-base
+	fixup -C fixup-second fixup-second
+	EOF
+	(
+		set_replace_editor todo &&
+		git rebase -i --trailer "$REVIEWED_BY_TRAILER" HEAD~2
+	) &&
+	test_commit_message HEAD <<-EOF
+	fixup-second
+
+	$REVIEWED_BY_TRAILER
+	EOF
+'
+
+test_expect_success 'rebase --root honors trailer.<name>.key' '
+	git checkout -B root-trailer first &&
+	git -c trailer.review.key=Reviewed-by rebase --root \
+		--trailer=review="Dev <dev@example.com>" &&
+	test_commit_message HEAD <<-EOF &&
+	first
+
+	Reviewed-by: Dev <dev@example.com>
+	EOF
+	test_commit_message HEAD^ <<-EOF
+	Initial empty commit
+
+	Reviewed-by: Dev <dev@example.com>
+	EOF
+'
+test_done
-- 
2.52.0

Re: [PATCH v7 0/5] rebase: support --trailer

[Junio C Hamano]

Li Chen <me@linux.beauty> writes:

   > Apologies for the long delay in sending v7.
>
> v7 is based on origin/master at v2.53.0-154-g7c02d39fc2.
>
> This series routes trailer insertion through an in-process path, removing the
> fork/exec to builtin/interpret-trailers.
>
> The first four commits refactor trailer rewriting in builtin/interpret-trailers
> and trailer.c so callers can reuse a single in-process helper (used by git
> interpret-trailers, git commit and git tag). The final commit adds git rebase
> --trailer, currently supported with the merge backend only (rejecting apply-only
> scenarios and validating input early).
>
> v7:
> Rebased onto origin/master at v2.53.0-154-g7c02d39fc2.
> Split out a new patch to parse --trailer with OPT_STRVEC in git commit and git
> tag.
> Use strbuf_write() in interpret-trailers when emitting buffered output.
> Restore --in-place rewriting semantics via tempfile+rename.
> Drop wrapper.c/h and validate trailer args via validate_trailer_args().
> Drop redundant rebase basic-state save/restore for --trailer arguments.
> ...
> Comments very very welcome!

Yes indeed.  The discussion thread for v6 saw quite a bit of
activity, but this one is quiet.  Is everybody happy with this
iteration?

Thanks.

Re: [PATCH v7 0/5] rebase: support --trailer

[Phillip Wood]

On 26/02/2026 16:52, Junio C Hamano wrote:
> Li Chen <me@linux.beauty> writes:
> 
>     > Apologies for the long delay in sending v7.
>>
>> v7 is based on origin/master at v2.53.0-154-g7c02d39fc2.
>>
>> This series routes trailer insertion through an in-process path, removing the
>> fork/exec to builtin/interpret-trailers.
>>
>> The first four commits refactor trailer rewriting in builtin/interpret-trailers
>> and trailer.c so callers can reuse a single in-process helper (used by git
>> interpret-trailers, git commit and git tag). The final commit adds git rebase
>> --trailer, currently supported with the merge backend only (rejecting apply-only
>> scenarios and validating input early).
>>
>> v7:
>> Rebased onto origin/master at v2.53.0-154-g7c02d39fc2.
>> Split out a new patch to parse --trailer with OPT_STRVEC in git commit and git
>> tag.
>> Use strbuf_write() in interpret-trailers when emitting buffered output.
>> Restore --in-place rewriting semantics via tempfile+rename.
>> Drop wrapper.c/h and validate trailer args via validate_trailer_args().
>> Drop redundant rebase basic-state save/restore for --trailer arguments.
>> ...
>> Comments very very welcome!
> 
> Yes indeed.  The discussion thread for v6 saw quite a bit of
> activity, but this one is quiet.  Is everybody happy with this
> iteration?

I've not had chance to read this version yet, I'm planning to do so next 
week.

Thanks

Phillip

Re: [PATCH v7 0/5] rebase: support --trailer

[Kristoffer Haugsbakk]

On Tue, Feb 24, 2026, at 08:05, Li Chen wrote:
> Apologies for the long delay in sending v7.
>
> v7 is based on origin/master at v2.53.0-154-g7c02d39fc2.
>
> This series routes trailer insertion through an in-process path, removing the
> fork/exec to builtin/interpret-trailers.
>
>[snip]

This round solves the documentation problem I was complaining about in
the previous round.

Thanks. I will make use of this option. :)

Re: [PATCH v7 1/5] interpret-trailers: factor trailer rewriting

[Phillip Wood]

Hi Li

On 24/02/2026 07:05, Li Chen wrote:
> Extract the trailer rewriting logic into a helper that appends to an
> output strbuf.
> 
> Update interpret_trailers() to handle file I/O only: read input once,
> call the helper, and write the buffered result.
> 
> This separation makes it easier to move the helper into trailer.c in the
> next commit.

This is still missing my sign off c.f. 
https://lore.kernel.org/f5152523-f7ff-4dee-a685-fb0b74cd6a56@gmail.com

> Signed-off-by: Li Chen <me@linux.beauty>
> ---
> v7:
> Use strbuf_write() when emitting buffered output.

Also renamed "sb" to "input"

Apart from the missing sign off this all looks good.

Thanks

Phillip

> 
>   builtin/interpret-trailers.c | 53 ++++++++++++++++++++----------------
>   1 file changed, 30 insertions(+), 23 deletions(-)
> 
> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
> index 41b0750e5a..69f9d67ec0 100644
> --- a/builtin/interpret-trailers.c
> +++ b/builtin/interpret-trailers.c
> @@ -136,32 +136,21 @@ static void read_input_file(struct strbuf *sb, const char *file)
>   	strbuf_complete_line(sb);
>   }
>   
> -static void interpret_trailers(const struct process_trailer_options *opts,
> -			       struct list_head *new_trailer_head,
> -			       const char *file)
> +static void process_trailers(const struct process_trailer_options *opts,
> +			     struct list_head *new_trailer_head,
> +			     struct strbuf *input, struct strbuf *out)
>   {
>   	LIST_HEAD(head);
> -	struct strbuf sb = STRBUF_INIT;
> -	struct strbuf trailer_block_sb = STRBUF_INIT;
>   	struct trailer_block *trailer_block;
> -	FILE *outfile = stdout;
>   
> -	trailer_config_init();
> -
> -	read_input_file(&sb, file);
> -
> -	if (opts->in_place)
> -		outfile = create_in_place_tempfile(file);
> -
> -	trailer_block = parse_trailers(opts, sb.buf, &head);
> +	trailer_block = parse_trailers(opts, input->buf, &head);
>   
>   	/* Print the lines before the trailer block */
>   	if (!opts->only_trailers)
> -		fwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);
> +		strbuf_add(out, input->buf, trailer_block_start(trailer_block));
>   
>   	if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
> -		fprintf(outfile, "\n");
> -
> +		strbuf_addch(out, '\n');
>   
>   	if (!opts->only_input) {
>   		LIST_HEAD(config_head);
> @@ -173,22 +162,40 @@ static void interpret_trailers(const struct process_trailer_options *opts,
>   	}
>   
>   	/* Print trailer block. */
> -	format_trailers(opts, &head, &trailer_block_sb);
> +	format_trailers(opts, &head, out);
>   	free_trailers(&head);
> -	fwrite(trailer_block_sb.buf, 1, trailer_block_sb.len, outfile);
> -	strbuf_release(&trailer_block_sb);
>   
>   	/* Print the lines after the trailer block as is. */
>   	if (!opts->only_trailers)
> -		fwrite(sb.buf + trailer_block_end(trailer_block), 1,
> -		       sb.len - trailer_block_end(trailer_block), outfile);
> +		strbuf_add(out, input->buf + trailer_block_end(trailer_block),
> +			   input->len - trailer_block_end(trailer_block));
>   	trailer_block_release(trailer_block);
> +}
> +
> +static void interpret_trailers(const struct process_trailer_options *opts,
> +			       struct list_head *new_trailer_head,
> +			       const char *file)
> +{
> +	struct strbuf input = STRBUF_INIT;
> +	struct strbuf out = STRBUF_INIT;
> +	FILE *outfile = stdout;
> +
> +	trailer_config_init();
> +
> +	read_input_file(&input, file);
> +
> +	if (opts->in_place)
> +		outfile = create_in_place_tempfile(file);
> +
> +	process_trailers(opts, new_trailer_head, &input, &out);
>   
> +	strbuf_write(&out, outfile);
>   	if (opts->in_place)
>   		if (rename_tempfile(&trailers_tempfile, file))
>   			die_errno(_("could not rename temporary file to %s"), file);
>   
> -	strbuf_release(&sb);
> +	strbuf_release(&input);
> +	strbuf_release(&out);
>   }
>   
>   int cmd_interpret_trailers(int argc,

Re: [PATCH v7 2/5] trailer: move process_trailers to trailer.h

[phillip.wood123]

On 24/02/2026 07:05, Li Chen wrote:
> Move process_trailers() from builtin/interpret-trailers.c into trailer.c
> and expose it via trailer.h.
> 
> This lets other call sites reuse the same trailer rewriting logic.

This looks good, thanks

Phillip

> Signed-off-by: Li Chen <me@linux.beauty>
> ---
> v7:
> Rename the input parameter from sb to input.
> 
>   builtin/interpret-trailers.c | 36 ------------------------------------
>   trailer.c                    | 36 ++++++++++++++++++++++++++++++++++++
>   trailer.h                    |  3 +++
>   3 files changed, 39 insertions(+), 36 deletions(-)
> 
> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
> index 69f9d67ec0..1354109e0f 100644
> --- a/builtin/interpret-trailers.c
> +++ b/builtin/interpret-trailers.c
> @@ -136,42 +136,6 @@ static void read_input_file(struct strbuf *sb, const char *file)
>   	strbuf_complete_line(sb);
>   }
>   
> -static void process_trailers(const struct process_trailer_options *opts,
> -			     struct list_head *new_trailer_head,
> -			     struct strbuf *input, struct strbuf *out)
> -{
> -	LIST_HEAD(head);
> -	struct trailer_block *trailer_block;
> -
> -	trailer_block = parse_trailers(opts, input->buf, &head);
> -
> -	/* Print the lines before the trailer block */
> -	if (!opts->only_trailers)
> -		strbuf_add(out, input->buf, trailer_block_start(trailer_block));
> -
> -	if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
> -		strbuf_addch(out, '\n');
> -
> -	if (!opts->only_input) {
> -		LIST_HEAD(config_head);
> -		LIST_HEAD(arg_head);
> -		parse_trailers_from_config(&config_head);
> -		parse_trailers_from_command_line_args(&arg_head, new_trailer_head);
> -		list_splice(&config_head, &arg_head);
> -		process_trailers_lists(&head, &arg_head);
> -	}
> -
> -	/* Print trailer block. */
> -	format_trailers(opts, &head, out);
> -	free_trailers(&head);
> -
> -	/* Print the lines after the trailer block as is. */
> -	if (!opts->only_trailers)
> -		strbuf_add(out, input->buf + trailer_block_end(trailer_block),
> -			   input->len - trailer_block_end(trailer_block));
> -	trailer_block_release(trailer_block);
> -}
> -
>   static void interpret_trailers(const struct process_trailer_options *opts,
>   			       struct list_head *new_trailer_head,
>   			       const char *file)
> diff --git a/trailer.c b/trailer.c
> index 911a81ed99..0c9200506d 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -1235,3 +1235,39 @@ int amend_file_with_trailers(const char *path, const struct strvec *trailer_args
>   	strvec_pushv(&run_trailer.args, trailer_args->v);
>   	return run_command(&run_trailer);
>   }
> +
> +void process_trailers(const struct process_trailer_options *opts,
> +		      struct list_head *new_trailer_head,
> +		      struct strbuf *input, struct strbuf *out)
> +{
> +	LIST_HEAD(head);
> +	struct trailer_block *trailer_block;
> +
> +	trailer_block = parse_trailers(opts, input->buf, &head);
> +
> +	/* Print the lines before the trailer block */
> +	if (!opts->only_trailers)
> +		strbuf_add(out, input->buf, trailer_block_start(trailer_block));
> +
> +	if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
> +		strbuf_addch(out, '\n');
> +
> +	if (!opts->only_input) {
> +		LIST_HEAD(config_head);
> +		LIST_HEAD(arg_head);
> +		parse_trailers_from_config(&config_head);
> +		parse_trailers_from_command_line_args(&arg_head, new_trailer_head);
> +		list_splice(&config_head, &arg_head);
> +		process_trailers_lists(&head, &arg_head);
> +	}
> +
> +	/* Print trailer block. */
> +	format_trailers(opts, &head, out);
> +	free_trailers(&head);
> +
> +	/* Print the lines after the trailer block as is. */
> +	if (!opts->only_trailers)
> +		strbuf_add(out, input->buf + trailer_block_end(trailer_block),
> +			   input->len - trailer_block_end(trailer_block));
> +	trailer_block_release(trailer_block);
> +}
> diff --git a/trailer.h b/trailer.h
> index 4740549586..531fa1a13f 100644
> --- a/trailer.h
> +++ b/trailer.h
> @@ -202,4 +202,7 @@ void trailer_iterator_release(struct trailer_iterator *iter);
>    */
>   int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
>   
> +void process_trailers(const struct process_trailer_options *opts,
> +		      struct list_head *new_trailer_head,
> +		      struct strbuf *input, struct strbuf *out);
>   #endif /* TRAILER_H */

Re: [PATCH v7 3/5] trailer: append trailers without fork/exec

[Phillip Wood]

Hi Li

On 24/02/2026 07:05, Li Chen wrote:
> Introduce amend_strbuf_with_trailers() to apply trailer additions to a
> message buffer via process_trailers(), avoiding the need to run git
> interpret-trailers as a child process.
> 
> Update amend_file_with_trailers() to use the in-process helper and
> rewrite the target file via tempfile+rename, preserving the previous
> in-place semantics.
> 
> Keep existing callers unchanged by continuing to accept argv-style
> --trailer=<trailer> entries and stripping the prefix before feeding the
> in-process implementation.

This looks pretty good, there are a few rough edges in the tempfile 
handling and a couple of unrelated changes but the basics are sound.

> Signed-off-by: Li Chen <me@linux.beauty>
> ---
> v7:
> Drop wrapper.c/h and validate trailer args via validate_trailer_args().
> Rewrite the target file via tempfile+rename to preserve --in-place semantics.
> 
>   builtin/interpret-trailers.c |   4 +-
>   trailer.c                    | 162 +++++++++++++++++++++++++++++++++--
>   trailer.h                    |  27 +++++-
>   3 files changed, 179 insertions(+), 14 deletions(-)
> 
> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
> index 1354109e0f..d4aff68746 100644
> --- a/builtin/interpret-trailers.c
> +++ b/builtin/interpret-trailers.c
> @@ -144,8 +144,6 @@ static void interpret_trailers(const struct process_trailer_options *opts,
>   	struct strbuf out = STRBUF_INIT;
>   	FILE *outfile = stdout;
>   
> -	trailer_config_init();
> -
>   	read_input_file(&input, file);
>   
>   	if (opts->in_place)
> @@ -203,6 +201,8 @@ int cmd_interpret_trailers(int argc,
>   			git_interpret_trailers_usage,
>   			options);
>   
> +	trailer_config_init();
> +

The changes above are unrelated to everything else in this patch.

>   	if (argc) {
>   		int i;
>   		for (i = 0; i < argc; i++)
> diff --git a/trailer.c b/trailer.c
> index 0c9200506d..8e87d185d9 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -772,6 +775,30 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,
>   	free(cl_separators);
>   }
>   
> +void validate_trailer_args(const struct strvec *cli_args)
> +{
> +	char *cl_separators;
> +
> +	trailer_config_init();
> +
> +	cl_separators = xstrfmt("=%s", separators);
> +
> +	for (size_t i = 0; i < cli_args->nr; i++) {
> +		const char *txt = cli_args->v[i];
> +		ssize_t separator_pos;
> +
> +		if (!*txt)
> +			die(_("empty --trailer argument"));
> +
> +		separator_pos = find_separator(txt, cl_separators);
> +		if (separator_pos == 0)
> +			die(_("invalid trailer '%s': missing key before separator"),
> +			    txt);
> +	}
> +
> +	free(cl_separators);
> +}

This function is never called, is it left over by a mistake, or should 
it be called by amend_file_with_trailers()?

>   static const char *next_line(const char *str)
>   {
>   	const char *nl = strchrnul(str, '\n');
> @@ -1224,16 +1251,133 @@ void trailer_iterator_release(struct trailer_iterator *iter)
>   	strbuf_release(&iter->key);
>   }
>   
> -int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)
> +static void new_trailer_items_clear(struct list_head *items)
> +{
> +	while (!list_empty(items)) {
> +		struct new_trailer_item *item =
> +			list_first_entry(items, struct new_trailer_item, list);
> +		list_del(&item->list);
> +		free(item);
> +	}
> +}

Isn't this just doing what free_trailers() does c.f. 
https://lore.kernel.org/git/ef12ada7-13ae-4df0-a823-6f428c797223@gmail.com/

> +void amend_strbuf_with_trailers(struct strbuf *buf,
> +				const struct strvec *trailer_args)
> +{
> +	struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
> +	LIST_HEAD(new_trailer_head);
> +	struct strbuf out = STRBUF_INIT;
> +	size_t i;
> +
> +	opts.no_divider = 1;
> +
> +	for (i = 0; i < trailer_args->nr; i++) {
> +		const char *text = trailer_args->v[i];
> +		struct new_trailer_item *item;
> +
> +		if (!*text)
> +			die(_("empty --trailer argument"));

The existing callers of amend_file_with_trailers() are not expecting it 
to die so we should return an error here so they cleanup before exiting.

> +		item = xcalloc(1, sizeof(*item));
> +		item->text = text;
> +		list_add_tail(&item->list, &new_trailer_head);
> +	}
> +
> +	trailer_config_init();
> +	process_trailers(&opts, &new_trailer_head, buf, &out);
> +
> +	strbuf_swap(buf, &out);
> +	strbuf_release(&out);
> +
> +	new_trailer_items_clear(&new_trailer_head);
> +}
> +
> +static int write_file_in_place(const char *path, const struct strbuf *buf)
> +{
> +	struct stat st;
> +	struct strbuf filename_template = STRBUF_INIT;
> +	const char *tail;
> +	struct tempfile *tempfile;
> +	FILE *outfile;
> +
> +	if (stat(path, &st))
> +		return error_errno(_("could not stat %s"), path);
> +	if (!S_ISREG(st.st_mode))
> +		return error(_("file %s is not a regular file"), path);
> +	if (!(st.st_mode & S_IWUSR))
> +		return error(_("file %s is not writable by user"), path);

This is copied from create_in_place_tempfile() replacing die() with 
error() - ok

> +	/* Create temporary file in the same directory as the original */
> +	tail = strrchr(path, '/');

This is also copied from create_in_place_tempfile() but it would be 
better to use find_last_dir_sep()

> +	if (tail)
> +		strbuf_add(&filename_template, path, tail - path + 1);
> +	strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
> +
> +	tempfile = mks_tempfile_sm(filename_template.buf, 0, st.st_mode);

This could be mks_tempfile_m() like create_in_place_tempfile() as we 
don't have a suffix to add.

> +	strbuf_release(&filename_template);
> +	if (!tempfile)
> +		return error_errno(_("could not create temporary file"));
> +
> +	outfile = fdopen_tempfile(tempfile, "w");
> +	if (!outfile) {
> +		int saved_errno = errno;
> +		delete_tempfile(&tempfile);
> +		errno = saved_errno;
> +		return error_errno(_("could not open temporary file"));
> +	}
> +
> +	if (buf->len && fwrite(buf->buf, 1, buf->len, outfile) < buf->len) {
> +		int saved_errno = errno;
> +		delete_tempfile(&tempfile);
> +		errno = saved_errno;
> +		return error_errno(_("could not write to temporary file"));
> +	}

As all we're doing is writing the contents of buf we can use 
write_in_full() here and avoid the call to fdopen_tempfile() above. Also 
note that the documentation in tempfile.h says

     * If the program exits before `rename_tempfile()` or
     * `delete_tempfile()` is called, an `atexit(3)` handler will close
     * and remove the temporary file.

so we don't need to call delete_tempfile() if the write fails.

We could probably share this code with builtin/interpret-trailers.c (it 
no-longer needs to call fdopen_tempfile() either) which would make it 
more obvious that we're basically copying it here.

> +	if (rename_tempfile(&tempfile, path))
> +		return error_errno(_("could not rename temporary file to %s"), path);
> +
> +	return 0;
> +}
> +
> +int amend_file_with_trailers(const char *path,
> +			     const struct strvec *trailer_args)
>   {
> -	struct child_process run_trailer = CHILD_PROCESS_INIT;
> -
> -	run_trailer.git_cmd = 1;
> -	strvec_pushl(&run_trailer.args, "interpret-trailers",
> -		     "--in-place", "--no-divider",
> -		     path, NULL);
> -	strvec_pushv(&run_trailer.args, trailer_args->v);
> -	return run_command(&run_trailer);
> +	struct strbuf buf = STRBUF_INIT;
> +	struct strvec stripped_trailer_args = STRVEC_INIT;
> +	int ret = 0;
> +	size_t i;
> +
> +	if (!trailer_args)
> +		BUG("amend_file_with_trailers called with NULL trailer_args");
> +	if (!trailer_args->nr)
> +		return 0;
> +
> +	for (i = 0; i < trailer_args->nr; i++) {
> +		const char *txt = trailer_args->v[i];
> +
> +		/*
> +		 * Historically amend_file_with_trailers() passed its arguments
> +		 * to "git interpret-trailers", which expected argv entries in
> +		 * "--trailer=<trailer>" form. Continue to accept those for
> +		 * existing callers, but pass only the value portion to the
> +		 * in-process implementation.
> +		 */
> +		skip_prefix(txt, "--trailer=", &txt);
> +		if (!*txt)
> +			die(_("empty --trailer argument"));

It seems a shame to die() here when the rest of the function uses 
error() especially as the existing callers of amend_file_with_trailers() 
are not expecting it to die() and may want to perform some cleanup 
before exiting.

> +		strvec_push(&stripped_trailer_args, txt);
> +	}
> +
> +	if (strbuf_read_file(&buf, path, 0) < 0)
> +		ret = error_errno(_("could not read '%s'"), path);
> +	else
> +		amend_strbuf_with_trailers(&buf, &stripped_trailer_args);
> +
> +	if (!ret)
> +		ret = write_file_in_place(path, &buf);
> +
> +	strvec_clear(&stripped_trailer_args);
> +	strbuf_release(&buf);
> +	return ret;
>   }

> [...]   
> +/*
> + * Rewrite the contents of input by processing its trailer block according to
> + * opts and (optionally) appending trailers from new_trailer_head.
> + *
> + * The rewritten message is appended to out (callers should strbuf_reset()
> + * first if needed).
> + */
This comment should be part of the previous commit that adds 
process_trailers().

>   void process_trailers(const struct process_trailer_options *opts,
>   		      struct list_head *new_trailer_head,
>   		      struct strbuf *input, struct strbuf *out);

Thanks

Phillip

Re: [PATCH v7 4/5] commit, tag: parse --trailer with OPT_STRVEC

[Phillip Wood]

Hi Li

On 24/02/2026 07:05, Li Chen wrote:
> Now that amend_file_with_trailers() expects raw trailer lines, do not
> store argv-style "--trailer=<trailer>" strings in git commit and git
> tag.
> 
> Parse --trailer using OPT_STRVEC so trailer_args contains only the
> trailer value, and drop the temporary prefix stripping in
> amend_file_with_trailers().

This looks good. I'll stop here for today and look at the last patch 
tomorrow

Thanks

Phillip

> Signed-off-by: Li Chen <me@linux.beauty>
> ---
> v7:
> New patch.
> 
>   builtin/commit.c |  3 ++-
>   builtin/tag.c    |  4 ++--
>   trailer.c        | 21 +--------------------
>   trailer.h        |  4 ++--
>   4 files changed, 7 insertions(+), 25 deletions(-)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 9e3a09d532..d9983230de 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1720,7 +1720,8 @@ int cmd_commit(int argc,
>   		OPT_STRING(0, "fixup", &fixup_message, N_("[(amend|reword):]commit"), N_("use autosquash formatted message to fixup or amend/reword specified commit")),
>   		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
>   		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
> -		OPT_PASSTHRU_ARGV(0, "trailer", &trailer_args, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG),
> +		OPT_STRVEC(0, "trailer", &trailer_args, N_("trailer"),
> +			   N_("add custom trailer(s)")),
>   		OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
>   		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
>   		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
> diff --git a/builtin/tag.c b/builtin/tag.c
> index aeb04c487f..15aee1b03a 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -499,8 +499,8 @@ int cmd_tag(int argc,
>   		OPT_CALLBACK_F('m', "message", &msg, N_("message"),
>   			       N_("tag message"), PARSE_OPT_NONEG, parse_msg_arg),
>   		OPT_FILENAME('F', "file", &msgfile, N_("read message from file")),
> -		OPT_PASSTHRU_ARGV(0, "trailer", &trailer_args, N_("trailer"),
> -				  N_("add custom trailer(s)"), PARSE_OPT_NONEG),
> +		OPT_STRVEC(0, "trailer", &trailer_args, N_("trailer"),
> +			   N_("add custom trailer(s)")),
>   		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of tag message")),
>   		OPT_BOOL('s', "sign", &opt.sign, N_("annotated and GPG-signed tag")),
>   		OPT_CLEANUP(&cleanup_arg),
> diff --git a/trailer.c b/trailer.c
> index 8e87d185d9..e85c6c9fbe 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -1342,40 +1342,21 @@ int amend_file_with_trailers(const char *path,
>   			     const struct strvec *trailer_args)
>   {
>   	struct strbuf buf = STRBUF_INIT;
> -	struct strvec stripped_trailer_args = STRVEC_INIT;
>   	int ret = 0;
> -	size_t i;
>   
>   	if (!trailer_args)
>   		BUG("amend_file_with_trailers called with NULL trailer_args");
>   	if (!trailer_args->nr)
>   		return 0;
>   
> -	for (i = 0; i < trailer_args->nr; i++) {
> -		const char *txt = trailer_args->v[i];
> -
> -		/*
> -		 * Historically amend_file_with_trailers() passed its arguments
> -		 * to "git interpret-trailers", which expected argv entries in
> -		 * "--trailer=<trailer>" form. Continue to accept those for
> -		 * existing callers, but pass only the value portion to the
> -		 * in-process implementation.
> -		 */
> -		skip_prefix(txt, "--trailer=", &txt);
> -		if (!*txt)
> -			die(_("empty --trailer argument"));
> -		strvec_push(&stripped_trailer_args, txt);
> -	}
> -
>   	if (strbuf_read_file(&buf, path, 0) < 0)
>   		ret = error_errno(_("could not read '%s'"), path);
>   	else
> -		amend_strbuf_with_trailers(&buf, &stripped_trailer_args);
> +		amend_strbuf_with_trailers(&buf, trailer_args);
>   
>   	if (!ret)
>   		ret = write_file_in_place(path, &buf);
>   
> -	strvec_clear(&stripped_trailer_args);
>   	strbuf_release(&buf);
>   	return ret;
>   }
> diff --git a/trailer.h b/trailer.h
> index d05dab050b..e5bd355aad 100644
> --- a/trailer.h
> +++ b/trailer.h
> @@ -209,8 +209,8 @@ void amend_strbuf_with_trailers(struct strbuf *buf,
>   /*
>    * Augment a file by appending trailers specified in trailer_args.
>    *
> - * Each element of trailer_args should be an argv-style --trailer=<trailer>
> - * option (i.e., including the --trailer= prefix).
> + * Each element of trailer_args should be in the same format as the value
> + * accepted by --trailer=<trailer> (i.e., without the --trailer= prefix).
>    *
>    * Returns 0 on success or a non-zero error code on failure.
>    */

Re: [PATCH v7 1/5] interpret-trailers: factor trailer rewriting

[Li Chen]

Hi Phillip,

 ---- On Mon, 02 Mar 2026 22:56:04 +0800  Phillip Wood <phillip.wood123@gmail.com> wrote --- 
 > Hi Li
 > 
 > On 24/02/2026 07:05, Li Chen wrote:
 > > Extract the trailer rewriting logic into a helper that appends to an
 > > output strbuf.
 > > 
 > > Update interpret_trailers() to handle file I/O only: read input once,
 > > call the helper, and write the buffered result.
 > > 
 > > This separation makes it easier to move the helper into trailer.c in the
 > > next commit.
 > 
 > This is still missing my sign off c.f. 
 > https://lore.kernel.org/f5152523-f7ff-4dee-a685-fb0b74cd6a56@gmail.com
 > 
 > > Signed-off-by: Li Chen <me@linux.beauty>
 > > ---
 > > v7:
 > > Use strbuf_write() when emitting buffered output.
 > 
 > Also renamed "sb" to "input"
 > 
 > Apart from the missing sign off this all looks good.
 
I sincerely apologize for that! I did check it during development, but somehow lost
track in the end. I will definitely include your sign-off in the next version!

Regards,
Li

Re: [PATCH v7 5/5] rebase: support --trailer

[Phillip Wood]

Hi Li

On 24/02/2026 07:05, Li Chen wrote:
> 
> diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc
> index e177808004..908717991a 100644
> --- a/Documentation/git-rebase.adoc
> +++ b/Documentation/git-rebase.adoc
> @@ -497,6 +497,13 @@ See also INCOMPATIBLE OPTIONS below.
>   +
>   See also INCOMPATIBLE OPTIONS below.
>   
> +--trailer=<trailer>::
> +	Append the given trailer to every rebased commit message, processed
> +	via linkgit:git-interpret-trailers[1]. This option implies
> +	`--force-rebase` so that fast-forwarded commits are also rewritten.

If the commits are rewritten then they're not fast-forwarded, I just say 
"This option implies `--force-rebase`." as all the other options that 
imply `--force-rebase` do.

> +See also INCOMPATIBLE OPTIONS below.

That section needs updating to include `--trailer`

> diff --git a/sequencer.c b/sequencer.c
> index a3eb39bb25..a60c2a0cde 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> [...]
> @@ -2025,6 +2027,9 @@ static int append_squash_message(struct strbuf *buf, const char *body,
>   		if (opts->signoff)
>   			append_signoff(buf, 0, 0);
>   
> +		if (opts->trailer_args.nr)
> +			amend_strbuf_with_trailers(buf, &opts->trailer_args);

I wonder if it would be better to add the trailers before the signoff so 
that "git rebase --signoff --trailer='Reviewed-by: ...'" adds the 
"Reviewed-by:" trailer before the "Signed-off-by:" trailer.

> @@ -3234,6 +3242,18 @@ static int read_populate_opts(struct replay_opts *opts)
>   
>   		read_strategy_opts(opts, &buf);
>   		strbuf_reset(&buf);
> +		if (strbuf_read_file(&buf, rebase_path_trailer(), 0) >= 0) {

Most of the other options are read with read_oneliner() which warns if 
there is a read error - we should do the same here. We should also warn 
if this file is empty as we only write it when trailer_args.nr > 0.

This is all looking pretty good

Thanks

Phillip

> +			char *p = buf.buf, *nl;
> +
> +			while ((nl = strchr(p, '\n'))) {
> +				*nl = '\0';
> +				if (!*p)
> +					BUG("rebase-merge/trailer has an empty line");
> +				strvec_push(&opts->trailer_args, p);
> +				p = nl + 1;
> +			}
> +			strbuf_reset(&buf);
> +		}
>   
>   		if (read_oneliner(&ctx->current_fixups,
>   				  rebase_path_current_fixups(),
> @@ -3328,6 +3348,14 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
>   		write_file(rebase_path_reschedule_failed_exec(), "%s", "");
>   	else
>   		write_file(rebase_path_no_reschedule_failed_exec(), "%s", "");
> +	if (opts->trailer_args.nr) {
> +		struct strbuf buf = STRBUF_INIT;
> +
> +		for (size_t i = 0; i < opts->trailer_args.nr; i++)
> +			strbuf_addf(&buf, "%s\n", opts->trailer_args.v[i]);
> +		write_file(rebase_path_trailer(), "%s", buf.buf);
> +		strbuf_release(&buf);
> +	}
>   
>   	return 0;
>   }
> diff --git a/sequencer.h b/sequencer.h
> index 719684c8a9..bea20da085 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -57,6 +57,8 @@ struct replay_opts {
>   	int ignore_date;
>   	int commit_use_reference;
>   
> +	struct strvec trailer_args;
> +
>   	int mainline;
>   
>   	char *gpg_sign;
> @@ -84,6 +86,7 @@ struct replay_opts {
>   #define REPLAY_OPTS_INIT {			\
>   	.edit = -1,				\
>   	.action = -1,				\
> +	.trailer_args = STRVEC_INIT,		\
>   	.xopts = STRVEC_INIT,			\
>   	.ctx = replay_ctx_new(),		\
>   }
> diff --git a/t/meson.build b/t/meson.build
> index f80e366cff..1f6f8ac20b 100644
> --- a/t/meson.build
> +++ b/t/meson.build
> @@ -388,6 +388,7 @@ integration_tests = [
>     't3436-rebase-more-options.sh',
>     't3437-rebase-fixup-options.sh',
>     't3438-rebase-broken-files.sh',
> +  't3440-rebase-trailer.sh',
>     't3450-history.sh',
>     't3451-history-reword.sh',
>     't3500-cherry.sh',
> diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
> new file mode 100755
> index 0000000000..8b47579566
> --- /dev/null
> +++ b/t/t3440-rebase-trailer.sh
> @@ -0,0 +1,147 @@
> +#!/bin/sh
> +#
> +
> +test_description='git rebase --trailer integration tests
> +We verify that --trailer works with the merge backend,
> +and that it is rejected early when the apply backend is requested.'
> +
> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +
> +. ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-rebase.sh # test_commit_message, helpers
> +
> +REVIEWED_BY_TRAILER="Reviewed-by: Dev <dev@example.com>"
> +SP=" "
> +
> +test_expect_success 'setup repo with a small history' '
> +	git commit --allow-empty -m "Initial empty commit" &&
> +	test_commit first file a &&
> +	test_commit second file &&
> +	git checkout -b conflict-branch first &&
> +	test_commit file-2 file-2 &&
> +	test_commit conflict file &&
> +	test_commit third file &&
> +	git checkout main
> +'
> +
> +test_expect_success 'apply backend is rejected with --trailer' '
> +	git checkout -B apply-backend third &&
> +	test_expect_code 128 \
> +		git rebase --apply --trailer "$REVIEWED_BY_TRAILER" HEAD^ 2>err &&
> +	test_grep "fatal: --trailer requires the merge backend" err
> +'
> +
> +test_expect_success 'reject empty --trailer argument' '
> +	git checkout -B empty-trailer third &&
> +	test_expect_code 128 git rebase --trailer "" HEAD^ 2>err &&
> +	test_grep "empty --trailer" err
> +'
> +
> +test_expect_success 'reject trailer with missing key before separator' '
> +	git checkout -B missing-key third &&
> +	test_expect_code 128 git rebase --trailer ": no-key" HEAD^ 2>err &&
> +	test_grep "missing key before separator" err
> +'
> +
> +test_expect_success 'allow trailer with missing value after separator' '
> +	git checkout -B missing-value third &&
> +	git rebase --trailer "Acked-by:" HEAD^ &&
> +	test_commit_message HEAD <<-EOF
> +	third
> +
> +	Acked-by:${SP}
> +	EOF
> +'
> +
> +test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last' '
> +	git checkout -B replace-policy third &&
> +	git -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add \
> +		rebase --trailer "Bug: 123" --trailer "Bug: 456" HEAD^ &&
> +	test_commit_message HEAD <<-EOF
> +	third
> +
> +	Bug: 456
> +	EOF
> +'
> +
> +test_expect_success 'multiple Signed-off-by trailers all preserved' '
> +	git checkout -B multiple-signoff third &&
> +	git rebase --trailer "Signed-off-by: Dev A <a@example.com>" \
> +		--trailer "Signed-off-by: Dev B <b@example.com>" HEAD^ &&
> +	test_commit_message HEAD <<-EOF
> +	third
> +
> +	Signed-off-by: Dev A <a@example.com>
> +	Signed-off-by: Dev B <b@example.com>
> +	EOF
> +'
> +
> +test_expect_success 'rebase --trailer adds trailer after conflicts' '
> +	git checkout -B trailer-conflict third &&
> +	test_commit fourth file &&
> +	test_must_fail git rebase --trailer "$REVIEWED_BY_TRAILER" second &&
> +	git checkout --theirs file &&
> +	git add file &&
> +	git rebase --continue &&
> +	test_commit_message HEAD <<-EOF &&
> +	fourth
> +
> +	$REVIEWED_BY_TRAILER
> +	EOF
> +	test_commit_message HEAD^ <<-EOF
> +	third
> +
> +	$REVIEWED_BY_TRAILER
> +	EOF
> +'
> +
> +test_expect_success '--trailer handles fixup commands in todo list' '
> +	git checkout -B fixup-trailer third &&
> +	test_commit fixup-base base &&
> +	test_commit fixup-second second &&
> +	cat >todo <<-\EOF &&
> +	pick fixup-base fixup-base
> +	fixup fixup-second fixup-second
> +	EOF
> +	(
> +		set_replace_editor todo &&
> +		git rebase -i --trailer "$REVIEWED_BY_TRAILER" HEAD~2
> +	) &&
> +	test_commit_message HEAD <<-EOF &&
> +	fixup-base
> +
> +	$REVIEWED_BY_TRAILER
> +	EOF
> +	git reset --hard fixup-second &&
> +	cat >todo <<-\EOF &&
> +	pick fixup-base fixup-base
> +	fixup -C fixup-second fixup-second
> +	EOF
> +	(
> +		set_replace_editor todo &&
> +		git rebase -i --trailer "$REVIEWED_BY_TRAILER" HEAD~2
> +	) &&
> +	test_commit_message HEAD <<-EOF
> +	fixup-second
> +
> +	$REVIEWED_BY_TRAILER
> +	EOF
> +'
> +
> +test_expect_success 'rebase --root honors trailer.<name>.key' '
> +	git checkout -B root-trailer first &&
> +	git -c trailer.review.key=Reviewed-by rebase --root \
> +		--trailer=review="Dev <dev@example.com>" &&
> +	test_commit_message HEAD <<-EOF &&
> +	first
> +
> +	Reviewed-by: Dev <dev@example.com>
> +	EOF
> +	test_commit_message HEAD^ <<-EOF
> +	Initial empty commit
> +
> +	Reviewed-by: Dev <dev@example.com>
> +	EOF
> +'
> +test_done

Re: [PATCH v7 5/5] rebase: support --trailer

[Kristoffer Haugsbakk]

On Tue, Mar 3, 2026, at 16:05, Phillip Wood wrote:
>>[snip]
>> diff --git a/sequencer.c b/sequencer.c
>> index a3eb39bb25..a60c2a0cde 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> [...]
>> @@ -2025,6 +2027,9 @@ static int append_squash_message(struct strbuf *buf, const char *body,
>>   		if (opts->signoff)
>>   			append_signoff(buf, 0, 0);
>>
>> +		if (opts->trailer_args.nr)
>> +			amend_strbuf_with_trailers(buf, &opts->trailer_args);
>
> I wonder if it would be better to add the trailers before the signoff so
> that "git rebase --signoff --trailer='Reviewed-by: ...'" adds the
> "Reviewed-by:" trailer before the "Signed-off-by:" trailer.

Why is that? Is that because that is the practice in this project (and
maybe others)?

I would expect it to act like however `--trailer` already acts on
git-commit(1) and git-tag(1). I would have to test that.

In any case these `--signoff` options are considered a historical
mistake now (since they special-case one key).

The logic for before/after and so on are supposed to be handled by the
trailer config, it seems. But last I looked that was only for same-key
trailers and duplicates. Not for logic like keeping your own signoff
last.

>[snip]

Re: [PATCH v7 5/5] rebase: support --trailer

[Junio C Hamano]

"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:

>> I wonder if it would be better to add the trailers before the signoff so
>> that "git rebase --signoff --trailer='Reviewed-by: ...'" adds the
>> "Reviewed-by:" trailer before the "Signed-off-by:" trailer.
>
> Why is that? Is that because that is the practice in this project (and
> maybe others)?

I do not think it is a good idea for the above sample command, where
we can argue that the intent of the user is to have sign-off and
then reviewed-by, expressed in the order of options given.

If we want to control where the new trailers are added, perhaps we
would need to match the --where option interpret-trailers has and
let the configuration honored by that command take care of the
ordering.

Re: [PATCH v7 0/5] rebase: support --trailer

[Phillip Wood]

Hi Li

On 24/02/2026 07:05, Li Chen wrote:
> Apologies for the long delay in sending v7.
> 
> v7 is based on origin/master at v2.53.0-154-g7c02d39fc2.
> 
> This series routes trailer insertion through an in-process path, removing the
> fork/exec to builtin/interpret-trailers.
> 
> The first four commits refactor trailer rewriting in builtin/interpret-trailers
> and trailer.c so callers can reuse a single in-process helper (used by git
> interpret-trailers, git commit and git tag). The final commit adds git rebase
> --trailer, currently supported with the merge backend only (rejecting apply-only
> scenarios and validating input early).

This all looks pretty good. The main thing I think we want to change is
sharing the code that creates the temporary file in patch 3. I've push a
version that does that and fixes my other small comments to
https://github.com/phillipwood/git/commits/refs/heads/rebase-trailers-v8
The range diff is below. If you're happy I can post that as a hopefully
final v8. Of course if you want to work on it yourself you're very
welcome to do that.

Thanks

Phillip

1:  b2685e34c22 ! 1:  0d08b361995 interpret-trailers: factor trailer rewriting
     @@ Commit message
          This separation makes it easier to move the helper into trailer.c in the
          next commit.
      
     +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
          Signed-off-by: Li Chen <me@linux.beauty>
      
       ## builtin/interpret-trailers.c ##
      @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)
-:  ----------- > 2:  5a4d03ab375 interpret-trailers: refactor create_in_place_tempfile()
2:  1bac3025045 ! 3:  ab7e232a95d trailer: move process_trailers to trailer.h
     @@ Metadata
      Author: Li Chen <me@linux.beauty>
      
       ## Commit message ##
     -    trailer: move process_trailers to trailer.h
     -
     -    Move process_trailers() from builtin/interpret-trailers.c into trailer.c
     -    and expose it via trailer.h.
     -
     -    This lets other call sites reuse the same trailer rewriting logic.
     +    trailer: libify a couple of functions
     +
     +    Move create_in_place_tempfile() and process_trailers() from
     +    builtin/interpret-trailers.c into trailer.c and expose it via trailer.h.
     +
     +    This reverts most of ae0ec2e0e0b (trailer: move interpret_trailers()
     +    to interpret-trailers.c, 2024-03-01) and lets other call sites reuse
     +    the same trailer rewriting logic.
      
          Signed-off-by: Li Chen <me@linux.beauty>
     +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
      
       ## builtin/interpret-trailers.c ##
     +@@ builtin/interpret-trailers.c: static int parse_opt_parse(const struct option *opt, const char *arg,
     + 	return 0;
     + }
     +
     +-
     +-static struct tempfile *create_in_place_tempfile(const char *file)
     +-{
     +-	struct tempfile *tempfile = NULL;
     +-	struct stat st;
     +-	struct strbuf filename_template = STRBUF_INIT;
     +-	const char *tail;
     +-
     +-	if (stat(file, &st)) {
     +-		error_errno(_("could not stat %s"), file);
     +-		return NULL;
     +-	}
     +-	if (!S_ISREG(st.st_mode)) {
     +-		error(_("file %s is not a regular file"), file);
     +-		return NULL;
     +-	}
     +-	if (!(st.st_mode & S_IWUSR)) {
     +-		error(_("file %s is not writable by user"), file);
     +-		return NULL;
     +-	}
     +-	/* Create temporary file in the same directory as the original */
     +-	tail = find_last_dir_sep(file);
     +-	if (tail)
     +-		strbuf_add(&filename_template, file, tail - file + 1);
     +-	strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
     +-
     +-	tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);
     +-
     +-	strbuf_release(&filename_template);
     +-
     +-	return tempfile;
     +-}
     +-
     + static void read_input_file(struct strbuf *sb, const char *file)
     + {
     + 	if (file) {
      @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)
       	strbuf_complete_line(sb);
       }
     @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, con
       static void interpret_trailers(const struct process_trailer_options *opts,
       			       struct list_head *new_trailer_head,
       			       const char *file)
     +@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
     + 	read_input_file(&input, file);
     +
     + 	if (opts->in_place) {
     +-		tempfile = create_in_place_tempfile(file);
     ++		tempfile = trailer_create_in_place_tempfile(file);
     + 		if (!tempfile)
     + 			die(NULL);
     + 		fd = tempfile->fd;
      
       ## trailer.c ##
     +@@
     + #include "commit.h"
     + #include "trailer.h"
     + #include "list.h"
     ++#include "tempfile.h"
     ++
     + /*
     +  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
     +  */
     +@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
     + 	strbuf_release(&iter->key);
     + }
     +
     ++struct tempfile *trailer_create_in_place_tempfile(const char *file)
     ++{
     ++	struct tempfile *tempfile = NULL;
     ++	struct stat st;
     ++	struct strbuf filename_template = STRBUF_INIT;
     ++	const char *tail;
     ++
     ++	if (stat(file, &st)) {
     ++		error_errno(_("could not stat %s"), file);
     ++		return NULL;
     ++	}
     ++	if (!S_ISREG(st.st_mode)) {
     ++		error(_("file %s is not a regular file"), file);
     ++		return NULL;
     ++	}
     ++	if (!(st.st_mode & S_IWUSR)) {
     ++		error(_("file %s is not writable by user"), file);
     ++		return NULL;
     ++	}
     ++	/* Create temporary file in the same directory as the original */
     ++	tail = find_last_dir_sep(file);
     ++	if (tail)
     ++		strbuf_add(&filename_template, file, tail - file + 1);
     ++	strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
     ++
     ++	tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);
     ++
     ++	strbuf_release(&filename_template);
     ++
     ++	return tempfile;
     ++}
     ++
     + int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)
     + {
     + 	struct child_process run_trailer = CHILD_PROCESS_INIT;
      @@ trailer.c: int amend_file_with_trailers(const char *path, const struct strvec *trailer_args
       	strvec_pushv(&run_trailer.args, trailer_args->v);
       	return run_command(&run_trailer);
     @@ trailer.h: void trailer_iterator_release(struct trailer_iterator *iter);
        */
       int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
       
     ++/*
     ++ * Create a tempfile ""git-interpret-trailers-XXXXXX" in the same
     ++ * directory as file.
     ++ */
     ++struct tempfile *trailer_create_in_place_tempfile(const char *file);
     ++
     ++/*
     ++ * Rewrite the contents of input by processing its trailer block according to
     ++ * opts and (optionally) appending trailers from new_trailer_head.
     ++ *
     ++ * The rewritten message is appended to out (callers should strbuf_reset()
     ++ * first if needed).
     ++ */
      +void process_trailers(const struct process_trailer_options *opts,
      +		      struct list_head *new_trailer_head,
      +		      struct strbuf *input, struct strbuf *out);
3:  3114f0dbb57 ! 4:  1f24917eb64 trailer: append trailers without fork/exec
     @@ Commit message
      
          Update amend_file_with_trailers() to use the in-process helper and
          rewrite the target file via tempfile+rename, preserving the previous
     -    in-place semantics.
     +    in-place semantics. As the trailers are no longer added in a separate
     +    process and trailer_config_init() die()s on missing config values it
     +    is called early on in cmd_commit() and cmd_tag() so that they die()
     +    early before writing the message file. The trailer arguments are now
     +    also sanity checked.
      
          Keep existing callers unchanged by continuing to accept argv-style
          --trailer=<trailer> entries and stripping the prefix before feeding the
          in-process implementation.
      
          Signed-off-by: Li Chen <me@linux.beauty>
     +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
     -
     - ## builtin/interpret-trailers.c ##
     -@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
     - 	struct strbuf out = STRBUF_INIT;
     - 	FILE *outfile = stdout;
     -
     --	trailer_config_init();
     --
     - 	read_input_file(&input, file);
     -
     - 	if (opts->in_place)
     -@@ builtin/interpret-trailers.c: int cmd_interpret_trailers(int argc,
     - 			git_interpret_trailers_usage,
     - 			options);
     -
     -+	trailer_config_init();
     -+
     - 	if (argc) {
     - 		int i;
     - 		for (i = 0; i < argc; i++)
     +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
     +
     + ## builtin/commit.c ##
     +@@ builtin/commit.c: int cmd_commit(int argc,
     + 	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
     + 					  builtin_commit_usage,
     + 					  prefix, current_head, &s);
     ++	if (trailer_args.nr)
     ++		trailer_config_init();
     ++
     + 	if (verbose == -1)
     + 		verbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;
     +
     +
     + ## builtin/tag.c ##
     +@@ builtin/tag.c: int cmd_tag(int argc,
     + 	if (cmdmode == 'l')
     + 		setup_auto_pager("tag", 1);
     +
     ++	if (trailer_args.nr)
     ++		trailer_config_init();
     ++
     + 	if (opt.sign == -1)
     + 		opt.sign = cmdmode ? 0 : config_sign_tag > 0;
     +
      
       ## trailer.c ##
      @@
       #include "string-list.h"
       #include "run-command.h"
       #include "commit.h"
      +#include "strvec.h"
     -+#include "tempfile.h"
       #include "trailer.h"
       #include "list.h"
     -+
     - /*
     -  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
     -  */
     + #include "tempfile.h"
      @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head,
       	free(cl_separators);
       }
       
     -+void validate_trailer_args(const struct strvec *cli_args)
     ++int validate_trailer_args(const struct strvec *cli_args)
      +{
      +	char *cl_separators;
     ++	int ret = 0;
      +
      +	trailer_config_init();
      +
     @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head
      +		const char *txt = cli_args->v[i];
      +		ssize_t separator_pos;
      +
     -+		if (!*txt)
     -+			die(_("empty --trailer argument"));
     -+
     ++		if (!*txt) {
     ++			ret = error(_("empty --trailer argument"));
     ++			goto out;
     ++		}
      +		separator_pos = find_separator(txt, cl_separators);
     -+		if (separator_pos == 0)
     -+			die(_("invalid trailer '%s': missing key before separator"),
     -+			    txt);
     ++		if (separator_pos == 0) {
     ++			ret = error(_("invalid trailer '%s': missing key before separator"),
     ++				    txt);
     ++			goto out;
     ++		}
      +	}
     -+
     ++out:
      +	free(cl_separators);
     ++	return ret;
      +}
      +
       static const char *next_line(const char *str)
       {
       	const char *nl = strchrnul(str, '\n');
     -@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
     - 	strbuf_release(&iter->key);
     +@@ trailer.c: struct tempfile *trailer_create_in_place_tempfile(const char *file)
     + 	return tempfile;
       }
       
      -int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)
     @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
      -		     path, NULL);
      -	strvec_pushv(&run_trailer.args, trailer_args->v);
      -	return run_command(&run_trailer);
     -+static void new_trailer_items_clear(struct list_head *items)
     -+{
     -+	while (!list_empty(items)) {
     -+		struct new_trailer_item *item =
     -+			list_first_entry(items, struct new_trailer_item, list);
     -+		list_del(&item->list);
     -+		free(item);
     -+	}
     -+}
     -+
     -+void amend_strbuf_with_trailers(struct strbuf *buf,
     ++int amend_strbuf_with_trailers(struct strbuf *buf,
      +				const struct strvec *trailer_args)
      +{
      +	struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
      +	LIST_HEAD(new_trailer_head);
      +	struct strbuf out = STRBUF_INIT;
      +	size_t i;
     ++	int ret = 0;
      +
      +	opts.no_divider = 1;
      +
      +	for (i = 0; i < trailer_args->nr; i++) {
      +		const char *text = trailer_args->v[i];
      +		struct new_trailer_item *item;
      +
     -+		if (!*text)
     -+			die(_("empty --trailer argument"));
     ++		if (!*text) {
     ++			ret = error(_("empty --trailer argument"));
     ++			goto out;
     ++		}
      +		item = xcalloc(1, sizeof(*item));
     -+		item->text = text;
     ++		item->text = xstrdup(text);
      +		list_add_tail(&item->list, &new_trailer_head);
      +	}
      +
     -+	trailer_config_init();
      +	process_trailers(&opts, &new_trailer_head, buf, &out);
      +
      +	strbuf_swap(buf, &out);
     ++out:
      +	strbuf_release(&out);
     ++	free_trailers(&new_trailer_head);
      +
     -+	new_trailer_items_clear(&new_trailer_head);
     ++	return ret;
      +}
      +
      +static int write_file_in_place(const char *path, const struct strbuf *buf)
      +{
     -+	struct stat st;
     -+	struct strbuf filename_template = STRBUF_INIT;
     -+	const char *tail;
     -+	struct tempfile *tempfile;
     -+	FILE *outfile;
     -+
     -+	if (stat(path, &st))
     -+		return error_errno(_("could not stat %s"), path);
     -+	if (!S_ISREG(st.st_mode))
     -+		return error(_("file %s is not a regular file"), path);
     -+	if (!(st.st_mode & S_IWUSR))
     -+		return error(_("file %s is not writable by user"), path);
     -+
     -+	/* Create temporary file in the same directory as the original */
     -+	tail = strrchr(path, '/');
     -+	if (tail)
     -+		strbuf_add(&filename_template, path, tail - path + 1);
     -+	strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
     -+
     -+	tempfile = mks_tempfile_sm(filename_template.buf, 0, st.st_mode);
     -+	strbuf_release(&filename_template);
     ++	struct tempfile *tempfile = trailer_create_in_place_tempfile(path);
      +	if (!tempfile)
     -+		return error_errno(_("could not create temporary file"));
     -+
     -+	outfile = fdopen_tempfile(tempfile, "w");
     -+	if (!outfile) {
     -+		int saved_errno = errno;
     -+		delete_tempfile(&tempfile);
     -+		errno = saved_errno;
     -+		return error_errno(_("could not open temporary file"));
     -+	}
     -+
     -+	if (buf->len && fwrite(buf->buf, 1, buf->len, outfile) < buf->len) {
     -+		int saved_errno = errno;
     -+		delete_tempfile(&tempfile);
     -+		errno = saved_errno;
     ++		return -1;
     ++
     ++	if (write_in_full(tempfile->fd, buf->buf, buf->len) < 0)
      +		return error_errno(_("could not write to temporary file"));
     -+	}
      +
      +	if (rename_tempfile(&tempfile, path))
      +		return error_errno(_("could not rename temporary file to %s"), path);
     @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
      +		 * in-process implementation.
      +		 */
      +		skip_prefix(txt, "--trailer=", &txt);
     -+		if (!*txt)
     -+			die(_("empty --trailer argument"));
     ++		if (!*txt) {
     ++			ret = error(_("empty --trailer argument"));
     ++			goto out;
     ++		}
      +		strvec_push(&stripped_trailer_args, txt);
      +	}
      +
     ++	if (validate_trailer_args(&stripped_trailer_args)) {
     ++		ret = -1;
     ++		goto out;
     ++	}
      +	if (strbuf_read_file(&buf, path, 0) < 0)
      +		ret = error_errno(_("could not read '%s'"), path);
      +	else
      +		amend_strbuf_with_trailers(&buf, &stripped_trailer_args);
      +
      +	if (!ret)
      +		ret = write_file_in_place(path, &buf);
     -+
     ++out:
      +	strvec_clear(&stripped_trailer_args);
      +	strbuf_release(&buf);
      +	return ret;
     @@ trailer.h: void parse_trailers_from_config(struct list_head *config_head);
       void parse_trailers_from_command_line_args(struct list_head *arg_head,
       					   struct list_head *new_trailer_head);
       
     -+void validate_trailer_args(const struct strvec *cli_args);
     ++int validate_trailer_args(const struct strvec *cli_args);
      +
       void process_trailers_lists(struct list_head *head,
       			    struct list_head *arg_head);
     @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);
      + * Each element of trailer_args should be in the same format as the value
      + * accepted by --trailer=<trailer> (i.e., without the --trailer= prefix).
      + */
     -+void amend_strbuf_with_trailers(struct strbuf *buf,
     ++int amend_strbuf_with_trailers(struct strbuf *buf,
      +				const struct strvec *trailer_args);
      +
      +/*
     @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);
        */
       int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
       
     -+/*
     -+ * Rewrite the contents of input by processing its trailer block according to
     -+ * opts and (optionally) appending trailers from new_trailer_head.
     -+ *
     -+ * The rewritten message is appended to out (callers should strbuf_reset()
     -+ * first if needed).
     -+ */
     - void process_trailers(const struct process_trailer_options *opts,
     - 		      struct list_head *new_trailer_head,
     - 		      struct strbuf *input, struct strbuf *out);
4:  147595a9317 ! 5:  3c1fa9e8579 commit, tag: parse --trailer with OPT_STRVEC
     @@ Commit message
          amend_file_with_trailers().
      
          Signed-off-by: Li Chen <me@linux.beauty>
      
       ## builtin/commit.c ##
      @@ builtin/commit.c: int cmd_commit(int argc,
     @@ trailer.c: int amend_file_with_trailers(const char *path,
      -		 * in-process implementation.
      -		 */
      -		skip_prefix(txt, "--trailer=", &txt);
     --		if (!*txt)
     --			die(_("empty --trailer argument"));
     +-		if (!*txt) {
     +-			ret = error(_("empty --trailer argument"));
     +-			goto out;
     +-		}
      -		strvec_push(&stripped_trailer_args, txt);
      -	}
      -
     +-	if (validate_trailer_args(&stripped_trailer_args)) {
     ++	if (validate_trailer_args(trailer_args)) {
     + 		ret = -1;
     + 		goto out;
     + 	}
       	if (strbuf_read_file(&buf, path, 0) < 0)
       		ret = error_errno(_("could not read '%s'"), path);
       	else
     @@ trailer.c: int amend_file_with_trailers(const char *path,
       
       	if (!ret)
       		ret = write_file_in_place(path, &buf);
     -
     + out:
      -	strvec_clear(&stripped_trailer_args);
       	strbuf_release(&buf);
       	return ret;
       }
      
       ## trailer.h ##
     -@@ trailer.h: void amend_strbuf_with_trailers(struct strbuf *buf,
     +@@ trailer.h: int amend_strbuf_with_trailers(struct strbuf *buf,
       /*
        * Augment a file by appending trailers specified in trailer_args.
        *
5:  864cf5f8eb6 ! 6:  99654d80547 rebase: support --trailer
     @@ Commit message
          non-interactive and interactive rebases.
      
          Signed-off-by: Li Chen <me@linux.beauty>
     +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
      
       ## Documentation/git-rebase.adoc ##
      @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.
     @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.
      +--trailer=<trailer>::
      +	Append the given trailer to every rebased commit message, processed
      +	via linkgit:git-interpret-trailers[1]. This option implies
     -+	`--force-rebase` so that fast-forwarded commits are also rewritten.
     ++	`--force-rebase`.
      ++
      +See also INCOMPATIBLE OPTIONS below.
      +
       -i::
       --interactive::
       	Make a list of the commits which are about to be rebased.  Let the
     +@@ Documentation/git-rebase.adoc: are incompatible with the following options:
     +  * --[no-]reapply-cherry-picks when used without --keep-base
     +  * --update-refs
     +  * --root when used without --onto
     ++ * --trailer
     +
     + In addition, the following pairs of options are incompatible:
     +
      
       ## builtin/rebase.c ##
      @@
     @@ builtin/rebase.c: int cmd_rebase(int argc,
       			     builtin_rebase_usage, 0);
       
      +	if (options.trailer_args.nr) {
     -+		validate_trailer_args(&options.trailer_args);
     ++		if (validate_trailer_args(&options.trailer_args))
     ++			die(NULL);
      +		options.flags |= REBASE_FORCE;
      +	}
      +
     @@ sequencer.c: void replay_opts_release(struct replay_opts *opts)
       	free(opts->ctx);
       }
      @@ sequencer.c: static int append_squash_message(struct strbuf *buf, const char *body,
     + 	if (is_fixup_flag(command, flag) && !seen_squash(ctx)) {
     + 		/*
     + 		 * We're replacing the commit message so we need to
     +-		 * append the Signed-off-by: trailer if the user
     +-		 * requested '--signoff'.
     ++		 * append any trailers if the user requested
     ++		 * '--signoff' or '--trailer'.
     + 		 */
       		if (opts->signoff)
       			append_signoff(buf, 0, 0);
       
     @@ sequencer.c: static int do_pick_commit(struct repository *r,
       	if (is_rebase_i(opts) && write_author_script(msg.message) < 0)
       		res = -1;
       	else if (!opts->strategy ||
     +@@ sequencer.c: static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)
     + 	parse_strategy_opts(opts, buf->buf);
     + }
     +
     ++static int read_trailers(struct replay_opts *opts, struct strbuf *buf)
     ++{
     ++	ssize_t len;
     ++
     ++	strbuf_reset(buf);
     ++	len = strbuf_read_file(buf, rebase_path_trailer(), 0);
     ++	if (len > 0) {
     ++		char *p = buf->buf, *nl;
     ++
     ++		trailer_config_init();
     ++
     ++		while ((nl = strchr(p, '\n'))) {
     ++			*nl = '\0';
     ++			if (!*p)
     ++				return error(_("trailers file contains empty line"));
     ++			strvec_push(&opts->trailer_args, p);
     ++			p = nl + 1;
     ++		}
     ++	} else if (!len) {
     ++		return error(_("trailers file is empty"));
     ++	} else if (errno != ENOENT) {
     ++		return error(_("cannot read trailers files"));
     ++	}
     ++
     ++	return 0;
     ++}
     ++
     + static int read_populate_opts(struct replay_opts *opts)
     + {
     + 	struct replay_ctx *ctx = opts->ctx;
      @@ sequencer.c: static int read_populate_opts(struct replay_opts *opts)
     + 			opts->keep_redundant_commits = 1;
       
       		read_strategy_opts(opts, &buf);
     ++
     ++		if (read_trailers(opts, &buf)) {
     ++			ret = -1;
     ++			goto done_rebase_i;
     ++		}
       		strbuf_reset(&buf);
     -+		if (strbuf_read_file(&buf, rebase_path_trailer(), 0) >= 0) {
     -+			char *p = buf.buf, *nl;
     -+
     -+			while ((nl = strchr(p, '\n'))) {
     -+				*nl = '\0';
     -+				if (!*p)
     -+					BUG("rebase-merge/trailer has an empty line");
     -+				strvec_push(&opts->trailer_args, p);
     -+				p = nl + 1;
     -+			}
     -+			strbuf_reset(&buf);
     -+		}
       
       		if (read_oneliner(&ctx->current_fixups,
     - 				  rebase_path_current_fixups(),
      @@ sequencer.c: int write_basic_state(struct replay_opts *opts, const char *head_name,
       		write_file(rebase_path_reschedule_failed_exec(), "%s", "");
       	else

Re: [PATCH v7 5/5] rebase: support --trailer

[Phillip Wood]

On 03/03/2026 21:18, Junio C Hamano wrote:
> "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
> 
>>> I wonder if it would be better to add the trailers before the signoff so
>>> that "git rebase --signoff --trailer='Reviewed-by: ...'" adds the
>>> "Reviewed-by:" trailer before the "Signed-off-by:" trailer.
>>
>> Why is that? Is that because that is the practice in this project (and
>> maybe others)?
> 
> I do not think it is a good idea for the above sample command, where
> we can argue that the intent of the user is to have sign-off and
> then reviewed-by, expressed in the order of options given.
> 
> If we want to control where the new trailers are added, perhaps we
> would need to match the --where option interpret-trailers has and
> let the configuration honored by that command take care of the
> ordering.

Let's leave it as it is for now as that matches what "git commit 
--signoff --trailer=..." does and no one has complained about that.

Thanks

Phillip

Re: [PATCH v7 5/5] rebase: support --trailer

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 03/03/2026 21:18, Junio C Hamano wrote:
>> "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
>> 
>>>> I wonder if it would be better to add the trailers before the signoff so
>>>> that "git rebase --signoff --trailer='Reviewed-by: ...'" adds the
>>>> "Reviewed-by:" trailer before the "Signed-off-by:" trailer.
>>>
>>> Why is that? Is that because that is the practice in this project (and
>>> maybe others)?
>> 
>> I do not think it is a good idea for the above sample command, where
>> we can argue that the intent of the user is to have sign-off and
>> then reviewed-by, expressed in the order of options given.
>> 
>> If we want to control where the new trailers are added, perhaps we
>> would need to match the --where option interpret-trailers has and
>> let the configuration honored by that command take care of the
>> ordering.
>
> Let's leave it as it is for now as that matches what "git commit 
> --signoff --trailer=..." does and no one has complained about that.

Fair enough.

Re: [PATCH v7 0/5] rebase: support --trailer

[Li Chen]

Hi Phillip,

 ---- On Wed, 04 Mar 2026 22:29:42 +0800  Phillip Wood <phillip.wood123@gmail.com> wrote --- 
 > Hi Li
 > 
 > On 24/02/2026 07:05, Li Chen wrote:
 > > Apologies for the long delay in sending v7.
 > > 
 > > v7 is based on origin/master at v2.53.0-154-g7c02d39fc2.
 > > 
 > > This series routes trailer insertion through an in-process path, removing the
 > > fork/exec to builtin/interpret-trailers.
 > > 
 > > The first four commits refactor trailer rewriting in builtin/interpret-trailers
 > > and trailer.c so callers can reuse a single in-process helper (used by git
 > > interpret-trailers, git commit and git tag). The final commit adds git rebase
 > > --trailer, currently supported with the merge backend only (rejecting apply-only
 > > scenarios and validating input early).
 > 
 > This all looks pretty good. The main thing I think we want to change is
 > sharing the code that creates the temporary file in patch 3. I've push a
 > version that does that and fixes my other small comments to
 > https://github.com/phillipwood/git/commits/refs/heads/rebase-trailers-v8
 > The range diff is below. If you're happy I can post that as a hopefully
 > final v8. Of course if you want to work on it yourself you're very
 > welcome to do that.

These changes all look good to me! Thank you very much for your patience and work! Please help 
post this as v8.

Regards,
Li

 > 
 > 1:  b2685e34c22 ! 1:  0d08b361995 interpret-trailers: factor trailer rewriting
 >      @@ Commit message
 >           This separation makes it easier to move the helper into trailer.c in the
 >           next commit.
 >       
 >      +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
 >           Signed-off-by: Li Chen <me@linux.beauty>
 >       
 >        ## builtin/interpret-trailers.c ##
 >       @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)
 > -:  ----------- > 2:  5a4d03ab375 interpret-trailers: refactor create_in_place_tempfile()
 > 2:  1bac3025045 ! 3:  ab7e232a95d trailer: move process_trailers to trailer.h
 >      @@ Metadata
 >       Author: Li Chen <me@linux.beauty>
 >       
 >        ## Commit message ##
 >      -    trailer: move process_trailers to trailer.h
 >      -
 >      -    Move process_trailers() from builtin/interpret-trailers.c into trailer.c
 >      -    and expose it via trailer.h.
 >      -
 >      -    This lets other call sites reuse the same trailer rewriting logic.
 >      +    trailer: libify a couple of functions
 >      +
 >      +    Move create_in_place_tempfile() and process_trailers() from
 >      +    builtin/interpret-trailers.c into trailer.c and expose it via trailer.h.
 >      +
 >      +    This reverts most of ae0ec2e0e0b (trailer: move interpret_trailers()
 >      +    to interpret-trailers.c, 2024-03-01) and lets other call sites reuse
 >      +    the same trailer rewriting logic.
 >       
 >           Signed-off-by: Li Chen <me@linux.beauty>
 >      +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
 >       
 >        ## builtin/interpret-trailers.c ##
 >      +@@ builtin/interpret-trailers.c: static int parse_opt_parse(const struct option *opt, const char *arg,
 >      +     return 0;
 >      + }
 >      +
 >      +-
 >      +-static struct tempfile *create_in_place_tempfile(const char *file)
 >      +-{
 >      +-    struct tempfile *tempfile = NULL;
 >      +-    struct stat st;
 >      +-    struct strbuf filename_template = STRBUF_INIT;
 >      +-    const char *tail;
 >      +-
 >      +-    if (stat(file, &st)) {
 >      +-        error_errno(_("could not stat %s"), file);
 >      +-        return NULL;
 >      +-    }
 >      +-    if (!S_ISREG(st.st_mode)) {
 >      +-        error(_("file %s is not a regular file"), file);
 >      +-        return NULL;
 >      +-    }
 >      +-    if (!(st.st_mode & S_IWUSR)) {
 >      +-        error(_("file %s is not writable by user"), file);
 >      +-        return NULL;
 >      +-    }
 >      +-    /* Create temporary file in the same directory as the original */
 >      +-    tail = find_last_dir_sep(file);
 >      +-    if (tail)
 >      +-        strbuf_add(&filename_template, file, tail - file + 1);
 >      +-    strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
 >      +-
 >      +-    tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);
 >      +-
 >      +-    strbuf_release(&filename_template);
 >      +-
 >      +-    return tempfile;
 >      +-}
 >      +-
 >      + static void read_input_file(struct strbuf *sb, const char *file)
 >      + {
 >      +     if (file) {
 >       @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)
 >            strbuf_complete_line(sb);
 >        }
 >      @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, con
 >        static void interpret_trailers(const struct process_trailer_options *opts,
 >                           struct list_head *new_trailer_head,
 >                           const char *file)
 >      +@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
 >      +     read_input_file(&input, file);
 >      +
 >      +     if (opts->in_place) {
 >      +-        tempfile = create_in_place_tempfile(file);
 >      ++        tempfile = trailer_create_in_place_tempfile(file);
 >      +         if (!tempfile)
 >      +             die(NULL);
 >      +         fd = tempfile->fd;
 >       
 >        ## trailer.c ##
 >      +@@
 >      + #include "commit.h"
 >      + #include "trailer.h"
 >      + #include "list.h"
 >      ++#include "tempfile.h"
 >      ++
 >      + /*
 >      +  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
 >      +  */
 >      +@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
 >      +     strbuf_release(&iter->key);
 >      + }
 >      +
 >      ++struct tempfile *trailer_create_in_place_tempfile(const char *file)
 >      ++{
 >      ++    struct tempfile *tempfile = NULL;
 >      ++    struct stat st;
 >      ++    struct strbuf filename_template = STRBUF_INIT;
 >      ++    const char *tail;
 >      ++
 >      ++    if (stat(file, &st)) {
 >      ++        error_errno(_("could not stat %s"), file);
 >      ++        return NULL;
 >      ++    }
 >      ++    if (!S_ISREG(st.st_mode)) {
 >      ++        error(_("file %s is not a regular file"), file);
 >      ++        return NULL;
 >      ++    }
 >      ++    if (!(st.st_mode & S_IWUSR)) {
 >      ++        error(_("file %s is not writable by user"), file);
 >      ++        return NULL;
 >      ++    }
 >      ++    /* Create temporary file in the same directory as the original */
 >      ++    tail = find_last_dir_sep(file);
 >      ++    if (tail)
 >      ++        strbuf_add(&filename_template, file, tail - file + 1);
 >      ++    strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
 >      ++
 >      ++    tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);
 >      ++
 >      ++    strbuf_release(&filename_template);
 >      ++
 >      ++    return tempfile;
 >      ++}
 >      ++
 >      + int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)
 >      + {
 >      +     struct child_process run_trailer = CHILD_PROCESS_INIT;
 >       @@ trailer.c: int amend_file_with_trailers(const char *path, const struct strvec *trailer_args
 >            strvec_pushv(&run_trailer.args, trailer_args->v);
 >            return run_command(&run_trailer);
 >      @@ trailer.h: void trailer_iterator_release(struct trailer_iterator *iter);
 >         */
 >        int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
 >        
 >      ++/*
 >      ++ * Create a tempfile ""git-interpret-trailers-XXXXXX" in the same
 >      ++ * directory as file.
 >      ++ */
 >      ++struct tempfile *trailer_create_in_place_tempfile(const char *file);
 >      ++
 >      ++/*
 >      ++ * Rewrite the contents of input by processing its trailer block according to
 >      ++ * opts and (optionally) appending trailers from new_trailer_head.
 >      ++ *
 >      ++ * The rewritten message is appended to out (callers should strbuf_reset()
 >      ++ * first if needed).
 >      ++ */
 >       +void process_trailers(const struct process_trailer_options *opts,
 >       +              struct list_head *new_trailer_head,
 >       +              struct strbuf *input, struct strbuf *out);
 > 3:  3114f0dbb57 ! 4:  1f24917eb64 trailer: append trailers without fork/exec
 >      @@ Commit message
 >       
 >           Update amend_file_with_trailers() to use the in-process helper and
 >           rewrite the target file via tempfile+rename, preserving the previous
 >      -    in-place semantics.
 >      +    in-place semantics. As the trailers are no longer added in a separate
 >      +    process and trailer_config_init() die()s on missing config values it
 >      +    is called early on in cmd_commit() and cmd_tag() so that they die()
 >      +    early before writing the message file. The trailer arguments are now
 >      +    also sanity checked.
 >       
 >           Keep existing callers unchanged by continuing to accept argv-style
 >           --trailer=<trailer> entries and stripping the prefix before feeding the
 >           in-process implementation.
 >       
 >           Signed-off-by: Li Chen <me@linux.beauty>
 >      +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
 >      -
 >      - ## builtin/interpret-trailers.c ##
 >      -@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
 >      -     struct strbuf out = STRBUF_INIT;
 >      -     FILE *outfile = stdout;
 >      -
 >      --    trailer_config_init();
 >      --
 >      -     read_input_file(&input, file);
 >      -
 >      -     if (opts->in_place)
 >      -@@ builtin/interpret-trailers.c: int cmd_interpret_trailers(int argc,
 >      -             git_interpret_trailers_usage,
 >      -             options);
 >      -
 >      -+    trailer_config_init();
 >      -+
 >      -     if (argc) {
 >      -         int i;
 >      -         for (i = 0; i < argc; i++)
 >      +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
 >      +
 >      + ## builtin/commit.c ##
 >      +@@ builtin/commit.c: int cmd_commit(int argc,
 >      +     argc = parse_and_validate_options(argc, argv, builtin_commit_options,
 >      +                       builtin_commit_usage,
 >      +                       prefix, current_head, &s);
 >      ++    if (trailer_args.nr)
 >      ++        trailer_config_init();
 >      ++
 >      +     if (verbose == -1)
 >      +         verbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;
 >      +
 >      +
 >      + ## builtin/tag.c ##
 >      +@@ builtin/tag.c: int cmd_tag(int argc,
 >      +     if (cmdmode == 'l')
 >      +         setup_auto_pager("tag", 1);
 >      +
 >      ++    if (trailer_args.nr)
 >      ++        trailer_config_init();
 >      ++
 >      +     if (opt.sign == -1)
 >      +         opt.sign = cmdmode ? 0 : config_sign_tag > 0;
 >      +
 >       
 >        ## trailer.c ##
 >       @@
 >        #include "string-list.h"
 >        #include "run-command.h"
 >        #include "commit.h"
 >       +#include "strvec.h"
 >      -+#include "tempfile.h"
 >        #include "trailer.h"
 >        #include "list.h"
 >      -+
 >      - /*
 >      -  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
 >      -  */
 >      + #include "tempfile.h"
 >       @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head,
 >            free(cl_separators);
 >        }
 >        
 >      -+void validate_trailer_args(const struct strvec *cli_args)
 >      ++int validate_trailer_args(const struct strvec *cli_args)
 >       +{
 >       +    char *cl_separators;
 >      ++    int ret = 0;
 >       +
 >       +    trailer_config_init();
 >       +
 >      @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head
 >       +        const char *txt = cli_args->v[i];
 >       +        ssize_t separator_pos;
 >       +
 >      -+        if (!*txt)
 >      -+            die(_("empty --trailer argument"));
 >      -+
 >      ++        if (!*txt) {
 >      ++            ret = error(_("empty --trailer argument"));
 >      ++            goto out;
 >      ++        }
 >       +        separator_pos = find_separator(txt, cl_separators);
 >      -+        if (separator_pos == 0)
 >      -+            die(_("invalid trailer '%s': missing key before separator"),
 >      -+                txt);
 >      ++        if (separator_pos == 0) {
 >      ++            ret = error(_("invalid trailer '%s': missing key before separator"),
 >      ++                    txt);
 >      ++            goto out;
 >      ++        }
 >       +    }
 >      -+
 >      ++out:
 >       +    free(cl_separators);
 >      ++    return ret;
 >       +}
 >       +
 >        static const char *next_line(const char *str)
 >        {
 >            const char *nl = strchrnul(str, '\n');
 >      -@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
 >      -     strbuf_release(&iter->key);
 >      +@@ trailer.c: struct tempfile *trailer_create_in_place_tempfile(const char *file)
 >      +     return tempfile;
 >        }
 >        
 >       -int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)
 >      @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
 >       -             path, NULL);
 >       -    strvec_pushv(&run_trailer.args, trailer_args->v);
 >       -    return run_command(&run_trailer);
 >      -+static void new_trailer_items_clear(struct list_head *items)
 >      -+{
 >      -+    while (!list_empty(items)) {
 >      -+        struct new_trailer_item *item =
 >      -+            list_first_entry(items, struct new_trailer_item, list);
 >      -+        list_del(&item->list);
 >      -+        free(item);
 >      -+    }
 >      -+}
 >      -+
 >      -+void amend_strbuf_with_trailers(struct strbuf *buf,
 >      ++int amend_strbuf_with_trailers(struct strbuf *buf,
 >       +                const struct strvec *trailer_args)
 >       +{
 >       +    struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
 >       +    LIST_HEAD(new_trailer_head);
 >       +    struct strbuf out = STRBUF_INIT;
 >       +    size_t i;
 >      ++    int ret = 0;
 >       +
 >       +    opts.no_divider = 1;
 >       +
 >       +    for (i = 0; i < trailer_args->nr; i++) {
 >       +        const char *text = trailer_args->v[i];
 >       +        struct new_trailer_item *item;
 >       +
 >      -+        if (!*text)
 >      -+            die(_("empty --trailer argument"));
 >      ++        if (!*text) {
 >      ++            ret = error(_("empty --trailer argument"));
 >      ++            goto out;
 >      ++        }
 >       +        item = xcalloc(1, sizeof(*item));
 >      -+        item->text = text;
 >      ++        item->text = xstrdup(text);
 >       +        list_add_tail(&item->list, &new_trailer_head);
 >       +    }
 >       +
 >      -+    trailer_config_init();
 >       +    process_trailers(&opts, &new_trailer_head, buf, &out);
 >       +
 >       +    strbuf_swap(buf, &out);
 >      ++out:
 >       +    strbuf_release(&out);
 >      ++    free_trailers(&new_trailer_head);
 >       +
 >      -+    new_trailer_items_clear(&new_trailer_head);
 >      ++    return ret;
 >       +}
 >       +
 >       +static int write_file_in_place(const char *path, const struct strbuf *buf)
 >       +{
 >      -+    struct stat st;
 >      -+    struct strbuf filename_template = STRBUF_INIT;
 >      -+    const char *tail;
 >      -+    struct tempfile *tempfile;
 >      -+    FILE *outfile;
 >      -+
 >      -+    if (stat(path, &st))
 >      -+        return error_errno(_("could not stat %s"), path);
 >      -+    if (!S_ISREG(st.st_mode))
 >      -+        return error(_("file %s is not a regular file"), path);
 >      -+    if (!(st.st_mode & S_IWUSR))
 >      -+        return error(_("file %s is not writable by user"), path);
 >      -+
 >      -+    /* Create temporary file in the same directory as the original */
 >      -+    tail = strrchr(path, '/');
 >      -+    if (tail)
 >      -+        strbuf_add(&filename_template, path, tail - path + 1);
 >      -+    strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
 >      -+
 >      -+    tempfile = mks_tempfile_sm(filename_template.buf, 0, st.st_mode);
 >      -+    strbuf_release(&filename_template);
 >      ++    struct tempfile *tempfile = trailer_create_in_place_tempfile(path);
 >       +    if (!tempfile)
 >      -+        return error_errno(_("could not create temporary file"));
 >      -+
 >      -+    outfile = fdopen_tempfile(tempfile, "w");
 >      -+    if (!outfile) {
 >      -+        int saved_errno = errno;
 >      -+        delete_tempfile(&tempfile);
 >      -+        errno = saved_errno;
 >      -+        return error_errno(_("could not open temporary file"));
 >      -+    }
 >      -+
 >      -+    if (buf->len && fwrite(buf->buf, 1, buf->len, outfile) < buf->len) {
 >      -+        int saved_errno = errno;
 >      -+        delete_tempfile(&tempfile);
 >      -+        errno = saved_errno;
 >      ++        return -1;
 >      ++
 >      ++    if (write_in_full(tempfile->fd, buf->buf, buf->len) < 0)
 >       +        return error_errno(_("could not write to temporary file"));
 >      -+    }
 >       +
 >       +    if (rename_tempfile(&tempfile, path))
 >       +        return error_errno(_("could not rename temporary file to %s"), path);
 >      @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
 >       +         * in-process implementation.
 >       +         */
 >       +        skip_prefix(txt, "--trailer=", &txt);
 >      -+        if (!*txt)
 >      -+            die(_("empty --trailer argument"));
 >      ++        if (!*txt) {
 >      ++            ret = error(_("empty --trailer argument"));
 >      ++            goto out;
 >      ++        }
 >       +        strvec_push(&stripped_trailer_args, txt);
 >       +    }
 >       +
 >      ++    if (validate_trailer_args(&stripped_trailer_args)) {
 >      ++        ret = -1;
 >      ++        goto out;
 >      ++    }
 >       +    if (strbuf_read_file(&buf, path, 0) < 0)
 >       +        ret = error_errno(_("could not read '%s'"), path);
 >       +    else
 >       +        amend_strbuf_with_trailers(&buf, &stripped_trailer_args);
 >       +
 >       +    if (!ret)
 >       +        ret = write_file_in_place(path, &buf);
 >      -+
 >      ++out:
 >       +    strvec_clear(&stripped_trailer_args);
 >       +    strbuf_release(&buf);
 >       +    return ret;
 >      @@ trailer.h: void parse_trailers_from_config(struct list_head *config_head);
 >        void parse_trailers_from_command_line_args(struct list_head *arg_head,
 >                               struct list_head *new_trailer_head);
 >        
 >      -+void validate_trailer_args(const struct strvec *cli_args);
 >      ++int validate_trailer_args(const struct strvec *cli_args);
 >       +
 >        void process_trailers_lists(struct list_head *head,
 >                        struct list_head *arg_head);
 >      @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);
 >       + * Each element of trailer_args should be in the same format as the value
 >       + * accepted by --trailer=<trailer> (i.e., without the --trailer= prefix).
 >       + */
 >      -+void amend_strbuf_with_trailers(struct strbuf *buf,
 >      ++int amend_strbuf_with_trailers(struct strbuf *buf,
 >       +                const struct strvec *trailer_args);
 >       +
 >       +/*
 >      @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);
 >         */
 >        int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
 >        
 >      -+/*
 >      -+ * Rewrite the contents of input by processing its trailer block according to
 >      -+ * opts and (optionally) appending trailers from new_trailer_head.
 >      -+ *
 >      -+ * The rewritten message is appended to out (callers should strbuf_reset()
 >      -+ * first if needed).
 >      -+ */
 >      - void process_trailers(const struct process_trailer_options *opts,
 >      -               struct list_head *new_trailer_head,
 >      -               struct strbuf *input, struct strbuf *out);
 > 4:  147595a9317 ! 5:  3c1fa9e8579 commit, tag: parse --trailer with OPT_STRVEC
 >      @@ Commit message
 >           amend_file_with_trailers().
 >       
 >           Signed-off-by: Li Chen <me@linux.beauty>
 >       
 >        ## builtin/commit.c ##
 >       @@ builtin/commit.c: int cmd_commit(int argc,
 >      @@ trailer.c: int amend_file_with_trailers(const char *path,
 >       -         * in-process implementation.
 >       -         */
 >       -        skip_prefix(txt, "--trailer=", &txt);
 >      --        if (!*txt)
 >      --            die(_("empty --trailer argument"));
 >      +-        if (!*txt) {
 >      +-            ret = error(_("empty --trailer argument"));
 >      +-            goto out;
 >      +-        }
 >       -        strvec_push(&stripped_trailer_args, txt);
 >       -    }
 >       -
 >      +-    if (validate_trailer_args(&stripped_trailer_args)) {
 >      ++    if (validate_trailer_args(trailer_args)) {
 >      +         ret = -1;
 >      +         goto out;
 >      +     }
 >            if (strbuf_read_file(&buf, path, 0) < 0)
 >                ret = error_errno(_("could not read '%s'"), path);
 >            else
 >      @@ trailer.c: int amend_file_with_trailers(const char *path,
 >        
 >            if (!ret)
 >                ret = write_file_in_place(path, &buf);
 >      -
 >      + out:
 >       -    strvec_clear(&stripped_trailer_args);
 >            strbuf_release(&buf);
 >            return ret;
 >        }
 >       
 >        ## trailer.h ##
 >      -@@ trailer.h: void amend_strbuf_with_trailers(struct strbuf *buf,
 >      +@@ trailer.h: int amend_strbuf_with_trailers(struct strbuf *buf,
 >        /*
 >         * Augment a file by appending trailers specified in trailer_args.
 >         *
 > 5:  864cf5f8eb6 ! 6:  99654d80547 rebase: support --trailer
 >      @@ Commit message
 >           non-interactive and interactive rebases.
 >       
 >           Signed-off-by: Li Chen <me@linux.beauty>
 >      +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
 >       
 >        ## Documentation/git-rebase.adoc ##
 >       @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.
 >      @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.
 >       +--trailer=<trailer>::
 >       +    Append the given trailer to every rebased commit message, processed
 >       +    via linkgit:git-interpret-trailers[1]. This option implies
 >      -+    `--force-rebase` so that fast-forwarded commits are also rewritten.
 >      ++    `--force-rebase`.
 >       ++
 >       +See also INCOMPATIBLE OPTIONS below.
 >       +
 >        -i::
 >        --interactive::
 >            Make a list of the commits which are about to be rebased.  Let the
 >      +@@ Documentation/git-rebase.adoc: are incompatible with the following options:
 >      +  * --[no-]reapply-cherry-picks when used without --keep-base
 >      +  * --update-refs
 >      +  * --root when used without --onto
 >      ++ * --trailer
 >      +
 >      + In addition, the following pairs of options are incompatible:
 >      +
 >       
 >        ## builtin/rebase.c ##
 >       @@
 >      @@ builtin/rebase.c: int cmd_rebase(int argc,
 >                         builtin_rebase_usage, 0);
 >        
 >       +    if (options.trailer_args.nr) {
 >      -+        validate_trailer_args(&options.trailer_args);
 >      ++        if (validate_trailer_args(&options.trailer_args))
 >      ++            die(NULL);
 >       +        options.flags |= REBASE_FORCE;
 >       +    }
 >       +
 >      @@ sequencer.c: void replay_opts_release(struct replay_opts *opts)
 >            free(opts->ctx);
 >        }
 >       @@ sequencer.c: static int append_squash_message(struct strbuf *buf, const char *body,
 >      +     if (is_fixup_flag(command, flag) && !seen_squash(ctx)) {
 >      +         /*
 >      +          * We're replacing the commit message so we need to
 >      +-         * append the Signed-off-by: trailer if the user
 >      +-         * requested '--signoff'.
 >      ++         * append any trailers if the user requested
 >      ++         * '--signoff' or '--trailer'.
 >      +          */
 >                if (opts->signoff)
 >                    append_signoff(buf, 0, 0);
 >        
 >      @@ sequencer.c: static int do_pick_commit(struct repository *r,
 >            if (is_rebase_i(opts) && write_author_script(msg.message) < 0)
 >                res = -1;
 >            else if (!opts->strategy ||
 >      +@@ sequencer.c: static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)
 >      +     parse_strategy_opts(opts, buf->buf);
 >      + }
 >      +
 >      ++static int read_trailers(struct replay_opts *opts, struct strbuf *buf)
 >      ++{
 >      ++    ssize_t len;
 >      ++
 >      ++    strbuf_reset(buf);
 >      ++    len = strbuf_read_file(buf, rebase_path_trailer(), 0);
 >      ++    if (len > 0) {
 >      ++        char *p = buf->buf, *nl;
 >      ++
 >      ++        trailer_config_init();
 >      ++
 >      ++        while ((nl = strchr(p, '\n'))) {
 >      ++            *nl = '\0';
 >      ++            if (!*p)
 >      ++                return error(_("trailers file contains empty line"));
 >      ++            strvec_push(&opts->trailer_args, p);
 >      ++            p = nl + 1;
 >      ++        }
 >      ++    } else if (!len) {
 >      ++        return error(_("trailers file is empty"));
 >      ++    } else if (errno != ENOENT) {
 >      ++        return error(_("cannot read trailers files"));
 >      ++    }
 >      ++
 >      ++    return 0;
 >      ++}
 >      ++
 >      + static int read_populate_opts(struct replay_opts *opts)
 >      + {
 >      +     struct replay_ctx *ctx = opts->ctx;
 >       @@ sequencer.c: static int read_populate_opts(struct replay_opts *opts)
 >      +             opts->keep_redundant_commits = 1;
 >        
 >                read_strategy_opts(opts, &buf);
 >      ++
 >      ++        if (read_trailers(opts, &buf)) {
 >      ++            ret = -1;
 >      ++            goto done_rebase_i;
 >      ++        }
 >                strbuf_reset(&buf);
 >      -+        if (strbuf_read_file(&buf, rebase_path_trailer(), 0) >= 0) {
 >      -+            char *p = buf.buf, *nl;
 >      -+
 >      -+            while ((nl = strchr(p, '\n'))) {
 >      -+                *nl = '\0';
 >      -+                if (!*p)
 >      -+                    BUG("rebase-merge/trailer has an empty line");
 >      -+                strvec_push(&opts->trailer_args, p);
 >      -+                p = nl + 1;
 >      -+            }
 >      -+            strbuf_reset(&buf);
 >      -+        }
 >        
 >                if (read_oneliner(&ctx->current_fixups,
 >      -                   rebase_path_current_fixups(),
 >       @@ sequencer.c: int write_basic_state(struct replay_opts *opts, const char *head_name,
 >                write_file(rebase_path_reschedule_failed_exec(), "%s", "");
 >            else
 > 
 > 

[PATCH v8 0/6] rebase: support --trailer

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

This series adds support for creating trailers when rebasing, along
the lines of "git commit --trailer". Patches 1-3 refactor and libify
the code to add trailers to a buffer, patches 4,5 update "git commit
--trailer" and "git tag --trailer" to use the new code and the last
patch adds support for "git rebase --trailer".

Thanks to Li for working on this series, the main change in this
hopefully final iteration is that more code is shared between "git
interpret-trailers" and the builtin commands that add trailers via
a --trailer option, see the range-diff for patches 2-4.

Changes since V7:

 - Patch 1: Added missing sign-off.
 - Patch 2: New, refactor create_in_place_tempfile() in preparation for
            moving it.
 - Patch 3: Move create_in_place_tempfile() in addition to
            process_trailers().
 - Patch 4: Use the libified create_in_place_tempfile(), initialize
            trailer config earlier and validate --trailer args for
            "git commit" and "git tag". Library code now calls error()
            rather than die()
 - Patch 5: Unchanged, apart from the effect of previous patches
 - Patch 6: Small doc fixes, initialize trailer_config() when continuing
            a rebase that was started with --trailer. Report errors when
            reading trailer state file.

Li Chen (5):
  interpret-trailers: factor trailer rewriting
  trailer: libify a couple of functions
  trailer: append trailers without fork/exec
  commit, tag: parse --trailer with OPT_STRVEC
  rebase: support --trailer

Phillip Wood (1):
  interpret-trailers: refactor create_in_place_tempfile()

 Documentation/git-rebase.adoc |   8 ++
 builtin/commit.c              |   6 +-
 builtin/interpret-trailers.c  |  91 ++++-------------
 builtin/rebase.c              |  19 ++++
 builtin/tag.c                 |   7 +-
 sequencer.c                   |  52 +++++++++-
 sequencer.h                   |   3 +
 t/meson.build                 |   1 +
 t/t3440-rebase-trailer.sh     | 147 +++++++++++++++++++++++++++
 trailer.c                     | 184 ++++++++++++++++++++++++++++++++--
 trailer.h                     |  36 ++++++-
 11 files changed, 463 insertions(+), 91 deletions(-)
 create mode 100755 t/t3440-rebase-trailer.sh

Range-diff:
1:  b2685e34c22 ! 1:  0d08b361995 interpret-trailers: factor trailer rewriting
    @@ Commit message
         This separation makes it easier to move the helper into trailer.c in the
         next commit.
     
    +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
         Signed-off-by: Li Chen <me@linux.beauty>
     
      ## builtin/interpret-trailers.c ##
     @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)
-:  ----------- > 2:  5a4d03ab375 interpret-trailers: refactor create_in_place_tempfile()
2:  1bac3025045 ! 3:  ab7e232a95d trailer: move process_trailers to trailer.h
    @@ Metadata
     Author: Li Chen <me@linux.beauty>
     
      ## Commit message ##
    -    trailer: move process_trailers to trailer.h
    -
    -    Move process_trailers() from builtin/interpret-trailers.c into trailer.c
    -    and expose it via trailer.h.
    -
    -    This lets other call sites reuse the same trailer rewriting logic.
    +    trailer: libify a couple of functions
    +
    +    Move create_in_place_tempfile() and process_trailers() from
    +    builtin/interpret-trailers.c into trailer.c and expose it via trailer.h.
    +
    +    This reverts most of ae0ec2e0e0b (trailer: move interpret_trailers()
    +    to interpret-trailers.c, 2024-03-01) and lets other call sites reuse
    +    the same trailer rewriting logic.
     
         Signed-off-by: Li Chen <me@linux.beauty>
    +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
     
      ## builtin/interpret-trailers.c ##
    +@@ builtin/interpret-trailers.c: static int parse_opt_parse(const struct option *opt, const char *arg,
    + 	return 0;
    + }
    + 
    +-
    +-static struct tempfile *create_in_place_tempfile(const char *file)
    +-{
    +-	struct tempfile *tempfile = NULL;
    +-	struct stat st;
    +-	struct strbuf filename_template = STRBUF_INIT;
    +-	const char *tail;
    +-
    +-	if (stat(file, &st)) {
    +-		error_errno(_("could not stat %s"), file);
    +-		return NULL;
    +-	}
    +-	if (!S_ISREG(st.st_mode)) {
    +-		error(_("file %s is not a regular file"), file);
    +-		return NULL;
    +-	}
    +-	if (!(st.st_mode & S_IWUSR)) {
    +-		error(_("file %s is not writable by user"), file);
    +-		return NULL;
    +-	}
    +-	/* Create temporary file in the same directory as the original */
    +-	tail = find_last_dir_sep(file);
    +-	if (tail)
    +-		strbuf_add(&filename_template, file, tail - file + 1);
    +-	strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
    +-
    +-	tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);
    +-
    +-	strbuf_release(&filename_template);
    +-
    +-	return tempfile;
    +-}
    +-
    + static void read_input_file(struct strbuf *sb, const char *file)
    + {
    + 	if (file) {
     @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)
      	strbuf_complete_line(sb);
      }
    @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, con
      static void interpret_trailers(const struct process_trailer_options *opts,
      			       struct list_head *new_trailer_head,
      			       const char *file)
    +@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
    + 	read_input_file(&input, file);
    + 
    + 	if (opts->in_place) {
    +-		tempfile = create_in_place_tempfile(file);
    ++		tempfile = trailer_create_in_place_tempfile(file);
    + 		if (!tempfile)
    + 			die(NULL);
    + 		fd = tempfile->fd;
     
      ## trailer.c ##
    +@@
    + #include "commit.h"
    + #include "trailer.h"
    + #include "list.h"
    ++#include "tempfile.h"
    ++
    + /*
    +  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
    +  */
    +@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
    + 	strbuf_release(&iter->key);
    + }
    + 
    ++struct tempfile *trailer_create_in_place_tempfile(const char *file)
    ++{
    ++	struct tempfile *tempfile = NULL;
    ++	struct stat st;
    ++	struct strbuf filename_template = STRBUF_INIT;
    ++	const char *tail;
    ++
    ++	if (stat(file, &st)) {
    ++		error_errno(_("could not stat %s"), file);
    ++		return NULL;
    ++	}
    ++	if (!S_ISREG(st.st_mode)) {
    ++		error(_("file %s is not a regular file"), file);
    ++		return NULL;
    ++	}
    ++	if (!(st.st_mode & S_IWUSR)) {
    ++		error(_("file %s is not writable by user"), file);
    ++		return NULL;
    ++	}
    ++	/* Create temporary file in the same directory as the original */
    ++	tail = find_last_dir_sep(file);
    ++	if (tail)
    ++		strbuf_add(&filename_template, file, tail - file + 1);
    ++	strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
    ++
    ++	tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);
    ++
    ++	strbuf_release(&filename_template);
    ++
    ++	return tempfile;
    ++}
    ++
    + int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)
    + {
    + 	struct child_process run_trailer = CHILD_PROCESS_INIT;
     @@ trailer.c: int amend_file_with_trailers(const char *path, const struct strvec *trailer_args
      	strvec_pushv(&run_trailer.args, trailer_args->v);
      	return run_command(&run_trailer);
    @@ trailer.h: void trailer_iterator_release(struct trailer_iterator *iter);
       */
      int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
      
    ++/*
    ++ * Create a tempfile ""git-interpret-trailers-XXXXXX" in the same
    ++ * directory as file.
    ++ */
    ++struct tempfile *trailer_create_in_place_tempfile(const char *file);
    ++
    ++/*
    ++ * Rewrite the contents of input by processing its trailer block according to
    ++ * opts and (optionally) appending trailers from new_trailer_head.
    ++ *
    ++ * The rewritten message is appended to out (callers should strbuf_reset()
    ++ * first if needed).
    ++ */
     +void process_trailers(const struct process_trailer_options *opts,
     +		      struct list_head *new_trailer_head,
     +		      struct strbuf *input, struct strbuf *out);
3:  3114f0dbb57 ! 4:  1f24917eb64 trailer: append trailers without fork/exec
    @@ Commit message
     
         Update amend_file_with_trailers() to use the in-process helper and
         rewrite the target file via tempfile+rename, preserving the previous
    -    in-place semantics.
    +    in-place semantics. As the trailers are no longer added in a separate
    +    process and trailer_config_init() die()s on missing config values it
    +    is called early on in cmd_commit() and cmd_tag() so that they die()
    +    early before writing the message file. The trailer arguments are now
    +    also sanity checked.
     
         Keep existing callers unchanged by continuing to accept argv-style
         --trailer=<trailer> entries and stripping the prefix before feeding the
         in-process implementation.
     
         Signed-off-by: Li Chen <me@linux.beauty>
    -
    - ## builtin/interpret-trailers.c ##
    -@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
    - 	struct strbuf out = STRBUF_INIT;
    - 	FILE *outfile = stdout;
    - 
    --	trailer_config_init();
    --
    - 	read_input_file(&input, file);
    - 
    - 	if (opts->in_place)
    -@@ builtin/interpret-trailers.c: int cmd_interpret_trailers(int argc,
    - 			git_interpret_trailers_usage,
    - 			options);
    - 
    -+	trailer_config_init();
    -+
    - 	if (argc) {
    - 		int i;
    - 		for (i = 0; i < argc; i++)
    +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
    +
    + ## builtin/commit.c ##
    +@@ builtin/commit.c: int cmd_commit(int argc,
    + 	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
    + 					  builtin_commit_usage,
    + 					  prefix, current_head, &s);
    ++	if (trailer_args.nr)
    ++		trailer_config_init();
    ++
    + 	if (verbose == -1)
    + 		verbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;
    + 
    +
    + ## builtin/tag.c ##
    +@@ builtin/tag.c: int cmd_tag(int argc,
    + 	if (cmdmode == 'l')
    + 		setup_auto_pager("tag", 1);
    + 
    ++	if (trailer_args.nr)
    ++		trailer_config_init();
    ++
    + 	if (opt.sign == -1)
    + 		opt.sign = cmdmode ? 0 : config_sign_tag > 0;
    + 
     
      ## trailer.c ##
     @@
      #include "string-list.h"
      #include "run-command.h"
      #include "commit.h"
     +#include "strvec.h"
    -+#include "tempfile.h"
      #include "trailer.h"
      #include "list.h"
    -+
    - /*
    -  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
    -  */
    + #include "tempfile.h"
     @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head,
      	free(cl_separators);
      }
      
    -+void validate_trailer_args(const struct strvec *cli_args)
    ++int validate_trailer_args(const struct strvec *cli_args)
     +{
     +	char *cl_separators;
    ++	int ret = 0;
     +
     +	trailer_config_init();
     +
    @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head
     +		const char *txt = cli_args->v[i];
     +		ssize_t separator_pos;
     +
    -+		if (!*txt)
    -+			die(_("empty --trailer argument"));
    -+
    ++		if (!*txt) {
    ++			ret = error(_("empty --trailer argument"));
    ++			goto out;
    ++		}
     +		separator_pos = find_separator(txt, cl_separators);
    -+		if (separator_pos == 0)
    -+			die(_("invalid trailer '%s': missing key before separator"),
    -+			    txt);
    ++		if (separator_pos == 0) {
    ++			ret = error(_("invalid trailer '%s': missing key before separator"),
    ++				    txt);
    ++			goto out;
    ++		}
     +	}
    -+
    ++out:
     +	free(cl_separators);
    ++	return ret;
     +}
     +
      static const char *next_line(const char *str)
      {
      	const char *nl = strchrnul(str, '\n');
    -@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
    - 	strbuf_release(&iter->key);
    +@@ trailer.c: struct tempfile *trailer_create_in_place_tempfile(const char *file)
    + 	return tempfile;
      }
      
     -int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)
    @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
     -		     path, NULL);
     -	strvec_pushv(&run_trailer.args, trailer_args->v);
     -	return run_command(&run_trailer);
    -+static void new_trailer_items_clear(struct list_head *items)
    -+{
    -+	while (!list_empty(items)) {
    -+		struct new_trailer_item *item =
    -+			list_first_entry(items, struct new_trailer_item, list);
    -+		list_del(&item->list);
    -+		free(item);
    -+	}
    -+}
    -+
    -+void amend_strbuf_with_trailers(struct strbuf *buf,
    ++int amend_strbuf_with_trailers(struct strbuf *buf,
     +				const struct strvec *trailer_args)
     +{
     +	struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
     +	LIST_HEAD(new_trailer_head);
     +	struct strbuf out = STRBUF_INIT;
     +	size_t i;
    ++	int ret = 0;
     +
     +	opts.no_divider = 1;
     +
     +	for (i = 0; i < trailer_args->nr; i++) {
     +		const char *text = trailer_args->v[i];
     +		struct new_trailer_item *item;
     +
    -+		if (!*text)
    -+			die(_("empty --trailer argument"));
    ++		if (!*text) {
    ++			ret = error(_("empty --trailer argument"));
    ++			goto out;
    ++		}
     +		item = xcalloc(1, sizeof(*item));
    -+		item->text = text;
    ++		item->text = xstrdup(text);
     +		list_add_tail(&item->list, &new_trailer_head);
     +	}
     +
    -+	trailer_config_init();
     +	process_trailers(&opts, &new_trailer_head, buf, &out);
     +
     +	strbuf_swap(buf, &out);
    ++out:
     +	strbuf_release(&out);
    ++	free_trailers(&new_trailer_head);
     +
    -+	new_trailer_items_clear(&new_trailer_head);
    ++	return ret;
     +}
     +
     +static int write_file_in_place(const char *path, const struct strbuf *buf)
     +{
    -+	struct stat st;
    -+	struct strbuf filename_template = STRBUF_INIT;
    -+	const char *tail;
    -+	struct tempfile *tempfile;
    -+	FILE *outfile;
    -+
    -+	if (stat(path, &st))
    -+		return error_errno(_("could not stat %s"), path);
    -+	if (!S_ISREG(st.st_mode))
    -+		return error(_("file %s is not a regular file"), path);
    -+	if (!(st.st_mode & S_IWUSR))
    -+		return error(_("file %s is not writable by user"), path);
    -+
    -+	/* Create temporary file in the same directory as the original */
    -+	tail = strrchr(path, '/');
    -+	if (tail)
    -+		strbuf_add(&filename_template, path, tail - path + 1);
    -+	strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
    -+
    -+	tempfile = mks_tempfile_sm(filename_template.buf, 0, st.st_mode);
    -+	strbuf_release(&filename_template);
    ++	struct tempfile *tempfile = trailer_create_in_place_tempfile(path);
     +	if (!tempfile)
    -+		return error_errno(_("could not create temporary file"));
    -+
    -+	outfile = fdopen_tempfile(tempfile, "w");
    -+	if (!outfile) {
    -+		int saved_errno = errno;
    -+		delete_tempfile(&tempfile);
    -+		errno = saved_errno;
    -+		return error_errno(_("could not open temporary file"));
    -+	}
    -+
    -+	if (buf->len && fwrite(buf->buf, 1, buf->len, outfile) < buf->len) {
    -+		int saved_errno = errno;
    -+		delete_tempfile(&tempfile);
    -+		errno = saved_errno;
    ++		return -1;
    ++
    ++	if (write_in_full(tempfile->fd, buf->buf, buf->len) < 0)
     +		return error_errno(_("could not write to temporary file"));
    -+	}
     +
     +	if (rename_tempfile(&tempfile, path))
     +		return error_errno(_("could not rename temporary file to %s"), path);
    @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
     +		 * in-process implementation.
     +		 */
     +		skip_prefix(txt, "--trailer=", &txt);
    -+		if (!*txt)
    -+			die(_("empty --trailer argument"));
    ++		if (!*txt) {
    ++			ret = error(_("empty --trailer argument"));
    ++			goto out;
    ++		}
     +		strvec_push(&stripped_trailer_args, txt);
     +	}
     +
    ++	if (validate_trailer_args(&stripped_trailer_args)) {
    ++		ret = -1;
    ++		goto out;
    ++	}
     +	if (strbuf_read_file(&buf, path, 0) < 0)
     +		ret = error_errno(_("could not read '%s'"), path);
     +	else
     +		amend_strbuf_with_trailers(&buf, &stripped_trailer_args);
     +
     +	if (!ret)
     +		ret = write_file_in_place(path, &buf);
    -+
    ++out:
     +	strvec_clear(&stripped_trailer_args);
     +	strbuf_release(&buf);
     +	return ret;
    @@ trailer.h: void parse_trailers_from_config(struct list_head *config_head);
      void parse_trailers_from_command_line_args(struct list_head *arg_head,
      					   struct list_head *new_trailer_head);
      
    -+void validate_trailer_args(const struct strvec *cli_args);
    ++int validate_trailer_args(const struct strvec *cli_args);
     +
      void process_trailers_lists(struct list_head *head,
      			    struct list_head *arg_head);
    @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);
     + * Each element of trailer_args should be in the same format as the value
     + * accepted by --trailer=<trailer> (i.e., without the --trailer= prefix).
     + */
    -+void amend_strbuf_with_trailers(struct strbuf *buf,
    ++int amend_strbuf_with_trailers(struct strbuf *buf,
     +				const struct strvec *trailer_args);
     +
     +/*
    @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);
       */
      int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
      
    -+/*
    -+ * Rewrite the contents of input by processing its trailer block according to
    -+ * opts and (optionally) appending trailers from new_trailer_head.
    -+ *
    -+ * The rewritten message is appended to out (callers should strbuf_reset()
    -+ * first if needed).
    -+ */
    - void process_trailers(const struct process_trailer_options *opts,
    - 		      struct list_head *new_trailer_head,
    - 		      struct strbuf *input, struct strbuf *out);
4:  147595a9317 ! 5:  3c1fa9e8579 commit, tag: parse --trailer with OPT_STRVEC
    @@ Commit message
         amend_file_with_trailers().
     
         Signed-off-by: Li Chen <me@linux.beauty>
     
      ## builtin/commit.c ##
     @@ builtin/commit.c: int cmd_commit(int argc,
    @@ trailer.c: int amend_file_with_trailers(const char *path,
     -		 * in-process implementation.
     -		 */
     -		skip_prefix(txt, "--trailer=", &txt);
    --		if (!*txt)
    --			die(_("empty --trailer argument"));
    +-		if (!*txt) {
    +-			ret = error(_("empty --trailer argument"));
    +-			goto out;
    +-		}
     -		strvec_push(&stripped_trailer_args, txt);
     -	}
     -
    +-	if (validate_trailer_args(&stripped_trailer_args)) {
    ++	if (validate_trailer_args(trailer_args)) {
    + 		ret = -1;
    + 		goto out;
    + 	}
      	if (strbuf_read_file(&buf, path, 0) < 0)
      		ret = error_errno(_("could not read '%s'"), path);
      	else
    @@ trailer.c: int amend_file_with_trailers(const char *path,
      
      	if (!ret)
      		ret = write_file_in_place(path, &buf);
    - 
    + out:
     -	strvec_clear(&stripped_trailer_args);
      	strbuf_release(&buf);
      	return ret;
      }
     
      ## trailer.h ##
    -@@ trailer.h: void amend_strbuf_with_trailers(struct strbuf *buf,
    +@@ trailer.h: int amend_strbuf_with_trailers(struct strbuf *buf,
      /*
       * Augment a file by appending trailers specified in trailer_args.
       *
5:  864cf5f8eb6 ! 6:  99654d80547 rebase: support --trailer
    @@ Commit message
         non-interactive and interactive rebases.
     
         Signed-off-by: Li Chen <me@linux.beauty>
    +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
     
      ## Documentation/git-rebase.adoc ##
     @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.
    @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.
     +--trailer=<trailer>::
     +	Append the given trailer to every rebased commit message, processed
     +	via linkgit:git-interpret-trailers[1]. This option implies
    -+	`--force-rebase` so that fast-forwarded commits are also rewritten.
    ++	`--force-rebase`.
     ++
     +See also INCOMPATIBLE OPTIONS below.
     +
      -i::
      --interactive::
      	Make a list of the commits which are about to be rebased.  Let the
    +@@ Documentation/git-rebase.adoc: are incompatible with the following options:
    +  * --[no-]reapply-cherry-picks when used without --keep-base
    +  * --update-refs
    +  * --root when used without --onto
    ++ * --trailer
    + 
    + In addition, the following pairs of options are incompatible:
    + 
     
      ## builtin/rebase.c ##
     @@
    @@ builtin/rebase.c: int cmd_rebase(int argc,
      			     builtin_rebase_usage, 0);
      
     +	if (options.trailer_args.nr) {
    -+		validate_trailer_args(&options.trailer_args);
    ++		if (validate_trailer_args(&options.trailer_args))
    ++			die(NULL);
     +		options.flags |= REBASE_FORCE;
     +	}
     +
    @@ sequencer.c: void replay_opts_release(struct replay_opts *opts)
      	free(opts->ctx);
      }
     @@ sequencer.c: static int append_squash_message(struct strbuf *buf, const char *body,
    + 	if (is_fixup_flag(command, flag) && !seen_squash(ctx)) {
    + 		/*
    + 		 * We're replacing the commit message so we need to
    +-		 * append the Signed-off-by: trailer if the user
    +-		 * requested '--signoff'.
    ++		 * append any trailers if the user requested
    ++		 * '--signoff' or '--trailer'.
    + 		 */
      		if (opts->signoff)
      			append_signoff(buf, 0, 0);
      
    @@ sequencer.c: static int do_pick_commit(struct repository *r,
      	if (is_rebase_i(opts) && write_author_script(msg.message) < 0)
      		res = -1;
      	else if (!opts->strategy ||
    +@@ sequencer.c: static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)
    + 	parse_strategy_opts(opts, buf->buf);
    + }
    + 
    ++static int read_trailers(struct replay_opts *opts, struct strbuf *buf)
    ++{
    ++	ssize_t len;
    ++
    ++	strbuf_reset(buf);
    ++	len = strbuf_read_file(buf, rebase_path_trailer(), 0);
    ++	if (len > 0) {
    ++		char *p = buf->buf, *nl;
    ++
    ++		trailer_config_init();
    ++
    ++		while ((nl = strchr(p, '\n'))) {
    ++			*nl = '\0';
    ++			if (!*p)
    ++				return error(_("trailers file contains empty line"));
    ++			strvec_push(&opts->trailer_args, p);
    ++			p = nl + 1;
    ++		}
    ++	} else if (!len) {
    ++		return error(_("trailers file is empty"));
    ++	} else if (errno != ENOENT) {
    ++		return error(_("cannot read trailers files"));
    ++	}
    ++
    ++	return 0;
    ++}
    ++
    + static int read_populate_opts(struct replay_opts *opts)
    + {
    + 	struct replay_ctx *ctx = opts->ctx;
     @@ sequencer.c: static int read_populate_opts(struct replay_opts *opts)
    + 			opts->keep_redundant_commits = 1;
      
      		read_strategy_opts(opts, &buf);
    ++
    ++		if (read_trailers(opts, &buf)) {
    ++			ret = -1;
    ++			goto done_rebase_i;
    ++		}
      		strbuf_reset(&buf);
    -+		if (strbuf_read_file(&buf, rebase_path_trailer(), 0) >= 0) {
    -+			char *p = buf.buf, *nl;
    -+
    -+			while ((nl = strchr(p, '\n'))) {
    -+				*nl = '\0';
    -+				if (!*p)
    -+					BUG("rebase-merge/trailer has an empty line");
    -+				strvec_push(&opts->trailer_args, p);
    -+				p = nl + 1;
    -+			}
    -+			strbuf_reset(&buf);
    -+		}
      
      		if (read_oneliner(&ctx->current_fixups,
    - 				  rebase_path_current_fixups(),
     @@ sequencer.c: int write_basic_state(struct replay_opts *opts, const char *head_name,
      		write_file(rebase_path_reschedule_failed_exec(), "%s", "");
      	else
-- 
2.52.0.362.g884e03848a9

[PATCH v8 1/6] interpret-trailers: factor trailer rewriting

[Phillip Wood]

From: Li Chen <me@linux.beauty>

Extract the trailer rewriting logic into a helper that appends to an
output strbuf.

Update interpret_trailers() to handle file I/O only: read input once,
call the helper, and write the buffered result.

This separation makes it easier to move the helper into trailer.c in the
next commit.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Li Chen <me@linux.beauty>
---
 builtin/interpret-trailers.c | 57 ++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 41b0750e5af..69f9d67ec0e 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -136,32 +136,21 @@ static void read_input_file(struct strbuf *sb, const char *file)
 	strbuf_complete_line(sb);
 }
 
-static void interpret_trailers(const struct process_trailer_options *opts,
-			       struct list_head *new_trailer_head,
-			       const char *file)
+static void process_trailers(const struct process_trailer_options *opts,
+			     struct list_head *new_trailer_head,
+			     struct strbuf *input, struct strbuf *out)
 {
 	LIST_HEAD(head);
-	struct strbuf sb = STRBUF_INIT;
-	struct strbuf trailer_block_sb = STRBUF_INIT;
 	struct trailer_block *trailer_block;
-	FILE *outfile = stdout;
-
-	trailer_config_init();
-
-	read_input_file(&sb, file);
-
-	if (opts->in_place)
-		outfile = create_in_place_tempfile(file);
-
-	trailer_block = parse_trailers(opts, sb.buf, &head);
+
+	trailer_block = parse_trailers(opts, input->buf, &head);
 
 	/* Print the lines before the trailer block */
 	if (!opts->only_trailers)
-		fwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);
+		strbuf_add(out, input->buf, trailer_block_start(trailer_block));
 
 	if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
-		fprintf(outfile, "\n");
-
+		strbuf_addch(out, '\n');
 
 	if (!opts->only_input) {
 		LIST_HEAD(config_head);
@@ -173,22 +162,40 @@ static void interpret_trailers(const struct process_trailer_options *opts,
 	}
 
 	/* Print trailer block. */
-	format_trailers(opts, &head, &trailer_block_sb);
+	format_trailers(opts, &head, out);
 	free_trailers(&head);
-	fwrite(trailer_block_sb.buf, 1, trailer_block_sb.len, outfile);
-	strbuf_release(&trailer_block_sb);
 
 	/* Print the lines after the trailer block as is. */
 	if (!opts->only_trailers)
-		fwrite(sb.buf + trailer_block_end(trailer_block), 1,
-		       sb.len - trailer_block_end(trailer_block), outfile);
+		strbuf_add(out, input->buf + trailer_block_end(trailer_block),
+			   input->len - trailer_block_end(trailer_block));
 	trailer_block_release(trailer_block);
-
+}
+
+static void interpret_trailers(const struct process_trailer_options *opts,
+			       struct list_head *new_trailer_head,
+			       const char *file)
+{
+	struct strbuf input = STRBUF_INIT;
+	struct strbuf out = STRBUF_INIT;
+	FILE *outfile = stdout;
+
+	trailer_config_init();
+
+	read_input_file(&input, file);
+
+	if (opts->in_place)
+		outfile = create_in_place_tempfile(file);
+
+	process_trailers(opts, new_trailer_head, &input, &out);
+
+	strbuf_write(&out, outfile);
 	if (opts->in_place)
 		if (rename_tempfile(&trailers_tempfile, file))
 			die_errno(_("could not rename temporary file to %s"), file);
 
-	strbuf_release(&sb);
+	strbuf_release(&input);
+	strbuf_release(&out);
 }
 
 int cmd_interpret_trailers(int argc,
-- 
2.52.0.362.g884e03848a9

[PATCH v8 2/6] interpret-trailers: refactor create_in_place_tempfile()

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Refactor create_in_place_tempfile() in preparation for moving it
to tralier.c. Change the return type to return a `struct tempfile*`
instead of a `FILE*` so that we can remove the file scope tempfile
variable. Since 076aa2cbda5 (tempfile: auto-allocate tempfiles on
heap, 2017-09-05) it has not been necessary to make tempfile varibales
static so this is safe. Also use error() and return NULL in place of
die() so the caller can exit gracefully and use find_last_dir_sep()
rather than strchr() to find the parent directory.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 builtin/interpret-trailers.c | 51 ++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 22 deletions(-)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 69f9d67ec0e..033c2e46713 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -93,35 +93,37 @@ static int parse_opt_parse(const struct option *opt, const char *arg,
 	return 0;
 }
 
-static struct tempfile *trailers_tempfile;
 
-static FILE *create_in_place_tempfile(const char *file)
+static struct tempfile *create_in_place_tempfile(const char *file)
 {
+	struct tempfile *tempfile = NULL;
 	struct stat st;
 	struct strbuf filename_template = STRBUF_INIT;
 	const char *tail;
-	FILE *outfile;
-
-	if (stat(file, &st))
-		die_errno(_("could not stat %s"), file);
-	if (!S_ISREG(st.st_mode))
-		die(_("file %s is not a regular file"), file);
-	if (!(st.st_mode & S_IWUSR))
-		die(_("file %s is not writable by user"), file);
 
+	if (stat(file, &st)) {
+		error_errno(_("could not stat %s"), file);
+		return NULL;
+	}
+	if (!S_ISREG(st.st_mode)) {
+		error(_("file %s is not a regular file"), file);
+		return NULL;
+	}
+	if (!(st.st_mode & S_IWUSR)) {
+		error(_("file %s is not writable by user"), file);
+		return NULL;
+	}
 	/* Create temporary file in the same directory as the original */
-	tail = strrchr(file, '/');
+	tail = find_last_dir_sep(file);
 	if (tail)
 		strbuf_add(&filename_template, file, tail - file + 1);
 	strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
 
-	trailers_tempfile = xmks_tempfile_m(filename_template.buf, st.st_mode);
+	tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);
+
 	strbuf_release(&filename_template);
-	outfile = fdopen_tempfile(trailers_tempfile, "w");
-	if (!outfile)
-		die_errno(_("could not open temporary file"));
 
-	return outfile;
+	return tempfile;
 }
 
 static void read_input_file(struct strbuf *sb, const char *file)
@@ -178,20 +180,25 @@ static void interpret_trailers(const struct process_trailer_options *opts,
 {
 	struct strbuf input = STRBUF_INIT;
 	struct strbuf out = STRBUF_INIT;
-	FILE *outfile = stdout;
+	struct tempfile *tempfile = NULL;
+	int fd = 1;
 
 	trailer_config_init();
 
 	read_input_file(&input, file);
 
-	if (opts->in_place)
-		outfile = create_in_place_tempfile(file);
-
+	if (opts->in_place) {
+		tempfile = create_in_place_tempfile(file);
+		if (!tempfile)
+			die(NULL);
+		fd = tempfile->fd;
+	}
 	process_trailers(opts, new_trailer_head, &input, &out);
 
-	strbuf_write(&out, outfile);
+	if (write_in_full(fd, out.buf, out.len) < 0)
+		die_errno(_("could not write to temporary file '%s'"), file);
 	if (opts->in_place)
-		if (rename_tempfile(&trailers_tempfile, file))
+		if (rename_tempfile(&tempfile, file))
 			die_errno(_("could not rename temporary file to %s"), file);
 
 	strbuf_release(&input);
-- 
2.52.0.362.g884e03848a9

[PATCH v8 3/6] trailer: libify a couple of functions

[Phillip Wood]

From: Li Chen <me@linux.beauty>

Move create_in_place_tempfile() and process_trailers() from
builtin/interpret-trailers.c into trailer.c and expose it via trailer.h.

This reverts most of ae0ec2e0e0b (trailer: move interpret_trailers()
to interpret-trailers.c, 2024-03-01) and lets other call sites reuse
the same trailer rewriting logic.

Signed-off-by: Li Chen <me@linux.beauty>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 builtin/interpret-trailers.c | 71 +-----------------------------------
 trailer.c                    | 70 +++++++++++++++++++++++++++++++++++
 trailer.h                    | 16 ++++++++
 3 files changed, 87 insertions(+), 70 deletions(-)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 033c2e46713..acaf42b2d93 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -93,39 +93,6 @@ static int parse_opt_parse(const struct option *opt, const char *arg,
 	return 0;
 }
 
-
-static struct tempfile *create_in_place_tempfile(const char *file)
-{
-	struct tempfile *tempfile = NULL;
-	struct stat st;
-	struct strbuf filename_template = STRBUF_INIT;
-	const char *tail;
-
-	if (stat(file, &st)) {
-		error_errno(_("could not stat %s"), file);
-		return NULL;
-	}
-	if (!S_ISREG(st.st_mode)) {
-		error(_("file %s is not a regular file"), file);
-		return NULL;
-	}
-	if (!(st.st_mode & S_IWUSR)) {
-		error(_("file %s is not writable by user"), file);
-		return NULL;
-	}
-	/* Create temporary file in the same directory as the original */
-	tail = find_last_dir_sep(file);
-	if (tail)
-		strbuf_add(&filename_template, file, tail - file + 1);
-	strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
-
-	tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);
-
-	strbuf_release(&filename_template);
-
-	return tempfile;
-}
-
 static void read_input_file(struct strbuf *sb, const char *file)
 {
 	if (file) {
@@ -138,42 +105,6 @@ static void read_input_file(struct strbuf *sb, const char *file)
 	strbuf_complete_line(sb);
 }
 
-static void process_trailers(const struct process_trailer_options *opts,
-			     struct list_head *new_trailer_head,
-			     struct strbuf *input, struct strbuf *out)
-{
-	LIST_HEAD(head);
-	struct trailer_block *trailer_block;
-
-	trailer_block = parse_trailers(opts, input->buf, &head);
-
-	/* Print the lines before the trailer block */
-	if (!opts->only_trailers)
-		strbuf_add(out, input->buf, trailer_block_start(trailer_block));
-
-	if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
-		strbuf_addch(out, '\n');
-
-	if (!opts->only_input) {
-		LIST_HEAD(config_head);
-		LIST_HEAD(arg_head);
-		parse_trailers_from_config(&config_head);
-		parse_trailers_from_command_line_args(&arg_head, new_trailer_head);
-		list_splice(&config_head, &arg_head);
-		process_trailers_lists(&head, &arg_head);
-	}
-
-	/* Print trailer block. */
-	format_trailers(opts, &head, out);
-	free_trailers(&head);
-
-	/* Print the lines after the trailer block as is. */
-	if (!opts->only_trailers)
-		strbuf_add(out, input->buf + trailer_block_end(trailer_block),
-			   input->len - trailer_block_end(trailer_block));
-	trailer_block_release(trailer_block);
-}
-
 static void interpret_trailers(const struct process_trailer_options *opts,
 			       struct list_head *new_trailer_head,
 			       const char *file)
@@ -188,7 +119,7 @@ static void interpret_trailers(const struct process_trailer_options *opts,
 	read_input_file(&input, file);
 
 	if (opts->in_place) {
-		tempfile = create_in_place_tempfile(file);
+		tempfile = trailer_create_in_place_tempfile(file);
 		if (!tempfile)
 			die(NULL);
 		fd = tempfile->fd;
diff --git a/trailer.c b/trailer.c
index 911a81ed993..163018483a5 100644
--- a/trailer.c
+++ b/trailer.c
@@ -9,6 +9,8 @@
 #include "commit.h"
 #include "trailer.h"
 #include "list.h"
+#include "tempfile.h"
+
 /*
  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
  */
@@ -1224,6 +1226,38 @@ void trailer_iterator_release(struct trailer_iterator *iter)
 	strbuf_release(&iter->key);
 }
 
+struct tempfile *trailer_create_in_place_tempfile(const char *file)
+{
+	struct tempfile *tempfile = NULL;
+	struct stat st;
+	struct strbuf filename_template = STRBUF_INIT;
+	const char *tail;
+
+	if (stat(file, &st)) {
+		error_errno(_("could not stat %s"), file);
+		return NULL;
+	}
+	if (!S_ISREG(st.st_mode)) {
+		error(_("file %s is not a regular file"), file);
+		return NULL;
+	}
+	if (!(st.st_mode & S_IWUSR)) {
+		error(_("file %s is not writable by user"), file);
+		return NULL;
+	}
+	/* Create temporary file in the same directory as the original */
+	tail = find_last_dir_sep(file);
+	if (tail)
+		strbuf_add(&filename_template, file, tail - file + 1);
+	strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
+
+	tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);
+
+	strbuf_release(&filename_template);
+
+	return tempfile;
+}
+
 int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)
 {
 	struct child_process run_trailer = CHILD_PROCESS_INIT;
@@ -1235,3 +1269,39 @@ int amend_file_with_trailers(const char *path, const struct strvec *trailer_args
 	strvec_pushv(&run_trailer.args, trailer_args->v);
 	return run_command(&run_trailer);
 }
+
+void process_trailers(const struct process_trailer_options *opts,
+		      struct list_head *new_trailer_head,
+		      struct strbuf *input, struct strbuf *out)
+{
+	LIST_HEAD(head);
+	struct trailer_block *trailer_block;
+
+	trailer_block = parse_trailers(opts, input->buf, &head);
+
+	/* Print the lines before the trailer block */
+	if (!opts->only_trailers)
+		strbuf_add(out, input->buf, trailer_block_start(trailer_block));
+
+	if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
+		strbuf_addch(out, '\n');
+
+	if (!opts->only_input) {
+		LIST_HEAD(config_head);
+		LIST_HEAD(arg_head);
+		parse_trailers_from_config(&config_head);
+		parse_trailers_from_command_line_args(&arg_head, new_trailer_head);
+		list_splice(&config_head, &arg_head);
+		process_trailers_lists(&head, &arg_head);
+	}
+
+	/* Print trailer block. */
+	format_trailers(opts, &head, out);
+	free_trailers(&head);
+
+	/* Print the lines after the trailer block as is. */
+	if (!opts->only_trailers)
+		strbuf_add(out, input->buf + trailer_block_end(trailer_block),
+			   input->len - trailer_block_end(trailer_block));
+	trailer_block_release(trailer_block);
+}
diff --git a/trailer.h b/trailer.h
index 4740549586a..7fd2564e035 100644
--- a/trailer.h
+++ b/trailer.h
@@ -202,4 +202,20 @@ void trailer_iterator_release(struct trailer_iterator *iter);
  */
 int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
 
+/*
+ * Create a tempfile ""git-interpret-trailers-XXXXXX" in the same
+ * directory as file.
+ */
+struct tempfile *trailer_create_in_place_tempfile(const char *file);
+
+/*
+ * Rewrite the contents of input by processing its trailer block according to
+ * opts and (optionally) appending trailers from new_trailer_head.
+ *
+ * The rewritten message is appended to out (callers should strbuf_reset()
+ * first if needed).
+ */
+void process_trailers(const struct process_trailer_options *opts,
+		      struct list_head *new_trailer_head,
+		      struct strbuf *input, struct strbuf *out);
 #endif /* TRAILER_H */
-- 
2.52.0.362.g884e03848a9

[PATCH v8 4/6] trailer: append trailers without fork/exec

[Phillip Wood]

From: Li Chen <me@linux.beauty>

Introduce amend_strbuf_with_trailers() to apply trailer additions to a
message buffer via process_trailers(), avoiding the need to run git
interpret-trailers as a child process.

Update amend_file_with_trailers() to use the in-process helper and
rewrite the target file via tempfile+rename, preserving the previous
in-place semantics. As the trailers are no longer added in a separate
process and trailer_config_init() die()s on missing config values it
is called early on in cmd_commit() and cmd_tag() so that they die()
early before writing the message file. The trailer arguments are now
also sanity checked.

Keep existing callers unchanged by continuing to accept argv-style
--trailer=<trailer> entries and stripping the prefix before feeding the
in-process implementation.

Signed-off-by: Li Chen <me@linux.beauty>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 builtin/commit.c |   3 ++
 builtin/tag.c    |   3 ++
 trailer.c        | 135 +++++++++++++++++++++++++++++++++++++++++++----
 trailer.h        |  20 +++++--
 4 files changed, 148 insertions(+), 13 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 9e3a09d532b..eb9013995c9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1820,6 +1820,9 @@ int cmd_commit(int argc,
 	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
 					  builtin_commit_usage,
 					  prefix, current_head, &s);
+	if (trailer_args.nr)
+		trailer_config_init();
+
 	if (verbose == -1)
 		verbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;
 
diff --git a/builtin/tag.c b/builtin/tag.c
index aeb04c487fe..68b581a9c26 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -568,6 +568,9 @@ int cmd_tag(int argc,
 	if (cmdmode == 'l')
 		setup_auto_pager("tag", 1);
 
+	if (trailer_args.nr)
+		trailer_config_init();
+
 	if (opt.sign == -1)
 		opt.sign = cmdmode ? 0 : config_sign_tag > 0;
 
diff --git a/trailer.c b/trailer.c
index 163018483a5..5eab4fa549d 100644
--- a/trailer.c
+++ b/trailer.c
@@ -7,6 +7,7 @@
 #include "string-list.h"
 #include "run-command.h"
 #include "commit.h"
+#include "strvec.h"
 #include "trailer.h"
 #include "list.h"
 #include "tempfile.h"
@@ -774,6 +775,35 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,
 	free(cl_separators);
 }
 
+int validate_trailer_args(const struct strvec *cli_args)
+{
+	char *cl_separators;
+	int ret = 0;
+
+	trailer_config_init();
+
+	cl_separators = xstrfmt("=%s", separators);
+
+	for (size_t i = 0; i < cli_args->nr; i++) {
+		const char *txt = cli_args->v[i];
+		ssize_t separator_pos;
+
+		if (!*txt) {
+			ret = error(_("empty --trailer argument"));
+			goto out;
+		}
+		separator_pos = find_separator(txt, cl_separators);
+		if (separator_pos == 0) {
+			ret = error(_("invalid trailer '%s': missing key before separator"),
+				    txt);
+			goto out;
+		}
+	}
+out:
+	free(cl_separators);
+	return ret;
+}
+
 static const char *next_line(const char *str)
 {
 	const char *nl = strchrnul(str, '\n');
@@ -1258,16 +1288,101 @@ struct tempfile *trailer_create_in_place_tempfile(const char *file)
 	return tempfile;
 }
 
-int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)
-{
-	struct child_process run_trailer = CHILD_PROCESS_INIT;
-
-	run_trailer.git_cmd = 1;
-	strvec_pushl(&run_trailer.args, "interpret-trailers",
-		     "--in-place", "--no-divider",
-		     path, NULL);
-	strvec_pushv(&run_trailer.args, trailer_args->v);
-	return run_command(&run_trailer);
+int amend_strbuf_with_trailers(struct strbuf *buf,
+				const struct strvec *trailer_args)
+{
+	struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
+	LIST_HEAD(new_trailer_head);
+	struct strbuf out = STRBUF_INIT;
+	size_t i;
+	int ret = 0;
+
+	opts.no_divider = 1;
+
+	for (i = 0; i < trailer_args->nr; i++) {
+		const char *text = trailer_args->v[i];
+		struct new_trailer_item *item;
+
+		if (!*text) {
+			ret = error(_("empty --trailer argument"));
+			goto out;
+		}
+		item = xcalloc(1, sizeof(*item));
+		item->text = xstrdup(text);
+		list_add_tail(&item->list, &new_trailer_head);
+	}
+
+	process_trailers(&opts, &new_trailer_head, buf, &out);
+
+	strbuf_swap(buf, &out);
+out:
+	strbuf_release(&out);
+	free_trailers(&new_trailer_head);
+
+	return ret;
+}
+
+static int write_file_in_place(const char *path, const struct strbuf *buf)
+{
+	struct tempfile *tempfile = trailer_create_in_place_tempfile(path);
+	if (!tempfile)
+		return -1;
+
+	if (write_in_full(tempfile->fd, buf->buf, buf->len) < 0)
+		return error_errno(_("could not write to temporary file"));
+
+	if (rename_tempfile(&tempfile, path))
+		return error_errno(_("could not rename temporary file to %s"), path);
+
+	return 0;
+}
+
+int amend_file_with_trailers(const char *path,
+			     const struct strvec *trailer_args)
+{
+	struct strbuf buf = STRBUF_INIT;
+	struct strvec stripped_trailer_args = STRVEC_INIT;
+	int ret = 0;
+	size_t i;
+
+	if (!trailer_args)
+		BUG("amend_file_with_trailers called with NULL trailer_args");
+	if (!trailer_args->nr)
+		return 0;
+
+	for (i = 0; i < trailer_args->nr; i++) {
+		const char *txt = trailer_args->v[i];
+
+		/*
+		 * Historically amend_file_with_trailers() passed its arguments
+		 * to "git interpret-trailers", which expected argv entries in
+		 * "--trailer=<trailer>" form. Continue to accept those for
+		 * existing callers, but pass only the value portion to the
+		 * in-process implementation.
+		 */
+		skip_prefix(txt, "--trailer=", &txt);
+		if (!*txt) {
+			ret = error(_("empty --trailer argument"));
+			goto out;
+		}
+		strvec_push(&stripped_trailer_args, txt);
+	}
+
+	if (validate_trailer_args(&stripped_trailer_args)) {
+		ret = -1;
+		goto out;
+	}
+	if (strbuf_read_file(&buf, path, 0) < 0)
+		ret = error_errno(_("could not read '%s'"), path);
+	else
+		amend_strbuf_with_trailers(&buf, &stripped_trailer_args);
+
+	if (!ret)
+		ret = write_file_in_place(path, &buf);
+out:
+	strvec_clear(&stripped_trailer_args);
+	strbuf_release(&buf);
+	return ret;
 }
 
 void process_trailers(const struct process_trailer_options *opts,
diff --git a/trailer.h b/trailer.h
index 7fd2564e035..3c5d9a6e199 100644
--- a/trailer.h
+++ b/trailer.h
@@ -68,6 +68,8 @@ void parse_trailers_from_config(struct list_head *config_head);
 void parse_trailers_from_command_line_args(struct list_head *arg_head,
 					   struct list_head *new_trailer_head);
 
+int validate_trailer_args(const struct strvec *cli_args);
+
 void process_trailers_lists(struct list_head *head,
 			    struct list_head *arg_head);
 
@@ -196,9 +198,21 @@ int trailer_iterator_advance(struct trailer_iterator *iter);
 void trailer_iterator_release(struct trailer_iterator *iter);
 
 /*
- * Augment a file to add trailers to it by running git-interpret-trailers.
- * This calls run_command() and its return value is the same (i.e. 0 for
- * success, various non-zero for other errors). See run-command.h.
+ * Append trailers specified in trailer_args to buf in-place.
+ *
+ * Each element of trailer_args should be in the same format as the value
+ * accepted by --trailer=<trailer> (i.e., without the --trailer= prefix).
+ */
+int amend_strbuf_with_trailers(struct strbuf *buf,
+				const struct strvec *trailer_args);
+
+/*
+ * Augment a file by appending trailers specified in trailer_args.
+ *
+ * Each element of trailer_args should be an argv-style --trailer=<trailer>
+ * option (i.e., including the --trailer= prefix).
+ *
+ * Returns 0 on success or a non-zero error code on failure.
  */
 int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
 
-- 
2.52.0.362.g884e03848a9

[PATCH v8 5/6] commit, tag: parse --trailer with OPT_STRVEC

[Phillip Wood]

From: Li Chen <me@linux.beauty>

Now that amend_file_with_trailers() expects raw trailer lines, do not
store argv-style "--trailer=<trailer>" strings in git commit and git
tag.

Parse --trailer using OPT_STRVEC so trailer_args contains only the
trailer value, and drop the temporary prefix stripping in
amend_file_with_trailers().

Signed-off-by: Li Chen <me@linux.beauty>
---
 builtin/commit.c |  3 ++-
 builtin/tag.c    |  4 ++--
 trailer.c        | 25 ++-----------------------
 trailer.h        |  4 ++--
 4 files changed, 8 insertions(+), 28 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index eb9013995c9..3d25c1856ce 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1720,7 +1720,8 @@ int cmd_commit(int argc,
 		OPT_STRING(0, "fixup", &fixup_message, N_("[(amend|reword):]commit"), N_("use autosquash formatted message to fixup or amend/reword specified commit")),
 		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
 		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
-		OPT_PASSTHRU_ARGV(0, "trailer", &trailer_args, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG),
+		OPT_STRVEC(0, "trailer", &trailer_args, N_("trailer"),
+			   N_("add custom trailer(s)")),
 		OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
 		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
 		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
diff --git a/builtin/tag.c b/builtin/tag.c
index 68b581a9c26..e0f05f94fdb 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -499,8 +499,8 @@ int cmd_tag(int argc,
 		OPT_CALLBACK_F('m', "message", &msg, N_("message"),
 			       N_("tag message"), PARSE_OPT_NONEG, parse_msg_arg),
 		OPT_FILENAME('F', "file", &msgfile, N_("read message from file")),
-		OPT_PASSTHRU_ARGV(0, "trailer", &trailer_args, N_("trailer"),
-				  N_("add custom trailer(s)"), PARSE_OPT_NONEG),
+		OPT_STRVEC(0, "trailer", &trailer_args, N_("trailer"),
+			   N_("add custom trailer(s)")),
 		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of tag message")),
 		OPT_BOOL('s', "sign", &opt.sign, N_("annotated and GPG-signed tag")),
 		OPT_CLEANUP(&cleanup_arg),
diff --git a/trailer.c b/trailer.c
index 5eab4fa549d..ca8abd18826 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1341,46 +1341,25 @@ int amend_file_with_trailers(const char *path,
 			     const struct strvec *trailer_args)
 {
 	struct strbuf buf = STRBUF_INIT;
-	struct strvec stripped_trailer_args = STRVEC_INIT;
 	int ret = 0;
-	size_t i;
 
 	if (!trailer_args)
 		BUG("amend_file_with_trailers called with NULL trailer_args");
 	if (!trailer_args->nr)
 		return 0;
 
-	for (i = 0; i < trailer_args->nr; i++) {
-		const char *txt = trailer_args->v[i];
-
-		/*
-		 * Historically amend_file_with_trailers() passed its arguments
-		 * to "git interpret-trailers", which expected argv entries in
-		 * "--trailer=<trailer>" form. Continue to accept those for
-		 * existing callers, but pass only the value portion to the
-		 * in-process implementation.
-		 */
-		skip_prefix(txt, "--trailer=", &txt);
-		if (!*txt) {
-			ret = error(_("empty --trailer argument"));
-			goto out;
-		}
-		strvec_push(&stripped_trailer_args, txt);
-	}
-
-	if (validate_trailer_args(&stripped_trailer_args)) {
+	if (validate_trailer_args(trailer_args)) {
 		ret = -1;
 		goto out;
 	}
 	if (strbuf_read_file(&buf, path, 0) < 0)
 		ret = error_errno(_("could not read '%s'"), path);
 	else
-		amend_strbuf_with_trailers(&buf, &stripped_trailer_args);
+		amend_strbuf_with_trailers(&buf, trailer_args);
 
 	if (!ret)
 		ret = write_file_in_place(path, &buf);
 out:
-	strvec_clear(&stripped_trailer_args);
 	strbuf_release(&buf);
 	return ret;
 }
diff --git a/trailer.h b/trailer.h
index 3c5d9a6e199..b49338858c4 100644
--- a/trailer.h
+++ b/trailer.h
@@ -209,8 +209,8 @@ int amend_strbuf_with_trailers(struct strbuf *buf,
 /*
  * Augment a file by appending trailers specified in trailer_args.
  *
- * Each element of trailer_args should be an argv-style --trailer=<trailer>
- * option (i.e., including the --trailer= prefix).
+ * Each element of trailer_args should be in the same format as the value
+ * accepted by --trailer=<trailer> (i.e., without the --trailer= prefix).
  *
  * Returns 0 on success or a non-zero error code on failure.
  */
-- 
2.52.0.362.g884e03848a9

[PATCH v8 6/6] rebase: support --trailer

[Phillip Wood]

From: Li Chen <me@linux.beauty>

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

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

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

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

Signed-off-by: Li Chen <me@linux.beauty>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 Documentation/git-rebase.adoc |   8 ++
 builtin/rebase.c              |  19 +++++
 sequencer.c                   |  52 +++++++++++-
 sequencer.h                   |   3 +
 t/meson.build                 |   1 +
 t/t3440-rebase-trailer.sh     | 147 ++++++++++++++++++++++++++++++++++
 6 files changed, 228 insertions(+), 2 deletions(-)
 create mode 100755 t/t3440-rebase-trailer.sh

diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc
index e177808004f..f6c22d15989 100644
--- a/Documentation/git-rebase.adoc
+++ b/Documentation/git-rebase.adoc
@@ -497,6 +497,13 @@ See also INCOMPATIBLE OPTIONS below.
 +
 See also INCOMPATIBLE OPTIONS below.
 
+--trailer=<trailer>::
+	Append the given trailer to every rebased commit message, processed
+	via linkgit:git-interpret-trailers[1]. This option implies
+	`--force-rebase`.
++
+See also INCOMPATIBLE OPTIONS below.
+
 -i::
 --interactive::
 	Make a list of the commits which are about to be rebased.  Let the
@@ -653,6 +660,7 @@ are incompatible with the following options:
  * --[no-]reapply-cherry-picks when used without --keep-base
  * --update-refs
  * --root when used without --onto
+ * --trailer
 
 In addition, the following pairs of options are incompatible:
 
diff --git a/builtin/rebase.c b/builtin/rebase.c
index c487e109077..fe25d2ad4bc 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -36,6 +36,7 @@
 #include "reset.h"
 #include "trace2.h"
 #include "hook.h"
+#include "trailer.h"
 
 static char const * const builtin_rebase_usage[] = {
 	N_("git rebase [-i] [options] [--exec <cmd>] "
@@ -113,6 +114,7 @@ struct rebase_options {
 	enum action action;
 	char *reflog_action;
 	int signoff;
+	struct strvec trailer_args;
 	int allow_rerere_autoupdate;
 	int keep_empty;
 	int autosquash;
@@ -143,6 +145,7 @@ struct rebase_options {
 		.flags = REBASE_NO_QUIET, 		\
 		.git_am_opts = STRVEC_INIT,		\
 		.exec = STRING_LIST_INIT_NODUP,		\
+		.trailer_args = STRVEC_INIT,		\
 		.git_format_patch_opt = STRBUF_INIT,	\
 		.fork_point = -1,			\
 		.reapply_cherry_picks = -1,             \
@@ -166,6 +169,7 @@ static void rebase_options_release(struct rebase_options *opts)
 	free(opts->strategy);
 	string_list_clear(&opts->strategy_opts, 0);
 	strbuf_release(&opts->git_format_patch_opt);
+	strvec_clear(&opts->trailer_args);
 }
 
 static struct replay_opts get_replay_opts(const struct rebase_options *opts)
@@ -177,6 +181,10 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)
 	sequencer_init_config(&replay);
 
 	replay.signoff = opts->signoff;
+
+	for (size_t i = 0; i < opts->trailer_args.nr; i++)
+		strvec_push(&replay.trailer_args, opts->trailer_args.v[i]);
+
 	replay.allow_ff = !(opts->flags & REBASE_FORCE);
 	if (opts->allow_rerere_autoupdate)
 		replay.allow_rerere_auto = opts->allow_rerere_autoupdate;
@@ -1132,6 +1140,8 @@ int cmd_rebase(int argc,
 			.flags = PARSE_OPT_NOARG,
 			.defval = REBASE_DIFFSTAT,
 		},
+		OPT_STRVEC(0, "trailer", &options.trailer_args, N_("trailer"),
+			   N_("add custom trailer(s)")),
 		OPT_BOOL(0, "signoff", &options.signoff,
 			 N_("add a Signed-off-by trailer to each commit")),
 		OPT_BOOL(0, "committer-date-is-author-date",
@@ -1285,6 +1295,12 @@ int cmd_rebase(int argc,
 			     builtin_rebase_options,
 			     builtin_rebase_usage, 0);
 
+	if (options.trailer_args.nr) {
+		if (validate_trailer_args(&options.trailer_args))
+			die(NULL);
+		options.flags |= REBASE_FORCE;
+	}
+
 	if (preserve_merges_selected)
 		die(_("--preserve-merges was replaced by --rebase-merges\n"
 			"Note: Your `pull.rebase` configuration may also be set to 'preserve',\n"
@@ -1542,6 +1558,9 @@ int cmd_rebase(int argc,
 	if (options.root && !options.onto_name)
 		imply_merge(&options, "--root without --onto");
 
+	if (options.trailer_args.nr)
+		imply_merge(&options, "--trailer");
+
 	if (isatty(2) && options.flags & REBASE_NO_QUIET)
 		strbuf_addstr(&options.git_format_patch_opt, " --progress");
 
diff --git a/sequencer.c b/sequencer.c
index a3eb39bb252..a2d72ce8b3b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -209,6 +209,7 @@ static GIT_PATH_FUNC(rebase_path_reschedule_failed_exec, "rebase-merge/reschedul
 static GIT_PATH_FUNC(rebase_path_no_reschedule_failed_exec, "rebase-merge/no-reschedule-failed-exec")
 static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, "rebase-merge/drop_redundant_commits")
 static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redundant_commits")
+static GIT_PATH_FUNC(rebase_path_trailer, "rebase-merge/trailer")
 
 /*
  * A 'struct replay_ctx' represents the private state of the sequencer.
@@ -420,6 +421,7 @@ void replay_opts_release(struct replay_opts *opts)
 	if (opts->revs)
 		release_revisions(opts->revs);
 	free(opts->revs);
+	strvec_clear(&opts->trailer_args);
 	replay_ctx_release(ctx);
 	free(opts->ctx);
 }
@@ -2019,12 +2021,15 @@ static int append_squash_message(struct strbuf *buf, const char *body,
 	if (is_fixup_flag(command, flag) && !seen_squash(ctx)) {
 		/*
 		 * We're replacing the commit message so we need to
-		 * append the Signed-off-by: trailer if the user
-		 * requested '--signoff'.
+		 * append any trailers if the user requested
+		 * '--signoff' or '--trailer'.
 		 */
 		if (opts->signoff)
 			append_signoff(buf, 0, 0);
 
+		if (opts->trailer_args.nr)
+			amend_strbuf_with_trailers(buf, &opts->trailer_args);
+
 		if ((command == TODO_FIXUP) &&
 		    (flag & TODO_REPLACE_FIXUP_MSG) &&
 		    (file_exists(rebase_path_fixup_msg()) ||
@@ -2443,6 +2448,9 @@ static int do_pick_commit(struct repository *r,
 	if (opts->signoff && !is_fixup(command))
 		append_signoff(&ctx->message, 0, 0);
 
+	if (opts->trailer_args.nr && !is_fixup(command))
+		amend_strbuf_with_trailers(&ctx->message, &opts->trailer_args);
+
 	if (is_rebase_i(opts) && write_author_script(msg.message) < 0)
 		res = -1;
 	else if (!opts->strategy ||
@@ -3172,6 +3180,33 @@ static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)
 	parse_strategy_opts(opts, buf->buf);
 }
 
+static int read_trailers(struct replay_opts *opts, struct strbuf *buf)
+{
+	ssize_t len;
+
+	strbuf_reset(buf);
+	len = strbuf_read_file(buf, rebase_path_trailer(), 0);
+	if (len > 0) {
+		char *p = buf->buf, *nl;
+
+		trailer_config_init();
+
+		while ((nl = strchr(p, '\n'))) {
+			*nl = '\0';
+			if (!*p)
+				return error(_("trailers file contains empty line"));
+			strvec_push(&opts->trailer_args, p);
+			p = nl + 1;
+		}
+	} else if (!len) {
+		return error(_("trailers file is empty"));
+	} else if (errno != ENOENT) {
+		return error(_("cannot read trailers files"));
+	}
+
+	return 0;
+}
+
 static int read_populate_opts(struct replay_opts *opts)
 {
 	struct replay_ctx *ctx = opts->ctx;
@@ -3233,6 +3268,11 @@ static int read_populate_opts(struct replay_opts *opts)
 			opts->keep_redundant_commits = 1;
 
 		read_strategy_opts(opts, &buf);
+
+		if (read_trailers(opts, &buf)) {
+			ret = -1;
+			goto done_rebase_i;
+		}
 		strbuf_reset(&buf);
 
 		if (read_oneliner(&ctx->current_fixups,
@@ -3328,6 +3368,14 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
 		write_file(rebase_path_reschedule_failed_exec(), "%s", "");
 	else
 		write_file(rebase_path_no_reschedule_failed_exec(), "%s", "");
+	if (opts->trailer_args.nr) {
+		struct strbuf buf = STRBUF_INIT;
+
+		for (size_t i = 0; i < opts->trailer_args.nr; i++)
+			strbuf_addf(&buf, "%s\n", opts->trailer_args.v[i]);
+		write_file(rebase_path_trailer(), "%s", buf.buf);
+		strbuf_release(&buf);
+	}
 
 	return 0;
 }
diff --git a/sequencer.h b/sequencer.h
index 719684c8a9f..bea20da085f 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -57,6 +57,8 @@ struct replay_opts {
 	int ignore_date;
 	int commit_use_reference;
 
+	struct strvec trailer_args;
+
 	int mainline;
 
 	char *gpg_sign;
@@ -84,6 +86,7 @@ struct replay_opts {
 #define REPLAY_OPTS_INIT {			\
 	.edit = -1,				\
 	.action = -1,				\
+	.trailer_args = STRVEC_INIT,		\
 	.xopts = STRVEC_INIT,			\
 	.ctx = replay_ctx_new(),		\
 }
diff --git a/t/meson.build b/t/meson.build
index 6d91470ebc1..3e0bb631af8 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -390,6 +390,7 @@ integration_tests = [
   't3436-rebase-more-options.sh',
   't3437-rebase-fixup-options.sh',
   't3438-rebase-broken-files.sh',
+  't3440-rebase-trailer.sh',
   't3450-history.sh',
   't3451-history-reword.sh',
   't3500-cherry.sh',
diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
new file mode 100755
index 00000000000..8b475795660
--- /dev/null
+++ b/t/t3440-rebase-trailer.sh
@@ -0,0 +1,147 @@
+#!/bin/sh
+#
+
+test_description='git rebase --trailer integration tests
+We verify that --trailer works with the merge backend,
+and that it is rejected early when the apply backend is requested.'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh # test_commit_message, helpers
+
+REVIEWED_BY_TRAILER="Reviewed-by: Dev <dev@example.com>"
+SP=" "
+
+test_expect_success 'setup repo with a small history' '
+	git commit --allow-empty -m "Initial empty commit" &&
+	test_commit first file a &&
+	test_commit second file &&
+	git checkout -b conflict-branch first &&
+	test_commit file-2 file-2 &&
+	test_commit conflict file &&
+	test_commit third file &&
+	git checkout main
+'
+
+test_expect_success 'apply backend is rejected with --trailer' '
+	git checkout -B apply-backend third &&
+	test_expect_code 128 \
+		git rebase --apply --trailer "$REVIEWED_BY_TRAILER" HEAD^ 2>err &&
+	test_grep "fatal: --trailer requires the merge backend" err
+'
+
+test_expect_success 'reject empty --trailer argument' '
+	git checkout -B empty-trailer third &&
+	test_expect_code 128 git rebase --trailer "" HEAD^ 2>err &&
+	test_grep "empty --trailer" err
+'
+
+test_expect_success 'reject trailer with missing key before separator' '
+	git checkout -B missing-key third &&
+	test_expect_code 128 git rebase --trailer ": no-key" HEAD^ 2>err &&
+	test_grep "missing key before separator" err
+'
+
+test_expect_success 'allow trailer with missing value after separator' '
+	git checkout -B missing-value third &&
+	git rebase --trailer "Acked-by:" HEAD^ &&
+	test_commit_message HEAD <<-EOF
+	third
+
+	Acked-by:${SP}
+	EOF
+'
+
+test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last' '
+	git checkout -B replace-policy third &&
+	git -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add \
+		rebase --trailer "Bug: 123" --trailer "Bug: 456" HEAD^ &&
+	test_commit_message HEAD <<-EOF
+	third
+
+	Bug: 456
+	EOF
+'
+
+test_expect_success 'multiple Signed-off-by trailers all preserved' '
+	git checkout -B multiple-signoff third &&
+	git rebase --trailer "Signed-off-by: Dev A <a@example.com>" \
+		--trailer "Signed-off-by: Dev B <b@example.com>" HEAD^ &&
+	test_commit_message HEAD <<-EOF
+	third
+
+	Signed-off-by: Dev A <a@example.com>
+	Signed-off-by: Dev B <b@example.com>
+	EOF
+'
+
+test_expect_success 'rebase --trailer adds trailer after conflicts' '
+	git checkout -B trailer-conflict third &&
+	test_commit fourth file &&
+	test_must_fail git rebase --trailer "$REVIEWED_BY_TRAILER" second &&
+	git checkout --theirs file &&
+	git add file &&
+	git rebase --continue &&
+	test_commit_message HEAD <<-EOF &&
+	fourth
+
+	$REVIEWED_BY_TRAILER
+	EOF
+	test_commit_message HEAD^ <<-EOF
+	third
+
+	$REVIEWED_BY_TRAILER
+	EOF
+'
+
+test_expect_success '--trailer handles fixup commands in todo list' '
+	git checkout -B fixup-trailer third &&
+	test_commit fixup-base base &&
+	test_commit fixup-second second &&
+	cat >todo <<-\EOF &&
+	pick fixup-base fixup-base
+	fixup fixup-second fixup-second
+	EOF
+	(
+		set_replace_editor todo &&
+		git rebase -i --trailer "$REVIEWED_BY_TRAILER" HEAD~2
+	) &&
+	test_commit_message HEAD <<-EOF &&
+	fixup-base
+
+	$REVIEWED_BY_TRAILER
+	EOF
+	git reset --hard fixup-second &&
+	cat >todo <<-\EOF &&
+	pick fixup-base fixup-base
+	fixup -C fixup-second fixup-second
+	EOF
+	(
+		set_replace_editor todo &&
+		git rebase -i --trailer "$REVIEWED_BY_TRAILER" HEAD~2
+	) &&
+	test_commit_message HEAD <<-EOF
+	fixup-second
+
+	$REVIEWED_BY_TRAILER
+	EOF
+'
+
+test_expect_success 'rebase --root honors trailer.<name>.key' '
+	git checkout -B root-trailer first &&
+	git -c trailer.review.key=Reviewed-by rebase --root \
+		--trailer=review="Dev <dev@example.com>" &&
+	test_commit_message HEAD <<-EOF &&
+	first
+
+	Reviewed-by: Dev <dev@example.com>
+	EOF
+	test_commit_message HEAD^ <<-EOF
+	Initial empty commit
+
+	Reviewed-by: Dev <dev@example.com>
+	EOF
+'
+test_done
-- 
2.52.0.362.g884e03848a9

Re: [PATCH v7 0/5] rebase: support --trailer

[Phillip Wood]

Hi Li

On 05/03/2026 13:49, Li Chen wrote:
>   >
>   > This all looks pretty good. The main thing I think we want to change is
>   > sharing the code that creates the temporary file in patch 3. I've push a
>   > version that does that and fixes my other small comments to
>   > https://github.com/phillipwood/git/commits/refs/heads/rebase-trailers-v8
>   > The range diff is below. If you're happy I can post that as a hopefully
>   > final v8. Of course if you want to work on it yourself you're very
>   > welcome to do that.
> 
> These changes all look good to me! Thank you very much for your patience and work! Please help
> post this as v8.

I've just posted v8, thanks for all your work on these patches

Phillip

> Regards,
> Li
> 
>   >
>   > 1:  b2685e34c22 ! 1:  0d08b361995 interpret-trailers: factor trailer rewriting
>   >      @@ Commit message
>   >           This separation makes it easier to move the helper into trailer.c in the
>   >           next commit.
>   >
>   >      +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>   >           Signed-off-by: Li Chen <me@linux.beauty>
>   >
>   >        ## builtin/interpret-trailers.c ##
>   >       @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)
>   > -:  ----------- > 2:  5a4d03ab375 interpret-trailers: refactor create_in_place_tempfile()
>   > 2:  1bac3025045 ! 3:  ab7e232a95d trailer: move process_trailers to trailer.h
>   >      @@ Metadata
>   >       Author: Li Chen <me@linux.beauty>
>   >
>   >        ## Commit message ##
>   >      -    trailer: move process_trailers to trailer.h
>   >      -
>   >      -    Move process_trailers() from builtin/interpret-trailers.c into trailer.c
>   >      -    and expose it via trailer.h.
>   >      -
>   >      -    This lets other call sites reuse the same trailer rewriting logic.
>   >      +    trailer: libify a couple of functions
>   >      +
>   >      +    Move create_in_place_tempfile() and process_trailers() from
>   >      +    builtin/interpret-trailers.c into trailer.c and expose it via trailer.h.
>   >      +
>   >      +    This reverts most of ae0ec2e0e0b (trailer: move interpret_trailers()
>   >      +    to interpret-trailers.c, 2024-03-01) and lets other call sites reuse
>   >      +    the same trailer rewriting logic.
>   >
>   >           Signed-off-by: Li Chen <me@linux.beauty>
>   >      +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>   >
>   >        ## builtin/interpret-trailers.c ##
>   >      +@@ builtin/interpret-trailers.c: static int parse_opt_parse(const struct option *opt, const char *arg,
>   >      +     return 0;
>   >      + }
>   >      +
>   >      +-
>   >      +-static struct tempfile *create_in_place_tempfile(const char *file)
>   >      +-{
>   >      +-    struct tempfile *tempfile = NULL;
>   >      +-    struct stat st;
>   >      +-    struct strbuf filename_template = STRBUF_INIT;
>   >      +-    const char *tail;
>   >      +-
>   >      +-    if (stat(file, &st)) {
>   >      +-        error_errno(_("could not stat %s"), file);
>   >      +-        return NULL;
>   >      +-    }
>   >      +-    if (!S_ISREG(st.st_mode)) {
>   >      +-        error(_("file %s is not a regular file"), file);
>   >      +-        return NULL;
>   >      +-    }
>   >      +-    if (!(st.st_mode & S_IWUSR)) {
>   >      +-        error(_("file %s is not writable by user"), file);
>   >      +-        return NULL;
>   >      +-    }
>   >      +-    /* Create temporary file in the same directory as the original */
>   >      +-    tail = find_last_dir_sep(file);
>   >      +-    if (tail)
>   >      +-        strbuf_add(&filename_template, file, tail - file + 1);
>   >      +-    strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
>   >      +-
>   >      +-    tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);
>   >      +-
>   >      +-    strbuf_release(&filename_template);
>   >      +-
>   >      +-    return tempfile;
>   >      +-}
>   >      +-
>   >      + static void read_input_file(struct strbuf *sb, const char *file)
>   >      + {
>   >      +     if (file) {
>   >       @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)
>   >            strbuf_complete_line(sb);
>   >        }
>   >      @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, con
>   >        static void interpret_trailers(const struct process_trailer_options *opts,
>   >                           struct list_head *new_trailer_head,
>   >                           const char *file)
>   >      +@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
>   >      +     read_input_file(&input, file);
>   >      +
>   >      +     if (opts->in_place) {
>   >      +-        tempfile = create_in_place_tempfile(file);
>   >      ++        tempfile = trailer_create_in_place_tempfile(file);
>   >      +         if (!tempfile)
>   >      +             die(NULL);
>   >      +         fd = tempfile->fd;
>   >
>   >        ## trailer.c ##
>   >      +@@
>   >      + #include "commit.h"
>   >      + #include "trailer.h"
>   >      + #include "list.h"
>   >      ++#include "tempfile.h"
>   >      ++
>   >      + /*
>   >      +  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
>   >      +  */
>   >      +@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
>   >      +     strbuf_release(&iter->key);
>   >      + }
>   >      +
>   >      ++struct tempfile *trailer_create_in_place_tempfile(const char *file)
>   >      ++{
>   >      ++    struct tempfile *tempfile = NULL;
>   >      ++    struct stat st;
>   >      ++    struct strbuf filename_template = STRBUF_INIT;
>   >      ++    const char *tail;
>   >      ++
>   >      ++    if (stat(file, &st)) {
>   >      ++        error_errno(_("could not stat %s"), file);
>   >      ++        return NULL;
>   >      ++    }
>   >      ++    if (!S_ISREG(st.st_mode)) {
>   >      ++        error(_("file %s is not a regular file"), file);
>   >      ++        return NULL;
>   >      ++    }
>   >      ++    if (!(st.st_mode & S_IWUSR)) {
>   >      ++        error(_("file %s is not writable by user"), file);
>   >      ++        return NULL;
>   >      ++    }
>   >      ++    /* Create temporary file in the same directory as the original */
>   >      ++    tail = find_last_dir_sep(file);
>   >      ++    if (tail)
>   >      ++        strbuf_add(&filename_template, file, tail - file + 1);
>   >      ++    strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
>   >      ++
>   >      ++    tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);
>   >      ++
>   >      ++    strbuf_release(&filename_template);
>   >      ++
>   >      ++    return tempfile;
>   >      ++}
>   >      ++
>   >      + int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)
>   >      + {
>   >      +     struct child_process run_trailer = CHILD_PROCESS_INIT;
>   >       @@ trailer.c: int amend_file_with_trailers(const char *path, const struct strvec *trailer_args
>   >            strvec_pushv(&run_trailer.args, trailer_args->v);
>   >            return run_command(&run_trailer);
>   >      @@ trailer.h: void trailer_iterator_release(struct trailer_iterator *iter);
>   >         */
>   >        int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
>   >
>   >      ++/*
>   >      ++ * Create a tempfile ""git-interpret-trailers-XXXXXX" in the same
>   >      ++ * directory as file.
>   >      ++ */
>   >      ++struct tempfile *trailer_create_in_place_tempfile(const char *file);
>   >      ++
>   >      ++/*
>   >      ++ * Rewrite the contents of input by processing its trailer block according to
>   >      ++ * opts and (optionally) appending trailers from new_trailer_head.
>   >      ++ *
>   >      ++ * The rewritten message is appended to out (callers should strbuf_reset()
>   >      ++ * first if needed).
>   >      ++ */
>   >       +void process_trailers(const struct process_trailer_options *opts,
>   >       +              struct list_head *new_trailer_head,
>   >       +              struct strbuf *input, struct strbuf *out);
>   > 3:  3114f0dbb57 ! 4:  1f24917eb64 trailer: append trailers without fork/exec
>   >      @@ Commit message
>   >
>   >           Update amend_file_with_trailers() to use the in-process helper and
>   >           rewrite the target file via tempfile+rename, preserving the previous
>   >      -    in-place semantics.
>   >      +    in-place semantics. As the trailers are no longer added in a separate
>   >      +    process and trailer_config_init() die()s on missing config values it
>   >      +    is called early on in cmd_commit() and cmd_tag() so that they die()
>   >      +    early before writing the message file. The trailer arguments are now
>   >      +    also sanity checked.
>   >
>   >           Keep existing callers unchanged by continuing to accept argv-style
>   >           --trailer=<trailer> entries and stripping the prefix before feeding the
>   >           in-process implementation.
>   >
>   >           Signed-off-by: Li Chen <me@linux.beauty>
>   >      +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>   >      -
>   >      - ## builtin/interpret-trailers.c ##
>   >      -@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
>   >      -     struct strbuf out = STRBUF_INIT;
>   >      -     FILE *outfile = stdout;
>   >      -
>   >      --    trailer_config_init();
>   >      --
>   >      -     read_input_file(&input, file);
>   >      -
>   >      -     if (opts->in_place)
>   >      -@@ builtin/interpret-trailers.c: int cmd_interpret_trailers(int argc,
>   >      -             git_interpret_trailers_usage,
>   >      -             options);
>   >      -
>   >      -+    trailer_config_init();
>   >      -+
>   >      -     if (argc) {
>   >      -         int i;
>   >      -         for (i = 0; i < argc; i++)
>   >      +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>   >      +
>   >      + ## builtin/commit.c ##
>   >      +@@ builtin/commit.c: int cmd_commit(int argc,
>   >      +     argc = parse_and_validate_options(argc, argv, builtin_commit_options,
>   >      +                       builtin_commit_usage,
>   >      +                       prefix, current_head, &s);
>   >      ++    if (trailer_args.nr)
>   >      ++        trailer_config_init();
>   >      ++
>   >      +     if (verbose == -1)
>   >      +         verbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;
>   >      +
>   >      +
>   >      + ## builtin/tag.c ##
>   >      +@@ builtin/tag.c: int cmd_tag(int argc,
>   >      +     if (cmdmode == 'l')
>   >      +         setup_auto_pager("tag", 1);
>   >      +
>   >      ++    if (trailer_args.nr)
>   >      ++        trailer_config_init();
>   >      ++
>   >      +     if (opt.sign == -1)
>   >      +         opt.sign = cmdmode ? 0 : config_sign_tag > 0;
>   >      +
>   >
>   >        ## trailer.c ##
>   >       @@
>   >        #include "string-list.h"
>   >        #include "run-command.h"
>   >        #include "commit.h"
>   >       +#include "strvec.h"
>   >      -+#include "tempfile.h"
>   >        #include "trailer.h"
>   >        #include "list.h"
>   >      -+
>   >      - /*
>   >      -  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
>   >      -  */
>   >      + #include "tempfile.h"
>   >       @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head,
>   >            free(cl_separators);
>   >        }
>   >
>   >      -+void validate_trailer_args(const struct strvec *cli_args)
>   >      ++int validate_trailer_args(const struct strvec *cli_args)
>   >       +{
>   >       +    char *cl_separators;
>   >      ++    int ret = 0;
>   >       +
>   >       +    trailer_config_init();
>   >       +
>   >      @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head
>   >       +        const char *txt = cli_args->v[i];
>   >       +        ssize_t separator_pos;
>   >       +
>   >      -+        if (!*txt)
>   >      -+            die(_("empty --trailer argument"));
>   >      -+
>   >      ++        if (!*txt) {
>   >      ++            ret = error(_("empty --trailer argument"));
>   >      ++            goto out;
>   >      ++        }
>   >       +        separator_pos = find_separator(txt, cl_separators);
>   >      -+        if (separator_pos == 0)
>   >      -+            die(_("invalid trailer '%s': missing key before separator"),
>   >      -+                txt);
>   >      ++        if (separator_pos == 0) {
>   >      ++            ret = error(_("invalid trailer '%s': missing key before separator"),
>   >      ++                    txt);
>   >      ++            goto out;
>   >      ++        }
>   >       +    }
>   >      -+
>   >      ++out:
>   >       +    free(cl_separators);
>   >      ++    return ret;
>   >       +}
>   >       +
>   >        static const char *next_line(const char *str)
>   >        {
>   >            const char *nl = strchrnul(str, '\n');
>   >      -@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
>   >      -     strbuf_release(&iter->key);
>   >      +@@ trailer.c: struct tempfile *trailer_create_in_place_tempfile(const char *file)
>   >      +     return tempfile;
>   >        }
>   >
>   >       -int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)
>   >      @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
>   >       -             path, NULL);
>   >       -    strvec_pushv(&run_trailer.args, trailer_args->v);
>   >       -    return run_command(&run_trailer);
>   >      -+static void new_trailer_items_clear(struct list_head *items)
>   >      -+{
>   >      -+    while (!list_empty(items)) {
>   >      -+        struct new_trailer_item *item =
>   >      -+            list_first_entry(items, struct new_trailer_item, list);
>   >      -+        list_del(&item->list);
>   >      -+        free(item);
>   >      -+    }
>   >      -+}
>   >      -+
>   >      -+void amend_strbuf_with_trailers(struct strbuf *buf,
>   >      ++int amend_strbuf_with_trailers(struct strbuf *buf,
>   >       +                const struct strvec *trailer_args)
>   >       +{
>   >       +    struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
>   >       +    LIST_HEAD(new_trailer_head);
>   >       +    struct strbuf out = STRBUF_INIT;
>   >       +    size_t i;
>   >      ++    int ret = 0;
>   >       +
>   >       +    opts.no_divider = 1;
>   >       +
>   >       +    for (i = 0; i < trailer_args->nr; i++) {
>   >       +        const char *text = trailer_args->v[i];
>   >       +        struct new_trailer_item *item;
>   >       +
>   >      -+        if (!*text)
>   >      -+            die(_("empty --trailer argument"));
>   >      ++        if (!*text) {
>   >      ++            ret = error(_("empty --trailer argument"));
>   >      ++            goto out;
>   >      ++        }
>   >       +        item = xcalloc(1, sizeof(*item));
>   >      -+        item->text = text;
>   >      ++        item->text = xstrdup(text);
>   >       +        list_add_tail(&item->list, &new_trailer_head);
>   >       +    }
>   >       +
>   >      -+    trailer_config_init();
>   >       +    process_trailers(&opts, &new_trailer_head, buf, &out);
>   >       +
>   >       +    strbuf_swap(buf, &out);
>   >      ++out:
>   >       +    strbuf_release(&out);
>   >      ++    free_trailers(&new_trailer_head);
>   >       +
>   >      -+    new_trailer_items_clear(&new_trailer_head);
>   >      ++    return ret;
>   >       +}
>   >       +
>   >       +static int write_file_in_place(const char *path, const struct strbuf *buf)
>   >       +{
>   >      -+    struct stat st;
>   >      -+    struct strbuf filename_template = STRBUF_INIT;
>   >      -+    const char *tail;
>   >      -+    struct tempfile *tempfile;
>   >      -+    FILE *outfile;
>   >      -+
>   >      -+    if (stat(path, &st))
>   >      -+        return error_errno(_("could not stat %s"), path);
>   >      -+    if (!S_ISREG(st.st_mode))
>   >      -+        return error(_("file %s is not a regular file"), path);
>   >      -+    if (!(st.st_mode & S_IWUSR))
>   >      -+        return error(_("file %s is not writable by user"), path);
>   >      -+
>   >      -+    /* Create temporary file in the same directory as the original */
>   >      -+    tail = strrchr(path, '/');
>   >      -+    if (tail)
>   >      -+        strbuf_add(&filename_template, path, tail - path + 1);
>   >      -+    strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
>   >      -+
>   >      -+    tempfile = mks_tempfile_sm(filename_template.buf, 0, st.st_mode);
>   >      -+    strbuf_release(&filename_template);
>   >      ++    struct tempfile *tempfile = trailer_create_in_place_tempfile(path);
>   >       +    if (!tempfile)
>   >      -+        return error_errno(_("could not create temporary file"));
>   >      -+
>   >      -+    outfile = fdopen_tempfile(tempfile, "w");
>   >      -+    if (!outfile) {
>   >      -+        int saved_errno = errno;
>   >      -+        delete_tempfile(&tempfile);
>   >      -+        errno = saved_errno;
>   >      -+        return error_errno(_("could not open temporary file"));
>   >      -+    }
>   >      -+
>   >      -+    if (buf->len && fwrite(buf->buf, 1, buf->len, outfile) < buf->len) {
>   >      -+        int saved_errno = errno;
>   >      -+        delete_tempfile(&tempfile);
>   >      -+        errno = saved_errno;
>   >      ++        return -1;
>   >      ++
>   >      ++    if (write_in_full(tempfile->fd, buf->buf, buf->len) < 0)
>   >       +        return error_errno(_("could not write to temporary file"));
>   >      -+    }
>   >       +
>   >       +    if (rename_tempfile(&tempfile, path))
>   >       +        return error_errno(_("could not rename temporary file to %s"), path);
>   >      @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
>   >       +         * in-process implementation.
>   >       +         */
>   >       +        skip_prefix(txt, "--trailer=", &txt);
>   >      -+        if (!*txt)
>   >      -+            die(_("empty --trailer argument"));
>   >      ++        if (!*txt) {
>   >      ++            ret = error(_("empty --trailer argument"));
>   >      ++            goto out;
>   >      ++        }
>   >       +        strvec_push(&stripped_trailer_args, txt);
>   >       +    }
>   >       +
>   >      ++    if (validate_trailer_args(&stripped_trailer_args)) {
>   >      ++        ret = -1;
>   >      ++        goto out;
>   >      ++    }
>   >       +    if (strbuf_read_file(&buf, path, 0) < 0)
>   >       +        ret = error_errno(_("could not read '%s'"), path);
>   >       +    else
>   >       +        amend_strbuf_with_trailers(&buf, &stripped_trailer_args);
>   >       +
>   >       +    if (!ret)
>   >       +        ret = write_file_in_place(path, &buf);
>   >      -+
>   >      ++out:
>   >       +    strvec_clear(&stripped_trailer_args);
>   >       +    strbuf_release(&buf);
>   >       +    return ret;
>   >      @@ trailer.h: void parse_trailers_from_config(struct list_head *config_head);
>   >        void parse_trailers_from_command_line_args(struct list_head *arg_head,
>   >                               struct list_head *new_trailer_head);
>   >
>   >      -+void validate_trailer_args(const struct strvec *cli_args);
>   >      ++int validate_trailer_args(const struct strvec *cli_args);
>   >       +
>   >        void process_trailers_lists(struct list_head *head,
>   >                        struct list_head *arg_head);
>   >      @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);
>   >       + * Each element of trailer_args should be in the same format as the value
>   >       + * accepted by --trailer=<trailer> (i.e., without the --trailer= prefix).
>   >       + */
>   >      -+void amend_strbuf_with_trailers(struct strbuf *buf,
>   >      ++int amend_strbuf_with_trailers(struct strbuf *buf,
>   >       +                const struct strvec *trailer_args);
>   >       +
>   >       +/*
>   >      @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);
>   >         */
>   >        int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
>   >
>   >      -+/*
>   >      -+ * Rewrite the contents of input by processing its trailer block according to
>   >      -+ * opts and (optionally) appending trailers from new_trailer_head.
>   >      -+ *
>   >      -+ * The rewritten message is appended to out (callers should strbuf_reset()
>   >      -+ * first if needed).
>   >      -+ */
>   >      - void process_trailers(const struct process_trailer_options *opts,
>   >      -               struct list_head *new_trailer_head,
>   >      -               struct strbuf *input, struct strbuf *out);
>   > 4:  147595a9317 ! 5:  3c1fa9e8579 commit, tag: parse --trailer with OPT_STRVEC
>   >      @@ Commit message
>   >           amend_file_with_trailers().
>   >
>   >           Signed-off-by: Li Chen <me@linux.beauty>
>   >
>   >        ## builtin/commit.c ##
>   >       @@ builtin/commit.c: int cmd_commit(int argc,
>   >      @@ trailer.c: int amend_file_with_trailers(const char *path,
>   >       -         * in-process implementation.
>   >       -         */
>   >       -        skip_prefix(txt, "--trailer=", &txt);
>   >      --        if (!*txt)
>   >      --            die(_("empty --trailer argument"));
>   >      +-        if (!*txt) {
>   >      +-            ret = error(_("empty --trailer argument"));
>   >      +-            goto out;
>   >      +-        }
>   >       -        strvec_push(&stripped_trailer_args, txt);
>   >       -    }
>   >       -
>   >      +-    if (validate_trailer_args(&stripped_trailer_args)) {
>   >      ++    if (validate_trailer_args(trailer_args)) {
>   >      +         ret = -1;
>   >      +         goto out;
>   >      +     }
>   >            if (strbuf_read_file(&buf, path, 0) < 0)
>   >                ret = error_errno(_("could not read '%s'"), path);
>   >            else
>   >      @@ trailer.c: int amend_file_with_trailers(const char *path,
>   >
>   >            if (!ret)
>   >                ret = write_file_in_place(path, &buf);
>   >      -
>   >      + out:
>   >       -    strvec_clear(&stripped_trailer_args);
>   >            strbuf_release(&buf);
>   >            return ret;
>   >        }
>   >
>   >        ## trailer.h ##
>   >      -@@ trailer.h: void amend_strbuf_with_trailers(struct strbuf *buf,
>   >      +@@ trailer.h: int amend_strbuf_with_trailers(struct strbuf *buf,
>   >        /*
>   >         * Augment a file by appending trailers specified in trailer_args.
>   >         *
>   > 5:  864cf5f8eb6 ! 6:  99654d80547 rebase: support --trailer
>   >      @@ Commit message
>   >           non-interactive and interactive rebases.
>   >
>   >           Signed-off-by: Li Chen <me@linux.beauty>
>   >      +    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>   >
>   >        ## Documentation/git-rebase.adoc ##
>   >       @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.
>   >      @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below.
>   >       +--trailer=<trailer>::
>   >       +    Append the given trailer to every rebased commit message, processed
>   >       +    via linkgit:git-interpret-trailers[1]. This option implies
>   >      -+    `--force-rebase` so that fast-forwarded commits are also rewritten.
>   >      ++    `--force-rebase`.
>   >       ++
>   >       +See also INCOMPATIBLE OPTIONS below.
>   >       +
>   >        -i::
>   >        --interactive::
>   >            Make a list of the commits which are about to be rebased.  Let the
>   >      +@@ Documentation/git-rebase.adoc: are incompatible with the following options:
>   >      +  * --[no-]reapply-cherry-picks when used without --keep-base
>   >      +  * --update-refs
>   >      +  * --root when used without --onto
>   >      ++ * --trailer
>   >      +
>   >      + In addition, the following pairs of options are incompatible:
>   >      +
>   >
>   >        ## builtin/rebase.c ##
>   >       @@
>   >      @@ builtin/rebase.c: int cmd_rebase(int argc,
>   >                         builtin_rebase_usage, 0);
>   >
>   >       +    if (options.trailer_args.nr) {
>   >      -+        validate_trailer_args(&options.trailer_args);
>   >      ++        if (validate_trailer_args(&options.trailer_args))
>   >      ++            die(NULL);
>   >       +        options.flags |= REBASE_FORCE;
>   >       +    }
>   >       +
>   >      @@ sequencer.c: void replay_opts_release(struct replay_opts *opts)
>   >            free(opts->ctx);
>   >        }
>   >       @@ sequencer.c: static int append_squash_message(struct strbuf *buf, const char *body,
>   >      +     if (is_fixup_flag(command, flag) && !seen_squash(ctx)) {
>   >      +         /*
>   >      +          * We're replacing the commit message so we need to
>   >      +-         * append the Signed-off-by: trailer if the user
>   >      +-         * requested '--signoff'.
>   >      ++         * append any trailers if the user requested
>   >      ++         * '--signoff' or '--trailer'.
>   >      +          */
>   >                if (opts->signoff)
>   >                    append_signoff(buf, 0, 0);
>   >
>   >      @@ sequencer.c: static int do_pick_commit(struct repository *r,
>   >            if (is_rebase_i(opts) && write_author_script(msg.message) < 0)
>   >                res = -1;
>   >            else if (!opts->strategy ||
>   >      +@@ sequencer.c: static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)
>   >      +     parse_strategy_opts(opts, buf->buf);
>   >      + }
>   >      +
>   >      ++static int read_trailers(struct replay_opts *opts, struct strbuf *buf)
>   >      ++{
>   >      ++    ssize_t len;
>   >      ++
>   >      ++    strbuf_reset(buf);
>   >      ++    len = strbuf_read_file(buf, rebase_path_trailer(), 0);
>   >      ++    if (len > 0) {
>   >      ++        char *p = buf->buf, *nl;
>   >      ++
>   >      ++        trailer_config_init();
>   >      ++
>   >      ++        while ((nl = strchr(p, '\n'))) {
>   >      ++            *nl = '\0';
>   >      ++            if (!*p)
>   >      ++                return error(_("trailers file contains empty line"));
>   >      ++            strvec_push(&opts->trailer_args, p);
>   >      ++            p = nl + 1;
>   >      ++        }
>   >      ++    } else if (!len) {
>   >      ++        return error(_("trailers file is empty"));
>   >      ++    } else if (errno != ENOENT) {
>   >      ++        return error(_("cannot read trailers files"));
>   >      ++    }
>   >      ++
>   >      ++    return 0;
>   >      ++}
>   >      ++
>   >      + static int read_populate_opts(struct replay_opts *opts)
>   >      + {
>   >      +     struct replay_ctx *ctx = opts->ctx;
>   >       @@ sequencer.c: static int read_populate_opts(struct replay_opts *opts)
>   >      +             opts->keep_redundant_commits = 1;
>   >
>   >                read_strategy_opts(opts, &buf);
>   >      ++
>   >      ++        if (read_trailers(opts, &buf)) {
>   >      ++            ret = -1;
>   >      ++            goto done_rebase_i;
>   >      ++        }
>   >                strbuf_reset(&buf);
>   >      -+        if (strbuf_read_file(&buf, rebase_path_trailer(), 0) >= 0) {
>   >      -+            char *p = buf.buf, *nl;
>   >      -+
>   >      -+            while ((nl = strchr(p, '\n'))) {
>   >      -+                *nl = '\0';
>   >      -+                if (!*p)
>   >      -+                    BUG("rebase-merge/trailer has an empty line");
>   >      -+                strvec_push(&opts->trailer_args, p);
>   >      -+                p = nl + 1;
>   >      -+            }
>   >      -+            strbuf_reset(&buf);
>   >      -+        }
>   >
>   >                if (read_oneliner(&ctx->current_fixups,
>   >      -                   rebase_path_current_fixups(),
>   >       @@ sequencer.c: int write_basic_state(struct replay_opts *opts, const char *head_name,
>   >                write_file(rebase_path_reschedule_failed_exec(), "%s", "");
>   >            else
>   >
>   >
> 

Re: [PATCH v8 1/6] interpret-trailers: factor trailer rewriting

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> From: Li Chen <me@linux.beauty>
>
> Extract the trailer rewriting logic into a helper that appends to an
> output strbuf.
>
> Update interpret_trailers() to handle file I/O only: read input once,
> call the helper, and write the buffered result.
>
> This separation makes it easier to move the helper into trailer.c in the
> next commit.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> Signed-off-by: Li Chen <me@linux.beauty>
> ---

If "Li wrote it and signed it off, Phillip is relaying" is what is
happening, the sign-offs are in reverse order.  If "Li incorporated
what Phillip wrote earlier in his larger work", then the sign-offs
may be fine, but it would be necessary to hint that that is what
happened in the proposed log message.  I cannot quite tell which is
the case.

>  builtin/interpret-trailers.c | 57 ++++++++++++++++++++----------------
>  1 file changed, 32 insertions(+), 25 deletions(-)
>
> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
> index 41b0750e5af..69f9d67ec0e 100644
> --- a/builtin/interpret-trailers.c
> +++ b/builtin/interpret-trailers.c
> @@ -136,32 +136,21 @@ static void read_input_file(struct strbuf *sb, const char *file)
>  	strbuf_complete_line(sb);
>  }
>  
> -static void interpret_trailers(const struct process_trailer_options *opts,
> -			       struct list_head *new_trailer_head,
> -			       const char *file)
> +static void process_trailers(const struct process_trailer_options *opts,
> +			     struct list_head *new_trailer_head,
> +			     struct strbuf *input, struct strbuf *out)
>  {
>  	LIST_HEAD(head);
> -	struct strbuf sb = STRBUF_INIT;
> -	struct strbuf trailer_block_sb = STRBUF_INIT;
>  	struct trailer_block *trailer_block;
> -	FILE *outfile = stdout;
> -
> -	trailer_config_init();
> -
> -	read_input_file(&sb, file);
> -
> -	if (opts->in_place)
> -		outfile = create_in_place_tempfile(file);
> -
> -	trailer_block = parse_trailers(opts, sb.buf, &head);
> +
> +	trailer_block = parse_trailers(opts, input->buf, &head);
>  
>  	/* Print the lines before the trailer block */
>  	if (!opts->only_trailers)
> -		fwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);
> +		strbuf_add(out, input->buf, trailer_block_start(trailer_block));
>  
>  	if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
> -		fprintf(outfile, "\n");
> -
> +		strbuf_addch(out, '\n');
>  
>  	if (!opts->only_input) {
>  		LIST_HEAD(config_head);
> @@ -173,22 +162,40 @@ static void interpret_trailers(const struct process_trailer_options *opts,
>  	}
>  
>  	/* Print trailer block. */
> -	format_trailers(opts, &head, &trailer_block_sb);
> +	format_trailers(opts, &head, out);
>  	free_trailers(&head);
> -	fwrite(trailer_block_sb.buf, 1, trailer_block_sb.len, outfile);
> -	strbuf_release(&trailer_block_sb);
>  
>  	/* Print the lines after the trailer block as is. */
>  	if (!opts->only_trailers)
> -		fwrite(sb.buf + trailer_block_end(trailer_block), 1,
> -		       sb.len - trailer_block_end(trailer_block), outfile);
> +		strbuf_add(out, input->buf + trailer_block_end(trailer_block),
> +			   input->len - trailer_block_end(trailer_block));
>  	trailer_block_release(trailer_block);
> -
> +}
> +
> +static void interpret_trailers(const struct process_trailer_options *opts,
> +			       struct list_head *new_trailer_head,
> +			       const char *file)
> +{
> +	struct strbuf input = STRBUF_INIT;
> +	struct strbuf out = STRBUF_INIT;
> +	FILE *outfile = stdout;
> +
> +	trailer_config_init();
> +
> +	read_input_file(&input, file);
> +
> +	if (opts->in_place)
> +		outfile = create_in_place_tempfile(file);
> +
> +	process_trailers(opts, new_trailer_head, &input, &out);
> +
> +	strbuf_write(&out, outfile);
>  	if (opts->in_place)
>  		if (rename_tempfile(&trailers_tempfile, file))
>  			die_errno(_("could not rename temporary file to %s"), file);
>  
> -	strbuf_release(&sb);
> +	strbuf_release(&input);
> +	strbuf_release(&out);
>  }
>  
>  int cmd_interpret_trailers(int argc,

Re: [PATCH v8 2/6] interpret-trailers: refactor create_in_place_tempfile()

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Refactor create_in_place_tempfile() in preparation for moving it
> to tralier.c. Change the return type to return a `struct tempfile*`
> instead of a `FILE*` so that we can remove the file scope tempfile
> variable. Since 076aa2cbda5 (tempfile: auto-allocate tempfiles on
> heap, 2017-09-05) it has not been necessary to make tempfile varibales
> static so this is safe. Also use error() and return NULL in place of
> die() so the caller can exit gracefully and use find_last_dir_sep()
> rather than strchr() to find the parent directory.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  builtin/interpret-trailers.c | 51 ++++++++++++++++++++----------------
>  1 file changed, 29 insertions(+), 22 deletions(-)

Yes, this organization is much nicer.  Thanks for cleaning it up.

>
> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
> index 69f9d67ec0e..033c2e46713 100644
> --- a/builtin/interpret-trailers.c
> +++ b/builtin/interpret-trailers.c
> @@ -93,35 +93,37 @@ static int parse_opt_parse(const struct option *opt, const char *arg,
>  	return 0;
>  }
>  
> -static struct tempfile *trailers_tempfile;
>  
> -static FILE *create_in_place_tempfile(const char *file)
> +static struct tempfile *create_in_place_tempfile(const char *file)
>  {
> +	struct tempfile *tempfile = NULL;
>  	struct stat st;
>  	struct strbuf filename_template = STRBUF_INIT;
>  	const char *tail;
> -	FILE *outfile;
> -
> -	if (stat(file, &st))
> -		die_errno(_("could not stat %s"), file);
> -	if (!S_ISREG(st.st_mode))
> -		die(_("file %s is not a regular file"), file);
> -	if (!(st.st_mode & S_IWUSR))
> -		die(_("file %s is not writable by user"), file);
>  
> +	if (stat(file, &st)) {
> +		error_errno(_("could not stat %s"), file);
> +		return NULL;
> +	}
> +	if (!S_ISREG(st.st_mode)) {
> +		error(_("file %s is not a regular file"), file);
> +		return NULL;
> +	}
> +	if (!(st.st_mode & S_IWUSR)) {
> +		error(_("file %s is not writable by user"), file);
> +		return NULL;
> +	}
>  	/* Create temporary file in the same directory as the original */
> -	tail = strrchr(file, '/');
> +	tail = find_last_dir_sep(file);
>  	if (tail)
>  		strbuf_add(&filename_template, file, tail - file + 1);
>  	strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
>  
> -	trailers_tempfile = xmks_tempfile_m(filename_template.buf, st.st_mode);
> +	tempfile = mks_tempfile_m(filename_template.buf, st.st_mode);
> +
>  	strbuf_release(&filename_template);
> -	outfile = fdopen_tempfile(trailers_tempfile, "w");
> -	if (!outfile)
> -		die_errno(_("could not open temporary file"));
>  
> -	return outfile;
> +	return tempfile;
>  }
>  
>  static void read_input_file(struct strbuf *sb, const char *file)
> @@ -178,20 +180,25 @@ static void interpret_trailers(const struct process_trailer_options *opts,
>  {
>  	struct strbuf input = STRBUF_INIT;
>  	struct strbuf out = STRBUF_INIT;
> -	FILE *outfile = stdout;
> +	struct tempfile *tempfile = NULL;
> +	int fd = 1;
>  
>  	trailer_config_init();
>  
>  	read_input_file(&input, file);
>  
> -	if (opts->in_place)
> -		outfile = create_in_place_tempfile(file);
> -
> +	if (opts->in_place) {
> +		tempfile = create_in_place_tempfile(file);
> +		if (!tempfile)
> +			die(NULL);
> +		fd = tempfile->fd;
> +	}
>  	process_trailers(opts, new_trailer_head, &input, &out);
>  
> -	strbuf_write(&out, outfile);
> +	if (write_in_full(fd, out.buf, out.len) < 0)
> +		die_errno(_("could not write to temporary file '%s'"), file);
>  	if (opts->in_place)
> -		if (rename_tempfile(&trailers_tempfile, file))
> +		if (rename_tempfile(&tempfile, file))
>  			die_errno(_("could not rename temporary file to %s"), file);
>  
>  	strbuf_release(&input);

Re: [PATCH v8 1/6] interpret-trailers: factor trailer rewriting

[Phillip Wood]

On 06/03/2026 21:04, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> From: Li Chen <me@linux.beauty>
>>
>> Extract the trailer rewriting logic into a helper that appends to an
>> output strbuf.
>>
>> Update interpret_trailers() to handle file I/O only: read input once,
>> call the helper, and write the buffered result.
>>
>> This separation makes it easier to move the helper into trailer.c in the
>> next commit.
>>
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> Signed-off-by: Li Chen <me@linux.beauty>
>> ---
> 
> If "Li wrote it and signed it off, Phillip is relaying" is what is
> happening, the sign-offs are in reverse order.  If "Li incorporated
> what Phillip wrote earlier in his larger work", then the sign-offs
> may be fine, but it would be necessary to hint that that is what
> happened in the proposed log message.  I cannot quite tell which is
> the case.

It's the latter, I posted a diff[1] and Li based this patch on that diff

Thanks

Phillip

[1] 
https://lore.kernel.org/git/7d12b046-365f-441c-af8e-8a39d61efbbd@gmail.com/
>>   builtin/interpret-trailers.c | 57 ++++++++++++++++++++----------------
>>   1 file changed, 32 insertions(+), 25 deletions(-)
>>
>> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
>> index 41b0750e5af..69f9d67ec0e 100644
>> --- a/builtin/interpret-trailers.c
>> +++ b/builtin/interpret-trailers.c
>> @@ -136,32 +136,21 @@ static void read_input_file(struct strbuf *sb, const char *file)
>>   	strbuf_complete_line(sb);
>>   }
>>   
>> -static void interpret_trailers(const struct process_trailer_options *opts,
>> -			       struct list_head *new_trailer_head,
>> -			       const char *file)
>> +static void process_trailers(const struct process_trailer_options *opts,
>> +			     struct list_head *new_trailer_head,
>> +			     struct strbuf *input, struct strbuf *out)
>>   {
>>   	LIST_HEAD(head);
>> -	struct strbuf sb = STRBUF_INIT;
>> -	struct strbuf trailer_block_sb = STRBUF_INIT;
>>   	struct trailer_block *trailer_block;
>> -	FILE *outfile = stdout;
>> -
>> -	trailer_config_init();
>> -
>> -	read_input_file(&sb, file);
>> -
>> -	if (opts->in_place)
>> -		outfile = create_in_place_tempfile(file);
>> -
>> -	trailer_block = parse_trailers(opts, sb.buf, &head);
>> +
>> +	trailer_block = parse_trailers(opts, input->buf, &head);
>>   
>>   	/* Print the lines before the trailer block */
>>   	if (!opts->only_trailers)
>> -		fwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);
>> +		strbuf_add(out, input->buf, trailer_block_start(trailer_block));
>>   
>>   	if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
>> -		fprintf(outfile, "\n");
>> -
>> +		strbuf_addch(out, '\n');
>>   
>>   	if (!opts->only_input) {
>>   		LIST_HEAD(config_head);
>> @@ -173,22 +162,40 @@ static void interpret_trailers(const struct process_trailer_options *opts,
>>   	}
>>   
>>   	/* Print trailer block. */
>> -	format_trailers(opts, &head, &trailer_block_sb);
>> +	format_trailers(opts, &head, out);
>>   	free_trailers(&head);
>> -	fwrite(trailer_block_sb.buf, 1, trailer_block_sb.len, outfile);
>> -	strbuf_release(&trailer_block_sb);
>>   
>>   	/* Print the lines after the trailer block as is. */
>>   	if (!opts->only_trailers)
>> -		fwrite(sb.buf + trailer_block_end(trailer_block), 1,
>> -		       sb.len - trailer_block_end(trailer_block), outfile);
>> +		strbuf_add(out, input->buf + trailer_block_end(trailer_block),
>> +			   input->len - trailer_block_end(trailer_block));
>>   	trailer_block_release(trailer_block);
>> -
>> +}
>> +
>> +static void interpret_trailers(const struct process_trailer_options *opts,
>> +			       struct list_head *new_trailer_head,
>> +			       const char *file)
>> +{
>> +	struct strbuf input = STRBUF_INIT;
>> +	struct strbuf out = STRBUF_INIT;
>> +	FILE *outfile = stdout;
>> +
>> +	trailer_config_init();
>> +
>> +	read_input_file(&input, file);
>> +
>> +	if (opts->in_place)
>> +		outfile = create_in_place_tempfile(file);
>> +
>> +	process_trailers(opts, new_trailer_head, &input, &out);
>> +
>> +	strbuf_write(&out, outfile);
>>   	if (opts->in_place)
>>   		if (rename_tempfile(&trailers_tempfile, file))
>>   			die_errno(_("could not rename temporary file to %s"), file);
>>   
>> -	strbuf_release(&sb);
>> +	strbuf_release(&input);
>> +	strbuf_release(&out);
>>   }
>>   
>>   int cmd_interpret_trailers(int argc,