Permalink
Browse files

Merge branch 'bc/append-signed-off-by'

Consolidate codepaths that inspect log-message-to-be and decide to
add a new Signed-off-by line in various commands.

* bc/append-signed-off-by:
  git-commit: populate the edit buffer with 2 blank lines before s-o-b
  Unify appending signoff in format-patch, commit and sequencer
  format-patch: update append_signoff prototype
  t4014: more tests about appending s-o-b lines
  sequencer.c: teach append_signoff to avoid adding a duplicate newline
  sequencer.c: teach append_signoff how to detect duplicate s-o-b
  sequencer.c: always separate "(cherry picked from" from commit body
  sequencer.c: require a conforming footer to be preceded by a blank line
  sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
  t/t3511: add some tests of 'cherry-pick -s' functionality
  t/test-lib-functions.sh: allow to specify the tag name to test_commit
  commit, cherry-pick -s: remove broken support for multiline rfc2822 fields
  sequencer.c: rework search for start of footer to improve clarity
  • Loading branch information...
2 parents f161fb0 + 8c613fd commit 900c8ecb5c98c3cae256dcc030476392fdf2bbf2 @gitster gitster committed Apr 1, 2013
Showing with 650 additions and 156 deletions.
  1. +1 −1 builtin/commit.c
  2. +1 −12 builtin/log.c
  3. +6 −86 log-tree.c
  4. +1 −1 revision.h
  5. +141 −51 sequencer.c
  6. +3 −1 sequencer.h
  7. +219 −0 t/t3511-cherry-pick-x.sh
  8. +262 −0 t/t4014-format-patch.sh
  9. +12 −0 t/t7502-commit.sh
  10. +4 −4 t/test-lib-functions.sh
View
@@ -702,7 +702,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
previous = eol;
}
- append_signoff(&sb, ignore_footer);
+ append_signoff(&sb, ignore_footer, 0);
}
if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
View
@@ -1086,7 +1086,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
struct commit *origin = NULL, *head = NULL;
const char *in_reply_to = NULL;
struct patch_ids ids;
- char *add_signoff = NULL;
struct strbuf buf = STRBUF_INIT;
int use_patch_format = 0;
int quiet = 0;
@@ -1193,16 +1192,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
rev.subject_prefix = strbuf_detach(&sprefix, NULL);
}
- if (do_signoff) {
- const char *committer;
- const char *endpos;
- committer = git_committer_info(IDENT_STRICT);
- endpos = strchr(committer, '>');
- if (!endpos)
- die(_("bogus committer info %s"), committer);
- add_signoff = xmemdupz(committer, endpos - committer + 1);
- }
-
for (i = 0; i < extra_hdr.nr; i++) {
strbuf_addstr(&buf, extra_hdr.items[i].string);
strbuf_addch(&buf, '\n');
@@ -1393,7 +1382,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
total++;
start_number--;
}
- rev.add_signoff = add_signoff;
+ rev.add_signoff = do_signoff;
while (0 <= --nr) {
int shown;
commit = list[nr];
View
@@ -9,6 +9,7 @@
#include "string-list.h"
#include "color.h"
#include "gpg-interface.h"
+#include "sequencer.h"
struct decoration name_decoration = { "object names" };
@@ -206,89 +207,6 @@ void show_decorations(struct rev_info *opt, struct commit *commit)
putchar(')');
}
-/*
- * Search for "^[-A-Za-z]+: [^@]+@" pattern. It usually matches
- * Signed-off-by: and Acked-by: lines.
- */
-static int detect_any_signoff(char *letter, int size)
-{
- char *cp;
- int seen_colon = 0;
- int seen_at = 0;
- int seen_name = 0;
- int seen_head = 0;
-
- cp = letter + size;
- while (letter <= --cp && *cp == '\n')
- continue;
-
- while (letter <= cp) {
- char ch = *cp--;
- if (ch == '\n')
- break;
-
- if (!seen_at) {
- if (ch == '@')
- seen_at = 1;
- continue;
- }
- if (!seen_colon) {
- if (ch == '@')
- return 0;
- else if (ch == ':')
- seen_colon = 1;
- else
- seen_name = 1;
- continue;
- }
- if (('A' <= ch && ch <= 'Z') ||
- ('a' <= ch && ch <= 'z') ||
- ch == '-') {
- seen_head = 1;
- continue;
- }
- /* no empty last line doesn't match */
- return 0;
- }
- return seen_head && seen_name;
-}
-
-static void append_signoff(struct strbuf *sb, const char *signoff)
-{
- static const char signed_off_by[] = "Signed-off-by: ";
- size_t signoff_len = strlen(signoff);
- int has_signoff = 0;
- char *cp;
-
- cp = sb->buf;
-
- /* First see if we already have the sign-off by the signer */
- while ((cp = strstr(cp, signed_off_by))) {
-
- has_signoff = 1;
-
- cp += strlen(signed_off_by);
- if (cp + signoff_len >= sb->buf + sb->len)
- break;
- if (strncmp(cp, signoff, signoff_len))
- continue;
- if (!isspace(cp[signoff_len]))
- continue;
- /* we already have him */
- return;
- }
-
- if (!has_signoff)
- has_signoff = detect_any_signoff(sb->buf, sb->len);
-
- if (!has_signoff)
- strbuf_addch(sb, '\n');
-
- strbuf_addstr(sb, signed_off_by);
- strbuf_add(sb, signoff, signoff_len);
- strbuf_addch(sb, '\n');
-}
-
static unsigned int digits_in_number(unsigned int number)
{
unsigned int i = 10, result = 1;
@@ -669,8 +587,10 @@ void show_log(struct rev_info *opt)
/*
* And then the pretty-printed message itself
*/
- if (ctx.need_8bit_cte >= 0)
- ctx.need_8bit_cte = has_non_ascii(opt->add_signoff);
+ if (ctx.need_8bit_cte >= 0 && opt->add_signoff)
+ ctx.need_8bit_cte =
+ has_non_ascii(fmt_name(getenv("GIT_COMMITTER_NAME"),
+ getenv("GIT_COMMITTER_EMAIL")));
ctx.date_mode = opt->date_mode;
ctx.date_mode_explicit = opt->date_mode_explicit;
ctx.abbrev = opt->diffopt.abbrev;
@@ -683,7 +603,7 @@ void show_log(struct rev_info *opt)
pretty_print_commit(&ctx, commit, &msgbuf);
if (opt->add_signoff)
- append_signoff(&msgbuf, opt->add_signoff);
+ append_signoff(&msgbuf, 0, APPEND_SIGNOFF_DEDUP);
if ((ctx.fmt != CMIT_FMT_USERFORMAT) &&
ctx.notes_message && *ctx.notes_message) {
View
@@ -138,7 +138,7 @@ struct rev_info {
int reroll_count;
char *message_id;
struct string_list *ref_message_ids;
- const char *add_signoff;
+ int add_signoff;
const char *extra_headers;
const char *log_reencode;
const char *subject_prefix;
View
@@ -18,6 +18,89 @@
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
const char sign_off_header[] = "Signed-off-by: ";
+static const char cherry_picked_prefix[] = "(cherry picked from commit ";
+
+static int is_rfc2822_line(const char *buf, int len)
+{
+ int i;
+
+ for (i = 0; i < len; i++) {
+ int ch = buf[i];
+ if (ch == ':')
+ return 1;
+ if (!isalnum(ch) && ch != '-')
+ break;
+ }
+
+ return 0;
+}
+
+static int is_cherry_picked_from_line(const char *buf, int len)
+{
+ /*
+ * We only care that it looks roughly like (cherry picked from ...)
+ */
+ return len > strlen(cherry_picked_prefix) + 1 &&
+ !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')';
+}
+
+/*
+ * Returns 0 for non-conforming footer
+ * Returns 1 for conforming footer
+ * Returns 2 when sob exists within conforming footer
+ * Returns 3 when sob exists within conforming footer as last entry
+ */
+static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
+ int ignore_footer)
+{
+ char prev;
+ int i, k;
+ int len = sb->len - ignore_footer;
+ const char *buf = sb->buf;
+ int found_sob = 0;
+
+ /* footer must end with newline */
+ if (!len || buf[len - 1] != '\n')
+ return 0;
+
+ prev = '\0';
+ for (i = len - 1; i > 0; i--) {
+ char ch = buf[i];
+ if (prev == '\n' && ch == '\n') /* paragraph break */
+ break;
+ prev = ch;
+ }
+
+ /* require at least one blank line */
+ if (prev != '\n' || buf[i] != '\n')
+ return 0;
+
+ /* advance to start of last paragraph */
+ while (i < len - 1 && buf[i] == '\n')
+ i++;
+
+ for (; i < len; i = k) {
+ int found_rfc2822;
+
+ for (k = i; k < len && buf[k] != '\n'; k++)
+ ; /* do nothing */
+ k++;
+
+ found_rfc2822 = is_rfc2822_line(buf + i, k - i - 1);
+ if (found_rfc2822 && sob &&
+ !strncmp(buf + i, sob->buf, sob->len))
+ found_sob = k;
+
+ if (!(found_rfc2822 ||
+ is_cherry_picked_from_line(buf + i, k - i - 1)))
+ return 0;
+ }
+ if (found_sob == i)
+ return 3;
+ if (found_sob)
+ return 2;
+ return 1;
+}
static void remove_sequencer_state(void)
{
@@ -237,7 +320,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
rollback_lock_file(&index_lock);
if (opts->signoff)
- append_signoff(msgbuf, 0);
+ append_signoff(msgbuf, 0, 0);
if (!clean) {
int i;
@@ -496,7 +579,9 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
if (opts->record_origin) {
- strbuf_addstr(&msgbuf, "(cherry picked from commit ");
+ if (!has_conforming_footer(&msgbuf, NULL, 0))
+ strbuf_addch(&msgbuf, '\n');
+ strbuf_addstr(&msgbuf, cherry_picked_prefix);
strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
strbuf_addstr(&msgbuf, ")\n");
}
@@ -1021,62 +1106,67 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return pick_commits(todo_list, opts);
}
-static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
-{
- int ch;
- int hit = 0;
- int i, j, k;
- int len = sb->len - ignore_footer;
- int first = 1;
- const char *buf = sb->buf;
-
- for (i = len - 1; i > 0; i--) {
- if (hit && buf[i] == '\n')
- break;
- hit = (buf[i] == '\n');
- }
-
- while (i < len - 1 && buf[i] == '\n')
- i++;
-
- for (; i < len; i = k) {
- for (k = i; k < len && buf[k] != '\n'; k++)
- ; /* do nothing */
- k++;
-
- if ((buf[k] == ' ' || buf[k] == '\t') && !first)
- continue;
-
- first = 0;
-
- for (j = 0; i + j < len; j++) {
- ch = buf[i + j];
- if (ch == ':')
- break;
- if (isalnum(ch) ||
- (ch == '-'))
- continue;
- return 0;
- }
- }
- return 1;
-}
-
-void append_signoff(struct strbuf *msgbuf, int ignore_footer)
+void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
{
+ unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;
struct strbuf sob = STRBUF_INIT;
- int i;
+ int has_footer;
strbuf_addstr(&sob, sign_off_header);
strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"),
getenv("GIT_COMMITTER_EMAIL")));
strbuf_addch(&sob, '\n');
- for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--)
- ; /* do nothing */
- if (prefixcmp(msgbuf->buf + i, sob.buf)) {
- if (!i || !ends_rfc2822_footer(msgbuf, ignore_footer))
- strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
- strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len);
+
+ /*
+ * If the whole message buffer is equal to the sob, pretend that we
+ * found a conforming footer with a matching sob
+ */
+ if (msgbuf->len - ignore_footer == sob.len &&
+ !strncmp(msgbuf->buf, sob.buf, sob.len))
+ has_footer = 3;
+ else
+ has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);
+
+ if (!has_footer) {
+ const char *append_newlines = NULL;
+ size_t len = msgbuf->len - ignore_footer;
+
+ if (!len) {
+ /*
+ * The buffer is completely empty. Leave foom for
+ * the title and body to be filled in by the user.
+ */
+ append_newlines = "\n\n";
+ } else if (msgbuf->buf[len - 1] != '\n') {
+ /*
+ * Incomplete line. Complete the line and add a
+ * blank one so that there is an empty line between
+ * the message body and the sob.
+ */
+ append_newlines = "\n\n";
+ } else if (len == 1) {
+ /*
+ * Buffer contains a single newline. Add another
+ * so that we leave room for the title and body.
+ */
+ append_newlines = "\n";
+ } else if (msgbuf->buf[len - 2] != '\n') {
+ /*
+ * Buffer ends with a single newline. Add another
+ * so that there is an empty line between the message
+ * body and the sob.
+ */
+ append_newlines = "\n";
+ } /* else, the buffer already ends with two newlines. */
+
+ if (append_newlines)
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
+ append_newlines, strlen(append_newlines));
}
+
+ if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
+ sob.buf, sob.len);
+
strbuf_release(&sob);
}
Oops, something went wrong.

0 comments on commit 900c8ec

Please sign in to comment.