diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index 033bd1556c..11f4ce9e4a 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -9,6 +9,7 @@ #include "gettext.h" #include "parse-options.h" #include "string-list.h" +#include "tempfile.h" #include "trailer.h" #include "config.h" @@ -91,6 +92,102 @@ 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) { + if (strbuf_read_file(sb, file, 0) < 0) + die_errno(_("could not read input file '%s'"), file); + } else { + if (strbuf_read(sb, fileno(stdin), 0) < 0) + die_errno(_("could not read from stdin")); + } +} + +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 = STRBUF_INIT; + struct trailer_info info; + FILE *outfile = stdout; + + trailer_config_init(); + + read_input_file(&sb, file); + + if (opts->in_place) + outfile = create_in_place_tempfile(file); + + parse_trailers(opts, &info, sb.buf, &head); + + /* Print the lines before the trailers */ + if (!opts->only_trailers) + fwrite(sb.buf, 1, info.trailer_block_start, outfile); + + if (!opts->only_trailers && !info.blank_line_before_trailer) + 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); + free_trailers(&head); + fwrite(trailer_block.buf, 1, trailer_block.len, outfile); + strbuf_release(&trailer_block); + + /* Print the lines after the trailers as is */ + if (!opts->only_trailers) + fwrite(sb.buf + info.trailer_block_end, 1, sb.len - info.trailer_block_end, outfile); + trailer_info_release(&info); + + 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) { struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT; @@ -132,11 +229,11 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) if (argc) { int i; for (i = 0; i < argc; i++) - process_trailers(argv[i], &opts, &trailers); + interpret_trailers(&opts, &trailers, argv[i]); } else { if (opts.in_place) die(_("no input file given for in-place editing")); - process_trailers(NULL, &opts, &trailers); + interpret_trailers(&opts, &trailers, NULL); } new_trailers_clear(&trailers); diff --git a/pretty.c b/pretty.c index cf964b060c..bdbed4295a 100644 --- a/pretty.c +++ b/pretty.c @@ -1759,7 +1759,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ goto trailer_out; } if (*arg == ')') { - format_trailers_from_commit(sb, msg + c->subject_off, &opts); + format_trailers_from_commit(&opts, msg + c->subject_off, sb); ret = arg - placeholder + 1; } trailer_out: diff --git a/ref-filter.c b/ref-filter.c index 0ec29f7385..03542d0236 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1991,7 +1991,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp struct strbuf s = STRBUF_INIT; /* Format the trailer info according to the trailer_opts given */ - format_trailers_from_commit(&s, subpos, &atom->u.contents.trailer_opts); + format_trailers_from_commit(&atom->u.contents.trailer_opts, subpos, &s); v->s = strbuf_detach(&s, NULL); } else if (atom->u.contents.option == C_BARE) diff --git a/sequencer.c b/sequencer.c index 5c6f541126..ea1441e617 100644 --- a/sequencer.c +++ b/sequencer.c @@ -332,7 +332,7 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, sb->buf[sb->len - ignore_footer] = '\0'; } - trailer_info_get(&info, sb->buf, &opts); + trailer_info_get(&opts, sb->buf, &info); if (ignore_footer) sb->buf[sb->len - ignore_footer] = saved_char; diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh index d7382709fc..f698d0c9ad 100755 --- a/t/t4201-shortlog.sh +++ b/t/t4201-shortlog.sh @@ -312,6 +312,38 @@ test_expect_success 'shortlog de-duplicates trailers in a single commit' ' test_cmp expect actual ' +# Trailers that have unfolded (single line) and folded (multiline) values which +# are otherwise identical are treated as the same trailer for de-duplication. +test_expect_success 'shortlog de-duplicates trailers in a single commit (folded/unfolded values)' ' + git commit --allow-empty -F - <<-\EOF && + subject one + + this message has two distinct values, plus a repeat (folded) + + Repeated-trailer: Foo foo foo + Repeated-trailer: Bar + Repeated-trailer: Foo + foo foo + EOF + + git commit --allow-empty -F - <<-\EOF && + subject two + + similar to the previous, but without the second distinct value + + Repeated-trailer: Foo foo foo + Repeated-trailer: Foo + foo foo + EOF + + cat >expect <<-\EOF && + 2 Foo foo foo + 1 Bar + EOF + git shortlog -ns --group=trailer:repeated-trailer -2 HEAD >actual && + test_cmp expect actual +' + test_expect_success 'shortlog can match multiple groups' ' git commit --allow-empty -F - <<-\EOF && subject one diff --git a/trailer.c b/trailer.c index ef9df4af55..57b4aa7d5a 100644 --- a/trailer.c +++ b/trailer.c @@ -5,7 +5,6 @@ #include "string-list.h" #include "run-command.h" #include "commit.h" -#include "tempfile.h" #include "trailer.h" #include "list.h" /* @@ -145,12 +144,12 @@ static char last_non_space_char(const char *s) return '\0'; } -static void print_tok_val(FILE *outfile, const char *tok, const char *val) +static void print_tok_val(struct strbuf *out, const char *tok, const char *val) { char c; if (!tok) { - fprintf(outfile, "%s\n", val); + strbuf_addf(out, "%s\n", val); return; } @@ -158,21 +157,22 @@ static void print_tok_val(FILE *outfile, const char *tok, const char *val) if (!c) return; if (strchr(separators, c)) - fprintf(outfile, "%s%s\n", tok, val); + strbuf_addf(out, "%s%s\n", tok, val); else - fprintf(outfile, "%s%c %s\n", tok, separators[0], val); + strbuf_addf(out, "%s%c %s\n", tok, separators[0], val); } -static void print_all(FILE *outfile, struct list_head *head, - const struct process_trailer_options *opts) +void format_trailers(const struct process_trailer_options *opts, + struct list_head *trailers, + struct strbuf *out) { struct list_head *pos; struct trailer_item *item; - list_for_each(pos, head) { + list_for_each(pos, trailers) { item = list_entry(pos, struct trailer_item, list); if ((!opts->trim_empty || strlen(item->value) > 0) && (!opts->only_trailers || item->token)) - print_tok_val(outfile, item->token, item->value); + print_tok_val(out, item->token, item->value); } } @@ -366,8 +366,8 @@ static int find_same_and_apply_arg(struct list_head *head, return 0; } -static void process_trailers_lists(struct list_head *head, - struct list_head *arg_head) +void process_trailers_lists(struct list_head *head, + struct list_head *arg_head) { struct list_head *pos, *p; struct arg_item *arg_tok; @@ -589,7 +589,7 @@ static int git_trailer_config(const char *conf_key, const char *value, return 0; } -static void ensure_configured(void) +void trailer_config_init(void) { if (configured) return; @@ -719,7 +719,7 @@ static void add_arg_item(struct list_head *arg_head, char *tok, char *val, list_add_tail(&new_item->list, arg_head); } -static void parse_trailers_from_config(struct list_head *config_head) +void parse_trailers_from_config(struct list_head *config_head) { struct arg_item *item; struct list_head *pos; @@ -735,8 +735,8 @@ static void parse_trailers_from_config(struct list_head *config_head) } } -static void parse_trailers_from_command_line_args(struct list_head *arg_head, - struct list_head *new_trailer_head) +void parse_trailers_from_command_line_args(struct list_head *arg_head, + struct list_head *new_trailer_head) { struct strbuf tok = STRBUF_INIT; struct strbuf val = STRBUF_INIT; @@ -775,17 +775,6 @@ static void parse_trailers_from_command_line_args(struct list_head *arg_head, free(cl_separators); } -static void read_input_file(struct strbuf *sb, const char *file) -{ - if (file) { - if (strbuf_read_file(sb, file, 0) < 0) - die_errno(_("could not read input file '%s'"), file); - } else { - if (strbuf_read(sb, fileno(stdin), 0) < 0) - die_errno(_("could not read from stdin")); - } -} - static const char *next_line(const char *str) { const char *nl = strchrnul(str, '\n'); @@ -999,16 +988,16 @@ static void unfold_value(struct strbuf *val) * Parse trailers in "str", populating the trailer info and "head" * linked list structure. */ -static void parse_trailers(struct trailer_info *info, - const char *str, - struct list_head *head, - const struct process_trailer_options *opts) +void parse_trailers(const struct process_trailer_options *opts, + struct trailer_info *info, + const char *str, + struct list_head *head) { struct strbuf tok = STRBUF_INIT; struct strbuf val = STRBUF_INIT; size_t i; - trailer_info_get(info, str, opts); + trailer_info_get(opts, str, info); for (i = 0; i < info->trailer_nr; i++) { int separator_pos; @@ -1034,99 +1023,18 @@ static void parse_trailers(struct trailer_info *info, } } -static void free_all(struct list_head *head) +void free_trailers(struct list_head *trailers) { struct list_head *pos, *p; - list_for_each_safe(pos, p, head) { + list_for_each_safe(pos, p, trailers) { list_del(pos); free_trailer_item(list_entry(pos, struct trailer_item, list)); } } -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; -} - -void process_trailers(const char *file, - const struct process_trailer_options *opts, - struct list_head *new_trailer_head) -{ - LIST_HEAD(head); - struct strbuf sb = STRBUF_INIT; - struct trailer_info info; - FILE *outfile = stdout; - - ensure_configured(); - - read_input_file(&sb, file); - - if (opts->in_place) - outfile = create_in_place_tempfile(file); - - parse_trailers(&info, sb.buf, &head, opts); - - /* Print the lines before the trailers */ - if (!opts->only_trailers) - fwrite(sb.buf, 1, info.trailer_block_start, outfile); - - if (!opts->only_trailers && !info.blank_line_before_trailer) - 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_all(outfile, &head, opts); - - free_all(&head); - trailer_info_release(&info); - - /* Print the lines after the trailers as is */ - if (!opts->only_trailers) - fwrite(sb.buf + info.trailer_block_end, 1, sb.len - info.trailer_block_end, outfile); - - if (opts->in_place) - if (rename_tempfile(&trailers_tempfile, file)) - die_errno(_("could not rename temporary file to %s"), file); - - strbuf_release(&sb); -} - -void trailer_info_get(struct trailer_info *info, const char *str, - const struct process_trailer_options *opts) +void trailer_info_get(const struct process_trailer_options *opts, + const char *str, + struct trailer_info *info) { size_t end_of_log_message = 0, trailer_block_start = 0; struct strbuf **trailer_lines, **ptr; @@ -1134,7 +1042,7 @@ void trailer_info_get(struct trailer_info *info, const char *str, size_t nr = 0, alloc = 0; char **last = NULL; - ensure_configured(); + trailer_config_init(); end_of_log_message = find_end_of_log_message(str, opts->no_divider); trailer_block_start = find_trailer_block_start(str, end_of_log_message); @@ -1176,23 +1084,13 @@ void trailer_info_release(struct trailer_info *info) free(info->trailers); } -static void format_trailer_info(struct strbuf *out, +static void format_trailer_info(const struct process_trailer_options *opts, const struct trailer_info *info, - const char *msg, - const struct process_trailer_options *opts) + struct strbuf *out) { size_t origlen = out->len; size_t i; - /* If we want the whole block untouched, we can take the fast path. */ - if (!opts->only_trailers && !opts->unfold && !opts->filter && - !opts->separator && !opts->key_only && !opts->value_only && - !opts->key_value_separator) { - strbuf_add(out, msg + info->trailer_block_start, - info->trailer_block_end - info->trailer_block_start); - return; - } - for (i = 0; i < info->trailer_nr; i++) { char *trailer = info->trailers[i]; ssize_t separator_pos = find_separator(trailer, separators); @@ -1237,13 +1135,25 @@ static void format_trailer_info(struct strbuf *out, } -void format_trailers_from_commit(struct strbuf *out, const char *msg, - const struct process_trailer_options *opts) +void format_trailers_from_commit(const struct process_trailer_options *opts, + const char *msg, + struct strbuf *out) { + LIST_HEAD(trailer_objects); struct trailer_info info; - trailer_info_get(&info, msg, opts); - format_trailer_info(out, &info, msg, opts); + parse_trailers(opts, &info, msg, &trailer_objects); + + /* If we want the whole block untouched, we can take the fast path. */ + if (!opts->only_trailers && !opts->unfold && !opts->filter && + !opts->separator && !opts->key_only && !opts->value_only && + !opts->key_value_separator) { + strbuf_add(out, msg + info.trailer_block_start, + info.trailer_block_end - info.trailer_block_start); + } else + format_trailer_info(opts, &info, out); + + free_trailers(&trailer_objects); trailer_info_release(&info); } @@ -1253,7 +1163,7 @@ void trailer_iterator_init(struct trailer_iterator *iter, const char *msg) strbuf_init(&iter->key, 0); strbuf_init(&iter->val, 0); opts.no_divider = 1; - trailer_info_get(&iter->internal.info, msg, &opts); + trailer_info_get(&opts, msg, &iter->internal.info); iter->internal.cur = 0; } @@ -1270,6 +1180,7 @@ int trailer_iterator_advance(struct trailer_iterator *iter) strbuf_reset(&iter->val); parse_trailer(&iter->key, &iter->val, NULL, trailer, separator_pos); + /* Always unfold values during iteration. */ unfold_value(&iter->val); return 1; } diff --git a/trailer.h b/trailer.h index 1644cd05f6..1d106b6dd4 100644 --- a/trailer.h +++ b/trailer.h @@ -81,15 +81,31 @@ struct process_trailer_options { #define PROCESS_TRAILER_OPTIONS_INIT {0} -void process_trailers(const char *file, - const struct process_trailer_options *opts, - struct list_head *new_trailer_head); +void parse_trailers_from_config(struct list_head *config_head); -void trailer_info_get(struct trailer_info *info, const char *str, - const struct process_trailer_options *opts); +void parse_trailers_from_command_line_args(struct list_head *arg_head, + struct list_head *new_trailer_head); + +void process_trailers_lists(struct list_head *head, + struct list_head *arg_head); + +void parse_trailers(const struct process_trailer_options *, + struct trailer_info *, + const char *str, + struct list_head *head); + +void trailer_info_get(const struct process_trailer_options *, + const char *str, + struct trailer_info *); void trailer_info_release(struct trailer_info *info); +void trailer_config_init(void); +void format_trailers(const struct process_trailer_options *, + struct list_head *trailers, + struct strbuf *out); +void free_trailers(struct list_head *); + /* * Format the trailers from the commit msg "msg" into the strbuf "out". * Note two caveats about "opts": @@ -101,8 +117,9 @@ void trailer_info_release(struct trailer_info *info); * only the trailer block itself, even if the "only_trailers" option is not * set. */ -void format_trailers_from_commit(struct strbuf *out, const char *msg, - const struct process_trailer_options *opts); +void format_trailers_from_commit(const struct process_trailer_options *opts, + const char *msg, + struct strbuf *out); /* * An interface for iterating over the trailers found in a particular commit