diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc index e177808004..908717991a 100644 --- a/Documentation/git-rebase.adoc +++ b/Documentation/git-rebase.adoc @@ -497,6 +497,13 @@ See also INCOMPATIBLE OPTIONS below. + See also INCOMPATIBLE OPTIONS below. +--trailer=:: + Append the given trailer to every rebased commit message, processed + via linkgit:git-interpret-trailers[1]. This option implies + `--force-rebase` so that fast-forwarded commits are also rewritten. ++ +See also INCOMPATIBLE OPTIONS below. + -i:: --interactive:: Make a list of the commits which are about to be rebased. Let the diff --git a/builtin/commit.c b/builtin/commit.c index 844bdcc728..ccd9bd7558 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/interpret-trailers.c b/builtin/interpret-trailers.c index 41b0750e5a..d4aff68746 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -140,55 +140,24 @@ 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; + struct strbuf input = STRBUF_INIT; + struct strbuf out = STRBUF_INIT; FILE *outfile = stdout; - trailer_config_init(); - - read_input_file(&sb, file); + read_input_file(&input, 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); + 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, @@ -232,6 +201,8 @@ int cmd_interpret_trailers(int argc, git_interpret_trailers_usage, options); + trailer_config_init(); + if (argc) { int i; for (i = 0; i < argc; i++) diff --git a/builtin/rebase.c b/builtin/rebase.c index c487e10907..3200506c89 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -36,6 +36,7 @@ #include "reset.h" #include "trace2.h" #include "hook.h" +#include "trailer.h" static char const * const builtin_rebase_usage[] = { N_("git rebase [-i] [options] [--exec ] " @@ -113,6 +114,7 @@ struct rebase_options { enum action action; char *reflog_action; int signoff; + struct strvec trailer_args; int allow_rerere_autoupdate; int keep_empty; int autosquash; @@ -143,6 +145,7 @@ struct rebase_options { .flags = REBASE_NO_QUIET, \ .git_am_opts = STRVEC_INIT, \ .exec = STRING_LIST_INIT_NODUP, \ + .trailer_args = STRVEC_INIT, \ .git_format_patch_opt = STRBUF_INIT, \ .fork_point = -1, \ .reapply_cherry_picks = -1, \ @@ -166,6 +169,7 @@ static void rebase_options_release(struct rebase_options *opts) free(opts->strategy); string_list_clear(&opts->strategy_opts, 0); strbuf_release(&opts->git_format_patch_opt); + strvec_clear(&opts->trailer_args); } static struct replay_opts get_replay_opts(const struct rebase_options *opts) @@ -177,6 +181,10 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) sequencer_init_config(&replay); replay.signoff = opts->signoff; + + for (size_t i = 0; i < opts->trailer_args.nr; i++) + strvec_push(&replay.trailer_args, opts->trailer_args.v[i]); + replay.allow_ff = !(opts->flags & REBASE_FORCE); if (opts->allow_rerere_autoupdate) replay.allow_rerere_auto = opts->allow_rerere_autoupdate; @@ -1132,6 +1140,8 @@ int cmd_rebase(int argc, .flags = PARSE_OPT_NOARG, .defval = REBASE_DIFFSTAT, }, + OPT_STRVEC(0, "trailer", &options.trailer_args, N_("trailer"), + N_("add custom trailer(s)")), OPT_BOOL(0, "signoff", &options.signoff, N_("add a Signed-off-by trailer to each commit")), OPT_BOOL(0, "committer-date-is-author-date", @@ -1285,6 +1295,11 @@ int cmd_rebase(int argc, builtin_rebase_options, builtin_rebase_usage, 0); + if (options.trailer_args.nr) { + validate_trailer_args(&options.trailer_args); + options.flags |= REBASE_FORCE; + } + if (preserve_merges_selected) die(_("--preserve-merges was replaced by --rebase-merges\n" "Note: Your `pull.rebase` configuration may also be set to 'preserve',\n" @@ -1542,6 +1557,9 @@ int cmd_rebase(int argc, if (options.root && !options.onto_name) imply_merge(&options, "--root without --onto"); + if (options.trailer_args.nr) + imply_merge(&options, "--trailer"); + if (isatty(2) && options.flags & REBASE_NO_QUIET) strbuf_addstr(&options.git_format_patch_opt, " --progress"); diff --git a/builtin/tag.c b/builtin/tag.c index aeb04c487f..15aee1b03a 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -499,8 +499,8 @@ int cmd_tag(int argc, OPT_CALLBACK_F('m', "message", &msg, N_("message"), N_("tag message"), PARSE_OPT_NONEG, parse_msg_arg), OPT_FILENAME('F', "file", &msgfile, N_("read message from file")), - OPT_PASSTHRU_ARGV(0, "trailer", &trailer_args, N_("trailer"), - N_("add custom trailer(s)"), PARSE_OPT_NONEG), + OPT_STRVEC(0, "trailer", &trailer_args, N_("trailer"), + N_("add custom trailer(s)")), OPT_BOOL('e', "edit", &edit_flag, N_("force edit of tag message")), OPT_BOOL('s', "sign", &opt.sign, N_("annotated and GPG-signed tag")), OPT_CLEANUP(&cleanup_arg), diff --git a/sequencer.c b/sequencer.c index c31da5ed2a..f794770bac 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); } @@ -2033,6 +2035,9 @@ static int append_squash_message(struct strbuf *buf, const char *body, if (opts->signoff) append_signoff(buf, 0, 0); + if (opts->trailer_args.nr) + amend_strbuf_with_trailers(buf, &opts->trailer_args); + if ((command == TODO_FIXUP) && (flag & TODO_REPLACE_FIXUP_MSG) && (file_exists(rebase_path_fixup_msg()) || @@ -2437,6 +2442,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 || @@ -3228,6 +3236,18 @@ static int read_populate_opts(struct replay_opts *opts) read_strategy_opts(opts, &buf); strbuf_reset(&buf); + if (strbuf_read_file(&buf, rebase_path_trailer(), 0) >= 0) { + char *p = buf.buf, *nl; + + while ((nl = strchr(p, '\n'))) { + *nl = '\0'; + if (!*p) + BUG("rebase-merge/trailer has an empty line"); + strvec_push(&opts->trailer_args, p); + p = nl + 1; + } + strbuf_reset(&buf); + } if (read_oneliner(&ctx->current_fixups, rebase_path_current_fixups(), @@ -3322,6 +3342,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 b7291cc52d..ed69698f69 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 2f74602a8b..a691283088 100644 --- a/t/meson.build +++ b/t/meson.build @@ -392,6 +392,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 diff --git a/trailer.c b/trailer.c index 911a81ed99..e85c6c9fbe 100644 --- a/trailer.c +++ b/trailer.c @@ -7,8 +7,11 @@ #include "string-list.h" #include "run-command.h" #include "commit.h" +#include "strvec.h" +#include "tempfile.h" #include "trailer.h" #include "list.h" + /* * Copyright (c) 2013, 2014 Christian Couder */ @@ -772,6 +775,30 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head, free(cl_separators); } +void validate_trailer_args(const struct strvec *cli_args) +{ + char *cl_separators; + + trailer_config_init(); + + cl_separators = xstrfmt("=%s", separators); + + for (size_t i = 0; i < cli_args->nr; i++) { + const char *txt = cli_args->v[i]; + ssize_t separator_pos; + + if (!*txt) + die(_("empty --trailer argument")); + + separator_pos = find_separator(txt, cl_separators); + if (separator_pos == 0) + die(_("invalid trailer '%s': missing key before separator"), + txt); + } + + free(cl_separators); +} + static const char *next_line(const char *str) { const char *nl = strchrnul(str, '\n'); @@ -1224,14 +1251,148 @@ void trailer_iterator_release(struct trailer_iterator *iter) strbuf_release(&iter->key); } -int amend_file_with_trailers(const char *path, const struct strvec *trailer_args) +static void new_trailer_items_clear(struct list_head *items) { - 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); + while (!list_empty(items)) { + struct new_trailer_item *item = + list_first_entry(items, struct new_trailer_item, list); + list_del(&item->list); + free(item); + } +} + +void amend_strbuf_with_trailers(struct strbuf *buf, + const struct strvec *trailer_args) +{ + struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT; + LIST_HEAD(new_trailer_head); + struct strbuf out = STRBUF_INIT; + size_t i; + + opts.no_divider = 1; + + for (i = 0; i < trailer_args->nr; i++) { + const char *text = trailer_args->v[i]; + struct new_trailer_item *item; + + if (!*text) + die(_("empty --trailer argument")); + item = xcalloc(1, sizeof(*item)); + item->text = text; + list_add_tail(&item->list, &new_trailer_head); + } + + trailer_config_init(); + process_trailers(&opts, &new_trailer_head, buf, &out); + + strbuf_swap(buf, &out); + strbuf_release(&out); + + new_trailer_items_clear(&new_trailer_head); +} + +static int write_file_in_place(const char *path, const struct strbuf *buf) +{ + struct stat st; + struct strbuf filename_template = STRBUF_INIT; + const char *tail; + struct tempfile *tempfile; + FILE *outfile; + + if (stat(path, &st)) + return error_errno(_("could not stat %s"), path); + if (!S_ISREG(st.st_mode)) + return error(_("file %s is not a regular file"), path); + if (!(st.st_mode & S_IWUSR)) + return error(_("file %s is not writable by user"), path); + + /* Create temporary file in the same directory as the original */ + tail = strrchr(path, '/'); + if (tail) + strbuf_add(&filename_template, path, tail - path + 1); + strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX"); + + tempfile = mks_tempfile_sm(filename_template.buf, 0, st.st_mode); + strbuf_release(&filename_template); + if (!tempfile) + return error_errno(_("could not create temporary file")); + + outfile = fdopen_tempfile(tempfile, "w"); + if (!outfile) { + int saved_errno = errno; + delete_tempfile(&tempfile); + errno = saved_errno; + return error_errno(_("could not open temporary file")); + } + + if (buf->len && fwrite(buf->buf, 1, buf->len, outfile) < buf->len) { + int saved_errno = errno; + delete_tempfile(&tempfile); + errno = saved_errno; + return error_errno(_("could not write to temporary file")); + } + + if (rename_tempfile(&tempfile, path)) + return error_errno(_("could not rename temporary file to %s"), path); + + return 0; +} + +int amend_file_with_trailers(const char *path, + const struct strvec *trailer_args) +{ + struct strbuf buf = STRBUF_INIT; + int ret = 0; + + if (!trailer_args) + BUG("amend_file_with_trailers called with NULL trailer_args"); + if (!trailer_args->nr) + return 0; + + if (strbuf_read_file(&buf, path, 0) < 0) + ret = error_errno(_("could not read '%s'"), path); + else + amend_strbuf_with_trailers(&buf, trailer_args); + + if (!ret) + ret = write_file_in_place(path, &buf); + + strbuf_release(&buf); + return ret; +} + +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..e5bd355aad 100644 --- a/trailer.h +++ b/trailer.h @@ -68,6 +68,8 @@ void parse_trailers_from_config(struct list_head *config_head); void parse_trailers_from_command_line_args(struct list_head *arg_head, struct list_head *new_trailer_head); +void validate_trailer_args(const struct strvec *cli_args); + void process_trailers_lists(struct list_head *head, struct list_head *arg_head); @@ -196,10 +198,32 @@ 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). + */ +void amend_strbuf_with_trailers(struct strbuf *buf, + const struct strvec *trailer_args); + +/* + * Augment a file by appending trailers specified in trailer_args. + * + * Each element of trailer_args should be 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. */ int amend_file_with_trailers(const char *path, const struct strvec *trailer_args); +/* + * Rewrite the contents of input by processing its trailer block according to + * opts and (optionally) appending trailers from new_trailer_head. + * + * The rewritten message is appended to out (callers should strbuf_reset() + * first if needed). + */ +void process_trailers(const struct process_trailer_options *opts, + struct list_head *new_trailer_head, + struct strbuf *input, struct strbuf *out); #endif /* TRAILER_H */