{"thread":{"id":"50430","subject":"t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","startedAt":"2019-02-07T21:59:38Z","lastAt":"2019-02-12T02:43:33Z","messageCount":30,"participants":["Kevin Daudt","brian m. carlson","Rich Felker","Junio C Hamano","Torsten Bögershausen","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"368744","messageId":"20190207215935.GA31515@alpha","threadId":"50430","inReplyTo":null,"subject":"t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2019-02-07T21:59:35Z","receivedAt":"2019-02-07T21:59:38Z","isPatch":false,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"I'm trying to get the git test suite passing on Alpine Linux, which is\nbased on musl libc.\n\nAll tests in t0028-working-tree-encoding.sh are currently failing,\nbecause musl iconv does not support statefull output of UTF-16/32 (eg,\nit does not output a BOM), while git is expecting that to be present:\n\n> hint: The file 'test.utf16' is missing a byte order mark (BOM). Please\n> use UTF-16BE or UTF-16LE (depending on the byte order) as\n> working-tree-encoding.\n> fatal: BOM is required in 'test.utf16' if encoded as utf-16\n\nBecause adding the file to get fails, all the other tests fail as well\nas they expect the file to be present in the repository.\n\nAny idea how to get around this?\n\nKevin\n"},{"id":"368760","messageId":"20190208001705.GC11927@genre.crustytoothpaste.net","threadId":"50430","inReplyTo":"20190207215935.GA31515@alpha","subject":"Re: t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-08T00:17:05Z","receivedAt":"2019-02-08T00:17:15Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"[Please skip using Reply-To and instead of Mail-Followup-To so that\nresponses also go to the list.]\n\nOn Thu, Feb 07, 2019 at 10:59:35PM +0100, Kevin Daudt wrote:\n> I'm trying to get the git test suite passing on Alpine Linux, which is\n> based on musl libc.\n> \n> All tests in t0028-working-tree-encoding.sh are currently failing,\n> because musl iconv does not support statefull output of UTF-16/32 (eg,\n> it does not output a BOM), while git is expecting that to be present:\n> \n> > hint: The file 'test.utf16' is missing a byte order mark (BOM). Please\n> > use UTF-16BE or UTF-16LE (depending on the byte order) as\n> > working-tree-encoding.\n> > fatal: BOM is required in 'test.utf16' if encoded as utf-16\n> \n> Because adding the file to get fails, all the other tests fail as well\n> as they expect the file to be present in the repository.\n> \n> Any idea how to get around this?\n\nI think musl needs to patch their libc. RFC 2781 says that if there's no\nBOM in UTF-16, then \"the text SHOULD be interpreted as being\nbig-endian.\"\n\nUnfortunately for all of us, many Windows-based programs have chosen to\nignore that advice (technically, it's only a SHOULD) and interpret it as\nlittle-endian instead. Git can't safely assume anything about the\nendianness of a UTF-16 stream that doesn't contain a BOM. Technically,\nsince the RFC doesn't specify a MUST requirement, musl can't, either.\n\nEven if Git were to produce a BOM to work around this issue, then we'd\nstill have the problem that any program using musl will write data in\nUTF-16 without a BOM. Moreover, because musl, in violation of the RFC,\ndoesn't read and process BOMs, someone using little-endian UTF-16 (with\na proper BOM) with musl and Git will have their data corrupted,\naccording to my reading of the musl website.\n\nIn other words, I believe this test is failing legitimately.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"368778","messageId":"20190208060403.GA29788@brightrain.aerifal.cx","threadId":"50430","inReplyTo":"20190208001705.GC11927@genre.crustytoothpaste.net","subject":"Re: t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","fromName":"Rich Felker","fromEmail":"dalias@libc.org","sentAt":"2019-02-08T06:04:03Z","receivedAt":"2019-02-08T06:15:48Z","isPatch":false,"sender":{"key":"dalias@libc.org","avatar":null},"body":"On Fri, Feb 08, 2019 at 12:17:05AM +0000, brian m. carlson wrote:\n> [Please skip using Reply-To and instead of Mail-Followup-To so that\n> responses also go to the list.]\n> \n> On Thu, Feb 07, 2019 at 10:59:35PM +0100, Kevin Daudt wrote:\n> > I'm trying to get the git test suite passing on Alpine Linux, which is\n> > based on musl libc.\n> > \n> > All tests in t0028-working-tree-encoding.sh are currently failing,\n> > because musl iconv does not support statefull output of UTF-16/32 (eg,\n> > it does not output a BOM), while git is expecting that to be present:\n> > \n> > > hint: The file 'test.utf16' is missing a byte order mark (BOM). Please\n> > > use UTF-16BE or UTF-16LE (depending on the byte order) as\n> > > working-tree-encoding.\n> > > fatal: BOM is required in 'test.utf16' if encoded as utf-16\n> > \n> > Because adding the file to get fails, all the other tests fail as well\n> > as they expect the file to be present in the repository.\n> > \n> > Any idea how to get around this?\n> \n> I think musl needs to patch their libc. RFC 2781 says that if there's no\n> BOM in UTF-16, then \"the text SHOULD be interpreted as being\n> big-endian.\"\n> \n> Unfortunately for all of us, many Windows-based programs have chosen to\n> ignore that advice (technically, it's only a SHOULD) and interpret it as\n> little-endian instead. Git can't safely assume anything about the\n> endianness of a UTF-16 stream that doesn't contain a BOM. Technically,\n> since the RFC doesn't specify a MUST requirement, musl can't, either.\n> \n> Even if Git were to produce a BOM to work around this issue, then we'd\n> still have the problem that any program using musl will write data in\n> UTF-16 without a BOM. Moreover, because musl, in violation of the RFC,\n> doesn't read and process BOMs, someone using little-endian UTF-16 (with\n> a proper BOM) with musl and Git will have their data corrupted,\n> according to my reading of the musl website.\n\nThat information is outdated and someone from our side should update\nit; since 1.1.19, musl treats \"UTF-16\" input as ambiguous endianness\ndetermined by BOM, defaulting to big if there's no BOM. However output\nis always big endian, such that processes conforming to the Unicode\nSHOULD clause will interpret it correctly.\n\nThe portable way to get little endian with a BOM is to open a\nconversion descriptor for \"UTF-16LE\" (which should not add any BOM)\nand write a BOM manually.\n\nIn any case, this test seems mainly relevant to Windows users wanting\nto store source files in UTF-16LE with BOM. This doesn't really make\nsense to do on a Linux/musl system, so I'm not sure any action is\nneeded here from either side.\n\nRich\n"},{"id":"368830","messageId":"20190208114502.GD11927@genre.crustytoothpaste.net","threadId":"50430","inReplyTo":"20190208060403.GA29788@brightrain.aerifal.cx","subject":"Re: t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-08T11:45:02Z","receivedAt":"2019-02-08T11:45:14Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, Feb 08, 2019 at 01:04:03AM -0500, Rich Felker wrote:\n> That information is outdated and someone from our side should update\n> it; since 1.1.19, musl treats \"UTF-16\" input as ambiguous endianness\n> determined by BOM, defaulting to big if there's no BOM. However output\n> is always big endian, such that processes conforming to the Unicode\n> SHOULD clause will interpret it correctly.\n\nIt's good to hear that musl now supports parsing UTF-16 BOMs.\n\n> The portable way to get little endian with a BOM is to open a\n> conversion descriptor for \"UTF-16LE\" (which should not add any BOM)\n> and write a BOM manually.\n\nRight, I think my point is that we have existing systems which we know\nignore the SHOULD and assume something different. Perhaps in retrospect,\nit would have been better to use MUST to specify areas where\ninteroperability is a concern.\n\n> In any case, this test seems mainly relevant to Windows users wanting\n> to store source files in UTF-16LE with BOM. This doesn't really make\n> sense to do on a Linux/musl system, so I'm not sure any action is\n> needed here from either side.\n\nI do know that some people use CIFS or the like to share repositories\nbetween Unix and Windows. However, I agree that such people aren't\nlikely to use UTF-16 on Unix systems. The working tree encoding\nfunctionality also supports other encodings which musl may or may not\nsupport.\n\nIf you and your users are comfortable with the fact that the test (and\nthe corresponding functionality) won't work as expected with UTF-16,\nthen I agree that no action is needed.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"368832","messageId":"20190208115511.GA30779@alpha","threadId":"50430","inReplyTo":"20190208114502.GD11927@genre.crustytoothpaste.net","subject":"Re: t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2019-02-08T11:55:11Z","receivedAt":"2019-02-08T11:55:14Z","isPatch":false,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"On Fri, Feb 08, 2019 at 11:45:02AM +0000, brian m. carlson wrote:\n> On Fri, Feb 08, 2019 at 01:04:03AM -0500, Rich Felker wrote:\n> [..]\n> > In any case, this test seems mainly relevant to Windows users wanting\n> > to store source files in UTF-16LE with BOM. This doesn't really make\n> > sense to do on a Linux/musl system, so I'm not sure any action is\n> > needed here from either side.\n> \n> I do know that some people use CIFS or the like to share repositories\n> between Unix and Windows. However, I agree that such people aren't\n> likely to use UTF-16 on Unix systems. The working tree encoding\n> functionality also supports other encodings which musl may or may not\n> support.\n> \n> If you and your users are comfortable with the fact that the test (and\n> the corresponding functionality) won't work as expected with UTF-16,\n> then I agree that no action is needed.\n\nSo would you suggest that we just skip this test on Alpine Linux?\n\n"},{"id":"368835","messageId":"20190208135137.GE11927@genre.crustytoothpaste.net","threadId":"50430","inReplyTo":"20190208115511.GA30779@alpha","subject":"Re: t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-08T13:51:38Z","receivedAt":"2019-02-08T13:51:46Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, Feb 08, 2019 at 12:55:11PM +0100, Kevin Daudt wrote:\n> On Fri, Feb 08, 2019 at 11:45:02AM +0000, brian m. carlson wrote:\n> > On Fri, Feb 08, 2019 at 01:04:03AM -0500, Rich Felker wrote:\n> > [..]\n> > > In any case, this test seems mainly relevant to Windows users wanting\n> > > to store source files in UTF-16LE with BOM. This doesn't really make\n> > > sense to do on a Linux/musl system, so I'm not sure any action is\n> > > needed here from either side.\n> > \n> > I do know that some people use CIFS or the like to share repositories\n> > between Unix and Windows. However, I agree that such people aren't\n> > likely to use UTF-16 on Unix systems. The working tree encoding\n> > functionality also supports other encodings which musl may or may not\n> > support.\n> > \n> > If you and your users are comfortable with the fact that the test (and\n> > the corresponding functionality) won't work as expected with UTF-16,\n> > then I agree that no action is needed.\n> \n> So would you suggest that we just skip this test on Alpine Linux?\n\nThat's not exactly what I said. If Alpine Linux users are never going to\nuse this functionality and don't care that it's broken, then that's a\nfine solution.\n\nAs originally mentioned, musl could change its libiconv to write a BOM,\nwhich would make it compatible with other known iconv implementations.\n\nThere's also the possibility of defining NO_ICONV. That basically means\nthat your system won't support encodings, and then this test shouldn't\nmatter.\n\nFinally, you could try applying a patch to the test to make it write the\nBOM for UTF-16 since your iconv doesn't. I expect that the test will\nfail again later on once you've done that, though.\n\nI don't have musl so I can't test a patch, but theoretically, you could\nuse a Makefile variable and lazy test prerequisite to control the\nbehavior of the code and test, and we could apply a patch. I'll try to\nwork something up later today which might work and which you could test.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"368837","messageId":"20190208161332.GW23599@brightrain.aerifal.cx","threadId":"50430","inReplyTo":"20190208115511.GA30779@alpha","subject":"Re: t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","fromName":"Rich Felker","fromEmail":"dalias@libc.org","sentAt":"2019-02-08T16:13:32Z","receivedAt":"2019-02-08T16:13:41Z","isPatch":false,"sender":{"key":"dalias@libc.org","avatar":null},"body":"On Fri, Feb 08, 2019 at 12:55:11PM +0100, Kevin Daudt wrote:\n> On Fri, Feb 08, 2019 at 11:45:02AM +0000, brian m. carlson wrote:\n> > On Fri, Feb 08, 2019 at 01:04:03AM -0500, Rich Felker wrote:\n> > [..]\n> > > In any case, this test seems mainly relevant to Windows users wanting\n> > > to store source files in UTF-16LE with BOM. This doesn't really make\n> > > sense to do on a Linux/musl system, so I'm not sure any action is\n> > > needed here from either side.\n> > \n> > I do know that some people use CIFS or the like to share repositories\n> > between Unix and Windows. However, I agree that such people aren't\n> > likely to use UTF-16 on Unix systems. The working tree encoding\n> > functionality also supports other encodings which musl may or may not\n> > support.\n> > \n> > If you and your users are comfortable with the fact that the test (and\n> > the corresponding functionality) won't work as expected with UTF-16,\n> > then I agree that no action is needed.\n> \n> So would you suggest that we just skip this test on Alpine Linux?\n\nI'm fine with that as an outcome here. Admittedly I'd rather see git\ndo this in a way that doesn't make assumptions about what an ambiguous\n\"UTF-16\" encoding argument to iconv_open does, but nothing is actually\nbreaking because of this.\n\nRich\n"},{"id":"368845","messageId":"xmqqr2cikw4w.fsf@gitster-ct.c.googlers.com","threadId":"50430","inReplyTo":"20190208135137.GE11927@genre.crustytoothpaste.net","subject":"Re: t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-08T17:50:07Z","receivedAt":"2019-02-08T17:50:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n>> So would you suggest that we just skip this test on Alpine Linux?\n>\n> That's not exactly what I said. If Alpine Linux users are never going to\n> use this functionality and don't care that it's broken, then that's a\n> fine solution.\n>\n> As originally mentioned, musl could change its libiconv to write a BOM,\n> which would make it compatible with other known iconv implementations.\n>\n> There's also the possibility of defining NO_ICONV. That basically means\n> that your system won't support encodings, and then this test shouldn't\n> matter.\n>\n> Finally, you could try applying a patch to the test to make it write the\n> BOM for UTF-16 since your iconv doesn't. I expect that the test will\n> fail again later on once you've done that, though.\n\nSorry for being late to the party, but is the crux of the issue this\npiece early in the test?\n\n    printf \"$text\" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&\n    ...\n    cp test.utf16.raw test.utf16 &&\n    ...\n    git add .gitattributes test.utf16 test.utf16lebom &&\n\nwhere we expect \"iconv -t UTF-16\" means \"write UTF16 in whatever\nbyteorder of your choice, but do write BOM\", and iconv\nimplementations we have seen so far are in line with that\nexpectation, but the one on Apline writes UTF16 in big endian\nwithout BOM?\n\nIf that is the case, I think it is our expectation that is at fault\nin this case, as I think the most natural interpretation of \"UTF-16\"\nwithout any modifiers (like \"BE\") ought to be \"UTF16 stream\nexpressed in any way of writers choice, as long as it is readable by\nstandard compliant readers\", in other words, \"write UTF16 in\nwhatever byteorder of your choice, with or without BOM, but if you\nomit BOM, you SHOULD write in big endian\".  So\n\n - If our later test assumes that test.utf16 is UTF16 with BOM, that\n   already assumes too much;\n\n - If our later test assumes that test.utf16 is UTF16 in big endian,\n   that assumes too much, too.\n\nAs suggested earlier in the thread, the easiest workaround would be\nto update the preparation of test.utf16.raw may to force big endian\nwith BOM by preprending BE-BOM by hand before \"iconv -t UTF-32BE\"\noutput (I am assuming that UTF-32BE will stay to be \"big endian\nwithout BOM\" in the future).  That would make sure that the\nassumption later tests have on test.utf16 is held true.\n"},{"id":"368877","messageId":"20190208202336.GA5284@alpha","threadId":"50430","inReplyTo":"xmqqr2cikw4w.fsf@gitster-ct.c.googlers.com","subject":"Re: t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2019-02-08T20:23:36Z","receivedAt":"2019-02-08T20:23:39Z","isPatch":false,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"On Fri, Feb 08, 2019 at 09:50:07AM -0800, Junio C Hamano wrote:\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n> \n> >> So would you suggest that we just skip this test on Alpine Linux?\n> >\n> > That's not exactly what I said. If Alpine Linux users are never going to\n> > use this functionality and don't care that it's broken, then that's a\n> > fine solution.\n> >\n> > As originally mentioned, musl could change its libiconv to write a BOM,\n> > which would make it compatible with other known iconv implementations.\n> >\n> > There's also the possibility of defining NO_ICONV. That basically means\n> > that your system won't support encodings, and then this test shouldn't\n> > matter.\n> >\n> > Finally, you could try applying a patch to the test to make it write the\n> > BOM for UTF-16 since your iconv doesn't. I expect that the test will\n> > fail again later on once you've done that, though.\n> \n> Sorry for being late to the party, but is the crux of the issue this\n> piece early in the test?\n> \n>     printf \"$text\" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&\n>     ...\n>     cp test.utf16.raw test.utf16 &&\n>     ...\n>     git add .gitattributes test.utf16 test.utf16lebom &&\n> \n> where we expect \"iconv -t UTF-16\" means \"write UTF16 in whatever\n> byteorder of your choice, but do write BOM\", and iconv\n> implementations we have seen so far are in line with that\n> expectation, but the one on Apline writes UTF16 in big endian\n> without BOM?\n\nFirstly, the tests expect iconv -t UTF-16 to output a BOM, which it\nindeed does not do on Alpine. Secondly, git itself also expects the BOM\nto be present when the encoding is set to UTF-16, otherwise it will\ncomplain.\n\n> \n> If that is the case, I think it is our expectation that is at fault\n> in this case, as I think the most natural interpretation of \"UTF-16\"\n> without any modifiers (like \"BE\") ought to be \"UTF16 stream\n> expressed in any way of writers choice, as long as it is readable by\n> standard compliant readers\", in other words, \"write UTF16 in\n> whatever byteorder of your choice, with or without BOM, but if you\n> omit BOM, you SHOULD write in big endian\".  So\n> \n>  - If our later test assumes that test.utf16 is UTF16 with BOM, that\n>    already assumes too much;\n> \n>  - If our later test assumes that test.utf16 is UTF16 in big endian,\n>    that assumes too much, too.\n> \n> As suggested earlier in the thread, the easiest workaround would be\n> to update the preparation of test.utf16.raw may to force big endian\n> with BOM by preprending BE-BOM by hand before \"iconv -t UTF-32BE\"\n> output (I am assuming that UTF-32BE will stay to be \"big endian\n> without BOM\" in the future).  That would make sure that the\n> assumption later tests have on test.utf16 is held true.\n\nI tried change the test to manually inject a BOM to the file (and\nsetting iconv to UTF-16LE / UTF16-BE, which lets the first test go\nthrough, but test 3 then fails, because git itself output the file\nwithout BOM, presumably because it's passed through iconv.\n\nSo I'm not sure if it's a matter of just fixing the tests.\n\n"},{"id":"368882","messageId":"20190208204219.GF11927@genre.crustytoothpaste.net","threadId":"50430","inReplyTo":"20190208202336.GA5284@alpha","subject":"Re: t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-08T20:42:19Z","receivedAt":"2019-02-08T20:42:29Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, Feb 08, 2019 at 09:23:36PM +0100, Kevin Daudt wrote:\n> Firstly, the tests expect iconv -t UTF-16 to output a BOM, which it\n> indeed does not do on Alpine. Secondly, git itself also expects the BOM\n> to be present when the encoding is set to UTF-16, otherwise it will\n> complain.\n\nYeah, we definitely want to require a BOM for UTF-16. As previously\nmentioned, it isn't safe for us to assume big-endian when it's missing.\n\n> I tried change the test to manually inject a BOM to the file (and\n> setting iconv to UTF-16LE / UTF16-BE, which lets the first test go\n> through, but test 3 then fails, because git itself output the file\n> without BOM, presumably because it's passed through iconv.\n> \n> So I'm not sure if it's a matter of just fixing the tests.\n\nI think something like the following will likely work in this scenario:\n\n------ %< ---------\nFrom: \"brian m. carlson\" <sandals@crustytoothpaste.net>\nDate: Fri, 8 Feb 2019 12:58:11 +0000\nSubject: [PATCH] WIP: utf8: handle missing musl UTF-16 BOM\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/t0028-working-tree-encoding.sh | 20 ++++++++++++++++++--\n utf8.c                           |  4 ++++\n 2 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex e58ecbfc44..ff02d03bad 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -6,6 +6,22 @@ test_description='working-tree-encoding conversion via gitattributes'\n \n GIT_TRACE_WORKING_TREE_ENCODING=1 && export GIT_TRACE_WORKING_TREE_ENCODING\n \n+test_lazy_prereq NO_BOM '\n+\tprintf abc | iconv -f UTF-8 -t UTF-16 &&\n+\ttest $(wc -c) = 6\n+'\n+\n+write_utf16 () {\n+\ttest_have_prereq NO_BOM && printf '\\xfe\\xff'\n+\ticonv -f UTF-8 -t UTF-16\n+\n+}\n+\n+write_utf32 () {\n+\ttest_have_prereq NO_BOM && printf '\\x00\\x00\\xfe\\xff'\n+\ticonv -f UTF-8 -t UTF-32\n+}\n+\n test_expect_success 'setup test files' '\n \tgit config core.eol lf &&\n \n@@ -13,8 +29,8 @@ test_expect_success 'setup test files' '\n \techo \"*.utf16 text working-tree-encoding=utf-16\" >.gitattributes &&\n \techo \"*.utf16lebom text working-tree-encoding=UTF-16LE-BOM\" >>.gitattributes &&\n \tprintf \"$text\" >test.utf8.raw &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-32 >test.utf32.raw &&\n+\tprintf \"$text\" | write_utf16 >test.utf16.raw &&\n+\tprintf \"$text\" | write_utf32 >test.utf32.raw &&\n \tprintf \"\\377\\376\"                         >test.utf16lebom.raw &&\n \tprintf \"$text\" | iconv -f UTF-8 -t UTF-32LE >>test.utf16lebom.raw &&\n \ndiff --git a/utf8.c b/utf8.c\nindex 83824dc2f4..4aa69cd65b 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -568,6 +568,10 @@ char *reencode_string_len(const char *in, size_t insz,\n \t\tbom_str = utf16_be_bom;\n \t\tbom_len = sizeof(utf16_be_bom);\n \t\tout_encoding = \"UTF-16BE\";\n+\t} else if (same_utf_encoding(\"UTF-16\", out_encoding)) {\n+\t\tbom_str = utf16_le_bom;\n+\t\tbom_len = sizeof(utf16_le_bom);\n+\t\tout_encoding = \"UTF-16LE\";\n \t}\n \n \tconv = iconv_open(out_encoding, in_encoding);\n------ %< ---------\n\nThis passes for me on glibc, but only on a little-endian system. If this\nworks for musl folks, then I'll add a config option for those people who\nhave UTF-16 without BOM.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"368899","messageId":"xmqqd0o1kh7t.fsf@gitster-ct.c.googlers.com","threadId":"50430","inReplyTo":"20190208204219.GF11927@genre.crustytoothpaste.net","subject":"Re: t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-08T23:12:22Z","receivedAt":"2019-02-08T23:12:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> +test_lazy_prereq NO_BOM '\n> +\tprintf abc | iconv -f UTF-8 -t UTF-16 &&\n> +\ttest $(wc -c) = 6\n> +'\n\nThis must be \"just for illustration of idea\" patch?  The pipeline\ngoes to the standard output, and nobody feeds \"wc\".\n\nBut I think I got the idea.\n\nIn the real implementation, it probably is a good idea to allow\nNO_BOM16 and NO_BOM32 be orthogonal.\n\n\n> +\n> +write_utf16 () {\n> +\ttest_have_prereq NO_BOM && printf '\\xfe\\xff'\n> +\ticonv -f UTF-8 -t UTF-16\n\nThis assumes \"iconv -t UTF-16\" on the platform gives little endian\n(with or without BOM), which may not be a good assumption.\n\nIf you are forcing the world to be where UTF-16 (no other\nspecificaiton) means LE with BOM, then perhaps doing\n\n\tprintf '\\xfe\\xff'; iconv -f UTF-8 -t UTF-16LE\n\nwithout any lazy prereq may be more explicit and in line with what\nyou did in utf8.c::reencode_string_len() below.\n\n> -\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&\n> -\tprintf \"$text\" | iconv -f UTF-8 -t UTF-32 >test.utf32.raw &&\n> +\tprintf \"$text\" | write_utf16 >test.utf16.raw &&\n> +\tprintf \"$text\" | write_utf32 >test.utf32.raw &&\n\n> diff --git a/utf8.c b/utf8.c\n> index 83824dc2f4..4aa69cd65b 100644\n> --- a/utf8.c\n> +++ b/utf8.c\n> @@ -568,6 +568,10 @@ char *reencode_string_len(const char *in, size_t insz,\n>  \t\tbom_str = utf16_be_bom;\n>  \t\tbom_len = sizeof(utf16_be_bom);\n>  \t\tout_encoding = \"UTF-16BE\";\n> +\t} else if (same_utf_encoding(\"UTF-16\", out_encoding)) {\n> +\t\tbom_str = utf16_le_bom;\n> +\t\tbom_len = sizeof(utf16_le_bom);\n> +\t\tout_encoding = \"UTF-16LE\";\n>  \t}\n\nI am not sure what is going on here.  When the caller asks for\n\"UTF-16\", we do not let the platform implementation of iconv() to\npick one of the allowed ones (i.e. BE with BOM, LE with BOM, or BE\nwithout BOM) but instead force LE with BOM?\n\n>  \tconv = iconv_open(out_encoding, in_encoding);\n> ------ %< ---------\n>\n> This passes for me on glibc, but only on a little-endian system. If this\n> works for musl folks, then I'll add a config option for those people who\n> have UTF-16 without BOM.\n"},{"id":"368903","messageId":"20190209002458.GJ11927@genre.crustytoothpaste.net","threadId":"50430","inReplyTo":"xmqqd0o1kh7t.fsf@gitster-ct.c.googlers.com","subject":"Re: t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-09T00:24:59Z","receivedAt":"2019-02-09T00:25:09Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, Feb 08, 2019 at 03:12:22PM -0800, Junio C Hamano wrote:\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n> \n> > +test_lazy_prereq NO_BOM '\n> > +\tprintf abc | iconv -f UTF-8 -t UTF-16 &&\n> > +\ttest $(wc -c) = 6\n> > +'\n> \n> This must be \"just for illustration of idea\" patch?  The pipeline\n> goes to the standard output, and nobody feeds \"wc\".\n\nWell, as I said, I have no way to test this. This code path works fine\non glibc because of course we would never exercise the prerequisite.\n\nBut I do appreciate you pointing it out. I'll fix it and send another\ntest patch.\n\n> But I think I got the idea.\n> \n> In the real implementation, it probably is a good idea to allow\n> NO_BOM16 and NO_BOM32 be orthogonal.\n\nSure.\n\n> > +\n> > +write_utf16 () {\n> > +\ttest_have_prereq NO_BOM && printf '\\xfe\\xff'\n> > +\ticonv -f UTF-8 -t UTF-16\n> \n> This assumes \"iconv -t UTF-16\" on the platform gives little endian\n> (with or without BOM), which may not be a good assumption.\n> \n> If you are forcing the world to be where UTF-16 (no other\n> specificaiton) means LE with BOM, then perhaps doing\n> \n> \tprintf '\\xfe\\xff'; iconv -f UTF-8 -t UTF-16LE\n> \n> without any lazy prereq may be more explicit and in line with what\n> you did in utf8.c::reencode_string_len() below.\n\nNo, I believe it assumes big-endian (BOM is U+FEFF). The only\njustifiable argument for endianness that doesn't include a BOM is\nbig-endian, because that's the only one supported by the RFC and the\nUnicode standard.\n\nI can't actually write it without the prerequisite because I don't plan\nto trigger the code I wrote below unless required. The endianness we\npick in the code and the tests has to be the same. If we're on glibc,\nthe code will use the glibc implementation, which is little-endian, and\ntherefore will need the tests to be little-endian as well.\n\nI will explain this thoroughly in the commit message, because it is\nindeed quite subtle.\n\n> > diff --git a/utf8.c b/utf8.c\n> > index 83824dc2f4..4aa69cd65b 100644\n> > --- a/utf8.c\n> > +++ b/utf8.c\n> > @@ -568,6 +568,10 @@ char *reencode_string_len(const char *in, size_t insz,\n> >  \t\tbom_str = utf16_be_bom;\n> >  \t\tbom_len = sizeof(utf16_be_bom);\n> >  \t\tout_encoding = \"UTF-16BE\";\n> > +\t} else if (same_utf_encoding(\"UTF-16\", out_encoding)) {\n> > +\t\tbom_str = utf16_le_bom;\n> > +\t\tbom_len = sizeof(utf16_le_bom);\n> > +\t\tout_encoding = \"UTF-16LE\";\n> >  \t}\n> \n> I am not sure what is going on here.  When the caller asks for\n> \"UTF-16\", we do not let the platform implementation of iconv() to\n> pick one of the allowed ones (i.e. BE with BOM, LE with BOM, or BE\n> without BOM) but instead force LE with BOM?\n\nThis is more of a test scenario to see how it works for musl. My\nproposal is that if this is sufficient to fix problems for musl, then we\nwrap it inside a #define (and Makefile) knob, and let users with an\niconv that doesn't write the BOM turn it on. Now that I'm thinking about\nit, it will probably need to be big-endian for compatibility with the\ntests.\n\nI plan to treat this as a platform-specific wart much like\nFREAD_READS_DIRECTORIES. Inspecting the stream would be complicated and\nnot performant, and I'm not aware of any other iconv implementations\nthat have this behavior (because it causes unhappiness with Windows,\nwhich is the primary consumer of UTF-16), so I think a compile-time\noption is the way to go.\n\nI'll try to reroll with a formal test patch this evening or tomorrow.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"368913","messageId":"0c3b77bf-2903-1cd0-0fce-2ec01be91d84@web.de","threadId":"50430","inReplyTo":"20190208060403.GA29788@brightrain.aerifal.cx","subject":"Re: t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2019-02-09T08:09:40Z","receivedAt":"2019-02-09T08:10:01Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 08.02.19 07:04, Rich Felker wrote:\n> On Fri, Feb 08, 2019 at 12:17:05AM +0000, brian m. carlson wrote:\n\n[]\n>> Even if Git were to produce a BOM to work around this issue, then we'd\n>> still have the problem that any program using musl will write data in\n>> UTF-16 without a BOM. Moreover, because musl, in violation of the RFC,\n>> doesn't read and process BOMs, someone using little-endian UTF-16 (with\n>> a proper BOM) with musl and Git will have their data corrupted,\n>> according to my reading of the musl website.\n>\n> That information is outdated and someone from our side should update\n> it; since 1.1.19, musl treats \"UTF-16\" input as ambiguous endianness\n> determined by BOM, defaulting to big if there's no BOM. However output\n> is always big endian, such that processes conforming to the Unicode\n> SHOULD clause will interpret it correctly.\n>\n> The portable way to get little endian with a BOM is to open a\n> conversion descriptor for \"UTF-16LE\" (which should not add any BOM)\n> and write a BOM manually.\n>\n\nThat is possible in the next upcoming version of Git:\n\ncommit 0fa3cc77ee9fb3b6bb53c73688c9b7500f996b83\nMerge: cfd9167c15 aab2a1ae48\nAuthor: Junio C Hamano <gitster@pobox.com>\nDate:   Wed Feb 6 22:05:21 2019 -0800\n\n    Merge branch 'tb/utf-16-le-with-explicit-bom'\n\n    A new encoding UTF-16LE-BOM has been invented to force encoding to\n    UTF-16 with BOM in little endian byte order, which cannot be directly\n    generated by using iconv.\n\n    * tb/utf-16-le-with-explicit-bom:\n      Support working-tree-encoding \"UTF-16LE-BOM\"\n\n\n"},{"id":"368922","messageId":"20190209145732.GA14229@alpha","threadId":"50430","inReplyTo":"20190208204219.GF11927@genre.crustytoothpaste.net","subject":"Re: t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2019-02-09T14:57:32Z","receivedAt":"2019-02-09T14:57:36Z","isPatch":false,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"On Fri, Feb 08, 2019 at 08:42:19PM +0000, brian m. carlson wrote:\n> On Fri, Feb 08, 2019 at 09:23:36PM +0100, Kevin Daudt wrote:\n> > Firstly, the tests expect iconv -t UTF-16 to output a BOM, which it\n> > indeed does not do on Alpine. Secondly, git itself also expects the BOM\n> > to be present when the encoding is set to UTF-16, otherwise it will\n> > complain.\n> \n> Yeah, we definitely want to require a BOM for UTF-16. As previously\n> mentioned, it isn't safe for us to assume big-endian when it's missing.\n> \n> > I tried change the test to manually inject a BOM to the file (and\n> > setting iconv to UTF-16LE / UTF16-BE, which lets the first test go\n> > through, but test 3 then fails, because git itself output the file\n> > without BOM, presumably because it's passed through iconv.\n> > \n> > So I'm not sure if it's a matter of just fixing the tests.\n> \n> I think something like the following will likely work in this scenario:\n> \n> [..]\n> \n> This passes for me on glibc, but only on a little-endian system. If this\n> works for musl folks, then I'll add a config option for those people who\n> have UTF-16 without BOM.\n> -- \n> brian m. carlson: Houston, Texas, US\n> OpenPGP: https://keybase.io/bk2204\n\nI tried your patch. The pre-requisite is broken in it's current form,\nthis would fix the prerequisite:\n\n    diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\n    index ff02d03bad..734c5a7dbb 100755\n    --- a/t/t0028-working-tree-encoding.sh\n    +++ b/t/t0028-working-tree-encoding.sh\n    @@ -7,8 +7,7 @@ test_description='working-tree-encoding conversion via gitattributes'\n     GIT_TRACE_WORKING_TREE_ENCODING=1 && export GIT_TRACE_WORKING_TREE_ENCODING\n\n     test_lazy_prereq NO_BOM '\n    -       printf abc | iconv -f UTF-8 -t UTF-16 &&\n    -       test $(wc -c) = 6\n    +       test $(printf abc | iconv -f UTF-8 -t UTF-16 | wc -c) = 6\n     '\n\n     write_utf16 () {\n\nBut test 3 still fails, because now the output from git is converted to\nUTF16-LE, which is different from the input, which is UTF16-BE.\n"},{"id":"368942","messageId":"20190209200802.277139-1-sandals@crustytoothpaste.net","threadId":"50430","inReplyTo":"20190209145732.GA14229@alpha","subject":"[PATCH] utf8: handle systems that don't write BOM for UTF-16","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-09T20:08:01Z","receivedAt":"2019-02-09T20:08:20Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When serializing UTF-16 (and UTF-32), there are three possible ways to\nwrite the stream. One can write the data with a BOM in either big-endian\nor little-endian format, or one can write the data without a BOM in\nbig-endian format.\n\nMost systems' iconv implementations choose to write it with a BOM in\nsome endianness, since this is the most foolproof, and it is resistant\nto misinterpretation on Windows, where UTF-16 and the little-endian\nserialization are very common. For compatibility with Windows and to\navoid accidental misuse there, Git always wants to write UTF-16 with a\nBOM, and will refuse to read UTF-16 without it.\n\nHowever, musl's iconv implementation writes UTF-16 without a BOM,\nrelying on the user to interpret it as big-endian. This causes t0028 and\nthe related functionality to fail, since Git won't read the file without\na BOM.\n\nAdd a Makefile and #define knob, ICONV_NEEDS_BOM, that can be set if the\niconv implementation has this behavior. When set, Git will write a BOM\nmanually for UTF-16 and UTF-32 and then force the data to be written in\nUTF-16BE or UTF-32BE. We choose big-endian behavior here because the\ntests use the raw \"UTF-16\" encoding, which will be big-endian when the\nimplementation requires this knob to be set.\n\nUpdate the tests to detect this case and write test data with an added\nBOM if necessary. Always write the BOM in the tests in big-endian\nformat, since all iconv implementations that omit a BOM must use\nbig-endian serialization according to the Unicode standard.\n\nPreserve the existing behavior for systems which do not have this knob\nenabled, since they may use optimized implementations, including\ndefaulting to the native endianness, to gain improved performance, which\ncan be significant with large checkouts.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n Makefile                         |  6 ++++++\n t/t0028-working-tree-encoding.sh | 25 ++++++++++++++++++++++---\n utf8.c                           | 10 ++++++++++\n 3 files changed, 38 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 571160a2c4..b2a4765e5f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -259,6 +259,9 @@ all::\n # Define OLD_ICONV if your library has an old iconv(), where the second\n # (input buffer pointer) parameter is declared with type (const char **).\n #\n+# Define ICONV_NEEDS_BOM if your iconv implementation does not write a\n+# byte-order mark (BOM) when writing UTF-16 or UTF-32.\n+#\n # Define NO_DEFLATE_BOUND if your zlib does not have deflateBound.\n #\n # Define NO_R_TO_GCC_LINKER if your gcc does not like \"-R/path/lib\"\n@@ -1415,6 +1418,9 @@ ifndef NO_ICONV\n \t\tEXTLIBS += $(ICONV_LINK) -liconv\n \tendif\n endif\n+ifdef ICONV_NEEDS_BOM\n+\tBASIC_CFLAGS += -DICONV_NEEDS_BOM\n+endif\n ifdef NEEDS_LIBGEN\n \tEXTLIBS += -lgen\n endif\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex e58ecbfc44..bfc4a9d4dd 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -6,6 +6,25 @@ test_description='working-tree-encoding conversion via gitattributes'\n \n GIT_TRACE_WORKING_TREE_ENCODING=1 && export GIT_TRACE_WORKING_TREE_ENCODING\n \n+test_lazy_prereq NO_UTF16_BOM '\n+\ttest $(printf abc | iconv -f UTF-8 -t UTF-16 | wc -c) = 6\n+'\n+\n+test_lazy_prereq NO_UTF32_BOM '\n+\ttest $(printf abc | iconv -f UTF-8 -t UTF-32 | wc -c) = 12\n+'\n+\n+write_utf16 () {\n+\ttest_have_prereq NO_UTF16_BOM && printf '\\xfe\\xff'\n+\ticonv -f UTF-8 -t UTF-16\n+\n+}\n+\n+write_utf32 () {\n+\ttest_have_prereq NO_UTF32_BOM && printf '\\x00\\x00\\xfe\\xff'\n+\ticonv -f UTF-8 -t UTF-32\n+}\n+\n test_expect_success 'setup test files' '\n \tgit config core.eol lf &&\n \n@@ -13,8 +32,8 @@ test_expect_success 'setup test files' '\n \techo \"*.utf16 text working-tree-encoding=utf-16\" >.gitattributes &&\n \techo \"*.utf16lebom text working-tree-encoding=UTF-16LE-BOM\" >>.gitattributes &&\n \tprintf \"$text\" >test.utf8.raw &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-32 >test.utf32.raw &&\n+\tprintf \"$text\" | write_utf16 >test.utf16.raw &&\n+\tprintf \"$text\" | write_utf32 >test.utf32.raw &&\n \tprintf \"\\377\\376\"                         >test.utf16lebom.raw &&\n \tprintf \"$text\" | iconv -f UTF-8 -t UTF-32LE >>test.utf16lebom.raw &&\n \n@@ -223,7 +242,7 @@ test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' '\n \n \ttext=\"hallo there!\\nroundtrip test here!\" &&\n \tprintf \"$text\" | iconv -f UTF-8 -t SHIFT-JIS >roundtrip.shift &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >roundtrip.utf16 &&\n+\tprintf \"$text\" | write_utf16 >roundtrip.utf16 &&\n \techo \"*.shift text working-tree-encoding=SHIFT-JIS\" >>.gitattributes &&\n \n \t# SHIFT-JIS encoded files are round-trip checked by default...\ndiff --git a/utf8.c b/utf8.c\nindex 83824dc2f4..133199de0e 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -568,6 +568,16 @@ char *reencode_string_len(const char *in, size_t insz,\n \t\tbom_str = utf16_be_bom;\n \t\tbom_len = sizeof(utf16_be_bom);\n \t\tout_encoding = \"UTF-16BE\";\n+#ifdef ICONV_NEEDS_BOM\n+\t} else if (same_utf_encoding(\"UTF-16\", out_encoding)) {\n+\t\tbom_str = utf16_be_bom;\n+\t\tbom_len = sizeof(utf16_be_bom);\n+\t\tout_encoding = \"UTF-16BE\";\n+\t} else if (same_utf_encoding(\"UTF-32\", out_encoding)) {\n+\t\tbom_str = utf32_be_bom;\n+\t\tbom_len = sizeof(utf32_be_bom);\n+\t\tout_encoding = \"UTF-32BE\";\n+#endif\n \t}\n \n \tconv = iconv_open(out_encoding, in_encoding);\n"},{"id":"368952","messageId":"CAPig+cRyzZMOM19ztgR_wqvk68P_1eNNVBBj5pbY=MhQm08WAw@mail.gmail.com","threadId":"50430","inReplyTo":"20190209200802.277139-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH] utf8: handle systems that don't write BOM for UTF-16","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-02-10T01:45:16Z","receivedAt":"2019-02-10T01:45:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Feb 9, 2019 at 3:08 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> [...]\n> Add a Makefile and #define knob, ICONV_NEEDS_BOM, that can be set if the\n> iconv implementation has this behavior. When set, Git will write a BOM\n> manually for UTF-16 and UTF-32 and then force the data to be written in\n> UTF-16BE or UTF-32BE. We choose big-endian behavior here because the\n> tests use the raw \"UTF-16\" encoding, which will be big-endian when the\n> implementation requires this knob to be set.\n\nThe name ICONV_NEEDS_BOM makes it sound as if we must feed a BOM\n_into_ 'iconv', which is quite confusing since the actual intention is\nthat 'iconv' doesn't emit a BOM and we need to make up for the\ndeficiency. Using a name such as ICONV_OMITS_BOM or ICONV_NEGLECTS_BOM\nmakes it somewhat clearer that there is some deficiency with which we\nneed to deal.\n\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n> diff --git a/Makefile b/Makefile\n> @@ -259,6 +259,9 @@ all::\n> +# Define ICONV_NEEDS_BOM if your iconv implementation does not write a\n> +# byte-order mark (BOM) when writing UTF-16 or UTF-32.\n\nNot a big deal, but I wonder if it would be helpful to tack on \"...,\nin which case it outputs big-endian unconditionally.\" or something.\n\n> diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\n> @@ -6,6 +6,25 @@ test_description='working-tree-encoding conversion via gitattributes'\n> +test_lazy_prereq NO_UTF16_BOM '\n> +       test $(printf abc | iconv -f UTF-8 -t UTF-16 | wc -c) = 6\n> +'\n> +\n> +test_lazy_prereq NO_UTF32_BOM '\n> +       test $(printf abc | iconv -f UTF-8 -t UTF-32 | wc -c) = 12\n> +'\n> +\n> +write_utf16 () {\n> +       test_have_prereq NO_UTF16_BOM && printf '\\xfe\\xff'\n> +       iconv -f UTF-8 -t UTF-16\n> +\n> +}\n\nStray blank line before the closing brace.\n\n> +\n> +write_utf32 () {\n> +       test_have_prereq NO_UTF32_BOM && printf '\\x00\\x00\\xfe\\xff'\n> +       iconv -f UTF-8 -t UTF-32\n> +}\n\nIt's probably doesn't matter much with these two tiny functions, but I\nwas wondering if it would make sense to maintain the &&-chain, perhaps\nlike this:\n\n    if test test_have_prereq NO_UTF32_BOM\n    then\n        printf '\\x00\\x00\\xfe\\xff'\n    fi &&\n    iconv -f UTF-8 -t UTF-32\n"},{"id":"368958","messageId":"20190210080413.u56vr3fgoejjzjfm@tb-raspi4","threadId":"50430","inReplyTo":"20190209200802.277139-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH] utf8: handle systems that don't write BOM for UTF-16","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2019-02-10T08:04:13Z","receivedAt":"2019-02-10T08:10:23Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Sat, Feb 09, 2019 at 08:08:01PM +0000, brian m. carlson wrote:\n> When serializing UTF-16 (and UTF-32), there are three possible ways to\n> write the stream. One can write the data with a BOM in either big-endian\n> or little-endian format, or one can write the data without a BOM in\n> big-endian format.\n>\n> Most systems' iconv implementations choose to write it with a BOM in\n> some endianness, since this is the most foolproof, and it is resistant\n> to misinterpretation on Windows, where UTF-16 and the little-endian\n> serialization are very common. For compatibility with Windows and to\n> avoid accidental misuse there, Git always wants to write UTF-16 with a\n> BOM, and will refuse to read UTF-16 without it.\n>\n> However, musl's iconv implementation writes UTF-16 without a BOM,\n> relying on the user to interpret it as big-endian. This causes t0028 and\n> the related functionality to fail, since Git won't read the file without\n> a BOM.\n>\n> Add a Makefile and #define knob, ICONV_NEEDS_BOM, that can be set if the\n> iconv implementation has this behavior. When set, Git will write a BOM\n> manually for UTF-16 and UTF-32 and then force the data to be written in\n> UTF-16BE or UTF-32BE. We choose big-endian behavior here because the\n> tests use the raw \"UTF-16\" encoding, which will be big-endian when the\n> implementation requires this knob to be set.\n>\n> Update the tests to detect this case and write test data with an added\n> BOM if necessary. Always write the BOM in the tests in big-endian\n> format, since all iconv implementations that omit a BOM must use\n> big-endian serialization according to the Unicode standard.\n>\n> Preserve the existing behavior for systems which do not have this knob\n> enabled, since they may use optimized implementations, including\n> defaulting to the native endianness, to gain improved performance, which\n> can be significant with large checkouts.\n\nIs the based on measurements on a real system ?\n\nI think we agree that Git will write UTF-16 always as big endian with BOM,\nfollowing the tradition of iconv/libiconv.\nIf yes, we can reduce the lines of code/#idefs somewhat, have the knob always on,\nand reduce the maintenance burden a little bit, giving a simpler patch.\n\nWhat do you think ?\n\n\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex e58ecbfc44..ef19c98e67 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -13,7 +13,8 @@ test_expect_success 'setup test files' '\n \techo \"*.utf16 text working-tree-encoding=utf-16\" >.gitattributes &&\n \techo \"*.utf16lebom text working-tree-encoding=UTF-16LE-BOM\" >>.gitattributes &&\n \tprintf \"$text\" >test.utf8.raw &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&\n+\tprintf \"\\376\\377\"                         >test.utf16.raw &&\n+\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16BE >>test.utf16.raw &&\n \tprintf \"$text\" | iconv -f UTF-8 -t UTF-32 >test.utf32.raw &&\n \tprintf \"\\377\\376\"                         >test.utf16lebom.raw &&\n \tprintf \"$text\" | iconv -f UTF-8 -t UTF-32LE >>test.utf16lebom.raw &&\ndiff --git a/utf8.c b/utf8.c\nindex 83824dc2f4..d3731273be 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -564,7 +564,8 @@ char *reencode_string_len(const char *in, size_t insz,\n \t\tbom_str = utf16_le_bom;\n \t\tbom_len = sizeof(utf16_le_bom);\n \t\tout_encoding = \"UTF-16LE\";\n-\t} else if (same_utf_encoding(\"UTF-16BE-BOM\", out_encoding)) {\n+\t} else if (same_utf_encoding(\"UTF-16BE-BOM\", out_encoding) ||\n+\t\t   same_utf_encoding(\"UTF-16\", out_encoding)) {\n \t\tbom_str = utf16_be_bom;\n \t\tbom_len = sizeof(utf16_be_bom);\n \t\tout_encoding = \"UTF-16BE\";\n\n\n>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>  Makefile                         |  6 ++++++\n>  t/t0028-working-tree-encoding.sh | 25 ++++++++++++++++++++++---\n>  utf8.c                           | 10 ++++++++++\n>  3 files changed, 38 insertions(+), 3 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index 571160a2c4..b2a4765e5f 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -259,6 +259,9 @@ all::\n>  # Define OLD_ICONV if your library has an old iconv(), where the second\n>  # (input buffer pointer) parameter is declared with type (const char **).\n>  #\n> +# Define ICONV_NEEDS_BOM if your iconv implementation does not write a\n> +# byte-order mark (BOM) when writing UTF-16 or UTF-32.\n> +#\n>  # Define NO_DEFLATE_BOUND if your zlib does not have deflateBound.\n>  #\n>  # Define NO_R_TO_GCC_LINKER if your gcc does not like \"-R/path/lib\"\n> @@ -1415,6 +1418,9 @@ ifndef NO_ICONV\n>  \t\tEXTLIBS += $(ICONV_LINK) -liconv\n>  \tendif\n>  endif\n> +ifdef ICONV_NEEDS_BOM\n> +\tBASIC_CFLAGS += -DICONV_NEEDS_BOM\n> +endif\n>  ifdef NEEDS_LIBGEN\n>  \tEXTLIBS += -lgen\n>  endif\n> diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\n> index e58ecbfc44..bfc4a9d4dd 100755\n> --- a/t/t0028-working-tree-encoding.sh\n> +++ b/t/t0028-working-tree-encoding.sh\n> @@ -6,6 +6,25 @@ test_description='working-tree-encoding conversion via gitattributes'\n>\n>  GIT_TRACE_WORKING_TREE_ENCODING=1 && export GIT_TRACE_WORKING_TREE_ENCODING\n>\n> +test_lazy_prereq NO_UTF16_BOM '\n> +\ttest $(printf abc | iconv -f UTF-8 -t UTF-16 | wc -c) = 6\n> +'\n> +\n> +test_lazy_prereq NO_UTF32_BOM '\n> +\ttest $(printf abc | iconv -f UTF-8 -t UTF-32 | wc -c) = 12\n> +'\n> +\n> +write_utf16 () {\n> +\ttest_have_prereq NO_UTF16_BOM && printf '\\xfe\\xff'\n> +\ticonv -f UTF-8 -t UTF-16\n> +\n> +}\n> +\n> +write_utf32 () {\n> +\ttest_have_prereq NO_UTF32_BOM && printf '\\x00\\x00\\xfe\\xff'\n> +\ticonv -f UTF-8 -t UTF-32\n> +}\n> +\n>  test_expect_success 'setup test files' '\n>  \tgit config core.eol lf &&\n>\n> @@ -13,8 +32,8 @@ test_expect_success 'setup test files' '\n>  \techo \"*.utf16 text working-tree-encoding=utf-16\" >.gitattributes &&\n>  \techo \"*.utf16lebom text working-tree-encoding=UTF-16LE-BOM\" >>.gitattributes &&\n>  \tprintf \"$text\" >test.utf8.raw &&\n> -\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&\n> -\tprintf \"$text\" | iconv -f UTF-8 -t UTF-32 >test.utf32.raw &&\n> +\tprintf \"$text\" | write_utf16 >test.utf16.raw &&\n> +\tprintf \"$text\" | write_utf32 >test.utf32.raw &&\n>  \tprintf \"\\377\\376\"                         >test.utf16lebom.raw &&\n>  \tprintf \"$text\" | iconv -f UTF-8 -t UTF-32LE >>test.utf16lebom.raw &&\n>\n> @@ -223,7 +242,7 @@ test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' '\n>\n>  \ttext=\"hallo there!\\nroundtrip test here!\" &&\n>  \tprintf \"$text\" | iconv -f UTF-8 -t SHIFT-JIS >roundtrip.shift &&\n> -\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >roundtrip.utf16 &&\n> +\tprintf \"$text\" | write_utf16 >roundtrip.utf16 &&\n>  \techo \"*.shift text working-tree-encoding=SHIFT-JIS\" >>.gitattributes &&\n>\n>  \t# SHIFT-JIS encoded files are round-trip checked by default...\n> diff --git a/utf8.c b/utf8.c\n> index 83824dc2f4..133199de0e 100644\n> --- a/utf8.c\n> +++ b/utf8.c\n> @@ -568,6 +568,16 @@ char *reencode_string_len(const char *in, size_t insz,\n>  \t\tbom_str = utf16_be_bom;\n>  \t\tbom_len = sizeof(utf16_be_bom);\n>  \t\tout_encoding = \"UTF-16BE\";\n> +#ifdef ICONV_NEEDS_BOM\n> +\t} else if (same_utf_encoding(\"UTF-16\", out_encoding)) {\n> +\t\tbom_str = utf16_be_bom;\n> +\t\tbom_len = sizeof(utf16_be_bom);\n> +\t\tout_encoding = \"UTF-16BE\";\n> +\t} else if (same_utf_encoding(\"UTF-32\", out_encoding)) {\n> +\t\tbom_str = utf32_be_bom;\n> +\t\tbom_len = sizeof(utf32_be_bom);\n> +\t\tout_encoding = \"UTF-32BE\";\n> +#endif\n>  \t}\n>\n>  \tconv = iconv_open(out_encoding, in_encoding);\n"},{"id":"368979","messageId":"20190210181430.GA28510@genre.crustytoothpaste.net","threadId":"50430","inReplyTo":"CAPig+cRyzZMOM19ztgR_wqvk68P_1eNNVBBj5pbY=MhQm08WAw@mail.gmail.com","subject":"Re: [PATCH] utf8: handle systems that don't write BOM for UTF-16","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-10T18:14:30Z","receivedAt":"2019-02-10T18:14:40Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sat, Feb 09, 2019 at 08:45:16PM -0500, Eric Sunshine wrote:\n> On Sat, Feb 9, 2019 at 3:08 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> > [...]\n> > Add a Makefile and #define knob, ICONV_NEEDS_BOM, that can be set if the\n> > iconv implementation has this behavior. When set, Git will write a BOM\n> > manually for UTF-16 and UTF-32 and then force the data to be written in\n> > UTF-16BE or UTF-32BE. We choose big-endian behavior here because the\n> > tests use the raw \"UTF-16\" encoding, which will be big-endian when the\n> > implementation requires this knob to be set.\n> \n> The name ICONV_NEEDS_BOM makes it sound as if we must feed a BOM\n> _into_ 'iconv', which is quite confusing since the actual intention is\n> that 'iconv' doesn't emit a BOM and we need to make up for the\n> deficiency. Using a name such as ICONV_OMITS_BOM or ICONV_NEGLECTS_BOM\n> makes it somewhat clearer that there is some deficiency with which we\n> need to deal.\n\nThat does sound like a better name.\n\n> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> > ---\n> > diff --git a/Makefile b/Makefile\n> > @@ -259,6 +259,9 @@ all::\n> > +# Define ICONV_NEEDS_BOM if your iconv implementation does not write a\n> > +# byte-order mark (BOM) when writing UTF-16 or UTF-32.\n> \n> Not a big deal, but I wonder if it would be helpful to tack on \"...,\n> in which case it outputs big-endian unconditionally.\" or something.\n\nSure, I can add that.\n\n> Stray blank line before the closing brace.\n\nWill fix.\n\n> It's probably doesn't matter much with these two tiny functions, but I\n> was wondering if it would make sense to maintain the &&-chain, perhaps\n> like this:\n> \n>     if test test_have_prereq NO_UTF32_BOM\n>     then\n>         printf '\\x00\\x00\\xfe\\xff'\n>     fi &&\n>     iconv -f UTF-8 -t UTF-32\n\nYeah, that sounds like a good idea.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"368980","messageId":"20190210185523.GB28510@genre.crustytoothpaste.net","threadId":"50430","inReplyTo":"20190210080413.u56vr3fgoejjzjfm@tb-raspi4","subject":"Re: [PATCH] utf8: handle systems that don't write BOM for UTF-16","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-10T18:55:24Z","receivedAt":"2019-02-10T18:55:34Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, Feb 10, 2019 at 08:04:13AM +0000, Torsten Bögershausen wrote:\n> On Sat, Feb 09, 2019 at 08:08:01PM +0000, brian m. carlson wrote:\n> > Preserve the existing behavior for systems which do not have this knob\n> > enabled, since they may use optimized implementations, including\n> > defaulting to the native endianness, to gain improved performance, which\n> > can be significant with large checkouts.\n> \n> Is the based on measurements on a real system ?\n\nNo, I haven't done any performance measurements. However, swapping bytes\nis a (IIRC 1-cycle) instruction on x86, which would be executed for each\niteration of the loop. My intuition tells me that will be a significant\nexpense when there are a lot of files, but I can omit that phrase since\nI haven't measured.\n\n> I think we agree that Git will write UTF-16 always as big endian with BOM,\n> following the tradition of iconv/libiconv.\n> If yes, we can reduce the lines of code/#idefs somewhat, have the knob always on,\n> and reduce the maintenance burden a little bit, giving a simpler patch.\n\nNo, I don't think it will. libiconv will always write big-endian, but\nglibc has a separate iconv implementation which writes the native\nendianness. (I believe FreeBSD's does the same thing as glibc's.) I\nthink it's useful for us to know that we can handle UTF-16 using the\nsystem behavior where possible, since that's what the system is going to\nproduce.\n\n> What do you think ?\n\nWhile I like the simplicity of the approach, as I mentioned above, and I\ndid consider this originally, I'd rather test the behavior of the system\nwe're operating on, provided it's suitable for our needs.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"368990","messageId":"20190211002307.686048-1-sandals@crustytoothpaste.net","threadId":"50430","inReplyTo":"20190209200802.277139-1-sandals@crustytoothpaste.net","subject":"[PATCH v2] utf8: handle systems that don't write BOM for UTF-16","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-11T00:23:07Z","receivedAt":"2019-02-11T00:23:19Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When serializing UTF-16 (and UTF-32), there are three possible ways to\nwrite the stream. One can write the data with a BOM in either big-endian\nor little-endian format, or one can write the data without a BOM in\nbig-endian format.\n\nMost systems' iconv implementations choose to write it with a BOM in\nsome endianness, since this is the most foolproof, and it is resistant\nto misinterpretation on Windows, where UTF-16 and the little-endian\nserialization are very common. For compatibility with Windows and to\navoid accidental misuse there, Git always wants to write UTF-16 with a\nBOM, and will refuse to read UTF-16 without it.\n\nHowever, musl's iconv implementation writes UTF-16 without a BOM,\nrelying on the user to interpret it as big-endian. This causes t0028 and\nthe related functionality to fail, since Git won't read the file without\na BOM.\n\nAdd a Makefile and #define knob, ICONV_OMITS_BOM, that can be set if the\niconv implementation has this behavior. When set, Git will write a BOM\nmanually for UTF-16 and UTF-32 and then force the data to be written in\nUTF-16BE or UTF-32BE. We choose big-endian behavior here because the\ntests use the raw \"UTF-16\" encoding, which will be big-endian when the\nimplementation requires this knob to be set.\n\nUpdate the tests to detect this case and write test data with an added\nBOM if necessary. Always write the BOM in the tests in big-endian\nformat, since all iconv implementations that omit a BOM must use\nbig-endian serialization according to the Unicode standard.\n\nPreserve the existing behavior for systems which do not have this knob\nenabled, since they may use optimized implementations, including\ndefaulting to the native endianness, which may improve performance.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n Makefile                         |  6 ++++++\n t/t0028-working-tree-encoding.sh | 25 ++++++++++++++++++++++---\n utf8.c                           | 10 ++++++++++\n 3 files changed, 38 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 571160a2c4..b2a4765e5f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -259,6 +259,9 @@ all::\n # Define OLD_ICONV if your library has an old iconv(), where the second\n # (input buffer pointer) parameter is declared with type (const char **).\n #\n+# Define ICONV_NEEDS_BOM if your iconv implementation does not write a\n+# byte-order mark (BOM) when writing UTF-16 or UTF-32.\n+#\n # Define NO_DEFLATE_BOUND if your zlib does not have deflateBound.\n #\n # Define NO_R_TO_GCC_LINKER if your gcc does not like \"-R/path/lib\"\n@@ -1415,6 +1418,9 @@ ifndef NO_ICONV\n \t\tEXTLIBS += $(ICONV_LINK) -liconv\n \tendif\n endif\n+ifdef ICONV_NEEDS_BOM\n+\tBASIC_CFLAGS += -DICONV_NEEDS_BOM\n+endif\n ifdef NEEDS_LIBGEN\n \tEXTLIBS += -lgen\n endif\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex e58ecbfc44..bfc4a9d4dd 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -6,6 +6,25 @@ test_description='working-tree-encoding conversion via gitattributes'\n \n GIT_TRACE_WORKING_TREE_ENCODING=1 && export GIT_TRACE_WORKING_TREE_ENCODING\n \n+test_lazy_prereq NO_UTF16_BOM '\n+\ttest $(printf abc | iconv -f UTF-8 -t UTF-16 | wc -c) = 6\n+'\n+\n+test_lazy_prereq NO_UTF32_BOM '\n+\ttest $(printf abc | iconv -f UTF-8 -t UTF-32 | wc -c) = 12\n+'\n+\n+write_utf16 () {\n+\ttest_have_prereq NO_UTF16_BOM && printf '\\xfe\\xff'\n+\ticonv -f UTF-8 -t UTF-16\n+\n+}\n+\n+write_utf32 () {\n+\ttest_have_prereq NO_UTF32_BOM && printf '\\x00\\x00\\xfe\\xff'\n+\ticonv -f UTF-8 -t UTF-32\n+}\n+\n test_expect_success 'setup test files' '\n \tgit config core.eol lf &&\n \n@@ -13,8 +32,8 @@ test_expect_success 'setup test files' '\n \techo \"*.utf16 text working-tree-encoding=utf-16\" >.gitattributes &&\n \techo \"*.utf16lebom text working-tree-encoding=UTF-16LE-BOM\" >>.gitattributes &&\n \tprintf \"$text\" >test.utf8.raw &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-32 >test.utf32.raw &&\n+\tprintf \"$text\" | write_utf16 >test.utf16.raw &&\n+\tprintf \"$text\" | write_utf32 >test.utf32.raw &&\n \tprintf \"\\377\\376\"                         >test.utf16lebom.raw &&\n \tprintf \"$text\" | iconv -f UTF-8 -t UTF-32LE >>test.utf16lebom.raw &&\n \n@@ -223,7 +242,7 @@ test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' '\n \n \ttext=\"hallo there!\\nroundtrip test here!\" &&\n \tprintf \"$text\" | iconv -f UTF-8 -t SHIFT-JIS >roundtrip.shift &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >roundtrip.utf16 &&\n+\tprintf \"$text\" | write_utf16 >roundtrip.utf16 &&\n \techo \"*.shift text working-tree-encoding=SHIFT-JIS\" >>.gitattributes &&\n \n \t# SHIFT-JIS encoded files are round-trip checked by default...\ndiff --git a/utf8.c b/utf8.c\nindex 83824dc2f4..133199de0e 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -568,6 +568,16 @@ char *reencode_string_len(const char *in, size_t insz,\n \t\tbom_str = utf16_be_bom;\n \t\tbom_len = sizeof(utf16_be_bom);\n \t\tout_encoding = \"UTF-16BE\";\n+#ifdef ICONV_NEEDS_BOM\n+\t} else if (same_utf_encoding(\"UTF-16\", out_encoding)) {\n+\t\tbom_str = utf16_be_bom;\n+\t\tbom_len = sizeof(utf16_be_bom);\n+\t\tout_encoding = \"UTF-16BE\";\n+\t} else if (same_utf_encoding(\"UTF-32\", out_encoding)) {\n+\t\tbom_str = utf32_be_bom;\n+\t\tbom_len = sizeof(utf32_be_bom);\n+\t\tout_encoding = \"UTF-32BE\";\n+#endif\n \t}\n \n \tconv = iconv_open(out_encoding, in_encoding);\n"},{"id":"368992","messageId":"CAPig+cRTRCeGZ115gJGGNPtQ7WyFWg4Y45WOPec-8CmnG6ZRMQ@mail.gmail.com","threadId":"50430","inReplyTo":"20190211002307.686048-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2] utf8: handle systems that don't write BOM for UTF-16","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-02-11T01:16:26Z","receivedAt":"2019-02-11T01:16:39Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Feb 10, 2019 at 7:23 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> When serializing UTF-16 (and UTF-32), there are three possible ways to\n> write the stream. One can write the data with a BOM in either big-endian\n> or little-endian format, or one can write the data without a BOM in\n> big-endian format.\n> [...]\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n\nPremature git-send-email invocation? The commit message of v2 seems to\nbe a bit different from v1, but the patch itself is identical.\n"},{"id":"368993","messageId":"20190211012047.GA684736@genre.crustytoothpaste.net","threadId":"50430","inReplyTo":"CAPig+cRTRCeGZ115gJGGNPtQ7WyFWg4Y45WOPec-8CmnG6ZRMQ@mail.gmail.com","subject":"Re: [PATCH v2] utf8: handle systems that don't write BOM for UTF-16","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-11T01:20:47Z","receivedAt":"2019-02-11T01:20:57Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, Feb 10, 2019 at 08:16:26PM -0500, Eric Sunshine wrote:\n> On Sun, Feb 10, 2019 at 7:23 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> > When serializing UTF-16 (and UTF-32), there are three possible ways to\n> > write the stream. One can write the data with a BOM in either big-endian\n> > or little-endian format, or one can write the data without a BOM in\n> > big-endian format.\n> > [...]\n> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> \n> Premature git-send-email invocation? The commit message of v2 seems to\n> be a bit different from v1, but the patch itself is identical.\n\nOof, I forgot to run \"git add -u\" before running \"git commit --amend\".\nThanks for catching this; I'll send out a v3 in a second.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"368994","messageId":"20190211012639.579489-1-sandals@crustytoothpaste.net","threadId":"50430","inReplyTo":"20190209200802.277139-1-sandals@crustytoothpaste.net","subject":"[PATCH v3] utf8: handle systems that don't write BOM for UTF-16","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-11T01:26:39Z","receivedAt":"2019-02-11T01:26:52Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When serializing UTF-16 (and UTF-32), there are three possible ways to\nwrite the stream. One can write the data with a BOM in either big-endian\nor little-endian format, or one can write the data without a BOM in\nbig-endian format.\n\nMost systems' iconv implementations choose to write it with a BOM in\nsome endianness, since this is the most foolproof, and it is resistant\nto misinterpretation on Windows, where UTF-16 and the little-endian\nserialization are very common. For compatibility with Windows and to\navoid accidental misuse there, Git always wants to write UTF-16 with a\nBOM, and will refuse to read UTF-16 without it.\n\nHowever, musl's iconv implementation writes UTF-16 without a BOM,\nrelying on the user to interpret it as big-endian. This causes t0028 and\nthe related functionality to fail, since Git won't read the file without\na BOM.\n\nAdd a Makefile and #define knob, ICONV_OMITS_BOM, that can be set if the\niconv implementation has this behavior. When set, Git will write a BOM\nmanually for UTF-16 and UTF-32 and then force the data to be written in\nUTF-16BE or UTF-32BE. We choose big-endian behavior here because the\ntests use the raw \"UTF-16\" encoding, which will be big-endian when the\nimplementation requires this knob to be set.\n\nUpdate the tests to detect this case and write test data with an added\nBOM if necessary. Always write the BOM in the tests in big-endian\nformat, since all iconv implementations that omit a BOM must use\nbig-endian serialization according to the Unicode standard.\n\nPreserve the existing behavior for systems which do not have this knob\nenabled, since they may use optimized implementations, including\ndefaulting to the native endianness, which may improve performance.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n Makefile                         |  7 +++++++\n t/t0028-working-tree-encoding.sh | 30 +++++++++++++++++++++++++++---\n utf8.c                           | 10 ++++++++++\n 3 files changed, 44 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 571160a2c4..c6172450af 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -259,6 +259,10 @@ all::\n # Define OLD_ICONV if your library has an old iconv(), where the second\n # (input buffer pointer) parameter is declared with type (const char **).\n #\n+# Define ICONV_OMITS_BOM if your iconv implementation does not write a\n+# byte-order mark (BOM) when writing UTF-16 or UTF-32 and always writes in\n+# big-endian format.\n+#\n # Define NO_DEFLATE_BOUND if your zlib does not have deflateBound.\n #\n # Define NO_R_TO_GCC_LINKER if your gcc does not like \"-R/path/lib\"\n@@ -1415,6 +1419,9 @@ ifndef NO_ICONV\n \t\tEXTLIBS += $(ICONV_LINK) -liconv\n \tendif\n endif\n+ifdef ICONV_OMITS_BOM\n+\tBASIC_CFLAGS += -DICONV_OMITS_BOM\n+endif\n ifdef NEEDS_LIBGEN\n \tEXTLIBS += -lgen\n endif\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex e58ecbfc44..8936ba6757 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -6,6 +6,30 @@ test_description='working-tree-encoding conversion via gitattributes'\n \n GIT_TRACE_WORKING_TREE_ENCODING=1 && export GIT_TRACE_WORKING_TREE_ENCODING\n \n+test_lazy_prereq NO_UTF16_BOM '\n+\ttest $(printf abc | iconv -f UTF-8 -t UTF-16 | wc -c) = 6\n+'\n+\n+test_lazy_prereq NO_UTF32_BOM '\n+\ttest $(printf abc | iconv -f UTF-8 -t UTF-32 | wc -c) = 12\n+'\n+\n+write_utf16 () {\n+\tif test_have_prereq NO_UTF16_BOM\n+\tthen\n+\t\tprintf '\\xfe\\xff'\n+\tfi &&\n+\ticonv -f UTF-8 -t UTF-16\n+}\n+\n+write_utf32 () {\n+\tif test_have_prereq NO_UTF32_BOM\n+\tthen\n+\t\tprintf '\\x00\\x00\\xfe\\xff'\n+\tfi &&\n+\ticonv -f UTF-8 -t UTF-32\n+}\n+\n test_expect_success 'setup test files' '\n \tgit config core.eol lf &&\n \n@@ -13,8 +37,8 @@ test_expect_success 'setup test files' '\n \techo \"*.utf16 text working-tree-encoding=utf-16\" >.gitattributes &&\n \techo \"*.utf16lebom text working-tree-encoding=UTF-16LE-BOM\" >>.gitattributes &&\n \tprintf \"$text\" >test.utf8.raw &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-32 >test.utf32.raw &&\n+\tprintf \"$text\" | write_utf16 >test.utf16.raw &&\n+\tprintf \"$text\" | write_utf32 >test.utf32.raw &&\n \tprintf \"\\377\\376\"                         >test.utf16lebom.raw &&\n \tprintf \"$text\" | iconv -f UTF-8 -t UTF-32LE >>test.utf16lebom.raw &&\n \n@@ -223,7 +247,7 @@ test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' '\n \n \ttext=\"hallo there!\\nroundtrip test here!\" &&\n \tprintf \"$text\" | iconv -f UTF-8 -t SHIFT-JIS >roundtrip.shift &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >roundtrip.utf16 &&\n+\tprintf \"$text\" | write_utf16 >roundtrip.utf16 &&\n \techo \"*.shift text working-tree-encoding=SHIFT-JIS\" >>.gitattributes &&\n \n \t# SHIFT-JIS encoded files are round-trip checked by default...\ndiff --git a/utf8.c b/utf8.c\nindex 83824dc2f4..5d9a917bc8 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -568,6 +568,16 @@ char *reencode_string_len(const char *in, size_t insz,\n \t\tbom_str = utf16_be_bom;\n \t\tbom_len = sizeof(utf16_be_bom);\n \t\tout_encoding = \"UTF-16BE\";\n+#ifdef ICONV_OMITS_BOM\n+\t} else if (same_utf_encoding(\"UTF-16\", out_encoding)) {\n+\t\tbom_str = utf16_be_bom;\n+\t\tbom_len = sizeof(utf16_be_bom);\n+\t\tout_encoding = \"UTF-16BE\";\n+\t} else if (same_utf_encoding(\"UTF-32\", out_encoding)) {\n+\t\tbom_str = utf32_be_bom;\n+\t\tbom_len = sizeof(utf32_be_bom);\n+\t\tout_encoding = \"UTF-32BE\";\n+#endif\n \t}\n \n \tconv = iconv_open(out_encoding, in_encoding);\n"},{"id":"369017","messageId":"xmqqk1i6i6xd.fsf@gitster-ct.c.googlers.com","threadId":"50430","inReplyTo":"20190210185523.GB28510@genre.crustytoothpaste.net","subject":"Re: [PATCH] utf8: handle systems that don't write BOM for UTF-16","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-11T17:14:22Z","receivedAt":"2019-02-11T17:14:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> On Sun, Feb 10, 2019 at 08:04:13AM +0000, Torsten Bögershausen wrote:\n>\n>> I think we agree that Git will write UTF-16 always as big endian with BOM,\n>> following the tradition of iconv/libiconv.\n>> If yes, we can reduce the lines of code/#idefs somewhat, have the knob always on,\n>> and reduce the maintenance burden a little bit, giving a simpler patch.\n>\n> No, I don't think it will. libiconv will always write big-endian, but\n> glibc has a separate iconv implementation which writes the native\n> endianness. (I believe FreeBSD's does the same thing as glibc's.) I\n> think it's useful for us to know that we can handle UTF-16 using the\n> system behavior where possible, since that's what the system is going to\n> produce.\n>\n>> What do you think ?\n>\n> While I like the simplicity of the approach, as I mentioned above, and I\n> did consider this originally, I'd rather test the behavior of the system\n> we're operating on, provided it's suitable for our needs.\n\nI see both sides of the argument, and each has its merit.\n\nLet's go with the \"follow the platform\" and make sure the decision\nis documented somewhere in the resulting code.\n\nThanks, all.\n"},{"id":"369041","messageId":"20190211214306.GB14229@alpha","threadId":"50430","inReplyTo":"20190211012639.579489-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v3] utf8: handle systems that don't write BOM for UTF-16","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2019-02-11T21:43:06Z","receivedAt":"2019-02-11T21:43:14Z","isPatch":true,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"On Mon, Feb 11, 2019 at 01:26:39AM +0000, brian m. carlson wrote:\n> When serializing UTF-16 (and UTF-32), there are three possible ways to\n> write the stream. One can write the data with a BOM in either big-endian\n> or little-endian format, or one can write the data without a BOM in\n> big-endian format.\n> \n> Most systems' iconv implementations choose to write it with a BOM in\n> some endianness, since this is the most foolproof, and it is resistant\n> to misinterpretation on Windows, where UTF-16 and the little-endian\n> serialization are very common. For compatibility with Windows and to\n> avoid accidental misuse there, Git always wants to write UTF-16 with a\n> BOM, and will refuse to read UTF-16 without it.\n> \n> However, musl's iconv implementation writes UTF-16 without a BOM,\n> relying on the user to interpret it as big-endian. This causes t0028 and\n> the related functionality to fail, since Git won't read the file without\n> a BOM.\n> \n> Add a Makefile and #define knob, ICONV_OMITS_BOM, that can be set if the\n> iconv implementation has this behavior. When set, Git will write a BOM\n> manually for UTF-16 and UTF-32 and then force the data to be written in\n> UTF-16BE or UTF-32BE. We choose big-endian behavior here because the\n> tests use the raw \"UTF-16\" encoding, which will be big-endian when the\n> implementation requires this knob to be set.\n> \n> Update the tests to detect this case and write test data with an added\n> BOM if necessary. Always write the BOM in the tests in big-endian\n> format, since all iconv implementations that omit a BOM must use\n> big-endian serialization according to the Unicode standard.\n> \n> Preserve the existing behavior for systems which do not have this knob\n> enabled, since they may use optimized implementations, including\n> defaulting to the native endianness, which may improve performance.\n> \n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>  Makefile                         |  7 +++++++\n>  t/t0028-working-tree-encoding.sh | 30 +++++++++++++++++++++++++++---\n>  utf8.c                           | 10 ++++++++++\n>  3 files changed, 44 insertions(+), 3 deletions(-)\n> \n> diff --git a/Makefile b/Makefile\n> index 571160a2c4..c6172450af 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -259,6 +259,10 @@ all::\n>  # Define OLD_ICONV if your library has an old iconv(), where the second\n>  # (input buffer pointer) parameter is declared with type (const char **).\n>  #\n> +# Define ICONV_OMITS_BOM if your iconv implementation does not write a\n> +# byte-order mark (BOM) when writing UTF-16 or UTF-32 and always writes in\n> +# big-endian format.\n> +#\n>  # Define NO_DEFLATE_BOUND if your zlib does not have deflateBound.\n>  #\n>  # Define NO_R_TO_GCC_LINKER if your gcc does not like \"-R/path/lib\"\n> @@ -1415,6 +1419,9 @@ ifndef NO_ICONV\n>  \t\tEXTLIBS += $(ICONV_LINK) -liconv\n>  \tendif\n>  endif\n> +ifdef ICONV_OMITS_BOM\n> +\tBASIC_CFLAGS += -DICONV_OMITS_BOM\n> +endif\n>  ifdef NEEDS_LIBGEN\n>  \tEXTLIBS += -lgen\n>  endif\n> diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\n> index e58ecbfc44..8936ba6757 100755\n> --- a/t/t0028-working-tree-encoding.sh\n> +++ b/t/t0028-working-tree-encoding.sh\n> @@ -6,6 +6,30 @@ test_description='working-tree-encoding conversion via gitattributes'\n>  \n>  GIT_TRACE_WORKING_TREE_ENCODING=1 && export GIT_TRACE_WORKING_TREE_ENCODING\n>  \n> +test_lazy_prereq NO_UTF16_BOM '\n> +\ttest $(printf abc | iconv -f UTF-8 -t UTF-16 | wc -c) = 6\n> +'\n> +\n> +test_lazy_prereq NO_UTF32_BOM '\n> +\ttest $(printf abc | iconv -f UTF-8 -t UTF-32 | wc -c) = 12\n> +'\n> +\n> +write_utf16 () {\n> +\tif test_have_prereq NO_UTF16_BOM\n> +\tthen\n> +\t\tprintf '\\xfe\\xff'\n> +\tfi &&\n> +\ticonv -f UTF-8 -t UTF-16\n> +}\n> +\n> +write_utf32 () {\n> +\tif test_have_prereq NO_UTF32_BOM\n> +\tthen\n> +\t\tprintf '\\x00\\x00\\xfe\\xff'\n> +\tfi &&\n> +\ticonv -f UTF-8 -t UTF-32\n> +}\n> +\n>  test_expect_success 'setup test files' '\n>  \tgit config core.eol lf &&\n>  \n> @@ -13,8 +37,8 @@ test_expect_success 'setup test files' '\n>  \techo \"*.utf16 text working-tree-encoding=utf-16\" >.gitattributes &&\n>  \techo \"*.utf16lebom text working-tree-encoding=UTF-16LE-BOM\" >>.gitattributes &&\n>  \tprintf \"$text\" >test.utf8.raw &&\n> -\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&\n> -\tprintf \"$text\" | iconv -f UTF-8 -t UTF-32 >test.utf32.raw &&\n> +\tprintf \"$text\" | write_utf16 >test.utf16.raw &&\n> +\tprintf \"$text\" | write_utf32 >test.utf32.raw &&\n>  \tprintf \"\\377\\376\"                         >test.utf16lebom.raw &&\n>  \tprintf \"$text\" | iconv -f UTF-8 -t UTF-32LE >>test.utf16lebom.raw &&\n>  \n> @@ -223,7 +247,7 @@ test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' '\n>  \n>  \ttext=\"hallo there!\\nroundtrip test here!\" &&\n>  \tprintf \"$text\" | iconv -f UTF-8 -t SHIFT-JIS >roundtrip.shift &&\n> -\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >roundtrip.utf16 &&\n> +\tprintf \"$text\" | write_utf16 >roundtrip.utf16 &&\n>  \techo \"*.shift text working-tree-encoding=SHIFT-JIS\" >>.gitattributes &&\n>  \n>  \t# SHIFT-JIS encoded files are round-trip checked by default...\n> diff --git a/utf8.c b/utf8.c\n> index 83824dc2f4..5d9a917bc8 100644\n> --- a/utf8.c\n> +++ b/utf8.c\n> @@ -568,6 +568,16 @@ char *reencode_string_len(const char *in, size_t insz,\n>  \t\tbom_str = utf16_be_bom;\n>  \t\tbom_len = sizeof(utf16_be_bom);\n>  \t\tout_encoding = \"UTF-16BE\";\n> +#ifdef ICONV_OMITS_BOM\n> +\t} else if (same_utf_encoding(\"UTF-16\", out_encoding)) {\n> +\t\tbom_str = utf16_be_bom;\n> +\t\tbom_len = sizeof(utf16_be_bom);\n> +\t\tout_encoding = \"UTF-16BE\";\n> +\t} else if (same_utf_encoding(\"UTF-32\", out_encoding)) {\n> +\t\tbom_str = utf32_be_bom;\n> +\t\tbom_len = sizeof(utf32_be_bom);\n> +\t\tout_encoding = \"UTF-32BE\";\n> +#endif\n>  \t}\n>  \n>  \tconv = iconv_open(out_encoding, in_encoding);\n\nWith some additional fixes, this indeed does solve the issue for Alpine\nLinux, thanks.\n\nI had to fix the following as well:\n\niff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex 8936ba6757..8491f216aa 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -148,8 +148,8 @@ do\n        test_when_finished \"rm -f crlf.utf${i}.raw lf.utf${i}.raw\" &&\n        test_when_finished \"git reset --hard HEAD^\" &&\n\n-       cat lf.utf8.raw | iconv -f UTF-8 -t UTF-${i} >lf.utf${i}.raw &&\n-       cat crlf.utf8.raw | iconv -f UTF-8 -t UTF-${i} >crlf.utf${i}.raw &&\n+       cat lf.utf8.raw | eval \"write_utf${i}\" >lf.utf${i}.raw &&\n+       cat crlf.utf8.raw | eval \"write_utf${i}\" >crlf.utf${i}.raw &&\n        cp crlf.utf${i}.raw eol.utf${i} &&\n\n        cat >expectIndexLF <<-EOF &&\n"},{"id":"369057","messageId":"20190211235835.GB684736@genre.crustytoothpaste.net","threadId":"50430","inReplyTo":"20190211214306.GB14229@alpha","subject":"Re: [PATCH v3] utf8: handle systems that don't write BOM for UTF-16","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-11T23:58:36Z","receivedAt":"2019-02-11T23:58:47Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Feb 11, 2019 at 10:43:06PM +0100, Kevin Daudt wrote:\n> With some additional fixes, this indeed does solve the issue for Alpine\n> Linux, thanks.\n> \n> I had to fix the following as well:\n> \n> iff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\n> index 8936ba6757..8491f216aa 100755\n> --- a/t/t0028-working-tree-encoding.sh\n> +++ b/t/t0028-working-tree-encoding.sh\n> @@ -148,8 +148,8 @@ do\n>         test_when_finished \"rm -f crlf.utf${i}.raw lf.utf${i}.raw\" &&\n>         test_when_finished \"git reset --hard HEAD^\" &&\n> \n> -       cat lf.utf8.raw | iconv -f UTF-8 -t UTF-${i} >lf.utf${i}.raw &&\n> -       cat crlf.utf8.raw | iconv -f UTF-8 -t UTF-${i} >crlf.utf${i}.raw &&\n> +       cat lf.utf8.raw | eval \"write_utf${i}\" >lf.utf${i}.raw &&\n> +       cat crlf.utf8.raw | eval \"write_utf${i}\" >crlf.utf${i}.raw &&\n>         cp crlf.utf${i}.raw eol.utf${i} &&\n> \n>         cat >expectIndexLF <<-EOF &&\n\nI'll squash in this fix, thanks.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"369069","messageId":"xmqqtvh9g857.fsf@gitster-ct.c.googlers.com","threadId":"50430","inReplyTo":"20190211235835.GB684736@genre.crustytoothpaste.net","subject":"Re: [PATCH v3] utf8: handle systems that don't write BOM for UTF-16","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-12T00:31:00Z","receivedAt":"2019-02-12T00:31:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n>> -       cat lf.utf8.raw | iconv -f UTF-8 -t UTF-${i} >lf.utf${i}.raw &&\n>> -       cat crlf.utf8.raw | iconv -f UTF-8 -t UTF-${i} >crlf.utf${i}.raw &&\n>> +       cat lf.utf8.raw | eval \"write_utf${i}\" >lf.utf${i}.raw &&\n>> +       cat crlf.utf8.raw | eval \"write_utf${i}\" >crlf.utf${i}.raw &&\n>>         cp crlf.utf${i}.raw eol.utf${i} &&\n>> \n>>         cat >expectIndexLF <<-EOF &&\n>\n> I'll squash in this fix, thanks.\n\nThanks, all.  In the meantime, what I've pushed out has this\napplied immediately on top.  Unless there is anything else, I could\nsquash it in in my next pushout I plan to do tonight, before getting\nready to tag -rc1 tomorrow.\n\n"},{"id":"369077","messageId":"20190212005206.1002049-1-sandals@crustytoothpaste.net","threadId":"50430","inReplyTo":"20190209200802.277139-1-sandals@crustytoothpaste.net","subject":"[PATCH v4] utf8: handle systems that don't write BOM for UTF-16","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-12T00:52:06Z","receivedAt":"2019-02-12T00:52:18Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When serializing UTF-16 (and UTF-32), there are three possible ways to\nwrite the stream. One can write the data with a BOM in either big-endian\nor little-endian format, or one can write the data without a BOM in\nbig-endian format.\n\nMost systems' iconv implementations choose to write it with a BOM in\nsome endianness, since this is the most foolproof, and it is resistant\nto misinterpretation on Windows, where UTF-16 and the little-endian\nserialization are very common. For compatibility with Windows and to\navoid accidental misuse there, Git always wants to write UTF-16 with a\nBOM, and will refuse to read UTF-16 without it.\n\nHowever, musl's iconv implementation writes UTF-16 without a BOM,\nrelying on the user to interpret it as big-endian. This causes t0028 and\nthe related functionality to fail, since Git won't read the file without\na BOM.\n\nAdd a Makefile and #define knob, ICONV_OMITS_BOM, that can be set if the\niconv implementation has this behavior. When set, Git will write a BOM\nmanually for UTF-16 and UTF-32 and then force the data to be written in\nUTF-16BE or UTF-32BE. We choose big-endian behavior here because the\ntests use the raw \"UTF-16\" encoding, which will be big-endian when the\nimplementation requires this knob to be set.\n\nUpdate the tests to detect this case and write test data with an added\nBOM if necessary. Always write the BOM in the tests in big-endian\nformat, since all iconv implementations that omit a BOM must use\nbig-endian serialization according to the Unicode standard.\n\nPreserve the existing behavior for systems which do not have this knob\nenabled, since they may use optimized implementations, including\ndefaulting to the native endianness, which may improve performance.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n Makefile                         |  7 +++++++\n t/t0028-working-tree-encoding.sh | 34 +++++++++++++++++++++++++++-----\n utf8.c                           | 14 +++++++++++++\n 3 files changed, 50 insertions(+), 5 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 571160a2c4..c6172450af 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -259,6 +259,10 @@ all::\n # Define OLD_ICONV if your library has an old iconv(), where the second\n # (input buffer pointer) parameter is declared with type (const char **).\n #\n+# Define ICONV_OMITS_BOM if your iconv implementation does not write a\n+# byte-order mark (BOM) when writing UTF-16 or UTF-32 and always writes in\n+# big-endian format.\n+#\n # Define NO_DEFLATE_BOUND if your zlib does not have deflateBound.\n #\n # Define NO_R_TO_GCC_LINKER if your gcc does not like \"-R/path/lib\"\n@@ -1415,6 +1419,9 @@ ifndef NO_ICONV\n \t\tEXTLIBS += $(ICONV_LINK) -liconv\n \tendif\n endif\n+ifdef ICONV_OMITS_BOM\n+\tBASIC_CFLAGS += -DICONV_OMITS_BOM\n+endif\n ifdef NEEDS_LIBGEN\n \tEXTLIBS += -lgen\n endif\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex e58ecbfc44..500229a9bd 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -6,6 +6,30 @@ test_description='working-tree-encoding conversion via gitattributes'\n \n GIT_TRACE_WORKING_TREE_ENCODING=1 && export GIT_TRACE_WORKING_TREE_ENCODING\n \n+test_lazy_prereq NO_UTF16_BOM '\n+\ttest $(printf abc | iconv -f UTF-8 -t UTF-16 | wc -c) = 6\n+'\n+\n+test_lazy_prereq NO_UTF32_BOM '\n+\ttest $(printf abc | iconv -f UTF-8 -t UTF-32 | wc -c) = 12\n+'\n+\n+write_utf16 () {\n+\tif test_have_prereq NO_UTF16_BOM\n+\tthen\n+\t\tprintf '\\xfe\\xff'\n+\tfi &&\n+\ticonv -f UTF-8 -t UTF-16\n+}\n+\n+write_utf32 () {\n+\tif test_have_prereq NO_UTF32_BOM\n+\tthen\n+\t\tprintf '\\x00\\x00\\xfe\\xff'\n+\tfi &&\n+\ticonv -f UTF-8 -t UTF-32\n+}\n+\n test_expect_success 'setup test files' '\n \tgit config core.eol lf &&\n \n@@ -13,8 +37,8 @@ test_expect_success 'setup test files' '\n \techo \"*.utf16 text working-tree-encoding=utf-16\" >.gitattributes &&\n \techo \"*.utf16lebom text working-tree-encoding=UTF-16LE-BOM\" >>.gitattributes &&\n \tprintf \"$text\" >test.utf8.raw &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-32 >test.utf32.raw &&\n+\tprintf \"$text\" | write_utf16 >test.utf16.raw &&\n+\tprintf \"$text\" | write_utf32 >test.utf32.raw &&\n \tprintf \"\\377\\376\"                         >test.utf16lebom.raw &&\n \tprintf \"$text\" | iconv -f UTF-8 -t UTF-32LE >>test.utf16lebom.raw &&\n \n@@ -124,8 +148,8 @@ do\n \t\ttest_when_finished \"rm -f crlf.utf${i}.raw lf.utf${i}.raw\" &&\n \t\ttest_when_finished \"git reset --hard HEAD^\" &&\n \n-\t\tcat lf.utf8.raw | iconv -f UTF-8 -t UTF-${i} >lf.utf${i}.raw &&\n-\t\tcat crlf.utf8.raw | iconv -f UTF-8 -t UTF-${i} >crlf.utf${i}.raw &&\n+\t\tcat lf.utf8.raw | write_utf${i} >lf.utf${i}.raw &&\n+\t\tcat crlf.utf8.raw | write_utf${i} >crlf.utf${i}.raw &&\n \t\tcp crlf.utf${i}.raw eol.utf${i} &&\n \n \t\tcat >expectIndexLF <<-EOF &&\n@@ -223,7 +247,7 @@ test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' '\n \n \ttext=\"hallo there!\\nroundtrip test here!\" &&\n \tprintf \"$text\" | iconv -f UTF-8 -t SHIFT-JIS >roundtrip.shift &&\n-\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >roundtrip.utf16 &&\n+\tprintf \"$text\" | write_utf16 >roundtrip.utf16 &&\n \techo \"*.shift text working-tree-encoding=SHIFT-JIS\" >>.gitattributes &&\n \n \t# SHIFT-JIS encoded files are round-trip checked by default...\ndiff --git a/utf8.c b/utf8.c\nindex 83824dc2f4..3b42fadffd 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -559,6 +559,10 @@ char *reencode_string_len(const char *in, size_t insz,\n \t/*\n \t * For writing, UTF-16 iconv typically creates \"UTF-16BE-BOM\"\n \t * Some users under Windows want the little endian version\n+\t *\n+\t * We handle UTF-16 and UTF-32 ourselves only if the platform does not\n+\t * provide a BOM (which we require), since we want to match the behavior\n+\t * of the system tools and libc as much as possible.\n \t */\n \tif (same_utf_encoding(\"UTF-16LE-BOM\", out_encoding)) {\n \t\tbom_str = utf16_le_bom;\n@@ -568,6 +572,16 @@ char *reencode_string_len(const char *in, size_t insz,\n \t\tbom_str = utf16_be_bom;\n \t\tbom_len = sizeof(utf16_be_bom);\n \t\tout_encoding = \"UTF-16BE\";\n+#ifdef ICONV_OMITS_BOM\n+\t} else if (same_utf_encoding(\"UTF-16\", out_encoding)) {\n+\t\tbom_str = utf16_be_bom;\n+\t\tbom_len = sizeof(utf16_be_bom);\n+\t\tout_encoding = \"UTF-16BE\";\n+\t} else if (same_utf_encoding(\"UTF-32\", out_encoding)) {\n+\t\tbom_str = utf32_be_bom;\n+\t\tbom_len = sizeof(utf32_be_bom);\n+\t\tout_encoding = \"UTF-32BE\";\n+#endif\n \t}\n \n \tconv = iconv_open(out_encoding, in_encoding);\n"},{"id":"369078","messageId":"20190212005329.GD684736@genre.crustytoothpaste.net","threadId":"50430","inReplyTo":"xmqqtvh9g857.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3] utf8: handle systems that don't write BOM for UTF-16","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-02-12T00:53:29Z","receivedAt":"2019-02-12T00:53:37Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Feb 11, 2019 at 04:31:00PM -0800, Junio C Hamano wrote:\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n> \n> >> -       cat lf.utf8.raw | iconv -f UTF-8 -t UTF-${i} >lf.utf${i}.raw &&\n> >> -       cat crlf.utf8.raw | iconv -f UTF-8 -t UTF-${i} >crlf.utf${i}.raw &&\n> >> +       cat lf.utf8.raw | eval \"write_utf${i}\" >lf.utf${i}.raw &&\n> >> +       cat crlf.utf8.raw | eval \"write_utf${i}\" >crlf.utf${i}.raw &&\n> >>         cp crlf.utf${i}.raw eol.utf${i} &&\n> >> \n> >>         cat >expectIndexLF <<-EOF &&\n> >\n> > I'll squash in this fix, thanks.\n> \n> Thanks, all.  In the meantime, what I've pushed out has this\n> applied immediately on top.  Unless there is anything else, I could\n> squash it in in my next pushout I plan to do tonight, before getting\n> ready to tag -rc1 tomorrow.\n\nI've just sent a v4 with this squashed in. Whether you want to pick that\nup or squash this into v3 is up to you.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"369115","messageId":"xmqqh8d9g20e.fsf@gitster-ct.c.googlers.com","threadId":"50430","inReplyTo":"20190212005329.GD684736@genre.crustytoothpaste.net","subject":"Re: [PATCH v3] utf8: handle systems that don't write BOM for UTF-16","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-12T02:43:29Z","receivedAt":"2019-02-12T02:43:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> I've just sent a v4 with this squashed in. Whether you want to pick that\n> up or squash this into v3 is up to you.\n\nLet's take yours, as there is no point doing an eval for these two;\nfor that matter, braces around ${i} are also pointless, but I'll let\nthem pass.\n\nThanks.\n"}]}