diff --git a/builtin/commit.c b/builtin/commit.c index e108c53015..cda74e9a68 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -800,32 +800,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix, if (clean_message_contents) stripspace(&sb, 0); - if (signoff) { - /* - * See if we have a Conflicts: block at the end. If yes, count - * its size, so we can ignore it. - */ - int ignore_footer = 0; - int i, eol, previous = 0; - const char *nl; - - for (i = 0; i < sb.len; i++) { - nl = memchr(sb.buf + i, '\n', sb.len - i); - if (nl) - eol = nl - sb.buf; - else - eol = sb.len; - if (starts_with(sb.buf + previous, "\nConflicts:\n")) { - ignore_footer = sb.len - previous; - break; - } - while (i < eol) - i++; - previous = eol; - } - - append_signoff(&sb, ignore_footer, 0); - } + if (signoff) + append_signoff(&sb, ignore_non_trailer(&sb), 0); if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len) die_errno(_("could not write commit template")); diff --git a/builtin/merge.c b/builtin/merge.c index bebbe5b308..215d4856e5 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -29,6 +29,7 @@ #include "remote.h" #include "fmt-merge-msg.h" #include "gpg-interface.h" +#include "sequencer.h" #define DEFAULT_TWOHEAD (1<<0) #define DEFAULT_OCTOPUS (1<<1) @@ -880,28 +881,19 @@ static int finish_automerge(struct commit *head, return 0; } -static int suggest_conflicts(int renormalizing) +static int suggest_conflicts(void) { const char *filename; FILE *fp; - int pos; + struct strbuf msgbuf = STRBUF_INIT; filename = git_path("MERGE_MSG"); fp = fopen(filename, "a"); if (!fp) die_errno(_("Could not open '%s' for writing"), filename); - fprintf(fp, "\nConflicts:\n"); - for (pos = 0; pos < active_nr; pos++) { - const struct cache_entry *ce = active_cache[pos]; - if (ce_stage(ce)) { - fprintf(fp, "\t%s\n", ce->name); - while (pos + 1 < active_nr && - !strcmp(ce->name, - active_cache[pos + 1]->name)) - pos++; - } - } + append_conflicts_hint(&msgbuf); + fputs(msgbuf.buf, fp); fclose(fp); rerere(allow_rerere_auto); printf(_("Automatic merge failed; " @@ -1550,7 +1542,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) fprintf(stderr, _("Automatic merge went well; " "stopped before committing as requested\n")); else - ret = suggest_conflicts(option_renormalize); + ret = suggest_conflicts(); done: free(branch_to_free); diff --git a/commit.c b/commit.c index 19cf8f9c67..a54cb9a454 100644 --- a/commit.c +++ b/commit.c @@ -1640,3 +1640,49 @@ const char *find_commit_header(const char *msg, const char *key, size_t *out_len } return NULL; } + +/* + * Inspect sb and determine the true "end" of the log message, in + * order to find where to put a new Signed-off-by: line. Ignored are + * trailing comment lines and blank lines, and also the traditional + * "Conflicts:" block that is not commented out, so that we can use + * "git commit -s --amend" on an existing commit that forgot to remove + * it. + * + * Returns the number of bytes from the tail to ignore, to be fed as + * the second parameter to append_signoff(). + */ +int ignore_non_trailer(struct strbuf *sb) +{ + int boc = 0; + int bol = 0; + int in_old_conflicts_block = 0; + + while (bol < sb->len) { + char *next_line; + + if (!(next_line = memchr(sb->buf + bol, '\n', sb->len - bol))) + next_line = sb->buf + sb->len; + else + next_line++; + + if (sb->buf[bol] == comment_line_char || sb->buf[bol] == '\n') { + /* is this the first of the run of comments? */ + if (!boc) + boc = bol; + /* otherwise, it is just continuing */ + } else if (starts_with(sb->buf + bol, "Conflicts:\n")) { + in_old_conflicts_block = 1; + if (!boc) + boc = bol; + } else if (in_old_conflicts_block && sb->buf[bol] == '\t') { + ; /* a pathname in the conflicts block */ + } else if (boc) { + /* the previous was not trailing comment */ + boc = 0; + in_old_conflicts_block = 0; + } + bol = next_line - sb->buf; + } + return boc ? sb->len - boc : 0; +} diff --git a/commit.h b/commit.h index bc68ccbe69..cd35ac150a 100644 --- a/commit.h +++ b/commit.h @@ -337,6 +337,9 @@ extern void free_commit_extra_headers(struct commit_extra_header *extra); extern const char *find_commit_header(const char *msg, const char *key, size_t *out_len); +/* Find the end of the log message, the right place for a new trailer. */ +extern int ignore_non_trailer(struct strbuf *sb); + typedef void (*each_mergetag_fn)(struct commit *commit, struct commit_extra_header *extra, void *cb_data); diff --git a/sequencer.c b/sequencer.c index a03d4fa252..77a1266760 100644 --- a/sequencer.c +++ b/sequencer.c @@ -267,6 +267,23 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from, return 0; } +void append_conflicts_hint(struct strbuf *msgbuf) +{ + int i; + + strbuf_addch(msgbuf, '\n'); + strbuf_commented_addf(msgbuf, "Conflicts:\n"); + for (i = 0; i < active_nr;) { + const struct cache_entry *ce = active_cache[i++]; + if (ce_stage(ce)) { + strbuf_commented_addf(msgbuf, "\t%s\n", ce->name); + while (i < active_nr && !strcmp(ce->name, + active_cache[i]->name)) + i++; + } + } +} + static int do_recursive_merge(struct commit *base, struct commit *next, const char *base_label, const char *next_label, unsigned char *head, struct strbuf *msgbuf, @@ -307,21 +324,8 @@ static int do_recursive_merge(struct commit *base, struct commit *next, if (opts->signoff) append_signoff(msgbuf, 0, 0); - if (!clean) { - int i; - strbuf_addstr(msgbuf, "\nConflicts:\n"); - for (i = 0; i < active_nr;) { - const struct cache_entry *ce = active_cache[i++]; - if (ce_stage(ce)) { - strbuf_addch(msgbuf, '\t'); - strbuf_addstr(msgbuf, ce->name); - strbuf_addch(msgbuf, '\n'); - while (i < active_nr && !strcmp(ce->name, - active_cache[i]->name)) - i++; - } - } - } + if (!clean) + append_conflicts_hint(msgbuf); return !clean; } diff --git a/sequencer.h b/sequencer.h index db43e9cf86..5ed5cb1d97 100644 --- a/sequencer.h +++ b/sequencer.h @@ -53,5 +53,6 @@ int sequencer_pick_revisions(struct replay_opts *opts); extern const char sign_off_header[]; void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag); +void append_conflicts_hint(struct strbuf *msgbuf); #endif diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh index 223b98433c..7c5ad08626 100755 --- a/t/t3507-cherry-pick-conflict.sh +++ b/t/t3507-cherry-pick-conflict.sh @@ -351,19 +351,45 @@ test_expect_success 'commit after failed cherry-pick does not add duplicated -s' test_expect_success 'commit after failed cherry-pick adds -s at the right place' ' pristine_detach initial && test_must_fail git cherry-pick picked && + git commit -a -s && - pwd && - cat < expected && -picked -Signed-off-by: C O Mitter + # Do S-o-b and Conflicts appear in the right order? + cat <<-\EOF >expect && + Signed-off-by: C O Mitter + # Conflicts: + EOF + grep -e "^# Conflicts:" -e '^Signed-off-by' <.git/COMMIT_EDITMSG >actual && + test_cmp expect actual && -Conflicts: - foo -EOF + cat <<-\EOF >expected && + picked - git show -s --pretty=format:%B > actual && + Signed-off-by: C O Mitter + EOF + + git show -s --pretty=format:%B >actual && test_cmp expected actual ' +test_expect_success 'commit --amend -s places the sign-off at the right place' ' + pristine_detach initial && + test_must_fail git cherry-pick picked && + + # emulate old-style conflicts block + mv .git/MERGE_MSG .git/MERGE_MSG+ && + sed -e "/^# Conflicts:/,\$s/^# *//" <.git/MERGE_MSG+ >.git/MERGE_MSG && + + git commit -a && + git commit --amend -s && + + # Do S-o-b and Conflicts appear in the right order? + cat <<-\EOF >expect && + Signed-off-by: C O Mitter + Conflicts: + EOF + grep -e "^Conflicts:" -e '^Signed-off-by' <.git/COMMIT_EDITMSG >actual && + test_cmp expect actual +' + test_done diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 1efb88051a..bd0ab46750 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -213,7 +213,7 @@ test_expect_success 'with 2 files arguments' ' ' test_expect_success 'with message that has comments' ' - cat basic_message >>message_with_comments && + cat basic_message >message_with_comments && sed -e "s/ Z\$/ /" >>message_with_comments <<-\EOF && # comment @@ -232,12 +232,44 @@ test_expect_success 'with message that has comments' ' Reviewed-by: Johan Cc: Peff + # last comment + EOF cat basic_patch >>expected && git interpret-trailers --trim-empty --trailer "Cc: Peff" message_with_comments >actual && test_cmp expected actual ' +test_expect_success 'with message that has an old style conflict block' ' + cat basic_message >message_with_comments && + sed -e "s/ Z\$/ /" >>message_with_comments <<-\EOF && + # comment + + # other comment + Cc: Z + # yet another comment + Reviewed-by: Johan + Reviewed-by: Z + # last comment + + Conflicts: + + EOF + cat basic_message >expected && + cat >>expected <<-\EOF && + # comment + + Reviewed-by: Johan + Cc: Peff + # last comment + + Conflicts: + + EOF + git interpret-trailers --trim-empty --trailer "Cc: Peff" message_with_comments >actual && + test_cmp expected actual +' + test_expect_success 'with commit complex message and trailer args' ' cat complex_message_body >expected && sed -e "s/ Z\$/ /" >>expected <<-\EOF && diff --git a/trailer.c b/trailer.c index a905f5c50c..623adeb02d 100644 --- a/trailer.c +++ b/trailer.c @@ -2,6 +2,7 @@ #include "string-list.h" #include "run-command.h" #include "string-list.h" +#include "commit.h" #include "trailer.h" /* * Copyright (c) 2013, 2014 Christian Couder @@ -768,6 +769,22 @@ static int find_trailer_start(struct strbuf **lines, int count) return only_spaces ? count : 0; } +/* Get the index of the end of the trailers */ +static int find_trailer_end(struct strbuf **lines, int patch_start) +{ + struct strbuf sb = STRBUF_INIT; + int i, ignore_bytes; + + for (i = 0; i < patch_start; i++) + strbuf_addbuf(&sb, lines[i]); + ignore_bytes = ignore_non_trailer(&sb); + strbuf_release(&sb); + for (i = patch_start - 1; i >= 0 && ignore_bytes > 0; i--) + ignore_bytes -= lines[i]->len; + + return i + 1; +} + static int has_blank_line_before(struct strbuf **lines, int start) { for (;start >= 0; start--) { @@ -790,14 +807,15 @@ static int process_input_file(struct strbuf **lines, struct trailer_item **in_tok_last) { int count = 0; - int patch_start, trailer_start, i; + int patch_start, trailer_start, trailer_end, i; /* Get the line count */ while (lines[count]) count++; patch_start = find_patch_start(lines, count); - trailer_start = find_trailer_start(lines, patch_start); + trailer_end = find_trailer_end(lines, patch_start); + trailer_start = find_trailer_start(lines, trailer_end); /* Print lines before the trailers as is */ print_lines(lines, 0, trailer_start); @@ -806,14 +824,14 @@ static int process_input_file(struct strbuf **lines, printf("\n"); /* Parse trailer lines */ - for (i = trailer_start; i < patch_start; i++) { + for (i = trailer_start; i < trailer_end; i++) { if (lines[i]->buf[0] != comment_line_char) { struct trailer_item *new = create_trailer_item(lines[i]->buf); add_trailer_item(in_tok_first, in_tok_last, new); } } - return patch_start; + return trailer_end; } static void free_all(struct trailer_item **first) @@ -830,7 +848,7 @@ void process_trailers(const char *file, int trim_empty, struct string_list *trai struct trailer_item *in_tok_last = NULL; struct trailer_item *arg_tok_first; struct strbuf **lines; - int patch_start; + int trailer_end; /* Default config must be setup first */ git_config(git_trailer_default_config, NULL); @@ -839,7 +857,7 @@ void process_trailers(const char *file, int trim_empty, struct string_list *trai lines = read_input_file(file); /* Print the lines before the trailers */ - patch_start = process_input_file(lines, &in_tok_first, &in_tok_last); + trailer_end = process_input_file(lines, &in_tok_first, &in_tok_last); arg_tok_first = process_command_line_args(trailers); @@ -850,7 +868,7 @@ void process_trailers(const char *file, int trim_empty, struct string_list *trai free_all(&in_tok_first); /* Print the lines after the trailers as is */ - print_lines(lines, patch_start, INT_MAX); + print_lines(lines, trailer_end, INT_MAX); strbuf_list_free(lines); }