git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3] utf8: handle systems that don't write BOM for UTF-16

From
Kevin Daudt <me@ikke.info>
Date
Feb 11, 2019, 21:43 UTC
Message-ID
<20190211214306.GB14229@alpha>
In-Reply-To
<20190211012639.579489-1-sandals@crustytoothpaste.net>
On Mon, Feb 11, 2019 at 01:26:39AM +0000, brian m. carlson wrote:
Show 141 quoted lines
> When serializing UTF-16 (and UTF-32), there are three possible ways to
> write the stream. One can write the data with a BOM in either big-endian
> or little-endian format, or one can write the data without a BOM in
> big-endian format.
> 
> Most systems' iconv implementations choose to write it with a BOM in
> some endianness, since this is the most foolproof, and it is resistant
> to misinterpretation on Windows, where UTF-16 and the little-endian
> serialization are very common. For compatibility with Windows and to
> avoid accidental misuse there, Git always wants to write UTF-16 with a
> BOM, and will refuse to read UTF-16 without it.
> 
> However, musl's iconv implementation writes UTF-16 without a BOM,
> relying on the user to interpret it as big-endian. This causes t0028 and
> the related functionality to fail, since Git won't read the file without
> a BOM.
> 
> Add a Makefile and #define knob, ICONV_OMITS_BOM, that can be set if the
> iconv implementation has this behavior. When set, Git will write a BOM
> manually for UTF-16 and UTF-32 and then force the data to be written in
> UTF-16BE or UTF-32BE. We choose big-endian behavior here because the
> tests use the raw "UTF-16" encoding, which will be big-endian when the
> implementation requires this knob to be set.
> 
> Update the tests to detect this case and write test data with an added
> BOM if necessary. Always write the BOM in the tests in big-endian
> format, since all iconv implementations that omit a BOM must use
> big-endian serialization according to the Unicode standard.
> 
> Preserve the existing behavior for systems which do not have this knob
> enabled, since they may use optimized implementations, including
> defaulting to the native endianness, which may improve performance.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  Makefile                         |  7 +++++++
>  t/t0028-working-tree-encoding.sh | 30 +++++++++++++++++++++++++++---
>  utf8.c                           | 10 ++++++++++
>  3 files changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 571160a2c4..c6172450af 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -259,6 +259,10 @@ all::
>  # Define OLD_ICONV if your library has an old iconv(), where the second
>  # (input buffer pointer) parameter is declared with type (const char **).
>  #
> +# Define ICONV_OMITS_BOM if your iconv implementation does not write a
> +# byte-order mark (BOM) when writing UTF-16 or UTF-32 and always writes in
> +# big-endian format.
> +#
>  # Define NO_DEFLATE_BOUND if your zlib does not have deflateBound.
>  #
>  # Define NO_R_TO_GCC_LINKER if your gcc does not like "-R/path/lib"
> @@ -1415,6 +1419,9 @@ ifndef NO_ICONV
>  		EXTLIBS += $(ICONV_LINK) -liconv
>  	endif
>  endif
> +ifdef ICONV_OMITS_BOM
> +	BASIC_CFLAGS += -DICONV_OMITS_BOM
> +endif
>  ifdef NEEDS_LIBGEN
>  	EXTLIBS += -lgen
>  endif
> diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh
> index e58ecbfc44..8936ba6757 100755
> --- a/t/t0028-working-tree-encoding.sh
> +++ b/t/t0028-working-tree-encoding.sh
> @@ -6,6 +6,30 @@ test_description='working-tree-encoding conversion via gitattributes'
>  
>  GIT_TRACE_WORKING_TREE_ENCODING=1 && export GIT_TRACE_WORKING_TREE_ENCODING
>  
> +test_lazy_prereq NO_UTF16_BOM '
> +	test $(printf abc | iconv -f UTF-8 -t UTF-16 | wc -c) = 6
> +'
> +
> +test_lazy_prereq NO_UTF32_BOM '
> +	test $(printf abc | iconv -f UTF-8 -t UTF-32 | wc -c) = 12
> +'
> +
> +write_utf16 () {
> +	if test_have_prereq NO_UTF16_BOM
> +	then
> +		printf '\xfe\xff'
> +	fi &&
> +	iconv -f UTF-8 -t UTF-16
> +}
> +
> +write_utf32 () {
> +	if test_have_prereq NO_UTF32_BOM
> +	then
> +		printf '\x00\x00\xfe\xff'
> +	fi &&
> +	iconv -f UTF-8 -t UTF-32
> +}
> +
>  test_expect_success 'setup test files' '
>  	git config core.eol lf &&
>  
> @@ -13,8 +37,8 @@ test_expect_success 'setup test files' '
>  	echo "*.utf16 text working-tree-encoding=utf-16" >.gitattributes &&
>  	echo "*.utf16lebom text working-tree-encoding=UTF-16LE-BOM" >>.gitattributes &&
>  	printf "$text" >test.utf8.raw &&
> -	printf "$text" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&
> -	printf "$text" | iconv -f UTF-8 -t UTF-32 >test.utf32.raw &&
> +	printf "$text" | write_utf16 >test.utf16.raw &&
> +	printf "$text" | write_utf32 >test.utf32.raw &&
>  	printf "\377\376"                         >test.utf16lebom.raw &&
>  	printf "$text" | iconv -f UTF-8 -t UTF-32LE >>test.utf16lebom.raw &&
>  
> @@ -223,7 +247,7 @@ test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' '
>  
>  	text="hallo there!\nroundtrip test here!" &&
>  	printf "$text" | iconv -f UTF-8 -t SHIFT-JIS >roundtrip.shift &&
> -	printf "$text" | iconv -f UTF-8 -t UTF-16 >roundtrip.utf16 &&
> +	printf "$text" | write_utf16 >roundtrip.utf16 &&
>  	echo "*.shift text working-tree-encoding=SHIFT-JIS" >>.gitattributes &&
>  
>  	# SHIFT-JIS encoded files are round-trip checked by default...
> diff --git a/utf8.c b/utf8.c
> index 83824dc2f4..5d9a917bc8 100644
> --- a/utf8.c
> +++ b/utf8.c
> @@ -568,6 +568,16 @@ char *reencode_string_len(const char *in, size_t insz,
>  		bom_str = utf16_be_bom;
>  		bom_len = sizeof(utf16_be_bom);
>  		out_encoding = "UTF-16BE";
> +#ifdef ICONV_OMITS_BOM
> +	} else if (same_utf_encoding("UTF-16", out_encoding)) {
> +		bom_str = utf16_be_bom;
> +		bom_len = sizeof(utf16_be_bom);
> +		out_encoding = "UTF-16BE";
> +	} else if (same_utf_encoding("UTF-32", out_encoding)) {
> +		bom_str = utf32_be_bom;
> +		bom_len = sizeof(utf32_be_bom);
> +		out_encoding = "UTF-32BE";
> +#endif
>  	}
>  
>  	conv = iconv_open(out_encoding, in_encoding);

With some additional fixes, this indeed does solve the issue for Alpine Linux, thanks.

I had to fix the following as well:

iff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh index 8936ba6757..8491f216aa 100755

--- a/t/t0028-working-tree-encoding.sh
+++ b/t/t0028-working-tree-encoding.sh
@@ -148,8 +148,8 @@ do
        test_when_finished "rm -f crlf.utf${i}.raw lf.utf${i}.raw" &&
        test_when_finished "git reset --hard HEAD^" &&

-       cat lf.utf8.raw | iconv -f UTF-8 -t UTF-${i} >lf.utf${i}.raw &&
-       cat crlf.utf8.raw | iconv -f UTF-8 -t UTF-${i} >crlf.utf${i}.raw &&
+       cat lf.utf8.raw | eval "write_utf${i}" >lf.utf${i}.raw &&
+       cat crlf.utf8.raw | eval "write_utf${i}" >crlf.utf${i}.raw &&
        cp crlf.utf${i}.raw eol.utf${i} &&

        cat >expectIndexLF <<-EOF &&
Previous: brian m. carlsonNext: brian m. carlson
Message 23 of 30 in “t0028-working-tree-encoding.sh failing on musl based systems (Alpine Linux)”
  1. Kevin DaudtFeb 7, 2019
  2. brian m. carlsonFeb 8, 2019
  3. Rich FelkerFeb 8, 2019
  4. brian m. carlsonFeb 8, 2019
  5. Kevin DaudtFeb 8, 2019
  6. brian m. carlsonFeb 8, 2019
  7. Junio C HamanoFeb 8, 2019
  8. Kevin DaudtFeb 8, 2019
  9. brian m. carlsonFeb 8, 2019
  10. Junio C HamanoFeb 8, 2019
  11. brian m. carlsonFeb 9, 2019
  12. Kevin DaudtFeb 9, 2019
  13. utf8: handle systems that don't write BOM for UTF-16brian m. carlson, Feb 9, 2019
  14. Eric SunshineFeb 10, 2019
  15. brian m. carlsonFeb 10, 2019
  16. Torsten BögershausenFeb 10, 2019
  17. brian m. carlsonFeb 10, 2019
  18. Junio C HamanoFeb 11, 2019
  19. utf8: handle systems that don't write BOM for UTF-16brian m. carlson, Feb 11, 2019
  20. Eric SunshineFeb 11, 2019
  21. brian m. carlsonFeb 11, 2019
  22. utf8: handle systems that don't write BOM for UTF-16brian m. carlson, Feb 11, 2019
  23. Kevin DaudtFeb 11, 2019
  24. brian m. carlsonFeb 11, 2019
  25. Junio C HamanoFeb 12, 2019
  26. brian m. carlsonFeb 12, 2019
  27. Junio C HamanoFeb 12, 2019
  28. utf8: handle systems that don't write BOM for UTF-16brian m. carlson, Feb 12, 2019
  29. Rich FelkerFeb 8, 2019
  30. Torsten BögershausenFeb 9, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.