{"thread":{"id":"47977","subject":"[PATCH v9 0/8] convert: add support for different encodings","startedAt":"2018-03-04T20:15:49Z","lastAt":"2018-03-07T17:39:48Z","messageCount":26,"participants":["lars.schneider@autodesk.com","Junio C Hamano","Lars Schneider","Eric Sunshine","Torsten Bögershausen"],"isPatch":true,"patchVersion":9,"patchTotal":8},"messages":[{"id":"340984","messageId":"20180304201418.60958-1-lars.schneider@autodesk.com","threadId":"47977","inReplyTo":null,"subject":"[PATCH v9 0/8] convert: add support for different encodings","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-04T20:14:10Z","receivedAt":"2018-03-04T20:15:49Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nHi,\n\nPatches 1-4,7 are preparation and helper functions.\nPatch 5,6,8 are the actual change.\n\nThis series depends on Torsten's 8462ff43e4 (convert_to_git():\nsafe_crlf/checksafe becomes int conv_flags, 2018-01-13) which is\nalready in master.\n\nChanges since v8:\n\n* move UTF BOM error checks in a new dedicated function\n validate_encoding() and into a dedicated commit (6)\n* remove unnecessary encoding struct (became a plain char*)\n* fail early and do not try to run the reencode content in case of a\n validation error\n* return early if roundtrip encoding was not found (avoid undefined\n pointer arithmetic)\n* fix wrong argument order in encode_to_worktree() error message\n* use test_when_finished to cleanup tests\n* move UTF-16/32 BOM test file generation into \"setup test\"\n* reduce code duplication in tests\n* improve documentation:\n - use *.rc and *.ps1 as examples as they are usually UTF-16 encoded\n* fix comment: /advise/advice/\n\nThanks a lot Eric for your great review! I think I fixed everything you\nobjected with one exception. You noticed that the current code only\nchecks for BOMs corresponding to the declared size (16 or 32 bits) [1].\nI understand your point of view and I agree that any BOM in these cases\nis *most likely* an error. However, according to the spec it might\nstill be valid. The comments on my related question on StackOverflow\nseem to support that view [2]. Therefore, I would like to leave it as\nit is in this series. If it turns out to be a problem in practice, then\nI am happy to change it later. OK for you?\n\n\nThanks,\nLars\n\n[1] https://public-inbox.org/git/DF6F3855-EFE7-4C13-AA53-819AAE0DEF7A@gmail.com/\n[2] https://stackoverflow.com/questions/49038872/is-a-utf-32be-bom-valid-in-an-utf-16le-declared-data-stream\n\n\n RFC: https://public-inbox.org/git/BDB9B884-6D17-4BE3-A83C-F67E2AFA2B46@gmail.com/\n v1: https://public-inbox.org/git/20171211155023.1405-1-lars.schneider@autodesk.com/\n v2: https://public-inbox.org/git/20171229152222.39680-1-lars.schneider@autodesk.com/\n v3: https://public-inbox.org/git/20180106004808.77513-1-lars.schneider@autodesk.com/\n v4: https://public-inbox.org/git/20180120152418.52859-1-lars.schneider@autodesk.com/\n v5: https://public-inbox.org/git/20180129201855.9182-1-tboegi@web.de/\n v6: https://public-inbox.org/git/20180209132830.55385-1-lars.schneider@autodesk.com/\n v7: https://public-inbox.org/git/20180215152711.158-1-lars.schneider@autodesk.com/\n v8: https://public-inbox.org/git/20180224162801.98860-1-lars.schneider@autodesk.com/\n\n\nBase Ref:\nWeb-Diff: https://github.com/larsxschneider/git/commit/fdf0d63188\nCheckout: git fetch https://github.com/larsxschneider/git encoding-v9 && git checkout fdf0d63188\n\n\n### Interdiff (v8..v9):\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 11315054f4..aa3deae392 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -297,14 +297,16 @@ number of pitfalls:\n in your repository, then it is strongly recommended to ensure that all\n clients working with the repository support it.\n\n- If you declare `*.proj` files as UTF-16 and you add `foo.proj` with an\n- `working-tree-encoding` enabled Git client, then `foo.proj` will be\n+ For example, Microsoft Visual Studio resources files (`*.rc`) or\n+ PowerShell script files (`*.ps1`) are sometimes encoded in UTF-16.\n+ If you declare `*.ps1` as files as UTF-16 and you add `foo.ps1` with\n+ a `working-tree-encoding` enabled Git client, then `foo.ps1` will be\n stored as UTF-8 internally. A client without `working-tree-encoding`\n- support will checkout `foo.proj` as UTF-8 encoded file. This will\n+ support will checkout `foo.ps1` as UTF-8 encoded file. This will\n typically cause trouble for the users of this file.\n\n If a Git client, that does not support the `working-tree-encoding`\n- attribute, adds a new file `bar.proj`, then `bar.proj` will be\n+ attribute, adds a new file `bar.ps1`, then `bar.ps1` will be\n stored \"as-is\" internally (in this example probably as UTF-16).\n A client with `working-tree-encoding` support will interpret the\n internal contents as UTF-8 and try to convert it to UTF-16 on checkout.\n@@ -325,22 +327,22 @@ Use the `working-tree-encoding` attribute only if you cannot store a file\n in UTF-8 encoding and if you want Git to be able to process the content\n as text.\n\n-As an example, use the following attributes if your '*.proj' files are\n+As an example, use the following attributes if your '*.ps1' files are\n UTF-16 encoded with byte order mark (BOM) and you want Git to perform\n automatic line ending conversion based on your platform.\n\n ------------------------\n-*.proj\t\ttext working-tree-encoding=UTF-16\n+*.ps1\t\ttext working-tree-encoding=UTF-16\n ------------------------\n\n-Use the following attributes if your '*.proj' files are UTF-16 little\n+Use the following attributes if your '*.ps1' files are UTF-16 little\n endian encoded without BOM and you want Git to use Windows line endings\n in the working directory. Please note, it is highly recommended to\n explicitly define the line endings with `eol` if the `working-tree-encoding`\n attribute is used to avoid ambiguity.\n\n ------------------------\n-*.proj \t\ttext working-tree-encoding=UTF-16LE eol=CRLF\n+*.ps1\t\ttext working-tree-encoding=UTF-16LE eol=CRLF\n ------------------------\n\n You can get a list of all available encodings on your platform with the\n@@ -354,7 +356,7 @@ If you do not know the encoding of a file, then you can use the `file`\n command to guess the encoding:\n\n ------------------------\n-file foo.proj\n+file foo.ps1\n ------------------------\n\n\ndiff --git a/convert.c b/convert.c\nindex 398cd9cf7b..6cbb2b2618 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -266,6 +266,53 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n\n }\n\n+static int validate_encoding(const char *path, const char *enc,\n+\t\t const char *data, size_t len, int die_on_error)\n+{\n+\tif (!memcmp(\"UTF-\", enc, 4)) {\n+\t\t/*\n+\t\t * Check for detectable errors in UTF encodings\n+\t\t */\n+\t\tif (has_prohibited_utf_bom(enc, data, len)) {\n+\t\t\tconst char *error_msg = _(\n+\t\t\t\t\"BOM is prohibited in '%s' if encoded as %s\");\n+\t\t\t/*\n+\t\t\t * This advice is shown for UTF-??BE and UTF-??LE\n+\t\t\t * encodings. We truncate the encoding name to 6\n+\t\t\t * chars with %.6s to cut off the last two \"byte\n+\t\t\t * order\" characters.\n+\t\t\t */\n+\t\t\tconst char *advise_msg = _(\n+\t\t\t\t\"The file '%s' contains a byte order \"\n+\t\t\t\t\"mark (BOM). Please use %.6s as \"\n+\t\t\t\t\"working-tree-encoding.\");\n+\t\t\tadvise(advise_msg, path, enc);\n+\t\t\tif (die_on_error)\n+\t\t\t\tdie(error_msg, path, enc);\n+\t\t\telse {\n+\t\t\t\treturn error(error_msg, path, enc);\n+\t\t\t}\n+\n+\t\t} else if (is_missing_required_utf_bom(enc, data, len)) {\n+\t\t\tconst char *error_msg = _(\n+\t\t\t\t\"BOM is required in '%s' if encoded as %s\");\n+\t\t\tconst char *advise_msg = _(\n+\t\t\t\t\"The file '%s' is missing a byte order \"\n+\t\t\t\t\"mark (BOM). Please use %sBE or %sLE \"\n+\t\t\t\t\"(depending on the byte order) as \"\n+\t\t\t\t\"working-tree-encoding.\");\n+\t\t\tadvise(advise_msg, path, enc, enc);\n+\t\t\tif (die_on_error)\n+\t\t\t\tdie(error_msg, path, enc);\n+\t\t\telse {\n+\t\t\t\treturn error(error_msg, path, enc);\n+\t\t\t}\n+\t\t}\n+\n+\t}\n+\treturn 0;\n+}\n+\n static void trace_encoding(const char *context, const char *path,\n \t\t\t const char *encoding, const char *buf, size_t len)\n {\n@@ -297,8 +344,12 @@ static int check_roundtrip(const char* enc_name)\n \t * Search for the given encoding in that string.\n \t */\n \tconst char *found = strcasestr(check_roundtrip_encoding, enc_name);\n-\tconst char *next = found + strlen(enc_name);\n-\tint len = strlen(check_roundtrip_encoding);\n+\tconst char *next;\n+\tint len;\n+\tif (!found)\n+\t\treturn 0;\n+\tnext = found + strlen(enc_name);\n+\tlen = strlen(check_roundtrip_encoding);\n \treturn (found && (\n \t\t\t/*\n \t\t\t * check that the found encoding is at the\n@@ -322,17 +373,14 @@ static int check_roundtrip(const char* enc_name)\n \t\t));\n }\n\n-static struct encoding {\n-\tconst char *name;\n-\tstruct encoding *next;\n-} *encoding, **encoding_tail;\n static const char *default_encoding = \"UTF-8\";\n\n static int encode_to_git(const char *path, const char *src, size_t src_len,\n-\t\t\t struct strbuf *buf, struct encoding *enc, int conv_flags)\n+\t\t\t struct strbuf *buf, const char *enc, int conv_flags)\n {\n \tchar *dst;\n \tint dst_len;\n+\tint die_on_error = conv_flags & CONV_WRITE_OBJECT;\n\n \t/*\n \t * No encoding is specified or there is nothing to encode.\n@@ -350,39 +398,11 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \tif (!buf && !src)\n \t\treturn 1;\n\n-\tif (has_prohibited_utf_bom(enc->name, src, src_len)) {\n-\t\tconst char *error_msg = _(\n-\t\t\t\"BOM is prohibited in '%s' if encoded as %s\");\n-\t\t/*\n-\t\t * This advise is shown for UTF-??BE and UTF-??LE encodings.\n-\t\t * We truncate the encoding name to 6 chars with %.6s to cut\n-\t\t * off the last two \"byte order\" characters.\n-\t\t */\n-\t\tconst char *advise_msg = _(\n-\t\t\t\"The file '%s' contains a byte order mark (BOM). \"\n-\t\t\t\"Please use %.6s as working-tree-encoding.\");\n-\t\tadvise(advise_msg, path, enc->name);\n-\t\tif (conv_flags & CONV_WRITE_OBJECT)\n-\t\t\tdie(error_msg, path, enc->name);\n-\t\telse\n-\t\t\terror(error_msg, path, enc->name);\n-\n-\t} else if (is_missing_required_utf_bom(enc->name, src, src_len)) {\n-\t\tconst char *error_msg = _(\n-\t\t\t\"BOM is required in '%s' if encoded as %s\");\n-\t\tconst char *advise_msg = _(\n-\t\t\t\"The file '%s' is missing a byte order mark (BOM). \"\n-\t\t\t\"Please use %sBE or %sLE (depending on the byte order) \"\n-\t\t\t\"as working-tree-encoding.\");\n-\t\tadvise(advise_msg, path, enc->name, enc->name);\n-\t\tif (conv_flags & CONV_WRITE_OBJECT)\n-\t\t\tdie(error_msg, path, enc->name);\n-\t\telse\n-\t\t\terror(error_msg, path, enc->name);\n-\t}\n+\tif (validate_encoding(path, enc, src, src_len, die_on_error))\n+\t\treturn 0;\n\n-\ttrace_encoding(\"source\", path, enc->name, src, src_len);\n-\tdst = reencode_string_len(src, src_len, default_encoding, enc->name,\n+\ttrace_encoding(\"source\", path, enc, src, src_len);\n+\tdst = reencode_string_len(src, src_len, default_encoding, enc,\n \t\t\t\t &dst_len);\n \tif (!dst) {\n \t\t/*\n@@ -392,10 +412,12 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \t\t * working tree. Let's try to avoid this by screaming loud.\n \t\t */\n \t\tconst char* msg = _(\"failed to encode '%s' from %s to %s\");\n-\t\tif (conv_flags & CONV_WRITE_OBJECT)\n-\t\t\tdie(msg, path, enc->name, default_encoding);\n-\t\telse\n-\t\t\terror(msg, path, enc->name, default_encoding);\n+\t\tif (die_on_error)\n+\t\t\tdie(msg, path, enc, default_encoding);\n+\t\telse {\n+\t\t\terror(msg, path, enc, default_encoding);\n+\t\t\treturn 0;\n+\t\t}\n \t}\n \ttrace_encoding(\"destination\", path, default_encoding, dst, dst_len);\n\n@@ -418,23 +440,23 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \t * [1] http://unicode.org/faq/utf_bom.html#gen2\n \t * [2] https://support.microsoft.com/en-us/help/170559/prb-conversion-problem-between-shift-jis-and-unicode\n \t */\n-\tif ((conv_flags & CONV_WRITE_OBJECT) && check_roundtrip(enc->name)) {\n+\tif (die_on_error && check_roundtrip(enc)) {\n \t\tchar *re_src;\n \t\tint re_src_len;\n\n \t\tre_src = reencode_string_len(dst, dst_len,\n-\t\t\t\t\t enc->name, default_encoding,\n+\t\t\t\t\t enc, default_encoding,\n \t\t\t\t\t &re_src_len);\n\n-\t\ttrace_printf(\"Checking roundtrip encoding for %s...\\n\", enc->name);\n-\t\ttrace_encoding(\"reencoded source\", path, enc->name,\n+\t\ttrace_printf(\"Checking roundtrip encoding for %s...\\n\", enc);\n+\t\ttrace_encoding(\"reencoded source\", path, enc,\n \t\t\t re_src, re_src_len);\n\n \t\tif (!re_src || src_len != re_src_len ||\n \t\t memcmp(src, re_src, src_len)) {\n \t\t\tconst char* msg = _(\"encoding '%s' from %s to %s and \"\n \t\t\t\t\t \"back is not the same\");\n-\t\t\tdie(msg, path, enc->name, default_encoding);\n+\t\t\tdie(msg, path, enc, default_encoding);\n \t\t}\n\n \t\tfree(re_src);\n@@ -445,7 +467,7 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n }\n\n static int encode_to_worktree(const char *path, const char *src, size_t src_len,\n-\t\t\t struct strbuf *buf, struct encoding *enc)\n+\t\t\t struct strbuf *buf, const char *enc)\n {\n \tchar *dst;\n \tint dst_len;\n@@ -457,11 +479,11 @@ static int encode_to_worktree(const char *path, const char *src, size_t src_len,\n \tif (!enc || (src && !src_len))\n \t\treturn 0;\n\n-\tdst = reencode_string_len(src, src_len, enc->name, default_encoding,\n+\tdst = reencode_string_len(src, src_len, enc, default_encoding,\n \t\t\t\t &dst_len);\n \tif (!dst) {\n \t\terror(\"failed to encode '%s' from %s to %s\",\n-\t\t\tpath, enc->name, default_encoding);\n+\t\t\tpath, default_encoding, enc);\n \t\treturn 0;\n \t}\n\n@@ -1182,33 +1204,23 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,\n \treturn 1;\n }\n\n-static struct encoding *git_path_check_encoding(struct attr_check_item *check)\n+static const char *git_path_check_encoding(struct attr_check_item *check)\n {\n \tconst char *value = check->value;\n-\tstruct encoding *enc;\n\n \tif (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value) ||\n \t !strlen(value))\n \t\treturn NULL;\n\n-\tfor (enc = encoding; enc; enc = enc->next)\n-\t\tif (!strcasecmp(value, enc->name))\n-\t\t\treturn enc;\n-\n \t/* Don't encode to the default encoding */\n \tif (!strcasecmp(value, default_encoding))\n \t\treturn NULL;\n\n-\tenc = xcalloc(1, sizeof(*enc));\n \t/*\n \t * Ensure encoding names are always upper case (e.g. UTF-8) to\n \t * simplify subsequent string comparisons.\n \t */\n-\tenc->name = xstrdup_toupper(value);\n-\t*encoding_tail = enc;\n-\tencoding_tail = &(enc->next);\n-\n-\treturn enc;\n+\treturn xstrdup_toupper(value);\n }\n\n static enum crlf_action git_path_check_crlf(struct attr_check_item *check)\n@@ -1266,7 +1278,7 @@ struct conv_attrs {\n \tenum crlf_action attr_action; /* What attr says */\n \tenum crlf_action crlf_action; /* When no attr is set, use core.autocrlf */\n \tint ident;\n-\tstruct encoding *working_tree_encoding; /* Supported encoding or default encoding if NULL */\n+\tconst char *working_tree_encoding; /* Supported encoding or default encoding if NULL */\n };\n\n static void convert_attrs(struct conv_attrs *ca, const char *path)\n@@ -1278,7 +1290,6 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)\n \t\t\t\t\t \"eol\", \"text\", \"working-tree-encoding\",\n \t\t\t\t\t NULL);\n \t\tuser_convert_tail = &user_convert;\n-\t\tencoding_tail = &encoding;\n \t\tgit_config(read_convert_config, NULL);\n \t}\n\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex e34c21eb29..23e89ae623 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -6,7 +6,7 @@ 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_expect_success 'setup test repo' '\n+test_expect_success 'setup test files' '\n \tgit config core.eol lf &&\n\n \ttext=\"hallo there!\\ncan you read me?\" &&\n@@ -14,164 +14,135 @@ test_expect_success 'setup test repo' '\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-\tcp test.utf16.raw test.utf16 &&\n-\tcp test.utf32.raw test.utf32 &&\n+\n+\t# Line ending tests\n+\tprintf \"one\\ntwo\\nthree\\n\" >lf.utf8.raw &&\n+\tprintf \"one\\r\\ntwo\\r\\nthree\\r\\n\" >crlf.utf8.raw &&\n+\n+\t# BOM tests\n+\tprintf \"\\0a\\0b\\0c\" >nobom.utf16be.raw &&\n+\tprintf \"a\\0b\\0c\\0\" >nobom.utf16le.raw &&\n+\tprintf \"\\376\\777\\0a\\0b\\0c\" >bebom.utf16be.raw &&\n+\tprintf \"\\777\\376a\\0b\\0c\\0\" >lebom.utf16le.raw &&\n+\tprintf \"\\0\\0\\0a\\0\\0\\0b\\0\\0\\0c\" >nobom.utf32be.raw &&\n+\tprintf \"a\\0\\0\\0b\\0\\0\\0c\\0\\0\\0\" >nobom.utf32le.raw &&\n+\tprintf \"\\0\\0\\376\\777\\0\\0\\0a\\0\\0\\0b\\0\\0\\0c\" >bebom.utf32be.raw &&\n+\tprintf \"\\777\\376\\0\\0a\\0\\0\\0b\\0\\0\\0c\\0\\0\\0\" >lebom.utf32le.raw &&\n\n \t# Add only UTF-16 file, we will add the UTF-32 file later\n+\tcp test.utf16.raw test.utf16 &&\n+\tcp test.utf32.raw test.utf32 &&\n \tgit add .gitattributes test.utf16 &&\n \tgit commit -m initial\n '\n\n test_expect_success 'ensure UTF-8 is stored in Git' '\n-\tgit cat-file -p :test.utf16 >test.utf16.git &&\n-\ttest_cmp_bin test.utf8.raw test.utf16.git &&\n+\ttest_when_finished \"rm -f test.utf16.git\" &&\n\n-\t# cleanup\n-\trm test.utf16.git\n+\tgit cat-file -p :test.utf16 >test.utf16.git &&\n+\ttest_cmp_bin test.utf8.raw test.utf16.git\n '\n\n test_expect_success 're-encode to UTF-16 on checkout' '\n+\ttest_when_finished \"rm -f test.utf16.raw\" &&\n+\n \trm test.utf16 &&\n \tgit checkout test.utf16 &&\n-\ttest_cmp_bin test.utf16.raw test.utf16 &&\n-\n-\t# cleanup\n-\trm test.utf16.raw\n+\ttest_cmp_bin test.utf16.raw test.utf16\n '\n\n test_expect_success 'check $GIT_DIR/info/attributes support' '\n-\techo \"*.utf32 text working-tree-encoding=utf-32\" >.git/info/attributes &&\n+\ttest_when_finished \"rm -f test.utf8.raw test.utf32.raw test.utf32.git\" &&\n+\ttest_when_finished \"git reset --hard HEAD\" &&\n\n+\techo \"*.utf32 text working-tree-encoding=utf-32\" >.git/info/attributes &&\n \tgit add test.utf32 &&\n\n \tgit cat-file -p :test.utf32 >test.utf32.git &&\n-\ttest_cmp_bin test.utf8.raw test.utf32.git &&\n-\n-\t# cleanup\n-\tgit reset --hard HEAD &&\n-\trm test.utf8.raw test.utf32.raw test.utf32.git\n+\ttest_cmp_bin test.utf8.raw test.utf32.git\n '\n\n-test_expect_success 'check prohibited UTF BOM' '\n-\tprintf \"\\0a\\0b\\0c\" >nobom.utf16be.raw &&\n-\tprintf \"a\\0b\\0c\\0\" >nobom.utf16le.raw &&\n-\tprintf \"\\376\\777\\0a\\0b\\0c\" >bebom.utf16be.raw &&\n-\tprintf \"\\777\\376a\\0b\\0c\\0\" >lebom.utf16le.raw &&\n+for i in 16 32\n+do\n+\ttest_expect_success \"check prohibited UTF-${i} BOM\" '\n+\t\ttest_when_finished \"git reset --hard HEAD\" &&\n\n-\tprintf \"\\0\\0\\0a\\0\\0\\0b\\0\\0\\0c\" >nobom.utf32be.raw &&\n-\tprintf \"a\\0\\0\\0b\\0\\0\\0c\\0\\0\\0\" >nobom.utf32le.raw &&\n-\tprintf \"\\0\\0\\376\\777\\0\\0\\0a\\0\\0\\0b\\0\\0\\0c\" >bebom.utf32be.raw &&\n-\tprintf \"\\777\\376\\0\\0a\\0\\0\\0b\\0\\0\\0c\\0\\0\\0\" >lebom.utf32le.raw &&\n+\t\techo \"*.utf${i}be text working-tree-encoding=utf-${i}be\" >>.gitattributes &&\n+\t\techo \"*.utf${i}le text working-tree-encoding=utf-${i}le\" >>.gitattributes &&\n\n-\techo \"*.utf16be text working-tree-encoding=utf-16be\" >>.gitattributes &&\n-\techo \"*.utf16le text working-tree-encoding=utf-16le\" >>.gitattributes &&\n-\techo \"*.utf32be text working-tree-encoding=utf-32be\" >>.gitattributes &&\n-\techo \"*.utf32le text working-tree-encoding=utf-32le\" >>.gitattributes &&\n-\n-\t# Here we add a UTF-16 files with BOM (big-endian and little-endian)\n-\t# but we tell Git to treat it as UTF-16BE/UTF-16LE. In these cases\n-\t# the BOM is prohibited.\n-\tcp bebom.utf16be.raw bebom.utf16be &&\n-\ttest_must_fail git add bebom.utf16be 2>err.out &&\n-\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-16BE\" err.out &&\n-\n-\tcp lebom.utf16le.raw lebom.utf16be &&\n-\ttest_must_fail git add lebom.utf16be 2>err.out &&\n-\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-16BE\" err.out &&\n-\n-\tcp bebom.utf16be.raw bebom.utf16le &&\n-\ttest_must_fail git add bebom.utf16le 2>err.out &&\n-\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-16LE\" err.out &&\n-\n-\tcp lebom.utf16le.raw lebom.utf16le &&\n-\ttest_must_fail git add lebom.utf16le 2>err.out &&\n-\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-16LE\" err.out &&\n-\n-\t# ... and the same for UTF-32\n-\tcp bebom.utf32be.raw bebom.utf32be &&\n-\ttest_must_fail git add bebom.utf32be 2>err.out &&\n-\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-32BE\" err.out &&\n-\n-\tcp lebom.utf32le.raw lebom.utf32be &&\n-\ttest_must_fail git add lebom.utf32be 2>err.out &&\n-\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-32BE\" err.out &&\n-\n-\tcp bebom.utf32be.raw bebom.utf32le &&\n-\ttest_must_fail git add bebom.utf32le 2>err.out &&\n-\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-32LE\" err.out &&\n-\n-\tcp lebom.utf32le.raw lebom.utf32le &&\n-\ttest_must_fail git add lebom.utf32le 2>err.out &&\n-\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-32LE\" err.out &&\n-\n-\t# cleanup\n-\tgit reset --hard HEAD\n-'\n+\t\t# Here we add a UTF-16 (resp. UTF-32) files with BOM (big/little-endian)\n+\t\t# but we tell Git to treat it as UTF-16BE/UTF-16LE (resp. UTF-32).\n+\t\t# In these cases the BOM is prohibited.\n+\t\tcp bebom.utf${i}be.raw bebom.utf${i}be &&\n+\t\ttest_must_fail git add bebom.utf${i}be 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-${i}BE\" err.out &&\n+\n+\t\tcp lebom.utf${i}le.raw lebom.utf${i}be &&\n+\t\ttest_must_fail git add lebom.utf${i}be 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-${i}BE\" err.out &&\n\n-test_expect_success 'check required UTF BOM' '\n-\techo \"*.utf32 text working-tree-encoding=utf-32\" >>.gitattributes &&\n+\t\tcp bebom.utf${i}be.raw bebom.utf${i}le &&\n+\t\ttest_must_fail git add bebom.utf${i}le 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-${i}LE\" err.out &&\n\n-\tcp nobom.utf16be.raw nobom.utf16 &&\n-\ttest_must_fail git add nobom.utf16 2>err.out &&\n-\ttest_i18ngrep \"fatal: BOM is required .* UTF-16\" err.out &&\n+\t\tcp lebom.utf${i}le.raw lebom.utf${i}le &&\n+\t\ttest_must_fail git add lebom.utf${i}le 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-${i}LE\" err.out\n+\t'\n\n-\tcp nobom.utf16le.raw nobom.utf16 &&\n-\ttest_must_fail git add nobom.utf16 2>err.out &&\n-\ttest_i18ngrep \"fatal: BOM is required .* UTF-16\" err.out &&\n+\ttest_expect_success \"check required UTF-${i} BOM\" '\n+\t\ttest_when_finished \"git reset --hard HEAD\" &&\n\n-\tcp nobom.utf32be.raw nobom.utf32 &&\n-\ttest_must_fail git add nobom.utf32 2>err.out &&\n-\ttest_i18ngrep \"fatal: BOM is required .* UTF-32\" err.out &&\n+\t\techo \"*.utf${i} text working-tree-encoding=utf-${i}\" >>.gitattributes &&\n\n-\tcp nobom.utf32le.raw nobom.utf32 &&\n-\ttest_must_fail git add nobom.utf32 2>err.out &&\n-\ttest_i18ngrep \"fatal: BOM is required .* UTF-32\" err.out &&\n+\t\tcp nobom.utf${i}be.raw nobom.utf${i} &&\n+\t\ttest_must_fail git add nobom.utf${i} 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is required .* UTF-${i}\" err.out &&\n\n-\t# cleanup\n-\trm nobom.utf16 nobom.utf32 &&\n-\tgit reset --hard HEAD\n+\t\tcp nobom.utf${i}le.raw nobom.utf${i} &&\n+\t\ttest_must_fail git add nobom.utf${i} 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is required .* UTF-${i}\" err.out\n \t'\n\n-test_expect_success 'eol conversion for UTF-16 encoded files on checkout' '\n-\tprintf \"one\\ntwo\\nthree\\n\" >lf.utf8.raw &&\n-\tprintf \"one\\r\\ntwo\\r\\nthree\\r\\n\" >crlf.utf8.raw &&\n+\ttest_expect_success \"eol conversion for UTF-${i} encoded files on checkout\" '\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-\tcat lf.utf8.raw | iconv -f UTF-8 -t UTF-16 >lf.utf16.raw &&\n-\tcat crlf.utf8.raw | iconv -f UTF-8 -t UTF-16 >crlf.utf16.raw &&\n-\tcp crlf.utf16.raw eol.utf16 &&\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\tcp crlf.utf${i}.raw eol.utf${i} &&\n\n-\tcat >expectIndexLF <<-\\EOF &&\n-\t\ti/lf w/-text attr/text \teol.utf16\n+\t\tcat >expectIndexLF <<-EOF &&\n+\t\t\ti/lf w/-text attr/text \teol.utf${i}\n \t\tEOF\n\n-\tgit add eol.utf16 &&\n+\t\tgit add eol.utf${i} &&\n \t\tgit commit -m eol &&\n\n-\t# UTF-16 with CRLF (Windows line endings)\n-\trm eol.utf16 &&\n-\tgit -c core.eol=crlf checkout eol.utf16 &&\n-\ttest_cmp_bin crlf.utf16.raw eol.utf16 &&\n+\t\t# UTF-${i} with CRLF (Windows line endings)\n+\t\trm eol.utf${i} &&\n+\t\tgit -c core.eol=crlf checkout eol.utf${i} &&\n+\t\ttest_cmp_bin crlf.utf${i}.raw eol.utf${i} &&\n\n-\t# Although the file has CRLF in the working tree, ensure LF in the index\n-\tgit ls-files --eol eol.utf16 >actual &&\n+\t\t# Although the file has CRLF in the working tree,\n+\t\t# ensure LF in the index\n+\t\tgit ls-files --eol eol.utf${i} >actual &&\n \t\ttest_cmp expectIndexLF actual &&\n\n-\t# UTF-16 with LF (Unix line endings)\n-\trm eol.utf16 &&\n-\tgit -c core.eol=lf checkout eol.utf16 &&\n-\ttest_cmp_bin lf.utf16.raw eol.utf16 &&\n+\t\t# UTF-${i} with LF (Unix line endings)\n+\t\trm eol.utf${i} &&\n+\t\tgit -c core.eol=lf checkout eol.utf${i} &&\n+\t\ttest_cmp_bin lf.utf${i}.raw eol.utf${i} &&\n\n \t\t# The file LF in the working tree, ensure LF in the index\n-\tgit ls-files --eol eol.utf16 >actual &&\n-\ttest_cmp expectIndexLF actual&&\n-\n-\trm crlf.utf16.raw crlf.utf8.raw lf.utf16.raw lf.utf8.raw &&\n-\n-\t# cleanup\n-\tgit reset --hard HEAD^\n+\t\tgit ls-files --eol eol.utf${i} >actual &&\n+\t\ttest_cmp expectIndexLF actual\n \t'\n+done\n\n test_expect_success 'check unsupported encodings' '\n+\ttest_when_finished \"rm -f err.out\" &&\n+\ttest_when_finished \"git reset --hard HEAD\" &&\n\n \techo \"*.nothing text working-tree-encoding=\" >>.gitattributes &&\n \tprintf \"nothing\" >t.nothing &&\n@@ -180,34 +151,33 @@ test_expect_success 'check unsupported encodings' '\n \techo \"*.garbage text working-tree-encoding=garbage\" >>.gitattributes &&\n \tprintf \"garbage\" >t.garbage &&\n \ttest_must_fail git add t.garbage 2>err.out &&\n-\ttest_i18ngrep \"fatal: failed to encode\" err.out &&\n-\n-\t# cleanup\n-\trm err.out &&\n-\tgit reset --hard HEAD\n+\ttest_i18ngrep \"fatal: failed to encode\" err.out\n '\n\n test_expect_success 'error if encoding round trip is not the same during refresh' '\n \tBEFORE_STATE=$(git rev-parse HEAD) &&\n-\n-\t# Skip the UTF-16 filter for the added file\n-\t# This simulates a Git version that has no working tree encoding support\n-\techo \"hallo\" >nonsense.utf16 &&\n-\tTEST_HASH=$(git hash-object --no-filters -w nonsense.utf16) &&\n-\tgit update-index --add --cacheinfo 100644 $TEST_HASH nonsense.utf16 &&\n+\ttest_when_finished \"rm -f err.out\" &&\n+\ttest_when_finished \"git reset --hard $BEFORE_STATE\" &&\n+\n+\t# Add and commit a UTF-16 file but skip the \"working-tree-encoding\"\n+\t# filter. Consequently, the in-repo representation is UTF-16 and not\n+\t# UTF-8. This simulates a Git version that has no working tree encoding\n+\t# support.\n+\techo \"*.utf16le text working-tree-encoding=utf-16le\" >.gitattributes &&\n+\techo \"hallo\" >nonsense.utf16le &&\n+\tTEST_HASH=$(git hash-object --no-filters -w nonsense.utf16le) &&\n+\tgit update-index --add --cacheinfo 100644 $TEST_HASH nonsense.utf16le &&\n \tCOMMIT=$(git commit-tree -p $(git rev-parse HEAD) -m \"plain commit\" $(git write-tree)) &&\n \tgit update-ref refs/heads/master $COMMIT &&\n\n \ttest_must_fail git checkout HEAD^ 2>err.out &&\n-\ttest_i18ngrep \"error: .* overwritten by checkout:\" err.out &&\n-\n-\t# cleanup\n-\trm err.out &&\n-\tgit reset --hard $BEFORE_STATE\n+\ttest_i18ngrep \"error: .* overwritten by checkout:\" err.out\n '\n\n test_expect_success 'error if encoding garbage is already in Git' '\n \tBEFORE_STATE=$(git rev-parse HEAD) &&\n+\ttest_when_finished \"rm -f err.out\" &&\n+\ttest_when_finished \"git reset --hard $BEFORE_STATE\" &&\n\n \t# Skip the UTF-16 filter for the added file\n \t# This simulates a Git version that has no checkoutEncoding support\n@@ -218,52 +188,46 @@ test_expect_success 'error if encoding garbage is already in Git' '\n \tgit update-ref refs/heads/master $COMMIT &&\n\n \tgit diff 2>err.out &&\n-\ttest_i18ngrep \"error: BOM is required\" err.out &&\n-\n-\t# cleanup\n-\trm err.out &&\n-\tgit reset --hard $BEFORE_STATE\n+\ttest_i18ngrep \"error: BOM is required\" err.out\n '\n\n test_expect_success 'check roundtrip encoding' '\n+\ttest_when_finished \"rm -f roundtrip.shift roundtrip.utf16\" &&\n+\ttest_when_finished \"git reset --hard HEAD\" &&\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 \techo \"*.shift text working-tree-encoding=SHIFT-JIS\" >>.gitattributes &&\n\n \t# SHIFT-JIS encoded files are round-trip checked by default...\n-\tGIT_TRACE=1 git add .gitattributes roundtrip.shift 2>&1 >/dev/null |\n+\tGIT_TRACE=1 git add .gitattributes roundtrip.shift 2>&1 |\n \t\tgrep \"Checking roundtrip encoding for SHIFT-JIS\" &&\n \tgit reset &&\n\n \t# ... unless we overwrite the Git config!\n-\ttest_config core.checkRoundtripEncoding \"garbage\" &&\n-\t! GIT_TRACE=1 git add .gitattributes roundtrip.shift 2>&1 >/dev/null |\n+\t! GIT_TRACE=1 git -c core.checkRoundtripEncoding=garbage \\\n+\t\tadd .gitattributes roundtrip.shift 2>&1 |\n \t\tgrep \"Checking roundtrip encoding for SHIFT-JIS\" &&\n-\ttest_unconfig core.checkRoundtripEncoding &&\n \tgit reset &&\n\n \t# UTF-16 encoded files should not be round-trip checked by default...\n-\t! GIT_TRACE=1 git add roundtrip.utf16 2>&1 >/dev/null |\n+\t! GIT_TRACE=1 git add roundtrip.utf16 2>&1 |\n \t\tgrep \"Checking roundtrip encoding for UTF-16\" &&\n \tgit reset &&\n\n \t# ... unless we tell Git to check it!\n-\ttest_config_global core.checkRoundtripEncoding \"UTF-16, UTF-32\" &&\n-\tGIT_TRACE=1 git add roundtrip.utf16 2>&1 >/dev/null |\n+\tGIT_TRACE=1 git -c core.checkRoundtripEncoding=\"UTF-16, UTF-32\" \\\n+\t\tadd roundtrip.utf16 2>&1 |\n \t\tgrep \"Checking roundtrip encoding for UTF-16\" &&\n \tgit reset &&\n\n \t# ... unless we tell Git to check it!\n \t# (here we also check that the casing of the encoding is irrelevant)\n-\ttest_config_global core.checkRoundtripEncoding \"UTF-32, utf-16\" &&\n-\tGIT_TRACE=1 git add roundtrip.utf16 2>&1 >/dev/null |\n+\tGIT_TRACE=1 git -c core.checkRoundtripEncoding=\"UTF-32, utf-16\" \\\n+\t\tadd roundtrip.utf16 2>&1 |\n \t\tgrep \"Checking roundtrip encoding for UTF-16\" &&\n-\tgit reset &&\n-\n-\t# cleanup\n-\trm roundtrip.shift roundtrip.utf16 &&\n-\tgit reset --hard HEAD\n+\tgit reset\n '\n\n test_done\ndiff --git a/utf8.h b/utf8.h\nindex 62f86fba64..cce654a64a 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -71,9 +71,9 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid\n \t\t const char *s);\n\n /*\n- * Whenever a data stream is declared to be UTF-16BE, UTF-16LE, UTF-32BE\n- * or UTF-32LE a BOM must not be used [1]. The function returns true if\n- * this is the case.\n+ * If a data stream is declared as UTF-16BE or UTF-16LE, then a UTF-16\n+ * BOM must not be used [1]. The same applies for the UTF-32 equivalents.\n+ * The function returns true if this rule is violated.\n *\n * [1] http://unicode.org/faq/utf_bom.html#bom10\n */\n@@ -83,10 +83,13 @@ int has_prohibited_utf_bom(const char *enc, const char *data, size_t len);\n * If the endianness is not defined in the encoding name, then we\n * require a BOM. The function returns true if a required BOM is missing.\n *\n- * The Unicode standard instructs to assume big-endian if there\n- * in no BOM for UTF-16/32 [1][2]. However, the W3C/WHATWG\n- * encoding standard used in HTML5 recommends to assume\n- * little-endian to \"deal with deployed content\" [3].\n+ * The Unicode standard instructs to assume big-endian if there in no\n+ * BOM for UTF-16/32 [1][2]. However, the W3C/WHATWG encoding standard\n+ * used in HTML5 recommends to assume little-endian to \"deal with\n+ * deployed content\" [3].\n+ *\n+ * Therefore, strictly requiring a BOM seems to be the safest option for\n+ * content in Git.\n *\n * [1] http://unicode.org/faq/utf_bom.html#gen6\n * [2] http://www.unicode.org/versions/Unicode10.0.0/ch03.pdf\n\n\n### Patches\n\nLars Schneider (8):\n strbuf: remove unnecessary NUL assignment in xstrdup_tolower()\n strbuf: add xstrdup_toupper()\n utf8: add function to detect prohibited UTF-16/32 BOM\n utf8: add function to detect a missing UTF-16/32 BOM\n convert: add 'working-tree-encoding' attribute\n convert: check for detectable errors in UTF encodings\n convert: add tracing for 'working-tree-encoding' attribute\n convert: add round trip check based on 'core.checkRoundtripEncoding'\n\n Documentation/config.txt | 6 +\n Documentation/gitattributes.txt | 88 +++++++++++++\n config.c | 5 +\n convert.c | 267 ++++++++++++++++++++++++++++++++++++++-\n convert.h | 2 +\n environment.c | 1 +\n sha1_file.c | 2 +-\n strbuf.c | 13 +-\n strbuf.h | 1 +\n t/t0028-working-tree-encoding.sh | 233 ++++++++++++++++++++++++++++++++++\n utf8.c | 37 ++++++\n utf8.h | 28 ++++\n 12 files changed, 680 insertions(+), 3 deletions(-)\n create mode 100755 t/t0028-working-tree-encoding.sh\n\n\nbase-commit: 8a2f0888555ce46ac87452b194dec5cb66fb1417\n--\n2.16.2\n\n"},{"id":"340985","messageId":"20180304201418.60958-2-lars.schneider@autodesk.com","threadId":"47977","inReplyTo":"20180304201418.60958-1-lars.schneider@autodesk.com","subject":"[PATCH v9 1/8] strbuf: remove unnecessary NUL assignment in xstrdup_tolower()","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-04T20:14:11Z","receivedAt":"2018-03-04T20:15:55Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nSince 3733e69464 (use xmallocz to avoid size arithmetic, 2016-02-22) we\nallocate the buffer for the lower case string with xmallocz(). This\nalready ensures a NUL at the end of the allocated buffer.\n\nRemove the unnecessary assignment.\n\nSigned-off-by: Lars Schneider \n---\n strbuf.c | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 1df674e919..55b7daeb35 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -781,7 +781,6 @@ char *xstrdup_tolower(const char *string)\n \tresult = xmallocz(len);\n \tfor (i = 0; i < len; i++)\n \t\tresult[i] = tolower(string[i]);\n-\tresult[i] = '\\0';\n \treturn result;\n }\n \n-- \n2.16.2\n\n"},{"id":"340986","messageId":"20180304201418.60958-4-lars.schneider@autodesk.com","threadId":"47977","inReplyTo":"20180304201418.60958-1-lars.schneider@autodesk.com","subject":"[PATCH v9 3/8] utf8: add function to detect prohibited UTF-16/32 BOM","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-04T20:14:13Z","receivedAt":"2018-03-04T20:15:56Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nWhenever a data stream is declared to be UTF-16BE, UTF-16LE, UTF-32BE\nor UTF-32LE a BOM must not be used [1]. The function returns true if\nthis is the case.\n\nThis function is used in a subsequent commit.\n\n[1] http://unicode.org/faq/utf_bom.html#bom10\n\nSigned-off-by: Lars Schneider \n---\n utf8.c | 24 ++++++++++++++++++++++++\n utf8.h | 9 +++++++++\n 2 files changed, 33 insertions(+)\n\ndiff --git a/utf8.c b/utf8.c\nindex 2c27ce0137..914881cd1f 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -538,6 +538,30 @@ char *reencode_string_len(const char *in, int insz,\n }\n #endif\n \n+static int has_bom_prefix(const char *data, size_t len,\n+\t\t\t const char *bom, size_t bom_len)\n+{\n+\treturn (len >= bom_len) && !memcmp(data, bom, bom_len);\n+}\n+\n+static const char utf16_be_bom[] = {0xFE, 0xFF};\n+static const char utf16_le_bom[] = {0xFF, 0xFE};\n+static const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n+static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n+\n+int has_prohibited_utf_bom(const char *enc, const char *data, size_t len)\n+{\n+\treturn (\n+\t (!strcmp(enc, \"UTF-16BE\") || !strcmp(enc, \"UTF-16LE\")) &&\n+\t (has_bom_prefix(data, len, utf16_be_bom, sizeof(utf16_be_bom)) ||\n+\t has_bom_prefix(data, len, utf16_le_bom, sizeof(utf16_le_bom)))\n+\t) || (\n+\t (!strcmp(enc, \"UTF-32BE\") || !strcmp(enc, \"UTF-32LE\")) &&\n+\t (has_bom_prefix(data, len, utf32_be_bom, sizeof(utf32_be_bom)) ||\n+\t has_bom_prefix(data, len, utf32_le_bom, sizeof(utf32_le_bom)))\n+\t);\n+}\n+\n /*\n * Returns first character length in bytes for multi-byte `text` according to\n * `encoding`.\ndiff --git a/utf8.h b/utf8.h\nindex 6bbcf31a83..0db1db4519 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -70,4 +70,13 @@ typedef enum {\n void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int width,\n \t\t const char *s);\n \n+/*\n+ * If a data stream is declared as UTF-16BE or UTF-16LE, then a UTF-16\n+ * BOM must not be used [1]. The same applies for the UTF-32 equivalents.\n+ * The function returns true if this rule is violated.\n+ *\n+ * [1] http://unicode.org/faq/utf_bom.html#bom10\n+ */\n+int has_prohibited_utf_bom(const char *enc, const char *data, size_t len);\n+\n #endif\n-- \n2.16.2\n\n"},{"id":"340987","messageId":"20180304201418.60958-3-lars.schneider@autodesk.com","threadId":"47977","inReplyTo":"20180304201418.60958-1-lars.schneider@autodesk.com","subject":"[PATCH v9 2/8] strbuf: add xstrdup_toupper()","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-04T20:14:12Z","receivedAt":"2018-03-04T20:15:58Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nCreate a copy of an existing string and make all characters upper case.\nSimilar xstrdup_tolower().\n\nThis function is used in a subsequent commit.\n\nSigned-off-by: Lars Schneider \n---\n strbuf.c | 12 ++++++++++++\n strbuf.h | 1 +\n 2 files changed, 13 insertions(+)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 55b7daeb35..b635f0bdc4 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -784,6 +784,18 @@ char *xstrdup_tolower(const char *string)\n \treturn result;\n }\n \n+char *xstrdup_toupper(const char *string)\n+{\n+\tchar *result;\n+\tsize_t len, i;\n+\n+\tlen = strlen(string);\n+\tresult = xmallocz(len);\n+\tfor (i = 0; i < len; i++)\n+\t\tresult[i] = toupper(string[i]);\n+\treturn result;\n+}\n+\n char *xstrvfmt(const char *fmt, va_list ap)\n {\n \tstruct strbuf buf = STRBUF_INIT;\ndiff --git a/strbuf.h b/strbuf.h\nindex 14c8c10d66..df7ced53ed 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -607,6 +607,7 @@ __attribute__((format (printf,2,3)))\n extern int fprintf_ln(FILE *fp, const char *fmt, ...);\n \n char *xstrdup_tolower(const char *);\n+char *xstrdup_toupper(const char *);\n \n /**\n * Create a newly allocated string using printf format. You can do this easily\n-- \n2.16.2\n\n"},{"id":"340988","messageId":"20180304201418.60958-5-lars.schneider@autodesk.com","threadId":"47977","inReplyTo":"20180304201418.60958-1-lars.schneider@autodesk.com","subject":"[PATCH v9 4/8] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-04T20:14:14Z","receivedAt":"2018-03-04T20:16:01Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nIf the endianness is not defined in the encoding name, then let's\nbe strict and require a BOM to avoid any encoding confusion. The\nis_missing_required_utf_bom() function returns true if a required BOM\nis missing.\n\nThe Unicode standard instructs to assume big-endian if there in no BOM\nfor UTF-16/32 [1][2]. However, the W3C/WHATWG encoding standard used\nin HTML5 recommends to assume little-endian to \"deal with deployed\ncontent\" [3]. Strictly requiring a BOM seems to be the safest option\nfor content in Git.\n\nThis function is used in a subsequent commit.\n\n[1] http://unicode.org/faq/utf_bom.html#gen6\n[2] http://www.unicode.org/versions/Unicode10.0.0/ch03.pdf\n Section 3.10, D98, page 132\n[3] https://encoding.spec.whatwg.org/#utf-16le\n\nSigned-off-by: Lars Schneider \n---\n utf8.c | 13 +++++++++++++\n utf8.h | 19 +++++++++++++++++++\n 2 files changed, 32 insertions(+)\n\ndiff --git a/utf8.c b/utf8.c\nindex 914881cd1f..5113d26e56 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -562,6 +562,19 @@ int has_prohibited_utf_bom(const char *enc, const char *data, size_t len)\n \t);\n }\n \n+int is_missing_required_utf_bom(const char *enc, const char *data, size_t len)\n+{\n+\treturn (\n+\t !strcmp(enc, \"UTF-16\") &&\n+\t !(has_bom_prefix(data, len, utf16_be_bom, sizeof(utf16_be_bom)) ||\n+\t has_bom_prefix(data, len, utf16_le_bom, sizeof(utf16_le_bom)))\n+\t) || (\n+\t !strcmp(enc, \"UTF-32\") &&\n+\t !(has_bom_prefix(data, len, utf32_be_bom, sizeof(utf32_be_bom)) ||\n+\t has_bom_prefix(data, len, utf32_le_bom, sizeof(utf32_le_bom)))\n+\t);\n+}\n+\n /*\n * Returns first character length in bytes for multi-byte `text` according to\n * `encoding`.\ndiff --git a/utf8.h b/utf8.h\nindex 0db1db4519..cce654a64a 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -79,4 +79,23 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid\n */\n int has_prohibited_utf_bom(const char *enc, const char *data, size_t len);\n \n+/*\n+ * If the endianness is not defined in the encoding name, then we\n+ * require a BOM. The function returns true if a required BOM is missing.\n+ *\n+ * The Unicode standard instructs to assume big-endian if there in no\n+ * BOM for UTF-16/32 [1][2]. However, the W3C/WHATWG encoding standard\n+ * used in HTML5 recommends to assume little-endian to \"deal with\n+ * deployed content\" [3].\n+ *\n+ * Therefore, strictly requiring a BOM seems to be the safest option for\n+ * content in Git.\n+ *\n+ * [1] http://unicode.org/faq/utf_bom.html#gen6\n+ * [2] http://www.unicode.org/versions/Unicode10.0.0/ch03.pdf\n+ * Section 3.10, D98, page 132\n+ * [3] https://encoding.spec.whatwg.org/#utf-16le\n+ */\n+int is_missing_required_utf_bom(const char *enc, const char *data, size_t len);\n+\n #endif\n-- \n2.16.2\n\n"},{"id":"340989","messageId":"20180304201418.60958-6-lars.schneider@autodesk.com","threadId":"47977","inReplyTo":"20180304201418.60958-1-lars.schneider@autodesk.com","subject":"[PATCH v9 5/8] convert: add 'working-tree-encoding' attribute","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-04T20:14:15Z","receivedAt":"2018-03-04T20:16:05Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nGit recognizes files encoded with ASCII or one of its supersets (e.g.\nUTF-8 or ISO-8859-1) as text files. All other encodings are usually\ninterpreted as binary and consequently built-in Git text processing\ntools (e.g. 'git diff') as well as most Git web front ends do not\nvisualize the content.\n\nAdd an attribute to tell Git what encoding the user has defined for a\ngiven file. If the content is added to the index, then Git converts the\ncontent to a canonical UTF-8 representation. On checkout Git will\nreverse the conversion.\n\nSigned-off-by: Lars Schneider \n---\n Documentation/gitattributes.txt | 80 +++++++++++++++++++++++\n convert.c | 114 ++++++++++++++++++++++++++++++++-\n convert.h | 1 +\n sha1_file.c | 2 +-\n t/t0028-working-tree-encoding.sh | 135 +++++++++++++++++++++++++++++++++++++++\n 5 files changed, 330 insertions(+), 2 deletions(-)\n create mode 100755 t/t0028-working-tree-encoding.sh\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 30687de81a..31a4f92840 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -272,6 +272,86 @@ few exceptions. Even though...\n catch potential problems early, safety triggers.\n \n \n+`working-tree-encoding`\n+^^^^^^^^^^^^^^^^^^^^^^^\n+\n+Git recognizes files encoded in ASCII or one of its supersets (e.g.\n+UTF-8, ISO-8859-1, ...) as text files. Files encoded in certain other\n+encodings (e.g. UTF-16) are interpreted as binary and consequently\n+built-in Git text processing tools (e.g. 'git diff') as well as most Git\n+web front ends do not visualize the contents of these files by default.\n+\n+In these cases you can tell Git the encoding of a file in the working\n+directory with the `working-tree-encoding` attribute. If a file with this\n+attribute is added to Git, then Git reencodes the content from the\n+specified encoding to UTF-8. Finally, Git stores the UTF-8 encoded\n+content in its internal data structure (called \"the index\"). On checkout\n+the content is reencoded back to the specified encoding.\n+\n+Please note that using the `working-tree-encoding` attribute may have a\n+number of pitfalls:\n+\n+- Alternative Git implementations (e.g. JGit or libgit2) and older Git\n+ versions (as of March 2018) do not support the `working-tree-encoding`\n+ attribute. If you decide to use the `working-tree-encoding` attribute\n+ in your repository, then it is strongly recommended to ensure that all\n+ clients working with the repository support it.\n+\n+ For example, Microsoft Visual Studio resources files (`*.rc`) or\n+ PowerShell script files (`*.ps1`) are sometimes encoded in UTF-16.\n+ If you declare `*.ps1` as files as UTF-16 and you add `foo.ps1` with\n+ a `working-tree-encoding` enabled Git client, then `foo.ps1` will be\n+ stored as UTF-8 internally. A client without `working-tree-encoding`\n+ support will checkout `foo.ps1` as UTF-8 encoded file. This will\n+ typically cause trouble for the users of this file.\n+\n+ If a Git client, that does not support the `working-tree-encoding`\n+ attribute, adds a new file `bar.ps1`, then `bar.ps1` will be\n+ stored \"as-is\" internally (in this example probably as UTF-16).\n+ A client with `working-tree-encoding` support will interpret the\n+ internal contents as UTF-8 and try to convert it to UTF-16 on checkout.\n+ That operation will fail and cause an error.\n+\n+- Reencoding content requires resources that might slow down certain\n+ Git operations (e.g 'git checkout' or 'git add').\n+\n+Use the `working-tree-encoding` attribute only if you cannot store a file\n+in UTF-8 encoding and if you want Git to be able to process the content\n+as text.\n+\n+As an example, use the following attributes if your '*.ps1' files are\n+UTF-16 encoded with byte order mark (BOM) and you want Git to perform\n+automatic line ending conversion based on your platform.\n+\n+------------------------\n+*.ps1\t\ttext working-tree-encoding=UTF-16\n+------------------------\n+\n+Use the following attributes if your '*.ps1' files are UTF-16 little\n+endian encoded without BOM and you want Git to use Windows line endings\n+in the working directory. Please note, it is highly recommended to\n+explicitly define the line endings with `eol` if the `working-tree-encoding`\n+attribute is used to avoid ambiguity.\n+\n+------------------------\n+*.ps1\t\ttext working-tree-encoding=UTF-16LE eol=CRLF\n+------------------------\n+\n+You can get a list of all available encodings on your platform with the\n+following command:\n+\n+------------------------\n+iconv --list\n+------------------------\n+\n+If you do not know the encoding of a file, then you can use the `file`\n+command to guess the encoding:\n+\n+------------------------\n+file foo.ps1\n+------------------------\n+\n+\n `ident`\n ^^^^^^^\n \ndiff --git a/convert.c b/convert.c\nindex b976eb968c..2f864df258 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -7,6 +7,7 @@\n #include \"sigchain.h\"\n #include \"pkt-line.h\"\n #include \"sub-process.h\"\n+#include \"utf8.h\"\n \n /*\n * convert.c - convert a file when checking it out and checking it in.\n@@ -265,6 +266,78 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n \n }\n \n+static const char *default_encoding = \"UTF-8\";\n+\n+static int encode_to_git(const char *path, const char *src, size_t src_len,\n+\t\t\t struct strbuf *buf, const char *enc, int conv_flags)\n+{\n+\tchar *dst;\n+\tint dst_len;\n+\tint die_on_error = conv_flags & CONV_WRITE_OBJECT;\n+\n+\t/*\n+\t * No encoding is specified or there is nothing to encode.\n+\t * Tell the caller that the content was not modified.\n+\t */\n+\tif (!enc || (src && !src_len))\n+\t\treturn 0;\n+\n+\t/*\n+\t * Looks like we got called from \"would_convert_to_git()\".\n+\t * This means Git wants to know if it would encode (= modify!)\n+\t * the content. Let's answer with \"yes\", since an encoding was\n+\t * specified.\n+\t */\n+\tif (!buf && !src)\n+\t\treturn 1;\n+\n+\tdst = reencode_string_len(src, src_len, default_encoding, enc,\n+\t\t\t\t &dst_len);\n+\tif (!dst) {\n+\t\t/*\n+\t\t * We could add the blob \"as-is\" to Git. However, on checkout\n+\t\t * we would try to reencode to the original encoding. This\n+\t\t * would fail and we would leave the user with a messed-up\n+\t\t * working tree. Let's try to avoid this by screaming loud.\n+\t\t */\n+\t\tconst char* msg = _(\"failed to encode '%s' from %s to %s\");\n+\t\tif (die_on_error)\n+\t\t\tdie(msg, path, enc, default_encoding);\n+\t\telse {\n+\t\t\terror(msg, path, enc, default_encoding);\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\n+\tstrbuf_attach(buf, dst, dst_len, dst_len + 1);\n+\treturn 1;\n+}\n+\n+static int encode_to_worktree(const char *path, const char *src, size_t src_len,\n+\t\t\t struct strbuf *buf, const char *enc)\n+{\n+\tchar *dst;\n+\tint dst_len;\n+\n+\t/*\n+\t * No encoding is specified or there is nothing to encode.\n+\t * Tell the caller that the content was not modified.\n+\t */\n+\tif (!enc || (src && !src_len))\n+\t\treturn 0;\n+\n+\tdst = reencode_string_len(src, src_len, enc, default_encoding,\n+\t\t\t\t &dst_len);\n+\tif (!dst) {\n+\t\terror(\"failed to encode '%s' from %s to %s\",\n+\t\t\tpath, default_encoding, enc);\n+\t\treturn 0;\n+\t}\n+\n+\tstrbuf_attach(buf, dst, dst_len, dst_len + 1);\n+\treturn 1;\n+}\n+\n static int crlf_to_git(const struct index_state *istate,\n \t\t const char *path, const char *src, size_t len,\n \t\t struct strbuf *buf,\n@@ -978,6 +1051,25 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,\n \treturn 1;\n }\n \n+static const char *git_path_check_encoding(struct attr_check_item *check)\n+{\n+\tconst char *value = check->value;\n+\n+\tif (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value) ||\n+\t !strlen(value))\n+\t\treturn NULL;\n+\n+\t/* Don't encode to the default encoding */\n+\tif (!strcasecmp(value, default_encoding))\n+\t\treturn NULL;\n+\n+\t/*\n+\t * Ensure encoding names are always upper case (e.g. UTF-8) to\n+\t * simplify subsequent string comparisons.\n+\t */\n+\treturn xstrdup_toupper(value);\n+}\n+\n static enum crlf_action git_path_check_crlf(struct attr_check_item *check)\n {\n \tconst char *value = check->value;\n@@ -1033,6 +1125,7 @@ struct conv_attrs {\n \tenum crlf_action attr_action; /* What attr says */\n \tenum crlf_action crlf_action; /* When no attr is set, use core.autocrlf */\n \tint ident;\n+\tconst char *working_tree_encoding; /* Supported encoding or default encoding if NULL */\n };\n \n static void convert_attrs(struct conv_attrs *ca, const char *path)\n@@ -1041,7 +1134,8 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)\n \n \tif (!check) {\n \t\tcheck = attr_check_initl(\"crlf\", \"ident\", \"filter\",\n-\t\t\t\t\t \"eol\", \"text\", NULL);\n+\t\t\t\t\t \"eol\", \"text\", \"working-tree-encoding\",\n+\t\t\t\t\t NULL);\n \t\tuser_convert_tail = &user_convert;\n \t\tgit_config(read_convert_config, NULL);\n \t}\n@@ -1064,6 +1158,7 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)\n \t\t\telse if (eol_attr == EOL_CRLF)\n \t\t\t\tca->crlf_action = CRLF_TEXT_CRLF;\n \t\t}\n+\t\tca->working_tree_encoding = git_path_check_encoding(ccheck + 5);\n \t} else {\n \t\tca->drv = NULL;\n \t\tca->crlf_action = CRLF_UNDEFINED;\n@@ -1144,6 +1239,13 @@ int convert_to_git(const struct index_state *istate,\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n+\n+\tret |= encode_to_git(path, src, len, dst, ca.working_tree_encoding, conv_flags);\n+\tif (ret && dst) {\n+\t\tsrc = dst->buf;\n+\t\tlen = dst->len;\n+\t}\n+\n \tif (!(conv_flags & CONV_EOL_KEEP_CRLF)) {\n \t\tret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, conv_flags);\n \t\tif (ret && dst) {\n@@ -1167,6 +1269,7 @@ void convert_to_git_filter_fd(const struct index_state *istate,\n \tif (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN, NULL))\n \t\tdie(\"%s: clean filter '%s' failed\", path, ca.drv->name);\n \n+\tencode_to_git(path, dst->buf, dst->len, dst, ca.working_tree_encoding, conv_flags);\n \tcrlf_to_git(istate, path, dst->buf, dst->len, dst, ca.crlf_action, conv_flags);\n \tident_to_git(path, dst->buf, dst->len, dst, ca.ident);\n }\n@@ -1198,6 +1301,12 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t\t}\n \t}\n \n+\tret |= encode_to_worktree(path, src, len, dst, ca.working_tree_encoding);\n+\tif (ret) {\n+\t\tsrc = dst->buf;\n+\t\tlen = dst->len;\n+\t}\n+\n \tret_filter = apply_filter(\n \t\tpath, src, len, -1, dst, ca.drv, CAP_SMUDGE, dco);\n \tif (!ret_filter && ca.drv && ca.drv->required)\n@@ -1664,6 +1773,9 @@ struct stream_filter *get_stream_filter(const char *path, const unsigned char *s\n \tif (ca.drv && (ca.drv->process || ca.drv->smudge || ca.drv->clean))\n \t\treturn NULL;\n \n+\tif (ca.working_tree_encoding)\n+\t\treturn NULL;\n+\n \tif (ca.crlf_action == CRLF_AUTO || ca.crlf_action == CRLF_AUTO_CRLF)\n \t\treturn NULL;\n \ndiff --git a/convert.h b/convert.h\nindex 65ab3e5167..1d9539ed0b 100644\n--- a/convert.h\n+++ b/convert.h\n@@ -12,6 +12,7 @@ struct index_state;\n #define CONV_EOL_RNDTRP_WARN (1<<1) /* Warn if CRLF to LF to CRLF is different */\n #define CONV_EOL_RENORMALIZE (1<<2) /* Convert CRLF to LF */\n #define CONV_EOL_KEEP_CRLF (1<<3) /* Keep CRLF line endings as is */\n+#define CONV_WRITE_OBJECT (1<<4) /* Content is written to the index */\n \n extern int global_conv_flags_eol;\n \ndiff --git a/sha1_file.c b/sha1_file.c\nindex 6bc7c6ada9..e2f319d677 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -138,7 +138,7 @@ static int get_conv_flags(unsigned flags)\n \tif (flags & HASH_RENORMALIZE)\n \t\treturn CONV_EOL_RENORMALIZE;\n \telse if (flags & HASH_WRITE_OBJECT)\n-\t return global_conv_flags_eol;\n+\t\treturn global_conv_flags_eol | CONV_WRITE_OBJECT;\n \telse\n \t\treturn 0;\n }\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nnew file mode 100755\nindex 0000000000..71e8e3700b\n--- /dev/null\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -0,0 +1,135 @@\n+#!/bin/sh\n+\n+test_description='working-tree-encoding conversion via gitattributes'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup test files' '\n+\tgit config core.eol lf &&\n+\n+\ttext=\"hallo there!\\ncan you read me?\" &&\n+\techo \"*.utf16 text working-tree-encoding=utf-16\" >.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+\n+\t# Line ending tests\n+\tprintf \"one\\ntwo\\nthree\\n\" >lf.utf8.raw &&\n+\tprintf \"one\\r\\ntwo\\r\\nthree\\r\\n\" >crlf.utf8.raw &&\n+\n+\t# BOM tests\n+\tprintf \"\\0a\\0b\\0c\" >nobom.utf16be.raw &&\n+\tprintf \"a\\0b\\0c\\0\" >nobom.utf16le.raw &&\n+\tprintf \"\\376\\777\\0a\\0b\\0c\" >bebom.utf16be.raw &&\n+\tprintf \"\\777\\376a\\0b\\0c\\0\" >lebom.utf16le.raw &&\n+\tprintf \"\\0\\0\\0a\\0\\0\\0b\\0\\0\\0c\" >nobom.utf32be.raw &&\n+\tprintf \"a\\0\\0\\0b\\0\\0\\0c\\0\\0\\0\" >nobom.utf32le.raw &&\n+\tprintf \"\\0\\0\\376\\777\\0\\0\\0a\\0\\0\\0b\\0\\0\\0c\" >bebom.utf32be.raw &&\n+\tprintf \"\\777\\376\\0\\0a\\0\\0\\0b\\0\\0\\0c\\0\\0\\0\" >lebom.utf32le.raw &&\n+\n+\t# Add only UTF-16 file, we will add the UTF-32 file later\n+\tcp test.utf16.raw test.utf16 &&\n+\tcp test.utf32.raw test.utf32 &&\n+\tgit add .gitattributes test.utf16 &&\n+\tgit commit -m initial\n+'\n+\n+test_expect_success 'ensure UTF-8 is stored in Git' '\n+\ttest_when_finished \"rm -f test.utf16.git\" &&\n+\n+\tgit cat-file -p :test.utf16 >test.utf16.git &&\n+\ttest_cmp_bin test.utf8.raw test.utf16.git\n+'\n+\n+test_expect_success 're-encode to UTF-16 on checkout' '\n+\ttest_when_finished \"rm -f test.utf16.raw\" &&\n+\n+\trm test.utf16 &&\n+\tgit checkout test.utf16 &&\n+\ttest_cmp_bin test.utf16.raw test.utf16\n+'\n+\n+test_expect_success 'check $GIT_DIR/info/attributes support' '\n+\ttest_when_finished \"rm -f test.utf8.raw test.utf32.raw test.utf32.git\" &&\n+\ttest_when_finished \"git reset --hard HEAD\" &&\n+\n+\techo \"*.utf32 text working-tree-encoding=utf-32\" >.git/info/attributes &&\n+\tgit add test.utf32 &&\n+\n+\tgit cat-file -p :test.utf32 >test.utf32.git &&\n+\ttest_cmp_bin test.utf8.raw test.utf32.git\n+'\n+\n+for i in 16 32\n+do\n+\ttest_expect_success \"eol conversion for UTF-${i} encoded files on checkout\" '\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\tcp crlf.utf${i}.raw eol.utf${i} &&\n+\n+\t\tcat >expectIndexLF <<-EOF &&\n+\t\t\ti/lf w/-text attr/text \teol.utf${i}\n+\t\tEOF\n+\n+\t\tgit add eol.utf${i} &&\n+\t\tgit commit -m eol &&\n+\n+\t\t# UTF-${i} with CRLF (Windows line endings)\n+\t\trm eol.utf${i} &&\n+\t\tgit -c core.eol=crlf checkout eol.utf${i} &&\n+\t\ttest_cmp_bin crlf.utf${i}.raw eol.utf${i} &&\n+\n+\t\t# Although the file has CRLF in the working tree,\n+\t\t# ensure LF in the index\n+\t\tgit ls-files --eol eol.utf${i} >actual &&\n+\t\ttest_cmp expectIndexLF actual &&\n+\n+\t\t# UTF-${i} with LF (Unix line endings)\n+\t\trm eol.utf${i} &&\n+\t\tgit -c core.eol=lf checkout eol.utf${i} &&\n+\t\ttest_cmp_bin lf.utf${i}.raw eol.utf${i} &&\n+\n+\t\t# The file LF in the working tree, ensure LF in the index\n+\t\tgit ls-files --eol eol.utf${i} >actual &&\n+\t\ttest_cmp expectIndexLF actual\n+\t'\n+done\n+\n+test_expect_success 'check unsupported encodings' '\n+\ttest_when_finished \"rm -f err.out\" &&\n+\ttest_when_finished \"git reset --hard HEAD\" &&\n+\n+\techo \"*.nothing text working-tree-encoding=\" >>.gitattributes &&\n+\tprintf \"nothing\" >t.nothing &&\n+\tgit add t.nothing &&\n+\n+\techo \"*.garbage text working-tree-encoding=garbage\" >>.gitattributes &&\n+\tprintf \"garbage\" >t.garbage &&\n+\ttest_must_fail git add t.garbage 2>err.out &&\n+\ttest_i18ngrep \"fatal: failed to encode\" err.out\n+'\n+\n+test_expect_success 'error if encoding round trip is not the same during refresh' '\n+\tBEFORE_STATE=$(git rev-parse HEAD) &&\n+\ttest_when_finished \"rm -f err.out\" &&\n+\ttest_when_finished \"git reset --hard $BEFORE_STATE\" &&\n+\n+\t# Add and commit a UTF-16 file but skip the \"working-tree-encoding\"\n+\t# filter. Consequently, the in-repo representation is UTF-16 and not\n+\t# UTF-8. This simulates a Git version that has no working tree encoding\n+\t# support.\n+\techo \"*.utf16le text working-tree-encoding=utf-16le\" >.gitattributes &&\n+\techo \"hallo\" >nonsense.utf16le &&\n+\tTEST_HASH=$(git hash-object --no-filters -w nonsense.utf16le) &&\n+\tgit update-index --add --cacheinfo 100644 $TEST_HASH nonsense.utf16le &&\n+\tCOMMIT=$(git commit-tree -p $(git rev-parse HEAD) -m \"plain commit\" $(git write-tree)) &&\n+\tgit update-ref refs/heads/master $COMMIT &&\n+\n+\ttest_must_fail git checkout HEAD^ 2>err.out &&\n+\ttest_i18ngrep \"error: .* overwritten by checkout:\" err.out\n+'\n+\n+test_done\n-- \n2.16.2\n\n"},{"id":"340990","messageId":"20180304201418.60958-9-lars.schneider@autodesk.com","threadId":"47977","inReplyTo":"20180304201418.60958-1-lars.schneider@autodesk.com","subject":"[PATCH v9 8/8] convert: add round trip check based on 'core.checkRoundtripEncoding'","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-04T20:14:18Z","receivedAt":"2018-03-04T20:16:08Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nUTF supports lossless conversion round tripping and conversions between\nUTF and other encodings are mostly round trip safe as Unicode aims to be\na superset of all other character encodings. However, certain encodings\n(e.g. SHIFT-JIS) are known to have round trip issues [1].\n\nAdd 'core.checkRoundtripEncoding', which contains a comma separated\nlist of encodings, to define for what encodings Git should check the\nconversion round trip if they are used in the 'working-tree-encoding'\nattribute.\n\nSet SHIFT-JIS as default value for 'core.checkRoundtripEncoding'.\n\n[1] https://support.microsoft.com/en-us/help/170559/prb-conversion-problem-between-shift-jis-and-unicode\n\nSigned-off-by: Lars Schneider \n---\n Documentation/config.txt | 6 ++++\n Documentation/gitattributes.txt | 8 +++++\n config.c | 5 +++\n convert.c | 78 ++++++++++++++++++++++++++++++++++++++++\n convert.h | 1 +\n environment.c | 1 +\n t/t0028-working-tree-encoding.sh | 39 ++++++++++++++++++++\n 7 files changed, 138 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 0e25b2c92b..d7a56054a5 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -530,6 +530,12 @@ core.autocrlf::\n \tThis variable can be set to 'input',\n \tin which case no output conversion is performed.\n \n+core.checkRoundtripEncoding::\n+\tA comma separated list of encodings that Git performs UTF-8 round\n+\ttrip checks on if they are used in an `working-tree-encoding`\n+\tattribute (see linkgit:gitattributes[5]). The default value is\n+\t`SHIFT-JIS`.\n+\n core.symlinks::\n \tIf false, symbolic links are checked out as small plain files that\n \tcontain the link text. linkgit:git-update-index[1] and\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 31a4f92840..aa3deae392 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -312,6 +312,14 @@ number of pitfalls:\n internal contents as UTF-8 and try to convert it to UTF-16 on checkout.\n That operation will fail and cause an error.\n \n+- Reencoding content to non-UTF encodings can cause errors as the\n+ conversion might not be UTF-8 round trip safe. If you suspect your\n+ encoding to not be round trip safe, then add it to\n+ `core.checkRoundtripEncoding` to make Git check the round trip\n+ encoding (see linkgit:git-config[1]). SHIFT-JIS (Japanese character\n+ set) is known to have round trip issues with UTF-8 and is checked by\n+ default.\n+\n - Reencoding content requires resources that might slow down certain\n Git operations (e.g 'git checkout' or 'git add').\n \ndiff --git a/config.c b/config.c\nindex 1f003fbb90..d0ada9fcd4 100644\n--- a/config.c\n+++ b/config.c\n@@ -1172,6 +1172,11 @@ static int git_default_core_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.checkroundtripencoding\")) {\n+\t\tcheck_roundtrip_encoding = xstrdup(value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"core.notesref\")) {\n \t\tnotes_ref_name = xstrdup(value);\n \t\treturn 0;\ndiff --git a/convert.c b/convert.c\nindex eec34a94b9..6cbb2b2618 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -336,6 +336,43 @@ static void trace_encoding(const char *context, const char *path,\n \tstrbuf_release(&trace);\n }\n \n+static int check_roundtrip(const char* enc_name)\n+{\n+\t/*\n+\t * check_roundtrip_encoding contains a string of space and/or\n+\t * comma separated encodings (eg. \"UTF-16, ASCII, CP1125\").\n+\t * Search for the given encoding in that string.\n+\t */\n+\tconst char *found = strcasestr(check_roundtrip_encoding, enc_name);\n+\tconst char *next;\n+\tint len;\n+\tif (!found)\n+\t\treturn 0;\n+\tnext = found + strlen(enc_name);\n+\tlen = strlen(check_roundtrip_encoding);\n+\treturn (found && (\n+\t\t\t/*\n+\t\t\t * check that the found encoding is at the\n+\t\t\t * beginning of check_roundtrip_encoding or\n+\t\t\t * that it is prefixed with a space or comma\n+\t\t\t */\n+\t\t\tfound == check_roundtrip_encoding || (\n+\t\t\t\tfound > check_roundtrip_encoding &&\n+\t\t\t\t(*(found-1) == ' ' || *(found-1) == ',')\n+\t\t\t)\n+\t\t) && (\n+\t\t\t/*\n+\t\t\t * check that the found encoding is at the\n+\t\t\t * end of check_roundtrip_encoding or\n+\t\t\t * that it is suffixed with a space or comma\n+\t\t\t */\n+\t\t\tnext == check_roundtrip_encoding + len || (\n+\t\t\t\tnext < check_roundtrip_encoding + len &&\n+\t\t\t\t(*next == ' ' || *next == ',')\n+\t\t\t)\n+\t\t));\n+}\n+\n static const char *default_encoding = \"UTF-8\";\n \n static int encode_to_git(const char *path, const char *src, size_t src_len,\n@@ -384,6 +421,47 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \t}\n \ttrace_encoding(\"destination\", path, default_encoding, dst, dst_len);\n \n+\t/*\n+\t * UTF supports lossless conversion round tripping [1] and conversions\n+\t * between UTF and other encodings are mostly round trip safe as\n+\t * Unicode aims to be a superset of all other character encodings.\n+\t * However, certain encodings (e.g. SHIFT-JIS) are known to have round\n+\t * trip issues [2]. Check the round trip conversion for all encodings\n+\t * listed in core.checkRoundtripEncoding.\n+\t *\n+\t * The round trip check is only performed if content is written to Git.\n+\t * This ensures that no information is lost during conversion to/from\n+\t * the internal UTF-8 representation.\n+\t *\n+\t * Please note, the code below is not tested because I was not able to\n+\t * generate a faulty round trip without an iconv error. Iconv errors\n+\t * are already caught above.\n+\t *\n+\t * [1] http://unicode.org/faq/utf_bom.html#gen2\n+\t * [2] https://support.microsoft.com/en-us/help/170559/prb-conversion-problem-between-shift-jis-and-unicode\n+\t */\n+\tif (die_on_error && check_roundtrip(enc)) {\n+\t\tchar *re_src;\n+\t\tint re_src_len;\n+\n+\t\tre_src = reencode_string_len(dst, dst_len,\n+\t\t\t\t\t enc, default_encoding,\n+\t\t\t\t\t &re_src_len);\n+\n+\t\ttrace_printf(\"Checking roundtrip encoding for %s...\\n\", enc);\n+\t\ttrace_encoding(\"reencoded source\", path, enc,\n+\t\t\t re_src, re_src_len);\n+\n+\t\tif (!re_src || src_len != re_src_len ||\n+\t\t memcmp(src, re_src, src_len)) {\n+\t\t\tconst char* msg = _(\"encoding '%s' from %s to %s and \"\n+\t\t\t\t\t \"back is not the same\");\n+\t\t\tdie(msg, path, enc, default_encoding);\n+\t\t}\n+\n+\t\tfree(re_src);\n+\t}\n+\n \tstrbuf_attach(buf, dst, dst_len, dst_len + 1);\n \treturn 1;\n }\ndiff --git a/convert.h b/convert.h\nindex 1d9539ed0b..765abfbd60 100644\n--- a/convert.h\n+++ b/convert.h\n@@ -56,6 +56,7 @@ struct delayed_checkout {\n };\n \n extern enum eol core_eol;\n+extern char *check_roundtrip_encoding;\n extern const char *get_cached_convert_stats_ascii(const struct index_state *istate,\n \t\t\t\t\t\t const char *path);\n extern const char *get_wt_convert_stats_ascii(const char *path);\ndiff --git a/environment.c b/environment.c\nindex 10a32c20ac..5bae9131ad 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -50,6 +50,7 @@ int check_replace_refs = 1;\n char *git_replace_ref_base;\n enum eol core_eol = EOL_UNSET;\n int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;\n+char *check_roundtrip_encoding = \"SHIFT-JIS\";\n unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;\n enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;\n enum rebase_setup_type autorebase = AUTOREBASE_NEVER;\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex bdc487b44f..23e89ae623 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -191,4 +191,43 @@ test_expect_success 'error if encoding garbage is already in Git' '\n \ttest_i18ngrep \"error: BOM is required\" err.out\n '\n \n+test_expect_success 'check roundtrip encoding' '\n+\ttest_when_finished \"rm -f roundtrip.shift roundtrip.utf16\" &&\n+\ttest_when_finished \"git reset --hard HEAD\" &&\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+\techo \"*.shift text working-tree-encoding=SHIFT-JIS\" >>.gitattributes &&\n+\n+\t# SHIFT-JIS encoded files are round-trip checked by default...\n+\tGIT_TRACE=1 git add .gitattributes roundtrip.shift 2>&1 |\n+\t\tgrep \"Checking roundtrip encoding for SHIFT-JIS\" &&\n+\tgit reset &&\n+\n+\t# ... unless we overwrite the Git config!\n+\t! GIT_TRACE=1 git -c core.checkRoundtripEncoding=garbage \\\n+\t\tadd .gitattributes roundtrip.shift 2>&1 |\n+\t\tgrep \"Checking roundtrip encoding for SHIFT-JIS\" &&\n+\tgit reset &&\n+\n+\t# UTF-16 encoded files should not be round-trip checked by default...\n+\t! GIT_TRACE=1 git add roundtrip.utf16 2>&1 |\n+\t\tgrep \"Checking roundtrip encoding for UTF-16\" &&\n+\tgit reset &&\n+\n+\t# ... unless we tell Git to check it!\n+\tGIT_TRACE=1 git -c core.checkRoundtripEncoding=\"UTF-16, UTF-32\" \\\n+\t\tadd roundtrip.utf16 2>&1 |\n+\t\tgrep \"Checking roundtrip encoding for UTF-16\" &&\n+\tgit reset &&\n+\n+\t# ... unless we tell Git to check it!\n+\t# (here we also check that the casing of the encoding is irrelevant)\n+\tGIT_TRACE=1 git -c core.checkRoundtripEncoding=\"UTF-32, utf-16\" \\\n+\t\tadd roundtrip.utf16 2>&1 |\n+\t\tgrep \"Checking roundtrip encoding for UTF-16\" &&\n+\tgit reset\n+'\n+\n test_done\n-- \n2.16.2\n\n"},{"id":"340991","messageId":"20180304201418.60958-7-lars.schneider@autodesk.com","threadId":"47977","inReplyTo":"20180304201418.60958-1-lars.schneider@autodesk.com","subject":"[PATCH v9 6/8] convert: check for detectable errors in UTF encodings","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-04T20:14:16Z","receivedAt":"2018-03-04T20:16:11Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nCheck that new content is valid with respect to the user defined\n'working-tree-encoding' attribute.\n\nSigned-off-by: Lars Schneider \n---\n convert.c | 50 +++++++++++++++++++++++++++++++++++\n t/t0028-working-tree-encoding.sh | 57 ++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 107 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex 2f864df258..9647b06679 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -266,6 +266,53 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n \n }\n \n+static int validate_encoding(const char *path, const char *enc,\n+\t\t const char *data, size_t len, int die_on_error)\n+{\n+\tif (!memcmp(\"UTF-\", enc, 4)) {\n+\t\t/*\n+\t\t * Check for detectable errors in UTF encodings\n+\t\t */\n+\t\tif (has_prohibited_utf_bom(enc, data, len)) {\n+\t\t\tconst char *error_msg = _(\n+\t\t\t\t\"BOM is prohibited in '%s' if encoded as %s\");\n+\t\t\t/*\n+\t\t\t * This advice is shown for UTF-??BE and UTF-??LE\n+\t\t\t * encodings. We truncate the encoding name to 6\n+\t\t\t * chars with %.6s to cut off the last two \"byte\n+\t\t\t * order\" characters.\n+\t\t\t */\n+\t\t\tconst char *advise_msg = _(\n+\t\t\t\t\"The file '%s' contains a byte order \"\n+\t\t\t\t\"mark (BOM). Please use %.6s as \"\n+\t\t\t\t\"working-tree-encoding.\");\n+\t\t\tadvise(advise_msg, path, enc);\n+\t\t\tif (die_on_error)\n+\t\t\t\tdie(error_msg, path, enc);\n+\t\t\telse {\n+\t\t\t\treturn error(error_msg, path, enc);\n+\t\t\t}\n+\n+\t\t} else if (is_missing_required_utf_bom(enc, data, len)) {\n+\t\t\tconst char *error_msg = _(\n+\t\t\t\t\"BOM is required in '%s' if encoded as %s\");\n+\t\t\tconst char *advise_msg = _(\n+\t\t\t\t\"The file '%s' is missing a byte order \"\n+\t\t\t\t\"mark (BOM). Please use %sBE or %sLE \"\n+\t\t\t\t\"(depending on the byte order) as \"\n+\t\t\t\t\"working-tree-encoding.\");\n+\t\t\tadvise(advise_msg, path, enc, enc);\n+\t\t\tif (die_on_error)\n+\t\t\t\tdie(error_msg, path, enc);\n+\t\t\telse {\n+\t\t\t\treturn error(error_msg, path, enc);\n+\t\t\t}\n+\t\t}\n+\n+\t}\n+\treturn 0;\n+}\n+\n static const char *default_encoding = \"UTF-8\";\n \n static int encode_to_git(const char *path, const char *src, size_t src_len,\n@@ -291,6 +338,9 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \tif (!buf && !src)\n \t\treturn 1;\n \n+\tif (validate_encoding(path, enc, src, src_len, die_on_error))\n+\t\treturn 0;\n+\n \tdst = reencode_string_len(src, src_len, default_encoding, enc,\n \t\t\t\t &dst_len);\n \tif (!dst) {\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex 71e8e3700b..5c7e36a164 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -62,6 +62,46 @@ test_expect_success 'check $GIT_DIR/info/attributes support' '\n \n for i in 16 32\n do\n+\ttest_expect_success \"check prohibited UTF-${i} BOM\" '\n+\t\ttest_when_finished \"git reset --hard HEAD\" &&\n+\n+\t\techo \"*.utf${i}be text working-tree-encoding=utf-${i}be\" >>.gitattributes &&\n+\t\techo \"*.utf${i}le text working-tree-encoding=utf-${i}le\" >>.gitattributes &&\n+\n+\t\t# Here we add a UTF-16 (resp. UTF-32) files with BOM (big/little-endian)\n+\t\t# but we tell Git to treat it as UTF-16BE/UTF-16LE (resp. UTF-32).\n+\t\t# In these cases the BOM is prohibited.\n+\t\tcp bebom.utf${i}be.raw bebom.utf${i}be &&\n+\t\ttest_must_fail git add bebom.utf${i}be 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-${i}BE\" err.out &&\n+\n+\t\tcp lebom.utf${i}le.raw lebom.utf${i}be &&\n+\t\ttest_must_fail git add lebom.utf${i}be 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-${i}BE\" err.out &&\n+\n+\t\tcp bebom.utf${i}be.raw bebom.utf${i}le &&\n+\t\ttest_must_fail git add bebom.utf${i}le 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-${i}LE\" err.out &&\n+\n+\t\tcp lebom.utf${i}le.raw lebom.utf${i}le &&\n+\t\ttest_must_fail git add lebom.utf${i}le 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-${i}LE\" err.out\n+\t'\n+\n+\ttest_expect_success \"check required UTF-${i} BOM\" '\n+\t\ttest_when_finished \"git reset --hard HEAD\" &&\n+\n+\t\techo \"*.utf${i} text working-tree-encoding=utf-${i}\" >>.gitattributes &&\n+\n+\t\tcp nobom.utf${i}be.raw nobom.utf${i} &&\n+\t\ttest_must_fail git add nobom.utf${i} 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is required .* UTF-${i}\" err.out &&\n+\n+\t\tcp nobom.utf${i}le.raw nobom.utf${i} &&\n+\t\ttest_must_fail git add nobom.utf${i} 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is required .* UTF-${i}\" err.out\n+\t'\n+\n \ttest_expect_success \"eol conversion for UTF-${i} encoded files on checkout\" '\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@@ -132,4 +172,21 @@ test_expect_success 'error if encoding round trip is not the same during refresh\n \ttest_i18ngrep \"error: .* overwritten by checkout:\" err.out\n '\n \n+test_expect_success 'error if encoding garbage is already in Git' '\n+\tBEFORE_STATE=$(git rev-parse HEAD) &&\n+\ttest_when_finished \"rm -f err.out\" &&\n+\ttest_when_finished \"git reset --hard $BEFORE_STATE\" &&\n+\n+\t# Skip the UTF-16 filter for the added file\n+\t# This simulates a Git version that has no checkoutEncoding support\n+\tcp nobom.utf16be.raw nonsense.utf16 &&\n+\tTEST_HASH=$(git hash-object --no-filters -w nonsense.utf16) &&\n+\tgit update-index --add --cacheinfo 100644 $TEST_HASH nonsense.utf16 &&\n+\tCOMMIT=$(git commit-tree -p $(git rev-parse HEAD) -m \"plain commit\" $(git write-tree)) &&\n+\tgit update-ref refs/heads/master $COMMIT &&\n+\n+\tgit diff 2>err.out &&\n+\ttest_i18ngrep \"error: BOM is required\" err.out\n+'\n+\n test_done\n-- \n2.16.2\n\n"},{"id":"340992","messageId":"20180304201418.60958-8-lars.schneider@autodesk.com","threadId":"47977","inReplyTo":"20180304201418.60958-1-lars.schneider@autodesk.com","subject":"[PATCH v9 7/8] convert: add tracing for 'working-tree-encoding' attribute","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-04T20:14:17Z","receivedAt":"2018-03-04T20:16:13Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nAdd the GIT_TRACE_WORKING_TREE_ENCODING environment variable to enable\ntracing for content that is reencoded with the 'working-tree-encoding'\nattribute. This is useful to debug encoding issues.\n\nSigned-off-by: Lars Schneider \n---\n convert.c | 25 +++++++++++++++++++++++++\n t/t0028-working-tree-encoding.sh | 2 ++\n 2 files changed, 27 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex 9647b06679..eec34a94b9 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -313,6 +313,29 @@ static int validate_encoding(const char *path, const char *enc,\n \treturn 0;\n }\n \n+static void trace_encoding(const char *context, const char *path,\n+\t\t\t const char *encoding, const char *buf, size_t len)\n+{\n+\tstatic struct trace_key coe = TRACE_KEY_INIT(WORKING_TREE_ENCODING);\n+\tstruct strbuf trace = STRBUF_INIT;\n+\tint i;\n+\n+\tstrbuf_addf(&trace, \"%s (%s, considered %s):\\n\", context, path, encoding);\n+\tfor (i = 0; i < len && buf; ++i) {\n+\t\tstrbuf_addf(\n+\t\t\t&trace,\"| \\e[2m%2i:\\e[0m %2x \\e[2m%c\\e[0m%c\",\n+\t\t\ti,\n+\t\t\t(unsigned char) buf[i],\n+\t\t\t(buf[i] > 32 && buf[i] < 127 ? buf[i] : ' '),\n+\t\t\t((i+1) % 8 && (i+1) < len ? ' ' : '\\n')\n+\t\t);\n+\t}\n+\tstrbuf_addchars(&trace, '\\n', 1);\n+\n+\ttrace_strbuf(&coe, &trace);\n+\tstrbuf_release(&trace);\n+}\n+\n static const char *default_encoding = \"UTF-8\";\n \n static int encode_to_git(const char *path, const char *src, size_t src_len,\n@@ -341,6 +364,7 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \tif (validate_encoding(path, enc, src, src_len, die_on_error))\n \t\treturn 0;\n \n+\ttrace_encoding(\"source\", path, enc, src, src_len);\n \tdst = reencode_string_len(src, src_len, default_encoding, enc,\n \t\t\t\t &dst_len);\n \tif (!dst) {\n@@ -358,6 +382,7 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \t\t\treturn 0;\n \t\t}\n \t}\n+\ttrace_encoding(\"destination\", path, default_encoding, dst, dst_len);\n \n \tstrbuf_attach(buf, dst, dst_len, dst_len + 1);\n \treturn 1;\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex 5c7e36a164..bdc487b44f 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -4,6 +4,8 @@ test_description='working-tree-encoding conversion via gitattributes'\n \n . ./test-lib.sh\n \n+GIT_TRACE_WORKING_TREE_ENCODING=1 && export GIT_TRACE_WORKING_TREE_ENCODING\n+\n test_expect_success 'setup test files' '\n \tgit config core.eol lf &&\n \n-- \n2.16.2\n\n"},{"id":"341070","messageId":"xmqqfu5e8a53.fsf@gitster-ct.c.googlers.com","threadId":"47977","inReplyTo":"20180304201418.60958-7-lars.schneider@autodesk.com","subject":"Re: [PATCH v9 6/8] convert: check for detectable errors in UTF encodings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-05T21:50:32Z","receivedAt":"2018-03-05T21:50:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"lars.schneider@autodesk.com writes:\n\n> +static int validate_encoding(const char *path, const char *enc,\n> +\t\t const char *data, size_t len, int die_on_error)\n> +{\n> +\tif (!memcmp(\"UTF-\", enc, 4)) {\n\nDoes the caller already know that enc is sufficiently long that\nusing memcmp is safe?\n"},{"id":"341083","messageId":"55633B6F-EB6E-42D6-8E58-79158A99774A@gmail.com","threadId":"47977","inReplyTo":"xmqqfu5e8a53.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v9 6/8] convert: check for detectable errors in UTF encodings","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-05T23:45:02Z","receivedAt":"2018-03-05T23:46:13Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 05 Mar 2018, at 22:50, Junio C Hamano wrote:\n> \n> lars.schneider@autodesk.com writes:\n> \n>> +static int validate_encoding(const char *path, const char *enc,\n>> +\t\t const char *data, size_t len, int die_on_error)\n>> +{\n>> +\tif (!memcmp(\"UTF-\", enc, 4)) {\n> \n> Does the caller already know that enc is sufficiently long that\n> using memcmp is safe?\n\nNo :-(\n\nWould you be willing to squash that in?\n\n if (strlen(enc) > 4 && !memcmp(\"UTF-\", enc, 4)) {\n\nI deliberately used \"> 4\" as plain \"UTF-\" is not even valid.\n\n\nThanks for spotting this,\nLars\n"},{"id":"341086","messageId":"xmqqvaea6lpn.fsf@gitster-ct.c.googlers.com","threadId":"47977","inReplyTo":"55633B6F-EB6E-42D6-8E58-79158A99774A@gmail.com","subject":"Re: [PATCH v9 6/8] convert: check for detectable errors in UTF encodings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-06T01:23:32Z","receivedAt":"2018-03-06T01:23:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n>> On 05 Mar 2018, at 22:50, Junio C Hamano wrote:\n>> \n>> lars.schneider@autodesk.com writes:\n>> \n>>> +static int validate_encoding(const char *path, const char *enc,\n>>> +\t\t const char *data, size_t len, int die_on_error)\n>>> +{\n>>> +\tif (!memcmp(\"UTF-\", enc, 4)) {\n>> \n>> Does the caller already know that enc is sufficiently long that\n>> using memcmp is safe?\n>\n> No :-(\n>\n> Would you be willing to squash that in?\n>\n> if (strlen(enc) > 4 && !memcmp(\"UTF-\", enc, 4)) {\n>\n> I deliberately used \"> 4\" as plain \"UTF-\" is not even valid.\n\nI'd rather not. The code does not have to even look at 6th and\nlater bytes in the enc[] even if it wanted to reject \"UTF-\" followed\nby nothing, but use of strlen() forces it to look at everything.\n\nStepping back, shouldn't\n\n\tif (starts_with(enc, \"UTF-\") \n\nbe sufficient? If you really care about the case where \"UTF-\" alone\ncomes here, you could write\n\n\tif (starts_with(enc, \"UTF-\") && enc[4])\n\nbut I do not think \"&& enc[4]\" is even needed. The functions called\nfrom this block would not consider \"UTF-\" alone as something valid\nanyway, so with that \"&& enf[4]\" we would be piling more code only\nfor invalid/rare case.\n"},{"id":"341134","messageId":"A4240C40-A3DF-45DB-88A6-125FB2D3CE15@gmail.com","threadId":"47977","inReplyTo":"xmqqvaea6lpn.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v9 6/8] convert: check for detectable errors in UTF encodings","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-06T17:03:29Z","receivedAt":"2018-03-06T17:03:40Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 06 Mar 2018, at 02:23, Junio C Hamano wrote:\n> \n> Lars Schneider writes:\n> \n>>> On 05 Mar 2018, at 22:50, Junio C Hamano wrote:\n>>> \n>>> lars.schneider@autodesk.com writes:\n>>> \n>>>> +static int validate_encoding(const char *path, const char *enc,\n>>>> +\t\t const char *data, size_t len, int die_on_error)\n>>>> +{\n>>>> +\tif (!memcmp(\"UTF-\", enc, 4)) {\n>>> \n>>> Does the caller already know that enc is sufficiently long that\n>>> using memcmp is safe?\n>> \n>> No :-(\n>> \n>> Would you be willing to squash that in?\n>> \n>> if (strlen(enc) > 4 && !memcmp(\"UTF-\", enc, 4)) {\n>> \n>> I deliberately used \"> 4\" as plain \"UTF-\" is not even valid.\n> \n> I'd rather not. The code does not have to even look at 6th and\n> later bytes in the enc[] even if it wanted to reject \"UTF-\" followed\n> by nothing, but use of strlen() forces it to look at everything.\n> \n> Stepping back, shouldn't\n> \n> \tif (starts_with(enc, \"UTF-\") \n> \n> be sufficient? If you really care about the case where \"UTF-\" alone\n> comes here, you could write\n> \n> \tif (starts_with(enc, \"UTF-\") && enc[4])\n> \n> but I do not think \"&& enc[4]\" is even needed. The functions called\n> from this block would not consider \"UTF-\" alone as something valid\n> anyway, so with that \"&& enf[4]\" we would be piling more code only\n> for invalid/rare case.\n\nAgreed, \"if (starts_with(enc, \"UTF-\"))\" is sufficient. Can you squash\nthat in? Thanks for pointing me to starts_with() as I forgot about this\nfunction!\n\n- Lars\n"},{"id":"341154","messageId":"CAPig+cTOpBODeoHV=+4-4MEjKM=pUrZSa=BEmHh4mVP=xPpFCA@mail.gmail.com","threadId":"47977","inReplyTo":"20180304201418.60958-6-lars.schneider@autodesk.com","subject":"Re: [PATCH v9 5/8] convert: add 'working-tree-encoding' attribute","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-03-06T20:42:35Z","receivedAt":"2018-03-06T20:42:41Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Mar 4, 2018 at 3:14 PM, wrote:\n> Git recognizes files encoded with ASCII or one of its supersets (e.g.\n> UTF-8 or ISO-8859-1) as text files. All other encodings are usually\n> interpreted as binary and consequently built-in Git text processing\n> tools (e.g. 'git diff') as well as most Git web front ends do not\n> visualize the content.\n> [...]\n> Signed-off-by: Lars Schneider \n> ---\n> diff --git a/convert.c b/convert.c\n> @@ -978,6 +1051,25 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,\n> +static const char *git_path_check_encoding(struct attr_check_item *check)\n> +{\n> + [...]\n> + /*\n> + * Ensure encoding names are always upper case (e.g. UTF-8) to\n> + * simplify subsequent string comparisons.\n> + */\n> + return xstrdup_toupper(value);\n\nxstrdup_toupper() allocates memory...\n\n> +}\n> @@ -1033,6 +1125,7 @@ struct conv_attrs {\n> enum crlf_action attr_action; /* What attr says */\n> enum crlf_action crlf_action; /* When no attr is set, use core.autocrlf */\n> int ident;\n> + const char *working_tree_encoding; /* Supported encoding or default encoding if NULL */\n\n...which is assigned to 'const char *'...\n\n> };\n> @@ -1064,6 +1158,7 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)\n> else if (eol_attr == EOL_CRLF)\n> ca->crlf_action = CRLF_TEXT_CRLF;\n> }\n> + ca->working_tree_encoding = git_path_check_encoding(ccheck + 5);\n\n...by this code, and eventually leaked.\n\nIt's too bad it isn't cleaned up (freed), but looking at the callers,\nfixing this leak would be mildly noisy (though not particularly\ninvasive). How much do we care about this leak?\n\n> } else {\n> ca->drv = NULL;\n> ca->crlf_action = CRLF_UNDEFINED;\n> diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\n> @@ -0,0 +1,135 @@\n> +test_expect_success 'check $GIT_DIR/info/attributes support' '\n> + test_when_finished \"rm -f test.utf8.raw test.utf32.raw test.utf32.git\" &&\n\nIt seems weird to be cleaning up files this test didn't create\n(test.utf8.raw and test.utf32.raw).\n\n> + test_when_finished \"git reset --hard HEAD\" &&\n> +\n> + echo \"*.utf32 text working-tree-encoding=utf-32\" >.git/info/attributes &&\n> + git add test.utf32 &&\n> +\n> + git cat-file -p :test.utf32 >test.utf32.git &&\n> + test_cmp_bin test.utf8.raw test.utf32.git\n> +'\n> +\n> +test_expect_success 'check unsupported encodings' '\n> + test_when_finished \"rm -f err.out\" &&\n> + test_when_finished \"git reset --hard HEAD\" &&\n\nResetting to HEAD here is an important cleanup action, but tests don't\nusually clean up files such as 'err.out' since such detritus doesn't\nusually impact subsequent tests negatively. (Just an observation; no\nre-roll needed.)\n\n> + echo \"*.nothing text working-tree-encoding=\" >>.gitattributes &&\n> + printf \"nothing\" >t.nothing &&\n> + git add t.nothing &&\n> +\n> + echo \"*.garbage text working-tree-encoding=garbage\" >>.gitattributes &&\n> + printf \"garbage\" >t.garbage &&\n> + test_must_fail git add t.garbage 2>err.out &&\n> + test_i18ngrep \"fatal: failed to encode\" err.out\n> +'\n"},{"id":"341155","messageId":"xmqqvae92ajv.fsf@gitster-ct.c.googlers.com","threadId":"47977","inReplyTo":"20180304201418.60958-5-lars.schneider@autodesk.com","subject":"Re: [PATCH v9 4/8] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-06T20:50:28Z","receivedAt":"2018-03-06T20:50:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"lars.schneider@autodesk.com writes:\n\n> +int is_missing_required_utf_bom(const char *enc, const char *data, size_t len)\n> +{\n> +\treturn (\n> +\t !strcmp(enc, \"UTF-16\") &&\n> +\t !(has_bom_prefix(data, len, utf16_be_bom, sizeof(utf16_be_bom)) ||\n> +\t has_bom_prefix(data, len, utf16_le_bom, sizeof(utf16_le_bom)))\n> +\t) || (\n> +\t !strcmp(enc, \"UTF-32\") &&\n> +\t !(has_bom_prefix(data, len, utf32_be_bom, sizeof(utf32_be_bom)) ||\n> +\t has_bom_prefix(data, len, utf32_le_bom, sizeof(utf32_le_bom)))\n> +\t);\n> +}\n\nThese strcmp() calls seem inconsistent with the principle embodied\nby utf8.c::fallback_encoding(), i.e. \"be lenient to what we accept\",\nand make the interface uneven. I am wondering if we also want to\ncomplain when the user gave us \"utf16\" and there is no byte order\nmark in the contents, for example? Also \"UTF16\" or other spelling\nthe platform may support but this code fails to recognise will go\nunchecked.\n\nWhich actually may be a feature, not a bug, to be able to bypass\nthis check---I dunno.\n\nThe same comment applies to the previous step.\n\n"},{"id":"341156","messageId":"CAPig+cSoka-yBTYBz42JGQTyCH7LDWnToeOvdZfG0_64o9QnBQ@mail.gmail.com","threadId":"47977","inReplyTo":"20180304201418.60958-7-lars.schneider@autodesk.com","subject":"Re: [PATCH v9 6/8] convert: check for detectable errors in UTF encodings","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-03-06T20:55:12Z","receivedAt":"2018-03-06T20:55:18Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Mar 4, 2018 at 3:14 PM, wrote:\n> Check that new content is valid with respect to the user defined\n> 'working-tree-encoding' attribute.\n>\n> Signed-off-by: Lars Schneider \n> ---\n> diff --git a/convert.c b/convert.c\n> @@ -266,6 +266,53 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n> +static int validate_encoding(const char *path, const char *enc,\n> + const char *data, size_t len, int die_on_error)\n> +{\n> + if (!memcmp(\"UTF-\", enc, 4)) {\n> + [...]\n> + if (has_prohibited_utf_bom(enc, data, len)) {\n> + [...]\n> + if (die_on_error)\n> + die(error_msg, path, enc);\n> + else {\n> + return error(error_msg, path, enc);\n> + }\n> + } [...]\n> + return 0;\n> +}\n> @@ -291,6 +338,9 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n> + if (validate_encoding(path, enc, src, src_len, die_on_error))\n> + return 0;\n\nThere could be a cleaner separation of responsibilities here which, as\na nice side-effect, would eliminate the repeated \"if (die_on_error)\ndie(...); else return error(...);\" pattern. Rather than passing a\n'die_on_error' flag to validate_encoding(), it could accept a 'strbuf\n*err' to populate in case of error:\n\n static int validate_encoding(..., struct strbuf *err)\n {\n if validation error:\n populate 'err'\n return -1;\n return 0\n }\n\nand let the caller be responsible for deciding how to handle failure:\n\n struct strbuf err = STRBUF_INIT;\n ...\n if (validate_encoding(..., &err)) {\n if (die_on_error)\n die(err.buf);\n else {\n error(err.buf);\n strbuf_release(&err);\n return 0;\n }\n }\n\nNot necessarily worth a re-roll, but perhaps a cleanup someone could\nsubmit at some point if interested.\n\n> dst = reencode_string_len(src, src_len, default_encoding, enc,\n> &dst_len);\n"},{"id":"341158","messageId":"CAPig+cTZGVqWSUpRuHdGwDEq5FBwzpW4j=v_1VXe_Z2+YCTrOQ@mail.gmail.com","threadId":"47977","inReplyTo":"20180304201418.60958-1-lars.schneider@autodesk.com","subject":"Re: [PATCH v9 0/8] convert: add support for different encodings","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-03-06T21:05:38Z","receivedAt":"2018-03-06T21:05:43Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Mar 4, 2018 at 3:14 PM, wrote:\n> Changes since v8: [...]\n>\n> Thanks a lot Eric for your great review! I think I fixed everything you\n> objected with one exception. You noticed that the current code only\n> checks for BOMs corresponding to the declared size (16 or 32 bits) [1].\n> I understand your point of view and I agree that any BOM in these cases\n> is *most likely* an error. However, according to the spec it might\n> still be valid. The comments on my related question on StackOverflow\n> seem to support that view [2]. Therefore, I would like to leave it as\n> it is in this series. If it turns out to be a problem in practice, then\n> I am happy to change it later. OK for you?\n\nFine. As this is not a correctness issue with the conversion itself,\nbut rather an attempt to detect misconfiguration by the user, it can\nalways be revisited later if someone comes up with a more solid\nargument that one interpretation is more correct than the other.\n\nI did leave a few review comments on v9, but I don't think any are\nnecessarily worth a re-roll. If anything, and if they seem actionable,\nthen perhaps a patch or two atop the series could address them (at\nsome point).\n"},{"id":"341163","messageId":"570D707A-DD9E-4397-8155-E8B3C3D09760@gmail.com","threadId":"47977","inReplyTo":"CAPig+cTOpBODeoHV=+4-4MEjKM=pUrZSa=BEmHh4mVP=xPpFCA@mail.gmail.com","subject":"Re: [PATCH v9 5/8] convert: add 'working-tree-encoding' attribute","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-06T22:13:24Z","receivedAt":"2018-03-06T22:13:36Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 06 Mar 2018, at 21:42, Eric Sunshine wrote:\n> \n> On Sun, Mar 4, 2018 at 3:14 PM, wrote:\n>> Git recognizes files encoded with ASCII or one of its supersets (e.g.\n>> UTF-8 or ISO-8859-1) as text files. All other encodings are usually\n>> interpreted as binary and consequently built-in Git text processing\n>> tools (e.g. 'git diff') as well as most Git web front ends do not\n>> visualize the content.\n>> [...]\n>> Signed-off-by: Lars Schneider \n>> ---\n>> diff --git a/convert.c b/convert.c\n>> @@ -978,6 +1051,25 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,\n>> +static const char *git_path_check_encoding(struct attr_check_item *check)\n>> +{\n>> + [...]\n>> + /*\n>> + * Ensure encoding names are always upper case (e.g. UTF-8) to\n>> + * simplify subsequent string comparisons.\n>> + */\n>> + return xstrdup_toupper(value);\n> \n> xstrdup_toupper() allocates memory...\n> \n>> +}\n>> @@ -1033,6 +1125,7 @@ struct conv_attrs {\n>> enum crlf_action attr_action; /* What attr says */\n>> enum crlf_action crlf_action; /* When no attr is set, use core.autocrlf */\n>> int ident;\n>> + const char *working_tree_encoding; /* Supported encoding or default encoding if NULL */\n> \n> ...which is assigned to 'const char *'...\n> \n>> };\n>> @@ -1064,6 +1158,7 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)\n>> else if (eol_attr == EOL_CRLF)\n>> ca->crlf_action = CRLF_TEXT_CRLF;\n>> }\n>> + ca->working_tree_encoding = git_path_check_encoding(ccheck + 5);\n> \n> ...by this code, and eventually leaked.\n> \n> It's too bad it isn't cleaned up (freed), but looking at the callers,\n> fixing this leak would be mildly noisy (though not particularly\n> invasive). How much do we care about this leak?\n\nHmm. You are right. That was previously handled by the encoding struct \nlinked list that I removed in this iteration. I forgot about that aspect :/\nI don't like it leaking. I think I would like to reintroduce the linked\nlist. This way every encoding is only once in memory. What do you think?\n\n\n>> } else {\n>> ca->drv = NULL;\n>> ca->crlf_action = CRLF_UNDEFINED;\n>> diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\n>> @@ -0,0 +1,135 @@\n>> +test_expect_success 'check $GIT_DIR/info/attributes support' '\n>> + test_when_finished \"rm -f test.utf8.raw test.utf32.raw test.utf32.git\" &&\n> \n> It seems weird to be cleaning up files this test didn't create\n> (test.utf8.raw and test.utf32.raw).\n\nAgreed.\n\n\n>> + test_when_finished \"git reset --hard HEAD\" &&\n>> +\n>> + echo \"*.utf32 text working-tree-encoding=utf-32\" >.git/info/attributes &&\n>> + git add test.utf32 &&\n>> +\n>> + git cat-file -p :test.utf32 >test.utf32.git &&\n>> + test_cmp_bin test.utf8.raw test.utf32.git\n>> +'\n>> +\n>> +test_expect_success 'check unsupported encodings' '\n>> + test_when_finished \"rm -f err.out\" &&\n>> + test_when_finished \"git reset --hard HEAD\" &&\n> \n> Resetting to HEAD here is an important cleanup action, but tests don't\n> usually clean up files such as 'err.out' since such detritus doesn't\n> usually impact subsequent tests negatively. (Just an observation; no\n> re-roll needed.)\n\nOK. I'll fix it if I reroll.\n\n- Lars"},{"id":"341164","messageId":"CAPig+cQE0pKs-AMvh4GndyCXBMnx=70jPpDM6K4jJTe-74FecQ@mail.gmail.com","threadId":"47977","inReplyTo":"570D707A-DD9E-4397-8155-E8B3C3D09760@gmail.com","subject":"Re: [PATCH v9 5/8] convert: add 'working-tree-encoding' attribute","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-03-06T22:22:10Z","receivedAt":"2018-03-06T22:22:18Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Mar 6, 2018 at 5:13 PM, Lars Schneider wrote:\n>> On 06 Mar 2018, at 21:42, Eric Sunshine wrote:\n>> On Sun, Mar 4, 2018 at 3:14 PM, wrote:\n>>> + return xstrdup_toupper(value);\n>>\n>> xstrdup_toupper() allocates memory...\n>>\n>>> + const char *working_tree_encoding; /* Supported encoding or default encoding if NULL */\n>>\n>> ...which is assigned to 'const char *'...\n>>\n>>> + ca->working_tree_encoding = git_path_check_encoding(ccheck + 5);\n>>\n>> ...by this code, and eventually leaked.\n>>\n>> It's too bad it isn't cleaned up (freed), but looking at the callers,\n>> fixing this leak would be mildly noisy (though not particularly\n>> invasive). How much do we care about this leak?\n>\n> Hmm. You are right. That was previously handled by the encoding struct\n> linked list that I removed in this iteration. I forgot about that aspect :/\n> I don't like it leaking. I think I would like to reintroduce the linked\n> list. This way every encoding is only once in memory. What do you think?\n\nIt's subjective, but I find the use of a linked-list just for the\npurpose of not leaking these strings unnecessarily confusing.\n\nIf I was doing it, I'd probably add a conv_attrs_free() function and\ncall it from each function allocates a 'struct conv_attrs' (including\ncalling it before early returns -- which prompted my earlier comment\nabout it being a \"mildly noisy\" fix).\n"},{"id":"341166","messageId":"0DCED348-C6DB-49B2-8C0A-E4E1F485B255@gmail.com","threadId":"47977","inReplyTo":"xmqqvae92ajv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v9 4/8] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-06T22:39:16Z","receivedAt":"2018-03-06T22:39:26Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 06 Mar 2018, at 21:50, Junio C Hamano wrote:\n> \n> lars.schneider@autodesk.com writes:\n> \n>> +int is_missing_required_utf_bom(const char *enc, const char *data, size_t len)\n>> +{\n>> +\treturn (\n>> +\t !strcmp(enc, \"UTF-16\") &&\n>> +\t !(has_bom_prefix(data, len, utf16_be_bom, sizeof(utf16_be_bom)) ||\n>> +\t has_bom_prefix(data, len, utf16_le_bom, sizeof(utf16_le_bom)))\n>> +\t) || (\n>> +\t !strcmp(enc, \"UTF-32\") &&\n>> +\t !(has_bom_prefix(data, len, utf32_be_bom, sizeof(utf32_be_bom)) ||\n>> +\t has_bom_prefix(data, len, utf32_le_bom, sizeof(utf32_le_bom)))\n>> +\t);\n>> +}\n> \n> These strcmp() calls seem inconsistent with the principle embodied\n> by utf8.c::fallback_encoding(), i.e. \"be lenient to what we accept\",\n> and make the interface uneven. I am wondering if we also want to\n> complain when the user gave us \"utf16\" and there is no byte order\n> mark in the contents, for example?\n\nWell, if I use stricmp() then I don't need to call and cleanup\nxstrdup_toupper() as discussed with Eric [1]. Is there a case\ninsensitive starts_with() method?\n\n[1] https://public-inbox.org/git/CAPig+cQE0pKs-AMvh4GndyCXBMnx=70jPpDM6K4jJTe-74FecQ@mail.gmail.com/\n\n\n> Also \"UTF16\" or other spelling\n> the platform may support but this code fails to recognise will go\n> unchecked.\n\nThat is true. However, I would assume all iconv implementations use the\nsame encoding names for UTF encodings, no? That means UTF16 would never be\nvalid. Would you agree?\n\n- Lars"},{"id":"341168","messageId":"xmqq8tb43jfe.fsf@gitster-ct.c.googlers.com","threadId":"47977","inReplyTo":"0DCED348-C6DB-49B2-8C0A-E4E1F485B255@gmail.com","subject":"Re: [PATCH v9 4/8] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-06T22:53:25Z","receivedAt":"2018-03-06T22:53:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n>> Also \"UTF16\" or other spelling\n>> the platform may support but this code fails to recognise will go\n>> unchecked.\n>\n> That is true. However, I would assume all iconv implementations use the\n> same encoding names for UTF encodings, no? That means UTF16 would never be\n> valid. Would you agree?\n\nAfter seeing \"UTF16\" and others in \"iconv -l | grep -i utf\" output,\nI am not sure what you mean by \"Would you agree?\" People can say in\ntheir .gitattributes file that this path is in \"UTF16\" without dash\nand that is what will be fed to this coe, no?\n"},{"id":"341169","messageId":"CF78BB76-B42C-4A83-B18C-3FBAB0339346@gmail.com","threadId":"47977","inReplyTo":"xmqq8tb43jfe.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v9 4/8] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-06T23:00:54Z","receivedAt":"2018-03-06T23:01:04Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 06 Mar 2018, at 23:53, Junio C Hamano wrote:\n> \n> Lars Schneider writes:\n> \n>>> Also \"UTF16\" or other spelling\n>>> the platform may support but this code fails to recognise will go\n>>> unchecked.\n>> \n>> That is true. However, I would assume all iconv implementations use the\n>> same encoding names for UTF encodings, no? That means UTF16 would never be\n>> valid. Would you agree?\n> \n> After seeing \"UTF16\" and others in \"iconv -l | grep -i utf\" output,\n> I am not sure what you mean by \"Would you agree?\" People can say in\n> their .gitattributes file that this path is in \"UTF16\" without dash\n> and that is what will be fed to this coe, no?\n\nOn macOS I don't see UTF16... but I just checked on my Linux box and\nthere it is. Consequently, we need to check both versions: with and\nwithout dash.\n\nI'll reroll ASAP (I try to do it first thing in the morning).\n\nThanks,\nLars\n"},{"id":"341172","messageId":"xmqq4lls3irx.fsf@gitster-ct.c.googlers.com","threadId":"47977","inReplyTo":"xmqq8tb43jfe.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v9 4/8] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-06T23:07:30Z","receivedAt":"2018-03-06T23:07:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Lars Schneider writes:\n>\n>>> Also \"UTF16\" or other spelling\n>>> the platform may support but this code fails to recognise will go\n>>> unchecked.\n>>\n>> That is true. However, I would assume all iconv implementations use the\n>> same encoding names for UTF encodings, no? That means UTF16 would never be\n>> valid. Would you agree?\n>\n> After seeing \"UTF16\" and others in \"iconv -l | grep -i utf\" output,\n> I am not sure what you mean by \"Would you agree?\" People can say in\n> their .gitattributes file that this path is in \"UTF16\" without dash\n> and that is what will be fed to this code, no?\n\nFWIW, \"iconv -f utf8 -t utf16\" seems not to complain, so I am\nreasonably sure that people expect downcased ones to work as well.\n"},{"id":"341173","messageId":"EC8DF4D3-CDF5-4CEB-BD18-585341BBB2C5@gmail.com","threadId":"47977","inReplyTo":"xmqq4lls3irx.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v9 4/8] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-06T23:12:44Z","receivedAt":"2018-03-06T23:12:54Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 07 Mar 2018, at 00:07, Junio C Hamano wrote:\n> \n> Junio C Hamano writes:\n> \n>> Lars Schneider writes:\n>> \n>>>> Also \"UTF16\" or other spelling\n>>>> the platform may support but this code fails to recognise will go\n>>>> unchecked.\n>>> \n>>> That is true. However, I would assume all iconv implementations use the\n>>> same encoding names for UTF encodings, no? That means UTF16 would never be\n>>> valid. Would you agree?\n>> \n>> After seeing \"UTF16\" and others in \"iconv -l | grep -i utf\" output,\n>> I am not sure what you mean by \"Would you agree?\" People can say in\n>> their .gitattributes file that this path is in \"UTF16\" without dash\n>> and that is what will be fed to this code, no?\n> \n> FWIW, \"iconv -f utf8 -t utf16\" seems not to complain, so I am\n> reasonably sure that people expect downcased ones to work as well.\n\nSure! That's why I normalized it to upper case:\nhttps://public-inbox.org/git/CAPig+cQE0pKs-AMvh4GndyCXBMnx=70jPpDM6K4jJTe-74FecQ@mail.gmail.com/\n\nAfter thinking about it I wonder if we should barf on \"utf16\" without\ndash. Your Linux iconv would handle this correctly. My macOS iconv would not.\nThat means the repo would checkout correctly on your machine but not on mine.\n\nWhat do you think?\n\n- Lars\n\n"},{"id":"341178","messageId":"xmqqpo4g22tv.fsf@gitster-ct.c.googlers.com","threadId":"47977","inReplyTo":"EC8DF4D3-CDF5-4CEB-BD18-585341BBB2C5@gmail.com","subject":"Re: [PATCH v9 4/8] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-06T23:37:16Z","receivedAt":"2018-03-06T23:37:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n> After thinking about it I wonder if we should barf on \"utf16\" without\n> dash. Your Linux iconv would handle this correctly. My macOS iconv would not.\n> That means the repo would checkout correctly on your machine but not on mine.\n>\n> What do you think?\n\nTo be bluntly honest, I prefer not to have excess \"sanity checks\";\nthere is no need to barf on utf16 in a project run by those who are\nwithout macOS friends, for example. For that matter, while I do not\nhate it so much to reject it, I am not all that supportive of this\n\"The consortium says without -LE or -BE suffix there must be BOM,\nand this lacks one, so barf loudly\" step in this topic myself.\n"},{"id":"341219","messageId":"20180307173911.GA19262@tor.lan","threadId":"47977","inReplyTo":"xmqqpo4g22tv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v9 4/8] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-03-07T17:39:11Z","receivedAt":"2018-03-07T17:39:48Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Tue, Mar 06, 2018 at 03:37:16PM -0800, Junio C Hamano wrote:\n> Lars Schneider writes:\n> \n> > After thinking about it I wonder if we should barf on \"utf16\" without\n> > dash. Your Linux iconv would handle this correctly. My macOS iconv would not.\n> > That means the repo would checkout correctly on your machine but not on mine.\n> >\n> > What do you think?\n> \n> To be bluntly honest, I prefer not to have excess \"sanity checks\";\n> there is no need to barf on utf16 in a project run by those who are\n> without macOS friends, for example. For that matter, while I do not\n> hate it so much to reject it, I am not all that supportive of this\n> \"The consortium says without -LE or -BE suffix there must be BOM,\n> and this lacks one, so barf loudly\" step in this topic myself.\n\nLoosing or adding a BOM couild render a file useless, depending\non the SW that reads and processes it.\n\nThe main reason for being critacal is to make sure that the\nmaterial in the working-tree does a safe roundtrip when commited,\npushed, pulled, checked out...\n\nThe best thing we can do is probably to follow the specification as\ngood as possible.\n\nHaving a clear specification (UTF-16LE/BE has no BOM, UTF-16 has none,\nUTF-16 has a BOM) would even open the chance to work around a buggy\niconv library, because Git can check the BOM.\nIf, and only if, a platform messes it up, and we want to\nmake a (compile time) workaround in Git for this very platform.\n\nA consistant behavior across all platforms is a good thing,\nsometimes I have a network share mounted to Linux, MacOS and\nWindows and it doesn't matter under which Git on which\nmachine I checkout or commit.\n\nOh, I see, there is a new patch coming...\n"}]}