From: Li Chen <me@linux.beauty>
To: "phillipwood" <phillip.wood@dunelm.org.uk>,
"git" <git@vger.kernel.org>, "Junio C Hamano" <gitster@pobox.com>
Subject: [PATCH v2 1/2] trailer: append trailers in-process and drop the fork to `interpret-trailers`
Date: Tue, 10 Jun 2025 20:34:58 +0800 [thread overview]
Message-ID: <20250610123459.278582-2-me@linux.beauty> (raw)
In-Reply-To: <20250610123459.278582-1-me@linux.beauty>
From: Li Chen <chenl311@chinatelecom.cn>
The built-in commands that support `--trailer` (tag, commit --amend,
rebase --trailer, etc.) have always appended trailers by spawning an
external `git interpret-trailers --in-place` process. That extra fork
is negligible for a single commit, but it scales poorly when hundreds or
thousands of commits are rewritten (e.g. an interactive rebase or a
history-wide `filter-repo`). This patch moves the heavy lifting fully
in-process:
* `amend_strbuf_with_trailers()`
– parses the existing message,
– merges trailers from the command line and config,
– formats the final trailer block, and
– rewrites the supplied `struct strbuf`
without ever leaving the current process.
* `amend_file_with_trailers()` becomes a thin wrapper that reads a file
into a `strbuf`, calls the new helper, and writes the result back.
* `builtin/rebase.c` now calls `amend_file_with_trailers()` instead of
executing `interpret-trailers`.
Edge-cases that used to be handled implicitly by the external helper are
now replicated:
* `trailer.ifexists=replace` and other per-key policies are honoured by
running the existing `parse_trailers_from_command_line_args()` logic
on the raw CLI list, so the same *replace / add / ignore* semantics
are preserved.
* A message that contains an RFC-2822 style `---` separator but no
prior trailer block gets its trailers appended **after** the
separator, matching the external command’s output.
* When `git commit --verbose` leaves a diff after the message (comment
lines beginning with `#`), trailers are inserted before that comment
section so they survive the subsequent strip-space cleanup.
* Ordering remains identical to the old path: CLI trailers first,
followed by trailers introduced via `trailer.<key>.*` config.
Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
trailer.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++++----
trailer.h | 17 +++++++
2 files changed, 155 insertions(+), 9 deletions(-)
diff --git a/trailer.c b/trailer.c
index 310cf582dc..70b04736a8 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1224,14 +1224,143 @@ void trailer_iterator_release(struct trailer_iterator *iter)
strbuf_release(&iter->key);
}
-int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)
+static size_t first_comment_pos(const struct strbuf *buf)
{
- 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);
+ const char *p = buf->buf;
+ const char *end = buf->buf + buf->len;
+
+ while (p < end) {
+ const char *line = p;
+ const char *nl = memchr(p, '\n', end - p);
+ size_t len = nl ? (size_t)(nl - p) : (size_t)(end - p);
+
+ /* skip leading whitespace */
+ size_t i = 0;
+ while (i < len && isspace((unsigned char)line[i]))
+ i++;
+
+ if (i < len && line[i] == '#')
+ return (size_t)(line - buf->buf); /* comment starts here */
+
+ if (!nl) /* last line without newline */
+ break;
+ p = nl + 1;
+ }
+ return buf->len; /* no comment line found */
+}
+
+int amend_strbuf_with_trailers(struct strbuf *buf,
+ const struct strvec *trailer_args)
+{
+ struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
+ struct strbuf trailers_sb = STRBUF_INIT;
+ struct strbuf out = STRBUF_INIT;
+ size_t i;
+ /* 1. parse message ------------------------------------------------- */
+ LIST_HEAD(orig_head); /* existing trailers, if any */
+ LIST_HEAD(cfg_head); /* config trailers */
+ LIST_HEAD(cli_raw); /* new_trailer_item from CLI */
+ LIST_HEAD(cli_head); /* arg_item after helper */
+ LIST_HEAD(all_new); /* merged list */
+ struct trailer_block *blk = parse_trailers(&opts, buf->buf, &orig_head);
+ bool had_trailer_before = !list_empty(&orig_head);
+
+ /* 2. CLI trailers -------------------------------------------------- */
+ if (trailer_args && trailer_args->nr) {
+ for (i = 0; i < trailer_args->nr; i++) {
+ const char *arg = trailer_args->v[i];
+ const char *text;
+ struct new_trailer_item *ni;
+
+ if (!skip_prefix(arg, "--trailer=", &text))
+ text = arg;
+
+ if (!*text)
+ continue;
+
+ ni = xcalloc(1, sizeof(*ni));
+ INIT_LIST_HEAD(&ni->list);
+ ni->text = (char *)text;
+ list_add_tail(&ni->list, &cli_raw);
+ }
+ parse_trailers_from_command_line_args(&cli_head, &cli_raw);
+
+ while (!list_empty(&cli_raw)) {
+ struct new_trailer_item *ni =
+ list_first_entry(&cli_raw, struct new_trailer_item, list);
+ list_del(&ni->list);
+ free(ni);
+ }
+ }
+
+ /* 3. config trailers ---------------------------------------------- */
+ parse_trailers_from_config(&cfg_head);
+
+ /* 4. merge lists --------------------------------------------------- */
+ list_splice(&cli_head, &all_new);
+ list_splice(&cfg_head, &all_new);
+
+ /* 5. apply --------------------------------------------------------- */
+ process_trailers_lists(&orig_head, &all_new);
+
+ /* 6. format updated trailer block --------------------------------- */
+ format_trailers(&opts, &orig_head, &trailers_sb);
+
+ /* 7. decide insertion point --------------------------------------- */
+ if (had_trailer_before) {
+ /* insert at existing trailer block */
+ strbuf_add(&out, buf->buf, trailer_block_start(blk));
+ if (!blank_line_before_trailer_block(blk))
+ strbuf_addch(&out, '\n');
+ strbuf_addbuf(&out, &trailers_sb);
+ strbuf_add(&out,
+ buf->buf + trailer_block_end(blk),
+ buf->len - trailer_block_end(blk));
+ } else {
+ /* insert before first comment (git --verbose) if any */
+ size_t cpos = first_comment_pos(buf);
+
+ /* copy body up to comment (or whole buf if none) */
+ strbuf_add(&out, buf->buf, cpos);
+
+ /* ensure single blank line separating body & trailers */
+ if (!out.len || out.buf[out.len - 1] != '\n')
+ strbuf_addch(&out, '\n');
+ if (out.len >= 2 && out.buf[out.len - 2] != '\n')
+ strbuf_addch(&out, '\n');
+
+ strbuf_addbuf(&out, &trailers_sb);
+
+ /* copy remaining comment lines (if any) */
+ strbuf_add(&out, buf->buf + cpos, buf->len - cpos);
+ }
+
+ strbuf_swap(buf, &out);
+
+ /* 8. cleanup ------------------------------------------------------- */
+ strbuf_release(&out);
+ strbuf_release(&trailers_sb);
+ free_trailers(&orig_head);
+ trailer_block_release(blk);
+ return 0;
+}
+
+int amend_file_with_trailers(const char *path,
+ const struct strvec *trailer_args)
+{
+ struct strbuf buf = STRBUF_INIT;
+
+ if (!trailer_args || !trailer_args->nr)
+ return 0;
+
+ if (strbuf_read_file(&buf, path, 0) < 0)
+ return error_errno("could not read '%s'", path);
+
+ if (amend_strbuf_with_trailers(&buf, trailer_args))
+ die("failed to append trailers");
+
+ /* `write_file_buf()` aborts on error internally */
+ write_file_buf(path, buf.buf, buf.len);
+ strbuf_release(&buf);
+ return 0;
}
diff --git a/trailer.h b/trailer.h
index 4740549586..17986d5dd0 100644
--- a/trailer.h
+++ b/trailer.h
@@ -202,4 +202,21 @@ void trailer_iterator_release(struct trailer_iterator *iter);
*/
int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
+/*
+ * In‑memory variant of amend_file_with_trailers(): instead of rewriting a file
+ * on disk (and therefore spawning a separate `git interpret‑trailers` process)
+ * we operate directly on a struct strbuf that already contains the commit
+ * message. The rules for positioning, deduplication, and formatting are the
+ * same as those implemented by the builtin `interpret‑trailers` command.
+ *
+ * - @buf : the message to be amended. On success, its contents are
+ * replaced with the new message that has the trailers
+ * inserted.
+ * - @trailer_args : the list of trailer strings exactly as would be passed on
+ * the command‑line via repeated `--trailer` options.
+ *
+ * Returns 0 on success, <0 on error.
+ */
+int amend_strbuf_with_trailers(struct strbuf *buf,
+ const struct strvec *trailer_args);
#endif /* TRAILER_H */
--
2.49.0
next prev parent reply other threads:[~2025-06-10 12:35 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-10 12:34 [PATCH v2 0/2] rebase: support --trailer Li Chen
2025-06-10 12:34 ` Li Chen [this message]
2025-06-11 0:09 ` [PATCH v2 1/2] trailer: append trailers in-process and drop the fork to `interpret-trailers` Junio C Hamano
2025-06-11 9:15 ` phillip.wood123
2025-06-11 15:34 ` Junio C Hamano
2025-06-12 5:40 ` Li Chen
2025-06-10 12:34 ` [PATCH v2 2/2] rebase: support --trailer Li Chen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250610123459.278582-2-me@linux.beauty \
--to=me@linux.beauty \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=phillip.wood@dunelm.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox