[PATCH v2 1/2] trailer: append trailers in-process and drop the fork to `interpret-trailers`

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Li Chen <chenl311@xxxxxxxxxxxxxxx>

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@xxxxxxxxxxxxxxx>
---
 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





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux