From 6145e03295ca33af14683f2973b8bbcbb63b4edf Mon Sep 17 00:00:00 2001 From: Li Chen Date: Wed, 5 Nov 2025 22:29:41 +0800 Subject: interpret-trailers: factor out buffer-based processing to process_trailers() Extracted trailer processing into a helper that accumulates output in a strbuf before writing. Updated interpret_trailers() to reuse the helper, buffer output, and clean up both input and output buffers after writing. Signed-off-by: Li Chen Signed-off-by: Junio C Hamano --- builtin/interpret-trailers.c | 51 +++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index 41b0750e5a..4c90580fff 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, -- cgit v1.2.3 From 7aeb71a5167812f27a71e86d480e1e2c841f28ab Mon Sep 17 00:00:00 2001 From: Li Chen Date: Wed, 5 Nov 2025 22:29:42 +0800 Subject: trailer: move process_trailers to trailer.h This function would be used by trailer_process in following commits. Signed-off-by: Li Chen Signed-off-by: Junio C Hamano --- builtin/interpret-trailers.c | 36 ------------------------------------ trailer.c | 36 ++++++++++++++++++++++++++++++++++++ trailer.h | 3 +++ 3 files changed, 39 insertions(+), 36 deletions(-) diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index 4c90580fff..bce2e791d6 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -136,42 +136,6 @@ static void read_input_file(struct strbuf *sb, const char *file) strbuf_complete_line(sb); } -static void process_trailers(const struct process_trailer_options *opts, - struct list_head *new_trailer_head, - struct strbuf *sb, struct strbuf *out) -{ - LIST_HEAD(head); - struct trailer_block *trailer_block; - - trailer_block = parse_trailers(opts, sb->buf, &head); - - /* Print the lines before the trailer block */ - if (!opts->only_trailers) - strbuf_add(out, sb->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, 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) diff --git a/trailer.c b/trailer.c index 911a81ed99..b735ec8a53 100644 --- a/trailer.c +++ b/trailer.c @@ -1235,3 +1235,39 @@ int amend_file_with_trailers(const char *path, const struct strvec *trailer_args strvec_pushv(&run_trailer.args, trailer_args->v); return run_command(&run_trailer); } + +void process_trailers(const struct process_trailer_options *opts, + struct list_head *new_trailer_head, + struct strbuf *sb, struct strbuf *out) +{ + LIST_HEAD(head); + struct trailer_block *trailer_block; + + trailer_block = parse_trailers(opts, sb->buf, &head); + + /* Print the lines before the trailer block */ + if (!opts->only_trailers) + strbuf_add(out, sb->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, sb->buf + trailer_block_end(trailer_block), + sb->len - trailer_block_end(trailer_block)); + trailer_block_release(trailer_block); +} diff --git a/trailer.h b/trailer.h index 4740549586..44d406b763 100644 --- a/trailer.h +++ b/trailer.h @@ -202,4 +202,7 @@ void trailer_iterator_release(struct trailer_iterator *iter); */ int amend_file_with_trailers(const char *path, const struct strvec *trailer_args); +void process_trailers(const struct process_trailer_options *opts, + struct list_head *new_trailer_head, + struct strbuf *sb, struct strbuf *out); #endif /* TRAILER_H */ -- cgit v1.2.3 From 534a87d6f49c6a4313c5b224de40b2d7b47486e1 Mon Sep 17 00:00:00 2001 From: Li Chen Date: Wed, 5 Nov 2025 22:29:43 +0800 Subject: trailer: append trailers in-process and drop the fork to `interpret-trailers` 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 Signed-off-by: Junio C Hamano --- builtin/commit.c | 2 +- builtin/interpret-trailers.c | 46 ++++-------------------------- builtin/tag.c | 3 +- trailer.c | 68 ++++++++++++++++++++++++++++++++++++++------ trailer.h | 5 ++-- wrapper.c | 16 +++++++++++ wrapper.h | 6 ++++ 7 files changed, 90 insertions(+), 56 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/interpret-trailers.c b/builtin/interpret-trailers.c index bce2e791d6..268a43372b 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) { @@ -142,21 +110,15 @@ static void interpret_trailers(const struct process_trailer_options *opts, { 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); + write_file_buf(file, out.buf, out.len); + else + strbuf_write(&out, stdout); strbuf_release(&sb); strbuf_release(&out); @@ -203,6 +165,8 @@ int cmd_interpret_trailers(int argc, git_interpret_trailers_usage, options); + trailer_config_init(); + if (argc) { int i; for (i = 0; i < argc; i++) diff --git a/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 b735ec8a53..f5838f5699 100644 --- a/trailer.c +++ b/trailer.c @@ -9,6 +9,8 @@ #include "commit.h" #include "trailer.h" #include "list.h" +#include "wrapper.h" + /* * Copyright (c) 2013, 2014 Christian Couder */ @@ -1224,18 +1226,66 @@ 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 *text = trailer_args->v[i]; + struct new_trailer_item *item; + + if (!*text) + continue; + item = xcalloc(1, sizeof(*item)); + INIT_LIST_HEAD(&item->list); + item->text = text; + list_add_tail(&item->list, &new_trailer_head); + } + + process_trailers(&opts, &new_trailer_head, buf, &out); + + 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 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)) { + strbuf_release(&buf); + return error("failed to append trailers"); + } + + if (write_file_buf_gently(path, buf.buf, buf.len)) { + strbuf_release(&buf); + return -1; + } + + strbuf_release(&buf); + return 0; + } + void process_trailers(const struct process_trailer_options *opts, struct list_head *new_trailer_head, struct strbuf *sb, struct strbuf *out) diff --git a/trailer.h b/trailer.h index 44d406b763..daea46ca5d 100644 --- a/trailer.h +++ b/trailer.h @@ -196,9 +196,8 @@ 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); 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 -- cgit v1.2.3 From 036e2d476c8513037baeb8d627dcbb416417ae02 Mon Sep 17 00:00:00 2001 From: Li Chen Date: Wed, 5 Nov 2025 22:29:44 +0800 Subject: rebase: support --trailer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement a new `--trailer ` 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 Signed-off-by: Junio C Hamano --- Documentation/git-rebase.adoc | 9 ++- builtin/rebase.c | 50 ++++++++++++++++ sequencer.c | 34 +++++++++++ sequencer.h | 4 +- t/meson.build | 1 + t/t3440-rebase-trailer.sh | 134 ++++++++++++++++++++++++++++++++++++++++++ trailer.c | 29 ++++++++- trailer.h | 5 ++ 8 files changed, 262 insertions(+), 4 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=:: + 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..a88abe08b4 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; @@ -500,6 +508,23 @@ static int read_basic_state(struct rebase_options *opts) opts->gpg_sign_opt = xstrdup(buf.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; + + 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); return 0; @@ -528,6 +553,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", opts), + "%s", buf.buf); + strbuf_release(&buf); + } + return 0; } @@ -1132,6 +1172,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 +1327,11 @@ int cmd_rebase(int argc, builtin_rebase_options, builtin_rebase_usage, 0); + if (options.trailer_args.nr) { + validate_trailer_args_after_config(&options.trailer_args); + options.flags |= REBASE_FORCE; + } + if (preserve_merges_selected) die(_("--preserve-merges was replaced by --rebase-merges\n" "Note: Your `pull.rebase` configuration may also be set to 'preserve',\n" @@ -1542,6 +1589,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..fbf35cb474 100644 --- a/sequencer.c +++ b/sequencer.c @@ -209,6 +209,7 @@ static GIT_PATH_FUNC(rebase_path_reschedule_failed_exec, "rebase-merge/reschedul static GIT_PATH_FUNC(rebase_path_no_reschedule_failed_exec, "rebase-merge/no-reschedule-failed-exec") static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, "rebase-merge/drop_redundant_commits") static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redundant_commits") +static GIT_PATH_FUNC(rebase_path_trailer, "rebase-merge/trailer") /* * A 'struct replay_ctx' represents the private state of the sequencer. @@ -420,6 +421,7 @@ void replay_opts_release(struct replay_opts *opts) if (opts->revs) release_revisions(opts->revs); free(opts->revs); + strvec_clear(&opts->trailer_args); replay_ctx_release(ctx); free(opts->ctx); } @@ -2025,6 +2027,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()) || @@ -2443,6 +2449,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 || @@ -2517,6 +2531,7 @@ 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 (!opts->no_commit && !drop_commit) { if (author || command == TODO_REVERT || (flags & AMEND_MSG)) res = do_commit(r, msg_file, author, reflog_action, @@ -3234,6 +3249,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(), @@ -3328,6 +3354,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..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 a5531df415..56bc3291ce 100644 --- a/t/meson.build +++ b/t/meson.build @@ -385,6 +385,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..d0e0434664 --- /dev/null +++ b/t/t3440-rebase-trailer.sh @@ -0,0 +1,134 @@ +#!/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 " + +expect_trailer_msg() { + test_commit_message "$1" <<-EOF + $2 + + ${3:-$REVIEWED_BY_TRAILER} + 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 +' + +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_TRAILER" \ + HEAD^ 2>err && + test_grep "fatal: --trailer requires the merge backend" err && + test_cmp_rev HEAD $head_before +' + +test_expect_success 'reject empty --trailer argument' ' + 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' ' + test_expect_code 128 git rebase -m --trailer ": no-key" HEAD^ 2>err && + 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 && + cat >expect <<-\EOF && + third + + Bug: 456 + EOF + test_commit_message HEAD expect +' + +test_expect_success 'multiple Signed-off-by trailers all preserved' ' + git rebase -m \ + --trailer "Signed-off-by: Dev A " \ + --trailer "Signed-off-by: Dev B " HEAD~1 third && + cat >expect <<-\EOF && + third + + Signed-off-by: Dev A + Signed-off-by: Dev B + EOF + test_commit_message HEAD expect +' + +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 && + git checkout --theirs file && + git add file && + git rebase --continue && + expect_trailer_msg HEAD "fourth" && + 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 <todo <" && + expect_trailer_msg HEAD "first" && + expect_trailer_msg HEAD^ "Initial empty commit" +' +test_done diff --git a/trailer.c b/trailer.c index f5838f5699..f6ff2f01ee 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" @@ -774,6 +775,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'); @@ -1226,8 +1251,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 daea46ca5d..541657a11f 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); @@ -195,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. -- cgit v1.2.3