{"thread":{"id":"32122","subject":"[PATCH] Unify appending signoff in format-patch, commit and sequencer","startedAt":"2012-11-15T12:32:58Z","lastAt":"2012-11-16T04:13:42Z","messageCount":3,"participants":["Nguyễn Thái Ngọc Duy","Brandon Casey","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"203287","messageId":"1352982778-28631-1-git-send-email-pclouds@gmail.com","threadId":"32122","inReplyTo":null,"subject":"[PATCH] Unify appending signoff in format-patch, commit and sequencer","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-11-15T12:32:58Z","receivedAt":"2012-11-15T12:32:58Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"There are two implementations of append_signoff in log-tree.c and\nsequencer.c, which do more or less the same thing. This patch\n\n - teaches sequencer.c's append_signoff() not to append the signoff if\n   it's already there but not at the bottom\n\n - removes log-tree.c's\n\n - make sure \"Signed-off-by: foo\" in the middle of a line is not\n   counted as a sign off\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Interestingly this patch triggers the fault that it fixes.\n I was surprised that there was no blank line before my S-o-b\n and thought I broke something. It turns out I used unmodified\n format-patch and it mistook the S-o-b quote as true S-o-b line.\n The modified one puts the blank line back.\n\n builtin/log.c | 13 +--------\n log-tree.c    | 92 ++++-------------------------------------------------------\n revision.h    |  2 +-\n sequencer.c   | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++----\n 4 files changed, 88 insertions(+), 105 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex e7b7db1..bb48344 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1058,7 +1058,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct commit *origin = NULL, *head = NULL;\n \tconst char *in_reply_to = NULL;\n \tstruct patch_ids ids;\n-\tchar *add_signoff = NULL;\n \tstruct strbuf buf = STRBUF_INIT;\n \tint use_patch_format = 0;\n \tint quiet = 0;\n@@ -1154,16 +1153,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |\n \t\t\t     PARSE_OPT_KEEP_DASHDASH);\n \n-\tif (do_signoff) {\n-\t\tconst char *committer;\n-\t\tconst char *endpos;\n-\t\tcommitter = git_committer_info(IDENT_STRICT);\n-\t\tendpos = strchr(committer, '>');\n-\t\tif (!endpos)\n-\t\t\tdie(_(\"bogus committer info %s\"), committer);\n-\t\tadd_signoff = xmemdupz(committer, endpos - committer + 1);\n-\t}\n-\n \tfor (i = 0; i < extra_hdr.nr; i++) {\n \t\tstrbuf_addstr(&buf, extra_hdr.items[i].string);\n \t\tstrbuf_addch(&buf, '\\n');\n@@ -1354,7 +1343,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\ttotal++;\n \t\tstart_number--;\n \t}\n-\trev.add_signoff = add_signoff;\n+\trev.add_signoff = do_signoff;\n \twhile (0 <= --nr) {\n \t\tint shown;\n \t\tcommit = list[nr];\ndiff --git a/log-tree.c b/log-tree.c\nindex c894930..18cf006 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -9,6 +9,7 @@\n #include \"string-list.h\"\n #include \"color.h\"\n #include \"gpg-interface.h\"\n+#include \"sequencer.h\"\n \n struct decoration name_decoration = { \"object names\" };\n \n@@ -206,89 +207,6 @@ void show_decorations(struct rev_info *opt, struct commit *commit)\n \tputchar(')');\n }\n \n-/*\n- * Search for \"^[-A-Za-z]+: [^@]+@\" pattern. It usually matches\n- * Signed-off-by: and Acked-by: lines.\n- */\n-static int detect_any_signoff(char *letter, int size)\n-{\n-\tchar *cp;\n-\tint seen_colon = 0;\n-\tint seen_at = 0;\n-\tint seen_name = 0;\n-\tint seen_head = 0;\n-\n-\tcp = letter + size;\n-\twhile (letter <= --cp && *cp == '\\n')\n-\t\tcontinue;\n-\n-\twhile (letter <= cp) {\n-\t\tchar ch = *cp--;\n-\t\tif (ch == '\\n')\n-\t\t\tbreak;\n-\n-\t\tif (!seen_at) {\n-\t\t\tif (ch == '@')\n-\t\t\t\tseen_at = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!seen_colon) {\n-\t\t\tif (ch == '@')\n-\t\t\t\treturn 0;\n-\t\t\telse if (ch == ':')\n-\t\t\t\tseen_colon = 1;\n-\t\t\telse\n-\t\t\t\tseen_name = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (('A' <= ch && ch <= 'Z') ||\n-\t\t    ('a' <= ch && ch <= 'z') ||\n-\t\t    ch == '-') {\n-\t\t\tseen_head = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\t/* no empty last line doesn't match */\n-\t\treturn 0;\n-\t}\n-\treturn seen_head && seen_name;\n-}\n-\n-static void append_signoff(struct strbuf *sb, const char *signoff)\n-{\n-\tstatic const char signed_off_by[] = \"Signed-off-by: \";\n-\tsize_t signoff_len = strlen(signoff);\n-\tint has_signoff = 0;\n-\tchar *cp;\n-\n-\tcp = sb->buf;\n-\n-\t/* First see if we already have the sign-off by the signer */\n-\twhile ((cp = strstr(cp, signed_off_by))) {\n-\n-\t\thas_signoff = 1;\n-\n-\t\tcp += strlen(signed_off_by);\n-\t\tif (cp + signoff_len >= sb->buf + sb->len)\n-\t\t\tbreak;\n-\t\tif (strncmp(cp, signoff, signoff_len))\n-\t\t\tcontinue;\n-\t\tif (!isspace(cp[signoff_len]))\n-\t\t\tcontinue;\n-\t\t/* we already have him */\n-\t\treturn;\n-\t}\n-\n-\tif (!has_signoff)\n-\t\thas_signoff = detect_any_signoff(sb->buf, sb->len);\n-\n-\tif (!has_signoff)\n-\t\tstrbuf_addch(sb, '\\n');\n-\n-\tstrbuf_addstr(sb, signed_off_by);\n-\tstrbuf_add(sb, signoff, signoff_len);\n-\tstrbuf_addch(sb, '\\n');\n-}\n-\n static unsigned int digits_in_number(unsigned int number)\n {\n \tunsigned int i = 10, result = 1;\n@@ -651,8 +569,10 @@ void show_log(struct rev_info *opt)\n \t/*\n \t * And then the pretty-printed message itself\n \t */\n-\tif (ctx.need_8bit_cte >= 0)\n-\t\tctx.need_8bit_cte = has_non_ascii(opt->add_signoff);\n+\tif (ctx.need_8bit_cte >= 0 && opt->add_signoff)\n+\t\tctx.need_8bit_cte =\n+\t\t\thas_non_ascii(fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n+\t\t\t\t\t       getenv(\"GIT_COMMITTER_EMAIL\")));\n \tctx.date_mode = opt->date_mode;\n \tctx.date_mode_explicit = opt->date_mode_explicit;\n \tctx.abbrev = opt->diffopt.abbrev;\n@@ -663,7 +583,7 @@ void show_log(struct rev_info *opt)\n \tpretty_print_commit(&ctx, commit, &msgbuf);\n \n \tif (opt->add_signoff)\n-\t\tappend_signoff(&msgbuf, opt->add_signoff);\n+\t\tappend_signoff(&msgbuf, 0);\n \tif (opt->show_log_size) {\n \t\tprintf(\"log size %i\\n\", (int)msgbuf.len);\n \t\tgraph_show_oneline(opt->graph);\ndiff --git a/revision.h b/revision.h\nindex a95bd0b..af35325 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -136,7 +136,7 @@ struct rev_info {\n \tint\t\tnumbered_files;\n \tchar\t\t*message_id;\n \tstruct string_list *ref_message_ids;\n-\tconst char\t*add_signoff;\n+\tint              add_signoff;\n \tconst char\t*extra_headers;\n \tconst char\t*log_reencode;\n \tconst char\t*subject_prefix;\ndiff --git a/sequencer.c b/sequencer.c\nindex be0cb8b..4eb59e4 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1058,21 +1058,95 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)\n \treturn 1;\n }\n \n+/*\n+ * Search for \"^[-A-Za-z]+: [^@]+@\" pattern. It usually matches\n+ * Signed-off-by: and Acked-by: lines.\n+ */\n+static int detect_any_signoff(char *letter, int size)\n+{\n+\tchar *cp;\n+\tint seen_colon = 0;\n+\tint seen_at = 0;\n+\tint seen_name = 0;\n+\tint seen_head = 0;\n+\n+\tcp = letter + size;\n+\twhile (letter <= --cp && *cp == '\\n')\n+\t\tcontinue;\n+\n+\twhile (letter <= cp) {\n+\t\tchar ch = *cp--;\n+\t\tif (ch == '\\n')\n+\t\t\tbreak;\n+\n+\t\tif (!seen_at) {\n+\t\t\tif (ch == '@')\n+\t\t\t\tseen_at = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!seen_colon) {\n+\t\t\tif (ch == '@')\n+\t\t\t\treturn 0;\n+\t\t\telse if (ch == ':')\n+\t\t\t\tseen_colon = 1;\n+\t\t\telse\n+\t\t\t\tseen_name = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (('A' <= ch && ch <= 'Z') ||\n+\t\t    ('a' <= ch && ch <= 'z') ||\n+\t\t    ch == '-') {\n+\t\t\tseen_head = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\t/* no empty last line doesn't match */\n+\t\treturn 0;\n+\t}\n+\treturn seen_head && seen_name;\n+}\n+\n void append_signoff(struct strbuf *msgbuf, int ignore_footer)\n {\n \tstruct strbuf sob = STRBUF_INIT;\n-\tint i;\n+\tconst char *cp;\n+\tint i, has_signoff = 0;\n+\tint signoff_header_len = strlen(sign_off_header);\n \n \tstrbuf_addstr(&sob, sign_off_header);\n \tstrbuf_addstr(&sob, fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n \t\t\t\tgetenv(\"GIT_COMMITTER_EMAIL\")));\n-\tstrbuf_addch(&sob, '\\n');\n \tfor (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\\n'; i--)\n \t\t; /* do nothing */\n-\tif (prefixcmp(msgbuf->buf + i, sob.buf)) {\n-\t\tif (!i || !ends_rfc2822_footer(msgbuf, ignore_footer))\n-\t\t\tstrbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, \"\\n\", 1);\n-\t\tstrbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len);\n+\n+\t/* First see if we already have the sign-off by the signer */\n+\tcp = msgbuf->buf;\n+\twhile ((cp = strstr(cp, sign_off_header)) &&\n+\t       cp + signoff_header_len < msgbuf->buf + msgbuf->len - ignore_footer) {\n+\n+\t\tif (cp > msgbuf->buf && cp[-1] != '\\n' && cp[-1] != '\\r') {\n+\t\t\t/*\n+\t\t\t * Signed-off-by: found in the middle of the\n+\t\t\t * commit message is not really a sign off\n+\t\t\t */\n+\t\t\tcp += signoff_header_len;\n+\t\t\tcontinue;\n+\t\t}\n+\t\thas_signoff = 1;\n+\n+\t       if (cp + sob.len >= msgbuf->buf + msgbuf->len)\n+\t\t       break;\n+\t       if (!strncmp(cp, sob.buf, sob.len) && isspace(cp[sob.len]))\n+\t\t       return;\t/* we already have him */\n+\t       cp += signoff_header_len;\n \t}\n+\n+\tif (!has_signoff)\n+\t\thas_signoff = detect_any_signoff(msgbuf->buf,\n+\t\t\t\t\t\t msgbuf->len - ignore_footer);\n+\n+\tif (!i || (!has_signoff && !ends_rfc2822_footer(msgbuf, ignore_footer)))\n+\t\tstrbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, \"\\n\", 1);\n+\tstrbuf_addch(&sob, '\\n');\n+\tstrbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len);\n \tstrbuf_release(&sob);\n }\n-- \n1.8.0.151.g12dbe03\n"},{"id":"203315","messageId":"CA+sFfMcfGbGDeXKZD1o4pmhbrJ1E5WgV3qrQBPT3eH=W54tNGw@mail.gmail.com","threadId":"32122","inReplyTo":"1352982778-28631-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] Unify appending signoff in format-patch, commit and sequencer","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2012-11-15T20:42:41Z","receivedAt":"2012-11-15T20:42:41Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On Thu, Nov 15, 2012 at 4:32 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:\n> There are two implementations of append_signoff in log-tree.c and\n> sequencer.c, which do more or less the same thing. This patch\n>\n>  - teaches sequencer.c's append_signoff() not to append the signoff if\n>    it's already there but not at the bottom\n>\n>  - removes log-tree.c's\n>\n>  - make sure \"Signed-off-by: foo\" in the middle of a line is not\n>    counted as a sign off\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  Interestingly this patch triggers the fault that it fixes.\n>  I was surprised that there was no blank line before my S-o-b\n>  and thought I broke something. It turns out I used unmodified\n>  format-patch and it mistook the S-o-b quote as true S-o-b line.\n>  The modified one puts the blank line back.\n\nHeh, yeah I noticed this bug yesterday when I submitted my changes\naffecting the same area of code (sequence.c).  Glad I didn't waste too\nmuch time fixing it.\n\nHave you looked at this:\n\n   http://thread.gmane.org/gmane.comp.version-control.git/209781\n\nThat series doesn't duplicate your work, but the two series's should\nbe resolved.\n\n-Brandon\n"},{"id":"203327","messageId":"CACsJy8ABogKMujC9KhDeLXkwLpd0pkg03hqWzbH0Lmi0A7xoCA@mail.gmail.com","threadId":"32122","inReplyTo":"CA+sFfMcfGbGDeXKZD1o4pmhbrJ1E5WgV3qrQBPT3eH=W54tNGw@mail.gmail.com","subject":"Re: [PATCH] Unify appending signoff in format-patch, commit and sequencer","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-11-16T04:13:42Z","receivedAt":"2012-11-16T04:13:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Nov 16, 2012 at 3:42 AM, Brandon Casey <drafnel@gmail.com> wrote:\n>>  Interestingly this patch triggers the fault that it fixes.\n>>  I was surprised that there was no blank line before my S-o-b\n>>  and thought I broke something. It turns out I used unmodified\n>>  format-patch and it mistook the S-o-b quote as true S-o-b line.\n>>  The modified one puts the blank line back.\n>\n> Heh, yeah I noticed this bug yesterday when I submitted my changes\n> affecting the same area of code (sequence.c).  Glad I didn't waste too\n> much time fixing it.\n>\n> Have you looked at this:\n>\n>    http://thread.gmane.org/gmane.comp.version-control.git/209781\n>\n> That series doesn't duplicate your work, but the two series's should\n> be resolved.\n\nThanks I'm watching that discussion now and probably will rebase on\ntop of yours once it's finalized. I actually wrote this patch a while\nago, just waiting for nd/builtin-to-libgit to graduate because of some\nlinking issues this patch causes. I can wait a bit longer until yours\ngraduates.\n-- \nDuy\n"}]}