From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com [209.85.128.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6A68F39448D for ; Wed, 4 Mar 2026 14:29:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.51 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772634592; cv=none; b=n174FncRM3Sge2DfuRhIAfW5YlgKaRW5S7/vDDopCm6whBSmQ9lrg4FZvox68RshuupBXAFxCNht1vbPsYLuEtID89hChz0XvBZq/BbURnE3FovgwhBeZyf7K15GXB1AQbr1c4gngPQAqx2lwAzYv5DbqABjZxcax3j8XOo/4tE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772634592; c=relaxed/simple; bh=prJXhenyf+Cj5BzhEXMb1wh+owQTtadWKI2GM44wQbI=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=pxKq2/NKMVDNkHisYoMmyksoXdFKcmABgugefXRAFZfbClnuYWG80lJnkMpqYYlJKMHGNosXOF/HSl3NvY8NmdaP8yfzxM5BBJjOoISakffIROmCWgtaQ9V2FSItaOvaslqqMCAN2pCftJZVXE9SqGt3dxQSpJz+hS1Q5PkzQo8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=AQSt2cX7; arc=none smtp.client-ip=209.85.128.51 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="AQSt2cX7" Received: by mail-wm1-f51.google.com with SMTP id 5b1f17b1804b1-4837584120eso51665095e9.1 for ; Wed, 04 Mar 2026 06:29:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1772634589; x=1773239389; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:reply-to:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=kjaVuJiJCGlrVWJgQ06B/vEFar/2fj+cHSymtWH0ziE=; b=AQSt2cX777H9fnfkL0v8BeEUgBFtIywgpUYfhtE8Mkcd9aPWD61G4wPOyOOEQpNWRO DScwmQEOidfwumsqbvQnWxOBVbzK3YBKkVRlhL0lk7wnyPuZh+BlbmkUXn4Cc+peNdmx rqJr2O4BC70fOAaVLZd1IBiXwYlp8F+/eGX79cpDKCXHhxIr0NZsoldVr3DkwMngdyW1 nEBTfx6aB94Ln4qhgq3eYPmLPoRNXsbWvc4weoC8WAEzQx1K8GO2kTQEZ9cJHkxZnMp3 7HDSZ8bV5Ai60mkjQFTKl13dawdTaiTznHJVkO9Xwk6zIWWhkOKbOZSpKTvud5mNBj7z mkVA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772634589; x=1773239389; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:reply-to:user-agent:mime-version:date :message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=kjaVuJiJCGlrVWJgQ06B/vEFar/2fj+cHSymtWH0ziE=; b=srOqvkEZXASRU3KG32svhkycrMomWx1VSWAHIhxCvWI9T4aytyv5HKIWQVeQU9Vknc QR63OcKiuJGD+RI/9wRJhKDMr2yXRIUrAKA/Q7buICujZog7P7A3KV+lu4QUOTjyGuE2 VXL7ctuPwkPw7f2lIhVNWCN2KRLcm51h40XtISbuGGCnhjiSR+is++bxszyOUP27nflr O1j5g9OryAXG6f4E3QEvtHVugM28JnqpwoFySmI4w3IFaKXicPNZwyZig2SJLdOwNKt8 1yBI9RiqdEwbfTWOfIIkHFmypVSTC5Y7RYwKdKQpdyIk8mNjP74davW39M5zyu33MOvU feVw== X-Forwarded-Encrypted: i=1; AJvYcCV3/PxSQAZprfZ5FTOVBlVYQg6J3LVplMPCgA5B8oicjL1ckoRag3IxoRxXCUYN5xwwJ5g=@vger.kernel.org X-Gm-Message-State: AOJu0Yxy9umK/T0mkdx2ErucksQcQcjg8+o2BNA/utc3aGXJN57/08NJ 0dHmMwhX+zutVc4nOzLrjHJoJN6g9QKU6JpoifE3xgAFqW63/sgS023z X-Gm-Gg: ATEYQzwZNaba/PUpjREcsAsuf4XfTkod0pDKuRyUuHiNSD6pJuByF3tgq66cyqhZys/ 8SfE+NnMHMD0iKNBYLDyAv2wqkMKF/mpoHxD3tryYG+Cw81zc7D+Vztzfz4z4wfEPoAx0v9aZaA c5scycKtO3NJfjOrYRVTz72ZILuulHbJTOkJlt9GQFz5NxG+AdBsy7GzavGP/e5mM/IH0r7XyIZ tNp+I8rdcMyhQAropbnCXxaymXtJzgjdxMCrLaR3ZAQ/MM9BEa/GoAZoc3sanIPBZpUXN7/vWkT MsoARFy/CzrxJLR0rqcLLXDhkDS78uuYvjAEenHU6DcOCzal7227Ys6jgDqND81A9IxoFG6mC84 bNpiEM7WH8MVEqaFyXqVCg66pRR9CAbEV24HVQoxcZJ4R7j14J+oF7+SkYFtXQjqbotuLlkiFmG MYQDkoBeJFqCTVD00pvq4cWI7CxR5piW5v9eOx3y4lZXPhkXXO1cM+yyAkcvN1aD3seZImtyfla BJmR7XpXUH/X6VP X-Received: by 2002:a05:600c:1c18:b0:480:69ae:f0e9 with SMTP id 5b1f17b1804b1-48519871aa4mr43676285e9.16.1772634588369; Wed, 04 Mar 2026 06:29:48 -0800 (PST) Received: from ?IPV6:2a0a:ef40:1785:c801:9102:504:16e7:c44e? ([2a0a:ef40:1785:c801:9102:504:16e7:c44e]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4851a8ae17csm12157605e9.6.2026.03.04.06.29.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 04 Mar 2026 06:29:47 -0800 (PST) Message-ID: Date: Wed, 4 Mar 2026 14:29:42 +0000 Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Reply-To: phillip.wood@dunelm.org.uk Subject: Re: [PATCH v7 0/5] rebase: support --trailer To: Li Chen , git@vger.kernel.org Cc: Junio C Hamano , Phillip Wood , Kristoffer Haugsbakk References: <20260224070552.148591-1-me@linux.beauty> From: Phillip Wood Content-Language: en-US In-Reply-To: <20260224070552.148591-1-me@linux.beauty> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi Li On 24/02/2026 07:05, Li Chen wrote: > Apologies for the long delay in sending v7. > > v7 is based on origin/master at v2.53.0-154-g7c02d39fc2. > > This series routes trailer insertion through an in-process path, removing the > fork/exec to builtin/interpret-trailers. > > The first four commits refactor trailer rewriting in builtin/interpret-trailers > and trailer.c so callers can reuse a single in-process helper (used by git > interpret-trailers, git commit and git tag). The final commit adds git rebase > --trailer, currently supported with the merge backend only (rejecting apply-only > scenarios and validating input early). This all looks pretty good. The main thing I think we want to change is sharing the code that creates the temporary file in patch 3. I've push a version that does that and fixes my other small comments to https://github.com/phillipwood/git/commits/refs/heads/rebase-trailers-v8 The range diff is below. If you're happy I can post that as a hopefully final v8. Of course if you want to work on it yourself you're very welcome to do that. Thanks Phillip 1: b2685e34c22 ! 1: 0d08b361995 interpret-trailers: factor trailer rewriting @@ Commit message 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 ## builtin/interpret-trailers.c ## @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file) -: ----------- > 2: 5a4d03ab375 interpret-trailers: refactor create_in_place_tempfile() 2: 1bac3025045 ! 3: ab7e232a95d trailer: move process_trailers to trailer.h @@ Metadata Author: Li Chen ## Commit message ## - trailer: move process_trailers to trailer.h - - Move process_trailers() from builtin/interpret-trailers.c into trailer.c - and expose it via trailer.h. - - This lets other call sites reuse the same trailer rewriting logic. + 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 ## builtin/interpret-trailers.c ## +@@ builtin/interpret-trailers.c: 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) { @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file) strbuf_complete_line(sb); } @@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, con static void interpret_trailers(const struct process_trailer_options *opts, struct list_head *new_trailer_head, const char *file) +@@ builtin/interpret-trailers.c: 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; ## trailer.c ## +@@ + #include "commit.h" + #include "trailer.h" + #include "list.h" ++#include "tempfile.h" ++ + /* + * Copyright (c) 2013, 2014 Christian Couder + */ +@@ trailer.c: 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; @@ trailer.c: 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); @@ trailer.h: 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); 3: 3114f0dbb57 ! 4: 1f24917eb64 trailer: append trailers without fork/exec @@ Commit message 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. + 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 - - ## builtin/interpret-trailers.c ## -@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts, - struct strbuf out = STRBUF_INIT; - FILE *outfile = stdout; - -- trailer_config_init(); -- - read_input_file(&input, file); - - if (opts->in_place) -@@ builtin/interpret-trailers.c: int cmd_interpret_trailers(int argc, - git_interpret_trailers_usage, - options); - -+ trailer_config_init(); -+ - if (argc) { - int i; - for (i = 0; i < argc; i++) + Signed-off-by: Phillip Wood + + ## builtin/commit.c ## +@@ builtin/commit.c: 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; + + + ## builtin/tag.c ## +@@ builtin/tag.c: 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; + ## trailer.c ## @@ #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 - */ + #include "tempfile.h" @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head, free(cl_separators); } -+void validate_trailer_args(const struct strvec *cli_args) ++int validate_trailer_args(const struct strvec *cli_args) +{ + char *cl_separators; ++ int ret = 0; + + trailer_config_init(); + @@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head + const char *txt = cli_args->v[i]; + ssize_t separator_pos; + -+ if (!*txt) -+ die(_("empty --trailer argument")); -+ ++ if (!*txt) { ++ ret = error(_("empty --trailer argument")); ++ goto out; ++ } + separator_pos = find_separator(txt, cl_separators); -+ if (separator_pos == 0) -+ die(_("invalid trailer '%s': missing key before separator"), -+ txt); ++ 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'); -@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter) - strbuf_release(&iter->key); +@@ trailer.c: 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) @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter) - path, NULL); - strvec_pushv(&run_trailer.args, trailer_args->v); - return run_command(&run_trailer); -+static void new_trailer_items_clear(struct list_head *items) -+{ -+ 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, ++int amend_strbuf_with_trailers(struct strbuf *buf, + const struct strvec *trailer_args) +{ + struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT; + LIST_HEAD(new_trailer_head); + 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) -+ die(_("empty --trailer argument")); ++ if (!*text) { ++ ret = error(_("empty --trailer argument")); ++ goto out; ++ } + item = xcalloc(1, sizeof(*item)); -+ item->text = text; ++ item->text = xstrdup(text); + list_add_tail(&item->list, &new_trailer_head); + } + -+ trailer_config_init(); + process_trailers(&opts, &new_trailer_head, buf, &out); + + strbuf_swap(buf, &out); ++out: + strbuf_release(&out); ++ free_trailers(&new_trailer_head); + -+ new_trailer_items_clear(&new_trailer_head); ++ return ret; +} + +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); ++ struct tempfile *tempfile = trailer_create_in_place_tempfile(path); + 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 -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); @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter) + * in-process implementation. + */ + skip_prefix(txt, "--trailer=", &txt); -+ if (!*txt) -+ die(_("empty --trailer argument")); ++ 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; @@ trailer.h: 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); ++int validate_trailer_args(const struct strvec *cli_args); + void process_trailers_lists(struct list_head *head, struct list_head *arg_head); @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter); + * 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, ++int amend_strbuf_with_trailers(struct strbuf *buf, + const struct strvec *trailer_args); + +/* @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter); */ 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); 4: 147595a9317 ! 5: 3c1fa9e8579 commit, tag: parse --trailer with OPT_STRVEC @@ Commit message amend_file_with_trailers(). Signed-off-by: Li Chen ## builtin/commit.c ## @@ builtin/commit.c: int cmd_commit(int argc, @@ trailer.c: int amend_file_with_trailers(const char *path, - * in-process implementation. - */ - skip_prefix(txt, "--trailer=", &txt); -- if (!*txt) -- die(_("empty --trailer argument")); +- 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 @@ trailer.c: int amend_file_with_trailers(const char *path, if (!ret) ret = write_file_in_place(path, &buf); - + out: - strvec_clear(&stripped_trailer_args); strbuf_release(&buf); return ret; } ## trailer.h ## -@@ trailer.h: void amend_strbuf_with_trailers(struct strbuf *buf, +@@ trailer.h: int amend_strbuf_with_trailers(struct strbuf *buf, /* * Augment a file by appending trailers specified in trailer_args. * 5: 864cf5f8eb6 ! 6: 99654d80547 rebase: support --trailer @@ Commit message non-interactive and interactive rebases. Signed-off-by: Li Chen + Signed-off-by: Phillip Wood ## Documentation/git-rebase.adoc ## @@ Documentation/git-rebase.adoc: See also INCOMPATIBLE OPTIONS below. @@ Documentation/git-rebase.adoc: 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. ++ `--force-rebase`. ++ +See also INCOMPATIBLE OPTIONS below. + -i:: --interactive:: Make a list of the commits which are about to be rebased. Let the +@@ Documentation/git-rebase.adoc: 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: + ## builtin/rebase.c ## @@ @@ builtin/rebase.c: int cmd_rebase(int argc, builtin_rebase_usage, 0); + if (options.trailer_args.nr) { -+ validate_trailer_args(&options.trailer_args); ++ if (validate_trailer_args(&options.trailer_args)) ++ die(NULL); + options.flags |= REBASE_FORCE; + } + @@ sequencer.c: void replay_opts_release(struct replay_opts *opts) free(opts->ctx); } @@ sequencer.c: 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); @@ sequencer.c: static int do_pick_commit(struct repository *r, if (is_rebase_i(opts) && write_author_script(msg.message) < 0) res = -1; else if (!opts->strategy || +@@ sequencer.c: 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; @@ sequencer.c: 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 (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(), @@ sequencer.c: int write_basic_state(struct replay_opts *opts, const char *head_name, write_file(rebase_path_reschedule_failed_exec(), "%s", ""); else