{"thread":{"id":"34963","subject":"[PATCH] shortlog: ignore commits with missing authors","startedAt":"2013-09-18T21:14:00Z","lastAt":"2013-09-18T21:14:00Z","messageCount":1,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"227867","messageId":"20130918211400.GA23856@sigill.intra.peff.net","threadId":"34963","inReplyTo":null,"subject":"[PATCH] shortlog: ignore commits with missing authors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-18T21:14:00Z","receivedAt":"2013-09-18T21:14:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nMost of git's traversals are robust against minor breakages\nin commit data. For example, \"git log\" will still output an\nentry for a commit that has a broken encoding or missing\nauthor, and will not abort the whole operation.\n\nShortlog, on the other hand, will die as soon as it sees a\ncommit without an author, meaning that a repository with\na broken commit cannot get any shortlog output at all.\n\nLet's downgrade this fatal error to a warning, and continue\nthe operation.\n\nWe simply ignore the commit and do not count it in the total\n(since we do not have any author under which to file it).\nAlternatively, we could output some kind of \"<empty>\" record\nto collect these bogus commits. It is probably not worth it,\nthough; we have already warned to stderr, so the user is\naware that such bogosities exist, and any placeholder we\ncame up with would either be syntactically invalid, or would\npotentially conflict with real data.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThe real-world breakage I saw that caused this was not a missing author\nline, but rather a commit that was in proper utf8, but whose encoding\nheader said \"utf16\". We can tweak the test to reproduce the exact case\nby making the sed line:\n\n  sed \"/^\\$/iencoding utf16\" commit.tmp >broken.tmp\n\nBut I think that may be less portable, as systems with iconv that does\nnot understand \"utf16\" might leave the text intact. Simply removing the\nauthor line, as the test below does, has the same effect.\n\n builtin/shortlog.c  |  6 ++++--\n t/t4201-shortlog.sh | 16 ++++++++++++++++\n 2 files changed, 20 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex ae73d17..c226f76 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -127,9 +127,11 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \t\t\tauthor = buffer + 7;\n \t\tbuffer = eol;\n \t}\n-\tif (!author)\n-\t\tdie(_(\"Missing author: %s\"),\n+\tif (!author) {\n+\t\twarning(_(\"Missing author: %s\"),\n \t\t    sha1_to_hex(commit->object.sha1));\n+\t\treturn;\n+\t}\n \tif (log->user_format) {\n \t\tstruct pretty_print_context ctx = {0};\n \t\tctx.fmt = CMIT_FMT_USERFORMAT;\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 5493500..4286699 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -172,4 +172,20 @@ test_cmp expect out'\n \tgit shortlog HEAD~2.. > out &&\n test_cmp expect out'\n \n+test_expect_success 'shortlog ignores commits with missing authors' '\n+\tgit commit --allow-empty -m normal &&\n+\tgit commit --allow-empty -m soon-to-be-broken &&\n+\tgit cat-file commit HEAD >commit.tmp &&\n+\tsed \"/^author/d\" commit.tmp >broken.tmp &&\n+\tcommit=$(git hash-object -w -t commit --stdin <broken.tmp) &&\n+\tgit update-ref HEAD $commit &&\n+\tcat >expect <<-\\EOF &&\n+\tA U Thor (1):\n+\t      normal\n+\n+\tEOF\n+\tgit shortlog HEAD~2.. >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.8.4.rc4.16.g228394f\n"}]}