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 v4 01/29] trailer: append trailers in-process and drop the fork to `interpret-trailers`
Date: Tue, 14 Oct 2025 20:24:14 +0800	[thread overview]
Message-ID: <20251014122452.1851103-2-me@linux.beauty> (raw)
In-Reply-To: <20251014122452.1851103-1-me@linux.beauty>

From: Li Chen <chenl311@chinatelecom.cn>

Route all trailer insertion through trailer_process() and make
builtin/interpret-trailers just do file I/O before calling into it.
amend_file_with_trailers() now shares the same code path.

This removes the fork/exec and tempfile juggling, cutting overhead and
simplifying error handling. No functional change is intended. It also
centralizes logic to prepare for follow-up rebase --trailer patch.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 builtin/interpret-trailers.c | 116 ++++++++------------------------
 trailer.c                    | 125 ++++++++++++++++++++++++++++++++---
 trailer.h                    |  18 ++++-
 3 files changed, 157 insertions(+), 102 deletions(-)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 41b0750e5a..be0fa83f79 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -10,7 +10,6 @@
 #include "gettext.h"
 #include "parse-options.h"
 #include "string-list.h"
-#include "tempfile.h"
 #include "trailer.h"
 #include "config.h"
 
@@ -93,37 +92,6 @@ 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)
-{
-	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);
-
-	/* Create temporary file in the same directory as the original */
-	tail = strrchr(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);
-	strbuf_release(&filename_template);
-	outfile = fdopen_tempfile(trailers_tempfile, "w");
-	if (!outfile)
-		die_errno(_("could not open temporary file"));
-
-	return outfile;
-}
-
 static void read_input_file(struct strbuf *sb, const char *file)
 {
 	if (file) {
@@ -136,61 +104,6 @@ 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)
-{
-	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);
-
-	/* Print the lines before the trailer block */
-	if (!opts->only_trailers)
-		fwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);
-
-	if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
-		fprintf(outfile, "\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, &trailer_block_sb);
-	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);
-	trailer_block_release(trailer_block);
-
-	if (opts->in_place)
-		if (rename_tempfile(&trailers_tempfile, file))
-			die_errno(_("could not rename temporary file to %s"), file);
-
-	strbuf_release(&sb);
-}
-
 int cmd_interpret_trailers(int argc,
 			   const char **argv,
 			   const char *prefix,
@@ -232,14 +145,37 @@ int cmd_interpret_trailers(int argc,
 			git_interpret_trailers_usage,
 			options);
 
+	trailer_config_init();
+
 	if (argc) {
 		int i;
-		for (i = 0; i < argc; i++)
-			interpret_trailers(&opts, &trailers, argv[i]);
+		for (i = 0; i < argc; i++) {
+			struct strbuf in_buf = STRBUF_INIT;
+			struct strbuf out_buf = STRBUF_INIT;
+
+			read_input_file(&in_buf, argv[i]);
+			if (trailer_process(&opts, in_buf.buf, &trailers, &out_buf) < 0)
+				die(_("failed to process trailers for %s"), argv[i]);
+			if (opts.in_place)
+				write_file_buf(argv[i], out_buf.buf, out_buf.len);
+			else
+				fwrite(out_buf.buf, 1, out_buf.len, stdout);
+			strbuf_release(&in_buf);
+			strbuf_release(&out_buf);
+		}
 	} else {
+		struct strbuf in_buf = STRBUF_INIT;
+		struct strbuf out_buf = STRBUF_INIT;
+
 		if (opts.in_place)
 			die(_("no input file given for in-place editing"));
-		interpret_trailers(&opts, &trailers, NULL);
+
+		read_input_file(&in_buf, NULL);
+		if (trailer_process(&opts, in_buf.buf, &trailers, &out_buf) < 0)
+			die(_("failed to process trailers"));
+		fwrite(out_buf.buf, 1, out_buf.len, stdout);
+		strbuf_release(&in_buf);
+		strbuf_release(&out_buf);
 	}
 
 	new_trailers_clear(&trailers);
diff --git a/trailer.c b/trailer.c
index 911a81ed99..8aec466b5f 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1224,14 +1224,121 @@ 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 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;
+
+	opts.no_divider = 1;
+
+	for (i = 0; i < trailer_args->nr; i++) {
+		const char *arg = trailer_args->v[i];
+		const char *text;
+		struct new_trailer_item *item;
+
+		if (!skip_prefix(arg, "--trailer=", &text))
+			text = arg;
+		if (!*text)
+			continue;
+		item = xcalloc(1, sizeof(*item));
+		INIT_LIST_HEAD(&item->list);
+		item->text = text;
+		list_add_tail(&item->list, &new_trailer_head);
+	}
+	if (trailer_process(&opts, buf->buf, &new_trailer_head, &out) < 0)
+		return -1;
+	strbuf_swap(buf, &out);
+	strbuf_release(&out);
+	while (!list_empty(&new_trailer_head)) {
+		struct new_trailer_item *item =
+			list_first_entry(&new_trailer_head, struct new_trailer_item, list);
+		list_del(&item->list);
+		free(item);
+	}
+	return 0;
+}
+
+int trailer_process(const struct process_trailer_options *opts,
+					const char *msg,
+					struct list_head *new_trailer_head,
+					struct strbuf *out)
+{
+	struct trailer_block *blk;
+	LIST_HEAD(orig_head);
+	LIST_HEAD(config_head);
+	LIST_HEAD(arg_head);
+	struct strbuf trailers_sb = STRBUF_INIT;
+	int had_trailer_before;
+
+	blk = parse_trailers(opts, msg, &orig_head);
+	had_trailer_before = !list_empty(&orig_head);
+	if (!opts->only_input) {
+		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(&orig_head, &arg_head);
+	}
+	format_trailers(opts, &orig_head, &trailers_sb);
+	if (!opts->only_trailers && !opts->only_input && !opts->unfold &&
+	    !opts->trim_empty && list_empty(&orig_head) &&
+	    (list_empty(new_trailer_head) || opts->only_input)) {
+		size_t split = trailer_block_start(blk); /* end-of-log-msg */
+		if (!blank_line_before_trailer_block(blk)) {
+			strbuf_add(out, msg, split);
+			strbuf_addch(out, '\n');
+			strbuf_addstr(out, msg + split);
+		} else
+			strbuf_addstr(out, msg);
+
+		strbuf_release(&trailers_sb);
+		trailer_block_release(blk);
+		return 0;
+	}
+	if (opts->only_trailers) {
+		strbuf_addbuf(out, &trailers_sb);
+	} else if (had_trailer_before) {
+		strbuf_add(out, msg, trailer_block_start(blk));
+		if (!blank_line_before_trailer_block(blk))
+			strbuf_addch(out, '\n');
+		strbuf_addbuf(out, &trailers_sb);
+		strbuf_add(out, msg + trailer_block_end(blk),
+			   strlen(msg) - trailer_block_end(blk));
+	} else {
+		size_t cpos = trailer_block_start(blk);
+		strbuf_add(out, msg, cpos);
+		if (cpos == 0) /* empty body → just one \n */
+			strbuf_addch(out, '\n');
+		else if (!blank_line_before_trailer_block(blk))
+			strbuf_addch(out, '\n'); /* body without trailing blank */
+
+		strbuf_addbuf(out, &trailers_sb);
+		strbuf_add(out, msg + cpos, strlen(msg) - cpos);
+	}
+	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..01f711fb13 100644
--- a/trailer.h
+++ b/trailer.h
@@ -196,10 +196,22 @@ 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.
+ * Augment a file to add trailers to it (similar to 'git interpret-trailers').
+ * 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);
 
+/*
+ * Process trailer lines for a commit message in-memory.
+ * @opts: trailer processing options (e.g. from parse-options)
+ * @msg: the input message string
+ * @new_trailer_head: list of new trailers to add (struct new_trailer_item)
+ * @out: strbuf to store the resulting message (must be initialized)
+ *
+ * Returns 0 on success, <0 on error.
+ */
+int trailer_process(const struct process_trailer_options *opts,
+			const char *msg,
+			struct list_head *new_trailer_head,
+			struct strbuf *out);
 #endif /* TRAILER_H */
-- 
2.51.0


  reply	other threads:[~2025-10-14 12:25 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-14 12:24 [PATCH v4 00/29] rebase: support --trailer Li Chen
2025-10-14 12:24 ` Li Chen [this message]
2025-10-14 20:43   ` [PATCH v4 01/29] trailer: append trailers in-process and drop the fork to `interpret-trailers` Kristoffer Haugsbakk
2025-10-21  9:57     ` Li Chen
2025-10-14 12:24 ` [PATCH v4 02/29] trailer: restore interpret_trailers helper Li Chen
2025-10-14 12:24 ` [PATCH v4 03/29] trailer: drop --trailer prefix handling in amend helper Li Chen
2025-10-14 12:24 ` [PATCH v4 04/29] trailer: move config_head and arg_head to if storage Li Chen
2025-10-14 12:24 ` [PATCH v4 05/29] trailer: use bool for had_trailer_before Li Chen
2025-10-14 12:24 ` [PATCH v4 06/29] interpret-trailers: buffer stdout output Li Chen
2025-10-14 12:24 ` [PATCH v4 07/29] trailer: mirror interpret-trailers output flow Li Chen
2025-10-14 12:24 ` [PATCH v4 08/29] trailer: handle trailer append failures gently Li Chen
2025-10-14 12:24 ` [PATCH v4 09/29] rebase: support --trailer Li Chen
2025-10-14 20:43   ` Kristoffer Haugsbakk
2025-10-22  3:55     ` Li Chen
2025-10-14 12:24 ` [PATCH v4 10/29] rebase: inline trailer state paths Li Chen
2025-10-14 12:24 ` [PATCH v4 11/29] rebase: reuse buffer for trailer args Li Chen
2025-10-14 12:24 ` [PATCH v4 12/29] rebase: drop redundant strbuf_release call Li Chen
2025-10-14 12:24 ` [PATCH v4 13/29] rebase: skip stripping of --trailer option prefix Li Chen
2025-10-14 12:24 ` [PATCH v4 14/29] rebase: die on invalid trailer args Li Chen
2025-10-14 12:24 ` [PATCH v4 15/29] rebase: validate trailers with configured separators Li Chen
2025-10-14 12:24 ` [PATCH v4 16/29] sequencer: add trailers to message before writing file Li Chen
2025-10-14 20:43   ` Kristoffer Haugsbakk
2025-10-14 12:24 ` [PATCH v4 17/29] tests: t3440: create expect files at point of use Li Chen
2025-10-14 20:41   ` Kristoffer Haugsbakk
2025-10-15 13:58     ` Li Chen
2025-10-15 14:02       ` Kristoffer Haugsbakk
2025-10-23  9:04       ` Phillip Wood
2025-10-28 10:26         ` Li Chen
2025-11-03 16:20           ` Phillip Wood
2025-10-14 12:24 ` [PATCH v4 18/29] tests: t3440: check apply backend error includes option Li Chen
2025-10-14 12:24 ` [PATCH v4 19/29] tests: t3440: use test_commit_message for trailer checks Li Chen
2025-10-14 12:24 ` [PATCH v4 20/29] tests: t3440: drop redundant resets and pass branch to rebase where needed Li Chen
2025-10-14 12:24 ` [PATCH v4 21/29] tests: t3440: assert trailer on HEAD after conflict rebase Li Chen
2025-10-14 12:24 ` [PATCH v4 22/29] rebase: persist --trailer options across restarts Li Chen
2025-10-14 12:24 ` [PATCH v4 23/29] tests: t3440: remove redundant --keep-empty Li Chen
2025-10-14 12:24 ` [PATCH v4 24/29] tests: t3440: use helper for trailer checks Li Chen
2025-10-14 12:24 ` [PATCH v4 25/29] tests: t3440: test --trailer without values Li Chen
2025-10-14 13:22   ` Kristoffer Haugsbakk
2025-10-14 12:24 ` [PATCH v4 26/29] tests: t3440: convert ex.com to example.com Li Chen
2025-10-14 12:24 ` [PATCH v4 27/29] tests: t3440: ensure trailers persist after rebase continue Li Chen
2025-10-14 12:24 ` [PATCH v4 28/29] tests: t3440: exercise trailer config mapping Li Chen
2025-10-14 12:24 ` [PATCH v4 29/29] sequencer: honor --trailer with fixup -C Li Chen
2025-10-14 12:31 ` [PATCH v4 00/29] 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=20251014122452.1851103-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