{"thread":{"id":"15998","subject":"[PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.","startedAt":"2008-10-21T20:55:57Z","lastAt":"2008-10-22T20:42:38Z","messageCount":13,"participants":["Alexander Gavrilov","Jeff King","Junio C Hamano","Jakub Narebski","Johannes Schindelin"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"93635","messageId":"1224622557-6669-1-git-send-email-angavrilov@gmail.com","threadId":"15998","inReplyTo":null,"subject":"[PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.","fromName":"Alexander Gavrilov","fromEmail":"angavrilov@gmail.com","sentAt":"2008-10-21T20:55:57Z","receivedAt":"2008-10-21T20:55:57Z","isPatch":true,"sender":{"key":"angavrilov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42666?v=4"},"body":"Currently git-blame outputs text from the commit messages\n(e.g. the author name and the summary string) as-is, without\neven providing any information about the encoding used for\nthe data. It makes interpreting the data in multilingual\nenvironment very difficult.\n\nThis commit changes the blame implementation to recode the\nmessages using the rules used by other commands like git-log.\nNamely, the target encoding can be specified through the\ni18n.commitEncoding or i18n.logOutputEncoding options, or\ndirectly on the command line using the --encoding parameter.\n\nConverting the encoding before output seems to be more\nfriendly to the porcelain tools than simply providing the\nvalue of the encoding header, and does not require changing\nthe output format.\n\nIf anybody needs the old behavior, it is possible to\nachieve it by specifying --encoding=none.\n\nSigned-off-by: Alexander Gavrilov <angavrilov@gmail.com>\n---\n\n\tI intentionally forced binary encoding for files\n\twith the localized strings, to avoid any possible\n\tcorruption.\n\t\n\t-- Alexander\n\n Documentation/blame-options.txt |    7 +++\n Documentation/i18n.txt          |    6 +-\n builtin-blame.c                 |   16 +++++--\n commit.h                        |    2 +\n pretty.c                        |   21 ++++++---\n t/t8005-blame-i18n.sh           |   92 +++++++++++++++++++++++++++++++++++++++\n t/t8005/cp1251.txt              |  Bin 0 -> 68 bytes\n t/t8005/sjis.txt                |  Bin 0 -> 100 bytes\n t/t8005/utf8.txt                |  Bin 0 -> 100 bytes\n 9 files changed, 130 insertions(+), 14 deletions(-)\n create mode 100755 t/t8005-blame-i18n.sh\n create mode 100644 t/t8005/cp1251.txt\n create mode 100644 t/t8005/sjis.txt\n create mode 100644 t/t8005/utf8.txt\n\ndiff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt\nindex 5428111..1ab1b96 100644\n--- a/Documentation/blame-options.txt\n+++ b/Documentation/blame-options.txt\n@@ -49,6 +49,13 @@ of lines before or after the line given by <start>.\n \tShow the result incrementally in a format designed for\n \tmachine consumption.\n \n+--encoding=<encoding>::\n+\tSpecifies the encoding used to output author names\n+\tand commit summaries. Setting it to `none` makes blame\n+\toutput unconverted data. For more information see the\n+\tdiscussion about encoding in the linkgit:git-log[1]\n+\tmanual page.\n+\n --contents <file>::\n \tWhen <rev> is not specified, the command annotates the\n \tchanges starting backwards from the working tree copy.\ndiff --git a/Documentation/i18n.txt b/Documentation/i18n.txt\nindex d2970f8..2cdacd9 100644\n--- a/Documentation/i18n.txt\n+++ b/Documentation/i18n.txt\n@@ -37,9 +37,9 @@ of `i18n.commitencoding` in its `encoding` header.  This is to\n help other people who look at them later.  Lack of this header\n implies that the commit log message is encoded in UTF-8.\n \n-. 'git-log', 'git-show' and friends looks at the `encoding`\n-  header of a commit object, and tries to re-code the log\n-  message into UTF-8 unless otherwise specified.  You can\n+. 'git-log', 'git-show', 'git-blame' and friends look at the\n+  `encoding` header of a commit object, and try to re-code the\n+  log message into UTF-8 unless otherwise specified.  You can\n   specify the desired output encoding with\n   `i18n.logoutputencoding` in `.git/config` file, like this:\n +\ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex 48cc0c1..2457e71 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -1431,7 +1431,7 @@ static void get_commit_info(struct commit *commit,\n \t\t\t    int detailed)\n {\n \tint len;\n-\tchar *tmp, *endp;\n+\tchar *tmp, *endp, *reencoded, *message;\n \tstatic char author_buf[1024];\n \tstatic char committer_buf[1024];\n \tstatic char summary_buf[1024];\n@@ -1449,24 +1449,29 @@ static void get_commit_info(struct commit *commit,\n \t\t\tdie(\"Cannot read commit %s\",\n \t\t\t    sha1_to_hex(commit->object.sha1));\n \t}\n+\treencoded = reencode_commit_message(commit, NULL);\n+\tmessage   = reencoded ? reencoded : commit->buffer;\n \tret->author = author_buf;\n-\tget_ac_line(commit->buffer, \"\\nauthor \",\n+\tget_ac_line(message, \"\\nauthor \",\n \t\t    sizeof(author_buf), author_buf, &ret->author_mail,\n \t\t    &ret->author_time, &ret->author_tz);\n \n-\tif (!detailed)\n+\tif (!detailed) {\n+\t\tfree(reencoded);\n \t\treturn;\n+\t}\n \n \tret->committer = committer_buf;\n-\tget_ac_line(commit->buffer, \"\\ncommitter \",\n+\tget_ac_line(message, \"\\ncommitter \",\n \t\t    sizeof(committer_buf), committer_buf, &ret->committer_mail,\n \t\t    &ret->committer_time, &ret->committer_tz);\n \n \tret->summary = summary_buf;\n-\ttmp = strstr(commit->buffer, \"\\n\\n\");\n+\ttmp = strstr(message, \"\\n\\n\");\n \tif (!tmp) {\n \terror_out:\n \t\tsprintf(summary_buf, \"(%s)\", sha1_to_hex(commit->object.sha1));\n+\t\tfree(reencoded);\n \t\treturn;\n \t}\n \ttmp += 2;\n@@ -1478,6 +1483,7 @@ static void get_commit_info(struct commit *commit,\n \t\tgoto error_out;\n \tmemcpy(summary_buf, tmp, len);\n \tsummary_buf[len] = 0;\n+\tfree(reencoded);\n }\n \n /*\ndiff --git a/commit.h b/commit.h\nindex 4c05864..3a7b06a 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -65,6 +65,8 @@ enum cmit_fmt {\n \n extern int non_ascii(int);\n struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */\n+extern char *reencode_commit_message(const struct commit *commit,\n+\t\t\t\t     const char **encoding_p);\n extern void get_commit_format(const char *arg, struct rev_info *);\n extern void format_commit_message(const struct commit *commit,\n \t\t\t\t  const void *format, struct strbuf *sb,\ndiff --git a/pretty.c b/pretty.c\nindex 1e79943..f6ff312 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -783,6 +783,20 @@ void pp_remainder(enum cmit_fmt fmt,\n \t}\n }\n \n+char *reencode_commit_message(const struct commit *commit, const char **encoding_p)\n+{\n+\tconst char *encoding;\n+\n+\tencoding = (git_log_output_encoding\n+\t\t    ? git_log_output_encoding\n+\t\t    : git_commit_encoding);\n+\tif (!encoding)\n+\t\tencoding = \"utf-8\";\n+\tif (encoding_p)\n+\t\t*encoding_p = encoding;\n+\treturn logmsg_reencode(commit, encoding);\n+}\n+\n void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,\n \t\t\t struct strbuf *sb, int abbrev,\n \t\t\t const char *subject, const char *after_subject,\n@@ -799,12 +813,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,\n \t\treturn;\n \t}\n \n-\tencoding = (git_log_output_encoding\n-\t\t    ? git_log_output_encoding\n-\t\t    : git_commit_encoding);\n-\tif (!encoding)\n-\t\tencoding = \"utf-8\";\n-\treencoded = logmsg_reencode(commit, encoding);\n+\treencoded = reencode_commit_message(commit, &encoding);\n \tif (reencoded) {\n \t\tmsg = reencoded;\n \t}\ndiff --git a/t/t8005-blame-i18n.sh b/t/t8005-blame-i18n.sh\nnew file mode 100755\nindex 0000000..4470a92\n--- /dev/null\n+++ b/t/t8005-blame-i18n.sh\n@@ -0,0 +1,92 @@\n+#!/bin/sh\n+\n+test_description='git blame encoding conversion'\n+. ./test-lib.sh\n+\n+. \"$TEST_DIRECTORY\"/t8005/utf8.txt\n+. \"$TEST_DIRECTORY\"/t8005/cp1251.txt\n+. \"$TEST_DIRECTORY\"/t8005/sjis.txt\n+\n+test_expect_success 'setup the repository' '\n+\t# Create the file\n+\techo \"UTF-8 LINE\" > file &&\n+\tgit add file &&\n+\tgit commit --author \"$UTF8_NAME <utf8@localhost>\" -m \"$UTF8_MSG\" &&\n+\n+\techo \"CP1251 LINE\" >> file &&\n+\tgit add file &&\n+\tgit config i18n.commitencoding cp1251 &&\n+\tgit commit --author \"$CP1251_NAME <cp1251@localhost>\" -m \"$CP1251_MSG\" &&\n+\n+\techo \"SJIS LINE\" >> file &&\n+\tgit add file &&\n+\tgit config i18n.commitencoding shift-jis &&\n+\tgit commit --author \"$SJIS_NAME <sjis@localhost>\" -m \"$SJIS_MSG\"\n+'\n+\n+cat >expected <<EOF\n+author $SJIS_NAME\n+summary $SJIS_MSG\n+author $SJIS_NAME\n+summary $SJIS_MSG\n+author $SJIS_NAME\n+summary $SJIS_MSG\n+EOF\n+\n+test_expect_success \\\n+\t'blame respects i18n.commitencoding' '\n+\tgit blame --incremental file | \\\n+\t\tgrep \"^\\(author\\|summary\\) \" > actual &&\n+\ttest_cmp actual expected\n+'\n+\n+cat >expected <<EOF\n+author $CP1251_NAME\n+summary $CP1251_MSG\n+author $CP1251_NAME\n+summary $CP1251_MSG\n+author $CP1251_NAME\n+summary $CP1251_MSG\n+EOF\n+\n+test_expect_success \\\n+\t'blame respects i18n.logoutputencoding' '\n+\tgit config i18n.logoutputencoding cp1251 &&\n+\tgit blame --incremental file | \\\n+\t\tgrep \"^\\(author\\|summary\\) \" > actual &&\n+\ttest_cmp actual expected\n+'\n+\n+cat >expected <<EOF\n+author $UTF8_NAME\n+summary $UTF8_MSG\n+author $UTF8_NAME\n+summary $UTF8_MSG\n+author $UTF8_NAME\n+summary $UTF8_MSG\n+EOF\n+\n+test_expect_success \\\n+\t'blame respects --encoding=utf-8' '\n+\tgit blame --incremental --encoding=utf-8 file | \\\n+\t\tgrep \"^\\(author\\|summary\\) \" > actual &&\n+\ttest_cmp actual expected\n+'\n+\n+cat >expected <<EOF\n+author $SJIS_NAME\n+summary $SJIS_MSG\n+author $CP1251_NAME\n+summary $CP1251_MSG\n+author $UTF8_NAME\n+summary $UTF8_MSG\n+EOF\n+\n+test_expect_success \\\n+\t'blame respects --encoding=none' '\n+\tgit blame --incremental --encoding=none file | \\\n+\t\tgrep \"^\\(author\\|summary\\) \" > actual &&\n+\ttest_cmp actual expected\n+'\n+\n+test_done\ndiff --git a/t/t8005/cp1251.txt b/t/t8005/cp1251.txt\nnew file mode 100644\nindex 0000000000000000000000000000000000000000..ce41e98b811b09c135115529b41e227ce5ec4ff6\nGIT binary patch\nliteral 68\nzcmZ<`Ff=kXjQ4Z&b+uJG@#w)@h4W88eR%ii#dn2^FP^*uvXr=-A!>Ys-EEaFJ^lFU\nW-J^F;6+XUu_weV_w=bS5aRC6=1t)(1\n\nliteral 0\nHcmV?d00001\n\ndiff --git a/t/t8005/sjis.txt b/t/t8005/sjis.txt\nnew file mode 100644\nindex 0000000000000000000000000000000000000000..2ccfbad207c6e96b1f4f528031d9e4938d364b92\nGIT binary patch\nliteral 100\nzcmWIc@(hmmbM$q!Rci5UDQYQbsZ(ePXen)JX=!R{018yLbSkt20jUxo7c8X26%5kk\nk8|)6$6AV<^3{(tK+R##}0OT|PVPQ)*P@)c~tyGB%00x;U@Bjb+\n\nliteral 0\nHcmV?d00001\n\ndiff --git a/t/t8005/utf8.txt b/t/t8005/utf8.txt\nnew file mode 100644\nindex 0000000000000000000000000000000000000000..f46cfc56d80797740c3ec15e166add052f905fcb\nGIT binary patch\nliteral 100\nzcmWFyakGf`bM$q!Rk|?a!lnxwF6>pfF#p2Vi%l0BF6;ve?6}yjaADzv9T&D-*as0(\nt;tB<6@(p$e>RAL-+IX=EtaRUntqK<#fy{juHeT$!u=T=Tpth|_TmU(XJDmUk\n\nliteral 0\nHcmV?d00001\n\n-- \n1.6.0.20.g6148bc\n"},{"id":"93658","messageId":"20081022082016.GA18473@sigill.intra.peff.net","threadId":"15998","inReplyTo":"1224622557-6669-1-git-send-email-angavrilov@gmail.com","subject":"Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-10-22T08:20:17Z","receivedAt":"2008-10-22T08:20:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 22, 2008 at 12:55:57AM +0400, Alexander Gavrilov wrote:\n\n> +test_expect_success \\\n> +\t'blame respects i18n.commitencoding' '\n> +\tgit blame --incremental file | \\\n> +\t\tgrep \"^\\(author\\|summary\\) \" > actual &&\n> +\ttest_cmp actual expected\n\nEven though it is POSIX, using backslashed grouping in 'grep' isn't\nportable. It fails at least on Solaris 8, and you have to do:\n\n  egrep \"^(author|summary) \"\n\ninstead. Of course, I can't get your test to pass even with that change,\nbut I think that is just a broken iconv on Solaris.\n\n-Peff\n"},{"id":"93665","messageId":"bb6f213e0810220135hd520f78odb83414c248a15e4@mail.gmail.com","threadId":"15998","inReplyTo":"20081022082016.GA18473@sigill.intra.peff.net","subject":"Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.","fromName":"Alexander Gavrilov","fromEmail":"angavrilov@gmail.com","sentAt":"2008-10-22T08:35:17Z","receivedAt":"2008-10-22T08:35:17Z","isPatch":true,"sender":{"key":"angavrilov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42666?v=4"},"body":"On Wed, Oct 22, 2008 at 12:20 PM, Jeff King <peff@peff.net> wrote:\n> On Wed, Oct 22, 2008 at 12:55:57AM +0400, Alexander Gavrilov wrote:\n>\n>> +test_expect_success \\\n>> +     'blame respects i18n.commitencoding' '\n>> +     git blame --incremental file | \\\n>> +             grep \"^\\(author\\|summary\\) \" > actual &&\n>> +     test_cmp actual expected\n>\n> Even though it is POSIX, using backslashed grouping in 'grep' isn't\n> portable. It fails at least on Solaris 8, and you have to do:\n>\n>  egrep \"^(author|summary) \"\n>\n> instead. Of course, I can't get your test to pass even with that change,\n> but I think that is just a broken iconv on Solaris.\n\nHm, which of the conversions fail? You can try manually converting the\ntxt files with the iconv command to figure it out.\n\nAlexander\n"},{"id":"93668","messageId":"20081022085036.GA1454@sigill.intra.peff.net","threadId":"15998","inReplyTo":"bb6f213e0810220135hd520f78odb83414c248a15e4@mail.gmail.com","subject":"Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-10-22T08:50:37Z","receivedAt":"2008-10-22T08:50:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 22, 2008 at 12:35:17PM +0400, Alexander Gavrilov wrote:\n\n> > instead. Of course, I can't get your test to pass even with that change,\n> > but I think that is just a broken iconv on Solaris.\n> \n> Hm, which of the conversions fail? You can try manually converting the\n> txt files with the iconv command to figure it out.\n\nTest 2 fails for CP1251 and SJIS.  Manually converting with iconv seems\nto work ok (at least, doing \"iconv -f CP1251 -t UTF-8 cp1251.txt\"\nproduces the same output as \"cat utf8.txt\".\n\nThe output from \"t8005-blame-i18n.sh -v -i\" is attached (as\napplication/octet-stream to protect it from email munging) if that is\nhelpful to you. I can't make heads or tails of it.\n\n-Peff\n"},{"id":"93671","messageId":"bb6f213e0810220232p2e7fe3daoa32f46fbe5bef2ac@mail.gmail.com","threadId":"15998","inReplyTo":"20081022085036.GA1454@sigill.intra.peff.net","subject":"Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.","fromName":"Alexander Gavrilov","fromEmail":"angavrilov@gmail.com","sentAt":"2008-10-22T09:32:56Z","receivedAt":"2008-10-22T09:32:56Z","isPatch":true,"sender":{"key":"angavrilov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42666?v=4"},"body":"On Wed, Oct 22, 2008 at 12:50 PM, Jeff King <peff@peff.net> wrote:\n> On Wed, Oct 22, 2008 at 12:35:17PM +0400, Alexander Gavrilov wrote:\n>\n>> > instead. Of course, I can't get your test to pass even with that change,\n>> > but I think that is just a broken iconv on Solaris.\n>>\n>> Hm, which of the conversions fail? You can try manually converting the\n>> txt files with the iconv command to figure it out.\n>\n> Test 2 fails for CP1251 and SJIS.  Manually converting with iconv seems\n> to work ok (at least, doing \"iconv -f CP1251 -t UTF-8 cp1251.txt\"\n> produces the same output as \"cat utf8.txt\".\n>\n> The output from \"t8005-blame-i18n.sh -v -i\" is attached (as\n> application/octet-stream to protect it from email munging) if that is\n> helpful to you. I can't make heads or tails of it.\n\n>From the output it seems that what fails is \"-f cp1251 -t sjis\" and\n\"-f utf-8 -t sjis\" (remember that blame --incremental produces its\noutput in reverse order compared to the order of commits). Unless\nblame for some reason actually tries to convert to an entirely\ndifferent encoding. If tests 4 and 5 pass, you can try adding\n--encoding=shift-jis to test 2 to check it.\n\nAlexander\n"},{"id":"93679","messageId":"20081022123926.GA7774@coredump.intra.peff.net","threadId":"15998","inReplyTo":"bb6f213e0810220232p2e7fe3daoa32f46fbe5bef2ac@mail.gmail.com","subject":"Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-10-22T12:39:27Z","receivedAt":"2008-10-22T12:39:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 22, 2008 at 01:32:56PM +0400, Alexander Gavrilov wrote:\n\n> From the output it seems that what fails is \"-f cp1251 -t sjis\" and\n> \"-f utf-8 -t sjis\" (remember that blame --incremental produces its\n\nOh, indeed. Converting to and from utf-8 seems to work, but not between\ncp1251 and sjis:\n\n  $ iconv -f SJIS -t UTF-8 sjis.txt | sed s/SJIS/UTF8/ >test &&\n  >   cmp test utf8.txt && echo ok\n  ok\n\n  $ iconv -f UTF-8 -t SJIS utf8.txt | sed s/UTF8/SJIS/ >test &&\n  >   cmp test sjis.txt && echo ok\n  ok\n\n  $ iconv -f SJIS -t CP1251 sjis.txt\n  Not supported SJIS to CP1251\n\n  $ iconv -f CP1251 -t SJIS cp1251.txt\n  Not supported CP1251 to SJIS\n\nSo I think it is simply a limitation of the platform with respect to the\nparticular encodings used. There is a similar problem in t3900, I think,\nwith EUCJP and ISO-2022-JP. I assume one _can_ install these encodings\nfor Solaris, but the admins of my box haven't done so (and this is a\nproduction box that I can't ask for such things on).\n\n> blame for some reason actually tries to convert to an entirely\n> different encoding. If tests 4 and 5 pass, you can try adding\n> --encoding=shift-jis to test 2 to check it.\n\nTests 2, 3, and 4 fail. Test 5 does pass.\n\n-Peff\n"},{"id":"93706","messageId":"7vd4hsv46z.fsf@gitster.siamese.dyndns.org","threadId":"15998","inReplyTo":"20081022082016.GA18473@sigill.intra.peff.net","subject":"Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-22T19:07:48Z","receivedAt":"2008-10-22T19:07:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Wed, Oct 22, 2008 at 12:55:57AM +0400, Alexander Gavrilov wrote:\n>\n>> +test_expect_success \\\n>> +\t'blame respects i18n.commitencoding' '\n>> +\tgit blame --incremental file | \\\n>> +\t\tgrep \"^\\(author\\|summary\\) \" > actual &&\n>> +\ttest_cmp actual expected\n>\n> Even though it is POSIX, using backslashed grouping in 'grep' isn't\n> portable. It fails at least on Solaris 8, and you have to do:\n>\n>   egrep \"^(author|summary) \"\n>\n> instead. Of course, I can't get your test to pass even with that change,\n> but I think that is just a broken iconv on Solaris.\n\nYuck.  Solaris 8 /usr/bin/grep does not even grok \"-e\", so we cannot do a\nmore obvious:\n\n\tgrep -e \"^author \" -e \"^summary \"\n\nDo people build with NO_EXTERNAL_GREP on older Solaris?\n\ngit-submodule.sh uses grep \"-e\" to look for two patterns and I suspect\nolder Solaris would have the same issue.\n"},{"id":"93708","messageId":"20081022191415.GA31568@coredump.intra.peff.net","threadId":"15998","inReplyTo":"7vd4hsv46z.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-10-22T19:14:16Z","receivedAt":"2008-10-22T19:14:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 22, 2008 at 12:07:48PM -0700, Junio C Hamano wrote:\n\n> Yuck.  Solaris 8 /usr/bin/grep does not even grok \"-e\", so we cannot do a\n> more obvious:\n> \n> \tgrep -e \"^author \" -e \"^summary \"\n\nYep. I already introduced one use of egrep for a similar case in\n8753941 (tests: grep portability fixes).\n\n> Do people build with NO_EXTERNAL_GREP on older Solaris?\n\nYep. See:\n\n  http://repo.or.cz/w/git/gitbuild.git?a=blob;f=jk/solaris/config.mak;hb=platform\n\nfor the gory details (boy, I wish we had nice PATH_INFO-based gitweb\nURLs...).\n\n> git-submodule.sh uses grep \"-e\" to look for two patterns and I suspect\n> older Solaris would have the same issue.\n\nYes, that code will break on Solaris. Most of my portability fixes have\nbeen in direct response to tests, so I guess we are not testing\ngit-submodule very well.\n\n-Peff\n"},{"id":"93711","messageId":"20081022192253.GC31568@coredump.intra.peff.net","threadId":"15998","inReplyTo":"20081022191415.GA31568@coredump.intra.peff.net","subject":"Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-10-22T19:22:53Z","receivedAt":"2008-10-22T19:22:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 22, 2008 at 03:14:16PM -0400, Jeff King wrote:\n\n> > git-submodule.sh uses grep \"-e\" to look for two patterns and I suspect\n> > older Solaris would have the same issue.\n> \n> Yes, that code will break on Solaris. Most of my portability fixes have\n> been in direct response to tests, so I guess we are not testing\n> git-submodule very well.\n\nAnd here's a patch. Though I believe this is the last \"grep -e\", I\nwonder if it wouldn't have been wiser to simply force people on such\nplatforms to use GNU grep (I already have to use GNU tools to build, and\nbash to run the scripts).\n\n-- >8 --\nsubmodule: fix some non-portable grep invocations\n\nNot all greps support \"-e\", but in this case we can easily\nconvert it to a single extended regex.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nPasses the test scripts, but I'm not sure they are exercising this code,\nanyway, since it passed on Solaris. Please double-check my conversion.\n\n git-submodule.sh |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 65178ae..b63e5c3 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -424,7 +424,7 @@ cmd_summary() {\n \tcd_to_toplevel\n \t# Get modified modules cared by user\n \tmodules=$(git diff-index $cached --raw $head -- \"$@\" |\n-\t\tgrep -e '^:160000' -e '^:[0-7]* 160000' |\n+\t\tegrep '^:([0-7]* )?160000' |\n \t\twhile read mod_src mod_dst sha1_src sha1_dst status name\n \t\tdo\n \t\t\t# Always show modules deleted or type-changed (blob<->module)\n@@ -438,7 +438,7 @@ cmd_summary() {\n \ttest -z \"$modules\" && return\n \n \tgit diff-index $cached --raw $head -- $modules |\n-\tgrep -e '^:160000' -e '^:[0-7]* 160000' |\n+\tegrep '^:([0-7]* )?160000' |\n \tcut -c2- |\n \twhile read mod_src mod_dst sha1_src sha1_dst status name\n \tdo\n-- \n1.6.0.2.825.g6d19d\n"},{"id":"93713","messageId":"gdo1a3$akl$1@ger.gmane.org","threadId":"15998","inReplyTo":"20081022191415.GA31568@coredump.intra.peff.net","subject":"Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-10-22T20:12:54Z","receivedAt":"2008-10-22T20:12:54Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jeff King wrote:\n\n>> Do people build with NO_EXTERNAL_GREP on older Solaris?\n> \n> Yep. See:\n> \n>   http://repo.or.cz/w/git/gitbuild.git?a=blob;f=jk/solaris/config.mak;hb=platform\n> \n> for the gory details (boy, I wish we had nice PATH_INFO-based gitweb\n> URLs...).\n\nCurrently you can use path_info URL for blob_plain\n\n  http://repo.or.cz/w/git/gitbuild.git/platform:/jk/solaris/config.mak\n\nSoon (thanks to Giuseppe patches) you would be able to use\n\n  http://repo.or.cz/w/git/gitbuild.git/blob/platform:/jk/solaris/config.mak\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"93714","messageId":"alpine.DEB.1.00.0810222220410.22125@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"15998","inReplyTo":"20081022192253.GC31568@coredump.intra.peff.net","subject":"Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-10-22T20:29:39Z","receivedAt":"2008-10-22T20:29:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 22 Oct 2008, Jeff King wrote:\n\n> submodule: fix some non-portable grep invocations\n> \n> Not all greps support \"-e\", but in this case we can easily\n> convert it to a single extended regex.\n\nI really wonder if we cannot catch these things (unportable grep, sed, etc \ninvokations) with a simple patch to the pre-commit hook.\n\nCiao,\nDscho\n"},{"id":"93724","messageId":"20081022204030.GB4585@coredump.intra.peff.net","threadId":"15998","inReplyTo":"alpine.DEB.1.00.0810222220410.22125@pacific.mpi-cbg.de.mpi-cbg.de","subject":"Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-10-22T20:40:31Z","receivedAt":"2008-10-22T20:40:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 22, 2008 at 10:29:39PM +0200, Johannes Schindelin wrote:\n\n> > Not all greps support \"-e\", but in this case we can easily\n> > convert it to a single extended regex.\n> \n> I really wonder if we cannot catch these things (unportable grep, sed, etc \n> invokations) with a simple patch to the pre-commit hook.\n\nWe could probably write a hook for some of the simpler ones, but we\nwould have quite a few false negatives, I suspect.\n\nFWIW, I am not finding these portability problems by hand. I am nightly\nauto-building and testing Junio's maint, master, and next on Solaris 8\nand FreeBSD, and Mike Ralphson is doing the same for AIX. So while we\ncould perhaps catch them sooner, I am very happy to have caught several\nissues recently in next, _before_ they hit master.\n\n-Peff\n"},{"id":"93726","messageId":"20081022204238.GC4585@coredump.intra.peff.net","threadId":"15998","inReplyTo":"gdo1a3$akl$1@ger.gmane.org","subject":"Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-10-22T20:42:38Z","receivedAt":"2008-10-22T20:42:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 22, 2008 at 10:12:54PM +0200, Jakub Narebski wrote:\n\n> Currently you can use path_info URL for blob_plain\n> \n>   http://repo.or.cz/w/git/gitbuild.git/platform:/jk/solaris/config.mak\n\nAh, I didn't know that. Thanks.\n\n> Soon (thanks to Giuseppe patches) you would be able to use\n> \n>   http://repo.or.cz/w/git/gitbuild.git/blob/platform:/jk/solaris/config.mak\n\nYes, those patches were actually what spurred my comment. :) I am\nlooking forward to playing with them once they are on repo.or.cz.\n\n-Peff\n"}]}