trailer: append trailers without fork/exec

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. As the trailers are no longer added in a separate
process and trailer_config_init() die()s on missing config values it
is called early on in cmd_commit() and cmd_tag() so that they die()
early before writing the message file. The trailer arguments are now
also sanity checked.

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>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/commit.c b/builtin/commit.c
index 9e3a09d532bfcebe0c4118355e53b853e8e5b4d1..eb9013995c95d404dccffc8923ff669fec4b3846 100644 (file)
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1820,6 +1820,9 @@ int cmd_commit(int argc,
        argc = parse_and_validate_options(argc, argv, builtin_commit_options,
                                          builtin_commit_usage,
                                          prefix, current_head, &s);
+       if (trailer_args.nr)
+               trailer_config_init();
+
        if (verbose == -1)
                verbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;
 

diff --git a/builtin/tag.c b/builtin/tag.c
index aeb04c487fe95a671932937a83d577b0e76ac4a4..68b581a9c26671ea3c4a5b2d28a4eef3f06a2c6c 100644 (file)
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -568,6 +568,9 @@ int cmd_tag(int argc,
        if (cmdmode == 'l')
                setup_auto_pager("tag", 1);
 
+       if (trailer_args.nr)
+               trailer_config_init();
+
        if (opt.sign == -1)
                opt.sign = cmdmode ? 0 : config_sign_tag > 0;
 

diff --git a/trailer.c b/trailer.c
index 163018483a5fb5de2acd0f3402599522f3e302b9..5eab4fa549d6e7ce411237fe4fc6a5457dd01b23 100644 (file)
--- a/trailer.c
+++ b/trailer.c
@@ -7,6 +7,7 @@
 #include "string-list.h"
 #include "run-command.h"
 #include "commit.h"
+#include "strvec.h"
 #include "trailer.h"
 #include "list.h"
 #include "tempfile.h"
@@ -774,6 +775,35 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,
        free(cl_separators);
 }
 
+int validate_trailer_args(const struct strvec *cli_args)
+{
+       char *cl_separators;
+       int ret = 0;
+
+       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) {
+                       ret = error(_("empty --trailer argument"));
+                       goto out;
+               }
+               separator_pos = find_separator(txt, cl_separators);
+               if (separator_pos == 0) {
+                       ret = error(_("invalid trailer '%s': missing key before separator"),
+                                   txt);
+                       goto out;
+               }
+       }
+out:
+       free(cl_separators);
+       return ret;
+}
+
 static const char *next_line(const char *str)
 {
        const char *nl = strchrnul(str, '\n');
@@ -1258,16 +1288,101 @@ struct tempfile *trailer_create_in_place_tempfile(const char *file)
        return tempfile;
 }
 
-int amend_file_with_trailers(const char *path, const struct strvec *trailer_args)
+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;
+       int ret = 0;
+
+       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) {
+                       ret = error(_("empty --trailer argument"));
+                       goto out;
+               }
+               item = xcalloc(1, sizeof(*item));
+               item->text = xstrdup(text);
+               list_add_tail(&item->list, &new_trailer_head);
+       }
+
+       process_trailers(&opts, &new_trailer_head, buf, &out);
+
+       strbuf_swap(buf, &out);
+out:
+       strbuf_release(&out);
+       free_trailers(&new_trailer_head);
+
+       return ret;
+}
+
+static int write_file_in_place(const char *path, const struct strbuf *buf)
+{
+       struct tempfile *tempfile = trailer_create_in_place_tempfile(path);
+       if (!tempfile)
+               return -1;
+
+       if (write_in_full(tempfile->fd, buf->buf, buf->len) < 0)
+               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 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) {
+                       ret = error(_("empty --trailer argument"));
+                       goto out;
+               }
+               strvec_push(&stripped_trailer_args, txt);
+       }
+
+       if (validate_trailer_args(&stripped_trailer_args)) {
+               ret = -1;
+               goto out;
+       }
+       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);
+out:
+       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 7fd2564e035e8b126740bc52aff8d975b973e09f..3c5d9a6e199248f78e79c6fd1382e5ad3376eab5 100644 (file)
--- 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);
 
+int validate_trailer_args(const struct strvec *cli_args);
+
 void process_trailers_lists(struct list_head *head,
                            struct list_head *arg_head);
 
@@ -196,9 +198,21 @@ 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).
+ */
+int 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);