trailer: append trailers in-process and drop the fork to `interpret-trailers`

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. It also
centralizes logic to prepare for follow-up rebase --trailer patch.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/commit.c b/builtin/commit.c
index 0243f17d53c97c62432ce77249966d4ef4ed7100..67070d6a546c4e01a023f86f18f2488ebc8e2a75 100644 (file)
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1719,7 +1719,7 @@ int cmd_commit(int argc,
                OPT_STRING(0, "fixup", &fixup_message, N_("[(amend|reword):]commit"), N_("use autosquash formatted message to fixup or amend/reword specified commit")),
                OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
                OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
-               OPT_PASSTHRU_ARGV(0, "trailer", &trailer_args, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG),
+               OPT_CALLBACK_F(0, "trailer", &trailer_args, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG, parse_opt_strvec),
                OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
                OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
                OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index bce2e791d60149454da1a0d54079b4b02a3e79d5..268a43372bd444e7003e7e096af663d8bdce6e89 100644 (file)
--- 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) {
@@ -142,21 +110,15 @@ static void interpret_trailers(const struct process_trailer_options *opts,
 {
        struct strbuf sb = STRBUF_INIT;
        struct strbuf out = STRBUF_INIT;
-       FILE *outfile = stdout;
-
-       trailer_config_init();
 
        read_input_file(&sb, file);
 
-       if (opts->in_place)
-               outfile = create_in_place_tempfile(file);
-
        process_trailers(opts, new_trailer_head, &sb, &out);
 
-       fwrite(out.buf, out.len, 1, outfile);
        if (opts->in_place)
-               if (rename_tempfile(&trailers_tempfile, file))
-                       die_errno(_("could not rename temporary file to %s"), file);
+               write_file_buf(file, out.buf, out.len);
+       else
+               strbuf_write(&out, stdout);
 
        strbuf_release(&sb);
        strbuf_release(&out);
@@ -203,6 +165,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/builtin/tag.c b/builtin/tag.c
index f0665af3acdf6caf056769900e20c17d4672d31c..65c4a0b36bb08e79cb4a167a284d2a64f13cf276 100644 (file)
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -499,8 +499,7 @@ int cmd_tag(int argc,
                OPT_CALLBACK_F('m', "message", &msg, N_("message"),
                               N_("tag message"), PARSE_OPT_NONEG, parse_msg_arg),
                OPT_FILENAME('F', "file", &msgfile, N_("read message from file")),
-               OPT_PASSTHRU_ARGV(0, "trailer", &trailer_args, N_("trailer"),
-                                 N_("add custom trailer(s)"), PARSE_OPT_NONEG),
+               OPT_CALLBACK_F(0, "trailer", &trailer_args, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG, parse_opt_strvec),
                OPT_BOOL('e', "edit", &edit_flag, N_("force edit of tag message")),
                OPT_BOOL('s', "sign", &opt.sign, N_("annotated and GPG-signed tag")),
                OPT_CLEANUP(&cleanup_arg),

diff --git a/trailer.c b/trailer.c
index b735ec8a5314cd8854a149da538bfd44d29685b2..f5838f5699e3b05614126e5c4e1ef6e551cbb7ba 100644 (file)
--- a/trailer.c
+++ b/trailer.c
@@ -9,6 +9,8 @@
 #include "commit.h"
 #include "trailer.h"
 #include "list.h"
+#include "wrapper.h"
+
 /*
  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
  */
@@ -1224,18 +1226,66 @@ 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 *text = trailer_args->v[i];
+               struct new_trailer_item *item;
+
+               if (!*text)
+                       continue;
+               item = xcalloc(1, sizeof(*item));
+               INIT_LIST_HEAD(&item->list);
+               item->text = text;
+               list_add_tail(&item->list, &new_trailer_head);
+       }
+
+       process_trailers(&opts, &new_trailer_head, buf, &out);
+
+       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 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)) {
+               strbuf_release(&buf);
+               return error("failed to append trailers");
+       }
+
+       if (write_file_buf_gently(path, buf.buf, buf.len)) {
+               strbuf_release(&buf);
+               return -1;
+       }
+
+       strbuf_release(&buf);
+       return 0;
+ }
+
 void process_trailers(const struct process_trailer_options *opts,
                      struct list_head *new_trailer_head,
                      struct strbuf *sb, struct strbuf *out)

diff --git a/trailer.h b/trailer.h
index 44d406b763d34f3a3ad3d24ad9ae5d931a262d42..daea46ca5d358cff93c61ab35f05bb6475666e82 100644 (file)
--- a/trailer.h
+++ b/trailer.h
@@ -196,9 +196,8 @@ 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);
 

diff --git a/wrapper.c b/wrapper.c
index 3d507d420452030e77fee9e181762513e5aa9fe8..1f12dbb2fa911b1330e024c4bb1f6d62646cbc0b 100644 (file)
--- a/wrapper.c
+++ b/wrapper.c
@@ -688,6 +688,22 @@ void write_file_buf(const char *path, const char *buf, size_t len)
                die_errno(_("could not close '%s'"), path);
 }
 
+int write_file_buf_gently(const char *path, const char *buf, size_t len)
+{
+       int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0666);
+
+       if (fd < 0)
+               return error_errno(_("could not open '%s'"), path);
+       if (write_in_full(fd, buf, len) < 0) {
+               int ret = error_errno(_("could not write to '%s'"), path);
+               close(fd);
+               return ret;
+       }
+       if (close(fd))
+               return error_errno(_("could not close '%s'"), path);
+       return 0;
+}
+
 void write_file(const char *path, const char *fmt, ...)
 {
        va_list params;

diff --git a/wrapper.h b/wrapper.h
index 44a8597ac314267d80b5cd45b11d2684bbde0cf9..e5f867b2000cb51e8104272ac4c0074799c4436f 100644 (file)
--- a/wrapper.h
+++ b/wrapper.h
@@ -56,6 +56,12 @@ static inline ssize_t write_str_in_full(int fd, const char *str)
  */
 void write_file_buf(const char *path, const char *buf, size_t len);
 
+/**
+ * Like write_file_buf(), but report errors instead of exiting. Returns 0 on
+ * success or a negative value on error after emitting a message.
+ */
+int write_file_buf_gently(const char *path, const char *buf, size_t len);
+
 /**
  * Like write_file_buf(), but format the contents into a buffer first.
  * Additionally, write_file() will append a newline if one is not already