From 8efabc9e64f9c93a764b0d38981bf99d75f502bd Mon Sep 17 00:00:00 2001 From: Li Chen Date: Fri, 6 Mar 2026 14:53:27 +0000 Subject: [PATCH 01/18] 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 41b0750e5af324..69f9d67ec0e52a 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 02/18] 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 | 51 ++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index 69f9d67ec0e52a..033c2e46713d14 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -93,35 +93,37 @@ static int parse_opt_parse(const struct option *opt, const char *arg, return 0; } -static struct tempfile *trailers_tempfile; -static FILE *create_in_place_tempfile(const char *file) +static struct tempfile *create_in_place_tempfile(const char *file) { + struct tempfile *tempfile = NULL; struct stat st; struct strbuf filename_template = STRBUF_INIT; const char *tail; - FILE *outfile; - - if (stat(file, &st)) - die_errno(_("could not stat %s"), file); - if (!S_ISREG(st.st_mode)) - die(_("file %s is not a regular file"), file); - if (!(st.st_mode & S_IWUSR)) - die(_("file %s is not writable by user"), file); + if (stat(file, &st)) { + error_errno(_("could not stat %s"), file); + return NULL; + } + if (!S_ISREG(st.st_mode)) { + error(_("file %s is not a regular file"), file); + return NULL; + } + if (!(st.st_mode & S_IWUSR)) { + error(_("file %s is not writable by user"), file); + return NULL; + } /* Create temporary file in the same directory as the original */ - tail = strrchr(file, '/'); + tail = find_last_dir_sep(file); if (tail) strbuf_add(&filename_template, file, tail - file + 1); strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX"); - trailers_tempfile = xmks_tempfile_m(filename_template.buf, st.st_mode); + tempfile = mks_tempfile_m(filename_template.buf, st.st_mode); + strbuf_release(&filename_template); - outfile = fdopen_tempfile(trailers_tempfile, "w"); - if (!outfile) - die_errno(_("could not open temporary file")); - return outfile; + return tempfile; } static void read_input_file(struct strbuf *sb, const char *file) @@ -178,20 +180,25 @@ static void interpret_trailers(const struct process_trailer_options *opts, { struct strbuf input = STRBUF_INIT; struct strbuf out = STRBUF_INIT; - FILE *outfile = stdout; + struct tempfile *tempfile = NULL; + int fd = 1; trailer_config_init(); read_input_file(&input, file); - if (opts->in_place) - outfile = create_in_place_tempfile(file); - + if (opts->in_place) { + tempfile = create_in_place_tempfile(file); + if (!tempfile) + die(NULL); + fd = tempfile->fd; + } process_trailers(opts, new_trailer_head, &input, &out); - strbuf_write(&out, outfile); + if (write_in_full(fd, out.buf, out.len) < 0) + die_errno(_("could not write to temporary file '%s'"), file); if (opts->in_place) - if (rename_tempfile(&trailers_tempfile, file)) + if (rename_tempfile(&tempfile, file)) die_errno(_("could not rename temporary file to %s"), file); strbuf_release(&input); From a4fd4c523444f6b7d11b7af4dc6d790ac4fd8ec5 Mon Sep 17 00:00:00 2001 From: Li Chen Date: Fri, 6 Mar 2026 14:53:29 +0000 Subject: [PATCH 03/18] 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 033c2e46713d14..acaf42b2d9333b 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 911a81ed993ec6..163018483a5fb5 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 4740549586a8e5..7fd2564e035e8b 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 04/18] 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 | 133 +++++++++++++++++++++++++++++++++++++++++++---- trailer.h | 20 +++++-- 4 files changed, 147 insertions(+), 12 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 9e3a09d532bfce..eb9013995c95d4 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 aeb04c487fe95a..68b581a9c26671 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 163018483a5fb5..5eab4fa549d6e7 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; - - 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; + int ret = 0; + + opts.no_divider = 1; + + for (i = 0; i < trailer_args->nr; i++) { + const char *text = trailer_args->v[i]; + struct new_trailer_item *item; + + if (!*text) { + ret = error(_("empty --trailer argument")); + goto out; + } + item = xcalloc(1, sizeof(*item)); + item->text = xstrdup(text); + list_add_tail(&item->list, &new_trailer_head); + } + + process_trailers(&opts, &new_trailer_head, buf, &out); + + strbuf_swap(buf, &out); +out: + strbuf_release(&out); + free_trailers(&new_trailer_head); + + return ret; +} + +static int write_file_in_place(const char *path, const struct strbuf *buf) +{ + struct tempfile *tempfile = trailer_create_in_place_tempfile(path); + if (!tempfile) + return -1; + + if (write_in_full(tempfile->fd, buf->buf, buf->len) < 0) + return error_errno(_("could not write to temporary file")); + + if (rename_tempfile(&tempfile, path)) + return error_errno(_("could not rename temporary file to %s"), path); + + return 0; +} + +int amend_file_with_trailers(const char *path, + const struct strvec *trailer_args) +{ + struct strbuf buf = STRBUF_INIT; + struct strvec stripped_trailer_args = STRVEC_INIT; + int ret = 0; + size_t i; + + if (!trailer_args) + BUG("amend_file_with_trailers called with NULL trailer_args"); + if (!trailer_args->nr) + return 0; + + for (i = 0; i < trailer_args->nr; i++) { + const char *txt = trailer_args->v[i]; + + /* + * Historically amend_file_with_trailers() passed its arguments + * to "git interpret-trailers", which expected argv entries in + * "--trailer=" 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 7fd2564e035e8b..3c5d9a6e199248 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 05/18] 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 eb9013995c95d4..3d25c1856ced5e 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 68b581a9c26671..e0f05f94fdbe3e 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 5eab4fa549d6e7..ca8abd18826bf2 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 3c5d9a6e199248..b49338858c482a 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 06/18] 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 e177808004fa81..f6c22d1598978a 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 c487e1090779c2..fe25d2ad4bc7cc 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 a3eb39bb25248a..a2d72ce8b3b3c5 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 719684c8a9fb2e..bea20da085f0a2 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 f80e366cff73f3..1f6f8ac20b6c48 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 00000000000000..8b4757956603db --- /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 From 476365ac85bc7e9edd2e5bdc93ddb8a2aa9bad9a Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 9 Mar 2026 15:15:05 -0700 Subject: [PATCH 07/18] SubmittingPatches: spell out "replace fully to pretend to be perfect" It unfortunately is a recurring theme that new developers tend to pile more "fixup" patches on top of the already reviewed patches, making the topic longer and keeping the history of all wrong turns, which interests nobody in the larger picture. Even picking a narrow search in the list archive for "pretend to be a perfect " substring, we find these: https://lore.kernel.org/git/xmqqk29bsz2o.fsf@gitster.mtv.corp.google.com/ https://lore.kernel.org/git/xmqqd0ds5ysq.fsf@gitster-ct.c.googlers.com/ https://lore.kernel.org/git/xmqqr173faez.fsf@gitster.g/ The SubmittingPatches guide does talk about going incremental once a topic hits the 'next' branch, but it does not say much about how a new iteration of the topic should be prepared before that happens, and it does not mention that the developers are encouraged to seize the opportunity to pretend to be perfect with a full replacement set of patches. Add a new paragraph to stress this point in the section that describes the life-cycle of a patch series. Signed-off-by: Junio C Hamano --- Documentation/SubmittingPatches | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index e270ccbe85b087..a8cdd7aba78c00 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -43,6 +43,18 @@ help you find out who they are. respond to them with "Reply-All" on the mailing list, while taking them into account while preparing an updated set of patches. +. These early update iterations are expected to be full replacements, + not incremental updates on top of what you posted already. If you + are correcting mistakes you made in the previous iteration that a + reviewer noticed and pointed out in their review, you _fix_ that + mistake by rewriting your history (e.g., by using "git rebase -i") + to pretend that you never made the mistake in the first place. In + other words, this is a chance to pretend to be a perfect developer, + and you are expected to take advantage of that. In the larger + picture, nobody is interested in your earlier mistakes. Just + present a logical progression made by a perfect developer who makes + no mistakes while working on the topic. + . Polish, refine, and re-send your patches to the list and to the people who spent their time to improve your patch. Go back to step (2). From 8a1b789c2f926d855eea04d731d43f97c3e7c83a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 11 Mar 2026 08:09:18 +0100 Subject: [PATCH 08/18] editorconfig: fix style not applying to subdirs anymore In 046e1117d5 (templates: add .gitattributes entry for sample hooks, 2026-02-13) we have added another pattern to our EditorConfig that sets the style for our hook templates. As our templates are located in "templates/hooks/", we explicitly specify that subdirectory as part of the globbing pattern. This change causes files in other subdirectories, like for example "builtin/add.c", to not be configured properly anymore. This seems to stem from a subtlety in the EditorConfig specification [1]: If the glob contains a path separator (a / not inside square brackets), then the glob is relative to the directory level of the particular .editorconfig file itself. Otherwise the pattern may also match at any level below the .editorconfig level. What's interesting is that the _whole_ expression is considered to be the glob. So when the expression used is for example "{*.c,foo/*.h}", then it will be considered a single glob, and because it contains a path separator we will now anchor "*.c" matches to the same directory as the ".editorconfig" file. Fix this issue by splitting out the configuration for hook templates into a separate section. It leads to a tiny bit of duplication, but the alternative would be something like the following (note the "{,**/}"): [{{,**/}*.{c,h,sh,bash,perl,pl,pm,txt,adoc},config.mak.*,{,**/}Makefile,templates/hooks/*.sample}] indent_style = tab tab_width = 8 This starts to become somewhat hard to read, so the duplication feels like the better tradeoff. [1]: https://spec.editorconfig.org/#glob-expressions Signed-off-by: Patrick Steinhardt Acked-by: Phillip Wood Signed-off-by: Junio C Hamano --- .editorconfig | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.editorconfig b/.editorconfig index 6e4eaa8e955341..82e121a41754b5 100644 --- a/.editorconfig +++ b/.editorconfig @@ -4,7 +4,11 @@ insert_final_newline = true # The settings for C (*.c and *.h) files are mirrored in .clang-format. Keep # them in sync. -[{*.{c,h,sh,bash,perl,pl,pm,txt,adoc},config.mak.*,Makefile,templates/hooks/*.sample}] +[{*.{c,h,sh,bash,perl,pl,pm,txt,adoc},config.mak.*,Makefile}] +indent_style = tab +tab_width = 8 + +[templates/hooks/*.sample] indent_style = tab tab_width = 8 From 4107c0bb3455905aeacdba3be09b20e62b310eaa Mon Sep 17 00:00:00 2001 From: Deveshi Dwivedi Date: Wed, 11 Mar 2026 17:33:35 +0000 Subject: [PATCH 09/18] worktree: do not pass strbuf by value write_worktree_linking_files() takes two struct strbuf parameters by value, even though it only reads path strings from them. Passing a strbuf by value is misleading and dangerous. The structure carries a pointer to its underlying character array; caller and callee end up sharing that storage. If the callee ever causes the strbuf to be reallocated, the caller's copy becomes a dangling pointer, which results in a double-free when the caller does strbuf_release(). The function only needs the string values, not the strbuf machinery. Switch it to take const char * and update all callers to pass .buf. Signed-off-by: Deveshi Dwivedi Signed-off-by: Junio C Hamano --- builtin/worktree.c | 2 +- worktree.c | 22 +++++++++++----------- worktree.h | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index bc2d0d645ba945..4035b1cb06cba7 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -539,7 +539,7 @@ static int add_worktree(const char *path, const char *refname, strbuf_reset(&sb); strbuf_addf(&sb, "%s/gitdir", sb_repo.buf); - write_worktree_linking_files(sb_git, sb, opts->relative_paths); + write_worktree_linking_files(sb_git.buf, sb.buf, opts->relative_paths); strbuf_reset(&sb); strbuf_addf(&sb, "%s/commondir", sb_repo.buf); write_file(sb.buf, "../.."); diff --git a/worktree.c b/worktree.c index 6e2f0f78283dbf..7eba12c6edc248 100644 --- a/worktree.c +++ b/worktree.c @@ -445,7 +445,7 @@ void update_worktree_location(struct worktree *wt, const char *path_, strbuf_realpath(&path, path_, 1); strbuf_addf(&dotgit, "%s/.git", path.buf); if (fspathcmp(wt->path, path.buf)) { - write_worktree_linking_files(dotgit, gitdir, use_relative_paths); + write_worktree_linking_files(dotgit.buf, gitdir.buf, use_relative_paths); free(wt->path); wt->path = strbuf_detach(&path, NULL); @@ -684,7 +684,7 @@ static void repair_gitfile(struct worktree *wt, if (repair) { fn(0, wt->path, repair, cb_data); - write_worktree_linking_files(dotgit, gitdir, use_relative_paths); + write_worktree_linking_files(dotgit.buf, gitdir.buf, use_relative_paths); } done: @@ -742,7 +742,7 @@ void repair_worktree_after_gitdir_move(struct worktree *wt, const char *old_path if (!file_exists(dotgit.buf)) goto done; - write_worktree_linking_files(dotgit, gitdir, is_relative_path); + write_worktree_linking_files(dotgit.buf, gitdir.buf, is_relative_path); done: strbuf_release(&gitdir); strbuf_release(&dotgit); @@ -913,7 +913,7 @@ void repair_worktree_at_path(const char *path, if (repair) { fn(0, gitdir.buf, repair, cb_data); - write_worktree_linking_files(dotgit, gitdir, use_relative_paths); + write_worktree_linking_files(dotgit.buf, gitdir.buf, use_relative_paths); } done: free(dotgit_contents); @@ -1087,17 +1087,17 @@ int init_worktree_config(struct repository *r) return res; } -void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir, +void write_worktree_linking_files(const char *dotgit, const char *gitdir, int use_relative_paths) { struct strbuf path = STRBUF_INIT; struct strbuf repo = STRBUF_INIT; struct strbuf tmp = STRBUF_INIT; - strbuf_addbuf(&path, &dotgit); + strbuf_addstr(&path, dotgit); strbuf_strip_suffix(&path, "/.git"); strbuf_realpath(&path, path.buf, 1); - strbuf_addbuf(&repo, &gitdir); + strbuf_addstr(&repo, gitdir); strbuf_strip_suffix(&repo, "/gitdir"); strbuf_realpath(&repo, repo.buf, 1); @@ -1110,11 +1110,11 @@ void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir, } if (use_relative_paths) { - write_file(gitdir.buf, "%s/.git", relative_path(path.buf, repo.buf, &tmp)); - write_file(dotgit.buf, "gitdir: %s", relative_path(repo.buf, path.buf, &tmp)); + write_file(gitdir, "%s/.git", relative_path(path.buf, repo.buf, &tmp)); + write_file(dotgit, "gitdir: %s", relative_path(repo.buf, path.buf, &tmp)); } else { - write_file(gitdir.buf, "%s/.git", path.buf); - write_file(dotgit.buf, "gitdir: %s", repo.buf); + write_file(gitdir, "%s/.git", path.buf); + write_file(dotgit, "gitdir: %s", repo.buf); } strbuf_release(&path); diff --git a/worktree.h b/worktree.h index 06efe26b835a81..f4e46be385afd4 100644 --- a/worktree.h +++ b/worktree.h @@ -240,7 +240,7 @@ int init_worktree_config(struct repository *r); * dotgit: "/path/to/foo/.git" * gitdir: "/path/to/repo/worktrees/foo/gitdir" */ -void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir, +void write_worktree_linking_files(const char *dotgit, const char *gitdir, int use_relative_paths); #endif From f21967e5415673824d501b318a252c6e8a91d6fb Mon Sep 17 00:00:00 2001 From: Deveshi Dwivedi Date: Wed, 11 Mar 2026 17:33:36 +0000 Subject: [PATCH 10/18] list-objects-filter-options: avoid strbuf_split_str() parse_combine_filter() splits a combine: filter spec at '+' using strbuf_split_str(), which yields an array of strbufs with the delimiter left at the end of each non-final piece. The code then mutates each non-final piece to strip the trailing '+' before parsing. Allocating an array of strbufs is unnecessary. The function processes one sub-spec at a time and does not use strbuf editing on the pieces. The two helpers it calls, has_reserved_character() and parse_combine_subfilter(), only read the string content of the strbuf they receive. Walk the input string directly with strchrnul() to find each '+', copying each sub-spec into a reusable temporary buffer. The '+' delimiter is naturally excluded. Empty sub-specs (e.g. from a trailing '+') are silently skipped for consistency. Change the helpers to take const char * instead of struct strbuf *. The test that expected an error on a trailing '+' is removed, since that behavior was incorrect. Signed-off-by: Deveshi Dwivedi Signed-off-by: Junio C Hamano --- list-objects-filter-options.c | 40 ++++++++++++++--------------- t/t6112-rev-list-filters-objects.sh | 4 --- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c index 7f3e7b8f505ebc..cef67e591954a9 100644 --- a/list-objects-filter-options.c +++ b/list-objects-filter-options.c @@ -125,9 +125,9 @@ int gently_parse_list_objects_filter( static const char *RESERVED_NON_WS = "~`!@#$^&*()[]{}\\;'\",<>?"; static int has_reserved_character( - struct strbuf *sub_spec, struct strbuf *errbuf) + const char *sub_spec, struct strbuf *errbuf) { - const char *c = sub_spec->buf; + const char *c = sub_spec; while (*c) { if (*c <= ' ' || strchr(RESERVED_NON_WS, *c)) { strbuf_addf( @@ -144,7 +144,7 @@ static int has_reserved_character( static int parse_combine_subfilter( struct list_objects_filter_options *filter_options, - struct strbuf *subspec, + const char *subspec, struct strbuf *errbuf) { size_t new_index = filter_options->sub_nr; @@ -155,7 +155,7 @@ static int parse_combine_subfilter( filter_options->sub_alloc); list_objects_filter_init(&filter_options->sub[new_index]); - decoded = url_percent_decode(subspec->buf); + decoded = url_percent_decode(subspec); result = has_reserved_character(subspec, errbuf); if (result) @@ -182,34 +182,34 @@ static int parse_combine_filter( const char *arg, struct strbuf *errbuf) { - struct strbuf **subspecs = strbuf_split_str(arg, '+', 0); - size_t sub; + const char *p = arg; + struct strbuf sub = STRBUF_INIT; int result = 0; - if (!subspecs[0]) { + if (!*p) { strbuf_addstr(errbuf, _("expected something after combine:")); result = 1; goto cleanup; } - for (sub = 0; subspecs[sub] && !result; sub++) { - if (subspecs[sub + 1]) { - /* - * This is not the last subspec. Remove trailing "+" so - * we can parse it. - */ - size_t last = subspecs[sub]->len - 1; - assert(subspecs[sub]->buf[last] == '+'); - strbuf_remove(subspecs[sub], last, 1); - } - result = parse_combine_subfilter( - filter_options, subspecs[sub], errbuf); + while (*p && !result) { + const char *end = strchrnul(p, '+'); + + strbuf_reset(&sub); + strbuf_add(&sub, p, end - p); + + if (sub.len) + result = parse_combine_subfilter(filter_options, sub.buf, errbuf); + + if (!*end) + break; + p = end + 1; } + strbuf_release(&sub); filter_options->choice = LOFC_COMBINE; cleanup: - strbuf_list_free(subspecs); if (result) list_objects_filter_release(filter_options); return result; diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh index 0387f35a326d74..39211ef989cc58 100755 --- a/t/t6112-rev-list-filters-objects.sh +++ b/t/t6112-rev-list-filters-objects.sh @@ -483,10 +483,6 @@ test_expect_success 'combine:... with non-encoded reserved chars' ' "must escape char in sub-filter-spec: .\~." ' -test_expect_success 'validate err msg for "combine:+"' ' - expect_invalid_filter_spec combine:tree:2+ "expected .tree:." -' - test_expect_success 'combine:... with edge-case hex digits: Ff Aa 0 9' ' git -C r3 rev-list --objects --filter="combine:tree:2+bl%6Fb:n%6fne" \ HEAD >actual && From 30310f3cc474ff26b8306da48566667f67206808 Mon Sep 17 00:00:00 2001 From: Siddharth Shrimali Date: Wed, 11 Mar 2026 23:11:20 +0530 Subject: [PATCH 11/18] t3200: replace hardcoded null OID with $ZERO_OID To support the SHA-256 transition, replace the hardcoded 40-zero string in 'git branch --merged' with '$ZERO_OID'. The current 40-character string causes the test to fail prematurely in SHA-256 environments because Git identifies a "malformed object name" (due to the 40 vs 64 character mismatch) before it even validates the object type. By using '$ZERO_OID', we ensure the hash length is always correct for the active algorithm. Additionally, use 'test_grep' to verify the "must point to a commit" error message, ensuring the test validates the object type logic rather than just string syntax. Suggested-by: Patrick Steinhardt Signed-off-by: Siddharth Shrimali Signed-off-by: Junio C Hamano --- t/t3200-branch.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index c58e505c43f9b9..e7829c2c4bfdc3 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -1494,7 +1494,8 @@ test_expect_success 'refuse --edit-description on unborn branch for now' ' ' test_expect_success '--merged catches invalid object names' ' - test_must_fail git branch --merged 0000000000000000000000000000000000000000 + test_must_fail git branch --merged $ZERO_OID 2>err && + test_grep "must point to a commit" err ' test_expect_success '--list during rebase' ' From 35f220b639849d43cd8aaae34ccbe38e9935ab2b Mon Sep 17 00:00:00 2001 From: Siddharth Shrimali Date: Tue, 10 Mar 2026 22:14:12 +0530 Subject: [PATCH 12/18] submodule--helper: replace malloc with xmalloc The submodule_summary_callback() function currently uses a raw malloc() which could lead to a NULL pointer dereference. Standardize this by replacing malloc() with xmalloc() for error handling. To improve maintainability, use sizeof(*temp) instead of the struct name, and drop the typecast of void pointer assignment. Signed-off-by: Siddharth Shrimali Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index d537ab087a02e9..20dd9d04b664f2 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1059,7 +1059,7 @@ static void submodule_summary_callback(struct diff_queue_struct *q, if (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode)) continue; - temp = (struct module_cb*)malloc(sizeof(struct module_cb)); + temp = xmalloc(sizeof(*temp)); temp->mod_src = p->one->mode; temp->mod_dst = p->two->mode; temp->oid_src = p->one->oid; From e30e9442fd9b90d96dde2fad4a6b26d7a03dee5e Mon Sep 17 00:00:00 2001 From: Pablo Sabater Date: Wed, 11 Mar 2026 04:14:42 +0100 Subject: [PATCH 13/18] test-lib: print escape sequence names When printing expected/actual characters in failed checks, use their names (\a, \b, \n, ...) instead of their octal representation, making it easier to read. Add tests to test-example-tap.c Update t0080-unit-test-output.sh to match the desired output Teach 'print_one_char()' the equivalent name Signed-off-by: Pablo Sabater Signed-off-by: Junio C Hamano --- t/helper/test-example-tap.c | 4 +++ t/t0080-unit-test-output.sh | 52 +++++++++++++++++++++++-------------- t/unit-tests/test-lib.c | 19 ++++++++++++-- 3 files changed, 53 insertions(+), 22 deletions(-) diff --git a/t/helper/test-example-tap.c b/t/helper/test-example-tap.c index 229d495ecfa668..998a1f0b428bed 100644 --- a/t/helper/test-example-tap.c +++ b/t/helper/test-example-tap.c @@ -63,6 +63,8 @@ static void t_messages(void) check_str("NULL", NULL); check_char('a', ==, '\n'); check_char('\\', ==, '\''); + check_char('\a', ==, '\v'); + check_char('\x00', ==, '\x01'); } static void t_empty(void) @@ -123,6 +125,8 @@ int cmd__example_tap(int argc UNUSED, const char **argv UNUSED) check_str("NULL", NULL); check_char('a', ==, '\n'); check_char('\\', ==, '\''); + check_char('\a', ==, '\v'); + check_char('\x00', ==, '\x01'); } if_test ("if_test test with no checks") ; /* nothing */ diff --git a/t/t0080-unit-test-output.sh b/t/t0080-unit-test-output.sh index 3db10f095c2edc..66838a00b220cc 100755 --- a/t/t0080-unit-test-output.sh +++ b/t/t0080-unit-test-output.sh @@ -6,10 +6,10 @@ test_description='Test the output of the unit test framework' test_expect_success 'TAP output from unit tests' - <<\EOT cat >expect <<-EOF && - # BUG: check outside of test at t/helper/test-example-tap.c:75 + # BUG: check outside of test at t/helper/test-example-tap.c:77 ok 1 - passing test ok 2 - passing test and assertion return 1 - # check "1 == 2" failed at t/helper/test-example-tap.c:79 + # check "1 == 2" failed at t/helper/test-example-tap.c:81 # left: 1 # right: 2 not ok 3 - failing test @@ -34,53 +34,65 @@ test_expect_success 'TAP output from unit tests' - <<\EOT not ok 15 - failing check after TEST_TODO() ok 16 - failing check after TEST_TODO() returns 0 # check "!strcmp("\thello\\\\", "there\"\n")" failed at t/helper/test-example-tap.c:62 - # left: "\011hello\\\\" - # right: "there\"\012" + # left: "\thello\\\\" + # right: "there\"\n" # check "!strcmp("NULL", NULL)" failed at t/helper/test-example-tap.c:63 # left: "NULL" # right: NULL # check "'a' == '\n'" failed at t/helper/test-example-tap.c:64 # left: 'a' - # right: '\012' + # right: '\n' # check "'\\\\' == '\\''" failed at t/helper/test-example-tap.c:65 # left: '\\\\' # right: '\\'' + # check "'\a' == '\v'" failed at t/helper/test-example-tap.c:66 + # left: '\a' + # right: '\v' + # check "'\x00' == '\x01'" failed at t/helper/test-example-tap.c:67 + # left: '\000' + # right: '\001' not ok 17 - messages from failing string and char comparison - # BUG: test has no checks at t/helper/test-example-tap.c:94 + # BUG: test has no checks at t/helper/test-example-tap.c:96 not ok 18 - test with no checks ok 19 - test with no checks returns 0 ok 20 - if_test passing test - # check "1 == 2" failed at t/helper/test-example-tap.c:100 + # check "1 == 2" failed at t/helper/test-example-tap.c:102 # left: 1 # right: 2 not ok 21 - if_test failing test not ok 22 - if_test passing TEST_TODO() # TODO - # todo check 'check(1)' succeeded at t/helper/test-example-tap.c:104 + # todo check 'check(1)' succeeded at t/helper/test-example-tap.c:106 not ok 23 - if_test failing TEST_TODO() - # check "0" failed at t/helper/test-example-tap.c:106 + # check "0" failed at t/helper/test-example-tap.c:108 # skipping test - missing prerequisite - # skipping check '1' at t/helper/test-example-tap.c:108 + # skipping check '1' at t/helper/test-example-tap.c:110 ok 24 - if_test test_skip() # SKIP # skipping test - missing prerequisite ok 25 - if_test test_skip() inside TEST_TODO() # SKIP - # check "0" failed at t/helper/test-example-tap.c:113 + # check "0" failed at t/helper/test-example-tap.c:115 not ok 26 - if_test TEST_TODO() after failing check - # check "0" failed at t/helper/test-example-tap.c:119 + # check "0" failed at t/helper/test-example-tap.c:121 not ok 27 - if_test failing check after TEST_TODO() - # check "!strcmp("\thello\\\\", "there\"\n")" failed at t/helper/test-example-tap.c:122 - # left: "\011hello\\\\" - # right: "there\"\012" - # check "!strcmp("NULL", NULL)" failed at t/helper/test-example-tap.c:123 + # check "!strcmp("\thello\\\\", "there\"\n")" failed at t/helper/test-example-tap.c:124 + # left: "\thello\\\\" + # right: "there\"\n" + # check "!strcmp("NULL", NULL)" failed at t/helper/test-example-tap.c:125 # left: "NULL" # right: NULL - # check "'a' == '\n'" failed at t/helper/test-example-tap.c:124 + # check "'a' == '\n'" failed at t/helper/test-example-tap.c:126 # left: 'a' - # right: '\012' - # check "'\\\\' == '\\''" failed at t/helper/test-example-tap.c:125 + # right: '\n' + # check "'\\\\' == '\\''" failed at t/helper/test-example-tap.c:127 # left: '\\\\' # right: '\\'' + # check "'\a' == '\v'" failed at t/helper/test-example-tap.c:128 + # left: '\a' + # right: '\v' + # check "'\x00' == '\x01'" failed at t/helper/test-example-tap.c:129 + # left: '\000' + # right: '\001' not ok 28 - if_test messages from failing string and char comparison - # BUG: test has no checks at t/helper/test-example-tap.c:127 + # BUG: test has no checks at t/helper/test-example-tap.c:131 not ok 29 - if_test test with no checks 1..29 EOF diff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c index 87e1f5c201f253..72ee20a06f3951 100644 --- a/t/unit-tests/test-lib.c +++ b/t/unit-tests/test-lib.c @@ -396,8 +396,23 @@ int check_uint_loc(const char *loc, const char *check, int ok, static void print_one_char(char ch, char quote) { if ((unsigned char)ch < 0x20u || ch == 0x7f) { - /* TODO: improve handling of \a, \b, \f ... */ - printf("\\%03o", (unsigned char)ch); + char esc; + switch (ch) { + case '\a': esc = 'a'; break; + case '\b': esc = 'b'; break; + case '\t': esc = 't'; break; + case '\n': esc = 'n'; break; + case '\v': esc = 'v'; break; + case '\f': esc = 'f'; break; + case '\r': esc = 'r'; break; + default: esc = 0; break; + } + if (esc) { + putc('\\', stdout); + putc(esc, stdout); + } else { + printf("\\%03o", (unsigned char)ch); + } } else { if (ch == '\\' || ch == quote) putc('\\', stdout); From 6523589a2c034de168f6240d040e0910f7aeddda Mon Sep 17 00:00:00 2001 From: Pablo Sabater Date: Wed, 11 Mar 2026 20:40:02 +0100 Subject: [PATCH 14/18] t9200: handle missing CVS with skip_all CVS initialization runs outside a test_expect_success and when it fails, the error report isn't good. Wrap CVS initialization in a skip_all check so when CVS initialization fails, the error report becomes clearer. Move the Git repo initialization into its own test_expect_success instead of being in the same CVS check. Signed-off-by: Pablo Sabater Signed-off-by: Junio C Hamano --- t/t9200-git-cvsexportcommit.sh | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh index a44eabf0d80fa8..cba3b1a28a327c 100755 --- a/t/t9200-git-cvsexportcommit.sh +++ b/t/t9200-git-cvsexportcommit.sh @@ -30,13 +30,17 @@ export CVSROOT CVSWORK GIT_DIR rm -rf "$CVSROOT" "$CVSWORK" -cvs init && -test -d "$CVSROOT" && -cvs -Q co -d "$CVSWORK" . && -echo >empty && -git add empty && -git commit -q -a -m "Initial" 2>/dev/null || -exit 1 +if ! cvs init || ! test -d "$CVSROOT" || ! cvs -Q co -d "$CVSWORK" . +then + skip_all="cvs repository set-up fails" + test_done +fi + +test_expect_success 'git setup' ' + echo >empty && + git add empty && + git commit -q -a -m Initial +' check_entries () { # $1 == directory, $2 == expected From 05c324b92fe723674cbf9ae1b0b1675821b6c275 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Kaan=20Kara=C3=A7ay?= Date: Thu, 12 Mar 2026 17:44:36 +0300 Subject: [PATCH 15/18] run-command: wean start_command() off the_repository MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The start_command() relies on the_repository due to the close_object_store flag in 'struct child_process'. When this flag is set, start_command() closes the object store associated with the_repository before spawning a child process. To eliminate this dependency, replace the 'close_object_store' with the new 'struct object_database *odb_to_close' field. This allows callers to specify the object store that needs to be closed. Suggested-by: René Scharfe Signed-off-by: Burak Kaan Karaçay Signed-off-by: Junio C Hamano --- builtin/gc.c | 14 +++++++++----- builtin/pull.c | 2 +- run-command.c | 6 +++--- run-command.h | 2 +- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index fb329c2cffab80..5d8d358f7a511d 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1030,7 +1030,7 @@ int cmd_gc(int argc, struct child_process repack_cmd = CHILD_PROCESS_INIT; repack_cmd.git_cmd = 1; - repack_cmd.close_object_store = 1; + repack_cmd.odb_to_close = the_repository->objects; strvec_pushv(&repack_cmd.args, repack_args.v); if (run_command(&repack_cmd)) die(FAILED_RUN, repack_args.v[0]); @@ -1199,7 +1199,8 @@ static int run_write_commit_graph(struct maintenance_run_opts *opts) { struct child_process child = CHILD_PROCESS_INIT; - child.git_cmd = child.close_object_store = 1; + child.git_cmd = 1; + child.odb_to_close = the_repository->objects; strvec_pushl(&child.args, "commit-graph", "write", "--split", "--reachable", NULL); @@ -1268,7 +1269,8 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts, { struct child_process child = CHILD_PROCESS_INIT; - child.git_cmd = child.close_object_store = 1; + child.git_cmd = 1; + child.odb_to_close = the_repository->objects; strvec_push(&child.args, "gc"); if (opts->auto_flag) @@ -1484,7 +1486,8 @@ static int multi_pack_index_expire(struct maintenance_run_opts *opts) { struct child_process child = CHILD_PROCESS_INIT; - child.git_cmd = child.close_object_store = 1; + child.git_cmd = 1; + child.odb_to_close = the_repository->objects; strvec_pushl(&child.args, "multi-pack-index", "expire", NULL); if (opts->quiet) @@ -1542,7 +1545,8 @@ static int multi_pack_index_repack(struct maintenance_run_opts *opts) { struct child_process child = CHILD_PROCESS_INIT; - child.git_cmd = child.close_object_store = 1; + child.git_cmd = 1; + child.odb_to_close = the_repository->objects; strvec_pushl(&child.args, "multi-pack-index", "repack", NULL); if (opts->quiet) diff --git a/builtin/pull.c b/builtin/pull.c index 6ad420ce6f9b41..7e67fdce97fd1d 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -454,7 +454,7 @@ static int run_fetch(const char *repo, const char **refspecs) } else if (*refspecs) BUG("refspecs without repo?"); cmd.git_cmd = 1; - cmd.close_object_store = 1; + cmd.odb_to_close = the_repository->objects; return run_command(&cmd); } diff --git a/run-command.c b/run-command.c index b27064ef5788a5..ed5e8be9761bcb 100644 --- a/run-command.c +++ b/run-command.c @@ -742,8 +742,8 @@ int start_command(struct child_process *cmd) fflush(NULL); - if (cmd->close_object_store) - odb_close(the_repository->objects); + if (cmd->odb_to_close) + odb_close(cmd->odb_to_close); #ifndef GIT_WINDOWS_NATIVE { @@ -1955,7 +1955,7 @@ int prepare_auto_maintenance(int quiet, struct child_process *maint) auto_detach = git_env_bool("GIT_TEST_MAINT_AUTO_DETACH", true); maint->git_cmd = 1; - maint->close_object_store = 1; + maint->odb_to_close = the_repository->objects; strvec_pushl(&maint->args, "maintenance", "run", "--auto", NULL); strvec_push(&maint->args, quiet ? "--quiet" : "--no-quiet"); strvec_push(&maint->args, auto_detach ? "--detach" : "--no-detach"); diff --git a/run-command.h b/run-command.h index e1ca965b5b1988..af4c9da279594a 100644 --- a/run-command.h +++ b/run-command.h @@ -136,7 +136,7 @@ struct child_process { * want to repack because that would delete `.pack` files (and on * Windows, you cannot delete files that are still in use). */ - unsigned close_object_store:1; + struct object_database *odb_to_close; unsigned stdout_to_stderr:1; unsigned clean_on_exit:1; From 9df3be8e2e7e2c9bf200de4bcfbd4e690a57f033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Kaan=20Kara=C3=A7ay?= Date: Thu, 12 Mar 2026 17:44:37 +0300 Subject: [PATCH 16/18] run-command: wean auto_maintenance() functions off the_repository MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prepare_auto_maintenance() relies on the_repository to read configurations. Since run_auto_maintenance() calls prepare_auto_maintenance(), it also implicitly depends the_repository. Add 'struct repository *' as a parameter to both functions and update all callers to pass the_repository. With no global repository dependencies left in this file, remove the USE_THE_REPOSITORY_VARIABLE macro. Suggested-by: Patrick Steinhardt Signed-off-by: Burak Kaan Karaçay Signed-off-by: Junio C Hamano --- builtin/am.c | 2 +- builtin/commit.c | 2 +- builtin/fetch.c | 2 +- builtin/merge.c | 2 +- builtin/rebase.c | 4 +++- builtin/receive-pack.c | 2 +- run-command.c | 16 ++++++++-------- run-command.h | 7 +++++-- 8 files changed, 21 insertions(+), 16 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index e0c767e223dbce..9d0b51c651924a 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1937,7 +1937,7 @@ static void am_run(struct am_state *state, int resume) */ if (!state->rebasing) { am_destroy(state); - run_auto_maintenance(state->quiet); + run_auto_maintenance(the_repository, state->quiet); } } diff --git a/builtin/commit.c b/builtin/commit.c index 844bdcc72803b9..7b23c1f883e718 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1958,7 +1958,7 @@ int cmd_commit(int argc, git_test_write_commit_graph_or_die(the_repository->objects->sources); repo_rerere(the_repository, 0); - run_auto_maintenance(quiet); + run_auto_maintenance(the_repository, quiet); run_commit_hook(use_editor, repo_get_index_file(the_repository), NULL, "post-commit", NULL); if (amend && !no_post_rewrite) { diff --git a/builtin/fetch.c b/builtin/fetch.c index 8a36cf67b5f140..4795b2a13c30e3 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -2873,7 +2873,7 @@ int cmd_fetch(int argc, if (opt_val != 0) git_config_push_parameter("maintenance.incremental-repack.auto=-1"); } - run_auto_maintenance(verbosity < 0); + run_auto_maintenance(the_repository, verbosity < 0); } cleanup: diff --git a/builtin/merge.c b/builtin/merge.c index 4e456a381c192d..2cbce56f8da9f7 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -506,7 +506,7 @@ static void finish(struct commit *head_commit, * We ignore errors in 'gc --auto', since the * user should see them. */ - run_auto_maintenance(verbosity < 0); + run_auto_maintenance(the_repository, verbosity < 0); } } if (new_head && show_diffstat) { diff --git a/builtin/rebase.c b/builtin/rebase.c index c487e1090779c2..8c1316db38b48d 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -562,7 +562,9 @@ static int finish_rebase(struct rebase_options *opts) * We ignore errors in 'git maintenance run --auto', since the * user should see them. */ - run_auto_maintenance(!(opts->flags & (REBASE_NO_QUIET|REBASE_VERBOSE))); + run_auto_maintenance(the_repository, + !(opts->flags & (REBASE_NO_QUIET|REBASE_VERBOSE))); + if (opts->type == REBASE_MERGE) { struct replay_opts replay = REPLAY_OPTS_INIT; diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index d6225df890d888..e34edff406959a 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -2727,7 +2727,7 @@ int cmd_receive_pack(int argc, if (auto_gc) { struct child_process proc = CHILD_PROCESS_INIT; - if (prepare_auto_maintenance(1, &proc)) { + if (prepare_auto_maintenance(the_repository, 1, &proc)) { proc.no_stdin = 1; proc.stdout_to_stderr = 1; proc.err = use_sideband ? -1 : 0; diff --git a/run-command.c b/run-command.c index ed5e8be9761bcb..38f4c699f81a7a 100644 --- a/run-command.c +++ b/run-command.c @@ -1,4 +1,3 @@ -#define USE_THE_REPOSITORY_VARIABLE #define DISABLE_SIGN_COMPARE_WARNINGS #include "git-compat-util.h" @@ -1937,11 +1936,12 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) trace2_region_leave(tr2_category, tr2_label, NULL); } -int prepare_auto_maintenance(int quiet, struct child_process *maint) +int prepare_auto_maintenance(struct repository *r, int quiet, + struct child_process *maint) { int enabled, auto_detach; - if (!repo_config_get_bool(the_repository, "maintenance.auto", &enabled) && + if (!repo_config_get_bool(r, "maintenance.auto", &enabled) && !enabled) return 0; @@ -1950,12 +1950,12 @@ int prepare_auto_maintenance(int quiet, struct child_process *maint) * honoring `gc.autoDetach`. This is somewhat weird, but required to * retain behaviour from when we used to run git-gc(1) here. */ - if (repo_config_get_bool(the_repository, "maintenance.autodetach", &auto_detach) && - repo_config_get_bool(the_repository, "gc.autodetach", &auto_detach)) + if (repo_config_get_bool(r, "maintenance.autodetach", &auto_detach) && + repo_config_get_bool(r, "gc.autodetach", &auto_detach)) auto_detach = git_env_bool("GIT_TEST_MAINT_AUTO_DETACH", true); maint->git_cmd = 1; - maint->odb_to_close = the_repository->objects; + maint->odb_to_close = r->objects; strvec_pushl(&maint->args, "maintenance", "run", "--auto", NULL); strvec_push(&maint->args, quiet ? "--quiet" : "--no-quiet"); strvec_push(&maint->args, auto_detach ? "--detach" : "--no-detach"); @@ -1963,10 +1963,10 @@ int prepare_auto_maintenance(int quiet, struct child_process *maint) return 1; } -int run_auto_maintenance(int quiet) +int run_auto_maintenance(struct repository *r, int quiet) { struct child_process maint = CHILD_PROCESS_INIT; - if (!prepare_auto_maintenance(quiet, &maint)) + if (!prepare_auto_maintenance(r, quiet, &maint)) return 0; return run_command(&maint); } diff --git a/run-command.h b/run-command.h index af4c9da279594a..ad25740fe6c092 100644 --- a/run-command.h +++ b/run-command.h @@ -5,6 +5,8 @@ #include "strvec.h" +struct repository; + /** * The run-command API offers a versatile tool to run sub-processes with * redirected input and output as well as with a modified environment @@ -227,12 +229,13 @@ int run_command(struct child_process *); * process has been prepared and is ready to run, or 0 in case auto-maintenance * should be skipped. */ -int prepare_auto_maintenance(int quiet, struct child_process *maint); +int prepare_auto_maintenance(struct repository *r, int quiet, + struct child_process *maint); /* * Trigger an auto-gc */ -int run_auto_maintenance(int quiet); +int run_auto_maintenance(struct repository *r, int quiet); /** * Execute the given command, sending "in" to its stdin, and capturing its From 90725761237c302e693089c8d9c4f2b206369a82 Mon Sep 17 00:00:00 2001 From: Pablo Sabater Date: Thu, 12 Mar 2026 18:33:05 +0100 Subject: [PATCH 17/18] t9200: replace test -f with modern path helper Replace old style 'test -f' with helper 'test_path_is_file', which make debugging a failing test easier by loudly reporting what expectation was not met. Signed-off-by: Pablo Sabater Signed-off-by: Junio C Hamano --- t/t9200-git-cvsexportcommit.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh index cba3b1a28a327c..14cbe9652779bc 100755 --- a/t/t9200-git-cvsexportcommit.sh +++ b/t/t9200-git-cvsexportcommit.sh @@ -307,7 +307,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' ' git commit -m "Added attic_gremlin" && git cvsexportcommit -w "$CVSWORK" -c HEAD && (cd "$CVSWORK" && cvs -Q update -d) && - test -f "$CVSWORK/attic_gremlin" + test_path_is_file "$CVSWORK/attic_gremlin" ' # the state of the CVS sandbox may be indeterminate for ' space' From 7ff1e8dc1e1680510c96e69965b3fa81372c5037 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 19 Mar 2026 09:54:41 -0700 Subject: [PATCH 18/18] The 18th batch Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.54.0.adoc | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/Documentation/RelNotes/2.54.0.adoc b/Documentation/RelNotes/2.54.0.adoc index d7c67cbb078aeb..d5e833512ee66a 100644 --- a/Documentation/RelNotes/2.54.0.adoc +++ b/Documentation/RelNotes/2.54.0.adoc @@ -88,6 +88,9 @@ UI, Workflows & Features * "git repo structure" command learns to report maximum values on various aspects of objects it inspects. + * "git rebase" learns "--trailer" command to drive the + interpret-trailers machinery. + Performance, Internal Implementation, Development Support etc. -------------------------------------------------------------- @@ -193,6 +196,20 @@ Performance, Internal Implementation, Development Support etc. * Reduce dependence on the global the_hash_algo and the_repository variables of wt-status code path. + * The way combined list-object filter options are parsed has been + revamped. + + * Editorconfig filename patterns were specified incorrectly, making + many source files inside subdirectories unaffected, which has been + corrected. + + * The run_command() API lost its implicit dependencyon the singleton + `the_repository` instance. + + * The unit test helper function was taught to use backslash + + mnemonic notation for certain control characters like "\t", instead + of octal notation like "\011". + Fixes since v2.53 ----------------- @@ -323,6 +340,10 @@ Fixes since v2.53 * Plug a few leaks where mmap'ed memory regions are not unmapped. (merge a8a69bbb64 jk/unleak-mmap later to maint). + * A test now uses the symbolic constant $ZERO_OID instead of 40 "0" to + work better with SHA-256 as well as SHA-1. + (merge 30310f3cc4 ss/t3200-test-zero-oid later to maint). + * Other code cleanup, docfix, build fix, etc. (merge d79fff4a11 jk/remote-tracking-ref-leakfix later to maint). (merge 7a747f972d dd/t5403-modernise later to maint). @@ -362,3 +383,5 @@ Fixes since v2.53 (merge 63c00a677b ss/t9123-setup-inside-test-expect-success later to maint). (merge beca0ca4be os/doc-git-custom-commands later to maint). (merge 4c223571be ty/patch-ids-document-lazy-eval later to maint). + (merge 476365ac85 jc/doc-wholesale-replace-before-next later to maint). + (merge 35f220b639 ss/submodule--helper-use-xmalloc later to maint).