{"thread":{"id":"29832","subject":"A possible fmt-merge-msg update?","startedAt":"2012-03-05T03:17:40Z","lastAt":"2012-06-06T21:11:06Z","messageCount":36,"participants":["Junio C Hamano","Linus Torvalds","Jeff King","René Scharfe","Jonathan Nieder","Phil Hord","Johannes Sixt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"186067","messageId":"7vvcmj68iz.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":null,"subject":"A possible fmt-merge-msg update?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-05T03:17:40Z","receivedAt":"2012-03-05T03:17:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Subject: Re: Merge git://www.linux-watchdog.org/linux-watchdog\nFrom: Junio C Hamano \nBcc: junio@pobox.com\n--text follows this line--\nLinus Torvalds writes:\n\n> Watchdog updates from Wim Van Sebroeck:\n>\n> * git://www.linux-watchdog.org/linux-watchdog:\n> watchdog: fix GETTIMEOUT ioctl in booke_wdt\n> watchdog: update maintainers git entry\n> watchdog: Fix typo in pnx4008_wdt.c\n> watchdog: Fix typo in Kconfig\n> watchdog: fix error in probe() of s3c2410_wdt (reset at booting)\n> watchdog: hpwdt: clean up set_memory_x call for 32 bit\n\nHaving observed a handful of your recent merge messages, I am wondering if\nit would help to teach fmt-merge-msg to include \"from Wim Van Sebroeck\" in\nits output by taking the committer of the MERGE_HEAD into account.\n\nNot worth the trouble?\n"},{"id":"186083","messageId":"CA+55aFzQ3+EFBVyE9PWOyH0XEC_oW+kUaBTYfixmi2wgMmiZvw@mail.gmail.com","threadId":"29832","inReplyTo":"7vvcmj68iz.fsf@alter.siamese.dyndns.org","subject":"Re: A possible fmt-merge-msg update?","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-03-05T05:24:41Z","receivedAt":"2012-03-05T05:24:41Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Sun, Mar 4, 2012 at 7:17 PM, Junio C Hamano wrote:\n>\n> Having observed a handful of your recent merge messages, I am wondering if\n> it would help to teach fmt-merge-msg to include \"from Wim Van Sebroeck\" in\n> its output by taking the committer of the MERGE_HEAD into account.\n>\n> Not worth the trouble?\n\nHmm. Maybe worth it.\n\nSo the reason I've started to try to do it is that I had a person\nemail me about appreciating the new merge messages, but he also said\nhe wanted to know who I pulled from. And I agree that that makes sense\nas a real piece of information.\n\nIt's not *necessarily* the same person as the committer info, though.\nIt probably matches a very high percentage of the time, but the pull\nrequest doesn't always come from the person doing the commits. The x86\npeople have this \"tip\" thing, and while branches tend to be owned by\nthe people doing the pull request, it's not a given. And sometimes you\nhave a submaintainer who pulled from *his* submaintainer, and didn't\nhave any work of his own, so he asks me to pull something where the\ntop is not him, but his submaintainer.\n\nSo I really dunno.\n\nIt might be interesting if the pre-written commit message had the top\ncommitter in a comment (the same way pulling a tag has the tag author\nin the comment about the tag verification). That way the information\nwould be right there when I edit the message, and since it's correct\n99% of the time it would make it easier to just edit it in the editor\nthan have to cut-and-paste it from the email. But because it's not a\nsure thing, it would be a comment-only thing where I can choose to\nremove the '# ' in front of it. Maybe that would be the best of both\nworlds - helping write a merge message without assuming it has to be\nthat way..\n\n Linus\n"},{"id":"186132","messageId":"7vipii27ka.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"CA+55aFzQ3+EFBVyE9PWOyH0XEC_oW+kUaBTYfixmi2wgMmiZvw@mail.gmail.com","subject":"Re: A possible fmt-merge-msg update?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-05T19:04:21Z","receivedAt":"2012-03-05T19:04:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> On Sun, Mar 4, 2012 at 7:17 PM, Junio C Hamano wrote:\n>>\n>> Having observed a handful of your recent merge messages, I am wondering if\n>> it would help to teach fmt-merge-msg to include \"from Wim Van Sebroeck\" in\n>> its output by taking the committer of the MERGE_HEAD into account.\n>>\n>> Not worth the trouble?\n>\n> Hmm. Maybe worth it.\n> ...\n> It might be interesting if the pre-written commit message had the top\n> committer in a comment (the same way pulling a tag has the tag author\n> in the comment about the tag verification). That way the information\n> would be right there when I edit the message, and since it's correct\n> 99% of the time it would make it easier to just edit it in the editor\n> than have to cut-and-paste it from the email. But because it's not a\n> sure thing,...\n\nThe attached would give me:\n\n| Merge branch 'jl/maint-submodule-relative'\n| \n| # Jens Lehmann (3) and Johannes Sixt (1)\n| * jl/maint-submodule-relative:\n| submodules: fix ambiguous absolute paths under Windows\n| submodules: refactor computation of relative gitdir path\n| submodules: always use a relative path from gitdir to work tree\n| submodules: always use a relative path to gitdir\n\nIt would be a sure thing for the commit-authorship, so we could use\n\"By \" instead of \"# \" above, but it should be obvious either way.\n\nThe existing test vectors need to be taught about this change if we\nwere to do something like this.\n\n-- >8 --\nSubject: [PATCH] fmt-merge-msg: show primary authors of a merged series\n\nAs we already walk the history of the branch that gets merged to\ncome up with a short log, let's label it with names of the primary\nauthors, so that the user who summarizes the merge can easily give\ncredit to them in the log message.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/fmt-merge-msg.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 61 insertions(+)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex c81a7fe..7eea066 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -180,6 +180,63 @@ static void add_branch_desc(struct strbuf *out, const char *name)\n \tstrbuf_release(&desc);\n }\n \n+static void record_author(struct string_list *authors, struct commit *commit)\n+{\n+\tchar name_buf[MAX_GITNAME], *name, *name_end;\n+\tstruct string_list_item *elem;\n+\n+\tname = strstr(commit->buffer, \"\\nauthor \");\n+\tif (!name)\n+\t\treturn;\n+\tname += strlen(\"\\nauthor \");\n+\tname_end = strchrnul(name, '<');\n+\tif (*name_end)\n+\t\tname_end--;\n+\twhile (isspace(*name_end) && name <= name_end)\n+\t\tname_end--;\n+\tif (name_end < name || name + MAX_GITNAME <= name_end)\n+\t\treturn;\n+\tmemcpy(name_buf, name, name_end - name + 1);\n+\tname_buf[name_end - name + 1] = '\\0';\n+\n+\telem = string_list_lookup(authors, name_buf);\n+\tif (!elem) {\n+\t\telem = string_list_insert(authors, name_buf);\n+\t\telem->util = (void *) 0;\n+\t}\n+\telem->util = (void*)(((intptr_t)elem->util) + 1);\n+}\n+\n+#define util_as_int(elem) ((intptr_t)((elem)->util))\n+\n+static int cmp_string_list_util_as_int(const void *a_, const void *b_)\n+{\n+\tconst struct string_list_item *a = a_, *b = b_;\n+\treturn util_as_int(b) - util_as_int(a);\n+}\n+\n+static void add_author_info(struct strbuf *out, struct string_list *authors)\n+{\n+\tif (!authors->nr)\n+\t\treturn;\n+\tqsort(authors->items, authors->nr, sizeof(authors->items[0]),\n+\t cmp_string_list_util_as_int);\n+\n+\tstrbuf_addstr(out, \"\\n# \");\n+\tif (authors->nr == 1)\n+\t\tstrbuf_addf(out, \"%s\", authors->items[0].string);\n+\telse if (authors->nr == 2)\n+\t\tstrbuf_addf(out, \"%s (%d) and %s (%d)\",\n+\t\t\t authors->items[0].string,\n+\t\t\t (int)util_as_int(&authors->items[0]),\n+\t\t\t authors->items[1].string,\n+\t\t\t (int)util_as_int(&authors->items[1]));\n+\telse\n+\t\tstrbuf_addf(out, \"%s (%d) and others\",\n+\t\t\t authors->items[0].string,\n+\t\t\t (int)util_as_int(&authors->items[0]));\n+}\n+\n static void shortlog(const char *name,\n \t\t struct origin_data *origin_data,\n \t\t struct commit *head,\n@@ -190,6 +247,7 @@ static void shortlog(const char *name,\n \tstruct commit *commit;\n \tstruct object *branch;\n \tstruct string_list subjects = STRING_LIST_INIT_DUP;\n+\tstruct string_list authors = STRING_LIST_INIT_DUP;\n \tint flags = UNINTERESTING | TREESAME | SEEN | SHOWN | ADDED;\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst unsigned char *sha1 = origin_data->sha1;\n@@ -212,6 +270,7 @@ static void shortlog(const char *name,\n \t\tif (commit->parents && commit->parents->next)\n \t\t\tcontinue;\n \n+\t\trecord_author(&authors, commit);\n \t\tcount++;\n \t\tif (subjects.nr > limit)\n \t\t\tcontinue;\n@@ -226,6 +285,7 @@ static void shortlog(const char *name,\n \t\t\tstring_list_append(&subjects, strbuf_detach(&sb, NULL));\n \t}\n \n+\tadd_author_info(out, &authors);\n \tif (count > limit)\n \t\tstrbuf_addf(out, \"\\n* %s: (%d commits)\\n\", name, count);\n \telse\n@@ -246,6 +306,7 @@ static void shortlog(const char *name,\n \trev->commits = NULL;\n \trev->pending.nr = 0;\n \n+\tstring_list_clear(&authors, 0);\n \tstring_list_clear(&subjects, 0);\n }\n \n-- \n1.7.9.2.413.ge58a8e\n"},{"id":"186140","messageId":"CA+55aFw-sS_p7JXNXbSbpiwh9_bZhSrTtC3is4NtLa_n9Hzk5A@mail.gmail.com","threadId":"29832","inReplyTo":"7vipii27ka.fsf@alter.siamese.dyndns.org","subject":"Re: A possible fmt-merge-msg update?","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-03-05T20:33:42Z","receivedAt":"2012-03-05T20:33:42Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Mon, Mar 5, 2012 at 11:04 AM, Junio C Hamano wrote:\n>\n> The attached would give me:\n\nSo this isn't interesting to me.\n\nAuthorship is less relevant than submaintainership. So I'm more\ninterested in *committer* information than authorship information.\n\nOf course, since you do it in branches that you maintain, to you\ncommitter information is pointless. But I pull from submaintainers,\nand then it really is the committer part that is way more relevant.\n\n Linus\n"},{"id":"186143","messageId":"7vmx7uzq8h.fsf_-_@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"CA+55aFw-sS_p7JXNXbSbpiwh9_bZhSrTtC3is4NtLa_n9Hzk5A@mail.gmail.com","subject":"[PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-05T21:34:38Z","receivedAt":"2012-03-05T21:34:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Of course, since you do it in branches that you maintain, to you\n> committer information is pointless. But I pull from submaintainers,\n> and then it really is the committer part that is way more relevant.\n\nFair enough.\n\nI do not think a simple population count on the committer field is a\ngood heuristics. For example, I suspect that you would want to see\na lot more weight given to names of committers near the tip of the\nhistory you are pulling from, iow, if your lieutenant pulled 100\ncommit series by a leaf contributor, you still want to see the name\nof the lieutenant, not the leaf contributor, even the latter may\nhave far more commits.\n\nThis patch punts the deep thinking part and just picks the tip\ncommitter and counts only merges in the history you are pulling in.\nIt may not be a very good heuristics, but the code structure should\nbe a good place to start.\n\nChanges to the test vector in t6200 illustrate how the output should\nlook like.\n\n-- >8 --\nAs we already walk the history of the branch that gets merged to\ncome up with a short log, let's label it with names of the primary\nauthors, so that the user who summarizes the merge can easily give\ncredit to them in the log message.\n\nAlso infer the names of \"lieutents\" to help integrators at higher\nlevel of the food-chain to give credit to them, by counting:\n\n * The committer of the 'tip' commit that is merged\n * The committer of merge commits that are merged\n\nOften the first one gives the owner of the history being pulled, but\nhis last pull from his sublieutenants may have been a fast-forward,\nin which case the first one would not be. The latter rule will\ncount the integrator of the history, so together it might be a\nreasonable heuristics.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/fmt-merge-msg.c | 101 +++++++++++++++++++++++++++++++++++++--\n t/t6200-fmt-merge-msg.sh | 32 +++++++++++--\n t/t7600-merge.sh | 1 +\n t/t7604-merge-custom-message.sh | 1 +\n 4 files changed, 128 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex c81a7fe..b2465b4 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -27,6 +27,8 @@ int fmt_merge_msg_config(const char *key, const char *value, void *cb)\n \t\t\tmerge_log_config = DEFAULT_MERGE_LOG_LEN;\n \t} else if (!strcmp(key, \"merge.branchdesc\")) {\n \t\tuse_branch_desc = git_config_bool(key, value);\n+\t} else {\n+\t\treturn git_default_config(key, value, cb);\n \t}\n \treturn 0;\n }\n@@ -180,6 +182,89 @@ static void add_branch_desc(struct strbuf *out, const char *name)\n \tstrbuf_release(&desc);\n }\n \n+static void record_person(int which, struct string_list *people,\n+\t\t\t struct commit *commit)\n+{\n+\tchar name_buf[MAX_GITNAME], *name, *name_end;\n+\tstruct string_list_item *elem;\n+\tconst char *field = (which == 'a') ? \"\\nauthor \" : \"\\ncommitter \";\n+\n+\tname = strstr(commit->buffer, field);\n+\tif (!name)\n+\t\treturn;\n+\tname += strlen(field);\n+\tname_end = strchrnul(name, '<');\n+\tif (*name_end)\n+\t\tname_end--;\n+\twhile (isspace(*name_end) && name <= name_end)\n+\t\tname_end--;\n+\tif (name_end < name || name + MAX_GITNAME <= name_end)\n+\t\treturn;\n+\tmemcpy(name_buf, name, name_end - name + 1);\n+\tname_buf[name_end - name + 1] = '\\0';\n+\n+\telem = string_list_lookup(people, name_buf);\n+\tif (!elem) {\n+\t\telem = string_list_insert(people, name_buf);\n+\t\telem->util = (void *) 0;\n+\t}\n+\telem->util = (void*)(((intptr_t)elem->util) + 1);\n+}\n+\n+#define util_as_int(elem) ((intptr_t)((elem)->util))\n+\n+static int cmp_string_list_util_as_int(const void *a_, const void *b_)\n+{\n+\tconst struct string_list_item *a = a_, *b = b_;\n+\treturn util_as_int(b) - util_as_int(a);\n+}\n+\n+static void add_people_count(struct strbuf *out, struct string_list *people)\n+{\n+\tif (people->nr == 1)\n+\t\tstrbuf_addf(out, \"%s\", people->items[0].string);\n+\telse if (people->nr == 2)\n+\t\tstrbuf_addf(out, \"%s (%d) and %s (%d)\",\n+\t\t\t people->items[0].string,\n+\t\t\t (int)util_as_int(&people->items[0]),\n+\t\t\t people->items[1].string,\n+\t\t\t (int)util_as_int(&people->items[1]));\n+\telse if (people->nr)\n+\t\tstrbuf_addf(out, \"%s (%d) and others\",\n+\t\t\t people->items[0].string,\n+\t\t\t (int)util_as_int(&people->items[0]));\n+}\n+\n+static int committer_is_me(const char *name)\n+{\n+\tint namelen = strlen(name);\n+\tconst char *me = git_committer_info(IDENT_NO_DATE);\n+\treturn (me && !memcmp(me, name, namelen) &&\n+\t\t!memcmp(me + namelen, \" <\", 2));\n+}\n+\n+static void add_people_info(struct strbuf *out,\n+\t\t\t struct string_list *authors,\n+\t\t\t struct string_list *committers)\n+{\n+\tif (authors->nr)\n+\t\tqsort(authors->items,\n+\t\t authors->nr, sizeof(authors->items[0]),\n+\t\t cmp_string_list_util_as_int);\n+\tif (committers->nr)\n+\t\tqsort(committers->items,\n+\t\t committers->nr, sizeof(committers->items[0]),\n+\t\t cmp_string_list_util_as_int);\n+\n+\tstrbuf_addstr(out, \"\\nBy \");\n+\tadd_people_count(out, authors);\n+\tif (committers->nr == 1 &&\n+\t committer_is_me(committers->items->string))\n+\t\treturn;\n+\tstrbuf_addstr(out, \"\\nvia \");\n+\tadd_people_count(out, committers);\n+}\n+\n static void shortlog(const char *name,\n \t\t struct origin_data *origin_data,\n \t\t struct commit *head,\n@@ -190,6 +275,8 @@ static void shortlog(const char *name,\n \tstruct commit *commit;\n \tstruct object *branch;\n \tstruct string_list subjects = STRING_LIST_INIT_DUP;\n+\tstruct string_list authors = STRING_LIST_INIT_DUP;\n+\tstruct string_list committers = STRING_LIST_INIT_DUP;\n \tint flags = UNINTERESTING | TREESAME | SEEN | SHOWN | ADDED;\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst unsigned char *sha1 = origin_data->sha1;\n@@ -199,7 +286,6 @@ static void shortlog(const char *name,\n \t\treturn;\n \n \tsetup_revisions(0, NULL, rev, NULL);\n-\trev->ignore_merges = 1;\n \tadd_pending_object(rev, branch, name);\n \tadd_pending_object(rev, &head->object, \"^HEAD\");\n \thead->object.flags |= UNINTERESTING;\n@@ -208,10 +294,15 @@ static void shortlog(const char *name,\n \twhile ((commit = get_revision(rev)) != NULL) {\n \t\tstruct pretty_print_context ctx = {0};\n \n-\t\t/* ignore merges */\n-\t\tif (commit->parents && commit->parents->next)\n+\t\tif (commit->parents && commit->parents->next) {\n+\t\t\t/* do not list a merge but count committer */\n+\t\t\trecord_person('c', &committers, commit);\n \t\t\tcontinue;\n-\n+\t\t}\n+\t\tif (!count)\n+\t\t\t/* the 'tip' committer */\n+\t\t\trecord_person('c', &committers, commit);\n+\t\trecord_person('a', &authors, commit);\n \t\tcount++;\n \t\tif (subjects.nr > limit)\n \t\t\tcontinue;\n@@ -226,6 +317,7 @@ static void shortlog(const char *name,\n \t\t\tstring_list_append(&subjects, strbuf_detach(&sb, NULL));\n \t}\n \n+\tadd_people_info(out, &authors, &committers);\n \tif (count > limit)\n \t\tstrbuf_addf(out, \"\\n* %s: (%d commits)\\n\", name, count);\n \telse\n@@ -246,6 +338,7 @@ static void shortlog(const char *name,\n \trev->commits = NULL;\n \trev->pending.nr = 0;\n \n+\tstring_list_clear(&authors, 0);\n \tstring_list_clear(&subjects, 0);\n }\n \ndiff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh\nindex 9a16806..b90015d 100755\n--- a/t/t6200-fmt-merge-msg.sh\n+++ b/t/t6200-fmt-merge-msg.sh\n@@ -35,15 +35,18 @@ test_expect_success setup '\n \n \techo \"l3\" >two &&\n \ttest_tick &&\n-\tgit commit -a -m \"Left #3\" &&\n+\tGIT_COMMITTER_NAME=\"Another Committer\" \\\n+\tGIT_AUTHOR_NAME=\"Another Author\" git commit -a -m \"Left #3\" &&\n \n \techo \"l4\" >two &&\n \ttest_tick &&\n-\tgit commit -a -m \"Left #4\" &&\n+\tGIT_COMMITTER_NAME=\"Another Committer\" \\\n+\tGIT_AUTHOR_NAME=\"Another Author\" git commit -a -m \"Left #4\" &&\n \n \techo \"l5\" >two &&\n \ttest_tick &&\n-\tgit commit -a -m \"Left #5\" &&\n+\tGIT_COMMITTER_NAME=\"Another Committer\" \\\n+\tGIT_AUTHOR_NAME=\"Another Author\" git commit -a -m \"Left #5\" &&\n \tgit tag tag-l5 &&\n \n \tgit checkout right &&\n@@ -99,6 +102,8 @@ test_expect_success '[merge] summary/log configuration' '\n \tcat >expected <<-EOF &&\n \tMerge branch ${apos}left${apos}\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* left:\n \t Left #5\n \t Left #4\n@@ -144,6 +149,8 @@ test_expect_success 'merge.log=3 limits shortlog length' '\n \tcat >expected <<-EOF &&\n \tMerge branch ${apos}left${apos}\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* left: (5 commits)\n \t Left #5\n \t Left #4\n@@ -159,6 +166,8 @@ test_expect_success 'merge.log=5 shows all 5 commits' '\n \tcat >expected <<-EOF &&\n \tMerge branch ${apos}left${apos}\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* left:\n \t Left #5\n \t Left #4\n@@ -181,6 +190,8 @@ test_expect_success '--log=3 limits shortlog length' '\n \tcat >expected <<-EOF &&\n \tMerge branch ${apos}left${apos}\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* left: (5 commits)\n \t Left #5\n \t Left #4\n@@ -196,6 +207,8 @@ test_expect_success '--log=5 shows all 5 commits' '\n \tcat >expected <<-EOF &&\n \tMerge branch ${apos}left${apos}\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* left:\n \t Left #5\n \t Left #4\n@@ -225,6 +238,8 @@ test_expect_success 'fmt-merge-msg -m' '\n \tcat >expected.log <<-EOF &&\n \tSync with left\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* ${apos}left${apos} of $(pwd):\n \t Left #5\n \t Left #4\n@@ -256,6 +271,8 @@ test_expect_success 'setup: expected shortlog for two branches' '\n \tcat >expected <<-EOF\n \tMerge branches ${apos}left${apos} and ${apos}right${apos}\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* left:\n \t Left #5\n \t Left #4\n@@ -263,6 +280,7 @@ test_expect_success 'setup: expected shortlog for two branches' '\n \t Common #2\n \t Common #1\n \n+\tBy A U Thor\n \t* right:\n \t Right #5\n \t Right #4\n@@ -353,6 +371,7 @@ test_expect_success 'merge-msg tag' '\n \tcat >expected <<-EOF &&\n \tMerge tag ${apos}tag-r3${apos}\n \n+\tBy A U Thor\n \t* tag ${apos}tag-r3${apos}:\n \t Right #3\n \t Common #2\n@@ -374,11 +393,14 @@ test_expect_success 'merge-msg two tags' '\n \tcat >expected <<-EOF &&\n \tMerge tags ${apos}tag-r3${apos} and ${apos}tag-l5${apos}\n \n+\tBy A U Thor\n \t* tag ${apos}tag-r3${apos}:\n \t Right #3\n \t Common #2\n \t Common #1\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* tag ${apos}tag-l5${apos}:\n \t Left #5\n \t Left #4\n@@ -402,11 +424,14 @@ test_expect_success 'merge-msg tag and branch' '\n \tcat >expected <<-EOF &&\n \tMerge branch ${apos}left${apos}, tag ${apos}tag-r3${apos}\n \n+\tBy A U Thor\n \t* tag ${apos}tag-r3${apos}:\n \t Right #3\n \t Common #2\n \t Common #1\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* left:\n \t Left #5\n \t Left #4\n@@ -431,6 +456,7 @@ test_expect_success 'merge-msg lots of commits' '\n \t\tcat <<-EOF &&\n \t\tMerge branch ${apos}long${apos}\n \n+\t\tBy A U Thor\n \t\t* long: (35 commits)\n \t\tEOF\n \ndiff --git a/t/t7600-merge.sh b/t/t7600-merge.sh\nindex 9e27bbf..e09b932 100755\n--- a/t/t7600-merge.sh\n+++ b/t/t7600-merge.sh\n@@ -58,6 +58,7 @@ create_merge_msgs () {\n \t} >squash.1-5-9 &&\n \techo >msg.nolog &&\n \t{\n+\t\techo \"By A U Thor\" &&\n \t\techo \"* tag 'c3':\" &&\n \t\techo \" commit 3\" &&\n \t\techo\ndiff --git a/t/t7604-merge-custom-message.sh b/t/t7604-merge-custom-message.sh\nindex 89619cf..b84cea3 100755\n--- a/t/t7604-merge-custom-message.sh\n+++ b/t/t7604-merge-custom-message.sh\n@@ -11,6 +11,7 @@ create_merge_msgs() {\n \n \tcp exp.subject exp.log &&\n \techo >>exp.log \"\" &&\n+\techo >>exp.log \"By A U Thor\" &&\n \techo >>exp.log \"* tag 'c2':\" &&\n \techo >>exp.log \" c2\"\n }\n-- \n1.7.9.2.413.ge58a8e\n"},{"id":"186145","messageId":"CA+55aFzuCJWfzp2ROxvMY78NZWb3ofYWOebfPfPba9kypyytKA@mail.gmail.com","threadId":"29832","inReplyTo":"7vmx7uzq8h.fsf_-_@alter.siamese.dyndns.org","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-03-05T21:46:58Z","receivedAt":"2012-03-05T21:46:58Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Mon, Mar 5, 2012 at 1:34 PM, Junio C Hamano wrote:\n>\n> This patch punts the deep thinking part and just picks the tip\n> committer and counts only merges in the history you are pulling in.\n> It may not be a very good heuristics, but the code structure should\n> be a good place to start.\n\nYeah, this looks good to me (but I didn't actually *test* it, so\nthat's just from looking at the patch).\n\n Linus\n"},{"id":"186146","messageId":"7veht6zpjv.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"CA+55aFzuCJWfzp2ROxvMY78NZWb3ofYWOebfPfPba9kypyytKA@mail.gmail.com","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-05T21:49:24Z","receivedAt":"2012-03-05T21:49:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> On Mon, Mar 5, 2012 at 1:34 PM, Junio C Hamano wrote:\n>>\n>> This patch punts the deep thinking part and just picks the tip\n>> committer and counts only merges in the history you are pulling in.\n>> It may not be a very good heuristics, but the code structure should\n>> be a good place to start.\n>\n> Yeah, this looks good to me (but I didn't actually *test* it, so\n> that's just from looking at the patch).\n\nAs I said, I didn't actually *test* it, either, and the committer\nbased heuristics may have a huge room for improvements.\n"},{"id":"186175","messageId":"20120306075940.GA21199@sigill.intra.peff.net","threadId":"29832","inReplyTo":"CA+55aFw-sS_p7JXNXbSbpiwh9_bZhSrTtC3is4NtLa_n9Hzk5A@mail.gmail.com","subject":"Re: A possible fmt-merge-msg update?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-06T07:59:40Z","receivedAt":"2012-03-06T07:59:40Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 05, 2012 at 12:33:42PM -0800, Linus Torvalds wrote:\n\n> On Mon, Mar 5, 2012 at 11:04 AM, Junio C Hamano wrote:\n> >\n> > The attached would give me:\n> \n> So this isn't interesting to me.\n> \n> Authorship is less relevant than submaintainership. So I'm more\n> interested in *committer* information than authorship information.\n> \n> Of course, since you do it in branches that you maintain, to you\n> committer information is pointless. But I pull from submaintainers,\n> and then it really is the committer part that is way more relevant.\n\nIf you're interested in the sub-maintainer, and the sub-maintainer is\nwho you pulled from, then isn't the right solution to better annotate\nthe source of the pull? For the kernel workflow, that often comes in the\nform of pulled tags; would providing the tagger in that case be helpful?\n(it's often already included in the commit template via the\ncommented-out GPG output, but there might be many UIDs attached to a\ngiven GPG key).\n\nThat wouldn't help the git.git workflow, of course, but I think you are\ntalking about two fundamentally different things. The kernel thing is\nabout annotating the source of the pull. The git.git thing (and Junio's\npatch) is about summarizing the contents of the branch not just with the\nsubject lines, but also with the author's names[1].\n\nBut looking through some recent kernel merges, the useful new thing in\nthe message doesn't seem to me to be the _who_, but rather the _what_.\nFor example, from f3969bf7:\n\n Pull perf fixes from Ingo Molnar:\n \"It contains three cherry-picked fixes from perf/core, which turned out\n to be more urgent than we originally thought.\"\n\nSo rather than focus on the identity of the sub-maintainer, perhaps a\nmore useful thing is to make it easier to pass information from a pull\nrequest into the resulting merge message. We already have \"git am\" for\nregular patches, and it relies on a few easy-to-generate microformats,\nso it's natural to use with \"git format-patch\", your own custom script,\nor even by hand. Could we do the same thing and have a \"git\napply-pull-request\" (or something with a less horrible name)?\n\nIngo's original message looked like:\n\n From: Ingo Molnar \n Subject: [GIT PULL] perf fixes\n\n Linus,\n\n Please pull the latest perf-urgent-for-linus git tree from:\n\n git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf-urgent-for-linus\n\n HEAD: b7c924274c456499264d1cfa3d44063bb11eb5db Merge tag 'perf-urgent-for-mingo' of\n git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent\n\n It contains three cherry-picked fixes from perf/core, which \n turned out to be more urgent than we originally thought.\n\n Thanks,\n\n Ingo\n\nIf this were instead formatted as:\n\n From: Ingo Molnar \n Subject: [GIT PULL] perf fixes\n\n Here are three cherry-picked fixes from perf/core, which turned out to\n be more urgent than we originally thought.\n\n ---\n git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf-urgent-for-linus\n\n HEAD: b7c924274c456499264d1cfa3d44063bb11eb5db Merge tag 'perf-urgent-for-mingo'\n of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent\n\nwe could trivially convert that into the same commit message you ended\nup writing. The format is simple enough that people who aren't\nrunning it through a script can read and write it, and we retain the\nsingle line with the repo and ref name for those who want to just cut\nand paste.\n\n-Peff\n"},{"id":"186338","messageId":"4F57D18B.5090506@lsrfire.ath.cx","threadId":"29832","inReplyTo":"7vmx7uzq8h.fsf_-_@alter.siamese.dyndns.org","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-03-07T21:22:19Z","receivedAt":"2012-03-07T21:22:19Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 05.03.2012 22:34, schrieb Junio C Hamano:\n> diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\n> index c81a7fe..b2465b4 100644\n> --- a/builtin/fmt-merge-msg.c\n> +++ b/builtin/fmt-merge-msg.c\n> @@ -27,6 +27,8 @@ int fmt_merge_msg_config(const char *key, const char *value, void *cb)\n> \t\t\tmerge_log_config = DEFAULT_MERGE_LOG_LEN;\n> \t} else if (!strcmp(key, \"merge.branchdesc\")) {\n> \t\tuse_branch_desc = git_config_bool(key, value);\n> +\t} else {\n> +\t\treturn git_default_config(key, value, cb);\n> \t}\n> \treturn 0;\n> }\n> @@ -180,6 +182,89 @@ static void add_branch_desc(struct strbuf *out, const char *name)\n> \tstrbuf_release(&desc);\n> }\n>\n> +static void record_person(int which, struct string_list *people,\n> +\t\t\t struct commit *commit)\n> +{\n> +\tchar name_buf[MAX_GITNAME], *name, *name_end;\n> +\tstruct string_list_item *elem;\n> +\tconst char *field = (which == 'a') ? \"\\nauthor \" : \"\\ncommitter \";\n> +\n> +\tname = strstr(commit->buffer, field);\n> +\tif (!name)\n> +\t\treturn;\n> +\tname += strlen(field);\n> +\tname_end = strchrnul(name, '<');\n> +\tif (*name_end)\n> +\t\tname_end--;\n> +\twhile (isspace(*name_end) && name <= name_end)\n> +\t\tname_end--;\n> +\tif (name_end < name || name + MAX_GITNAME <= name_end)\n> +\t\treturn;\n> +\tmemcpy(name_buf, name, name_end - name + 1);\n> +\tname_buf[name_end - name + 1] = '\\0';\n> +\n> +\telem = string_list_lookup(people, name_buf);\n> +\tif (!elem) {\n> +\t\telem = string_list_insert(people, name_buf);\n> +\t\telem->util = (void *) 0;\n> +\t}\n> +\telem->util = (void*)(((intptr_t)elem->util) + 1);\n> +}\n> +\n> +#define util_as_int(elem) ((intptr_t)((elem)->util))\n\nSomething that actually returns an int would fit the name better. ;)\n\n> +\n> +static int cmp_string_list_util_as_int(const void *a_, const void *b_)\n> +{\n> +\tconst struct string_list_item *a = a_, *b = b_;\n> +\treturn util_as_int(b) - util_as_int(a);\n> +}\n> +\n> +static void add_people_count(struct strbuf *out, struct string_list *people)\n> +{\n> +\tif (people->nr == 1)\n> +\t\tstrbuf_addf(out, \"%s\", people->items[0].string);\n> +\telse if (people->nr == 2)\n> +\t\tstrbuf_addf(out, \"%s (%d) and %s (%d)\",\n> +\t\t\t people->items[0].string,\n> +\t\t\t (int)util_as_int(&people->items[0]),\n> +\t\t\t people->items[1].string,\n> +\t\t\t (int)util_as_int(&people->items[1]));\n> +\telse if (people->nr)\n> +\t\tstrbuf_addf(out, \"%s (%d) and others\",\n> +\t\t\t people->items[0].string,\n> +\t\t\t (int)util_as_int(&people->items[0]));\n> +}\n> +\n> +static int committer_is_me(const char *name)\n> +{\n> +\tint namelen = strlen(name);\n> +\tconst char *me = git_committer_info(IDENT_NO_DATE);\n> +\treturn (me && !memcmp(me, name, namelen) &&\n> +\t\t!memcmp(me + namelen, \" <\", 2));\n> +}\n\nThis looks scary due to the missing length check of me before the \nmemcmp() call, but is actually safe because git_committer_info() returns \na pointer to a static buffer that is just as long as name can possibly \nbe. Still, perhaps this is nicer instead:\n\n\tconst char *me = git_committer_info(IDENT_NO_DATE);\n\tconst char *rest = skip_prefix(me, name);\n\treturn rest && skip_prefix(rest, \" <\");\n\nRené\n"},{"id":"186342","messageId":"7vzkbskr71.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"4F57D18B.5090506@lsrfire.ath.cx","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-07T21:59:46Z","receivedAt":"2012-03-07T21:59:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Am 05.03.2012 22:34, schrieb Junio C Hamano:\n>\n>> +#define util_as_int(elem) ((intptr_t)((elem)->util))\n>\n> Something that actually returns an int would fit the name better. ;)\n\nThe particular type would not matter to the callers of the helper\nmacro, would it?\n\n>> +static int committer_is_me(const char *name)\n>> +{\n>> +\tint namelen = strlen(name);\n>> +\tconst char *me = git_committer_info(IDENT_NO_DATE);\n>> +\treturn (me && !memcmp(me, name, namelen) &&\n>> +\t\t!memcmp(me + namelen, \" <\", 2));\n>> +}\n>\n> This looks scary due to the missing length check of me before the\n> memcmp() call, but is actually safe because git_committer_info()\n> returns a pointer to a static buffer that is just as long as name can\n> possibly be. Still, perhaps this is nicer instead:\n>\n> \tconst char *me = git_committer_info(IDENT_NO_DATE);\n> \tconst char *rest = skip_prefix(me, name);\n> \treturn rest && skip_prefix(rest, \" <\");\n\nProbably. Let me fix it up.\n\nThanks.\n"},{"id":"186469","messageId":"4F58F06A.1070108@lsrfire.ath.cx","threadId":"29832","inReplyTo":"7vzkbskr71.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-03-08T17:46:18Z","receivedAt":"2012-03-08T17:46:18Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 07.03.2012 22:59, schrieb Junio C Hamano:\n> René Scharfe writes:\n>\n>> Am 05.03.2012 22:34, schrieb Junio C Hamano:\n>>\n>>> +#define util_as_int(elem) ((intptr_t)((elem)->util))\n>>\n>> Something that actually returns an int would fit the name better. ;)\n>\n> The particular type would not matter to the callers of the helper\n> macro, would it?\n\nThree of the five callers introduced in that commit cast the result to \nint and the remaining two don't care, so it actually does seem to matter \nfor most of them, strictly speaking. When I see a nit, I can't resist \nthe urge to pick it, apparently.\n\nRené\n"},{"id":"186480","messageId":"7v1up2ew9u.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"4F58F06A.1070108@lsrfire.ath.cx","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-08T19:18:53Z","receivedAt":"2012-03-08T19:18:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Am 07.03.2012 22:59, schrieb Junio C Hamano:\n>> René Scharfe writes:\n>>\n>>> Am 05.03.2012 22:34, schrieb Junio C Hamano:\n>>>\n>>>> +#define util_as_int(elem) ((intptr_t)((elem)->util))\n>>>\n>>> Something that actually returns an int would fit the name better. ;)\n>>\n>> The particular type would not matter to the callers of the helper\n>> macro, would it?\n>\n> Three of the five callers introduced in that commit cast the result to\n> int and the remaining two don't care, so it actually does seem to\n> matter for most of them, strictly speaking. When I see a nit, I can't\n> resist the urge to pick it, apparently.\n\nUnfortunately, replacing intptr_t with int or casting the above\nagain as int will result in\n\nbuiltin/fmt-merge-msg.c: In function 'record_person':\nbuiltin/fmt-merge-msg.c:213: error: cast to pointer from integer of different size\n\nSo...\n"},{"id":"186499","messageId":"7vr4x2bwzy.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"7v1up2ew9u.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-08T21:31:29Z","receivedAt":"2012-03-08T21:31:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> Three of the five callers introduced in that commit cast the result to\n>> int and the remaining two don't care, so it actually does seem to\n>> matter for most of them, strictly speaking. When I see a nit, I can't\n>> resist the urge to pick it, apparently.\n>\n> Unfortunately, replacing intptr_t with int or casting the above\n> again as int will result in\n>\n> builtin/fmt-merge-msg.c: In function 'record_person':\n> builtin/fmt-merge-msg.c:213: error: cast to pointer from integer of different size\n>\n> So...\n\n-- >8 --\nSubject: [PATCH] fmt-merge-msg.c: make util_as_int() return \"int\"\n\nAs its name says, the return value from util_as_int() should be\nusable where an int is called for without casting.\n\nSpotted-by: René Scharfe\nSigned-off-by: Junio C Hamano \n---\n builtin/fmt-merge-msg.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 40b90b1..8ddefb3 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -182,7 +182,7 @@ static void add_branch_desc(struct strbuf *out, const char *name)\n \tstrbuf_release(&desc);\n }\n \n-#define util_as_int(elem) ((intptr_t)((elem)->util))\n+#define util_as_int(elem) ((int)((elem)->util))\n \n static void record_person(int which, struct string_list *people,\n \t\t\t struct commit *commit)\n@@ -210,7 +210,7 @@ static void record_person(int which, struct string_list *people,\n \t\telem = string_list_insert(people, name_buf);\n \t\telem->util = (void *)0;\n \t}\n-\telem->util = (void*)(util_as_int(elem) + 1);\n+\telem->util = (void*)((intptr_t)(util_as_int(elem) + 1));\n }\n \n static int cmp_string_list_util_as_int(const void *a_, const void *b_)\n@@ -226,13 +226,13 @@ static void add_people_count(struct strbuf *out, struct string_list *people)\n \telse if (people->nr == 2)\n \t\tstrbuf_addf(out, \"%s (%d) and %s (%d)\",\n \t\t\t people->items[0].string,\n-\t\t\t (int)util_as_int(&people->items[0]),\n+\t\t\t util_as_int(&people->items[0]),\n \t\t\t people->items[1].string,\n-\t\t\t (int)util_as_int(&people->items[1]));\n+\t\t\t util_as_int(&people->items[1]));\n \telse if (people->nr)\n \t\tstrbuf_addf(out, \"%s (%d) and others\",\n \t\t\t people->items[0].string,\n-\t\t\t (int)util_as_int(&people->items[0]));\n+\t\t\t util_as_int(&people->items[0]));\n }\n \n static int committer_is_me(const char *name)\n-- \n1.7.10.rc0.28.g709d0\n"},{"id":"186684","messageId":"20120312071121.GA17269@burratino","threadId":"29832","inReplyTo":"7vmx7uzq8h.fsf_-_@alter.siamese.dyndns.org","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-03-12T07:11:22Z","receivedAt":"2012-03-12T07:11:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n\n> * jc/fmt-merge-msg-people (2012-03-08) 3 commits\n> (merged to 'next' on 2012-03-08 at be31aa9)\n> + fmt-merge-msg.c: make util_as_int() return \"int\"\n> (merged to 'next' on 2012-03-07 at 76fbac3)\n> + fmt-merge-msg: finishing touches\n> (merged to 'next' on 2012-03-05 at 38de349)\n> + fmt-merge-msg: show those involved in a merged series\n>\n> The \"fmt-merge-msg\" command learns to list the primary contributors\n> involved in the side topic you are merging.\n\nAh, so that's where the funny\n\n\tBy Jonathan Nieder\n\tvia Jonathan Nieder\n\nlines in\n\n\tMerge branch 'jn/maint-fast-import-empty-ls' into pu\n\n\tBy Jonathan Nieder\n\tvia Jonathan Nieder\n\t* jn/maint-fast-import-empty-ls:\n\t fast-import: don't allow 'ls' of path with empty components\n\t fast-import: leakfix for 'ls' of dirty trees\n\ncome from. As a person reading the history, I admit I don't like it.\nI enjoyed being able to get a simple overview at a higher level of\nwhat has been happening in \"pu\" with \"git log --merges junio/pu\" or\n\"git log --first-parent junio/pu\", and these extra lines before and\nadjacent to the \"* name of topic:\" header interfere with that.\n\nBy contrast, the\n\n\tMerge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs\n\n\tPull btrfs updates from Chris Mason:\n\t[more words about that here]\n\n\t* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs:\n\t Btrfs: fix casting error in scrub reada code\n\t[...]\n\ndescriptions in Linus's repo are very pleasant. It's a subtle\ndifference, but the extra whitespace and the way it presents the\nimportant bits like \"Pull btrfs updates\" before the mechanics \"from\nChris Mason\" help.\n\nSo I like the goal, but something about this execution does not seem\nto be working. Sorry I have no better news or more concrete feedback\nto offer.\n\nJonathan\n"},{"id":"186787","messageId":"CABURp0oVn3s27Rtq2JQeLBgcXq1-R-=0qawcXVMxZ8qgOeZGkQ@mail.gmail.com","threadId":"29832","inReplyTo":"7v1up2ew9u.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2012-03-12T21:37:57Z","receivedAt":"2012-03-12T21:37:57Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Thu, Mar 8, 2012 at 2:18 PM, Junio C Hamano wrote:\n> René Scharfe writes:\n>\n>> Am 07.03.2012 22:59, schrieb Junio C Hamano:\n>>> René Scharfe  writes:\n>>>\n>>>> Am 05.03.2012 22:34, schrieb Junio C Hamano:\n>>>>\n>>>>> +#define util_as_int(elem) ((intptr_t)((elem)->util))\n>>>>\n>>>> Something that actually returns an int would fit the name better. ;)\n>>>\n>>> The particular type would not matter to the callers of the helper\n>>> macro, would it?\n>>\n>> Three of the five callers introduced in that commit cast the result to\n>> int and the remaining two don't care, so it actually does seem to\n>> matter for most of them, strictly speaking.  When I see a nit, I can't\n>> resist the urge to pick it, apparently.\n>\n> Unfortunately, replacing intptr_t with int or casting the above\n> again as int will result in\n>\n> builtin/fmt-merge-msg.c: In function 'record_person':\n> builtin/fmt-merge-msg.c:213: error: cast to pointer from integer of different size\n>\n> So...\n\nOut of the frying pan, into the fire...\n\n builtin/fmt-merge-msg.c: In function ‘record_person’:\n builtin/fmt-merge-msg.c:213:34: warning: cast from pointer to\ninteger of different size [-Wpointer-to-int-cast]\n builtin/fmt-merge-msg.c: In function ‘cmp_string_list_util_as_int’:\n builtin/fmt-merge-msg.c:219:9: warning: cast from pointer to\ninteger of different size [-Wpointer-to-int-cast]\n builtin/fmt-merge-msg.c:219:26: warning: cast from pointer to\ninteger of different size [-Wpointer-to-int-cast]\n builtin/fmt-merge-msg.c: In function ‘add_people_count’:\n builtin/fmt-merge-msg.c:229:8: warning: cast from pointer to\ninteger of different size [-Wpointer-to-int-cast]\n builtin/fmt-merge-msg.c:231:8: warning: cast from pointer to\ninteger of different size [-Wpointer-to-int-cast]\n builtin/fmt-merge-msg.c:235:8: warning: cast from pointer to\ninteger of different size [-Wpointer-to-int-cast]\n\nI see slightly different code in pu than in next, but it produces the\nsame warnings on my 64-bit Linux machine.\n\nHere's a fix against next:\n\n-- >8 --\n\nSubject: [PATCH] Appease compiler pedantry with an extra cast\n\nRecently git repurposed a pointer as an integer to hold some\ncounter which git fancies.\n\nCasting directly from 'pointer' to 'int' ((int)(void*)&x) causes a\npossible size mismatch because pointers can be bigger than ints.\nIn such a situation, the compiler complains:\n\n warning: cast from pointer to integer of different size\n [-Wpointer-to-int-cast]\n\nCast the value through intptr_t first to quell compiler complaints\nabout how this gun appears to be aimed near our feet. Then cast this\nvalue to an int; this path assures the compiler we are smarter than we\nlook, or at least that we intend to aim the gun this way for a reason.\n---\n builtin/fmt-merge-msg.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 8ddefb3..fee65e0 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -182,7 +182,7 @@ static void add_branch_desc(struct strbuf *out,\nconst char *name)\n strbuf_release(&desc);\n }\n\n-#define util_as_int(elem) ((int)((elem)->util))\n+#define util_as_int(elem) ((int)(intptr_t)((elem)->util))\n\n static void record_person(int which, struct string_list *people,\n struct commit *commit)\n-- \n1.7.9.3\n"},{"id":"186804","messageId":"7vipi9mfhx.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"20120312071121.GA17269@burratino","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-13T01:55:22Z","receivedAt":"2012-03-13T01:55:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> .... As a person reading the history, I admit I don't like it.\n> I enjoyed being able to get a simple overview at a higher level of\n> what has been happening in \"pu\" with \"git log --merges junio/pu\" or\n> \"git log --first-parent junio/pu\", and these extra lines before and\n> adjacent to the \"* name of topic:\" header interfere with that.\n\nI'd hold making that judgement for a several weeks until my eyes get\nused to if I were you. I've seen that people (including myself)\nreact really badly to _any_ change and make loud noises (including\n\"we will never get used to this updated output, it is horrible!\"),\nand then eventually get used to it as if nothing happened, and that\nhappened often enough recently.\n\nIn any case, if you only look at \"git log --first-parent\" output and\nsearch for your own topic, it of course is useless to see your name\nthere, as you already know.\n\n> By contrast, the\n> ...\n> descriptions in Linus's repo are very pleasant.\n\nWhen you compare Linus's history and my history between master..pu,\nyou are literally comparing apples and oranges.\n\nThe merges between master..pu are made several times a day, with a\nseries of mechanical \"merge --no-edit\" process and automated tree\ntweaking (including but not limited to rerere). The purpose of\nthese merges is primarily to reduce the risk of mismerges to master\n(and next to a lessor degree), especially when one topic among many\nthat have been cooking between master..pu gets closer to graduation.\nBy shuffling the order of topics that are merged between master..pu\nso that topics close to graduation come earlier in the fully rebuilt\npu, a mid-point in master..pu is verified to exactly match the tree\nof next (otherwise you may have spotted a mismerge to next, and I\ndid spot a few mismerges to next this way). This also allows earlier\nparts of the master..pu to be tested individually.\n\nThe purpose of these merges is _not_ about describing what the side\nbranches are about. Unlike Linus's lieutenants' \"for-linus\" branch\nnames, the branch names are often enough to describe that they are.\n\nOn the other hand, the merges on Linus's tree are etched in stone,\nand he has every incentive to record what happened in the side\nbranch for the _last_ time with carefully chosen words.\n\nHaving said that, I tweaked the automated rebuilding procedure a bit\ntoday, and made it annotate these merges with snippets from the\nbranch description in the \"What's cooking\" document, so the commits\non master..pu are hopefully \"very pleasant\"ly annotated. This not\nonly prettifies the merges between master..pu, but more importantly,\nwould save effort to explain the merges when a topic finally hits\nmaster. If I have a good description in \"What's cooking\", I can then\nreuse it in these merges and also in the release notes.\n\nThe update to the rebuild procedure is not published yet. I'll be\nplaying with it for a few days before publishing the changes.\n"},{"id":"186815","messageId":"20120313052335.GA16664@burratino","threadId":"29832","inReplyTo":"7vipi9mfhx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-03-13T05:23:35Z","receivedAt":"2012-03-13T05:23:35Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> Having said that, I tweaked the automated rebuilding procedure a bit\n> today, and made it annotate these merges with snippets from the\n> branch description in the \"What's cooking\" document, so the commits\n> on master..pu are hopefully \"very pleasant\"ly annotated.\n\nYes, that helps.\n\nI also wonder if it would make sense to change\n\n\tMerge branch 'mm/push-default-switch-warning' into pu\n\n\t... description ...\n\n\tBy Matthieu Moy\n\t* mm/push-default-switch-warning:\n\t push: start warning upcoming default change for push.default\n\nto\n\n\tMerge branch 'mm/push-default-switch-warning' into pu\n\n\t... description ...\n\n\t* 'mm/push-default-switch-warning' by Matthieu Moy:\n\t push: start warning upcoming default change for push.default\n\nwhich makes the author less distractingly prominent (since they will\nbe right there in the log soon later once we hit the commits\nthemselves) and makes the start of the list of commits easier to find\nby eye. This is what I was inarticulately hinting at in my message\nbefore. With two authors it still looks reasonable:\n\n\t* 'jc/pickaxe-ignore-case' by Junio C Hamano (2) and Ramsay Jones (1):\n\t ctype.c: Fix a sparse warning\n\t pickaxe: allow -i to search in patch case-insensitively\n\t grep: use static trans-case table\n\nIf this seems worth a patch, I'd be glad to try it (and even gladder\nto learn that someone else already tried it).\n\nJonathan\n"},{"id":"186816","messageId":"7v4nttm4uj.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"20120313052335.GA16664@burratino","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-13T05:45:24Z","receivedAt":"2012-03-13T05:45:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> I also wonder if it would make sense to change\n> ... to ...\n> \t* 'mm/push-default-switch-warning' by Matthieu Moy:\n> \t push: start warning upcoming default change for push.default\n> which makes the author less distractingly prominent (since they will\n> be right there in the log soon later once we hit the commits\n> themselves) and makes the start of the list of commits easier to find\n> by eye. This is what I was inarticulately hinting at in my message\n\nHaving the author/subsystem attribution on separate lines was a\ndeliberate design decision made by me, so that (1) cutting and\npasting Linus would prefer to do stays easier, and (2) giving the\ncredits more prominently in a default format. There is a paragraph\nbreak before the attribution begins anyway, and I think eyes easily\nlearn to scan for paragraphs that begin \"By \" instead of \" * \".\n"},{"id":"186821","messageId":"4F5EF6EC.20008@viscovery.net","threadId":"29832","inReplyTo":"7vipi9mfhx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-03-13T07:27:40Z","receivedAt":"2012-03-13T07:27:40Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 3/13/2012 2:55, schrieb Junio C Hamano:\n> I'd hold making that judgement for a several weeks until my eyes get\n> used to if I were you. I've seen that people (including myself)\n> react really badly to _any_ change and make loud noises (including\n> \"we will never get used to this updated output, it is horrible!\"),\n> and then eventually get used to it as if nothing happened, and that\n> happened often enough recently.\n\nI can buy that. I won't mind reading the new lines in foreign projects,\nbut in at least one project I'm working in I prefer not to have these new\nlines in the merge commit messages. Can it be opted-out?\n\n-- Hannes\n"},{"id":"186888","messageId":"7v4ntswe54.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"4F5EF6EC.20008@viscovery.net","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-13T18:26:47Z","receivedAt":"2012-03-13T18:26:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 3/13/2012 2:55, schrieb Junio C Hamano:\n>> I'd hold making that judgement for a several weeks until my eyes get\n>> used to if I were you. I've seen that people (including myself)\n>> react really badly to _any_ change and make loud noises (including\n>> \"we will never get used to this updated output, it is horrible!\"),\n>> and then eventually get used to it as if nothing happened, and that\n>> happened often enough recently.\n>\n> I can buy that.\n\nAssuming \"that\" refers to my \"hold ... for several weeks\", let me stop\nreading right here. We can talk about the rest of your message in several\nweeks.\n"},{"id":"186889","messageId":"7vzkbkuzh9.fsf_-_@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"4F5EF6EC.20008@viscovery.net","subject":"[PATCH v2 1/1] fmt-merge-msg: show those involved in a merged series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-13T18:28:50Z","receivedAt":"2012-03-13T18:28:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"As we already walk the history of the branch that gets merged to\ncome up with a short log, let's label it with names of the primary\nauthors, so that the user who summarizes the merge can easily give\ncredit to them in the log message.\n\nAlso infer the names of \"lieutents\" to help integrators at higher\nlevel of the food-chain to give credit to them, by counting:\n\n * The committer of the 'tip' commit that is merged\n * The committer of merge commits that are merged\n\nOften the first one gives the owner of the history being pulled, but\nhis last pull from his sublieutenants may have been a fast-forward,\nin which case the first one would not be. The latter rule will\ncount the integrator of the history, so together it might be a\nreasonable heuristics.\n\nThere are two special cases:\n\n - The \"author\" credit is omitted when the series is written solely\n by the same author who is making the merge. The name can be seen\n on the \"Author\" line of the \"git log\" output to view the log\n message anyway.\n\n - The \"lieutenant\" credit is omitted when there is only one key\n committer in the merged branch and it is the committer who is\n making the merge. Typically this applies to the case where the\n developer merges his own branch.\n\nSigned-off-by: Junio C Hamano \n---\n\n * This is a fresh re-roll without all those \"oops, this does not compile\n on a compiler that warns about casting between a pointer and intptr_t\n while being happy to see it further casted down to int\" fix-up patches.\n\n A major difference can be seen by the lack of changes to existing\n tests. The tests earlier series touched are all about single developer\n case, and there is no need to update the test vectors for this change.\n\n builtin/fmt-merge-msg.c | 114 ++++++++++++++++++++++++++++++++++++++++++++--\n t/t6200-fmt-merge-msg.sh | 27 +++++++++--\n 2 files changed, 134 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex c81a7fe..4e7196a 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -27,6 +27,8 @@ int fmt_merge_msg_config(const char *key, const char *value, void *cb)\n \t\t\tmerge_log_config = DEFAULT_MERGE_LOG_LEN;\n \t} else if (!strcmp(key, \"merge.branchdesc\")) {\n \t\tuse_branch_desc = git_config_bool(key, value);\n+\t} else {\n+\t\treturn git_default_config(key, value, cb);\n \t}\n \treturn 0;\n }\n@@ -180,6 +182,101 @@ static void add_branch_desc(struct strbuf *out, const char *name)\n \tstrbuf_release(&desc);\n }\n \n+#define util_as_int(elem) ((int)(intptr_t)((elem)->util))\n+\n+static void record_person(int which, struct string_list *people,\n+\t\t\t struct commit *commit)\n+{\n+\tchar name_buf[MAX_GITNAME], *name, *name_end;\n+\tstruct string_list_item *elem;\n+\tconst char *field = (which == 'a') ? \"\\nauthor \" : \"\\ncommitter \";\n+\n+\tname = strstr(commit->buffer, field);\n+\tif (!name)\n+\t\treturn;\n+\tname += strlen(field);\n+\tname_end = strchrnul(name, '<');\n+\tif (*name_end)\n+\t\tname_end--;\n+\twhile (isspace(*name_end) && name <= name_end)\n+\t\tname_end--;\n+\tif (name_end < name || name + MAX_GITNAME <= name_end)\n+\t\treturn;\n+\tmemcpy(name_buf, name, name_end - name + 1);\n+\tname_buf[name_end - name + 1] = '\\0';\n+\n+\telem = string_list_lookup(people, name_buf);\n+\tif (!elem) {\n+\t\telem = string_list_insert(people, name_buf);\n+\t\telem->util = (void *)0;\n+\t}\n+\telem->util = (void*)((intptr_t)(util_as_int(elem) + 1));\n+}\n+\n+static int cmp_string_list_util_as_int(const void *a_, const void *b_)\n+{\n+\tconst struct string_list_item *a = a_, *b = b_;\n+\treturn util_as_int(b) - util_as_int(a);\n+}\n+\n+static void add_people_count(struct strbuf *out, struct string_list *people)\n+{\n+\tif (people->nr == 1)\n+\t\tstrbuf_addf(out, \"%s\", people->items[0].string);\n+\telse if (people->nr == 2)\n+\t\tstrbuf_addf(out, \"%s (%d) and %s (%d)\",\n+\t\t\t people->items[0].string,\n+\t\t\t util_as_int(&people->items[0]),\n+\t\t\t people->items[1].string,\n+\t\t\t util_as_int(&people->items[1]));\n+\telse if (people->nr)\n+\t\tstrbuf_addf(out, \"%s (%d) and others\",\n+\t\t\t people->items[0].string,\n+\t\t\t util_as_int(&people->items[0]));\n+}\n+\n+static void credit_people(struct strbuf *out,\n+\t\t\t struct string_list *them,\n+\t\t\t int kind)\n+{\n+\tconst char *label;\n+\tconst char *me;\n+\n+\tif (kind == 'a') {\n+\t\tlabel = \"\\nBy \";\n+\t\tme = git_author_info(IDENT_NO_DATE);\n+\t} else {\n+\t\tlabel = \"\\nvia \";\n+\t\tme = git_committer_info(IDENT_NO_DATE);\n+\t}\n+\n+\tif (!them->nr ||\n+\t (them->nr == 1 &&\n+\t me &&\n+\t (me = skip_prefix(me, them->items->string)) != NULL &&\n+\t skip_prefix(me, \" <\")))\n+\t\treturn;\n+\tstrbuf_addstr(out, label);\n+\tadd_people_count(out, them);\n+}\n+\n+static void add_people_info(struct strbuf *out,\n+\t\t\t struct string_list *authors,\n+\t\t\t struct string_list *committers)\n+{\n+\tif (authors->nr)\n+\t\tqsort(authors->items,\n+\t\t authors->nr, sizeof(authors->items[0]),\n+\t\t cmp_string_list_util_as_int);\n+\tif (committers->nr)\n+\t\tqsort(committers->items,\n+\t\t committers->nr, sizeof(committers->items[0]),\n+\t\t cmp_string_list_util_as_int);\n+\n+\tcredit_people(out, authors, 'a');\n+\tcredit_people(out, committers, 'c');\n+}\n+\n static void shortlog(const char *name,\n \t\t struct origin_data *origin_data,\n \t\t struct commit *head,\n@@ -190,6 +287,8 @@ static void shortlog(const char *name,\n \tstruct commit *commit;\n \tstruct object *branch;\n \tstruct string_list subjects = STRING_LIST_INIT_DUP;\n+\tstruct string_list authors = STRING_LIST_INIT_DUP;\n+\tstruct string_list committers = STRING_LIST_INIT_DUP;\n \tint flags = UNINTERESTING | TREESAME | SEEN | SHOWN | ADDED;\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst unsigned char *sha1 = origin_data->sha1;\n@@ -199,7 +298,6 @@ static void shortlog(const char *name,\n \t\treturn;\n \n \tsetup_revisions(0, NULL, rev, NULL);\n-\trev->ignore_merges = 1;\n \tadd_pending_object(rev, branch, name);\n \tadd_pending_object(rev, &head->object, \"^HEAD\");\n \thead->object.flags |= UNINTERESTING;\n@@ -208,10 +306,15 @@ static void shortlog(const char *name,\n \twhile ((commit = get_revision(rev)) != NULL) {\n \t\tstruct pretty_print_context ctx = {0};\n \n-\t\t/* ignore merges */\n-\t\tif (commit->parents && commit->parents->next)\n+\t\tif (commit->parents && commit->parents->next) {\n+\t\t\t/* do not list a merge but count committer */\n+\t\t\trecord_person('c', &committers, commit);\n \t\t\tcontinue;\n-\n+\t\t}\n+\t\tif (!count)\n+\t\t\t/* the 'tip' committer */\n+\t\t\trecord_person('c', &committers, commit);\n+\t\trecord_person('a', &authors, commit);\n \t\tcount++;\n \t\tif (subjects.nr > limit)\n \t\t\tcontinue;\n@@ -226,6 +329,7 @@ static void shortlog(const char *name,\n \t\t\tstring_list_append(&subjects, strbuf_detach(&sb, NULL));\n \t}\n \n+\tadd_people_info(out, &authors, &committers);\n \tif (count > limit)\n \t\tstrbuf_addf(out, \"\\n* %s: (%d commits)\\n\", name, count);\n \telse\n@@ -246,6 +350,8 @@ static void shortlog(const char *name,\n \trev->commits = NULL;\n \trev->pending.nr = 0;\n \n+\tstring_list_clear(&authors, 0);\n+\tstring_list_clear(&committers, 0);\n \tstring_list_clear(&subjects, 0);\n }\n \ndiff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh\nindex 9a16806..9b50f54 100755\n--- a/t/t6200-fmt-merge-msg.sh\n+++ b/t/t6200-fmt-merge-msg.sh\n@@ -35,15 +35,18 @@ test_expect_success setup '\n \n \techo \"l3\" >two &&\n \ttest_tick &&\n-\tgit commit -a -m \"Left #3\" &&\n+\tGIT_COMMITTER_NAME=\"Another Committer\" \\\n+\tGIT_AUTHOR_NAME=\"Another Author\" git commit -a -m \"Left #3\" &&\n \n \techo \"l4\" >two &&\n \ttest_tick &&\n-\tgit commit -a -m \"Left #4\" &&\n+\tGIT_COMMITTER_NAME=\"Another Committer\" \\\n+\tGIT_AUTHOR_NAME=\"Another Author\" git commit -a -m \"Left #4\" &&\n \n \techo \"l5\" >two &&\n \ttest_tick &&\n-\tgit commit -a -m \"Left #5\" &&\n+\tGIT_COMMITTER_NAME=\"Another Committer\" \\\n+\tGIT_AUTHOR_NAME=\"Another Author\" git commit -a -m \"Left #5\" &&\n \tgit tag tag-l5 &&\n \n \tgit checkout right &&\n@@ -99,6 +102,8 @@ test_expect_success '[merge] summary/log configuration' '\n \tcat >expected <<-EOF &&\n \tMerge branch ${apos}left${apos}\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* left:\n \t Left #5\n \t Left #4\n@@ -144,6 +149,8 @@ test_expect_success 'merge.log=3 limits shortlog length' '\n \tcat >expected <<-EOF &&\n \tMerge branch ${apos}left${apos}\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* left: (5 commits)\n \t Left #5\n \t Left #4\n@@ -159,6 +166,8 @@ test_expect_success 'merge.log=5 shows all 5 commits' '\n \tcat >expected <<-EOF &&\n \tMerge branch ${apos}left${apos}\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* left:\n \t Left #5\n \t Left #4\n@@ -181,6 +190,8 @@ test_expect_success '--log=3 limits shortlog length' '\n \tcat >expected <<-EOF &&\n \tMerge branch ${apos}left${apos}\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* left: (5 commits)\n \t Left #5\n \t Left #4\n@@ -196,6 +207,8 @@ test_expect_success '--log=5 shows all 5 commits' '\n \tcat >expected <<-EOF &&\n \tMerge branch ${apos}left${apos}\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* left:\n \t Left #5\n \t Left #4\n@@ -225,6 +238,8 @@ test_expect_success 'fmt-merge-msg -m' '\n \tcat >expected.log <<-EOF &&\n \tSync with left\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* ${apos}left${apos} of $(pwd):\n \t Left #5\n \t Left #4\n@@ -256,6 +271,8 @@ test_expect_success 'setup: expected shortlog for two branches' '\n \tcat >expected <<-EOF\n \tMerge branches ${apos}left${apos} and ${apos}right${apos}\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* left:\n \t Left #5\n \t Left #4\n@@ -379,6 +396,8 @@ test_expect_success 'merge-msg two tags' '\n \t Common #2\n \t Common #1\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* tag ${apos}tag-l5${apos}:\n \t Left #5\n \t Left #4\n@@ -407,6 +426,8 @@ test_expect_success 'merge-msg tag and branch' '\n \t Common #2\n \t Common #1\n \n+\tBy Another Author (3) and A U Thor (2)\n+\tvia Another Committer\n \t* left:\n \t Left #5\n \t Left #4\n-- \n1.7.10.rc0.65.g3445e\n"},{"id":"186903","messageId":"20120313210357.GC27436@sigill.intra.peff.net","threadId":"29832","inReplyTo":"CABURp0oVn3s27Rtq2JQeLBgcXq1-R-=0qawcXVMxZ8qgOeZGkQ@mail.gmail.com","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-13T21:03:57Z","receivedAt":"2012-03-13T21:03:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 12, 2012 at 05:37:57PM -0400, Phil Hord wrote:\n\n> Subject: [PATCH] Appease compiler pedantry with an extra cast\n> \n> Recently git repurposed a pointer as an integer to hold some\n> counter which git fancies.\n> \n> Casting directly from 'pointer' to 'int' ((int)(void*)&x) causes a\n> possible size mismatch because pointers can be bigger than ints.\n> In such a situation, the compiler complains:\n> \n> warning: cast from pointer to integer of different size\n> [-Wpointer-to-int-cast]\n\nYeah, I've been seeing the same warning on my x86_64 box, and came up\nwith the same fix. However...\n\n> Cast the value through intptr_t first to quell compiler complaints\n> about how this gun appears to be aimed near our feet. Then cast this\n> value to an int; this path assures the compiler we are smarter than we\n> look, or at least that we intend to aim the gun this way for a reason.\n\nThis feels so hacky. One of the callsites does:\n\n elem->util = (void*)((intptr_t)(util_as_int(elem) + 1));\n\nwhich will truncate the value down to an int before replacing it back in\nthe void pointer. And that truncation is ultimately what the compiler is\nwarning about, and what we are sneaking around with the extra cast\n(because casting between integer sizes of different types is OK, even\nthough it can cause truncation).\n\nI don't think the truncation is a problem in practice, but it just feels\nlike we are not just silencing an over-zealous compiler, but actually\nburying type-size assumption behind a set of four (4!) casts.\n\nI wonder if we would be happier to declare the \"util\" field of\nstring_list as a union. Obviously that provides no safety that we read\nthe correct item out of the union, but that is no worse than the\nsituation with all of these casts, and I suspect the result would be\nmuch more obvious and readable.\n\nThe downside is that all current users of the \"util\" field would need\ns/util/&.ptr/ or similar to dereference the pointer field.\n\n-Peff\n"},{"id":"186915","messageId":"7vwr6nsv6z.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"20120313210357.GC27436@sigill.intra.peff.net","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-14T03:44:20Z","receivedAt":"2012-03-14T03:44:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Mar 12, 2012 at 05:37:57PM -0400, Phil Hord wrote:\n>\n> This feels so hacky. One of the callsites does:\n>\n> elem->util = (void*)((intptr_t)(util_as_int(elem) + 1));\n>\n> which will truncate the value down to an int before replacing it back in\n> the void pointer. And that truncation is ultimately what the compiler is\n> warning about, and what we are sneaking around with the extra cast\n> (because casting between integer sizes of different types is OK, even\n> though it can cause truncation).\n\nYeah, I find it utterly disgusting that Phil's compiler is picky about\n(int)(void *) and insists on (intptr_t)(void *), while totally ignoring\nthe same bit lossage coming from (int)(intptr_t)(void *).\n\nI am again starting to think that the very original was probably the least\nbad among the yuckies.\n\nAs the whole point of the helper macro is to cast the .util field as\n\"some\" integral type, I'm tempted to squash this into the v2 patch I\nposted earlier today. Earlier complaint from René was very sensible that\nit looked funny to cast xxx_as_int() explicitly to int, but having to cast\n\"some\" integral value explicitly to int is required in the context of\nsprintf() like vararg function, so it would no longer apply to this\nversion.\n\n builtin/fmt-merge-msg.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 4e7196a..1bc6b8b 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -182,7 +182,7 @@ static void add_branch_desc(struct strbuf *out, const char *name)\n \tstrbuf_release(&desc);\n }\n \n-#define util_as_int(elem) ((int)(intptr_t)((elem)->util))\n+#define util_as_integral(elem) ((intptr_t)((elem)->util))\n \n static void record_person(int which, struct string_list *people,\n \t\t\t struct commit *commit)\n@@ -210,13 +210,13 @@ static void record_person(int which, struct string_list *people,\n \t\telem = string_list_insert(people, name_buf);\n \t\telem->util = (void *)0;\n \t}\n-\telem->util = (void*)((intptr_t)(util_as_int(elem) + 1));\n+\telem->util = (void*)(util_as_integral(elem) + 1);\n }\n \n-static int cmp_string_list_util_as_int(const void *a_, const void *b_)\n+static int cmp_string_list_util_as_integral(const void *a_, const void *b_)\n {\n \tconst struct string_list_item *a = a_, *b = b_;\n-\treturn util_as_int(b) - util_as_int(a);\n+\treturn util_as_integral(b) - util_as_integral(a);\n }\n \n static void add_people_count(struct strbuf *out, struct string_list *people)\n@@ -226,13 +226,13 @@ static void add_people_count(struct strbuf *out, struct string_list *people)\n \telse if (people->nr == 2)\n \t\tstrbuf_addf(out, \"%s (%d) and %s (%d)\",\n \t\t\t people->items[0].string,\n-\t\t\t util_as_int(&people->items[0]),\n+\t\t\t (int)util_as_integral(&people->items[0]),\n \t\t\t people->items[1].string,\n-\t\t\t util_as_int(&people->items[1]));\n+\t\t\t (int)util_as_integral(&people->items[1]));\n \telse if (people->nr)\n \t\tstrbuf_addf(out, \"%s (%d) and others\",\n \t\t\t people->items[0].string,\n-\t\t\t util_as_int(&people->items[0]));\n+\t\t\t (int)util_as_integral(&people->items[0]));\n }\n \n static void credit_people(struct strbuf *out,\n@@ -267,11 +267,11 @@ static void add_people_info(struct strbuf *out,\n \tif (authors->nr)\n \t\tqsort(authors->items,\n \t\t authors->nr, sizeof(authors->items[0]),\n-\t\t cmp_string_list_util_as_int);\n+\t\t cmp_string_list_util_as_integral);\n \tif (committers->nr)\n \t\tqsort(committers->items,\n \t\t committers->nr, sizeof(committers->items[0]),\n-\t\t cmp_string_list_util_as_int);\n+\t\t cmp_string_list_util_as_integral);\n \n \tcredit_people(out, authors, 'a');\n \tcredit_people(out, committers, 'c');\n"},{"id":"186919","messageId":"4F603CA9.70906@viscovery.net","threadId":"29832","inReplyTo":"7v4ntswe54.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-03-14T06:37:29Z","receivedAt":"2012-03-14T06:37:29Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 3/13/2012 19:26, schrieb Junio C Hamano:\n> Johannes Sixt writes:\n>> I can buy that.\n> \n> Assuming \"that\" refers to my \"hold ... for several weeks\", let me stop\n> reading right here. We can talk about the rest of your message in several\n> weeks.\n\nNo, \"that\" refers to \"make loud noises (including \"we will never get used\nto this updated output, it is horrible!\"), and then eventually get used to\nit as if nothing happened\". Nevertheless, I would like to opt-out of the\nnew behavior for my own projects even if my eyes will have been trained to\nsee the new lines in the history of foreign projects.\n\n-- Hannes\n"},{"id":"186981","messageId":"CABURp0qE9XRg+a8WRdicojJNqcLnZn_PozFTDO8qZ8BMM1NsJQ@mail.gmail.com","threadId":"29832","inReplyTo":"20120313210357.GC27436@sigill.intra.peff.net","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2012-03-14T19:12:08Z","receivedAt":"2012-03-14T19:12:08Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Tue, Mar 13, 2012 at 5:03 PM, Jeff King wrote:\n> On Mon, Mar 12, 2012 at 05:37:57PM -0400, Phil Hord wrote:\n>\n>> Subject: [PATCH] Appease compiler pedantry with an extra cast\n>>\n>> Recently git repurposed a pointer as an integer to hold some\n>> counter which git fancies.\n>>\n>> Casting directly from 'pointer' to 'int' ((int)(void*)&x) causes a\n>> possible size mismatch because pointers can be bigger than ints.\n>> In such a situation, the compiler complains:\n>>\n>>    warning: cast from pointer to integer of different size\n>>             [-Wpointer-to-int-cast]\n>\n> Yeah, I've been seeing the same warning on my x86_64 box, and came up\n> with the same fix. However...\n>\n>> Cast the value through intptr_t first to quell compiler complaints\n>> about how this gun appears to be aimed near our feet.  Then cast this\n>> value to an int; this path assures the compiler we are smarter than we\n>> look, or at least that we intend to aim the gun this way for a reason.\n>\n> This feels so hacky.\n\nWell, that's because it is hacky. But it's the original code that's suspect.\n\n> One of the callsites does:\n>\n>    elem->util = (void*)((intptr_t)(util_as_int(elem) + 1));\n>\n> which will truncate the value down to an int before replacing it back in\n> the void pointer. And that truncation is ultimately what the compiler is\n> warning about, and what we are sneaking around with the extra cast\n> (because casting between integer sizes of different types is OK, even\n> though it can cause truncation).\n\nI think this one is ok because it's really just \"the hacky\" bit\nstoring an integer in a variable meant to hold a pointer. That's why\nit's incrementing here, I suppose, not because it really wants to\npoint at the next byte.\n\n> I don't think the truncation is a problem in practice, but it just feels\n> like we are not just silencing an over-zealous compiler, but actually\n> burying type-size assumption behind a set of four (4!) casts.\n\nThe compiler is doing its job here to warn us against storing big-ish\npointers in small-ish ints. But if we know we will never accidentally\nuse this as a pointer and if the integer will never overflow the\n32-bounds of the (int) representation, then it's all good.\n\nBut I agree, it is hacky.\n\nPhil\n"},{"id":"186990","messageId":"7vaa3iq5uo.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"4F603CA9.70906@viscovery.net","subject":"Re: [PATCH] fmt-merge-msg: show those involved in a merged series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-14T20:34:39Z","receivedAt":"2012-03-14T20:34:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 3/13/2012 19:26, schrieb Junio C Hamano:\n>> Johannes Sixt writes:\n>>> I can buy that.\n>> \n>> Assuming \"that\" refers to my \"hold ... for several weeks\", let me stop\n>> reading right here. We can talk about the rest of your message in several\n>> weeks.\n>\n> No, \"that\" refers to \"make loud noises (including \"we will never get used\n> to this updated output, it is horrible!\"), and then eventually get used to\n> it as if nothing happened\". Nevertheless,...\n\nI understand that. But ask yourself honestly and tell me (in several\nweeks) why you do not think that your \"would like to opt-out\" part falls\ninto exactly the same \"loud noises immediately after seeing a change\"\ncategory.\n"},{"id":"191384","messageId":"20120511103122.GA19573@burratino","threadId":"29832","inReplyTo":"7vipi9mfhx.fsf@alter.siamese.dyndns.org","subject":"[PATCH/RFC] fmt-merge-msg: add a blank line after people info","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-05-11T10:31:22Z","receivedAt":"2012-05-11T10:31:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The new \"credit people\" feature in fmt-merge-msg changed the format of\na typical \"merge --log\" message from\n\n\tMerge branch for-upstream of git://git.example.com/subsystem\n\n\t* for-upstream: (51 commits)\n\t foo: bar baz\n\nto\n\n\tMerge branch for-upstream of git://git.example.com/subsystem\n\n\tBy C O Ntributor\n\t* for-upstream: (51 commits)\n\t foo: bar baz\n\nThe message feels more natural with a line of breathing room before\nthe list of one-line descriptions headed by the branch name, like\nmessages already get when attached to a merge by \"fmt-merge-msg -m\" or\nautomatically incorporated during a merge of an annotated tag.\n\nSigned-off-by: Jonathan Nieder \n---\nHi again,\n\nJunio C Hamano wrote:\n\n> I'd hold making that judgement for a several weeks until my eyes get\n> used to if I were you.\n\nTurns out my reaction is pretty much the same as before. I still like\nthe idea and still am distracted by the spacing. After the small\npatch below, the log seems peaceful again and I am totally fine with\nit.\n\n\tMerge branch for-upstream of git://git.example.com/subsystem\n\n\tBy Fred the Clown\n\n\t* for-upstream: (51 commits)\n\t foo: bar baz\n\nHope that helps,\nJonathan\n\n builtin/fmt-merge-msg.c | 5 +++++\n t/t6200-fmt-merge-msg.sh | 9 +++++++++\n 2 files changed, 14 insertions(+)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 1bc6b8b8..8f228781 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -264,6 +264,8 @@ static void add_people_info(struct strbuf *out,\n \t\t\t struct string_list *authors,\n \t\t\t struct string_list *committers)\n {\n+\tsize_t pos = out->len;\n+\n \tif (authors->nr)\n \t\tqsort(authors->items,\n \t\t authors->nr, sizeof(authors->items[0]),\n@@ -275,6 +277,9 @@ static void add_people_info(struct strbuf *out,\n \n \tcredit_people(out, authors, 'a');\n \tcredit_people(out, committers, 'c');\n+\n+\tif (out->len > pos)\n+\t\tstrbuf_addch(out, '\\n');\n }\n \n static void shortlog(const char *name,\ndiff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh\nindex 9b50f54c..d39417ba 100755\n--- a/t/t6200-fmt-merge-msg.sh\n+++ b/t/t6200-fmt-merge-msg.sh\n@@ -104,6 +104,7 @@ test_expect_success '[merge] summary/log configuration' '\n \n \tBy Another Author (3) and A U Thor (2)\n \tvia Another Committer\n+\n \t* left:\n \t Left #5\n \t Left #4\n@@ -151,6 +152,7 @@ test_expect_success 'merge.log=3 limits shortlog length' '\n \n \tBy Another Author (3) and A U Thor (2)\n \tvia Another Committer\n+\n \t* left: (5 commits)\n \t Left #5\n \t Left #4\n@@ -168,6 +170,7 @@ test_expect_success 'merge.log=5 shows all 5 commits' '\n \n \tBy Another Author (3) and A U Thor (2)\n \tvia Another Committer\n+\n \t* left:\n \t Left #5\n \t Left #4\n@@ -192,6 +195,7 @@ test_expect_success '--log=3 limits shortlog length' '\n \n \tBy Another Author (3) and A U Thor (2)\n \tvia Another Committer\n+\n \t* left: (5 commits)\n \t Left #5\n \t Left #4\n@@ -209,6 +213,7 @@ test_expect_success '--log=5 shows all 5 commits' '\n \n \tBy Another Author (3) and A U Thor (2)\n \tvia Another Committer\n+\n \t* left:\n \t Left #5\n \t Left #4\n@@ -240,6 +245,7 @@ test_expect_success 'fmt-merge-msg -m' '\n \n \tBy Another Author (3) and A U Thor (2)\n \tvia Another Committer\n+\n \t* ${apos}left${apos} of $(pwd):\n \t Left #5\n \t Left #4\n@@ -273,6 +279,7 @@ test_expect_success 'setup: expected shortlog for two branches' '\n \n \tBy Another Author (3) and A U Thor (2)\n \tvia Another Committer\n+\n \t* left:\n \t Left #5\n \t Left #4\n@@ -398,6 +405,7 @@ test_expect_success 'merge-msg two tags' '\n \n \tBy Another Author (3) and A U Thor (2)\n \tvia Another Committer\n+\n \t* tag ${apos}tag-l5${apos}:\n \t Left #5\n \t Left #4\n@@ -428,6 +436,7 @@ test_expect_success 'merge-msg tag and branch' '\n \n \tBy Another Author (3) and A U Thor (2)\n \tvia Another Committer\n+\n \t* left:\n \t Left #5\n \t Left #4\n-- \n1.7.10.1\n"},{"id":"191424","messageId":"7vipg2jpzp.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"20120511103122.GA19573@burratino","subject":"Re: [PATCH/RFC] fmt-merge-msg: add a blank line after people info","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-11T22:46:02Z","receivedAt":"2012-05-11T22:46:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Turns out my reaction is pretty much the same as before. I still like\n> the idea and still am distracted by the spacing. After the small\n> patch below, the log seems peaceful again and I am totally fine with\n> it.\n>\n> \tMerge branch for-upstream of git://git.example.com/subsystem\n>\n> \tBy Fred the Clown\n>\n> \t* for-upstream: (51 commits)\n> \t foo: bar baz\n\nTwo possible factors that may not be visible in the above example are\n\n (1) hand-written description of the merge itself by the integrator; and\n (2) octopus merges.\n\nWith both of these elements, with a blank line after the submaintainer and\nthe contributor attribution, a sample log output would look like this:\n\n Merge branches fix-foo and fix-bar of git://git.example.com/subsystem\n\n Two last minute fixes from Fred, so that we won't have to\n scramble and tell people to upgrade again immediately after\n the upcoming release.\n\n By Fred the Clown\n\n * fix-foo: (2 commits)\n foo: fix forboz\n foo: reindent\n\n By Fred the Clown\n\n * fix-bar: (1 commit)\n bar: fix nitfol\n\nThe attribution to the submaintainer and contributors, the name of the\nbranch merged, and the list of the individual changes form a single unit\nof information \"What was done for us by whom\". At least to me, the above\nis easier to see without the additional blank line; the even-spacing\nbefore and after the attribution line makes it harder to see where the\nboundary between \"description by the integrator on the merge\" and\n\"information on the work that was done on the first branch that was\nmerged\" is (and the boundary between the first and the second work, if in\nan octopus).\n\nWe could add another blank line before the \"credit\" line. We would have\ntwo blank lines that separates the integrator comment and the per-branch\nblock, and also have two blank lines between the per-branch blocks, making\nit easy again to see where the boundaries are.\n\nBut I do not know if it is an improvement from the current output before\nyour patch, or if it is just wasting vertical space.\n"},{"id":"191427","messageId":"CA+55aFyNqncpHydQJYjLu5d2y+gGLVQVV8zk=2ckZ-LLgRRNWA@mail.gmail.com","threadId":"29832","inReplyTo":"7vipg2jpzp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] fmt-merge-msg: add a blank line after people info","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-05-11T23:20:05Z","receivedAt":"2012-05-11T23:20:05Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Fri, May 11, 2012 at 3:46 PM, Junio C Hamano wrote:\n>\n> But I do not know if it is an improvement from the current output before\n> your patch, or if it is just wasting vertical space.\n\nSo I tend to edit out the \"By Xyzzy\" and \"via So-and-so\" lines,\nalthough I do find them useful for actually writing the message.\n\nHowever, if I were to keep them, I think I'd prefer the extra line\nthat Jonathan added.\n\nWhat I *would* like, though, is that the \"via So-and-so\" shows up on\nthe same line as the \"By Xyzzy\", probably together with appropriate\nword-wrapping (using strbuf_add_wrapped_text()?)\n\nBtw, the counting of commits is broken for the merge people. Do this\nin the kernel tree, just to see an example of the breakage:\n\n git checkout -b test-merges 59068e369b6a\n git merge e9e7183fd267\n\nand see the suggested people counts:\n\n By Mark Brown (2) and others\n via Takashi Iwai (3) and Liam Girdwood (1)\n\nand those \"via\" numbers make very little sense.\n\nSo the \"By\" number makes sense (counting non-merges). But the \"via\"\nnumbers are just odd. Seven of the commits were committed by Mark\nBrown, but he's not mentioned in \"via\". Presumably because he's\nalready mentioned in the \"By\" line. But Mark was actually *more* of a\n\"via\" person than Liam was, although Takashi is perhaps the most\nimportant one because he's the \"latest\" one things flowed through.\n\nI dunno. But it looks odd, and the above is not the only example of\n\"those counts don't make sense\".\n\nAnyway, I think the \"via\" line is odd the way it is. Lower-case \"via\"\nimplies that it's a continuation of the \"By\" sentence, but then it's\ngiven a line of its own. Hmm?\n\n Linus\n\n\n Linus\n"},{"id":"191517","messageId":"7vipfyhaxc.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"CA+55aFyNqncpHydQJYjLu5d2y+gGLVQVV8zk=2ckZ-LLgRRNWA@mail.gmail.com","subject":"Re: [PATCH/RFC] fmt-merge-msg: add a blank line after people info","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-14T18:31:11Z","receivedAt":"2012-05-14T18:31:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Btw, the counting of commits is broken for the merge people. Do this\n> in the kernel tree, just to see an example of the breakage:\n> ...\n> I dunno. But it looks odd, and the above is not the only example of\n> \"those counts don't make sense\".\n\n\"By\" numbers were meant to give credits to people who wrote the code, and\n\"via\" numbers were meant to give credits to people who helped usher code\nby others' to the person who is making the merge.\n\nThe former is something like \"git shortlog --no-merges -n -s ..MERGE_HEAD\"\nand is quite straightforward to compute. I didn't think things through\nfor the latter and punted with an ad-hoc algorithm that does not require\nus to traverse reachability when I wrote that code, I guess, and I suspect\nthat is what is causing the \"odd\" numbers.\n\nHere are some things that \"via\" should count as \"integrator's\ncontribution\":\n\n - making a commit authored by others (1 \"credit\" per such commit);\n\n - merging a branch that has commits authored by others (1 \"credit\" per\n commit authored by others brought in with such a merge).\n\nAnd here are some things that \"via\" should not count:\n\n - merging your own topic branches into one branch for the person who is\n making the (final) merge to pull;\n\n - merging backwards, pulling bunch of unrelated commits from upstream.\n\nFor that, I think the code needs to annotate each \"new\" commit that is\nbrought into the history by the (final) merge with \"how many others'\ncommits does it pull into the history\" number, or something. But I am\nstill in \"thinking aloud\" phase here, so...\n"},{"id":"191564","messageId":"7v62bxdwgs.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"7vipfyhaxc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] fmt-merge-msg: add a blank line after people info","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-15T20:24:03Z","receivedAt":"2012-05-15T20:24:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Linus Torvalds writes:\n>\n>> Btw, the counting of commits is broken for the merge people. Do this\n>> in the kernel tree, just to see an example of the breakage:\n>> ...\n>> I dunno. But it looks odd, and the above is not the only example of\n>> \"those counts don't make sense\".\n>\n> \"By\" numbers were meant to give credits to people who wrote the code, and\n> \"via\" numbers were meant to give credits to people who helped usher code\n> by others' to the person who is making the merge.\n\nI took a look at this again today.\n\nThe implementation you saw was written before I did any of the thinking\nbelow, and it merely counts the committer of merges plus the committer of\nthe tip commit you are pulling, or something. It is slightly better than\nrandom number generator, but not by a huge margin.\n\nHere is an outline of my current thinking to give a good definition for\nthe \"via\" number, which is supposed to give credits to lieutenants (and\npossibly sublieutenants).\n\nSuppose the history behind the tip commit you are pulling looked like\nthis:\n\n E-----E-----E-----E-----E\n \\\n A/D--A/D E\n \\ \\\n A/B---A/B----B-----B-----B-----C-----C-----C\n /\n A-----A-----A-----A\n\nwhere a commit denoted by a single letter (e.g. A on the bottom line) is\nauthored and committed by that person (by definition a merge is authored\nand committed by the same person), and a commit deonted as X/Y was\nauthored by X and committed by Y. You are responding to a pull request to\nintegrate the tip commit authored and committed by C into your history.\n\nThe contributor B helped by applying patches from contributor A (the\nleftmost two patches on the middle line), merging the work authored by A\nand vetted by D (the first merge on the middle line), and the work\nauthored by A (the second merge on the middle line). He even fixed things\nup with the rightmost commit in his history before asking C to pull. He\nshould get the credit for all this work to help getting A's changes to the\nhistory, including the two commits made by D (which owe credit to D as\nwell).\n\nFor the same reason why the two commits in D's history owe credits both to\nB and D, the whole thing owes \"via\" credit to C, as C was the lieutenant\nwho was ultimately responsible for delivering this result to you (in other\nwords, he could have decided not to pull from B).\n\nWhat I am thinking is for each commit X (not necessarily merges), count\nnon-merge commits that are:\n\n - reachable from X;\n - are being merged by the final merge;\n - not authored by the same author as X itself; and\n - have not been counted to give credit to the author of X yet. \n\nFor example, the first two commits by B on the middle line will give 2\ncredits (because B helped A's patch by applying them), the first merge by\nB on the middle line will give 2 credits (because it contributes another 2\ncommits by A via D to the final history) to B, the second merge will give\nanother 4 credits (commits on the bottom line) but not for the commits\nthat were already counted for his first merge. Total credit to B is 8 in\nthis example.\n\nThe merge made by C will *count* all 8 commits by A (even though they are\ncredited also to B), 1 commit by B (i.e. fix-up after merging 4 commit\nseries from A), and 6 commits by E. D gets 2 credits for having applied\ntwo patches from A. A and E will get no \"via\" credits.\n\nWhile I find the double-counting that appear in the example somewhat\ndisturbing, it inherently give larger credit to sub-lieutenant that is\ncloser to the tip, so it might after all match intuition.\n\nNow, computing this efficiently may not be trivial, as you would need N^2\nreachability analysis when pulling in N commits. Among 2000 recent merges\nI sampled from the kernel history, 70+ pull in more than 1000 commits (the\nlargest one d4bbf7e77 pulls in 21k commits).\n"},{"id":"191572","messageId":"CA+55aFyX66RpUA2Xh8drGErHB7Wuni20cUAMGawLsqG6MTTohA@mail.gmail.com","threadId":"29832","inReplyTo":"7v62bxdwgs.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] fmt-merge-msg: add a blank line after people info","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-05-16T02:02:23Z","receivedAt":"2012-05-16T02:02:23Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Tue, May 15, 2012 at 1:24 PM, Junio C Hamano wrote:\n>\n> Now, computing this efficiently may not be trivial, as you would need N^2\n> reachability analysis when pulling in N commits.  Among 2000 recent merges\n> I sampled from the kernel history, 70+ pull in more than 1000 commits (the\n> largest one d4bbf7e77 pulls in 21k commits).\n\nSo I have to say, for my purposes, it not only might be inefficient,\nbut it can still be very misleading.\n\nI actually care most about the person I personally pull from. And if\nhe is a submaintainer who has other submaintainers, it can be that\nfollowing the commit history doesn't show him at *all*. He might have\ndone just a fast-forward merge, but he's still the person *I* want to\ncredit.\n\nSo I'm getting the feeling that the \"count authors/committers\" may be\ncute, but it's not necessarily all that relevant. It's the kind of\ninformation you can see later from the git tree itself.\n\n(Admittedly, so it the shortlog we put in the merge, so that \"you can\nfind it later in the git tree itself\" not *that* great of an argument\n- the real argument for me is that it doesn't matter what you count,\nyou'll not necessarily get the actual piece of information I care\nabout..).\n\nSo I think it's somewhat interesting information, and I haven't really\ndisliked seeing it, but I have mostly edited it away (although I see\nthat some other maintainers also run modern versions of git and have\nleft it in place, so we do have those in the kernel).\n\nThe one merge commit of mine that has it, I edited it so that the \"via\nC Committer\" was on the same lne as the \"By A Author\" information.\n\nSo I'm not entirely convinced yet. I don't *dislike* the concept, but\nI could definitely do without it (or maybe have it in the commented\npart of the commit message, so that you'd have to explicitly edit it\nto show up).\n\n Linus\n"},{"id":"191595","messageId":"7vpqa4c9wr.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"CA+55aFyX66RpUA2Xh8drGErHB7Wuni20cUAMGawLsqG6MTTohA@mail.gmail.com","subject":"Re: [PATCH/RFC] fmt-merge-msg: add a blank line after people info","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-16T17:28:52Z","receivedAt":"2012-05-16T17:28:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> On Tue, May 15, 2012 at 1:24 PM, Junio C Hamano wrote:\n>>\n>> Now, computing this efficiently may not be trivial, as you would need N^2\n>> reachability analysis when pulling in N commits. Among 2000 recent merges\n>> I sampled from the kernel history, 70+ pull in more than 1000 commits (the\n>> largest one d4bbf7e77 pulls in 21k commits).\n>\n> So I have to say, for my purposes, it not only might be inefficient,\n> but it can still be very misleading.\n> ...\n> (Admittedly, so it the shortlog we put in the merge, so that \"you can\n> find it later in the git tree itself\" not *that* great of an argument\n> - the real argument for me is that it doesn't matter what you count,\n> you'll not necessarily get the actual piece of information I care\n> about..).\n\nThere is no way to mine \"X pulled from Y\" out of the topology if you allow\nfast-forward anywhere, so \"I care about whom I pulled from\" is something\npeople need to give out of band. Your lieutenant may have fast-forwarded\nhis history from his lieutenant's, or you may even be fast-forwarding your\nhistory from your lieutenant's, in which case you do not even have a merge\ncommit to record that fact anyway. If you do not fast-forward, at least\nthe merge subject would give you where you got the history from, so it is\nnot like that the information *must* be obtained by looking at the history\nanyway (incidentally, that was the reason why the \"better than random\nnumber generator but not by a large margin\" version gives an extra weight\nto the tip commit you are pulling; unless your lieutenant fast-forwarded,\nthat is the person you pulled from).\n\n> So I'm not entirely convinced yet. I don't *dislike* the concept, but\n> I could definitely do without it (or maybe have it in the commented\n> part of the commit message, so that you'd have to explicitly edit it\n> to show up).\n\nI am tempted to suggest removing the \"via\" part as a failed experiment for\nnow.\n"},{"id":"193011","messageId":"20120606202703.GA13641@burratino","threadId":"29832","inReplyTo":"7vipg2jpzp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] fmt-merge-msg: add a blank line after people info","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-06T20:27:03Z","receivedAt":"2012-06-06T20:27:03Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi again,\n\nJunio C Hamano wrote:\n> Jonathan Nieder writes:\n\n>> I still like\n>> the idea and still am distracted by the spacing. After the small\n>> patch below, the log seems peaceful again and I am totally fine with\n>> it.\n>>\n>> \tMerge branch for-upstream of git://git.example.com/subsystem\n>>\n>> \tBy Fred the Clown\n>>\n>> \t* for-upstream: (51 commits)\n>> \t foo: bar baz\n>\n> Two possible factors that may not be visible in the above example are\n>\n> (1) hand-written description of the merge itself by the integrator; and\n> (2) octopus merges.\n\nI've had way less time than I would like recently, but since a release\nseems to be approaching and nothing has happened, let's revisit this.\n\nTo summarize the previous discussion:\n\n * Hannes prefers not to see the By and Via info in the merge message\n at all.\n\n * Linus always reformats and paraphrases so the exact format is not\n too important for him. He has concerns about the accuracy of the\n Via line.\n\n * Jonathan thinks the By info is ok as part of a merge message but\n finds it jarring when pressed up against the --log summary.\n\n * Junio mentioned what sounds like anothing infelicity in the current\n format: for octopus merges, there is not just one list of authors\n and submaintiners summarizing what the merge commit does, but\n separate By and Via lines for each branch being merged.\n\nSince everyone seems to agree that it is best when the integrator\nsanity-checks the author info and rearranges it to taste, why not make\nit commented by default, for example like this?\n\n\tMerge branch for-upstream of git://git.example.com/subsystem\n\n\tFoo, bar, baz.\n\n\t#\n\t# By Fred the Clown\n\t#\n\t* for-upstream: (51 commits)\n\t qux: quux\n\nJonathan\n"},{"id":"193013","messageId":"20120606204659.GA13858@burratino","threadId":"29832","inReplyTo":"20120606202703.GA13641@burratino","subject":"Re: [PATCH/RFC] fmt-merge-msg: add a blank line after people info","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-06T20:46:59Z","receivedAt":"2012-06-06T20:46:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> * Junio mentioned what sounds like anothing infelicity in the current\n> format: for octopus merges, there is not just one list of authors\n> and submaintiners summarizing what the merge commit does, but\n> separate By and Via lines for each branch being merged.\n\nSorry for the lack of clarity. This should say\n\n\t ... another infelicity ...\n\t... submaintainers summarizing ...\n\nIn other words, I think the objection given to the patch upthread was\na real one, but that a more appropriate fix than using strange spacing\nwould be to combine statistics from all branches being merged.\n\nThanks for your thoughtfulness.\nJonathan\n"},{"id":"193016","messageId":"7vfwa8kuxh.fsf@alter.siamese.dyndns.org","threadId":"29832","inReplyTo":"20120606204659.GA13858@burratino","subject":"Re: [PATCH/RFC] fmt-merge-msg: add a blank line after people info","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-06T21:11:06Z","receivedAt":"2012-06-06T21:11:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> In other words, I think the objection given to the patch upthread was\n> a real one, but that a more appropriate fix than using strange spacing\n> would be to combine statistics from all branches being merged.\n\nIt may be real for _you_ but not for everybody. Especially, the\ncurrent spacing is a lot more logically consistent than your Fred\nthe Clown example, which *does* use much more strange spacing.\n\nI do not deeply care either way, so let's do this sometime when the\ntree is calm (but not today).\n\n builtin/fmt-merge-msg.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex bf93b04..2c4d435 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -286,10 +286,10 @@ static void credit_people(struct strbuf *out,\n \tconst char *me;\n \n \tif (kind == 'a') {\n-\t\tlabel = \"\\nBy \";\n+\t\tlabel = \"\\n# By \";\n \t\tme = git_author_info(IDENT_NO_DATE);\n \t} else {\n-\t\tlabel = \"\\nvia \";\n+\t\tlabel = \"\\n# Via \";\n \t\tme = git_committer_info(IDENT_NO_DATE);\n \t}\n \n"}]}