public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 00/29] rebase: support --trailer
@ 2025-10-22  5:39 Li Chen
  2025-10-22  5:39 ` [PATCH v5 01/29] trailer: append trailers in-process and drop the fork to `interpret-trailers` Li Chen
                   ` (28 more replies)
  0 siblings, 29 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

From: Li Chen <chenl311@chinatelecom.cn>

This patch series teaches git rebase a new
--trailer <text> option and, as a prerequisite, moves all trailer
handling out of the external interpret-trailers helper and into the
builtin code path, as suggested by Phillip Wood.

Patch 0 switches trailer.c to an in-memory implementation
(amend_strbuf_with_trailers()). It removes every fork/exec.

Patch 1~8 fix all reviewer comments from v3 for patch 0. 

Patch 9 builds on that helper to implement
git rebase --trailer. When the option is given we:
force the merge backend (apply/am backend lacks a message filter),
automatically enable --force-rebase so that fast-forwarded
commits are rewritten, and append the requested trailer(s) to every
rewritten commit.
State is stored in $state_dir/trailer so an interrupted rebase can
resume safely. A dedicated test-suite (t3440) exercises plain,
conflict, --root, invalid-input scenarios and etc.

The rest patches address all reviewer comments from v3 for patch 9. 

All t/*.sh testcases have run successfully.

v5: fix all Kristoffer's review comments form 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 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/

Li Chen (29):
  trailer: append trailers in-process and drop the fork to
    `interpret-trailers`
  trailer: restore interpret_trailers helper
  trailer: drop --trailer prefix handling in amend helper
  trailer: move config_head and arg_head to if storage
  trailer: use bool for had_trailer_before
  interpret-trailers: buffer stdout output
  trailer: mirror interpret-trailers output flow
  trailer: handle trailer append failures gently
  rebase: support --trailer
  rebase: inline trailer state paths
  rebase: reuse buffer for trailer args
  rebase: drop redundant strbuf_release call
  rebase: skip stripping of --trailer option prefix
  rebase: die on invalid trailer args
  rebase: validate trailers with configured separators
  sequencer: add trailers to message before writing file
  t3440: create expect files at point of use
  t3440: check apply backend error includes option
  t3440: use test_commit_message for trailer checks
  t3440: drop redundant resets and pass branch to rebase where needed
  t3440: assert trailer on HEAD after conflict rebase
  rebase: persist --trailer options across restarts
  t3440: remove redundant --keep-empty
  t3440: use helper for trailer checks
  t3440: test --trailer without values
  t3440: convert ex.com to example.com
  t3440: ensure trailers persist after rebase continue
  t3440: exercise trailer config mapping
  sequencer: honor --trailer with fixup -C

 Documentation/git-rebase.adoc |   9 ++-
 builtin/commit.c              |   2 +-
 builtin/interpret-trailers.c  |  94 +++++-------------------
 builtin/rebase.c              |  50 +++++++++++++
 builtin/tag.c                 |   3 +-
 sequencer.c                   |  34 +++++++++
 sequencer.h                   |   4 +-
 t/meson.build                 |   1 +
 t/t3440-rebase-trailer.sh     | 134 ++++++++++++++++++++++++++++++++++
 trailer.c                     | 130 ++++++++++++++++++++++++++++++---
 trailer.h                     |  23 +++++-
 wrapper.c                     |  16 ++++
 wrapper.h                     |   6 ++
 13 files changed, 412 insertions(+), 94 deletions(-)
 create mode 100755 t/t3440-rebase-trailer.sh

-- 
2.51.0


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v5 01/29] trailer: append trailers in-process and drop the fork to `interpret-trailers`
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-23 13:21   ` Phillip Wood
  2025-10-22  5:39 ` [PATCH v5 02/29] trailer: restore interpret_trailers helper Li Chen
                   ` (27 subsequent siblings)
  28 siblings, 1 reply; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

From: Li Chen <chenl311@chinatelecom.cn>

Route all trailer insertion through trailer_process() and make
builtin/interpret-trailers just do file I/O before calling into it.
amend_file_with_trailers() now shares the same code path.

This removes the fork/exec and tempfile juggling, cutting overhead and
simplifying error handling. No functional change. It also
centralizes logic to prepare for follow-up rebase --trailer patch.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 builtin/interpret-trailers.c | 116 ++++++++------------------------
 trailer.c                    | 125 ++++++++++++++++++++++++++++++++---
 trailer.h                    |  18 ++++-
 3 files changed, 157 insertions(+), 102 deletions(-)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 41b0750e5a..be0fa83f79 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -10,7 +10,6 @@
 #include "gettext.h"
 #include "parse-options.h"
 #include "string-list.h"
-#include "tempfile.h"
 #include "trailer.h"
 #include "config.h"
 
@@ -93,37 +92,6 @@ 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)
-{
-	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);
-
-	/* Create temporary file in the same directory as the original */
-	tail = strrchr(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);
-	strbuf_release(&filename_template);
-	outfile = fdopen_tempfile(trailers_tempfile, "w");
-	if (!outfile)
-		die_errno(_("could not open temporary file"));
-
-	return outfile;
-}
-
 static void read_input_file(struct strbuf *sb, const char *file)
 {
 	if (file) {
@@ -136,61 +104,6 @@ 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)
-{
-	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);
-
-	/* Print the lines before the trailer block */
-	if (!opts->only_trailers)
-		fwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);
-
-	if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
-		fprintf(outfile, "\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, &trailer_block_sb);
-	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);
-	trailer_block_release(trailer_block);
-
-	if (opts->in_place)
-		if (rename_tempfile(&trailers_tempfile, file))
-			die_errno(_("could not rename temporary file to %s"), file);
-
-	strbuf_release(&sb);
-}
-
 int cmd_interpret_trailers(int argc,
 			   const char **argv,
 			   const char *prefix,
@@ -232,14 +145,37 @@ int cmd_interpret_trailers(int argc,
 			git_interpret_trailers_usage,
 			options);
 
+	trailer_config_init();
+
 	if (argc) {
 		int i;
-		for (i = 0; i < argc; i++)
-			interpret_trailers(&opts, &trailers, argv[i]);
+		for (i = 0; i < argc; i++) {
+			struct strbuf in_buf = STRBUF_INIT;
+			struct strbuf out_buf = STRBUF_INIT;
+
+			read_input_file(&in_buf, argv[i]);
+			if (trailer_process(&opts, in_buf.buf, &trailers, &out_buf) < 0)
+				die(_("failed to process trailers for %s"), argv[i]);
+			if (opts.in_place)
+				write_file_buf(argv[i], out_buf.buf, out_buf.len);
+			else
+				fwrite(out_buf.buf, 1, out_buf.len, stdout);
+			strbuf_release(&in_buf);
+			strbuf_release(&out_buf);
+		}
 	} else {
+		struct strbuf in_buf = STRBUF_INIT;
+		struct strbuf out_buf = STRBUF_INIT;
+
 		if (opts.in_place)
 			die(_("no input file given for in-place editing"));
-		interpret_trailers(&opts, &trailers, NULL);
+
+		read_input_file(&in_buf, NULL);
+		if (trailer_process(&opts, in_buf.buf, &trailers, &out_buf) < 0)
+			die(_("failed to process trailers"));
+		fwrite(out_buf.buf, 1, out_buf.len, stdout);
+		strbuf_release(&in_buf);
+		strbuf_release(&out_buf);
 	}
 
 	new_trailers_clear(&trailers);
diff --git a/trailer.c b/trailer.c
index 911a81ed99..2fe49df23a 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1224,14 +1224,121 @@ 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 int amend_strbuf_with_trailers(struct strbuf *buf,
+									  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 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 *arg = trailer_args->v[i];
+		const char *text;
+		struct new_trailer_item *item;
+
+		if (!skip_prefix(arg, "--trailer=", &text))
+			text = arg;
+		if (!*text)
+			continue;
+		item = xcalloc(1, sizeof(*item));
+		INIT_LIST_HEAD(&item->list);
+		item->text = text;
+		list_add_tail(&item->list, &new_trailer_head);
+	}
+	if (trailer_process(&opts, buf->buf, &new_trailer_head, &out) < 0)
+		return -1;
+	strbuf_swap(buf, &out);
+	strbuf_release(&out);
+	while (!list_empty(&new_trailer_head)) {
+		struct new_trailer_item *item =
+			list_first_entry(&new_trailer_head, struct new_trailer_item, list);
+		list_del(&item->list);
+		free(item);
+	}
+	return 0;
+}
+
+int trailer_process(const struct process_trailer_options *opts,
+		    const char *msg,
+		    struct list_head *new_trailer_head,
+		    struct strbuf *out)
+{
+	struct trailer_block *blk;
+	LIST_HEAD(orig_head);
+	LIST_HEAD(config_head);
+	LIST_HEAD(arg_head);
+	struct strbuf trailers_sb = STRBUF_INIT;
+	int had_trailer_before;
+
+	blk = parse_trailers(opts, msg, &orig_head);
+	had_trailer_before = !list_empty(&orig_head);
+	if (!opts->only_input) {
+		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(&orig_head, &arg_head);
+	}
+	format_trailers(opts, &orig_head, &trailers_sb);
+	if (!opts->only_trailers && !opts->only_input && !opts->unfold &&
+	    !opts->trim_empty && list_empty(&orig_head) &&
+	    (list_empty(new_trailer_head) || opts->only_input)) {
+		size_t split = trailer_block_start(blk); /* end-of-log-msg */
+		if (!blank_line_before_trailer_block(blk)) {
+			strbuf_add(out, msg, split);
+			strbuf_addch(out, '\n');
+			strbuf_addstr(out, msg + split);
+		} else
+			strbuf_addstr(out, msg);
+
+		strbuf_release(&trailers_sb);
+		trailer_block_release(blk);
+		return 0;
+	}
+	if (opts->only_trailers) {
+		strbuf_addbuf(out, &trailers_sb);
+	} else if (had_trailer_before) {
+		strbuf_add(out, msg, trailer_block_start(blk));
+		if (!blank_line_before_trailer_block(blk))
+			strbuf_addch(out, '\n');
+		strbuf_addbuf(out, &trailers_sb);
+		strbuf_add(out, msg + trailer_block_end(blk),
+			   strlen(msg) - trailer_block_end(blk));
+	} else {
+		size_t cpos = trailer_block_start(blk);
+		strbuf_add(out, msg, cpos);
+		if (cpos == 0) /* empty body → just one \n */
+			strbuf_addch(out, '\n');
+		else if (!blank_line_before_trailer_block(blk))
+			strbuf_addch(out, '\n'); /* body without trailing blank */
+
+		strbuf_addbuf(out, &trailers_sb);
+		strbuf_add(out, msg + cpos, strlen(msg) - cpos);
+	}
+	strbuf_release(&trailers_sb);
+	free_trailers(&orig_head);
+	trailer_block_release(blk);
+	return 0;
+}
+
+int amend_file_with_trailers(const char *path,
+			     const struct strvec *trailer_args)
+{
+	struct strbuf buf = STRBUF_INIT;
+
+	if (!trailer_args || !trailer_args->nr)
+		return 0;
+
+	if (strbuf_read_file(&buf, path, 0) < 0)
+		return error_errno("could not read '%s'", path);
+
+	if (amend_strbuf_with_trailers(&buf, trailer_args))
+		die("failed to append trailers");
+
+	/* `write_file_buf()` aborts on error internally */
+	write_file_buf(path, buf.buf, buf.len);
+	strbuf_release(&buf);
+	return 0;
 }
diff --git a/trailer.h b/trailer.h
index 4740549586..b4f28bfd65 100644
--- a/trailer.h
+++ b/trailer.h
@@ -196,10 +196,22 @@ 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.
+ * Augment a file to add trailers to it (similar to 'git interpret-trailers').
+ * 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);
 
+/*
+ * Process trailer lines for a commit message in-memory.
+ * @opts: trailer processing options (e.g. from parse-options)
+ * @msg: the input message string
+ * @new_trailer_head: list of new trailers to add (struct new_trailer_item)
+ * @out: strbuf to store the resulting message (must be initialized)
+ *
+ * Returns 0 on success, <0 on error.
+ */
+int trailer_process(const struct process_trailer_options *opts,
+		    const char *msg,
+		    struct list_head *new_trailer_head,
+		    struct strbuf *out);
 #endif /* TRAILER_H */
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 02/29] trailer: restore interpret_trailers helper
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
  2025-10-22  5:39 ` [PATCH v5 01/29] trailer: append trailers in-process and drop the fork to `interpret-trailers` Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 03/29] trailer: drop --trailer prefix handling in amend helper Li Chen
                   ` (26 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

The change factors the duplicated
read/process/write logic into a single
interpret_trailers() helper(which was removed
in previous commit) while preserving the
original error handling and output paths for
both in-place edits and stdout output.

cmd_interpret_trailers() now reuses the helper
for each filename and for the stdin path,
keeping the option parsing and safety checks intact.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 builtin/interpret-trailers.c | 50 +++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 23 deletions(-)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index be0fa83f79..2c8b6fc3b9 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -104,6 +104,30 @@ 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)
+{
+	struct strbuf in_buf = STRBUF_INIT;
+	struct strbuf out_buf = STRBUF_INIT;
+
+	read_input_file(&in_buf, file);
+	if (trailer_process(opts, in_buf.buf, new_trailer_head, &out_buf) < 0) {
+		if (file)
+			die(_("failed to process trailers for %s"), file);
+		else
+			die(_("failed to process trailers"));
+	}
+
+	if (opts->in_place)
+		write_file_buf(file, out_buf.buf, out_buf.len);
+	else
+		fwrite(out_buf.buf, 1, out_buf.len, stdout);
+
+	strbuf_release(&in_buf);
+	strbuf_release(&out_buf);
+}
+
 int cmd_interpret_trailers(int argc,
 			   const char **argv,
 			   const char *prefix,
@@ -149,33 +173,13 @@ int cmd_interpret_trailers(int argc,
 
 	if (argc) {
 		int i;
-		for (i = 0; i < argc; i++) {
-			struct strbuf in_buf = STRBUF_INIT;
-			struct strbuf out_buf = STRBUF_INIT;
-
-			read_input_file(&in_buf, argv[i]);
-			if (trailer_process(&opts, in_buf.buf, &trailers, &out_buf) < 0)
-				die(_("failed to process trailers for %s"), argv[i]);
-			if (opts.in_place)
-				write_file_buf(argv[i], out_buf.buf, out_buf.len);
-			else
-				fwrite(out_buf.buf, 1, out_buf.len, stdout);
-			strbuf_release(&in_buf);
-			strbuf_release(&out_buf);
-		}
+		for (i = 0; i < argc; i++)
+			interpret_trailers(&opts, &trailers, argv[i]);
 	} else {
-		struct strbuf in_buf = STRBUF_INIT;
-		struct strbuf out_buf = STRBUF_INIT;
-
 		if (opts.in_place)
 			die(_("no input file given for in-place editing"));
 
-		read_input_file(&in_buf, NULL);
-		if (trailer_process(&opts, in_buf.buf, &trailers, &out_buf) < 0)
-			die(_("failed to process trailers"));
-		fwrite(out_buf.buf, 1, out_buf.len, stdout);
-		strbuf_release(&in_buf);
-		strbuf_release(&out_buf);
+		interpret_trailers(&opts, &trailers, NULL);
 	}
 
 	new_trailers_clear(&trailers);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 03/29] trailer: drop --trailer prefix handling in amend helper
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
  2025-10-22  5:39 ` [PATCH v5 01/29] trailer: append trailers in-process and drop the fork to `interpret-trailers` Li Chen
  2025-10-22  5:39 ` [PATCH v5 02/29] trailer: restore interpret_trailers helper Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 04/29] trailer: move config_head and arg_head to if storage Li Chen
                   ` (25 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Make callers pass plain trailer text instead of recreating
the option prefix before invoking interpret-trailers.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 builtin/commit.c | 2 +-
 builtin/tag.c    | 3 +--
 trailer.c        | 5 +----
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 0243f17d53..67070d6a54 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1719,7 +1719,7 @@ 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_CALLBACK_F(0, "trailer", &trailer_args, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG, parse_opt_strvec),
 		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 f0665af3ac..65c4a0b36b 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -499,8 +499,7 @@ 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_CALLBACK_F(0, "trailer", &trailer_args, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG, parse_opt_strvec),
 		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 2fe49df23a..b7b0029e05 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1235,12 +1235,9 @@ static int amend_strbuf_with_trailers(struct strbuf *buf,
 	opts.no_divider = 1;
 
 	for (i = 0; i < trailer_args->nr; i++) {
-		const char *arg = trailer_args->v[i];
-		const char *text;
+		const char *text = trailer_args->v[i];
 		struct new_trailer_item *item;
 
-		if (!skip_prefix(arg, "--trailer=", &text))
-			text = arg;
 		if (!*text)
 			continue;
 		item = xcalloc(1, sizeof(*item));
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 04/29] trailer: move config_head and arg_head to if storage
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (2 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 03/29] trailer: drop --trailer prefix handling in amend helper Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 05/29] trailer: use bool for had_trailer_before Li Chen
                   ` (24 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

From: Li Chen <chenl311@chinatelecom.cn>

Move LIST_HEAD(config_head) and LIST_HEAD(arg_head) into the
non-only_input branch so they are created only when needed.
No functional change.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 trailer.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/trailer.c b/trailer.c
index b7b0029e05..9abb5a522a 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1265,14 +1265,15 @@ int trailer_process(const struct process_trailer_options *opts,
 {
 	struct trailer_block *blk;
 	LIST_HEAD(orig_head);
-	LIST_HEAD(config_head);
-	LIST_HEAD(arg_head);
 	struct strbuf trailers_sb = STRBUF_INIT;
 	int had_trailer_before;
 
 	blk = parse_trailers(opts, msg, &orig_head);
 	had_trailer_before = !list_empty(&orig_head);
 	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);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 05/29] trailer: use bool for had_trailer_before
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (3 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 04/29] trailer: move config_head and arg_head to if storage Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 06/29] interpret-trailers: buffer stdout output Li Chen
                   ` (23 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

From: Li Chen <chenl311@chinatelecom.cn>

Switch had_trailer_before from int to bool
to match its logical use. No functional change.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 trailer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/trailer.c b/trailer.c
index 9abb5a522a..a448380327 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1266,7 +1266,7 @@ int trailer_process(const struct process_trailer_options *opts,
 	struct trailer_block *blk;
 	LIST_HEAD(orig_head);
 	struct strbuf trailers_sb = STRBUF_INIT;
-	int had_trailer_before;
+	bool had_trailer_before;
 
 	blk = parse_trailers(opts, msg, &orig_head);
 	had_trailer_before = !list_empty(&orig_head);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 06/29] interpret-trailers: buffer stdout output
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (4 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 05/29] trailer: use bool for had_trailer_before Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 07/29] trailer: mirror interpret-trailers output flow Li Chen
                   ` (22 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Simplified the non-in-place path in interpret_trailers
by writing the existing output strbuf directly to
stdout without creating a redundant temporary buffer.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 builtin/interpret-trailers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 2c8b6fc3b9..cdf39dbca8 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -122,7 +122,7 @@ static void interpret_trailers(const struct process_trailer_options *opts,
 	if (opts->in_place)
 		write_file_buf(file, out_buf.buf, out_buf.len);
 	else
-		fwrite(out_buf.buf, 1, out_buf.len, stdout);
+		strbuf_write(&out_buf, stdout);
 
 	strbuf_release(&in_buf);
 	strbuf_release(&out_buf);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 07/29] trailer: mirror interpret-trailers output flow
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (5 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 06/29] interpret-trailers: buffer stdout output Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 08/29] trailer: handle trailer append failures gently Li Chen
                   ` (21 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Drop the early-return branch that mirrored the
special case. Let trailer_process() always
follow the same path as interpret-trailers.
Ensure trailer lists and buffers are freed
along the unified exit path.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 trailer.c | 38 +++++++-------------------------------
 1 file changed, 7 insertions(+), 31 deletions(-)

diff --git a/trailer.c b/trailer.c
index a448380327..ac6ac2ac20 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1266,10 +1266,8 @@ int trailer_process(const struct process_trailer_options *opts,
 	struct trailer_block *blk;
 	LIST_HEAD(orig_head);
 	struct strbuf trailers_sb = STRBUF_INIT;
-	bool had_trailer_before;
 
 	blk = parse_trailers(opts, msg, &orig_head);
-	had_trailer_before = !list_empty(&orig_head);
 	if (!opts->only_input) {
 		LIST_HEAD(config_head);
 		LIST_HEAD(arg_head);
@@ -1280,40 +1278,18 @@ int trailer_process(const struct process_trailer_options *opts,
 		process_trailers_lists(&orig_head, &arg_head);
 	}
 	format_trailers(opts, &orig_head, &trailers_sb);
-	if (!opts->only_trailers && !opts->only_input && !opts->unfold &&
-	    !opts->trim_empty && list_empty(&orig_head) &&
-	    (list_empty(new_trailer_head) || opts->only_input)) {
-		size_t split = trailer_block_start(blk); /* end-of-log-msg */
-		if (!blank_line_before_trailer_block(blk)) {
-			strbuf_add(out, msg, split);
-			strbuf_addch(out, '\n');
-			strbuf_addstr(out, msg + split);
-		} else
-			strbuf_addstr(out, msg);
-
-		strbuf_release(&trailers_sb);
-		trailer_block_release(blk);
-		return 0;
-	}
 	if (opts->only_trailers) {
 		strbuf_addbuf(out, &trailers_sb);
-	} else if (had_trailer_before) {
-		strbuf_add(out, msg, trailer_block_start(blk));
-		if (!blank_line_before_trailer_block(blk))
-			strbuf_addch(out, '\n');
-		strbuf_addbuf(out, &trailers_sb);
-		strbuf_add(out, msg + trailer_block_end(blk),
-			   strlen(msg) - trailer_block_end(blk));
 	} else {
-		size_t cpos = trailer_block_start(blk);
-		strbuf_add(out, msg, cpos);
-		if (cpos == 0) /* empty body → just one \n */
-			strbuf_addch(out, '\n');
-		else if (!blank_line_before_trailer_block(blk))
-			strbuf_addch(out, '\n'); /* body without trailing blank */
+		size_t block_start = trailer_block_start(blk);
+		size_t block_end = trailer_block_end(blk);
+		bool need_blank_line = !blank_line_before_trailer_block(blk);
 
+		strbuf_add(out, msg, block_start);
+		if (need_blank_line)
+			strbuf_addch(out, '\n');
 		strbuf_addbuf(out, &trailers_sb);
-		strbuf_add(out, msg + cpos, strlen(msg) - cpos);
+		strbuf_add(out, msg + block_end, strlen(msg) - block_end);
 	}
 	strbuf_release(&trailers_sb);
 	free_trailers(&orig_head);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 08/29] trailer: handle trailer append failures gently
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (6 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 07/29] trailer: mirror interpret-trailers output flow Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 09/29] rebase: support --trailer Li Chen
                   ` (20 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Added write_file_buf_gently so callers can rewrite
files while surfacing errors instead of aborting.

Updated amend_file_with_trailers to release buffers
and propagate trailer and write failures back to the caller,
because amend_file_with_trailers shuold not die.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 trailer.c | 14 ++++++++++----
 wrapper.c | 16 ++++++++++++++++
 wrapper.h |  6 ++++++
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/trailer.c b/trailer.c
index ac6ac2ac20..1f317f4d37 100644
--- a/trailer.c
+++ b/trailer.c
@@ -9,6 +9,7 @@
 #include "commit.h"
 #include "trailer.h"
 #include "list.h"
+#include "wrapper.h"
 /*
  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
  */
@@ -1308,11 +1309,16 @@ int amend_file_with_trailers(const char *path,
 	if (strbuf_read_file(&buf, path, 0) < 0)
 		return error_errno("could not read '%s'", path);
 
-	if (amend_strbuf_with_trailers(&buf, trailer_args))
-		die("failed to append trailers");
+	if (amend_strbuf_with_trailers(&buf, trailer_args)) {
+		strbuf_release(&buf);
+		return error("failed to append trailers");
+	}
+
+	if (write_file_buf_gently(path, buf.buf, buf.len)) {
+		strbuf_release(&buf);
+		return -1;
+	}
 
-	/* `write_file_buf()` aborts on error internally */
-	write_file_buf(path, buf.buf, buf.len);
 	strbuf_release(&buf);
 	return 0;
 }
diff --git a/wrapper.c b/wrapper.c
index 3d507d4204..1f12dbb2fa 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -688,6 +688,22 @@ void write_file_buf(const char *path, const char *buf, size_t len)
 		die_errno(_("could not close '%s'"), path);
 }
 
+int write_file_buf_gently(const char *path, const char *buf, size_t len)
+{
+	int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0666);
+
+	if (fd < 0)
+		return error_errno(_("could not open '%s'"), path);
+	if (write_in_full(fd, buf, len) < 0) {
+		int ret = error_errno(_("could not write to '%s'"), path);
+		close(fd);
+		return ret;
+	}
+	if (close(fd))
+		return error_errno(_("could not close '%s'"), path);
+	return 0;
+}
+
 void write_file(const char *path, const char *fmt, ...)
 {
 	va_list params;
diff --git a/wrapper.h b/wrapper.h
index 44a8597ac3..e5f867b200 100644
--- a/wrapper.h
+++ b/wrapper.h
@@ -56,6 +56,12 @@ static inline ssize_t write_str_in_full(int fd, const char *str)
  */
 void write_file_buf(const char *path, const char *buf, size_t len);
 
+/**
+ * Like write_file_buf(), but report errors instead of exiting. Returns 0 on
+ * success or a negative value on error after emitting a message.
+ */
+int write_file_buf_gently(const char *path, const char *buf, size_t len);
+
 /**
  * Like write_file_buf(), but format the contents into a buffer first.
  * Additionally, write_file() will append a newline if one is not already
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 09/29] rebase: support --trailer
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (7 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 08/29] trailer: handle trailer append failures gently Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-23 13:21   ` Phillip Wood
  2025-10-22  5:39 ` [PATCH v5 10/29] rebase: inline trailer state paths Li Chen
                   ` (19 subsequent siblings)
  28 siblings, 1 reply; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

From: Li Chen <chenl311@chinatelecom.cn>

Implement a new `--trailer <text>` option for `git rebase`
(support merge backend only now), which appends arbitrary
trailer lines to each rebased commit message.

Reject it if the user passes an option that requires the
apply backend (git am) since it lacks message‑filter/trailer
hook. otherwise we can just use the merge backend.

Automatically set REBASE_FORCE when any trailer is supplied.

And reject invalid input before user edits the interactive file.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 Documentation/git-rebase.adoc |  9 +++-
 builtin/rebase.c              | 89 +++++++++++++++++++++++++++++++++
 sequencer.c                   | 13 +++++
 sequencer.h                   |  4 +-
 t/meson.build                 |  1 +
 t/t3440-rebase-trailer.sh     | 94 +++++++++++++++++++++++++++++++++++
 6 files changed, 208 insertions(+), 2 deletions(-)
 create mode 100755 t/t3440-rebase-trailer.sh

diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc
index 005caf6164..4d2fe4be6e 100644
--- a/Documentation/git-rebase.adoc
+++ b/Documentation/git-rebase.adoc
@@ -487,9 +487,16 @@ See also INCOMPATIBLE OPTIONS below.
 	Add a `Signed-off-by` trailer to all the rebased commits. Note
 	that if `--interactive` is given then only commits marked to be
 	picked, edited or reworded will have the trailer added.
-+
+
 See also INCOMPATIBLE OPTIONS below.
 
+--trailer=<trailer>::
+       Append the given trailer line(s) to every rebased commit
+       message, processed via linkgit:git-interpret-trailers[1].
+       When this option is present *rebase automatically implies*
+       `--force-rebase` so that fast‑forwarded commits are also
+       rewritten.
+
 -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 c468828189..3db1439b52 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;
@@ -434,6 +442,8 @@ static int read_basic_state(struct rebase_options *opts)
 	struct strbuf head_name = STRBUF_INIT;
 	struct strbuf buf = STRBUF_INIT;
 	struct object_id oid;
+	const char trailer_state_name[] = "trailer";
+	const char *path = state_dir_path(trailer_state_name, opts);
 
 	if (!read_oneliner(&head_name, state_dir_path("head-name", opts),
 			   READ_ONELINER_WARN_MISSING) ||
@@ -502,11 +512,31 @@ static int read_basic_state(struct rebase_options *opts)
 
 	strbuf_release(&buf);
 
+	if (strbuf_read_file(&buf, path, 0) >= 0) {
+		const char *p = buf.buf, *end = buf.buf + buf.len;
+
+		while (p < end) {
+			char *nl = memchr(p, '\n', end - p);
+			if (!nl)
+				die("nl shouldn't be NULL");
+			*nl = '\0';
+
+			if (*p)
+				strvec_push(&opts->trailer_args, p);
+
+			p = nl + 1;
+		}
+		strbuf_release(&buf);
+	}
+	strbuf_release(&buf);
+
 	return 0;
 }
 
 static int rebase_write_basic_state(struct rebase_options *opts)
 {
+	const char trailer_state_name[] = "trailer";
+
 	write_file(state_dir_path("head-name", opts), "%s",
 		   opts->head_name ? opts->head_name : "detached HEAD");
 	write_file(state_dir_path("onto", opts), "%s",
@@ -528,6 +558,21 @@ static int rebase_write_basic_state(struct rebase_options *opts)
 	if (opts->signoff)
 		write_file(state_dir_path("signoff", opts), "--signoff");
 
+	/*
+	 * save opts->trailer_args into state_dir/trailer
+	 */
+	if (opts->trailer_args.nr) {
+		struct strbuf buf = STRBUF_INIT;
+
+		for (size_t i = 0; i < opts->trailer_args.nr; i++) {
+				strbuf_addstr(&buf, opts->trailer_args.v[i]);
+				strbuf_addch(&buf, '\n');
+		}
+		write_file(state_dir_path(trailer_state_name, opts),
+				   "%s", buf.buf);
+		strbuf_release(&buf);
+	}
+
 	return 0;
 }
 
@@ -1084,6 +1129,35 @@ static int check_exec_cmd(const char *cmd)
 	return 0;
 }
 
+static int validate_trailer_args_after_config(const struct strvec *cli_args,
+					      struct strbuf *err)
+{
+	for (size_t i = 0; i < cli_args->nr; i++) {
+		const char *raw = cli_args->v[i];
+		const char *txt; // Key[:=]Val
+		const char *sep;
+
+		if (!skip_prefix(raw, "--trailer=", &txt))
+			txt = raw;
+
+		if (!*txt) {
+			strbuf_addstr(err, _("empty --trailer argument"));
+			return -1;
+		}
+
+		sep = strpbrk(txt, ":=");
+
+		/* there must be key bfore seperator */
+		if (sep && sep == txt) {
+			strbuf_addf(err,
+				    _("invalid trailer '%s': missing key before separator"),
+				    txt);
+			return -1;
+		}
+	}
+	return 0;
+}
+
 int cmd_rebase(int argc,
 	       const char **argv,
 	       const char *prefix,
@@ -1132,6 +1206,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 +1361,16 @@ int cmd_rebase(int argc,
 			     builtin_rebase_options,
 			     builtin_rebase_usage, 0);
 
+	if (options.trailer_args.nr) {
+		struct strbuf err = STRBUF_INIT;
+
+		if (validate_trailer_args_after_config(&options.trailer_args, &err))
+			die("%s", err.buf);
+
+		options.flags |= REBASE_FORCE;
+		strbuf_release(&err);
+	}
+
 	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 +1628,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 5476d39ba9..5103ae786c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -420,6 +420,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);
 }
@@ -2517,6 +2518,18 @@ static int do_pick_commit(struct repository *r,
 			_("dropping %s %s -- patch contents already upstream\n"),
 			oid_to_hex(&commit->object.oid), msg.subject);
 	} /* else allow == 0 and there's nothing special to do */
+
+	if (!res && opts->trailer_args.nr && !drop_commit) {
+		const char *trailer_file =
+			msg_file ? msg_file : git_path_merge_msg(r);
+
+		if (amend_file_with_trailers(trailer_file,
+						&opts->trailer_args)) {
+			res = error(_("unable to add trailers to commit message"));
+			goto leave;
+		}
+	}
+
 	if (!opts->no_commit && !drop_commit) {
 		if (author || command == TODO_REVERT || (flags & AMEND_MSG))
 			res = do_commit(r, msg_file, author, reflog_action,
diff --git a/sequencer.h b/sequencer.h
index 719684c8a9..e21835c5a0 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -44,6 +44,7 @@ struct replay_opts {
 	int record_origin;
 	int no_commit;
 	int signoff;
+	struct strvec trailer_args;
 	int allow_ff;
 	int allow_rerere_auto;
 	int allow_empty;
@@ -82,8 +83,9 @@ struct replay_opts {
 	struct replay_ctx *ctx;
 };
 #define REPLAY_OPTS_INIT {			\
-	.edit = -1,				\
 	.action = -1,				\
+	.edit = -1,				\
+	.trailer_args = STRVEC_INIT, \
 	.xopts = STRVEC_INIT,			\
 	.ctx = replay_ctx_new(),		\
 }
diff --git a/t/meson.build b/t/meson.build
index 401b24e50e..990e8ad4be 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -384,6 +384,7 @@ integration_tests = [
   't3436-rebase-more-options.sh',
   't3437-rebase-fixup-options.sh',
   't3438-rebase-broken-files.sh',
+  't3440-rebase-trailer.sh',
   't3500-cherry.sh',
   't3501-revert-cherry-pick.sh',
   't3502-cherry-pick-merge.sh',
diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
new file mode 100755
index 0000000000..b1c7b03330
--- /dev/null
+++ b/t/t3440-rebase-trailer.sh
@@ -0,0 +1,94 @@
+#!/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
+
+create_expect() {
+	cat >"$1" <<-EOF
+	$2
+
+	Reviewed-by: Dev <dev@example.com>
+	EOF
+}
+
+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 &&
+	ident="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" &&
+	create_expect initial-signed  "Initial empty commit" &&
+	create_expect first-signed    "first"                 &&
+	create_expect second-signed   "second"                &&
+	create_expect file2-signed    "file-2"                &&
+	create_expect third-signed    "third"                 &&
+	create_expect conflict-signed "conflict"
+'
+
+test_expect_success 'apply backend is rejected with --trailer' '
+	head_before=$(git rev-parse HEAD) &&
+	test_expect_code 128 \
+	git rebase --apply --trailer "Reviewed-by: Dev <dev@example.com>" \
+				HEAD^ 2>err &&
+	test_grep "requires the merge backend" err &&
+	test_cmp_rev HEAD $head_before
+'
+
+test_expect_success 'reject empty --trailer argument' '
+	git reset --hard third &&
+	test_expect_code 128 git rebase -m --trailer "" HEAD^ 2>err &&
+	test_grep "empty --trailer" err
+'
+
+test_expect_success 'reject trailer with missing key before separator' '
+	git reset --hard third &&
+	test_expect_code 128 git rebase -m --trailer ": no-key" HEAD^ 2>err &&
+	test_grep "missing key before separator" err
+'
+
+test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last' '
+	git reset --hard third &&
+	git -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add rebase -m --trailer "Bug: 123" --trailer "Bug: 456" HEAD~1 &&
+	git cat-file commit HEAD | grep "^Bug: 456" &&
+	git cat-file commit HEAD | grep -v "^Bug: 123"
+'
+
+test_expect_success 'multiple Signed-off-by trailers all preserved' '
+	git reset --hard third &&
+	git rebase -m \
+		--trailer "Signed-off-by: Dev A <a@ex.com>" \
+		--trailer "Signed-off-by: Dev B <b@ex.com>" HEAD~1 &&
+	git cat-file commit HEAD | grep -c "^Signed-off-by:" >count &&
+	test "$(cat count)" = 2   # two new commits
+'
+
+test_expect_success 'rebase -m --trailer adds trailer after conflicts' '
+	git reset --hard third &&
+	test_must_fail git rebase -m \
+		--trailer "Reviewed-by: Dev <dev@example.com>" \
+		second third &&
+	git checkout --theirs file &&
+	git add file &&
+	git rebase --continue &&
+	test_commit_message HEAD~2 file2-signed
+'
+
+test_expect_success 'rebase --root --trailer updates every commit' '
+	git checkout first &&
+	git rebase --root --keep-empty \
+		--trailer "Reviewed-by: Dev <dev@example.com>" &&
+	test_commit_message HEAD   first-signed &&
+	test_commit_message HEAD^  initial-signed
+'
+test_done
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 10/29] rebase: inline trailer state paths
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (8 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 09/29] rebase: support --trailer Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 11/29] rebase: reuse buffer for trailer args Li Chen
                   ` (18 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Simplified read_basic_state() by dropping the
temporary trailer path variables and calling
state_dir_path("trailer", opts) directly when
loading trailer arguments.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 builtin/rebase.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 3db1439b52..b0f547ef2b 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -442,8 +442,6 @@ static int read_basic_state(struct rebase_options *opts)
 	struct strbuf head_name = STRBUF_INIT;
 	struct strbuf buf = STRBUF_INIT;
 	struct object_id oid;
-	const char trailer_state_name[] = "trailer";
-	const char *path = state_dir_path(trailer_state_name, opts);
 
 	if (!read_oneliner(&head_name, state_dir_path("head-name", opts),
 			   READ_ONELINER_WARN_MISSING) ||
@@ -512,7 +510,7 @@ static int read_basic_state(struct rebase_options *opts)
 
 	strbuf_release(&buf);
 
-	if (strbuf_read_file(&buf, path, 0) >= 0) {
+	if (strbuf_read_file(&buf, state_dir_path("trailer", opts), 0) >= 0) {
 		const char *p = buf.buf, *end = buf.buf + buf.len;
 
 		while (p < end) {
@@ -535,8 +533,6 @@ static int read_basic_state(struct rebase_options *opts)
 
 static int rebase_write_basic_state(struct rebase_options *opts)
 {
-	const char trailer_state_name[] = "trailer";
-
 	write_file(state_dir_path("head-name", opts), "%s",
 		   opts->head_name ? opts->head_name : "detached HEAD");
 	write_file(state_dir_path("onto", opts), "%s",
@@ -568,7 +564,7 @@ static int rebase_write_basic_state(struct rebase_options *opts)
 				strbuf_addstr(&buf, opts->trailer_args.v[i]);
 				strbuf_addch(&buf, '\n');
 		}
-		write_file(state_dir_path(trailer_state_name, opts),
+		write_file(state_dir_path("trailer", opts),
 				   "%s", buf.buf);
 		strbuf_release(&buf);
 	}
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 11/29] rebase: reuse buffer for trailer args
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (9 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 10/29] rebase: inline trailer state paths Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 12/29] rebase: drop redundant strbuf_release call Li Chen
                   ` (17 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Reset the reusable buffer before reading trailer
arguments in read_basic_state() so the existing
allocation can be reused.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 builtin/rebase.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index b0f547ef2b..89ecb84a31 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -508,7 +508,7 @@ static int read_basic_state(struct rebase_options *opts)
 		opts->gpg_sign_opt = xstrdup(buf.buf);
 	}
 
-	strbuf_release(&buf);
+	strbuf_reset(&buf);
 
 	if (strbuf_read_file(&buf, state_dir_path("trailer", opts), 0) >= 0) {
 		const char *p = buf.buf, *end = buf.buf + buf.len;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 12/29] rebase: drop redundant strbuf_release call
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (10 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 11/29] rebase: reuse buffer for trailer args Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 13/29] rebase: skip stripping of --trailer option prefix Li Chen
                   ` (16 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Removed the redundant strbuf_release() call in
read_basic_state() so the buffer is released once
even if strbuf_read_file() fails.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 builtin/rebase.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 89ecb84a31..a950005dfc 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -524,7 +524,6 @@ static int read_basic_state(struct rebase_options *opts)
 
 			p = nl + 1;
 		}
-		strbuf_release(&buf);
 	}
 	strbuf_release(&buf);
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 13/29] rebase: skip stripping of --trailer option prefix
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (11 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 12/29] rebase: drop redundant strbuf_release call Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 14/29] rebase: die on invalid trailer args Li Chen
                   ` (15 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Removed redundant --trailer= prefix stripping in
validate_trailer_args_after_config() since OPT_STRVEC
already stores only the argument text.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 builtin/rebase.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index a950005dfc..3ac1eda61b 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1128,13 +1128,9 @@ static int validate_trailer_args_after_config(const struct strvec *cli_args,
 					      struct strbuf *err)
 {
 	for (size_t i = 0; i < cli_args->nr; i++) {
-		const char *raw = cli_args->v[i];
-		const char *txt; // Key[:=]Val
+		const char *txt = cli_args->v[i]; // Key[:=]Val
 		const char *sep;
 
-		if (!skip_prefix(raw, "--trailer=", &txt))
-			txt = raw;
-
 		if (!*txt) {
 			strbuf_addstr(err, _("empty --trailer argument"));
 			return -1;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 14/29] rebase: die on invalid trailer args
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (12 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 13/29] rebase: skip stripping of --trailer option prefix Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 15/29] rebase: validate trailers with configured separators Li Chen
                   ` (14 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

This can simplify the error handle in
validate_trailer_args_after_config and
its caller.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 builtin/rebase.c | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 3ac1eda61b..872945a897 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1124,29 +1124,22 @@ static int check_exec_cmd(const char *cmd)
 	return 0;
 }
 
-static int validate_trailer_args_after_config(const struct strvec *cli_args,
-					      struct strbuf *err)
+static void validate_trailer_args_after_config(const struct strvec *cli_args)
 {
 	for (size_t i = 0; i < cli_args->nr; i++) {
 		const char *txt = cli_args->v[i]; // Key[:=]Val
 		const char *sep;
 
-		if (!*txt) {
-			strbuf_addstr(err, _("empty --trailer argument"));
-			return -1;
-		}
+		if (!*txt)
+			die(_("empty --trailer argument"));
 
 		sep = strpbrk(txt, ":=");
 
 		/* there must be key bfore seperator */
-		if (sep && sep == txt) {
-			strbuf_addf(err,
-				    _("invalid trailer '%s': missing key before separator"),
-				    txt);
-			return -1;
-		}
+		if (sep && sep == txt)
+			die(_("invalid trailer '%s': missing key before separator"),
+			    txt);
 	}
-	return 0;
 }
 
 int cmd_rebase(int argc,
@@ -1353,13 +1346,8 @@ int cmd_rebase(int argc,
 			     builtin_rebase_usage, 0);
 
 	if (options.trailer_args.nr) {
-		struct strbuf err = STRBUF_INIT;
-
-		if (validate_trailer_args_after_config(&options.trailer_args, &err))
-			die("%s", err.buf);
-
+		validate_trailer_args_after_config(&options.trailer_args);
 		options.flags |= REBASE_FORCE;
-		strbuf_release(&err);
 	}
 
 	if (preserve_merges_selected)
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 15/29] rebase: validate trailers with configured separators
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (13 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 14/29] rebase: die on invalid trailer args Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 16/29] sequencer: add trailers to message before writing file Li Chen
                   ` (13 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Moved validate_trailer_args_after_config() into
trailer.c so trailer argument validation reuses
find_separator() and respects configured separators.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 builtin/rebase.c | 18 ------------------
 trailer.c        | 25 +++++++++++++++++++++++++
 trailer.h        |  2 ++
 3 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 872945a897..a88abe08b4 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1124,24 +1124,6 @@ static int check_exec_cmd(const char *cmd)
 	return 0;
 }
 
-static void validate_trailer_args_after_config(const struct strvec *cli_args)
-{
-	for (size_t i = 0; i < cli_args->nr; i++) {
-		const char *txt = cli_args->v[i]; // Key[:=]Val
-		const char *sep;
-
-		if (!*txt)
-			die(_("empty --trailer argument"));
-
-		sep = strpbrk(txt, ":=");
-
-		/* there must be key bfore seperator */
-		if (sep && sep == txt)
-			die(_("invalid trailer '%s': missing key before separator"),
-			    txt);
-	}
-}
-
 int cmd_rebase(int argc,
 	       const char **argv,
 	       const char *prefix,
diff --git a/trailer.c b/trailer.c
index 1f317f4d37..85e42859ca 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 "wrapper.h"
@@ -773,6 +774,30 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,
 	free(cl_separators);
 }
 
+void validate_trailer_args_after_config(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');
diff --git a/trailer.h b/trailer.h
index b4f28bfd65..4654ff9c96 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_after_config(const struct strvec *cli_args);
+
 void process_trailers_lists(struct list_head *head,
 			    struct list_head *arg_head);
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 16/29] sequencer: add trailers to message before writing file
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (14 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 15/29] rebase: validate trailers with configured separators Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 17/29] t3440: create expect files at point of use Li Chen
                   ` (12 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Added trailer processing to the in-memory commit message
within do_pick_commit, ensuring fixup/squash commands
remain untouched before the message is written.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 sequencer.c | 19 ++++++++-----------
 trailer.c   |  4 ++--
 trailer.h   |  3 +++
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 5103ae786c..552e629e4f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2444,6 +2444,14 @@ 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)) {
+		if (amend_strbuf_with_trailers(&ctx->message,
+					       &opts->trailer_args)) {
+			res = error(_("unable to add trailers to commit message"));
+			goto leave;
+		}
+	}
+
 	if (is_rebase_i(opts) && write_author_script(msg.message) < 0)
 		res = -1;
 	else if (!opts->strategy ||
@@ -2519,17 +2527,6 @@ static int do_pick_commit(struct repository *r,
 			oid_to_hex(&commit->object.oid), msg.subject);
 	} /* else allow == 0 and there's nothing special to do */
 
-	if (!res && opts->trailer_args.nr && !drop_commit) {
-		const char *trailer_file =
-			msg_file ? msg_file : git_path_merge_msg(r);
-
-		if (amend_file_with_trailers(trailer_file,
-						&opts->trailer_args)) {
-			res = error(_("unable to add trailers to commit message"));
-			goto leave;
-		}
-	}
-
 	if (!opts->no_commit && !drop_commit) {
 		if (author || command == TODO_REVERT || (flags & AMEND_MSG))
 			res = do_commit(r, msg_file, author, reflog_action,
diff --git a/trailer.c b/trailer.c
index 85e42859ca..3e96d1624a 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1250,8 +1250,8 @@ void trailer_iterator_release(struct trailer_iterator *iter)
 	strbuf_release(&iter->key);
 }
 
-static int amend_strbuf_with_trailers(struct strbuf *buf,
-									  const struct strvec *trailer_args)
+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);
diff --git a/trailer.h b/trailer.h
index 4654ff9c96..479bc137cd 100644
--- a/trailer.h
+++ b/trailer.h
@@ -197,6 +197,9 @@ int trailer_iterator_advance(struct trailer_iterator *iter);
  */
 void trailer_iterator_release(struct trailer_iterator *iter);
 
+int amend_strbuf_with_trailers(struct strbuf *buf,
+			       const struct strvec *trailer_args);
+
 /*
  * Augment a file to add trailers to it (similar to 'git interpret-trailers').
  * Returns 0 on success or a non-zero error code on failure.
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 17/29] t3440: create expect files at point of use
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (15 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 16/29] sequencer: add trailers to message before writing file Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 18/29] t3440: check apply backend error includes option Li Chen
                   ` (11 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Created the expected trailer files within the individual rebase tests
that use them, simplifying the shared history setup and avoiding unused
fixtures.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 t/t3440-rebase-trailer.sh | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
index b1c7b03330..a8108f2296 100755
--- a/t/t3440-rebase-trailer.sh
+++ b/t/t3440-rebase-trailer.sh
@@ -26,14 +26,7 @@ test_expect_success 'setup repo with a small history' '
 	git checkout -b conflict-branch first &&
 	test_commit file-2 file-2 &&
 	test_commit conflict file &&
-	test_commit third file &&
-	ident="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" &&
-	create_expect initial-signed  "Initial empty commit" &&
-	create_expect first-signed    "first"                 &&
-	create_expect second-signed   "second"                &&
-	create_expect file2-signed    "file-2"                &&
-	create_expect third-signed    "third"                 &&
-	create_expect conflict-signed "conflict"
+	test_commit third file
 '
 
 test_expect_success 'apply backend is rejected with --trailer' '
@@ -74,6 +67,7 @@ test_expect_success 'multiple Signed-off-by trailers all preserved' '
 '
 
 test_expect_success 'rebase -m --trailer adds trailer after conflicts' '
+	create_expect file2-signed "file-2" &&
 	git reset --hard third &&
 	test_must_fail git rebase -m \
 		--trailer "Reviewed-by: Dev <dev@example.com>" \
@@ -85,6 +79,8 @@ test_expect_success 'rebase -m --trailer adds trailer after conflicts' '
 '
 
 test_expect_success 'rebase --root --trailer updates every commit' '
+	create_expect initial-signed "Initial empty commit" &&
+	create_expect first-signed "first" &&
 	git checkout first &&
 	git rebase --root --keep-empty \
 		--trailer "Reviewed-by: Dev <dev@example.com>" &&
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 18/29] t3440: check apply backend error includes option
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (16 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 17/29] t3440: create expect files at point of use Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 19/29] t3440: use test_commit_message for trailer checks Li Chen
                   ` (10 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

From: Li Chen <chenl311@chinatelecom.cn>

Updated the rebase trailer test to assert that
the apply-backend error explicitly includes the
--trailer option in its message while retaining
the existing backend check.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 t/t3440-rebase-trailer.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
index a8108f2296..6b1c93b4cb 100755
--- a/t/t3440-rebase-trailer.sh
+++ b/t/t3440-rebase-trailer.sh
@@ -34,7 +34,7 @@ test_expect_success 'apply backend is rejected with --trailer' '
 	test_expect_code 128 \
 	git rebase --apply --trailer "Reviewed-by: Dev <dev@example.com>" \
 				HEAD^ 2>err &&
-	test_grep "requires the merge backend" err &&
+	test_grep "fatal: --trailer requires the merge backend" err &&
 	test_cmp_rev HEAD $head_before
 '
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 19/29] t3440: use test_commit_message for trailer checks
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (17 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 18/29] t3440: check apply backend error includes option Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 20/29] t3440: drop redundant resets and pass branch to rebase where needed Li Chen
                   ` (9 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Replaced the pipeline-based trailer assertions with
explicit expectations verified by test_commit_message,
ensuring the rebase trailer tests catch git command
failures reliably.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 t/t3440-rebase-trailer.sh | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
index 6b1c93b4cb..1571dd2c97 100755
--- a/t/t3440-rebase-trailer.sh
+++ b/t/t3440-rebase-trailer.sh
@@ -52,18 +52,28 @@ test_expect_success 'reject trailer with missing key before separator' '
 
 test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last' '
 	git reset --hard third &&
-	git -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add rebase -m --trailer "Bug: 123" --trailer "Bug: 456" HEAD~1 &&
-	git cat-file commit HEAD | grep "^Bug: 456" &&
-	git cat-file commit HEAD | grep -v "^Bug: 123"
+	git -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add \
+		rebase -m --trailer "Bug: 123" --trailer "Bug: 456" HEAD~1 &&
+	cat >expect <<-\EOF &&
+	third
+
+	Bug: 456
+	EOF
+	test_commit_message HEAD expect
 '
 
 test_expect_success 'multiple Signed-off-by trailers all preserved' '
 	git reset --hard third &&
 	git rebase -m \
-		--trailer "Signed-off-by: Dev A <a@ex.com>" \
-		--trailer "Signed-off-by: Dev B <b@ex.com>" HEAD~1 &&
-	git cat-file commit HEAD | grep -c "^Signed-off-by:" >count &&
-	test "$(cat count)" = 2   # two new commits
+			--trailer "Signed-off-by: Dev A <a@ex.com>" \
+			--trailer "Signed-off-by: Dev B <b@ex.com>" HEAD~1 &&
+	cat >expect <<-\EOF &&
+	third
+
+	Signed-off-by: Dev A <a@ex.com>
+	Signed-off-by: Dev B <b@ex.com>
+	EOF
+	test_commit_message HEAD expect
 '
 
 test_expect_success 'rebase -m --trailer adds trailer after conflicts' '
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 20/29] t3440: drop redundant resets and pass branch to rebase where needed
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (18 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 19/29] t3440: use test_commit_message for trailer checks Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 21/29] t3440: assert trailer on HEAD after conflict rebase Li Chen
                   ` (8 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

From: Li Chen <chenl311@chinatelecom.cn>

Stop hard-resetting to third in these tests. Where the branch matters,
invoke git rebase -m ... HEAD~1 third to make the target explicit and
preserve the original semantics.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 t/t3440-rebase-trailer.sh | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
index 1571dd2c97..504bdd86fc 100755
--- a/t/t3440-rebase-trailer.sh
+++ b/t/t3440-rebase-trailer.sh
@@ -39,21 +39,18 @@ test_expect_success 'apply backend is rejected with --trailer' '
 '
 
 test_expect_success 'reject empty --trailer argument' '
-	git reset --hard third &&
 	test_expect_code 128 git rebase -m --trailer "" HEAD^ 2>err &&
 	test_grep "empty --trailer" err
 '
 
 test_expect_success 'reject trailer with missing key before separator' '
-	git reset --hard third &&
 	test_expect_code 128 git rebase -m --trailer ": no-key" HEAD^ 2>err &&
 	test_grep "missing key before separator" err
 '
 
 test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last' '
-	git reset --hard third &&
 	git -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add \
-		rebase -m --trailer "Bug: 123" --trailer "Bug: 456" HEAD~1 &&
+		rebase -m --trailer "Bug: 123" --trailer "Bug: 456" HEAD~1 third &&
 	cat >expect <<-\EOF &&
 	third
 
@@ -63,10 +60,9 @@ test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last'
 '
 
 test_expect_success 'multiple Signed-off-by trailers all preserved' '
-	git reset --hard third &&
 	git rebase -m \
 			--trailer "Signed-off-by: Dev A <a@ex.com>" \
-			--trailer "Signed-off-by: Dev B <b@ex.com>" HEAD~1 &&
+			--trailer "Signed-off-by: Dev B <b@ex.com>" HEAD~1 third &&
 	cat >expect <<-\EOF &&
 	third
 
@@ -78,7 +74,6 @@ test_expect_success 'multiple Signed-off-by trailers all preserved' '
 
 test_expect_success 'rebase -m --trailer adds trailer after conflicts' '
 	create_expect file2-signed "file-2" &&
-	git reset --hard third &&
 	test_must_fail git rebase -m \
 		--trailer "Reviewed-by: Dev <dev@example.com>" \
 		second third &&
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 21/29] t3440: assert trailer on HEAD after conflict rebase
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (19 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 20/29] t3440: drop redundant resets and pass branch to rebase where needed Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 22/29] rebase: persist --trailer options across restarts Li Chen
                   ` (7 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

From: Li Chen <chenl311@chinatelecom.cn>

Switch the test to check the trailer on HEAD (not HEAD~2) and build the
expected message for "third", matching the rebased tip after conflicts.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 t/t3440-rebase-trailer.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
index 504bdd86fc..fbc6f209f1 100755
--- a/t/t3440-rebase-trailer.sh
+++ b/t/t3440-rebase-trailer.sh
@@ -73,14 +73,14 @@ test_expect_success 'multiple Signed-off-by trailers all preserved' '
 '
 
 test_expect_success 'rebase -m --trailer adds trailer after conflicts' '
-	create_expect file2-signed "file-2" &&
+	create_expect third-signed "third" &&
 	test_must_fail git rebase -m \
 		--trailer "Reviewed-by: Dev <dev@example.com>" \
 		second third &&
 	git checkout --theirs file &&
 	git add file &&
 	git rebase --continue &&
-	test_commit_message HEAD~2 file2-signed
+	test_commit_message HEAD third-signed
 '
 
 test_expect_success 'rebase --root --trailer updates every commit' '
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 22/29] rebase: persist --trailer options across restarts
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (20 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 21/29] t3440: assert trailer on HEAD after conflict rebase Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 23/29] t3440: remove redundant --keep-empty Li Chen
                   ` (6 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

t3440 showed rebase -m loses trailers after conflicts.
Each --continue spawns a fresh replay_opts without trailer_args.
Serialize them into rebase-merge/trailer so new runs reload them.
This keeps the user requested trailers intact when resuming.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 sequencer.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 552e629e4f..c02364cfce 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.
@@ -3244,6 +3245,17 @@ 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)
+					strvec_push(&opts->trailer_args, p);
+				p = nl + 1;
+			}
+			strbuf_reset(&buf);
+		}
 
 		if (read_oneliner(&ctx->current_fixups,
 				  rebase_path_current_fixups(),
@@ -3338,6 +3350,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;
 }
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 23/29] t3440: remove redundant --keep-empty
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (21 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 22/29] rebase: persist --trailer options across restarts Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 24/29] t3440: use helper for trailer checks Li Chen
                   ` (5 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

From: Li Chen <chenl311@chinatelecom.cn>

--keep-empty is the default these days so
we can drop that.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 t/t3440-rebase-trailer.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
index fbc6f209f1..4687be3a21 100755
--- a/t/t3440-rebase-trailer.sh
+++ b/t/t3440-rebase-trailer.sh
@@ -87,7 +87,7 @@ test_expect_success 'rebase --root --trailer updates every commit' '
 	create_expect initial-signed "Initial empty commit" &&
 	create_expect first-signed "first" &&
 	git checkout first &&
-	git rebase --root --keep-empty \
+	git rebase --root \
 		--trailer "Reviewed-by: Dev <dev@example.com>" &&
 	test_commit_message HEAD   first-signed &&
 	test_commit_message HEAD^  initial-signed
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 24/29] t3440: use helper for trailer checks
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (22 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 23/29] t3440: remove redundant --keep-empty Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 25/29] t3440: test --trailer without values Li Chen
                   ` (4 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Introduce expect_trailer_msg() to wrap test_commit_message
and dedupe the trailer via REVIEWED_BY_TRAILER.
Drop create_expect and temp files. In the conflict case,
assert on HEAD (rebased "third") instead of HEAD~2. Update
--apply rejection and --root tests to use the helper.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 t/t3440-rebase-trailer.sh | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
index 4687be3a21..4b0b0ee2d3 100755
--- a/t/t3440-rebase-trailer.sh
+++ b/t/t3440-rebase-trailer.sh
@@ -11,11 +11,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-rebase.sh # test_commit_message, helpers
 
-create_expect() {
-	cat >"$1" <<-EOF
+REVIEWED_BY_TRAILER="Reviewed-by: Dev <dev@example.com>"
+
+expect_trailer_msg() {
+	test_commit_message "$1" <<-EOF
 	$2
 
-	Reviewed-by: Dev <dev@example.com>
+	${3:-$REVIEWED_BY_TRAILER}
 	EOF
 }
 
@@ -32,7 +34,7 @@ test_expect_success 'setup repo with a small history' '
 test_expect_success 'apply backend is rejected with --trailer' '
 	head_before=$(git rev-parse HEAD) &&
 	test_expect_code 128 \
-	git rebase --apply --trailer "Reviewed-by: Dev <dev@example.com>" \
+	git rebase --apply --trailer "$REVIEWED_BY_TRAILER" \
 				HEAD^ 2>err &&
 	test_grep "fatal: --trailer requires the merge backend" err &&
 	test_cmp_rev HEAD $head_before
@@ -73,23 +75,20 @@ test_expect_success 'multiple Signed-off-by trailers all preserved' '
 '
 
 test_expect_success 'rebase -m --trailer adds trailer after conflicts' '
-	create_expect third-signed "third" &&
 	test_must_fail git rebase -m \
-		--trailer "Reviewed-by: Dev <dev@example.com>" \
+		--trailer "$REVIEWED_BY_TRAILER" \
 		second third &&
 	git checkout --theirs file &&
 	git add file &&
 	git rebase --continue &&
-	test_commit_message HEAD third-signed
+	expect_trailer_msg HEAD "third"
 '
 
 test_expect_success 'rebase --root --trailer updates every commit' '
-	create_expect initial-signed "Initial empty commit" &&
-	create_expect first-signed "first" &&
 	git checkout first &&
 	git rebase --root \
-		--trailer "Reviewed-by: Dev <dev@example.com>" &&
-	test_commit_message HEAD   first-signed &&
-	test_commit_message HEAD^  initial-signed
+		--trailer "$REVIEWED_BY_TRAILER" &&
+	expect_trailer_msg HEAD  "first" &&
+	expect_trailer_msg HEAD^ "Initial empty commit"
 '
 test_done
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 25/29] t3440: test --trailer without values
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (23 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 24/29] t3440: use helper for trailer checks Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 26/29] t3440: convert ex.com to example.com Li Chen
                   ` (3 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Add a regression test to ensure git
rebase --trailer accepts trailers without
values while preserving the separator’s
trailing space in the recorded message.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 t/t3440-rebase-trailer.sh | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
index 4b0b0ee2d3..bed6955001 100755
--- a/t/t3440-rebase-trailer.sh
+++ b/t/t3440-rebase-trailer.sh
@@ -50,6 +50,16 @@ test_expect_success 'reject trailer with missing key before separator' '
 	test_grep "missing key before separator" err
 '
 
+test_expect_success 'allow trailer with missing value after separator' '
+	git rebase -m --trailer "Acked-by:" HEAD~1 third &&
+	sed -e "s/_/ /g" <<-\EOF >expect &&
+	third
+
+	Acked-by:_
+	EOF
+	test_commit_message HEAD expect
+'
+
 test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last' '
 	git -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add \
 		rebase -m --trailer "Bug: 123" --trailer "Bug: 456" HEAD~1 third &&
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 26/29] t3440: convert ex.com to example.com
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (24 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 25/29] t3440: test --trailer without values Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 27/29] t3440: ensure trailers persist after rebase continue Li Chen
                   ` (2 subsequent siblings)
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

From: Li Chen <chenl311@chinatelecom.cn>

Lets use example.com here rather than some
random domain that might actually exist.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 t/t3440-rebase-trailer.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
index bed6955001..bea98d08c6 100755
--- a/t/t3440-rebase-trailer.sh
+++ b/t/t3440-rebase-trailer.sh
@@ -73,13 +73,13 @@ test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last'
 
 test_expect_success 'multiple Signed-off-by trailers all preserved' '
 	git rebase -m \
-			--trailer "Signed-off-by: Dev A <a@ex.com>" \
-			--trailer "Signed-off-by: Dev B <b@ex.com>" HEAD~1 third &&
+			--trailer "Signed-off-by: Dev A <a@example.com>" \
+			--trailer "Signed-off-by: Dev B <b@example.com>" HEAD~1 third &&
 	cat >expect <<-\EOF &&
 	third
 
-	Signed-off-by: Dev A <a@ex.com>
-	Signed-off-by: Dev B <b@ex.com>
+	Signed-off-by: Dev A <a@example.com>
+	Signed-off-by: Dev B <b@example.com>
 	EOF
 	test_commit_message HEAD expect
 '
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 27/29] t3440: ensure trailers persist after rebase continue
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (25 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 26/29] t3440: convert ex.com to example.com Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 28/29] t3440: exercise trailer config mapping Li Chen
  2025-10-22  5:39 ` [PATCH v5 29/29] sequencer: honor --trailer with fixup -C Li Chen
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Rebuilt the conflict test branch, added a fourth commit,
and asserted that both the conflicted and subsequent commits
receive the --trailer data in t/t3440-rebase-trailer.sh.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 t/t3440-rebase-trailer.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
index bea98d08c6..35d2054716 100755
--- a/t/t3440-rebase-trailer.sh
+++ b/t/t3440-rebase-trailer.sh
@@ -85,13 +85,16 @@ test_expect_success 'multiple Signed-off-by trailers all preserved' '
 '
 
 test_expect_success 'rebase -m --trailer adds trailer after conflicts' '
+	git checkout -B conflict-branch third &&
+	test_commit fourth file &&
 	test_must_fail git rebase -m \
-		--trailer "$REVIEWED_BY_TRAILER" \
-		second third &&
+			--trailer "$REVIEWED_BY_TRAILER" \
+			second &&
 	git checkout --theirs file &&
 	git add file &&
 	git rebase --continue &&
-	expect_trailer_msg HEAD "third"
+	expect_trailer_msg HEAD "fourth" &&
+	expect_trailer_msg HEAD^ "third"
 '
 
 test_expect_success 'rebase --root --trailer updates every commit' '
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 28/29] t3440: exercise trailer config mapping
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (26 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 27/29] t3440: ensure trailers persist after rebase continue Li Chen
@ 2025-10-22  5:39 ` Li Chen
  2025-10-22  5:39 ` [PATCH v5 29/29] sequencer: honor --trailer with fixup -C Li Chen
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Updated the rebase --root --trailer test
to exercise trailer.review.key configuration
and the --trailer= CLI form that uses an
equals separator, ensuring we still add the
expected Reviewed-by trailer.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 t/t3440-rebase-trailer.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
index 35d2054716..d697bf558b 100755
--- a/t/t3440-rebase-trailer.sh
+++ b/t/t3440-rebase-trailer.sh
@@ -99,8 +99,8 @@ test_expect_success 'rebase -m --trailer adds trailer after conflicts' '
 
 test_expect_success 'rebase --root --trailer updates every commit' '
 	git checkout first &&
-	git rebase --root \
-		--trailer "$REVIEWED_BY_TRAILER" &&
+	git -c trailer.review.key=Reviewed-by rebase --root \
+		--trailer=review="Dev <dev@example.com>" &&
 	expect_trailer_msg HEAD  "first" &&
 	expect_trailer_msg HEAD^ "Initial empty commit"
 '
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v5 29/29] sequencer: honor --trailer with fixup -C
  2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
                   ` (27 preceding siblings ...)
  2025-10-22  5:39 ` [PATCH v5 28/29] t3440: exercise trailer config mapping Li Chen
@ 2025-10-22  5:39 ` Li Chen
  28 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-10-22  5:39 UTC (permalink / raw)
  To: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Add an interactive rebase test that exercises
todo lists containing fixup and fixup -C commands,
and teach append_squash_message() to append trailers
when replacing the commit message.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 sequencer.c               |  4 ++++
 t/t3440-rebase-trailer.sh | 27 +++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index c02364cfce..fbf35cb474 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2027,6 +2027,10 @@ 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))
+			return error(_("unable to add trailers to commit message"));
+
 		if ((command == TODO_FIXUP) &&
 		    (flag & TODO_REPLACE_FIXUP_MSG) &&
 		    (file_exists(rebase_path_fixup_msg()) ||
diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
index d697bf558b..d0e0434664 100755
--- a/t/t3440-rebase-trailer.sh
+++ b/t/t3440-rebase-trailer.sh
@@ -97,6 +97,33 @@ test_expect_success 'rebase -m --trailer adds trailer after conflicts' '
 	expect_trailer_msg HEAD^ "third"
 '
 
+test_expect_success '--trailer handles fixup commands in todo list' '
+	git checkout -B fixup-trailer HEAD &&
+	test_commit fixup-base base &&
+	test_commit fixup-second second &&
+	first_short=$(git rev-parse --short fixup-base) &&
+	second_short=$(git rev-parse --short fixup-second) &&
+	cat >todo <<EOF &&
+pick $first_short fixup-base
+fixup $second_short fixup-second
+EOF
+	(
+		set_replace_editor todo &&
+		git rebase -i --trailer "$REVIEWED_BY_TRAILER" HEAD~2
+	) &&
+	expect_trailer_msg HEAD "fixup-base" &&
+	git reset --hard fixup-second &&
+	cat >todo <<EOF &&
+pick $first_short fixup-base
+fixup -C $second_short fixup-second
+EOF
+	(
+		set_replace_editor todo &&
+		git rebase -i --trailer "$REVIEWED_BY_TRAILER" HEAD~2
+	) &&
+	expect_trailer_msg HEAD "fixup-second"
+'
+
 test_expect_success 'rebase --root --trailer updates every commit' '
 	git checkout first &&
 	git -c trailer.review.key=Reviewed-by rebase --root \
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 01/29] trailer: append trailers in-process and drop the fork to `interpret-trailers`
  2025-10-22  5:39 ` [PATCH v5 01/29] trailer: append trailers in-process and drop the fork to `interpret-trailers` Li Chen
@ 2025-10-23 13:21   ` Phillip Wood
  2025-11-04 11:53     ` Li Chen
  0 siblings, 1 reply; 33+ messages in thread
From: Phillip Wood @ 2025-10-23 13:21 UTC (permalink / raw)
  To: Li Chen, phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Hi Li

On 22/10/2025 06:39, Li Chen wrote:
> From: Li Chen <chenl311@chinatelecom.cn>
> 
> Route all trailer insertion through trailer_process() and make
> builtin/interpret-trailers just do file I/O before calling into it.
> amend_file_with_trailers() now shares the same code path.
> 
> This removes the fork/exec and tempfile juggling, cutting overhead and
> simplifying error handling. No functional change. It also
> centralizes logic to prepare for follow-up rebase --trailer patch.
> 
> Signed-off-by: Li Chen <chenl311@chinatelecom.cn>

When I review v3 of this series I said

>> As I said above reusing the existing code as you have done here is a
>> much better approach. However it would be much easier to review if
>> the code movement was separated from the refactoring. I'm also
>> struggling to see the benefit of a lot of the refactoring - I was
>> expecting the conversion to use an strubf would essentially look like
>> fwrite() being replaced with strbuf_add() and fprintf() being
>> replaced with strbuf_addf() etc. rather than reworking the logic.

Unfortunately this version has the same code changes as v3 that make it
are virtually impossible to verify if the behavior is changed or not.
The diff below shows what I was hoping to see as the first step. It
refactors interpret_trailers() in place to factor out the code that
processes the trailers into a separate function. That makes it easy to
see that fwrite() is replaced with strbuf_add() etc. and so verify that
the behavior is unchanged. If you view the diff with "--color-moved"
you'll see that virtually all of the code in the new
interpret_trailers() function is moved from the old one which in turn
makes it easy to verify that there is no change in behavior. I would
expect the next patch in the series to move the new function for
processing trailers into trailer.c and the patch after that to refactor
amend_file_with_trailers() to add and use amend_strbuf_with_trailers()
and stop forking "git interpret-trailers". Then builtin/rebase.c can be
modified to add support for trailers using amend_strbuf_with_trailers().

Thanks

Phillip

---- 8< ----
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 41b0750e5af..4c90580ffff 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 *sb, 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, sb->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, sb->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, sb->buf + trailer_block_end(trailer_block),
+			   sb->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 sb = STRBUF_INIT;
+	struct strbuf out = STRBUF_INIT;
+	FILE *outfile = stdout;
+
+	trailer_config_init();
+
+	read_input_file(&sb, file);
+
+	if (opts->in_place)
+		outfile = create_in_place_tempfile(file);
+
+	process_trailers(opts, new_trailer_head, &sb, &out);
+
+	fwrite(out.buf, out.len, 1, 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(&out);
  }
  
  int cmd_interpret_trailers(int argc,

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 09/29] rebase: support --trailer
  2025-10-22  5:39 ` [PATCH v5 09/29] rebase: support --trailer Li Chen
@ 2025-10-23 13:21   ` Phillip Wood
  0 siblings, 0 replies; 33+ messages in thread
From: Phillip Wood @ 2025-10-23 13:21 UTC (permalink / raw)
  To: Li Chen, phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Hi Chen

The code here seems to be largely unchanged from v3. I see there are 
some later patches in response to my review. They should be squashed 
into this patch as Kristoffer has previously explained.

Thanks

Phillip

On 22/10/2025 06:39, Li Chen wrote:
> From: Li Chen <chenl311@chinatelecom.cn>
> 
> Implement a new `--trailer <text>` option for `git rebase`
> (support merge backend only now), which appends arbitrary
> trailer lines to each rebased commit message.
> 
> Reject it if the user passes an option that requires the
> apply backend (git am) since it lacks message‑filter/trailer
> hook. otherwise we can just use the merge backend.
> 
> Automatically set REBASE_FORCE when any trailer is supplied.
> 
> And reject invalid input before user edits the interactive file.
> 
> Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
> ---
>   Documentation/git-rebase.adoc |  9 +++-
>   builtin/rebase.c              | 89 +++++++++++++++++++++++++++++++++
>   sequencer.c                   | 13 +++++
>   sequencer.h                   |  4 +-
>   t/meson.build                 |  1 +
>   t/t3440-rebase-trailer.sh     | 94 +++++++++++++++++++++++++++++++++++
>   6 files changed, 208 insertions(+), 2 deletions(-)
>   create mode 100755 t/t3440-rebase-trailer.sh
> 
> diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc
> index 005caf6164..4d2fe4be6e 100644
> --- a/Documentation/git-rebase.adoc
> +++ b/Documentation/git-rebase.adoc
> @@ -487,9 +487,16 @@ See also INCOMPATIBLE OPTIONS below.
>   	Add a `Signed-off-by` trailer to all the rebased commits. Note
>   	that if `--interactive` is given then only commits marked to be
>   	picked, edited or reworded will have the trailer added.
> -+
> +
>   See also INCOMPATIBLE OPTIONS below.
>   
> +--trailer=<trailer>::
> +       Append the given trailer line(s) to every rebased commit
> +       message, processed via linkgit:git-interpret-trailers[1].
> +       When this option is present *rebase automatically implies*
> +       `--force-rebase` so that fast‑forwarded commits are also
> +       rewritten.
> +
>   -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 c468828189..3db1439b52 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;
> @@ -434,6 +442,8 @@ static int read_basic_state(struct rebase_options *opts)
>   	struct strbuf head_name = STRBUF_INIT;
>   	struct strbuf buf = STRBUF_INIT;
>   	struct object_id oid;
> +	const char trailer_state_name[] = "trailer";
> +	const char *path = state_dir_path(trailer_state_name, opts);
>   
>   	if (!read_oneliner(&head_name, state_dir_path("head-name", opts),
>   			   READ_ONELINER_WARN_MISSING) ||
> @@ -502,11 +512,31 @@ static int read_basic_state(struct rebase_options *opts)
>   
>   	strbuf_release(&buf);
>   
> +	if (strbuf_read_file(&buf, path, 0) >= 0) {
> +		const char *p = buf.buf, *end = buf.buf + buf.len;
> +
> +		while (p < end) {
> +			char *nl = memchr(p, '\n', end - p);
> +			if (!nl)
> +				die("nl shouldn't be NULL");
> +			*nl = '\0';
> +
> +			if (*p)
> +				strvec_push(&opts->trailer_args, p);
> +
> +			p = nl + 1;
> +		}
> +		strbuf_release(&buf);
> +	}
> +	strbuf_release(&buf);
> +
>   	return 0;
>   }
>   
>   static int rebase_write_basic_state(struct rebase_options *opts)
>   {
> +	const char trailer_state_name[] = "trailer";
> +
>   	write_file(state_dir_path("head-name", opts), "%s",
>   		   opts->head_name ? opts->head_name : "detached HEAD");
>   	write_file(state_dir_path("onto", opts), "%s",
> @@ -528,6 +558,21 @@ static int rebase_write_basic_state(struct rebase_options *opts)
>   	if (opts->signoff)
>   		write_file(state_dir_path("signoff", opts), "--signoff");
>   
> +	/*
> +	 * save opts->trailer_args into state_dir/trailer
> +	 */
> +	if (opts->trailer_args.nr) {
> +		struct strbuf buf = STRBUF_INIT;
> +
> +		for (size_t i = 0; i < opts->trailer_args.nr; i++) {
> +				strbuf_addstr(&buf, opts->trailer_args.v[i]);
> +				strbuf_addch(&buf, '\n');
> +		}
> +		write_file(state_dir_path(trailer_state_name, opts),
> +				   "%s", buf.buf);
> +		strbuf_release(&buf);
> +	}
> +
>   	return 0;
>   }
>   
> @@ -1084,6 +1129,35 @@ static int check_exec_cmd(const char *cmd)
>   	return 0;
>   }
>   
> +static int validate_trailer_args_after_config(const struct strvec *cli_args,
> +					      struct strbuf *err)
> +{
> +	for (size_t i = 0; i < cli_args->nr; i++) {
> +		const char *raw = cli_args->v[i];
> +		const char *txt; // Key[:=]Val
> +		const char *sep;
> +
> +		if (!skip_prefix(raw, "--trailer=", &txt))
> +			txt = raw;
> +
> +		if (!*txt) {
> +			strbuf_addstr(err, _("empty --trailer argument"));
> +			return -1;
> +		}
> +
> +		sep = strpbrk(txt, ":=");
> +
> +		/* there must be key bfore seperator */
> +		if (sep && sep == txt) {
> +			strbuf_addf(err,
> +				    _("invalid trailer '%s': missing key before separator"),
> +				    txt);
> +			return -1;
> +		}
> +	}
> +	return 0;
> +}
> +
>   int cmd_rebase(int argc,
>   	       const char **argv,
>   	       const char *prefix,
> @@ -1132,6 +1206,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 +1361,16 @@ int cmd_rebase(int argc,
>   			     builtin_rebase_options,
>   			     builtin_rebase_usage, 0);
>   
> +	if (options.trailer_args.nr) {
> +		struct strbuf err = STRBUF_INIT;
> +
> +		if (validate_trailer_args_after_config(&options.trailer_args, &err))
> +			die("%s", err.buf);
> +
> +		options.flags |= REBASE_FORCE;
> +		strbuf_release(&err);
> +	}
> +
>   	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 +1628,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 5476d39ba9..5103ae786c 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -420,6 +420,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);
>   }
> @@ -2517,6 +2518,18 @@ static int do_pick_commit(struct repository *r,
>   			_("dropping %s %s -- patch contents already upstream\n"),
>   			oid_to_hex(&commit->object.oid), msg.subject);
>   	} /* else allow == 0 and there's nothing special to do */
> +
> +	if (!res && opts->trailer_args.nr && !drop_commit) {
> +		const char *trailer_file =
> +			msg_file ? msg_file : git_path_merge_msg(r);
> +
> +		if (amend_file_with_trailers(trailer_file,
> +						&opts->trailer_args)) {
> +			res = error(_("unable to add trailers to commit message"));
> +			goto leave;
> +		}
> +	}
> +
>   	if (!opts->no_commit && !drop_commit) {
>   		if (author || command == TODO_REVERT || (flags & AMEND_MSG))
>   			res = do_commit(r, msg_file, author, reflog_action,
> diff --git a/sequencer.h b/sequencer.h
> index 719684c8a9..e21835c5a0 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -44,6 +44,7 @@ struct replay_opts {
>   	int record_origin;
>   	int no_commit;
>   	int signoff;
> +	struct strvec trailer_args;
>   	int allow_ff;
>   	int allow_rerere_auto;
>   	int allow_empty;
> @@ -82,8 +83,9 @@ struct replay_opts {
>   	struct replay_ctx *ctx;
>   };
>   #define REPLAY_OPTS_INIT {			\
> -	.edit = -1,				\
>   	.action = -1,				\
> +	.edit = -1,				\
> +	.trailer_args = STRVEC_INIT, \
>   	.xopts = STRVEC_INIT,			\
>   	.ctx = replay_ctx_new(),		\
>   }
> diff --git a/t/meson.build b/t/meson.build
> index 401b24e50e..990e8ad4be 100644
> --- a/t/meson.build
> +++ b/t/meson.build
> @@ -384,6 +384,7 @@ integration_tests = [
>     't3436-rebase-more-options.sh',
>     't3437-rebase-fixup-options.sh',
>     't3438-rebase-broken-files.sh',
> +  't3440-rebase-trailer.sh',
>     't3500-cherry.sh',
>     't3501-revert-cherry-pick.sh',
>     't3502-cherry-pick-merge.sh',
> diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh
> new file mode 100755
> index 0000000000..b1c7b03330
> --- /dev/null
> +++ b/t/t3440-rebase-trailer.sh
> @@ -0,0 +1,94 @@
> +#!/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
> +
> +create_expect() {
> +	cat >"$1" <<-EOF
> +	$2
> +
> +	Reviewed-by: Dev <dev@example.com>
> +	EOF
> +}
> +
> +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 &&
> +	ident="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" &&
> +	create_expect initial-signed  "Initial empty commit" &&
> +	create_expect first-signed    "first"                 &&
> +	create_expect second-signed   "second"                &&
> +	create_expect file2-signed    "file-2"                &&
> +	create_expect third-signed    "third"                 &&
> +	create_expect conflict-signed "conflict"
> +'
> +
> +test_expect_success 'apply backend is rejected with --trailer' '
> +	head_before=$(git rev-parse HEAD) &&
> +	test_expect_code 128 \
> +	git rebase --apply --trailer "Reviewed-by: Dev <dev@example.com>" \
> +				HEAD^ 2>err &&
> +	test_grep "requires the merge backend" err &&
> +	test_cmp_rev HEAD $head_before
> +'
> +
> +test_expect_success 'reject empty --trailer argument' '
> +	git reset --hard third &&
> +	test_expect_code 128 git rebase -m --trailer "" HEAD^ 2>err &&
> +	test_grep "empty --trailer" err
> +'
> +
> +test_expect_success 'reject trailer with missing key before separator' '
> +	git reset --hard third &&
> +	test_expect_code 128 git rebase -m --trailer ": no-key" HEAD^ 2>err &&
> +	test_grep "missing key before separator" err
> +'
> +
> +test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last' '
> +	git reset --hard third &&
> +	git -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add rebase -m --trailer "Bug: 123" --trailer "Bug: 456" HEAD~1 &&
> +	git cat-file commit HEAD | grep "^Bug: 456" &&
> +	git cat-file commit HEAD | grep -v "^Bug: 123"
> +'
> +
> +test_expect_success 'multiple Signed-off-by trailers all preserved' '
> +	git reset --hard third &&
> +	git rebase -m \
> +		--trailer "Signed-off-by: Dev A <a@ex.com>" \
> +		--trailer "Signed-off-by: Dev B <b@ex.com>" HEAD~1 &&
> +	git cat-file commit HEAD | grep -c "^Signed-off-by:" >count &&
> +	test "$(cat count)" = 2   # two new commits
> +'
> +
> +test_expect_success 'rebase -m --trailer adds trailer after conflicts' '
> +	git reset --hard third &&
> +	test_must_fail git rebase -m \
> +		--trailer "Reviewed-by: Dev <dev@example.com>" \
> +		second third &&
> +	git checkout --theirs file &&
> +	git add file &&
> +	git rebase --continue &&
> +	test_commit_message HEAD~2 file2-signed
> +'
> +
> +test_expect_success 'rebase --root --trailer updates every commit' '
> +	git checkout first &&
> +	git rebase --root --keep-empty \
> +		--trailer "Reviewed-by: Dev <dev@example.com>" &&
> +	test_commit_message HEAD   first-signed &&
> +	test_commit_message HEAD^  initial-signed
> +'
> +test_done


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 01/29] trailer: append trailers in-process and drop the fork to `interpret-trailers`
  2025-10-23 13:21   ` Phillip Wood
@ 2025-11-04 11:53     ` Li Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Li Chen @ 2025-11-04 11:53 UTC (permalink / raw)
  To: Phillip Wood; +Cc: phillipwood, git, Junio C Hamano, Kristoffer Haugsbakk

Hi Phillip,

 ---- On Thu, 23 Oct 2025 21:21:28 +0800  Phillip Wood <phillip.wood123@gmail.com> wrote --- 
 > Hi Li
 > 
 > On 22/10/2025 06:39, Li Chen wrote:
 > > From: Li Chen <chenl311@chinatelecom.cn>
 > > 
 > > Route all trailer insertion through trailer_process() and make
 > > builtin/interpret-trailers just do file I/O before calling into it.
 > > amend_file_with_trailers() now shares the same code path.
 > > 
 > > This removes the fork/exec and tempfile juggling, cutting overhead and
 > > simplifying error handling. No functional change. It also
 > > centralizes logic to prepare for follow-up rebase --trailer patch.
 > > 
 > > Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
 > 
 > When I review v3 of this series I said
 > 
 > >> As I said above reusing the existing code as you have done here is a
 > >> much better approach. However it would be much easier to review if
 > >> the code movement was separated from the refactoring. I'm also
 > >> struggling to see the benefit of a lot of the refactoring - I was
 > >> expecting the conversion to use an strubf would essentially look like
 > >> fwrite() being replaced with strbuf_add() and fprintf() being
 > >> replaced with strbuf_addf() etc. rather than reworking the logic.
 > 
 > Unfortunately this version has the same code changes as v3 that make it
 > are virtually impossible to verify if the behavior is changed or not.
 > The diff below shows what I was hoping to see as the first step. It
 > refactors interpret_trailers() in place to factor out the code that
 > processes the trailers into a separate function. That makes it easy to
 > see that fwrite() is replaced with strbuf_add() etc. and so verify that
 > the behavior is unchanged. If you view the diff with "--color-moved"
 > you'll see that virtually all of the code in the new
 > interpret_trailers() function is moved from the old one which in turn
 > makes it easy to verify that there is no change in behavior. I would
 > expect the next patch in the series to move the new function for
 > processing trailers into trailer.c and the patch after that to refactor
 > amend_file_with_trailers() to add and use amend_strbuf_with_trailers()
 > and stop forking "git interpret-trailers". Then builtin/rebase.c can be
 > modified to add support for trailers using amend_strbuf_with_trailers().
 > 
 > Thanks
 > 
 > Phillip
 > 
 > ---- 8< ----
 > diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
 > index 41b0750e5af..4c90580ffff 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 *sb, 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, sb->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, sb->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, sb->buf + trailer_block_end(trailer_block),
 > +               sb->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 sb = STRBUF_INIT;
 > +    struct strbuf out = STRBUF_INIT;
 > +    FILE *outfile = stdout;
 > +
 > +    trailer_config_init();
 > +
 > +    read_input_file(&sb, file);
 > +
 > +    if (opts->in_place)
 > +        outfile = create_in_place_tempfile(file);
 > +
 > +    process_trailers(opts, new_trailer_head, &sb, &out);
 > +
 > +    fwrite(out.buf, out.len, 1, 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(&out);
 >   }
 >   
 >   int cmd_interpret_trailers(int argc,
 > 

Thank you for your suggestion and kindness. I will re-implement the first patch based on your change and
avoid the die in interpret_trailers (as suggested in v3) and replacing the fwrite with strbuf_write (also suggested in v3).

And then add patches as you said above.

Regards,

Li​


^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2025-11-04 11:53 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-22  5:39 [PATCH v5 00/29] rebase: support --trailer Li Chen
2025-10-22  5:39 ` [PATCH v5 01/29] trailer: append trailers in-process and drop the fork to `interpret-trailers` Li Chen
2025-10-23 13:21   ` Phillip Wood
2025-11-04 11:53     ` Li Chen
2025-10-22  5:39 ` [PATCH v5 02/29] trailer: restore interpret_trailers helper Li Chen
2025-10-22  5:39 ` [PATCH v5 03/29] trailer: drop --trailer prefix handling in amend helper Li Chen
2025-10-22  5:39 ` [PATCH v5 04/29] trailer: move config_head and arg_head to if storage Li Chen
2025-10-22  5:39 ` [PATCH v5 05/29] trailer: use bool for had_trailer_before Li Chen
2025-10-22  5:39 ` [PATCH v5 06/29] interpret-trailers: buffer stdout output Li Chen
2025-10-22  5:39 ` [PATCH v5 07/29] trailer: mirror interpret-trailers output flow Li Chen
2025-10-22  5:39 ` [PATCH v5 08/29] trailer: handle trailer append failures gently Li Chen
2025-10-22  5:39 ` [PATCH v5 09/29] rebase: support --trailer Li Chen
2025-10-23 13:21   ` Phillip Wood
2025-10-22  5:39 ` [PATCH v5 10/29] rebase: inline trailer state paths Li Chen
2025-10-22  5:39 ` [PATCH v5 11/29] rebase: reuse buffer for trailer args Li Chen
2025-10-22  5:39 ` [PATCH v5 12/29] rebase: drop redundant strbuf_release call Li Chen
2025-10-22  5:39 ` [PATCH v5 13/29] rebase: skip stripping of --trailer option prefix Li Chen
2025-10-22  5:39 ` [PATCH v5 14/29] rebase: die on invalid trailer args Li Chen
2025-10-22  5:39 ` [PATCH v5 15/29] rebase: validate trailers with configured separators Li Chen
2025-10-22  5:39 ` [PATCH v5 16/29] sequencer: add trailers to message before writing file Li Chen
2025-10-22  5:39 ` [PATCH v5 17/29] t3440: create expect files at point of use Li Chen
2025-10-22  5:39 ` [PATCH v5 18/29] t3440: check apply backend error includes option Li Chen
2025-10-22  5:39 ` [PATCH v5 19/29] t3440: use test_commit_message for trailer checks Li Chen
2025-10-22  5:39 ` [PATCH v5 20/29] t3440: drop redundant resets and pass branch to rebase where needed Li Chen
2025-10-22  5:39 ` [PATCH v5 21/29] t3440: assert trailer on HEAD after conflict rebase Li Chen
2025-10-22  5:39 ` [PATCH v5 22/29] rebase: persist --trailer options across restarts Li Chen
2025-10-22  5:39 ` [PATCH v5 23/29] t3440: remove redundant --keep-empty Li Chen
2025-10-22  5:39 ` [PATCH v5 24/29] t3440: use helper for trailer checks Li Chen
2025-10-22  5:39 ` [PATCH v5 25/29] t3440: test --trailer without values Li Chen
2025-10-22  5:39 ` [PATCH v5 26/29] t3440: convert ex.com to example.com Li Chen
2025-10-22  5:39 ` [PATCH v5 27/29] t3440: ensure trailers persist after rebase continue Li Chen
2025-10-22  5:39 ` [PATCH v5 28/29] t3440: exercise trailer config mapping Li Chen
2025-10-22  5:39 ` [PATCH v5 29/29] sequencer: honor --trailer with fixup -C Li Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox