public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
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


  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