[PATCH v7 3/5] trailer: append trailers without fork/exec

[Li Chen <me@linux.beauty>]

Introduce amend_strbuf_with_trailers() to apply trailer additions to a
message buffer via process_trailers(), avoiding the need to run git
interpret-trailers as a child process.

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.

Keep existing callers unchanged by continuing to accept argv-style
--trailer=<trailer> entries and stripping the prefix before feeding the
in-process implementation.

Signed-off-by: Li Chen <me@linux.beauty>
---
v7:
Drop wrapper.c/h and validate trailer args via validate_trailer_args().
Rewrite the target file via tempfile+rename to preserve --in-place semantics.

 builtin/interpret-trailers.c |   4 +-
 trailer.c                    | 162 +++++++++++++++++++++++++++++++++--
 trailer.h                    |  27 +++++-
 3 files changed, 179 insertions(+), 14 deletions(-)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 1354109e0f..d4aff68746 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -144,8 +144,6 @@ 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)
@@ -203,6 +201,8 @@ int cmd_interpret_trailers(int argc,
 			git_interpret_trailers_usage,
 			options);
 
+	trailer_config_init();
+
 	if (argc) {
 		int i;
 		for (i = 0; i < argc; i++)
diff --git a/trailer.c b/trailer.c
index 0c9200506d..8e87d185d9 100644
--- a/trailer.c
+++ b/trailer.c
@@ -7,8 +7,11 @@
 #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 <chriscool@tuxfamily.org>
  */
@@ -772,6 +775,30 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,
 	free(cl_separators);
 }
 
+void validate_trailer_args(const struct strvec *cli_args)
+{
+	char *cl_separators;
+
+	trailer_config_init();
+
+	cl_separators = xstrfmt("=%s", separators);
+
+	for (size_t i = 0; i < cli_args->nr; i++) {
+		const char *txt = cli_args->v[i];
+		ssize_t separator_pos;
+
+		if (!*txt)
+			die(_("empty --trailer argument"));
+
+		separator_pos = find_separator(txt, cl_separators);
+		if (separator_pos == 0)
+			die(_("invalid trailer '%s': missing key before separator"),
+			    txt);
+	}
+
+	free(cl_separators);
+}
+
 static const char *next_line(const char *str)
 {
 	const char *nl = strchrnul(str, '\n');
@@ -1224,16 +1251,133 @@ 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 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,
+				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;
+
+	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"));
+		item = xcalloc(1, sizeof(*item));
+		item->text = text;
+		list_add_tail(&item->list, &new_trailer_head);
+	}
+
+	trailer_config_init();
+	process_trailers(&opts, &new_trailer_head, buf, &out);
+
+	strbuf_swap(buf, &out);
+	strbuf_release(&out);
+
+	new_trailer_items_clear(&new_trailer_head);
+}
+
+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);
+	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 error_errno(_("could not write to temporary file"));
+	}
+
+	if (rename_tempfile(&tempfile, path))
+		return error_errno(_("could not rename temporary file to %s"), path);
+
+	return 0;
+}
+
+int amend_file_with_trailers(const char *path,
+			     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 strbuf buf = STRBUF_INIT;
+	struct strvec stripped_trailer_args = STRVEC_INIT;
+	int ret = 0;
+	size_t i;
+
+	if (!trailer_args)
+		BUG("amend_file_with_trailers called with NULL trailer_args");
+	if (!trailer_args->nr)
+		return 0;
+
+	for (i = 0; i < trailer_args->nr; i++) {
+		const char *txt = trailer_args->v[i];
+
+		/*
+		 * Historically amend_file_with_trailers() passed its arguments
+		 * to "git interpret-trailers", which expected argv entries in
+		 * "--trailer=<trailer>" form. Continue to accept those for
+		 * existing callers, but pass only the value portion to the
+		 * in-process implementation.
+		 */
+		skip_prefix(txt, "--trailer=", &txt);
+		if (!*txt)
+			die(_("empty --trailer argument"));
+		strvec_push(&stripped_trailer_args, txt);
+	}
+
+	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);
+
+	strvec_clear(&stripped_trailer_args);
+	strbuf_release(&buf);
+	return ret;
 }
 
 void process_trailers(const struct process_trailer_options *opts,
diff --git a/trailer.h b/trailer.h
index 531fa1a13f..d05dab050b 100644
--- a/trailer.h
+++ b/trailer.h
@@ -68,6 +68,8 @@ 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);
+
 void process_trailers_lists(struct list_head *head,
 			    struct list_head *arg_head);
 
@@ -196,12 +198,31 @@ 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.
+ * Append trailers specified in trailer_args to buf in-place.
+ *
+ * Each element of trailer_args should be in the same format as the value
+ * accepted by --trailer=<trailer> (i.e., without the --trailer= prefix).
+ */
+void amend_strbuf_with_trailers(struct strbuf *buf,
+				const struct strvec *trailer_args);
+
+/*
+ * Augment a file by appending trailers specified in trailer_args.
+ *
+ * Each element of trailer_args should be an argv-style --trailer=<trailer>
+ * option (i.e., including the --trailer= prefix).
+ *
+ * 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);
 
+/*
+ * 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);
-- 
2.52.0