{"thread":{"id":"39630","subject":"Should the --encoding argument to log/show commands make any guarantees about their output?","startedAt":"2015-06-15T08:50:26Z","lastAt":"2015-06-17T20:02:09Z","messageCount":9,"participants":["Jan-Philip Gehrcke","Torsten Bögershausen","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"263839","messageId":"557E91D2.3000908@googlemail.com","threadId":"39630","inReplyTo":null,"subject":"Should the --encoding argument to log/show commands make any guarantees about their output?","fromName":"Jan-Philip Gehrcke","fromEmail":"jgehrcke@googlemail.com","sentAt":"2015-06-15T08:50:26Z","receivedAt":"2015-06-15T08:50:26Z","isPatch":false,"sender":{"key":"jgehrcke@googlemail.com","avatar":null},"body":"Hello,\n\nI was surprised to see that the output of\n\n     git log --encoding=utf-8 \"--format=format:%b\"\n\ncan contain byte sequences that are invalid in UTF-8. Note: I am using \ngit 2.1.4 and the %b format specifier represents the commit message body.\n\nI have seen this with the Linux git repository and the following test:\n\n     git log --encoding=utf-8 \"--format=format:%b\" | python2 -c \\\n         'import sys; [l.decode(\"utf-8\") for l in sys.stdin]'\n\nSoon enough errors like this appears:\n\n     'utf8' codec can't decode byte 0xf6 in position 19\n\nThe help message to the --encoding argument reads:\n\n> The commit objects record the encoding used for the log message in\n> their encoding header; this option can be used to tell the command to\n> re-code the commit log message in the encoding preferred by the user\n\nI realize that this message does not give any guarantee about the output \nof the command, in the sense that --encoding=utf-8 produces valid UTF-8 \ndata in all cases.\n\nHowever, I wonder what --encoding precisely does and if it has the \nbehavior most users would expect.\n\nLet me describe what I think it currently does:\n\nThe program attempts to re-code a log message, so it follows the chain\n\n\traw input -> unicode -> raw output\n\nFor the first step, knowledge about the input encoding is required. This \nis retrieved from the encoding header of the commit object if present or \n(from the docs) \"lack of this header implies that the commit log message \nis encoded in UTF-8.\" If this step fails (if the entry contains a byte \nsequence that is invalid in the specified/assumed input codec), the \nprocedure is aborted and the data is dumped as is (obviously without \napplying the requested output encoding).\n\nIs that correct?\n\n From my point of view the most natural abstraction of a log *message* \nis *text*, not bytes. The same is true for author names. If I want to \nbuild a tool chain on top of log/show, this usually means that I want to \nwork with text information. Hence, I want to retrieve text (a sequence \nof code points) from git show/log. Text must be transported in encoded \nform, sure, but it must not contain byte sequences that are invalid in \nthis codec. Because otherwise it's just not text anymore.\n\nHence, from my point of view, the rational that git show/log should be \nable to output *text* information means that they should not emit byte \nsequences that are invalid in the codec specified via the --encoding \nargument. In the current situation, the work of dealing with invalid \nbyte sequences is just outsourced to software further below in the tool \nchain (at some point a replacement character � should be displayed to \nthe user instead of the invalid raw bytes).\n\nI am not entirely sure where this discussion should lead to. However, I \nthink that if the behavior of the software will not be changed, then the \ndocumentation for the --encoding option should be more precise and \nclarify what actually happens behind the scenes. What do you think?\n\n\nCheers,\n\n\nJan-Philip Gehrcke\n"},{"id":"263864","messageId":"557EFB94.3040104@web.de","threadId":"39630","inReplyTo":"557E91D2.3000908@googlemail.com","subject":"Re: Should the --encoding argument to log/show commands make any guarantees about their output?","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-06-15T16:21:40Z","receivedAt":"2015-06-15T16:21:40Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2015-06-15 10.50, Jan-Philip Gehrcke wrote:\n> Hello,\n> \n> I was surprised to see that the output of\n> \n>     git log --encoding=utf-8 \"--format=format:%b\"\n> \n> can contain byte sequences that are invalid in UTF-8. Note: I am using git 2.1.4 and the %b format specifier represents the commit message body.\n> \n> I have seen this with the Linux git repository and the following test:\n> \n>     git log --encoding=utf-8 \"--format=format:%b\" | python2 -c \\\n>         'import sys; [l.decode(\"utf-8\") for l in sys.stdin]'\n> \n> Soon enough errors like this appears:\n> \n>     'utf8' codec can't decode byte 0xf6 in position 19\n> \n> The help message to the --encoding argument reads:\n> \n>> The commit objects record the encoding used for the log message in\n>> their encoding header; this option can be used to tell the command to\n>> re-code the commit log message in the encoding preferred by the user\n> \n> I realize that this message does not give any guarantee about the output of the command, in the sense that --encoding=utf-8 produces valid UTF-8 data in all cases.\n> \n> However, I wonder what --encoding precisely does and if it has the behavior most users would expect.\n> \n> Let me describe what I think it currently does:\n> \n> The program attempts to re-code a log message, so it follows the chain\n> \n>     raw input -> unicode -> raw output\nNot sure what \"raw input/output\" means.\nBut there is only one reencode step involved, e.g.\ninput(8859) -> output(UTF-8)\nWhen the encoding of the commit message is undefined, UTF-8 is assumed.\nBut Git does no verify if the encoding is really UTF-8.\nWe could guess that if it is not UTF-8 then it is ISO-8859-1, but that is not implemented.\n\n> \n> For the first step, knowledge about the input encoding is required. \nWhen someone does a commit where the commit message does not conform to UTF-8,\nThis message is shown from Git:\n\"Warning: commit message did not conform to UTF-8.\\n\"\n\"You may want to amend it after fixing the message, or set the config\\n\"\n\"variable i18n.commitencoding to the encoding your project uses.\\n\";\n\nIf the user ignores this warning, how should Git guess the encoding  ?\n(Later Git versions try do an auto-conversion assuming ISO-8859-1) ,\nbut that doesn't help real existing repos.\n\n> This is retrieved from the encoding header of the commit object if present or (from the docs) \n>\"lack of this header implies that the commit log message is encoded in UTF-8.\" \n>If this step fails (if the entry contains a byte sequence that is invalid in the specified/assumed input codec), \n>the procedure is aborted and the data is dumped as is (obviously without applying the requested output encoding).\n> \n> Is that correct?\nYes, see above.\n> \n> From my point of view the most natural abstraction of a log *message* is *text*, not bytes. \n\n>The same is true for author names. \n\n>If I want to build a tool chain on top of log/show, this usually means that I want to work with text information. \n>Hence, I want to retrieve text (a sequence of code points) from git show/log. \n>Text must be transported in encoded form, sure, \n>but it must not contain byte sequences that are invalid in this codec. \n>Because otherwise it's just not text anymore.\n> \nCall it corrupted.\n> Hence, from my point of view, the rational that git show/log should be able to output *text* information means\n> that they should not emit byte sequences that are invalid in the codec specified via the --encoding argument. \n> In the current situation, the work of dealing with invalid byte sequences is just outsourced to software\n> further below in the tool chain \n>(at some point a replacement character � should be displayed to the user instead of the invalid raw bytes).\n> \n> I am not entirely sure where this discussion should lead to. \nYes, until someone writes a patch to improve either the documentation or the code,\nnothing will be changed.\n> However, I think that if the behavior of the software will not be changed, \n>then the documentation for the --encoding option should be more precise and \n>clarify what actually happens behind the scenes. What do you think?\nPatches are more than welcome.\n> \n> \n> Cheers,\n> \n> \n> Jan-Philip Gehrcke\n"},{"id":"263920","messageId":"557FEEA5.2080006@googlemail.com","threadId":"39630","inReplyTo":"557EFB94.3040104@web.de","subject":"Re: Should the --encoding argument to log/show commands make any guarantees about their output?","fromName":"Jan-Philip Gehrcke","fromEmail":"jgehrcke@googlemail.com","sentAt":"2015-06-16T09:38:45Z","receivedAt":"2015-06-16T09:38:45Z","isPatch":false,"sender":{"key":"jgehrcke@googlemail.com","avatar":null},"body":"On 15.06.2015 18:21, Torsten Bögershausen wrote:\n> On 2015-06-15 10.50, Jan-Philip Gehrcke wrote:\n>> Let me describe what I think it currently does:\n>>\n>> The program attempts to re-code a log message, so it follows the chain\n>>\n>>      raw input -> unicode -> raw output\n> Not sure what \"raw input/output\" means.\n> But there is only one reencode step involved, e.g.\n> input(8859) -> output(UTF-8)\n\nWe surely agree. With \"raw\" I meant a sequence of bytes, and with \n\"unicode\" I meant the intermediate state in the process of re-encoding \n(which can be thought of as decoding and encoding with a transient \nintermediate state).\n\n> If the user ignores this warning, how should Git guess the encoding  ?\n\nI entirely appreciate that there is no satisfying solution to this very \nproblem.\n\n>> If this step fails (if the entry contains a byte sequence that is invalid in the specified/assumed input codec),\n>> the procedure is aborted and the data is dumped as is (obviously without applying the requested output encoding).\n>>\n>> Is that correct?\n> Yes, see above.\n\nThanks!\n\n>> Hence, from my point of view, the rational that git show/log should be able to output *text* information means\n>> that they should not emit byte sequences that are invalid in the codec specified via the --encoding argument.\n>> In the current situation, the work of dealing with invalid byte sequences is just outsourced to software\n>> further below in the tool chain\n>> (at some point a replacement character � should be displayed to the user instead of the invalid raw bytes).\n>>\n>> I am not entirely sure where this discussion should lead to.\n> Yes, until someone writes a patch to improve either the documentation or the code,\n> nothing will be changed.\n>> However, I think that if the behavior of the software will not be changed,\n>> then the documentation for the --encoding option should be more precise and\n>> clarify what actually happens behind the scenes. What do you think?\n> Patches are more than welcome.\n\nI'd be willing to contribute, but of course there must be a discussion \nand an agreement before that, if there is need to change something at \nall, and what exactly.\n\nTo this discussion I would like to contribute that I am of the opinion \nthat there should be a command line option to make git show/log/friends \nemit a byte stream that is guaranteed to be valid in a given codec.\n\nThat would require detection and treatment of those cases where \ncorrupted text resides in the repository (we cannot prevent it from \nentering the repository, as discussed). In these cases, one could emit a \nreplacement symbol (e.g. '?') per invalid byte subsequence (this seems \nto be more established than just swallowing the invalid byte sequence).\n\nWhat do you think?\n\nI think the --encoding option would have ideal semantics for described \nbehavior.\n\nHowever, I guess maintaining backwards compatibility is an issue here. \nOn the other hand, I realize that the --encoding option undergoes \nchanges: the docs for git log in release 2.4.3 do not even list the \n--encoding option anymore. Why is that? I haven't found a corresponding \nchangelog/release notes entry.\n\n\nThanks,\n\n\nJan-Philip\n"},{"id":"263969","messageId":"55808133.6070802@web.de","threadId":"39630","inReplyTo":"557FEEA5.2080006@googlemail.com","subject":"Re: Should the --encoding argument to log/show commands make any guarantees about their output?","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-06-16T20:04:03Z","receivedAt":"2015-06-16T20:04:03Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2015-06-16 11.38, Jan-Philip Gehrcke wrote:\n> On 15.06.2015 18:21, Torsten Bögershausen wrote:\n>> On 2015-06-15 10.50, Jan-Philip Gehrcke wrote:\n>>> Let me describe what I think it currently does:\n>>>\n>>> The program attempts to re-code a log message, so it follows the chain\n>>>\n>>>      raw input -> unicode -> raw output\n>> Not sure what \"raw input/output\" means.\n>> But there is only one reencode step involved, e.g.\n>> input(8859) -> output(UTF-8)\n\nI probably need to correct myself:\npretty.c\nvoid format_commit_message(const struct commit *commit,\n\nThe the message is converted from the commit encoding into UTF-8.\nWhen the log encoding is different from UTF-8,\nThere is a second conversio (as you said).\n(But not in your case, here the second conversion is skipped)\n\n[snip]\n\n> \n> I'd be willing to contribute, but of course there must be a discussion and an\n> agreement before that, if there is need to change something at all, and what\n> exactly.\n[]\n> What do you think?\nSee commit.c:\n\n/*\n * This verifies that the buffer is in proper utf8 format.\n *\n * If it isn't, it assumes any non-utf8 characters are Latin1,\n * and does the conversion.\n */\nstatic int verify_utf8(struct strbuf *buf)\n\n> \n> I think the --encoding option would have ideal semantics for described behavior.\n> \n> However, I guess maintaining backwards compatibility is an issue here. On the\n> other hand, I realize that the --encoding option undergoes changes: the docs for\n> git log in release 2.4.3 do not even list the --encoding option anymore. Why is\n> that? I haven't found a corresponding changelog/release notes entry.\n\nIt still seems to work:\ngit log --encoding ISO-8859-1 | xxd | grep gersha | less\nincludes this line:\n7465 6e20 42f6 6765 7273 6861 7573 656e  ten B.gershausen\n\n\nSo my suggestion (in short):\nmove verify_utf8() from commit.c into utf8.c,\nmake it non-static, and add a prototype in utf8.h\n\nUse that function in pretty.c (and commit.c), test it.\nMake a patch out of it and send it to the list.\n\nIn the ideal world the patch will include a test case,\nbut I don't know how easy it is to create such a commit.\n"},{"id":"264041","messageId":"xmqqzj3y2snq.fsf@gitster.dls.corp.google.com","threadId":"39630","inReplyTo":"557E91D2.3000908@googlemail.com","subject":"Re: Should the --encoding argument to log/show commands make any guarantees about their output?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-17T16:42:17Z","receivedAt":"2015-06-17T16:42:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jan-Philip Gehrcke <jgehrcke@googlemail.com> writes:\n\n> I was surprised to see that the output of\n>\n>     git log --encoding=utf-8 \"--format=format:%b\"\n>\n> can contain byte sequences that are invalid in UTF-8. Note: I am using\n> git 2.1.4 and the %b format specifier represents the commit message\n> body.\n\nYeah, if the original was bad and cannot be sanely expressed in\nUTF-8, you have two options.  You can show the contents as raw bytes\nrecorded in the object with a warning so that the user can use it as\nsuch (e.g. perhaps the original was indeed an iso8859-2 but was\nincorrectly marked as UTF-8, or something like that, and a human\nthat is more intelligent than a tool _could_ guess and attempt to\nrecover).  Or you can error out and refuse to produce output.\n\nWe deliberately made a design choice to take the former option.\n"},{"id":"264046","messageId":"5581A964.4000500@googlemail.com","threadId":"39630","inReplyTo":"xmqqzj3y2snq.fsf@gitster.dls.corp.google.com","subject":"Re: Should the --encoding argument to log/show commands make any guarantees about their output?","fromName":"Jan-Philip Gehrcke","fromEmail":"jgehrcke@googlemail.com","sentAt":"2015-06-17T17:07:48Z","receivedAt":"2015-06-17T17:07:48Z","isPatch":false,"sender":{"key":"jgehrcke@googlemail.com","avatar":null},"body":"On 17.06.2015 18:42, Junio C Hamano wrote:\n> Jan-Philip Gehrcke <jgehrcke@googlemail.com> writes:\n>\n>> I was surprised to see that the output of\n>>\n>>      git log --encoding=utf-8 \"--format=format:%b\"\n>>\n>> can contain byte sequences that are invalid in UTF-8. Note: I am using\n>> git 2.1.4 and the %b format specifier represents the commit message\n>> body.\n>\n> Yeah, if the original was bad and cannot be sanely expressed in\n> UTF-8, you have two options.  You can show the contents as raw bytes\n> recorded in the object with a warning so that the user can use it as\n> such (e.g. perhaps the original was indeed an iso8859-2 but was\n> incorrectly marked as UTF-8, or something like that, and a human\n> that is more intelligent than a tool _could_ guess and attempt to\n> recover).  Or you can error out and refuse to produce output.\n\nThe two-option scenario is totally clear. Although one must stress that \nthe \"error-out\" option can, as discussed, be kept minimally invasive: it \nis sufficient (and common) to just skip those byte sequences (and \nreplace them with a replacement symbol) that would be invalid in the \nrequested output encoding. This would retain as much information as \npossible while guaranteeing a subsequent decoder to retrieve valid input.\n\n> We deliberately made a design choice to take the former option.\n\nI totally support this design choice in general, especially when \ninvoking `git whatever` without options. This here is, I think, mainly \nabout documentation and the semantics of \"--encoding\". From my point of \nview, `--encoding=utf-8` semantically suggests that the output *is* \nvalid UTF-8. But it is not, not always. May initial question was: what \ndo you think about this? Should we\n\n* just make this more clear in the docs and/or\n* should we adjust the behavior of --encoding or\n* should we do something entirely different, like adding a new command \nline option or\n* should we just leave things as they are?\n\nThanks and cheers,\n\n\nJan-Philip\n"},{"id":"264053","messageId":"20150617184607.GA28455@peff.net","threadId":"39630","inReplyTo":"5581A964.4000500@googlemail.com","subject":"Re: Should the --encoding argument to log/show commands make any guarantees about their output?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-17T18:46:08Z","receivedAt":"2015-06-17T18:46:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 17, 2015 at 07:07:48PM +0200, Jan-Philip Gehrcke wrote:\n\n> The two-option scenario is totally clear. Although one must stress that the\n> \"error-out\" option can, as discussed, be kept minimally invasive: it is\n> sufficient (and common) to just skip those byte sequences (and replace them\n> with a replacement symbol) that would be invalid in the requested output\n> encoding. This would retain as much information as possible while\n> guaranteeing a subsequent decoder to retrieve valid input.\n\nI think \"munge into valid UTF-8, even if it means losing data\" is a\ntotally valid and useful option. I'm not completely sure that git should\ndo that, though.  E.g., you could just as easily do:\n\n  git log --encoding=utf8 | drop_invalid_utf8 | your_script\n\nOr quite possibly, your_script could do the munging itself while reading\nthe data. I do not know much about Python's input handling, but in Perl,\nit is easy to say \"the input is utf8, and replace anything bogus with a\nsubstitution character\"[1].\n\n> Should we\n> \n> * just make this more clear in the docs and/or\n> * should we adjust the behavior of --encoding or\n> * should we do something entirely different, like adding a new command line\n> option or\n> * should we just leave things as they are?\n\nI would vote for a documentation change, perhaps like:\n\nSubject: docs: clarify that --encoding can produce invalid sequences\n\nIn the common case that the commit encoding matches the\noutput encoding, we do not touch the buffer at all, which\nmakes things much more efficient. But it might be unclear to\na consumer that we will pass through bogus sequences.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/pretty-options.txt | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex 74aa01a..642af6e 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -37,7 +37,10 @@ people using 80-column terminals.\n \tin their encoding header; this option can be used to tell the\n \tcommand to re-code the commit log message in the encoding\n \tpreferred by the user.  For non plumbing commands this\n-\tdefaults to UTF-8.\n+\tdefaults to UTF-8. Note that if an object claims to be encoded\n+\tin `X` and we are outputting in `X`, we will output the object\n+\tverbatim; this means that invalid sequences in the original\n+\tcommit may be copied to the output.\n \n --notes[=<ref>]::\n \tShow the notes (see linkgit:git-notes[1]) that annotate the\n-- \n2.4.4.719.g3984bc6\n"},{"id":"264074","messageId":"5581D0AA.9020605@web.de","threadId":"39630","inReplyTo":"5581A964.4000500@googlemail.com","subject":"Re: Should the --encoding argument to log/show commands make any guarantees about their output?","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-06-17T19:55:22Z","receivedAt":"2015-06-17T19:55:22Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"> * just make this more clear in the docs and/or\n> * should we adjust the behavior of --encoding or\n> * should we do something entirely different, like adding a new command line\n> option or\nThe general spirit is to keep things backwards compatible, so that users which\nexpect the \"raw\" (and possible corrupted UTF-8) data still get the same results,\nwhen they updata their Git installation.\n\nA new command line option will allow users to get clean UTF-8.\n\nOne suggestion could be\n--fixbroken=ISO-8859-1    (a)\n--fixbroken=octalescape   (b)\n--fixbroken=hexescape     (c)\n\n(a) would replace  \"0xf6\" with \"0xc3 0xb6\"\n(b) could write \"\\366\"\n(c) could write \"<F6>\"\n\nThe exact form of the syntax can be discussed of course.\n\nHowever, I would probably start with (a), and add other options\nif needed.\n\n> * should we just leave things as they are?\n.... not the ideal thing.\n"},{"id":"264077","messageId":"xmqqzj3y5cji.fsf@gitster.dls.corp.google.com","threadId":"39630","inReplyTo":"20150617184607.GA28455@peff.net","subject":"Re: Should the --encoding argument to log/show commands make any guarantees about their output?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-17T20:02:09Z","receivedAt":"2015-06-17T20:02:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I would vote for a documentation change, perhaps like:\n>\n> Subject: docs: clarify that --encoding can produce invalid sequences\n>\n> In the common case that the commit encoding matches the\n> output encoding, we do not touch the buffer at all, which\n> makes things much more efficient. But it might be unclear to\n> a consumer that we will pass through bogus sequences.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n\nSounds like a sensible thing to do.\n\n>  Documentation/pretty-options.txt | 5 ++++-\n>  1 file changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\n> index 74aa01a..642af6e 100644\n> --- a/Documentation/pretty-options.txt\n> +++ b/Documentation/pretty-options.txt\n> @@ -37,7 +37,10 @@ people using 80-column terminals.\n>  \tin their encoding header; this option can be used to tell the\n>  \tcommand to re-code the commit log message in the encoding\n>  \tpreferred by the user.  For non plumbing commands this\n> -\tdefaults to UTF-8.\n> +\tdefaults to UTF-8. Note that if an object claims to be encoded\n> +\tin `X` and we are outputting in `X`, we will output the object\n> +\tverbatim; this means that invalid sequences in the original\n> +\tcommit may be copied to the output.\n>  \n>  --notes[=<ref>]::\n>  \tShow the notes (see linkgit:git-notes[1]) that annotate the\n"}]}