{"thread":{"id":"12320","subject":"[PATCH 2/2] Improve collection of information for format-patch --cover-letter","startedAt":"2008-02-25T23:24:17Z","lastAt":"2008-02-28T15:20:59Z","messageCount":8,"participants":["Daniel Barkalow","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"69944","messageId":"alpine.LNX.1.00.0802251822240.19024@iabervon.org","threadId":"12320","inReplyTo":null,"subject":"[PATCH 2/2] Improve collection of information for format-patch --cover-letter","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-25T23:24:17Z","receivedAt":"2008-02-25T23:24:17Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"Use the \"boundary\" feature to find the origin (or find that there are\nmultiple origins), and use the actual list of commits to pass to\nshortlog.\n\nThis makes all cover letter include shortlogs, and all cover letters\nfor series with a single boundary commit include diffstats (if there\nare multiple boundary commits it's unclear what would be meaningful as\na diffstat). Note that the single boundary test is empirical, not \ntheoretical; even a -2 limiting condition will give a diffstat if there's \nonly one boundary commit in this particular case.\n\nSigned-off-by: Daniel Barkalow \n---\n builtin-log.c | 49 ++++++++++++++++++++++++++-----------------------\n 1 files changed, 26 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 2b70aa9..3209ea5 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -16,6 +16,7 @@\n #include \"patch-ids.h\"\n #include \"refs.h\"\n #include \"run-command.h\"\n+#include \"shortlog.h\"\n \n static int default_show_root = 1;\n static const char *fmt_patch_subject_prefix = \"PATCH\";\n@@ -640,9 +641,10 @@ static void gen_message_id(struct rev_info *info, char *base)\n \tinfo->message_id = strbuf_detach(&buf, NULL);\n }\n \n-static void make_cover_letter(struct rev_info *rev,\n-\t\tint use_stdout, int numbered, int numbered_files,\n-\t\t\t struct commit *origin, struct commit *head)\n+static void make_cover_letter(struct rev_info *rev, int use_stdout,\n+\t\t\t int numbered, int numbered_files,\n+\t\t\t struct commit *origin,\n+\t\t\t int nr, struct commit **list, struct commit *head)\n {\n \tconst char *committer;\n \tconst char *origin_sha1, *head_sha1;\n@@ -651,7 +653,9 @@ static void make_cover_letter(struct rev_info *rev,\n \tconst char *body = \"*** SUBJECT HERE ***\\n\\n*** BLURB HERE ***\\n\";\n \tconst char *msg;\n \tconst char *extra_headers = rev->extra_headers;\n+\tstruct shortlog log;\n \tstruct strbuf sb;\n+\tint i;\n \tconst char *encoding = \"utf-8\";\n \n \tif (rev->commit_format != CMIT_FMT_EMAIL)\n@@ -661,7 +665,6 @@ static void make_cover_letter(struct rev_info *rev,\n \t\t\t\tNULL : \"cover-letter\", 0, rev->total))\n \t\treturn;\n \n-\torigin_sha1 = sha1_to_hex(origin ? origin->object.sha1 : null_sha1);\n \thead_sha1 = sha1_to_hex(head->object.sha1);\n \n \tlog_write_email_headers(rev, head_sha1, &subject_start, &extra_headers);\n@@ -679,21 +682,19 @@ static void make_cover_letter(struct rev_info *rev,\n \n \tstrbuf_release(&sb);\n \n+\tshortlog_init(&log);\n+\tfor (i = 0; i < nr; i++)\n+\t\tshortlog_add_commit(&log, list[i]);\n+\n+\tshortlog_output(&log);\n+\n \t/*\n-\t * We can only do diffstat with a unique reference point, and\n-\t * log is a bit tricky, so just skip it.\n+\t * We can only do diffstat with a unique reference point\n \t */\n \tif (!origin)\n \t\treturn;\n \n-\targv[0] = \"shortlog\";\n-\targv[1] = head_sha1;\n-\targv[2] = \"--not\";\n-\targv[3] = origin_sha1;\n-\targv[4] = \"--\";\n-\targv[5] = NULL;\n-\tfflush(stdout);\n-\trun_command_v_opt(argv, RUN_GIT_CMD);\n+\torigin_sha1 = sha1_to_hex(origin->object.sha1);\n \n \targv[0] = \"diff\";\n \targv[1] = \"--stat\";\n@@ -746,6 +747,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tint ignore_if_in_upstream = 0;\n \tint thread = 0;\n \tint cover_letter = 0;\n+\tint boundary_count = 0;\n \tstruct commit *origin = NULL, *head = NULL;\n \tconst char *in_reply_to = NULL;\n \tstruct patch_ids ids;\n@@ -936,19 +938,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t}\n \tif (cover_letter) {\n \t\t/* remember the range */\n-\t\tint negative_count = 0;\n \t\tint i;\n \t\tfor (i = 0; i < rev.pending.nr; i++) {\n \t\t\tstruct object *o = rev.pending.objects[i].item;\n-\t\t\tif (o->flags & UNINTERESTING) {\n-\t\t\t\torigin = (struct commit *)o;\n-\t\t\t\tnegative_count++;\n-\t\t\t} else\n+\t\t\tif (!(o->flags & UNINTERESTING))\n \t\t\t\thead = (struct commit *)o;\n \t\t}\n-\t\t/* Multiple origins don't work for diffstat. */\n-\t\tif (negative_count > 1)\n-\t\t\torigin = NULL;\n \t\t/* We can't generate a cover letter without any patches */\n \t\tif (!head)\n \t\t\treturn 0;\n@@ -962,7 +957,15 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \n \tif (prepare_revision_walk(&rev))\n \t\tdie(\"revision walk setup failed\");\n+\trev.boundary = 1;\n \twhile ((commit = get_revision(&rev)) != NULL) {\n+\t\tif (commit->object.flags & BOUNDARY) {\n+\t\t\tfprintf(stderr, \"Boundary %s\\n\", sha1_to_hex(commit->object.sha1));\n+\t\t\tboundary_count++;\n+\t\t\torigin = (boundary_count == 1) ? commit : NULL;\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\t/* ignore merges */\n \t\tif (commit->parents && commit->parents->next)\n \t\t\tcontinue;\n@@ -986,7 +989,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (thread)\n \t\t\tgen_message_id(&rev, \"cover\");\n \t\tmake_cover_letter(&rev, use_stdout, numbered, numbered_files,\n-\t\t\t\t origin, head);\n+\t\t\t\t origin, nr, list, head);\n \t\ttotal++;\n \t\tstart_number--;\n \t}\n-- \n1.5.4.3.330.g53ab.dirty\n"},{"id":"69950","messageId":"7v1w70h96u.fsf@gitster.siamese.dyndns.org","threadId":"12320","inReplyTo":"alpine.LNX.1.00.0802251822240.19024@iabervon.org","subject":"Re: [PATCH 2/2] Improve collection of information for format-patch --cover-letter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-26T00:07:21Z","receivedAt":"2008-02-26T00:07:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> ... Note that the single boundary test is empirical, not \n> theoretical; even a -2 limiting condition will give a diffstat if there's \n> only one boundary commit in this particular case.\n\nIt is unclear what \"a -2 limiting condition\" means here...\n\n> @@ -679,21 +682,19 @@ static void make_cover_letter(struct rev_info *rev,\n> \n> \tstrbuf_release(&sb);\n> \n> +\tshortlog_init(&log);\n> +\tfor (i = 0; i < nr; i++)\n> +\t\tshortlog_add_commit(&log, list[i]);\n> +\n> +\tshortlog_output(&log);\n> +\n\nNice ;-)\n\n> @@ -962,7 +957,15 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \n> \tif (prepare_revision_walk(&rev))\n> \t\tdie(\"revision walk setup failed\");\n> +\trev.boundary = 1;\n> \twhile ((commit = get_revision(&rev)) != NULL) {\n> +\t\tif (commit->object.flags & BOUNDARY) {\n> +\t\t\tfprintf(stderr, \"Boundary %s\\n\", sha1_to_hex(commit->object.sha1));\n\nLeftover debugging?\n"},{"id":"69966","messageId":"7v4pbw847y.fsf@gitster.siamese.dyndns.org","threadId":"12320","inReplyTo":"alpine.LNX.1.00.0802251822240.19024@iabervon.org","subject":"Re: [PATCH 2/2] Improve collection of information for format-patch --cover-letter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-26T09:19:29Z","receivedAt":"2008-02-26T09:19:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This passes all the tests fine isolated, but when merged to\n'next' or 'pu' the result seem to barf at test #105 in t4013,\ncomplaining that it does not know about --cover-letter.\n\nSorry, but I've run out of time tonight, so 'pu' tonight has\nthis at the tip.\n"},{"id":"69970","messageId":"7voda46pez.fsf@gitster.siamese.dyndns.org","threadId":"12320","inReplyTo":"7v4pbw847y.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] Improve collection of information for format-patch --cover-letter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-26T09:24:36Z","receivedAt":"2008-02-26T09:24:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> This passes all the tests fine isolated, but when merged to\n> 'next' or 'pu' the result seem to barf at test #105 in t4013,\n> complaining that it does not know about --cover-letter.\n>\n> Sorry, but I've run out of time tonight, so 'pu' tonight has\n> this at the tip.\n\nFalse alarm. It seems to be Ok.\n"},{"id":"70005","messageId":"alpine.LNX.1.00.0802261123170.19024@iabervon.org","threadId":"12320","inReplyTo":"7v1w70h96u.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] Improve collection of information for format-patch --cover-letter","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-26T16:27:20Z","receivedAt":"2008-02-26T16:27:20Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 25 Feb 2008, Junio C Hamano wrote:\n\n> Daniel Barkalow writes:\n> \n> > ... Note that the single boundary test is empirical, not \n> > theoretical; even a -2 limiting condition will give a diffstat if there's \n> > only one boundary commit in this particular case.\n> \n> It is unclear what \"a -2 limiting condition\" means here...\n\nLike \"git format-patch -2\". What is that actually called?\n\n> > @@ -962,7 +957,15 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> > \n> > \tif (prepare_revision_walk(&rev))\n> > \t\tdie(\"revision walk setup failed\");\n> > +\trev.boundary = 1;\n> > \twhile ((commit = get_revision(&rev)) != NULL) {\n> > +\t\tif (commit->object.flags & BOUNDARY) {\n> > +\t\t\tfprintf(stderr, \"Boundary %s\\n\", sha1_to_hex(commit->object.sha1));\n> \n> Leftover debugging?\n\nThat last line, yes. I need a checkpatch for new uses of stderr or \nsomething.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70015","messageId":"7vzltn4kti.fsf@gitster.siamese.dyndns.org","threadId":"12320","inReplyTo":"alpine.LNX.1.00.0802261123170.19024@iabervon.org","subject":"Re: [PATCH 2/2] Improve collection of information for format-patch --cover-letter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-26T18:46:49Z","receivedAt":"2008-02-26T18:46:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> On Mon, 25 Feb 2008, Junio C Hamano wrote:\n>\n>> Daniel Barkalow writes:\n>> \n>> > ... Note that the single boundary test is empirical, not \n>> > theoretical; even a -2 limiting condition will give a diffstat if there's \n>> > only one boundary commit in this particular case.\n>> \n>> It is unclear what \"a -2 limiting condition\" means here...\n>\n> Like \"git format-patch -2\". What is that actually called?\n\n\"max count\"? So you wanted to say that the new code sometimes\nfigures out (with --boundary) what to compare against even when\nthe user does not explicitly specify where the range begins from\nthe command line. Thanks for a clarification.\n"},{"id":"70214","messageId":"7v8x15li2g.fsf_-_@gitster.siamese.dyndns.org","threadId":"12320","inReplyTo":"7v1w70h96u.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] format-patch: remove a leftover debugging message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-28T06:18:47Z","receivedAt":"2008-02-28T06:18:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sorry, Daniel ;-)\n\nSigned-off-by: Junio C Hamano \n---\n builtin-log.c | 1 -\n 1 files changed, 0 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 3209ea5..836b61e 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -960,7 +960,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \trev.boundary = 1;\n \twhile ((commit = get_revision(&rev)) != NULL) {\n \t\tif (commit->object.flags & BOUNDARY) {\n-\t\t\tfprintf(stderr, \"Boundary %s\\n\", sha1_to_hex(commit->object.sha1));\n \t\t\tboundary_count++;\n \t\t\torigin = (boundary_count == 1) ? commit : NULL;\n \t\t\tcontinue;\n-- \n1.5.4.3.393.g55409\n"},{"id":"70263","messageId":"alpine.LNX.1.00.0802281014180.19665@iabervon.org","threadId":"12320","inReplyTo":"7v8x15li2g.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH] format-patch: remove a leftover debugging message","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-28T15:20:59Z","receivedAt":"2008-02-28T15:20:59Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 27 Feb 2008, Junio C Hamano wrote:\n\n> Sorry, Daniel ;-)\n> \n> Signed-off-by: Junio C Hamano \n\nI wrote this patch, added it to that topic locally, and then forgot I \nhadn't sent it. (But my version had the advantage of mentioning which \ncommit it should be squashed into when going to master.) Then I was \nconfused to see it in my incoming email instead of in my outgoing email \n(Why are you forwarding me my own patch? Oh, because I didn't send it.)\n\nAcked-by: Daniel Barkalow \n"}]}