From 8efabc9e64f9c93a764b0d38981bf99d75f502bd Mon Sep 17 00:00:00 2001 From: Li Chen Date: Fri, 6 Mar 2026 14:53:27 +0000 Subject: [PATCH 1/6] interpret-trailers: factor trailer rewriting Extract the trailer rewriting logic into a helper that appends to an output strbuf. Update interpret_trailers() to handle file I/O only: read input once, call the helper, and write the buffered result. This separation makes it easier to move the helper into trailer.c in the next commit. Signed-off-by: Phillip Wood Signed-off-by: Li Chen Signed-off-by: Junio C Hamano --- builtin/interpret-trailers.c | 53 ++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index 41b0750e5a..69f9d67ec0 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -136,32 +136,21 @@ static void read_input_file(struct strbuf *sb, const char *file) strbuf_complete_line(sb); } -static void interpret_trailers(const struct process_trailer_options *opts, - struct list_head *new_trailer_head, - const char *file) +static void process_trailers(const struct process_trailer_options *opts, + struct list_head *new_trailer_head, + struct strbuf *input, struct strbuf *out) { LIST_HEAD(head); - struct strbuf sb = STRBUF_INIT; - struct strbuf trailer_block_sb = STRBUF_INIT; struct trailer_block *trailer_block; - FILE *outfile = stdout; - trailer_config_init(); - - read_input_file(&sb, file); - - if (opts->in_place) - outfile = create_in_place_tempfile(file); - - trailer_block = parse_trailers(opts, sb.buf, &head); + trailer_block = parse_trailers(opts, input->buf, &head); /* Print the lines before the trailer block */ if (!opts->only_trailers) - fwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile); + strbuf_add(out, input->buf, trailer_block_start(trailer_block)); if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block)) - fprintf(outfile, "\n"); - + strbuf_addch(out, '\n'); if (!opts->only_input) { LIST_HEAD(config_head); @@ -173,22 +162,40 @@ static void interpret_trailers(const struct process_trailer_options *opts, } /* Print trailer block. */ - format_trailers(opts, &head, &trailer_block_sb); + format_trailers(opts, &head, out); free_trailers(&head); - fwrite(trailer_block_sb.buf, 1, trailer_block_sb.len, outfile); - strbuf_release(&trailer_block_sb); /* Print the lines after the trailer block as is. */ if (!opts->only_trailers) - fwrite(sb.buf + trailer_block_end(trailer_block), 1, - sb.len - trailer_block_end(trailer_block), outfile); + strbuf_add(out, input->buf + trailer_block_end(trailer_block), + input->len - trailer_block_end(trailer_block)); trailer_block_release(trailer_block); +} +static void interpret_trailers(const struct process_trailer_options *opts, + struct list_head *new_trailer_head, + const char *file) +{ + struct strbuf input = STRBUF_INIT; + struct strbuf out = STRBUF_INIT; + FILE *outfile = stdout; + + trailer_config_init(); + + read_input_file(&input, file); + + if (opts->in_place) + outfile = create_in_place_tempfile(file); + + process_trailers(opts, new_trailer_head, &input, &out); + + strbuf_write(&out, outfile); if (opts->in_place) if (rename_tempfile(&trailers_tempfile, file)) die_errno(_("could not rename temporary file to %s"), file); - strbuf_release(&sb); + strbuf_release(&input); + strbuf_release(&out); } int cmd_interpret_trailers(int argc, From 876b2ebee2cd520f6ce78ac9bcf4c5486e509a1d Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 6 Mar 2026 14:53:28 +0000 Subject: [PATCH 2/6] interpret-trailers: refactor create_in_place_tempfile() Refactor create_in_place_tempfile() in preparation for moving it to tralier.c. Change the return type to return a `struct tempfile*` instead of a `FILE*` so that we can remove the file scope tempfile variable. Since 076aa2cbda5 (tempfile: auto-allocate tempfiles on heap, 2017-09-05) it has not been necessary to make tempfile varibales static so this is safe. Also use error() and return NULL in place of die() so the caller can exit gracefully and use find_last_dir_sep() rather than strchr() to find the parent directory. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- builtin/interpret-trailers.c | 53 ++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index 69f9d67ec0..033c2e4671 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -93,35 +93,37 @@ static int parse_opt_parse(const struct option *opt, const char *arg, return 0; } -static struct tempfile *trailers_tempfile; -static FILE *create_in_place_tempfile(const char *file) +static struct tempfile *create_in_place_tempfile(const char *file) { + struct tempfile *tempfile = NULL; struct stat st; struct strbuf filename_template = STRBUF_INIT; const char *tail; - FILE *outfile; - - if (stat(file, &st)) - die_errno(_("could not stat %s"), file); - if (!S_ISREG(st.st_mode)) - die(_("file %s is not a regular file"), file); - if (!(st.st_mode & S_IWUSR)) - die(_("file %s is not writable by user"), file); + if (stat(file, &st)) { + error_errno(_("could not stat %s"), file); + return NULL; + } + if (!S_ISREG(st.st_mode)) { + error(_("file %s is not a regular file"), file); + return NULL; + } + if (!(st.st_mode & S_IWUSR)) { + error(_("file %s is not writable by user"), file); + return NULL; + } /* Create temporary file in the same directory as the original */ - tail = strrchr(file, '/'); + tail = find_last_dir_sep(file); if (tail) strbuf_add(&filename_template, file, tail - file + 1); strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX"); - trailers_tempfile = xmks_tempfile_m(filename_template.buf, st.st_mode); - strbuf_release(&filename_template); - outfile = fdopen_tempfile(trailers_tempfile, "w"); - if (!outfile) - die_errno(_("could not open temporary file")); + tempfile = mks_tempfile_m(filename_template.buf, st.st_mode); - return outfile; + strbuf_release(&filename_template); + + return tempfile; } static void read_input_file(struct strbuf *sb, const char *file) @@ -178,20 +180,25 @@ static void interpret_trailers(const struct process_trailer_options *opts, { struct strbuf input = STRBUF_INIT; struct strbuf out = STRBUF_INIT; - FILE *outfile = stdout; + struct tempfile *tempfile = NULL; + int fd = 1; trailer_config_init(); read_input_file(&input, file); - if (opts->in_place) - outfile = create_in_place_tempfile(file); - + if (opts->in_place) { + tempfile = create_in_place_tempfile(file); + if (!tempfile) + die(NULL); + fd = tempfile->fd; + } process_trailers(opts, new_trailer_head, &input, &out); - strbuf_write(&out, outfile); + if (write_in_full(fd, out.buf, out.len) < 0) + die_errno(_("could not write to temporary file '%s'"), file); if (opts->in_place) - if (rename_tempfile(&trailers_tempfile, file)) + if (rename_tempfile(&tempfile, file)) die_errno(_("could not rename temporary file to %s"), file); strbuf_release(&input); From a4fd4c523444f6b7d11b7af4dc6d790ac4fd8ec5 Mon Sep 17 00:00:00 2001 From: Li Chen Date: Fri, 6 Mar 2026 14:53:29 +0000 Subject: [PATCH 3/6] trailer: libify a couple of functions Move create_in_place_tempfile() and process_trailers() from builtin/interpret-trailers.c into trailer.c and expose it via trailer.h. This reverts most of ae0ec2e0e0b (trailer: move interpret_trailers() to interpret-trailers.c, 2024-03-01) and lets other call sites reuse the same trailer rewriting logic. Signed-off-by: Li Chen Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- builtin/interpret-trailers.c | 71 +----------------------------------- trailer.c | 70 +++++++++++++++++++++++++++++++++++ trailer.h | 16 ++++++++ 3 files changed, 87 insertions(+), 70 deletions(-) diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index 033c2e4671..acaf42b2d9 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -93,39 +93,6 @@ static int parse_opt_parse(const struct option *opt, const char *arg, return 0; } - -static struct tempfile *create_in_place_tempfile(const char *file) -{ - struct tempfile *tempfile = NULL; - struct stat st; - struct strbuf filename_template = STRBUF_INIT; - const char *tail; - - if (stat(file, &st)) { - error_errno(_("could not stat %s"), file); - return NULL; - } - if (!S_ISREG(st.st_mode)) { - error(_("file %s is not a regular file"), file); - return NULL; - } - if (!(st.st_mode & S_IWUSR)) { - error(_("file %s is not writable by user"), file); - return NULL; - } - /* Create temporary file in the same directory as the original */ - tail = find_last_dir_sep(file); - if (tail) - strbuf_add(&filename_template, file, tail - file + 1); - strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX"); - - tempfile = mks_tempfile_m(filename_template.buf, st.st_mode); - - strbuf_release(&filename_template); - - return tempfile; -} - static void read_input_file(struct strbuf *sb, const char *file) { if (file) { @@ -138,42 +105,6 @@ static void read_input_file(struct strbuf *sb, const char *file) strbuf_complete_line(sb); } -static void process_trailers(const struct process_trailer_options *opts, - struct list_head *new_trailer_head, - struct strbuf *input, struct strbuf *out) -{ - LIST_HEAD(head); - struct trailer_block *trailer_block; - - trailer_block = parse_trailers(opts, input->buf, &head); - - /* Print the lines before the trailer block */ - if (!opts->only_trailers) - strbuf_add(out, input->buf, trailer_block_start(trailer_block)); - - if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block)) - strbuf_addch(out, '\n'); - - if (!opts->only_input) { - LIST_HEAD(config_head); - LIST_HEAD(arg_head); - parse_trailers_from_config(&config_head); - parse_trailers_from_command_line_args(&arg_head, new_trailer_head); - list_splice(&config_head, &arg_head); - process_trailers_lists(&head, &arg_head); - } - - /* Print trailer block. */ - format_trailers(opts, &head, out); - free_trailers(&head); - - /* Print the lines after the trailer block as is. */ - if (!opts->only_trailers) - strbuf_add(out, input->buf + trailer_block_end(trailer_block), - input->len - trailer_block_end(trailer_block)); - trailer_block_release(trailer_block); -} - static void interpret_trailers(const struct process_trailer_options *opts, struct list_head *new_trailer_head, const char *file) @@ -188,7 +119,7 @@ static void interpret_trailers(const struct process_trailer_options *opts, read_input_file(&input, file); if (opts->in_place) { - tempfile = create_in_place_tempfile(file); + tempfile = trailer_create_in_place_tempfile(file); if (!tempfile) die(NULL); fd = tempfile->fd; diff --git a/trailer.c b/trailer.c index 911a81ed99..163018483a 100644 --- a/trailer.c +++ b/trailer.c @@ -9,6 +9,8 @@ #include "commit.h" #include "trailer.h" #include "list.h" +#include "tempfile.h" + /* * Copyright (c) 2013, 2014 Christian Couder */ @@ -1224,6 +1226,38 @@ void trailer_iterator_release(struct trailer_iterator *iter) strbuf_release(&iter->key); } +struct tempfile *trailer_create_in_place_tempfile(const char *file) +{ + struct tempfile *tempfile = NULL; + struct stat st; + struct strbuf filename_template = STRBUF_INIT; + const char *tail; + + if (stat(file, &st)) { + error_errno(_("could not stat %s"), file); + return NULL; + } + if (!S_ISREG(st.st_mode)) { + error(_("file %s is not a regular file"), file); + return NULL; + } + if (!(st.st_mode & S_IWUSR)) { + error(_("file %s is not writable by user"), file); + return NULL; + } + /* Create temporary file in the same directory as the original */ + tail = find_last_dir_sep(file); + if (tail) + strbuf_add(&filename_template, file, tail - file + 1); + strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX"); + + tempfile = mks_tempfile_m(filename_template.buf, st.st_mode); + + strbuf_release(&filename_template); + + return tempfile; +} + int amend_file_with_trailers(const char *path, const struct strvec *trailer_args) { struct child_process run_trailer = CHILD_PROCESS_INIT; @@ -1235,3 +1269,39 @@ int amend_file_with_trailers(const char *path, const struct strvec *trailer_args strvec_pushv(&run_trailer.args, trailer_args->v); return run_command(&run_trailer); } + +void process_trailers(const struct process_trailer_options *opts, + struct list_head *new_trailer_head, + struct strbuf *input, struct strbuf *out) +{ + LIST_HEAD(head); + struct trailer_block *trailer_block; + + trailer_block = parse_trailers(opts, input->buf, &head); + + /* Print the lines before the trailer block */ + if (!opts->only_trailers) + strbuf_add(out, input->buf, trailer_block_start(trailer_block)); + + if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block)) + strbuf_addch(out, '\n'); + + if (!opts->only_input) { + LIST_HEAD(config_head); + LIST_HEAD(arg_head); + parse_trailers_from_config(&config_head); + parse_trailers_from_command_line_args(&arg_head, new_trailer_head); + list_splice(&config_head, &arg_head); + process_trailers_lists(&head, &arg_head); + } + + /* Print trailer block. */ + format_trailers(opts, &head, out); + free_trailers(&head); + + /* Print the lines after the trailer block as is. */ + if (!opts->only_trailers) + strbuf_add(out, input->buf + trailer_block_end(trailer_block), + input->len - trailer_block_end(trailer_block)); + trailer_block_release(trailer_block); +} diff --git a/trailer.h b/trailer.h index 4740549586..7fd2564e03 100644 --- a/trailer.h +++ b/trailer.h @@ -202,4 +202,20 @@ void trailer_iterator_release(struct trailer_iterator *iter); */ int amend_file_with_trailers(const char *path, const struct strvec *trailer_args); +/* + * Create a tempfile ""git-interpret-trailers-XXXXXX" in the same + * directory as file. + */ +struct tempfile *trailer_create_in_place_tempfile(const char *file); + +/* + * Rewrite the contents of input by processing its trailer block according to + * opts and (optionally) appending trailers from new_trailer_head. + * + * The rewritten message is appended to out (callers should strbuf_reset() + * first if needed). + */ +void process_trailers(const struct process_trailer_options *opts, + struct list_head *new_trailer_head, + struct strbuf *input, struct strbuf *out); #endif /* TRAILER_H */ From 6b2243fdd45f0596fc640823faaa6a1aec05a420 Mon Sep 17 00:00:00 2001 From: Li Chen Date: Fri, 6 Mar 2026 14:53:30 +0000 Subject: [PATCH 4/6] trailer: append trailers without fork/exec Introduce amend_strbuf_with_trailers() to apply trailer additions to a message buffer via process_trailers(), avoiding the need to run git interpret-trailers as a child process. Update amend_file_with_trailers() to use the in-process helper and rewrite the target file via tempfile+rename, preserving the previous in-place semantics. As the trailers are no longer added in a separate process and trailer_config_init() die()s on missing config values it is called early on in cmd_commit() and cmd_tag() so that they die() early before writing the message file. The trailer arguments are now also sanity checked. Keep existing callers unchanged by continuing to accept argv-style --trailer= entries and stripping the prefix before feeding the in-process implementation. Signed-off-by: Li Chen Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- builtin/commit.c | 3 ++ builtin/tag.c | 3 ++ trailer.c | 131 ++++++++++++++++++++++++++++++++++++++++++++--- trailer.h | 20 ++++++-- 4 files changed, 146 insertions(+), 11 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 9e3a09d532..eb9013995c 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1820,6 +1820,9 @@ int cmd_commit(int argc, argc = parse_and_validate_options(argc, argv, builtin_commit_options, builtin_commit_usage, prefix, current_head, &s); + if (trailer_args.nr) + trailer_config_init(); + if (verbose == -1) verbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose; diff --git a/builtin/tag.c b/builtin/tag.c index aeb04c487f..68b581a9c2 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -568,6 +568,9 @@ int cmd_tag(int argc, if (cmdmode == 'l') setup_auto_pager("tag", 1); + if (trailer_args.nr) + trailer_config_init(); + if (opt.sign == -1) opt.sign = cmdmode ? 0 : config_sign_tag > 0; diff --git a/trailer.c b/trailer.c index 163018483a..5eab4fa549 100644 --- a/trailer.c +++ b/trailer.c @@ -7,6 +7,7 @@ #include "string-list.h" #include "run-command.h" #include "commit.h" +#include "strvec.h" #include "trailer.h" #include "list.h" #include "tempfile.h" @@ -774,6 +775,35 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head, free(cl_separators); } +int validate_trailer_args(const struct strvec *cli_args) +{ + char *cl_separators; + int ret = 0; + + trailer_config_init(); + + cl_separators = xstrfmt("=%s", separators); + + for (size_t i = 0; i < cli_args->nr; i++) { + const char *txt = cli_args->v[i]; + ssize_t separator_pos; + + if (!*txt) { + ret = error(_("empty --trailer argument")); + goto out; + } + separator_pos = find_separator(txt, cl_separators); + if (separator_pos == 0) { + ret = error(_("invalid trailer '%s': missing key before separator"), + txt); + goto out; + } + } +out: + free(cl_separators); + return ret; +} + static const char *next_line(const char *str) { const char *nl = strchrnul(str, '\n'); @@ -1258,16 +1288,101 @@ struct tempfile *trailer_create_in_place_tempfile(const char *file) return tempfile; } -int amend_file_with_trailers(const char *path, const struct strvec *trailer_args) +int amend_strbuf_with_trailers(struct strbuf *buf, + const struct strvec *trailer_args) { - struct child_process run_trailer = CHILD_PROCESS_INIT; + struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT; + LIST_HEAD(new_trailer_head); + struct strbuf out = STRBUF_INIT; + size_t i; + int ret = 0; - 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); + opts.no_divider = 1; + + for (i = 0; i < trailer_args->nr; i++) { + const char *text = trailer_args->v[i]; + struct new_trailer_item *item; + + if (!*text) { + ret = error(_("empty --trailer argument")); + goto out; + } + item = xcalloc(1, sizeof(*item)); + item->text = xstrdup(text); + list_add_tail(&item->list, &new_trailer_head); + } + + process_trailers(&opts, &new_trailer_head, buf, &out); + + strbuf_swap(buf, &out); +out: + strbuf_release(&out); + free_trailers(&new_trailer_head); + + return ret; +} + +static int write_file_in_place(const char *path, const struct strbuf *buf) +{ + struct tempfile *tempfile = trailer_create_in_place_tempfile(path); + if (!tempfile) + return -1; + + if (write_in_full(tempfile->fd, buf->buf, buf->len) < 0) + return error_errno(_("could not write to temporary file")); + + if (rename_tempfile(&tempfile, path)) + return error_errno(_("could not rename temporary file to %s"), path); + + return 0; +} + +int amend_file_with_trailers(const char *path, + const struct strvec *trailer_args) +{ + struct strbuf buf = STRBUF_INIT; + struct strvec stripped_trailer_args = STRVEC_INIT; + int ret = 0; + size_t i; + + if (!trailer_args) + BUG("amend_file_with_trailers called with NULL trailer_args"); + if (!trailer_args->nr) + return 0; + + for (i = 0; i < trailer_args->nr; i++) { + const char *txt = trailer_args->v[i]; + + /* + * Historically amend_file_with_trailers() passed its arguments + * to "git interpret-trailers", which expected argv entries in + * "--trailer=" form. Continue to accept those for + * existing callers, but pass only the value portion to the + * in-process implementation. + */ + skip_prefix(txt, "--trailer=", &txt); + if (!*txt) { + ret = error(_("empty --trailer argument")); + goto out; + } + strvec_push(&stripped_trailer_args, txt); + } + + if (validate_trailer_args(&stripped_trailer_args)) { + ret = -1; + goto out; + } + if (strbuf_read_file(&buf, path, 0) < 0) + ret = error_errno(_("could not read '%s'"), path); + else + amend_strbuf_with_trailers(&buf, &stripped_trailer_args); + + if (!ret) + ret = write_file_in_place(path, &buf); +out: + strvec_clear(&stripped_trailer_args); + strbuf_release(&buf); + return ret; } void process_trailers(const struct process_trailer_options *opts, diff --git a/trailer.h b/trailer.h index 7fd2564e03..3c5d9a6e19 100644 --- a/trailer.h +++ b/trailer.h @@ -68,6 +68,8 @@ void parse_trailers_from_config(struct list_head *config_head); void parse_trailers_from_command_line_args(struct list_head *arg_head, struct list_head *new_trailer_head); +int validate_trailer_args(const struct strvec *cli_args); + void process_trailers_lists(struct list_head *head, struct list_head *arg_head); @@ -196,9 +198,21 @@ int trailer_iterator_advance(struct trailer_iterator *iter); void trailer_iterator_release(struct trailer_iterator *iter); /* - * Augment a file to add trailers to it by running git-interpret-trailers. - * This calls run_command() and its return value is the same (i.e. 0 for - * success, various non-zero for other errors). See run-command.h. + * Append trailers specified in trailer_args to buf in-place. + * + * Each element of trailer_args should be in the same format as the value + * accepted by --trailer= (i.e., without the --trailer= prefix). + */ +int amend_strbuf_with_trailers(struct strbuf *buf, + const struct strvec *trailer_args); + +/* + * Augment a file by appending trailers specified in trailer_args. + * + * Each element of trailer_args should be an argv-style --trailer= + * option (i.e., including the --trailer= prefix). + * + * Returns 0 on success or a non-zero error code on failure. */ int amend_file_with_trailers(const char *path, const struct strvec *trailer_args); From 5e148696bf86f0173dfc91571d15ba833ec19ccd Mon Sep 17 00:00:00 2001 From: Li Chen Date: Fri, 6 Mar 2026 14:53:31 +0000 Subject: [PATCH 5/6] commit, tag: parse --trailer with OPT_STRVEC Now that amend_file_with_trailers() expects raw trailer lines, do not store argv-style "--trailer=" strings in git commit and git tag. Parse --trailer using OPT_STRVEC so trailer_args contains only the trailer value, and drop the temporary prefix stripping in amend_file_with_trailers(). Signed-off-by: Li Chen Signed-off-by: Junio C Hamano --- builtin/commit.c | 3 ++- builtin/tag.c | 4 ++-- trailer.c | 25 ++----------------------- trailer.h | 4 ++-- 4 files changed, 8 insertions(+), 28 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index eb9013995c..3d25c1856c 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1720,7 +1720,8 @@ int cmd_commit(int argc, OPT_STRING(0, "fixup", &fixup_message, N_("[(amend|reword):]commit"), N_("use autosquash formatted message to fixup or amend/reword specified commit")), OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")), OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")), - OPT_PASSTHRU_ARGV(0, "trailer", &trailer_args, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG), + OPT_STRVEC(0, "trailer", &trailer_args, N_("trailer"), + N_("add custom trailer(s)")), OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")), OPT_FILENAME('t', "template", &template_file, N_("use specified template file")), OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")), diff --git a/builtin/tag.c b/builtin/tag.c index 68b581a9c2..e0f05f94fd 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -499,8 +499,8 @@ int cmd_tag(int argc, OPT_CALLBACK_F('m', "message", &msg, N_("message"), N_("tag message"), PARSE_OPT_NONEG, parse_msg_arg), OPT_FILENAME('F', "file", &msgfile, N_("read message from file")), - OPT_PASSTHRU_ARGV(0, "trailer", &trailer_args, N_("trailer"), - N_("add custom trailer(s)"), PARSE_OPT_NONEG), + OPT_STRVEC(0, "trailer", &trailer_args, N_("trailer"), + N_("add custom trailer(s)")), OPT_BOOL('e', "edit", &edit_flag, N_("force edit of tag message")), OPT_BOOL('s', "sign", &opt.sign, N_("annotated and GPG-signed tag")), OPT_CLEANUP(&cleanup_arg), diff --git a/trailer.c b/trailer.c index 5eab4fa549..ca8abd1882 100644 --- a/trailer.c +++ b/trailer.c @@ -1341,46 +1341,25 @@ int amend_file_with_trailers(const char *path, const struct strvec *trailer_args) { struct strbuf buf = STRBUF_INIT; - struct strvec stripped_trailer_args = STRVEC_INIT; int ret = 0; - size_t i; if (!trailer_args) BUG("amend_file_with_trailers called with NULL trailer_args"); if (!trailer_args->nr) return 0; - for (i = 0; i < trailer_args->nr; i++) { - const char *txt = trailer_args->v[i]; - - /* - * Historically amend_file_with_trailers() passed its arguments - * to "git interpret-trailers", which expected argv entries in - * "--trailer=" form. Continue to accept those for - * existing callers, but pass only the value portion to the - * in-process implementation. - */ - skip_prefix(txt, "--trailer=", &txt); - if (!*txt) { - ret = error(_("empty --trailer argument")); - goto out; - } - strvec_push(&stripped_trailer_args, txt); - } - - if (validate_trailer_args(&stripped_trailer_args)) { + if (validate_trailer_args(trailer_args)) { ret = -1; goto out; } if (strbuf_read_file(&buf, path, 0) < 0) ret = error_errno(_("could not read '%s'"), path); else - amend_strbuf_with_trailers(&buf, &stripped_trailer_args); + amend_strbuf_with_trailers(&buf, trailer_args); if (!ret) ret = write_file_in_place(path, &buf); out: - strvec_clear(&stripped_trailer_args); strbuf_release(&buf); return ret; } diff --git a/trailer.h b/trailer.h index 3c5d9a6e19..b49338858c 100644 --- a/trailer.h +++ b/trailer.h @@ -209,8 +209,8 @@ int amend_strbuf_with_trailers(struct strbuf *buf, /* * Augment a file by appending trailers specified in trailer_args. * - * Each element of trailer_args should be an argv-style --trailer= - * option (i.e., including the --trailer= prefix). + * Each element of trailer_args should be in the same format as the value + * accepted by --trailer= (i.e., without the --trailer= prefix). * * Returns 0 on success or a non-zero error code on failure. */ From e4f9d6b0ab2e1903765258991a6265599d0007ce Mon Sep 17 00:00:00 2001 From: Li Chen Date: Fri, 6 Mar 2026 14:53:32 +0000 Subject: [PATCH 6/6] rebase: support --trailer Add a new --trailer= option to git rebase to append trailer lines to each rewritten commit message (merge backend only). Because the apply backend does not provide a commit-message filter, reject --trailer when --apply is in effect and require the merge backend instead. This option implies --force-rebase so that fast-forwarded commits are also rewritten. Validate trailer arguments early to avoid starting an interactive rebase with invalid input. Add integration tests covering error paths and trailer insertion across non-interactive and interactive rebases. Signed-off-by: Li Chen Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- Documentation/git-rebase.adoc | 8 ++ builtin/rebase.c | 19 +++++ sequencer.c | 52 +++++++++++- sequencer.h | 3 + t/meson.build | 1 + t/t3440-rebase-trailer.sh | 147 ++++++++++++++++++++++++++++++++++ 6 files changed, 228 insertions(+), 2 deletions(-) create mode 100755 t/t3440-rebase-trailer.sh diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc index e177808004..f6c22d1598 100644 --- a/Documentation/git-rebase.adoc +++ b/Documentation/git-rebase.adoc @@ -497,6 +497,13 @@ See also INCOMPATIBLE OPTIONS below. + See also INCOMPATIBLE OPTIONS below. +--trailer=:: + Append the given trailer to every rebased commit message, processed + via linkgit:git-interpret-trailers[1]. This option implies + `--force-rebase`. ++ +See also INCOMPATIBLE OPTIONS below. + -i:: --interactive:: Make a list of the commits which are about to be rebased. Let the @@ -653,6 +660,7 @@ are incompatible with the following options: * --[no-]reapply-cherry-picks when used without --keep-base * --update-refs * --root when used without --onto + * --trailer In addition, the following pairs of options are incompatible: diff --git a/builtin/rebase.c b/builtin/rebase.c index c487e10907..fe25d2ad4b 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 ] " @@ -113,6 +114,7 @@ struct rebase_options { enum action action; char *reflog_action; int signoff; + struct strvec trailer_args; int allow_rerere_autoupdate; int keep_empty; int autosquash; @@ -143,6 +145,7 @@ struct rebase_options { .flags = REBASE_NO_QUIET, \ .git_am_opts = STRVEC_INIT, \ .exec = STRING_LIST_INIT_NODUP, \ + .trailer_args = STRVEC_INIT, \ .git_format_patch_opt = STRBUF_INIT, \ .fork_point = -1, \ .reapply_cherry_picks = -1, \ @@ -166,6 +169,7 @@ static void rebase_options_release(struct rebase_options *opts) free(opts->strategy); string_list_clear(&opts->strategy_opts, 0); strbuf_release(&opts->git_format_patch_opt); + strvec_clear(&opts->trailer_args); } static struct replay_opts get_replay_opts(const struct rebase_options *opts) @@ -177,6 +181,10 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) sequencer_init_config(&replay); replay.signoff = opts->signoff; + + for (size_t i = 0; i < opts->trailer_args.nr; i++) + strvec_push(&replay.trailer_args, opts->trailer_args.v[i]); + replay.allow_ff = !(opts->flags & REBASE_FORCE); if (opts->allow_rerere_autoupdate) replay.allow_rerere_auto = opts->allow_rerere_autoupdate; @@ -1132,6 +1140,8 @@ int cmd_rebase(int argc, .flags = PARSE_OPT_NOARG, .defval = REBASE_DIFFSTAT, }, + OPT_STRVEC(0, "trailer", &options.trailer_args, N_("trailer"), + N_("add custom trailer(s)")), OPT_BOOL(0, "signoff", &options.signoff, N_("add a Signed-off-by trailer to each commit")), OPT_BOOL(0, "committer-date-is-author-date", @@ -1285,6 +1295,12 @@ int cmd_rebase(int argc, builtin_rebase_options, builtin_rebase_usage, 0); + if (options.trailer_args.nr) { + if (validate_trailer_args(&options.trailer_args)) + die(NULL); + options.flags |= REBASE_FORCE; + } + if (preserve_merges_selected) die(_("--preserve-merges was replaced by --rebase-merges\n" "Note: Your `pull.rebase` configuration may also be set to 'preserve',\n" @@ -1542,6 +1558,9 @@ int cmd_rebase(int argc, if (options.root && !options.onto_name) imply_merge(&options, "--root without --onto"); + if (options.trailer_args.nr) + imply_merge(&options, "--trailer"); + if (isatty(2) && options.flags & REBASE_NO_QUIET) strbuf_addstr(&options.git_format_patch_opt, " --progress"); diff --git a/sequencer.c b/sequencer.c index a3eb39bb25..a2d72ce8b3 100644 --- a/sequencer.c +++ b/sequencer.c @@ -209,6 +209,7 @@ static GIT_PATH_FUNC(rebase_path_reschedule_failed_exec, "rebase-merge/reschedul static GIT_PATH_FUNC(rebase_path_no_reschedule_failed_exec, "rebase-merge/no-reschedule-failed-exec") static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, "rebase-merge/drop_redundant_commits") static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redundant_commits") +static GIT_PATH_FUNC(rebase_path_trailer, "rebase-merge/trailer") /* * A 'struct replay_ctx' represents the private state of the sequencer. @@ -420,6 +421,7 @@ void replay_opts_release(struct replay_opts *opts) if (opts->revs) release_revisions(opts->revs); free(opts->revs); + strvec_clear(&opts->trailer_args); replay_ctx_release(ctx); free(opts->ctx); } @@ -2019,12 +2021,15 @@ static int append_squash_message(struct strbuf *buf, const char *body, if (is_fixup_flag(command, flag) && !seen_squash(ctx)) { /* * We're replacing the commit message so we need to - * append the Signed-off-by: trailer if the user - * requested '--signoff'. + * append any trailers if the user requested + * '--signoff' or '--trailer'. */ if (opts->signoff) append_signoff(buf, 0, 0); + if (opts->trailer_args.nr) + amend_strbuf_with_trailers(buf, &opts->trailer_args); + if ((command == TODO_FIXUP) && (flag & TODO_REPLACE_FIXUP_MSG) && (file_exists(rebase_path_fixup_msg()) || @@ -2443,6 +2448,9 @@ static int do_pick_commit(struct repository *r, if (opts->signoff && !is_fixup(command)) append_signoff(&ctx->message, 0, 0); + if (opts->trailer_args.nr && !is_fixup(command)) + amend_strbuf_with_trailers(&ctx->message, &opts->trailer_args); + if (is_rebase_i(opts) && write_author_script(msg.message) < 0) res = -1; else if (!opts->strategy || @@ -3172,6 +3180,33 @@ static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf) parse_strategy_opts(opts, buf->buf); } +static int read_trailers(struct replay_opts *opts, struct strbuf *buf) +{ + ssize_t len; + + strbuf_reset(buf); + len = strbuf_read_file(buf, rebase_path_trailer(), 0); + if (len > 0) { + char *p = buf->buf, *nl; + + trailer_config_init(); + + while ((nl = strchr(p, '\n'))) { + *nl = '\0'; + if (!*p) + return error(_("trailers file contains empty line")); + strvec_push(&opts->trailer_args, p); + p = nl + 1; + } + } else if (!len) { + return error(_("trailers file is empty")); + } else if (errno != ENOENT) { + return error(_("cannot read trailers files")); + } + + return 0; +} + static int read_populate_opts(struct replay_opts *opts) { struct replay_ctx *ctx = opts->ctx; @@ -3233,6 +3268,11 @@ static int read_populate_opts(struct replay_opts *opts) opts->keep_redundant_commits = 1; read_strategy_opts(opts, &buf); + + if (read_trailers(opts, &buf)) { + ret = -1; + goto done_rebase_i; + } strbuf_reset(&buf); if (read_oneliner(&ctx->current_fixups, @@ -3328,6 +3368,14 @@ int write_basic_state(struct replay_opts *opts, const char *head_name, write_file(rebase_path_reschedule_failed_exec(), "%s", ""); else write_file(rebase_path_no_reschedule_failed_exec(), "%s", ""); + if (opts->trailer_args.nr) { + struct strbuf buf = STRBUF_INIT; + + for (size_t i = 0; i < opts->trailer_args.nr; i++) + strbuf_addf(&buf, "%s\n", opts->trailer_args.v[i]); + write_file(rebase_path_trailer(), "%s", buf.buf); + strbuf_release(&buf); + } return 0; } diff --git a/sequencer.h b/sequencer.h index 719684c8a9..bea20da085 100644 --- a/sequencer.h +++ b/sequencer.h @@ -57,6 +57,8 @@ struct replay_opts { int ignore_date; int commit_use_reference; + struct strvec trailer_args; + int mainline; char *gpg_sign; @@ -84,6 +86,7 @@ struct replay_opts { #define REPLAY_OPTS_INIT { \ .edit = -1, \ .action = -1, \ + .trailer_args = STRVEC_INIT, \ .xopts = STRVEC_INIT, \ .ctx = replay_ctx_new(), \ } diff --git a/t/meson.build b/t/meson.build index f80e366cff..1f6f8ac20b 100644 --- a/t/meson.build +++ b/t/meson.build @@ -388,6 +388,7 @@ integration_tests = [ 't3436-rebase-more-options.sh', 't3437-rebase-fixup-options.sh', 't3438-rebase-broken-files.sh', + 't3440-rebase-trailer.sh', 't3450-history.sh', 't3451-history-reword.sh', 't3500-cherry.sh', diff --git a/t/t3440-rebase-trailer.sh b/t/t3440-rebase-trailer.sh new file mode 100755 index 0000000000..8b47579566 --- /dev/null +++ b/t/t3440-rebase-trailer.sh @@ -0,0 +1,147 @@ +#!/bin/sh +# + +test_description='git rebase --trailer integration tests +We verify that --trailer works with the merge backend, +and that it is rejected early when the apply backend is requested.' + +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME + +. ./test-lib.sh +. "$TEST_DIRECTORY"/lib-rebase.sh # test_commit_message, helpers + +REVIEWED_BY_TRAILER="Reviewed-by: Dev " +SP=" " + +test_expect_success 'setup repo with a small history' ' + git commit --allow-empty -m "Initial empty commit" && + test_commit first file a && + test_commit second file && + git checkout -b conflict-branch first && + test_commit file-2 file-2 && + test_commit conflict file && + test_commit third file && + git checkout main +' + +test_expect_success 'apply backend is rejected with --trailer' ' + git checkout -B apply-backend third && + test_expect_code 128 \ + git rebase --apply --trailer "$REVIEWED_BY_TRAILER" HEAD^ 2>err && + test_grep "fatal: --trailer requires the merge backend" err +' + +test_expect_success 'reject empty --trailer argument' ' + git checkout -B empty-trailer third && + test_expect_code 128 git rebase --trailer "" HEAD^ 2>err && + test_grep "empty --trailer" err +' + +test_expect_success 'reject trailer with missing key before separator' ' + git checkout -B missing-key third && + test_expect_code 128 git rebase --trailer ": no-key" HEAD^ 2>err && + test_grep "missing key before separator" err +' + +test_expect_success 'allow trailer with missing value after separator' ' + git checkout -B missing-value third && + git rebase --trailer "Acked-by:" HEAD^ && + test_commit_message HEAD <<-EOF + third + + Acked-by:${SP} + EOF +' + +test_expect_success 'CLI trailer duplicates allowed; replace policy keeps last' ' + git checkout -B replace-policy third && + git -c trailer.Bug.ifexists=replace -c trailer.Bug.ifmissing=add \ + rebase --trailer "Bug: 123" --trailer "Bug: 456" HEAD^ && + test_commit_message HEAD <<-EOF + third + + Bug: 456 + EOF +' + +test_expect_success 'multiple Signed-off-by trailers all preserved' ' + git checkout -B multiple-signoff third && + git rebase --trailer "Signed-off-by: Dev A " \ + --trailer "Signed-off-by: Dev B " HEAD^ && + test_commit_message HEAD <<-EOF + third + + Signed-off-by: Dev A + Signed-off-by: Dev B + EOF +' + +test_expect_success 'rebase --trailer adds trailer after conflicts' ' + git checkout -B trailer-conflict third && + test_commit fourth file && + test_must_fail git rebase --trailer "$REVIEWED_BY_TRAILER" second && + git checkout --theirs file && + git add file && + git rebase --continue && + test_commit_message HEAD <<-EOF && + fourth + + $REVIEWED_BY_TRAILER + EOF + test_commit_message HEAD^ <<-EOF + third + + $REVIEWED_BY_TRAILER + EOF +' + +test_expect_success '--trailer handles fixup commands in todo list' ' + git checkout -B fixup-trailer third && + test_commit fixup-base base && + test_commit fixup-second second && + cat >todo <<-\EOF && + pick fixup-base fixup-base + fixup fixup-second fixup-second + EOF + ( + set_replace_editor todo && + git rebase -i --trailer "$REVIEWED_BY_TRAILER" HEAD~2 + ) && + test_commit_message HEAD <<-EOF && + fixup-base + + $REVIEWED_BY_TRAILER + EOF + git reset --hard fixup-second && + cat >todo <<-\EOF && + pick fixup-base fixup-base + fixup -C fixup-second fixup-second + EOF + ( + set_replace_editor todo && + git rebase -i --trailer "$REVIEWED_BY_TRAILER" HEAD~2 + ) && + test_commit_message HEAD <<-EOF + fixup-second + + $REVIEWED_BY_TRAILER + EOF +' + +test_expect_success 'rebase --root honors trailer..key' ' + git checkout -B root-trailer first && + git -c trailer.review.key=Reviewed-by rebase --root \ + --trailer=review="Dev " && + test_commit_message HEAD <<-EOF && + first + + Reviewed-by: Dev + EOF + test_commit_message HEAD^ <<-EOF + Initial empty commit + + Reviewed-by: Dev + EOF +' +test_done