trailer: move interpret_trailers() to interpret-trailers.c

The interpret-trailers.c builtin is the only place we need to call interpret_trailers(), so move its definition there (together with a few helper functions called only by it) and remove its external declaration from <trailer.h>. Several helper functions that are called by interpret_trailers() remain in trailer.c because other callers in the same file still call them. Declare them in <trailer.h> so that interpret_trailers() (now in builtin/interpret-trailers.c) can continue calling them as a trailer API user. This enriches <trailer.h> with a more granular API, which can then be unit-tested in the future (because interpret_trailers() by itself does too many things to be able to be easily unit-tested). Take this opportunity to demote some file-handling functions out of the trailer API implementation, as these have nothing to do with trailers. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Linus Arver committed Mar 1, 2024 at 00:14 UTC ae0ec2e0e0bb26474f395504c6ed6fef3f59091f
3 files changed +123 -109
builtin/interpret-trailers.c
+93
@@ -9,6 +9,7 @@
9 #include "gettext.h"
10 #include "parse-options.h"
11 #include "string-list.h"
12 +#include "tempfile.h"
13 #include "trailer.h"
14 #include "config.h"
15
@@ -91,6 +92,98 @@ static int parse_opt_parse(const struct option *opt, const char *arg,
92 return 0;
93 }
94
95 +static struct tempfile *trailers_tempfile;
96 +
97 +static FILE *create_in_place_tempfile(const char *file)
98 +{
99 + struct stat st;
100 + struct strbuf filename_template = STRBUF_INIT;
101 + const char *tail;
102 + FILE *outfile;
103 +
104 + if (stat(file, &st))
105 + die_errno(_("could not stat %s"), file);
106 + if (!S_ISREG(st.st_mode))
107 + die(_("file %s is not a regular file"), file);
108 + if (!(st.st_mode & S_IWUSR))
109 + die(_("file %s is not writable by user"), file);
110 +
111 + /* Create temporary file in the same directory as the original */
112 + tail = strrchr(file, '/');
113 + if (tail)
114 + strbuf_add(&filename_template, file, tail - file + 1);
115 + strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
116 +
117 + trailers_tempfile = xmks_tempfile_m(filename_template.buf, st.st_mode);
118 + strbuf_release(&filename_template);
119 + outfile = fdopen_tempfile(trailers_tempfile, "w");
120 + if (!outfile)
121 + die_errno(_("could not open temporary file"));
122 +
123 + return outfile;
124 +}
125 +
126 +static void read_input_file(struct strbuf *sb, const char *file)
127 +{
128 + if (file) {
129 + if (strbuf_read_file(sb, file, 0) < 0)
130 + die_errno(_("could not read input file '%s'"), file);
131 + } else {
132 + if (strbuf_read(sb, fileno(stdin), 0) < 0)
133 + die_errno(_("could not read from stdin"));
134 + }
135 +}
136 +
137 +static void interpret_trailers(const struct process_trailer_options *opts,
138 + struct list_head *new_trailer_head,
139 + const char *file)
140 +{
141 + LIST_HEAD(head);
142 + struct strbuf sb = STRBUF_INIT;
143 + struct trailer_info info;
144 + FILE *outfile = stdout;
145 +
146 + trailer_config_init();
147 +
148 + read_input_file(&sb, file);
149 +
150 + if (opts->in_place)
151 + outfile = create_in_place_tempfile(file);
152 +
153 + parse_trailers(opts, &info, sb.buf, &head);
154 +
155 + /* Print the lines before the trailers */
156 + if (!opts->only_trailers)
157 + fwrite(sb.buf, 1, info.trailer_block_start, outfile);
158 +
159 + if (!opts->only_trailers && !info.blank_line_before_trailer)
160 + fprintf(outfile, "\n");
161 +
162 +
163 + if (!opts->only_input) {
164 + LIST_HEAD(config_head);
165 + LIST_HEAD(arg_head);
166 + parse_trailers_from_config(&config_head);
167 + parse_trailers_from_command_line_args(&arg_head, new_trailer_head);
168 + list_splice(&config_head, &arg_head);
169 + process_trailers_lists(&head, &arg_head);
170 + }
171 +
172 + format_trailers(opts, &head, outfile);
173 + free_trailers(&head);
174 +
175 + /* Print the lines after the trailers as is */
176 + if (!opts->only_trailers)
177 + fwrite(sb.buf + info.trailer_block_end, 1, sb.len - info.trailer_block_end, outfile);
178 + trailer_info_release(&info);
179 +
180 + if (opts->in_place)
181 + if (rename_tempfile(&trailers_tempfile, file))
182 + die_errno(_("could not rename temporary file to %s"), file);
183 +
184 + strbuf_release(&sb);
185 +}
186 +
187 int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
188 {
189 struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
trailer.c
+13 -106
@@ -5,7 +5,6 @@
5 #include "string-list.h"
6 #include "run-command.h"
7 #include "commit.h"
8 -#include "tempfile.h"
8 #include "trailer.h"
9 #include "list.h"
10 /*
@@ -163,8 +162,8 @@ static void print_tok_val(FILE *outfile, const char *tok, const char *val)
162 fprintf(outfile, "%s%c %s\n", tok, separators[0], val);
163 }
164
166 -static void format_trailers(const struct process_trailer_options *opts,
167 - struct list_head *trailers, FILE *outfile)
165 +void format_trailers(const struct process_trailer_options *opts,
166 + struct list_head *trailers, FILE *outfile)
167 {
168 struct list_head *pos;
169 struct trailer_item *item;
@@ -366,8 +365,8 @@ static int find_same_and_apply_arg(struct list_head *head,
365 return 0;
366 }
367
369 -static void process_trailers_lists(struct list_head *head,
370 - struct list_head *arg_head)
368 +void process_trailers_lists(struct list_head *head,
369 + struct list_head *arg_head)
370 {
371 struct list_head *pos, *p;
372 struct arg_item *arg_tok;
@@ -589,7 +588,7 @@ static int git_trailer_config(const char *conf_key, const char *value,
588 return 0;
589 }
590
592 -static void trailer_config_init(void)
591 +void trailer_config_init(void)
592 {
593 if (configured)
594 return;
@@ -719,7 +718,7 @@ static void add_arg_item(struct list_head *arg_head, char *tok, char *val,
718 list_add_tail(&new_item->list, arg_head);
719 }
720
722 -static void parse_trailers_from_config(struct list_head *config_head)
721 +void parse_trailers_from_config(struct list_head *config_head)
722 {
723 struct arg_item *item;
724 struct list_head *pos;
@@ -735,8 +734,8 @@ static void parse_trailers_from_config(struct list_head *config_head)
734 }
735 }
736
738 -static void parse_trailers_from_command_line_args(struct list_head *arg_head,
739 - struct list_head *new_trailer_head)
737 +void parse_trailers_from_command_line_args(struct list_head *arg_head,
738 + struct list_head *new_trailer_head)
739 {
740 struct strbuf tok = STRBUF_INIT;
741 struct strbuf val = STRBUF_INIT;
@@ -775,17 +774,6 @@ static void parse_trailers_from_command_line_args(struct list_head *arg_head,
774 free(cl_separators);
775 }
776
778 -static void read_input_file(struct strbuf *sb, const char *file)
779 -{
780 - if (file) {
781 - if (strbuf_read_file(sb, file, 0) < 0)
782 - die_errno(_("could not read input file '%s'"), file);
783 - } else {
784 - if (strbuf_read(sb, fileno(stdin), 0) < 0)
785 - die_errno(_("could not read from stdin"));
786 - }
787 -}
788 -
777 static const char *next_line(const char *str)
778 {
779 const char *nl = strchrnul(str, '\n');
@@ -1000,10 +988,10 @@ static void unfold_value(struct strbuf *val)
988 * Parse trailers in "str", populating the trailer info and "head"
989 * linked list structure.
990 */
1003 -static void parse_trailers(struct trailer_info *info,
1004 - const char *str,
1005 - struct list_head *head,
1006 - const struct process_trailer_options *opts)
991 +void parse_trailers(const struct process_trailer_options *opts,
992 + struct trailer_info *info,
993 + const char *str,
994 + struct list_head *head)
995 {
996 struct strbuf tok = STRBUF_INIT;
997 struct strbuf val = STRBUF_INIT;
@@ -1035,7 +1023,7 @@ static void parse_trailers(struct trailer_info *info,
1023 }
1024 }
1025
1038 -static void free_trailers(struct list_head *trailers)
1026 +void free_trailers(struct list_head *trailers)
1027 {
1028 struct list_head *pos, *p;
1029 list_for_each_safe(pos, p, trailers) {
@@ -1044,87 +1032,6 @@ static void free_trailers(struct list_head *trailers)
1032 }
1033 }
1034
1047 -static struct tempfile *trailers_tempfile;
1048 -
1049 -static FILE *create_in_place_tempfile(const char *file)
1050 -{
1051 - struct stat st;
1052 - struct strbuf filename_template = STRBUF_INIT;
1053 - const char *tail;
1054 - FILE *outfile;
1055 -
1056 - if (stat(file, &st))
1057 - die_errno(_("could not stat %s"), file);
1058 - if (!S_ISREG(st.st_mode))
1059 - die(_("file %s is not a regular file"), file);
1060 - if (!(st.st_mode & S_IWUSR))
1061 - die(_("file %s is not writable by user"), file);
1062 -
1063 - /* Create temporary file in the same directory as the original */
1064 - tail = strrchr(file, '/');
1065 - if (tail)
1066 - strbuf_add(&filename_template, file, tail - file + 1);
1067 - strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
1068 -
1069 - trailers_tempfile = xmks_tempfile_m(filename_template.buf, st.st_mode);
1070 - strbuf_release(&filename_template);
1071 - outfile = fdopen_tempfile(trailers_tempfile, "w");
1072 - if (!outfile)
1073 - die_errno(_("could not open temporary file"));
1074 -
1075 - return outfile;
1076 -}
1077 -
1078 -void interpret_trailers(const struct process_trailer_options *opts,
1079 - struct list_head *new_trailer_head,
1080 - const char *file)
1081 -{
1082 - LIST_HEAD(head);
1083 - struct strbuf sb = STRBUF_INIT;
1084 - struct trailer_info info;
1085 - FILE *outfile = stdout;
1086 -
1087 - trailer_config_init();
1088 -
1089 - read_input_file(&sb, file);
1090 -
1091 - if (opts->in_place)
1092 - outfile = create_in_place_tempfile(file);
1093 -
1094 - parse_trailers(&info, sb.buf, &head, opts);
1095 -
1096 - /* Print the lines before the trailers */
1097 - if (!opts->only_trailers)
1098 - fwrite(sb.buf, 1, info.trailer_block_start, outfile);
1099 -
1100 - if (!opts->only_trailers && !info.blank_line_before_trailer)
1101 - fprintf(outfile, "\n");
1102 -
1103 -
1104 - if (!opts->only_input) {
1105 - LIST_HEAD(config_head);
1106 - LIST_HEAD(arg_head);
1107 - parse_trailers_from_config(&config_head);
1108 - parse_trailers_from_command_line_args(&arg_head, new_trailer_head);
1109 - list_splice(&config_head, &arg_head);
1110 - process_trailers_lists(&head, &arg_head);
1111 - }
1112 -
1113 - format_trailers(opts, &head, outfile);
1114 - free_trailers(&head);
1115 -
1116 - /* Print the lines after the trailers as is */
1117 - if (!opts->only_trailers)
1118 - fwrite(sb.buf + info.trailer_block_end, 1, sb.len - info.trailer_block_end, outfile);
1119 - trailer_info_release(&info);
1120 -
1121 - if (opts->in_place)
1122 - if (rename_tempfile(&trailers_tempfile, file))
1123 - die_errno(_("could not rename temporary file to %s"), file);
1124 -
1125 - strbuf_release(&sb);
1126 -}
1127 -
1035 void trailer_info_get(struct trailer_info *info, const char *str,
1036 const struct process_trailer_options *opts)
1037 {
trailer.h
+17 -3
@@ -81,15 +81,29 @@ struct process_trailer_options {
81
82 #define PROCESS_TRAILER_OPTIONS_INIT {0}
83
84 -void interpret_trailers(const struct process_trailer_options *opts,
85 - struct list_head *new_trailer_head,
86 - const char *file);
84 +void parse_trailers_from_config(struct list_head *config_head);
85 +
86 +void parse_trailers_from_command_line_args(struct list_head *arg_head,
87 + struct list_head *new_trailer_head);
88 +
89 +void process_trailers_lists(struct list_head *head,
90 + struct list_head *arg_head);
91 +
92 +void parse_trailers(const struct process_trailer_options *,
93 + struct trailer_info *,
94 + const char *str,
95 + struct list_head *head);
96
97 void trailer_info_get(struct trailer_info *info, const char *str,
98 const struct process_trailer_options *opts);
99
100 void trailer_info_release(struct trailer_info *info);
101
102 +void trailer_config_init(void);
103 +void format_trailers(const struct process_trailer_options *,
104 + struct list_head *trailers, FILE *outfile);
105 +void free_trailers(struct list_head *);
106 +
107 /*
108 * Format the trailers from the commit msg "msg" into the strbuf "out".
109 * Note two caveats about "opts":