{"thread":{"id":"28747","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","startedAt":"2011-10-22T19:09:15Z","lastAt":"2011-10-28T04:07:52Z","messageCount":26,"participants":["Jeff King","Junio C Hamano","Nguyen Thai Ngoc Duy","Robin Rosenberg","Štěpán Němec","Miles Bader"],"isPatch":true,"patchVersion":1,"patchTotal":22},"messages":[{"id":"178171","messageId":"20111022190914.GA1785@sigill.intra.peff.net","threadId":"28747","inReplyTo":"1319277881-4128-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-22T19:09:15Z","receivedAt":"2011-10-22T19:09:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 22, 2011 at 09:04:19PM +1100, Nguyen Thai Ngoc Duy wrote:\n\n> This series helps pass commit message size up to output functions,\n> though it does not change any output functions to print ^@.\n\nCan we take a step back for a second and discuss what git _should_ do\nwith commits that contain NUL?\n\nIf all of the pretty-print functions are just going consider \"foo\\0bar\"\nto be \"foo^@bar\", then maybe it would be much simpler to just\n\"normalize\" the commit message into a C string at a lower level, and\npass it around as a string as we currently do.\n\nOn the other hand, if we are eventually looking to add an option like\n\"--include-NUL-in-commit-message\", then it would make sense for the\nreal contents and size to get passed around.\n\n> All functions up to the last patch learn to accept a string as a pair\n> as a preparation step. These\n> changes are relatively simple. Or it could have been so if I did not\n> attempt to reduce some code duplication found while working on this\n> series.\n\nGreat. Reducing code duplication is always a plus.\n\n> The last patch turns commit_buffer field in struct commit to \"struct\n> strbuf *\". This approach costs us 12 bytes more each commit. We can\n> choose not to use strbuf to save memory.\n\nI think 12 bytes in the commit struct might be noticeable. But it looks\nlike you've done the sane thing, and replaced the pointer-to-char with a\npointer-to-strbuf. And that I don't think should be a big deal. The\nbuffer itself is way bigger than 12 bytes, so we don't care so much\nabout the \"we have a buffer\" case, but more about the 100,000 other\ncommits that we're not currently printing right now.\n\nOf course, some timings on things like \"rev-list\" and \"pack-objects\"\nwould be nice to double-check.\n\n-Peff\n"},{"id":"178184","messageId":"7vobx863v3.fsf@alter.siamese.dyndns.org","threadId":"28747","inReplyTo":"1319277881-4128-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-22T22:47:12Z","receivedAt":"2011-10-22T22:47:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I do not think we want to go this route.\n\nThere are two possible approaches to attack this.\n\n - If we want to show everything after a potential and rare NUL in the log\n message most of the time, then \"struct commit\" should just store\n pair. This grows \"struct commit\" with one extra ulong.\n\n - If we want to give us a way to notice and show these \"funnily, this\n commit log message has a NUL in it\" case as an exception in only\n selected codepaths, then \"struct commit\" should just gain \"flags\"\n 4-byte int field between \"indegree\" and \"date\", and\n parse_commit_buffer() should set one bit in the flags when the log\n message has NUL in it. And teach only these selected codepaths to find\n the length from the object name with sha1_object_info() as needed. This\n grows \"struct commit\" with one 4-byte int, with runtime overhead only\n where it matters.\n\nThe approach taken by the patch wastes two malloc() blocks with their own\nallocation overhead, and unused \"alloc\" field in the strbuf that does not\nhave to be there.\n"},{"id":"178185","messageId":"CACsJy8B=TsC4A=R6b3jyYBCvorEDBYHQ8uA864WrB0-3pgNyKA@mail.gmail.com","threadId":"28747","inReplyTo":"7vobx863v3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-23T01:24:25Z","receivedAt":"2011-10-23T01:24:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/10/23 Junio C Hamano :\n> I do not think we want to go this route.\n>\n> There are two possible approaches to attack this.\n>\n>  - If we want to show everything after a potential and rare NUL in the log\n>   message most of the time, then \"struct commit\" should just store\n>   pair. This grows \"struct commit\" with one extra ulong.\n>\n>  - If we want to give us a way to notice and show these \"funnily, this\n>   commit log message has a NUL in it\" case as an exception in only\n>   selected codepaths, then \"struct commit\" should just gain \"flags\"\n>   4-byte int field between \"indegree\" and \"date\", and\n>   parse_commit_buffer() should set one bit in the flags when the log\n>   message has NUL in it. And teach only these selected codepaths to find\n>   the length from the object name with sha1_object_info() as needed. This\n>   grows \"struct commit\" with one 4-byte int, with runtime overhead only\n>   where it matters.\n>\n> The approach taken by the patch wastes two malloc() blocks with their own\n> allocation overhead, and unused \"alloc\" field in the strbuf that does not\n> have to be there.\n\nWe could allocate just one block with length as the first field:\n\nstruct commit_buffer {\n unsigned long len;\n char buf[FLEX_ARRAY];\n};\n\nThe downside is commit_buffer field type in struct commit changes,\nwhich impacts many codepaths. If we agree to allow NUL in commit\nobjects, then all codepaths should be aware of that fact, otherwise\nfunny things may happen because string processing in this function\nstops early due to NUL, but others run fine..\n\nJeff's low-level normalization approach sounds much simpler, but\nprobably trickier because we need to identify where to normalize and\ndenormalize. At least with type change, the compiler spots all the\nplaces for me.\n\nI would not worry about runtime processing overhead. The string end\ncheck is basically converted from \"if (*msg)\" to \"if (msg < msg_end)\".\nThere will one more pointer (msg_end) in stack for each call. Unless\nwe do deep recursion, we should be fine. Memory overhead is still\nsomething to profile.\n-- \nDuy\n"},{"id":"178187","messageId":"7vipng5k80.fsf@alter.siamese.dyndns.org","threadId":"28747","inReplyTo":"CACsJy8B=TsC4A=R6b3jyYBCvorEDBYHQ8uA864WrB0-3pgNyKA@mail.gmail.com","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-23T05:51:27Z","receivedAt":"2011-10-23T05:51:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> We could allocate just one block with length as the first field:\n>\n> struct commit_buffer {\n> unsigned long len;\n> char buf[FLEX_ARRAY];\n> };\n>\n> The downside is commit_buffer field type in struct commit changes,\n> which impacts many codepaths.\n\nI think that is a good thing overall to _force_ us to audit all the code,\n*if* our goal were to avoid losing bytes. And the solution above is better\nthan adding a length field to \"struct commit\". It certainly is better than\nquoting NUL byte to ^@, keep using the \"char *\" field and risking some\ncodepaths forget to convert it back to NUL. For types of payloads for\nwhich losing everything after the first NUL matters, converting NUL to ^@\nand then forgetting to convert it back to NUL is equally bad breakage to\nthe payload anyway, so such a conversion would not be a particularly good\napproach to avoid losing bytes.\n\nBut as Jeff suggested, we should step back a bit and think what our goal\nis.\n\nThe low level object format of our commit is textual header fields, each\nof which is terminated with a LF, followed by a LF to mark the end of\nheader fields, and then opaque payload that can contain any bytes. It does\nnot forbid a non-Git application to reuse the object store infrastructure\nto store ASN.1 binary goo there, and the low level interface we give such\nas cat-file is a perfectly valid way to inspect such a \"commit\" object.\n\nBut when it comes to \"Git\" Porcelains (e.g. the log family of commands),\nwe do assume people do not store random binary byte sequences in commits,\nand we do take advantage of that assumption by splitting each \"line\" at\nLF, indenting them with 4 spaces, etc. In other words, a commit log in the\nGit context _is_ pretty much text and not arbitrary byte sequence. Even\nthe \"--pretty=raw\" option for \"log\" family is not about the \"raw\" body;\nthe \"raw\"-ness applies only to the header fields. So even if we _were_ to\nupdate the codepaths involved to avoid losing bytes, the end result will\nnot be useful for users to whom ability to include NUL matters.\n\nSo in that sense, I do not think it is unreasonable to chop it off at the\nfirst NUL, which is the current behaviour. IOW, it is entirely sane to\nargue that there is nothing to fix.\n"},{"id":"178188","messageId":"CACsJy8CA2cqJqt7cUN1CdnOb3=qE6B2XTd1oQKZ7osVz09kSGg@mail.gmail.com","threadId":"28747","inReplyTo":"7vipng5k80.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-23T06:37:16Z","receivedAt":"2011-10-23T06:37:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Oct 23, 2011 at 4:51 PM, Junio C Hamano wrote:\n> So in that sense, I do not think it is unreasonable to chop it off at the\n> first NUL, which is the current behaviour. IOW, it is entirely sane to\n> argue that there is nothing to fix.\n\nGood for me too if we go this way. I was just curious what if we\nchanged commit_buffer field and got hooked in.\n\n> But as Jeff suggested, we should step back a bit and think what our goal\n> is.\n>\n> The low level object format of our commit is textual header fields, each\n> of which is terminated with a LF, followed by a LF to mark the end of\n> header fields, and then opaque payload that can contain any bytes. It does\n> not forbid a non-Git application to reuse the object store infrastructure\n> to store ASN.1 binary goo there, and the low level interface we give such\n> as cat-file is a perfectly valid way to inspect such a \"commit\" object.\n\ncat-file is fine, commit-tree (or any commands that call\ncommit_tree()) cuts at NUL though.\n\nI wonder how git processes commit messages in utf-16. Did a quick\ntest, did not look good. But that's because git-commit cuts at NUL.\nBut even if git-commit makes a good object, I doubt if git-log shows\nit right.\n-- \nDuy\n"},{"id":"178192","messageId":"7vehy459bg.fsf@alter.siamese.dyndns.org","threadId":"28747","inReplyTo":"CACsJy8CA2cqJqt7cUN1CdnOb3=qE6B2XTd1oQKZ7osVz09kSGg@mail.gmail.com","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-23T09:46:59Z","receivedAt":"2011-10-23T09:46:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> On Sun, Oct 23, 2011 at 4:51 PM, Junio C Hamano wrote:\n> ...\n>> The low level object format of our commit is textual header fields, each\n>> of which is terminated with a LF, followed by a LF to mark the end of\n>> header fields, and then opaque payload that can contain any bytes. It does\n>> not forbid a non-Git application to reuse the object store infrastructure\n>> to store ASN.1 binary goo there, and the low level interface we give such\n>> as cat-file is a perfectly valid way to inspect such a \"commit\" object.\n>\n> cat-file is fine, commit-tree (or any commands that call\n> commit_tree()) cuts at NUL though.\n> I wonder how git processes commit messages in utf-16.\n\nThat is exactly what I am saying.\n\nPerhaps you didn't either read or understand what you omitted from your\nquoting; otherwise you even wouldn't have brought up utf-16.\n\nLet me requote that part for you.\n\n> But when it comes to \"Git\" Porcelains (e.g. the log family of commands),\n> we do assume people do not store random binary byte sequences in commits,\n> and we do take advantage of that assumption by splitting each \"line\" at\n> LF, indenting them with 4 spaces, etc. In other words, a commit log in the\n> Git context _is_ pretty much text and not arbitrary byte sequence.\n\nThink what would cutting at a byte whose value is 012 and adding four\nbytes whose values are 040 to each of \"lines\" that formed with such\ncutting do to UTF-16 goo, even if it does not contain any NUL byte. As far\nas Git Porcelains are concerned, it is no different from random binary\nbyte sequences.\n"},{"id":"178194","messageId":"CACsJy8C4nEQmgtTGSvwcVMdgksVuOj9mssuFinXp3=ZqLJtgUg@mail.gmail.com","threadId":"28747","inReplyTo":"7vehy459bg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-23T10:17:41Z","receivedAt":"2011-10-23T10:17:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Oct 23, 2011 at 8:46 PM, Junio C Hamano wrote:\n> Nguyen Thai Ngoc Duy writes:\n>\n>> On Sun, Oct 23, 2011 at 4:51 PM, Junio C Hamano wrote:\n>> ...\n>>> The low level object format of our commit is textual header fields, each\n>>> of which is terminated with a LF, followed by a LF to mark the end of\n>>> header fields, and then opaque payload that can contain any bytes. It does\n>>> not forbid a non-Git application to reuse the object store infrastructure\n>>> to store ASN.1 binary goo there, and the low level interface we give such\n>>> as cat-file is a perfectly valid way to inspect such a \"commit\" object.\n>>\n>> cat-file is fine, commit-tree (or any commands that call\n>> commit_tree()) cuts at NUL though.\n>> I wonder how git processes commit messages in utf-16.\n>\n> That is exactly what I am saying.\n>\n> Perhaps you didn't either read or understand what you omitted from your\n> quoting; otherwise you even wouldn't have brought up utf-16.\n>\n> Let me requote that part for you.\n>\n>> But when it comes to \"Git\" Porcelains (e.g. the log family of commands),\n>> we do assume people do not store random binary byte sequences in commits,\n>> and we do take advantage of that assumption by splitting each \"line\" at\n>> LF, indenting them with 4 spaces, etc. In other words, a commit log in the\n>> Git context _is_ pretty much text and not arbitrary byte sequence.\n>\n> Think what would cutting at a byte whose value is 012 and adding four\n> bytes whose values are 040 to each of \"lines\" that formed with such\n> cutting do to UTF-16 goo, even if it does not contain any NUL byte. As far\n> as Git Porcelains are concerned, it is no different from random binary\n> byte sequences.\n>\n\nI'm sorry. The utf-16 was an afterthought when I was nearly finished\nwith the reply and already cut that quote.\n\nThe assumption that people do not store random binary byte sequences\nin commits sort of conflicts with \"encoding\" field in the commit\nheader though. The assumption is documented in i18n.txt. I guess it's\njust me who did not read document carefully. But maybe it's good to\nstop people from shooting themselves in this case (i.e. setting\nencoding to utf-16 or similar).\n-- \nDuy\n"},{"id":"178196","messageId":"4EA3F00E.9040006@gmail.com","threadId":"28747","inReplyTo":"20111022190914.GA1785@sigill.intra.peff.net","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@gmail.com","sentAt":"2011-10-23T10:44:30Z","receivedAt":"2011-10-23T10:44:30Z","isPatch":true,"sender":{"key":"robin.rosenberg@gmail.com","avatar":null},"body":"Jeff King skrev 2011-10-22 21.09:\n> On Sat, Oct 22, 2011 at 09:04:19PM +1100, Nguyen Thai Ngoc Duy wrote:\n>\n>> This series helps pass commit message size up to output functions,\n>> though it does not change any output functions to print ^@.\n> Can we take a step back for a second and discuss what git _should_ do\n> with commits that contain NUL?\nYes please. I don't think allowing NUL makes sense, but it makes sense\nto state how NUL should be handled when anyone attempt it, so there\nmight be things to fix even if NUL is banned.\n\nAre there any such commits in the wild?\n\n-- robin\n"},{"id":"178202","messageId":"20111023160744.GA22444@sigill.intra.peff.net","threadId":"28747","inReplyTo":"7vehy459bg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-23T16:07:45Z","receivedAt":"2011-10-23T16:07:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 23, 2011 at 02:46:59AM -0700, Junio C Hamano wrote:\n\n> > But when it comes to \"Git\" Porcelains (e.g. the log family of commands),\n> > we do assume people do not store random binary byte sequences in commits,\n> > and we do take advantage of that assumption by splitting each \"line\" at\n> > LF, indenting them with 4 spaces, etc. In other words, a commit log in the\n> > Git context _is_ pretty much text and not arbitrary byte sequence.\n> \n> Think what would cutting at a byte whose value is 012 and adding four\n> bytes whose values are 040 to each of \"lines\" that formed with such\n> cutting do to UTF-16 goo, even if it does not contain any NUL byte. As far\n> as Git Porcelains are concerned, it is no different from random binary\n> byte sequences.\n\nBut as Duy mentions, we have an encoding header. Shouldn't we treat it\nlike binary goo until we do reencode_log_message, and _then_ we can\nbreak it into lines?\n\n-Peff\n"},{"id":"178203","messageId":"20111023160914.GB22444@sigill.intra.peff.net","threadId":"28747","inReplyTo":"4EA3F00E.9040006@gmail.com","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-23T16:09:15Z","receivedAt":"2011-10-23T16:09:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 23, 2011 at 12:44:30PM +0200, Robin Rosenberg wrote:\n\n> Jeff King skrev 2011-10-22 21.09:\n> >On Sat, Oct 22, 2011 at 09:04:19PM +1100, Nguyen Thai Ngoc Duy wrote:\n> >\n> >>This series helps pass commit message size up to output functions,\n> >>though it does not change any output functions to print ^@.\n> >Can we take a step back for a second and discuss what git _should_ do\n> >with commits that contain NUL?\n> Yes please. I don't think allowing NUL makes sense, but it makes sense\n> to state how NUL should be handled when anyone attempt it, so there\n> might be things to fix even if NUL is banned.\n> \n> Are there any such commits in the wild?\n\nAdding an arbitrary NUL, no, I don't think I've ever seen it outside of\npeople (myself included) trying to break git in interesting ways.\n\nBut utf16 may contains NUL bytes, so I expect configuring your editor to\noutput utf16 and running \"git commit\" would give you the most likely\nexample.\n\n-Peff\n"},{"id":"178207","messageId":"7v39ej5uqb.fsf@alter.siamese.dyndns.org","threadId":"28747","inReplyTo":"20111023160744.GA22444@sigill.intra.peff.net","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-23T20:16:44Z","receivedAt":"2011-10-23T20:16:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> But as Duy mentions, we have an encoding header. Shouldn't we treat it\n> like binary goo until we do reencode_log_message, and _then_ we can\n> break it into lines?\n\nThat's sensible. If we go that route, I think the \"one allocation of\nseparate struct commit_buffer pointed from a pointer field in struct\ncommit to replace the current member 'buffer'\" is a reasonable thing\nto do.\n"},{"id":"178211","messageId":"7vy5wb3sto.fsf@alter.siamese.dyndns.org","threadId":"28747","inReplyTo":"7v39ej5uqb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-24T04:40:51Z","receivedAt":"2011-10-24T04:40:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> But as Duy mentions, we have an encoding header. Shouldn't we treat it\n>> like binary goo until we do reencode_log_message, and _then_ we can\n>> break it into lines?\n>\n> That's sensible. If we go that route, I think the \"one allocation of\n> separate struct commit_buffer pointed from a pointer field in struct\n> commit to replace the current member 'buffer'\" is a reasonable thing\n> to do.\n\nHaving given that \"sensible\" comment, I am not convinced if this is worth\nit. We are talking about what is left in the ephemeral COMMIT_EDITMSG by\nthe chosen editor, but are there really editors that can _only_ write in\nUTF-16 and not in UTF-8, and is it worth bending backwards to add support\nsuch an editor?\n"},{"id":"178212","messageId":"CACsJy8AsfQnS3L1fabzB-z7BdH=jvB=XNnmP2RZu0qp7C1uGYQ@mail.gmail.com","threadId":"28747","inReplyTo":"7vy5wb3sto.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-24T05:10:08Z","receivedAt":"2011-10-24T05:10:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Oct 24, 2011 at 3:40 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Jeff King writes:\n>>\n>>> But as Duy mentions, we have an encoding header. Shouldn't we treat it\n>>> like binary goo until we do reencode_log_message, and _then_ we can\n>>> break it into lines?\n>>\n>> That's sensible. If we go that route, I think the \"one allocation of\n>> separate struct commit_buffer pointed from a pointer field in struct\n>> commit to replace the current member 'buffer'\" is a reasonable thing\n>> to do.\n>\n> Having given that \"sensible\" comment, I am not convinced if this is worth\n> it. We are talking about what is left in the ephemeral COMMIT_EDITMSG by\n> the chosen editor, but are there really editors that can _only_ write in\n> UTF-16 and not in UTF-8, and is it worth bending backwards to add support\n> such an editor?\n\nThis is argument for the sake of argument because I don't use utf-16\nand do not care much. UTF-16 can have more code points and some may\nprefer utf-16 to utf-8.\n\nI'd be happy with git's refusing to create broken commits because\npeople accidentally set editor encoding to utf-16. If people do use\nutf-16, they'll get caught and should yell up if they want utf-16\nsupported.\n-- \nDuy\n"},{"id":"178232","messageId":"87wrbu4peo.fsf@gmail.com","threadId":"28747","inReplyTo":"CACsJy8AsfQnS3L1fabzB-z7BdH=jvB=XNnmP2RZu0qp7C1uGYQ@mail.gmail.com","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Štěpán Němec","fromEmail":"stepnem@gmail.com","sentAt":"2011-10-24T11:09:19Z","receivedAt":"2011-10-24T11:09:19Z","isPatch":true,"sender":{"key":"stepnem@gmail.com","avatar":"https://avatars.githubusercontent.com/u/106838?v=4"},"body":"On Mon, 24 Oct 2011 07:10:08 +0200\nNguyen Thai Ngoc Duy wrote:\n\n> This is argument for the sake of argument because I don't use utf-16\n> and do not care much. UTF-16 can have more code points and some may\n> prefer utf-16 to utf-8.\n\nI suspect this is really tangential to this thread, but I can't make\nmuch sense of that last sentence -- if you meant that UTF-16 is somehow\nmore apt at encoding Unicode code points than UTF-8, then that's not the\ncase. Both can represent all Unicode characters. If anything, things are\n_more_, not less complicated in UTF-16, which apart from the NUL and\nendianness complications has to jump through the \"surrogate pairs\" hoop\nfor code points bigger than U+FFFF (so you'll actually find many apps\nwith buggy UTF-16 implementation which break for those code points,\nunlike when using UTF-8).\n\n-- \nŠtěpán\n"},{"id":"178250","messageId":"20111024224558.GB10481@sigill.intra.peff.net","threadId":"28747","inReplyTo":"7vy5wb3sto.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-24T22:45:58Z","receivedAt":"2011-10-24T22:45:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 23, 2011 at 09:40:51PM -0700, Junio C Hamano wrote:\n\n> >> But as Duy mentions, we have an encoding header. Shouldn't we treat it\n> >> like binary goo until we do reencode_log_message, and _then_ we can\n> >> break it into lines?\n> >\n> > That's sensible. If we go that route, I think the \"one allocation of\n> > separate struct commit_buffer pointed from a pointer field in struct\n> > commit to replace the current member 'buffer'\" is a reasonable thing\n> > to do.\n> \n> Having given that \"sensible\" comment, I am not convinced if this is worth\n> it. We are talking about what is left in the ephemeral COMMIT_EDITMSG by\n> the chosen editor, but are there really editors that can _only_ write in\n> UTF-16 and not in UTF-8, and is it worth bending backwards to add support\n> such an editor?\n\nCouldn't you make the same argument about iso8859-1, or any other\nencoding? The user has some encoding that they want to use, for whatever\nreason[1]. We have a slot for an encoding header; is there a reason that\ngit would allow some encodings and not others?\n\nI mean, besides the obvious that UTF-16 is annoying and contains\nembedded NULs and newlines.\n\n-Peff\n\n[1] English is my first language, so it's rare for me to even step\noutside of ASCII, let alone latin1. But aren't there some languages in\nwhich utf-16 is more efficient than utf-8?\n"},{"id":"178260","messageId":"87sjmh4brz.fsf@gmail.com","threadId":"28747","inReplyTo":"20111024224558.GB10481@sigill.intra.peff.net","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Štěpán Němec","fromEmail":"stepnem@gmail.com","sentAt":"2011-10-25T10:16:00Z","receivedAt":"2011-10-25T10:16:00Z","isPatch":true,"sender":{"key":"stepnem@gmail.com","avatar":"https://avatars.githubusercontent.com/u/106838?v=4"},"body":"On Tue, 25 Oct 2011 00:45:58 +0200\nJeff King wrote:\n\n> [1] English is my first language, so it's rare for me to even step\n> outside of ASCII, let alone latin1. But aren't there some languages in\n> which utf-16 is more efficient than utf-8?\n\nYou sometimes hear something along the lines of the second\n\"disadvantage\" listed in the article below, i.e. \"Characters U+0800\nthrough U+FFFF use three bytes in UTF-8, but only two in UTF-16.\":\n\nhttps://en.wikipedia.org/wiki/UTF-8#Disadvantages_4\n\n-- \nŠtěpán\n"},{"id":"178266","messageId":"7vvcrd411x.fsf@alter.siamese.dyndns.org","threadId":"28747","inReplyTo":"20111024224558.GB10481@sigill.intra.peff.net","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-25T14:07:38Z","receivedAt":"2011-10-25T14:07:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I mean, besides the obvious that UTF-16 is ...\n\nYes, you could, besides the obvious. But that obvious reason makes it\nsufficiently different that it may not be so outrageous to draw the line\nbetween it and all the others.\n"},{"id":"178370","messageId":"20111027181303.GF1967@sigill.intra.peff.net","threadId":"28747","inReplyTo":"7vvcrd411x.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-27T18:13:03Z","receivedAt":"2011-10-27T18:13:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 25, 2011 at 07:07:38AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > I mean, besides the obvious that UTF-16 is ...\n> \n> Yes, you could, besides the obvious. But that obvious reason makes it\n> sufficiently different that it may not be so outrageous to draw the line\n> between it and all the others.\n\nYeah, and I'm OK with that. It's just not a satisfying answer to give\nWindows people who think UTF-16 is a good idea. But at the very least,\nit's still unicode. It should be lossless for them to convert to utf8\nand back if they want.\n\nSpeaking of which, I've been looking at handling diffing of utf-16\nfiles. Right now we generally just consider them binary, which sucks.\nIt's easy to identify them by BOM in the is_buffer_binary() code, but\nthat's only part of it. We do an OK job of diffing them, except that:\n\n 1. The BOM makes some diffs a little noisier.\n\n 2. We split lines on 0x0a. But this byte can appear in other code\n points, like 0x010a (Ċ), or the entire entire 0x0a* code point (the\n entire Gurmukhi charset).\n\nI'm tempted to detect the UTF-{16,32}{LE,BE} by their BOM, reencode them\nto utf8, and then display them in utf8. Is that too gross for us to\nconsider?\n\nYou can kind-of implement this outside of git using textconv. But you\nhave to manually mark each file as utf-16, as there's no way to trigger\nan alternative diff driver on something like a BOM.\n\nI'm really not clear on how people with utf-16 files work. Even if we\ndid treat utf-16 like text, the _rest_ of git is outputting ascii, so\nit's not like their terminals are utf-16. But we do have projects on\ngithub with utf-16 and utf-32 encodings.\n\n-Peff\n"},{"id":"178373","messageId":"7v7h3qz2yo.fsf@alter.siamese.dyndns.org","threadId":"28747","inReplyTo":"20111027181303.GF1967@sigill.intra.peff.net","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-27T18:47:27Z","receivedAt":"2011-10-27T18:47:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I'm tempted to detect the UTF-{16,32}{LE,BE} by their BOM, reencode them\n> to utf8, and then display them in utf8. Is that too gross for us to\n> consider?\n\nI tend to think so; it is entirely a different matter if the user\ninstructed us to clean/smudge UTF-16 payload into/outof UTF-8.\n"},{"id":"178374","messageId":"20111027185220.GA26621@sigill.intra.peff.net","threadId":"28747","inReplyTo":"7v7h3qz2yo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-27T18:52:21Z","receivedAt":"2011-10-27T18:52:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 27, 2011 at 11:47:27AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > I'm tempted to detect the UTF-{16,32}{LE,BE} by their BOM, reencode them\n> > to utf8, and then display them in utf8. Is that too gross for us to\n> > consider?\n> \n> I tend to think so; it is entirely a different matter if the user\n> instructed us to clean/smudge UTF-16 payload into/outof UTF-8.\n\nMinor nit, but this is just for diff, so it is not about clean/smudge\nbut rather about doing something like textconv.\n\nThe other option I mentioned would be something like detecting the BOM\nand pretending as if the attribute \"diff=utf-16\" was set (which would do\nnothing by default). Then people could set themselves up to handle\nutf-16 if they wanted, but wouldn't have to go around marking each file\nwith .gitattributes.\n\nBut maybe that is too gross, too, and they should just use\n.gitattributes.\n\n-Peff\n"},{"id":"178375","messageId":"7v39eez1ph.fsf@alter.siamese.dyndns.org","threadId":"28747","inReplyTo":"20111027185220.GA26621@sigill.intra.peff.net","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-27T19:14:34Z","receivedAt":"2011-10-27T19:14:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Oct 27, 2011 at 11:47:27AM -0700, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > I'm tempted to detect the UTF-{16,32}{LE,BE} by their BOM, reencode them\n>> > to utf8, and then display them in utf8. Is that too gross for us to\n>> > consider?\n>> \n>> I tend to think so; it is entirely a different matter if the user\n>> instructed us to clean/smudge UTF-16 payload into/outof UTF-8.\n>\n> Minor nit, but this is just for diff, so it is not about clean/smudge\n> but rather about doing something like textconv.\n\nI can understand if some tools in the Windows land prefer to work with\nthese encodings, so clean/smudge to have the checkout in these encodings\nwould be a reasonable thing not just diff but things like grep. On the\nother hand, I do doubt the sanity of these people if they want to have\nin-repository representation also in these encodings.\n\nSo I do not think \"it is just for diff\" is any improvement.\n"},{"id":"178389","messageId":"20111027234429.GA28187@sigill.intra.peff.net","threadId":"28747","inReplyTo":"7v39eez1ph.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-27T23:44:29Z","receivedAt":"2011-10-27T23:44:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 27, 2011 at 12:14:34PM -0700, Junio C Hamano wrote:\n\n> > Minor nit, but this is just for diff, so it is not about clean/smudge\n> > but rather about doing something like textconv.\n> \n> I can understand if some tools in the Windows land prefer to work with\n> these encodings, so clean/smudge to have the checkout in these encodings\n> would be a reasonable thing not just diff but things like grep. On the\n> other hand, I do doubt the sanity of these people if they want to have\n> in-repository representation also in these encodings.\n\nI'm pretty much of the same mind. We do have people with utf-16 in their\nrepositories on github. I have no idea why they do such a thing, or what\nkinds of tricks they do to make it usable (because without it, they just\nget \"binary files differ\").\n\nMy interest is to make things like bare-repository diff (and everything\nbuilt on it; i.e., things like github, gitweb, or whatever) do the sane\nthing for these people, even if I think what they're doing is wrong. And\nas always, I try to structure the git portions of that as much as\npossible to be general and help everybody, so they can be pushed\nupstream (also, then I don't have to worry about managing local changes\n:) ).\n\nBut it sounds like this is probably just too ugly and should end up as a\ngithub-specific thing.\n\n-Peff\n"},{"id":"178390","messageId":"7v1utyx9ri.fsf@alter.siamese.dyndns.org","threadId":"28747","inReplyTo":"20111027234429.GA28187@sigill.intra.peff.net","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-28T00:03:29Z","receivedAt":"2011-10-28T00:03:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> My interest is to make things like bare-repository diff (and everything\n> built on it; i.e., things like github, gitweb, or whatever) do the sane\n> thing for these people, even if I think what they're doing is wrong.\n\nI do not think we are talking about right or wrong. I was primarily saying\nthat textconv may not be the right thing (think github/gitweb showing blob\ncontents, nicely formatted inside the chrome the site provides).\n\nThe solution you suggested feels like a gross layering violation, unless\nwe do it everywhere, in which case I wouldn't mind too much.\n\nWe have in-repository representation that diff and grep and friends work\non, and output conversion layer that externalizes the result of them in\nthe form of \"smudge\". Another layer above the in-repository representation\nand below operations could convert UTF-16 to UTF-8 when going outward and\nin the opposite when going inward.\n"},{"id":"178391","messageId":"20111028001905.GA10802@sigill.intra.peff.net","threadId":"28747","inReplyTo":"7v1utyx9ri.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-28T00:19:05Z","receivedAt":"2011-10-28T00:19:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 27, 2011 at 05:03:29PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > My interest is to make things like bare-repository diff (and everything\n> > built on it; i.e., things like github, gitweb, or whatever) do the sane\n> > thing for these people, even if I think what they're doing is wrong.\n> \n> I do not think we are talking about right or wrong. I was primarily saying\n> that textconv may not be the right thing (think github/gitweb showing blob\n> contents, nicely formatted inside the chrome the site provides).\n\nBut I think it is probably a wrong thing to store utf-16 as the\ncanonical format inside the git repository. Git simply can't handle it\nfor diffing. And the right thing, as you suggested, is clean/smudge.\n\nBut I'm dealing with repositories on the server side, where it is too\nlate to do clean/smudge; I just get whatever junk people commited.\n\n> We have in-repository representation that diff and grep and friends work\n> on, and output conversion layer that externalizes the result of them in\n> the form of \"smudge\". Another layer above the in-repository representation\n> and below operations could convert UTF-16 to UTF-8 when going outward and\n> in the opposite when going inward.\n\nI'm not sure that could sanely be done in a backwards compatible way.\nDoing it with just textual diffs is a hack, of course, but at least we\nknow that the damage is limited, and the diff we generate on top doesn't\ncare that much about the original sha1s[1]. But should read_object_sha1\nlearn to convert utf-16 into utf-8? I think madness lies that way, as\nwe are breaking assumptions about sha1 validity.\n\n-Peff\n\n[1] Actually, the text diff does mention the original and resulting\nsha1s, which would now either bear no relation to the diff text, or bear\nno relation to what's in the repo. Either way, I think we are creating\nsomething that can't necessarily be applied, which is bad. And is why I\nthought of textconv, which is basically the same concept (and has the\nsame problems).\n"},{"id":"178393","messageId":"buo39edao6r.fsf@dhlpc061.dev.necel.com","threadId":"28747","inReplyTo":"20111027234429.GA28187@sigill.intra.peff.net","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2011-10-28T01:40:28Z","receivedAt":"2011-10-28T01:40:28Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Jeff King writes:\n> We do have people with utf-16 in their repositories on github. I\n> have no idea why they do such a thing, or what kinds of tricks they\n> do to make it usable (because without it, they just get \"binary\n> files differ\").\n\nHmm, you could ask them ... [or, I suppose more diplomatically, post\na blog entry asking \"Hey all you people who use github for utf-16\nencoded files, ...\"]\n\n-Miles\n\n-- \nYear, n. A period of three hundred and sixty-five disappointments.\n"},{"id":"178396","messageId":"7vr51xwyg7.fsf@alter.siamese.dyndns.org","threadId":"28747","inReplyTo":"buo39edao6r.fsf@dhlpc061.dev.necel.com","subject":"Re: [PATCH 00/22] Refactor to accept NUL in commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-28T04:07:52Z","receivedAt":"2011-10-28T04:07:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miles Bader writes:\n\n> Jeff King writes:\n>> We do have people with utf-16 in their repositories on github. I\n>> have no idea why they do such a thing, or what kinds of tricks they\n>> do to make it usable (because without it, they just get \"binary\n>> files differ\").\n>\n> Hmm, you could ask them ... [or, I suppose more diplomatically, post\n> a blog entry asking \"Hey all you people who use github for utf-16\n> encoded files, ...\"]\n\nPeople will hate such a flag day event initially and later thank you for\nit ;-) \n\nI am afraid that that is a wishful thinking, though.\n"}]}