{"thread":{"id":"47654","subject":"[PATCH v4 0/6] convert: add support for different encodings","startedAt":"2018-01-20T15:24:53Z","lastAt":"2018-02-07T18:12:29Z","messageCount":43,"participants":["lars.schneider@autodesk.com","Simon Ruderich","Lars Schneider","Eric Sunshine","Jeff King","Torsten Bögershausen","Junio C Hamano","tboegi@web.de"],"isPatch":true,"patchVersion":4,"patchTotal":6},"messages":[{"id":"336983","messageId":"20180120152418.52859-1-lars.schneider@autodesk.com","threadId":"47654","inReplyTo":null,"subject":"[PATCH v4 0/6] convert: add support for different encodings","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-01-20T15:24:12Z","receivedAt":"2018-01-20T15:24:53Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\nHi,\n\nPatches 1-4 and 6 are preparation and helper functions.\nPatch 5 is the actual change.\n\nThis series depends on Torsten's \"convert_to_git(): safe_crlf/checksafe\nbecomes int conv_flags\" patch:\nhttps://public-inbox.org/git/20180113224931.27031-1-tboegi@web.de/\n\nChanges since v3:\n\n* I renamed the attribute from \"checkout-encoding\" to \"working-tree-encoding\"\n  in the hope to convey better what the attribute is about.\n\n* I rebased the series to Git 2.16 and removed Torsten's patch as he\n  posted the patch on his own.\n\n* Fix documentation wording. (Torsten)\n\n* A macro was used in a commit before it's introduction. Fixed!(Junio)\n\nThanks,\nLars\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\n\nBase Ref:\nWeb-Diff: https://github.com/larsxschneider/git/commit/21f4dac5ab\nCheckout: git fetch https://github.com/larsxschneider/git encoding-v4 && git checkout 21f4dac5ab\n\n\n### Interdiff (v3-rebased-2.16..v4):\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 1bc03e69cb..a8dbf4be30 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -272,8 +272,8 @@ few exceptions.  Even though...\n   catch potential problems early, safety triggers.\n\n\n-`checkout-encoding`\n-^^^^^^^^^^^^^^^^^^^\n+`working-tree-encoding`\n+^^^^^^^^^^^^^^^^^^^^^^^\n\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@@ -281,17 +281,17 @@ 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-In these cases you can teach Git the encoding of a file in the working\n-directory with the `checkout-encoding` attribute. If a file with this\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 attributes is added to Git, then Git reencodes the content from the\n specified encoding to UTF-8 and stores the result in its internal data\n structure (called \"the index\"). On checkout the content is encoded\n back to the specified encoding.\n\n-Please note that using the `checkout-encoding` attribute may have a\n+Please note that using the `working-tree-encoding` attribute may have a\n number of pitfalls:\n\n-- Git clients that do not support the `checkout-encoding` attribute\n+- Git clients that do not support the `working-tree-encoding` attribute\n   will checkout the respective files UTF-8 encoded and not in the\n   expected encoding. Consequently, these files will appear different\n   which typically causes trouble. This is in particular the case for\n@@ -304,7 +304,7 @@ number of pitfalls:\n - Reencoding content requires resources that might slow down certain\n   Git operations (e.g 'git checkout' or 'git add').\n\n-Use the `checkout-encoding` attribute only if you cannot store a file in\n+Use the `working-tree-encoding` attribute only if you cannot store a file in\n UTF-8 encoding and if you want Git to be able to process the content as\n text.\n\n@@ -313,7 +313,7 @@ with byte order mark (BOM) and you want Git to perform automatic line\n ending conversion based on your platform.\n\n ------------------------\n-*.txt    text checkout-encoding=UTF-16\n+*.txt    text working-tree-encoding=UTF-16\n ------------------------\n\n Use the following attributes if your '*.txt' files are UTF-16 little\n@@ -321,7 +321,7 @@ endian encoded without BOM and you want Git to use Windows line endings\n in the working directory.\n\n ------------------------\n-*.txt    checkout-encoding=UTF-16LE text eol=CRLF\n+*.txt    working-tree-encoding=UTF-16LE text eol=CRLF\n ------------------------\n\n You can get a list of all available encodings on your platform with the\ndiff --git a/convert.c b/convert.c\nindex 8559651b3f..13fad490ce 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -323,7 +323,7 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n    const char *advise_msg = _(\n      \"You told Git to treat '%s' as %s. A byte order mark \"\n      \"(BOM) is prohibited with this encoding. Either use \"\n-     \"%.6s as checkout encoding or remove the BOM from the \"\n+     \"%.6s as working tree encoding or remove the BOM from the \"\n      \"file.\");\n\n    advise(advise_msg, path, enc->name, enc->name, enc->name);\n@@ -339,7 +339,7 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n    const char *advise_msg = _(\n      \"You told Git to treat '%s' as %s. A byte order mark \"\n      \"(BOM) is required with this encoding. Either use \"\n-     \"%sBE/%sLE as checkout encoding or add a BOM to the \"\n+     \"%sBE/%sLE as working tree encoding or add a BOM to the \"\n      \"file.\");\n    advise(advise_msg, path, enc->name, enc->name, enc->name);\n    if (conv_flags & CONV_WRITE_OBJECT)\n@@ -1237,7 +1237,7 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)\n\n  if (!check) {\n    check = attr_check_initl(\"crlf\", \"ident\", \"filter\",\n-          \"eol\", \"text\", \"checkout-encoding\",\n+          \"eol\", \"text\", \"working-tree-encoding\",\n           NULL);\n    user_convert_tail = &user_convert;\n    encoding_tail = &encoding;\ndiff --git a/t/t0028-checkout-encoding.sh b/t/t0028-working-tree-encoding.sh\nsimilarity index 89%\nrename from t/t0028-checkout-encoding.sh\nrename to t/t0028-working-tree-encoding.sh\nindex 5f1c911c07..0f36d4990a 100755\n--- a/t/t0028-checkout-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n\n-test_description='checkout-encoding conversion via gitattributes'\n+test_description='working-tree-encoding conversion via gitattributes'\n\n . ./test-lib.sh\n\n@@ -10,7 +10,7 @@ test_expect_success 'setup test repo' '\n  git config core.eol lf &&\n\n  text=\"hallo there!\\ncan you read me?\" &&\n- echo \"*.utf16 text checkout-encoding=utf-16\" >.gitattributes &&\n+ echo \"*.utf16 text working-tree-encoding=utf-16\" >.gitattributes &&\n  printf \"$text\" >test.utf8.raw &&\n  printf \"$text\" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&\n  cp test.utf16.raw test.utf16 &&\n@@ -45,10 +45,10 @@ test_expect_success 'check prohibited UTF BOM' '\n  printf \"\\0\\0\\376\\777\\0\\0\\0a\\0\\0\\0b\\0\\0\\0c\" >bebom.utf32be.raw &&\n  printf \"\\777\\376\\0\\0a\\0\\0\\0b\\0\\0\\0c\\0\\0\\0\" >lebom.utf32le.raw &&\n\n- echo \"*.utf16be text checkout-encoding=utf-16be\" >>.gitattributes &&\n- echo \"*.utf16le text checkout-encoding=utf-16le\" >>.gitattributes &&\n- echo \"*.utf32be text checkout-encoding=utf-32be\" >>.gitattributes &&\n- echo \"*.utf32le text checkout-encoding=utf-32le\" >>.gitattributes &&\n+ echo \"*.utf16be text working-tree-encoding=utf-16be\" >>.gitattributes &&\n+ echo \"*.utf16le text working-tree-encoding=utf-16le\" >>.gitattributes &&\n+ echo \"*.utf32be text working-tree-encoding=utf-32be\" >>.gitattributes &&\n+ echo \"*.utf32le text working-tree-encoding=utf-32le\" >>.gitattributes &&\n\n  # Here we add a UTF-16 files with BOM (big-endian and little-endian)\n  # but we tell Git to treat it as UTF-16BE/UTF-16LE. In these cases\n@@ -91,7 +91,7 @@ test_expect_success 'check prohibited UTF BOM' '\n '\n\n test_expect_success 'check required UTF BOM' '\n- echo \"*.utf32 text checkout-encoding=utf-32\" >>.gitattributes &&\n+ echo \"*.utf32 text working-tree-encoding=utf-32\" >>.gitattributes &&\n\n  cp nobom.utf16be.raw nobom.utf16 &&\n  test_must_fail git add nobom.utf16 2>err.out &&\n@@ -143,11 +143,11 @@ test_expect_success 'eol conversion for UTF-16 encoded files on checkout' '\n\n test_expect_success 'check unsupported encodings' '\n\n- echo \"*.nothing text checkout-encoding=\" >>.gitattributes &&\n+ echo \"*.nothing text working-tree-encoding=\" >>.gitattributes &&\n  printf \"nothing\" >t.nothing &&\n  git add t.nothing &&\n\n- echo \"*.garbage text checkout-encoding=garbage\" >>.gitattributes &&\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@@ -161,7 +161,7 @@ test_expect_success 'error if encoding round trip is not the same during refresh\n  BEFORE_STATE=$(git rev-parse HEAD) &&\n\n  # Skip the UTF-16 filter for the added file\n- # This simulates a Git version that has no checkoutEncoding support\n+ # This simulates a Git version that has no working tree encoding support\n  echo \"hallo\" >nonsense.utf16 &&\n  TEST_HASH=$(git hash-object --no-filters -w nonsense.utf16) &&\n  git update-index --add --cacheinfo 100644 $TEST_HASH nonsense.utf16 &&\n\n\n\n### Patches\n\nLars Schneider (6):\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: add tracing for 'working-tree-encoding' attribute\n\n Documentation/gitattributes.txt  |  60 +++++++++++\n convert.c                        | 218 ++++++++++++++++++++++++++++++++++++++-\n convert.h                        |   1 +\n sha1_file.c                      |   2 +-\n strbuf.c                         |  13 ++-\n strbuf.h                         |   1 +\n t/t0028-working-tree-encoding.sh | 198 +++++++++++++++++++++++++++++++++++\n utf8.c                           |  37 +++++++\n utf8.h                           |  25 +++++\n 9 files changed, 552 insertions(+), 3 deletions(-)\n create mode 100755 t/t0028-working-tree-encoding.sh\n\n\nbase-commit: 8a2f0888555ce46ac87452b194dec5cb66fb1417\n--\n2.16.0\n\n"},{"id":"336984","messageId":"20180120152418.52859-2-lars.schneider@autodesk.com","threadId":"47654","inReplyTo":"20180120152418.52859-1-lars.schneider@autodesk.com","subject":"[PATCH v4 1/6] strbuf: remove unnecessary NUL assignment in xstrdup_tolower()","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-01-20T15:24:13Z","receivedAt":"2018-01-20T15:24:57Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\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 <larsxschneider@gmail.com>\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.0\n\n"},{"id":"336985","messageId":"20180120152418.52859-3-lars.schneider@autodesk.com","threadId":"47654","inReplyTo":"20180120152418.52859-1-lars.schneider@autodesk.com","subject":"[PATCH v4 2/6] strbuf: add xstrdup_toupper()","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-01-20T15:24:14Z","receivedAt":"2018-01-20T15:24:59Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\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 <larsxschneider@gmail.com>\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.0\n\n"},{"id":"336986","messageId":"20180120152418.52859-4-lars.schneider@autodesk.com","threadId":"47654","inReplyTo":"20180120152418.52859-1-lars.schneider@autodesk.com","subject":"[PATCH v4 3/6] utf8: add function to detect prohibited UTF-16/32 BOM","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-01-20T15:24:15Z","receivedAt":"2018-01-20T15:25:03Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\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 <larsxschneider@gmail.com>\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..4711429af9 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+ * 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+ *\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.0\n\n"},{"id":"336987","messageId":"20180120152418.52859-5-lars.schneider@autodesk.com","threadId":"47654","inReplyTo":"20180120152418.52859-1-lars.schneider@autodesk.com","subject":"[PATCH v4 4/6] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-01-20T15:24:16Z","receivedAt":"2018-01-20T15:25:06Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\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\nhas_missing_utf_bom() function returns true if a required BOM is\nmissing.\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 <larsxschneider@gmail.com>\n---\n utf8.c | 13 +++++++++++++\n utf8.h | 16 ++++++++++++++++\n 2 files changed, 29 insertions(+)\n\ndiff --git a/utf8.c b/utf8.c\nindex 914881cd1f..f033fec1c2 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 has_missing_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 4711429af9..26b5e91852 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -79,4 +79,20 @@ 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\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+ *\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 has_missing_utf_bom(const char *enc, const char *data, size_t len);\n+\n #endif\n-- \n2.16.0\n\n"},{"id":"336988","messageId":"20180120152418.52859-6-lars.schneider@autodesk.com","threadId":"47654","inReplyTo":"20180120152418.52859-1-lars.schneider@autodesk.com","subject":"[PATCH v4 5/6] convert: add 'working-tree-encoding' attribute","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-01-20T15:24:17Z","receivedAt":"2018-01-20T15:25:10Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\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 <larsxschneider@gmail.com>\n---\n Documentation/gitattributes.txt  |  60 ++++++++++++\n convert.c                        | 190 ++++++++++++++++++++++++++++++++++++-\n convert.h                        |   1 +\n sha1_file.c                      |   2 +-\n t/t0028-working-tree-encoding.sh | 196 +++++++++++++++++++++++++++++++++++++++\n 5 files changed, 447 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..a8dbf4be30 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -272,6 +272,66 @@ few exceptions.  Even though...\n   catch potential problems early, safety triggers.\n \n \n+`working-tree-encoding`\n+^^^^^^^^^^^^^^^^^^^^^^^\n+\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+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+attributes is added to Git, then Git reencodes the content from the\n+specified encoding to UTF-8 and stores the result in its internal data\n+structure (called \"the index\"). On checkout the content is encoded\n+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+- Git clients that do not support the `working-tree-encoding` attribute\n+  will checkout the respective files UTF-8 encoded and not in the\n+  expected encoding. Consequently, these files will appear different\n+  which typically causes trouble. This is in particular the case for\n+  older Git versions and alternative Git implementations such as JGit\n+  or libgit2 (as of January 2018).\n+\n+- Reencoding content to non-UTF encodings (e.g. SHIFT-JIS) can cause\n+  errors as the conversion might not be round trip safe.\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 in\n+UTF-8 encoding and if you want Git to be able to process the content as\n+text.\n+\n+Use the following attributes if your '*.txt' files are UTF-16 encoded\n+with byte order mark (BOM) and you want Git to perform automatic line\n+ending conversion based on your platform.\n+\n+------------------------\n+*.txt\t\ttext working-tree-encoding=UTF-16\n+------------------------\n+\n+Use the following attributes if your '*.txt' files are UTF-16 little\n+endian encoded without BOM and you want Git to use Windows line endings\n+in the working directory.\n+\n+------------------------\n+*.txt \t\tworking-tree-encoding=UTF-16LE text 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+\n `ident`\n ^^^^^^^\n \ndiff --git a/convert.c b/convert.c\nindex b976eb968c..0c372069b1 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,147 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n \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+{\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+\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+\tif (has_prohibited_utf_bom(enc->name, src, src_len)) {\n+\t\tconst char *error_msg = _(\n+\t\t\t\"BOM is prohibited for '%s' if encoded as %s\");\n+\t\tconst char *advise_msg = _(\n+\t\t\t\"You told Git to treat '%s' as %s. A byte order mark \"\n+\t\t\t\"(BOM) is prohibited with this encoding. Either use \"\n+\t\t\t\"%.6s as working tree encoding or remove the BOM from the \"\n+\t\t\t\"file.\");\n+\n+\t\tadvise(advise_msg, path, enc->name, 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+\n+\n+\t} else if (has_missing_utf_bom(enc->name, src, src_len)) {\n+\t\tconst char *error_msg = _(\n+\t\t\t\"BOM is required for '%s' if encoded as %s\");\n+\t\tconst char *advise_msg = _(\n+\t\t\t\"You told Git to treat '%s' as %s. A byte order mark \"\n+\t\t\t\"(BOM) is required with this encoding. Either use \"\n+\t\t\t\"%sBE/%sLE as working tree encoding or add a BOM to the \"\n+\t\t\t\"file.\");\n+\t\tadvise(advise_msg, path, enc->name, 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+\n+\tdst = reencode_string_len(src, src_len, default_encoding, enc->name,\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 (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}\n+\n+\t/*\n+\t * UTF supports lossless round tripping [1]. UTF to other encoding are\n+\t * mostly round trip safe as Unicode aims to be a superset of all other\n+\t * character encodings. However, the SHIFT-JIS (Japanese character set)\n+\t * is an exception as some codes are not round trip safe [2].\n+\t *\n+\t * Reverse the transformation of 'dst' and check the result with 'src'\n+\t * if content is written to Git. This ensures no information is lost\n+\t * during conversion to/from UTF-8.\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 iconv error.\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 ((conv_flags & CONV_WRITE_OBJECT) && !strcmp(enc->name, \"SHIFT-JIS\")) {\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     &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\tif (conv_flags & CONV_WRITE_OBJECT)\n+\t\t\t\tdie(msg, path, enc->name, default_encoding);\n+\t\t\telse\n+\t\t\t\terror(msg, path, enc->name, 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+}\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+{\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->name, 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\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 +1120,31 @@ 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+{\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(struct convert_driver));\n+\tenc->name = xstrdup_toupper(value);  /* aways use upper case names! */\n+\t*encoding_tail = enc;\n+\tencoding_tail = &(enc->next);\n+\n+\treturn enc;\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 +1200,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 *checkout_encoding; /* Supported encoding or default encoding if NULL */\n };\n \n static void convert_attrs(struct conv_attrs *ca, const char *path)\n@@ -1041,8 +1209,10 @@ 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\tencoding_tail = &encoding;\n \t\tgit_config(read_convert_config, NULL);\n \t}\n \n@@ -1064,6 +1234,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->checkout_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 +1315,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.checkout_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 +1345,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.checkout_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 +1377,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.checkout_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 +1849,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.checkout_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..4d85b42776\n--- /dev/null\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -0,0 +1,196 @@\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 repo' '\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+\tcp test.utf16.raw test.utf16 &&\n+\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+\trm test.utf8.raw test.utf16.git\n+'\n+\n+test_expect_success 're-encode to UTF-16 on checkout' '\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+'\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+\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+\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+\n+test_expect_success 'check required UTF BOM' '\n+\techo \"*.utf32 text working-tree-encoding=utf-32\" >>.gitattributes &&\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+\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+\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+\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+\n+\t# cleanup\n+\trm nobom.utf16 nobom.utf32 &&\n+\tgit reset --hard HEAD\n+'\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+\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+\n+\tgit add eol.utf16 &&\n+\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+\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+\n+\trm crlf.utf16.raw crlf.utf8.raw lf.utf16.raw lf.utf8.raw &&\n+\n+\t# cleanup\n+\tgit reset --hard HEAD^\n+'\n+\n+test_expect_success 'check unsupported encodings' '\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+\t# cleanup\n+\trm err.out &&\n+\tgit reset --hard HEAD\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+\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+'\n+\n+test_expect_success 'error if encoding garbage is already in Git' '\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 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+\t# cleanup\n+\trm err.out &&\n+\tgit reset --hard $BEFORE_STATE\n+'\n+\n+test_done\n-- \n2.16.0\n\n"},{"id":"336989","messageId":"20180120152418.52859-7-lars.schneider@autodesk.com","threadId":"47654","inReplyTo":"20180120152418.52859-1-lars.schneider@autodesk.com","subject":"[PATCH v4 6/6] convert: add tracing for 'working-tree-encoding' attribute","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-01-20T15:24:18Z","receivedAt":"2018-01-20T15:25:14Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\nAdd the GIT_TRACE_CHECKOUT_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 <larsxschneider@gmail.com>\n---\n convert.c                        | 28 ++++++++++++++++++++++++++++\n t/t0028-working-tree-encoding.sh |  2 ++\n 2 files changed, 30 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex 0c372069b1..13fad490ce 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -266,6 +266,29 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n \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(CHECKOUT_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 struct encoding {\n \tconst char *name;\n \tstruct encoding *next;\n@@ -325,6 +348,7 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \t\t\terror(error_msg, path, enc->name);\n \t}\n \n+\ttrace_encoding(\"source\", path, enc->name, src, src_len);\n \tdst = reencode_string_len(src, src_len, default_encoding, enc->name,\n \t\t\t\t  &dst_len);\n \tif (!dst) {\n@@ -340,6 +364,7 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \t\telse\n \t\t\terror(msg, path, enc->name, default_encoding);\n \t}\n+\ttrace_encoding(\"destination\", path, default_encoding, dst, dst_len);\n \n \t/*\n \t * UTF supports lossless round tripping [1]. UTF to other encoding are\n@@ -365,6 +390,9 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \t\t\t\t\t     enc->name, default_encoding,\n \t\t\t\t\t     &re_src_len);\n \n+\t\ttrace_encoding(\"reencoded source\", path, enc->name,\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 \"\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex 4d85b42776..0f36d4990a 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_CHECKOUT_ENCODING=1 && export GIT_TRACE_CHECKOUT_ENCODING\n+\n test_expect_success 'setup test repo' '\n \tgit config core.eol lf &&\n \n-- \n2.16.0\n\n"},{"id":"337029","messageId":"20180121142222.GA10248@ruderich.org","threadId":"47654","inReplyTo":"20180120152418.52859-6-lars.schneider@autodesk.com","subject":"Re: [PATCH v4 5/6] convert: add 'working-tree-encoding' attribute","fromName":"Simon Ruderich","fromEmail":"simon@ruderich.org","sentAt":"2018-01-21T14:22:22Z","receivedAt":"2018-01-21T14:22:29Z","isPatch":true,"sender":{"key":"simon@ruderich.org","avatar":"https://avatars.githubusercontent.com/u/390994?v=4"},"body":"On Sat, Jan 20, 2018 at 04:24:17PM +0100, lars.schneider@autodesk.com wrote:\n> +static struct encoding *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(struct convert_driver));\n\nI think this should be \"sizeof(struct encoding)\" but I prefer\n\"sizeof(*enc)\" which prevents these kind of mistakes.\n\n> +\tenc->name = xstrdup_toupper(value);  /* aways use upper case names! */\n\n\"aways\" -> \"always\" and I think the comment should say why\nuppercase is important.\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> +\trm test.utf8.raw test.utf16.git\n> +'\n> +\n> +test_expect_success 're-encode to UTF-16 on checkout' '\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\nMicro-nit: For consistency with the previous test, remove the\nempty line and comment (or just keep the files generated from the\n\"setup test repo\" phase and don't explicitly delete them)?\n\nRegards\nSimon\n-- \n+ privacy is necessary\n+ using gnupg http://gnupg.org\n+ public key id: 0x92FEFDB7E44C32F9\n"},{"id":"337080","messageId":"05265803-BD74-4667-ABB5-9752E55A5015@gmail.com","threadId":"47654","inReplyTo":"20180121142222.GA10248@ruderich.org","subject":"Re: [PATCH v4 5/6] convert: add 'working-tree-encoding' attribute","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-01-22T12:35:25Z","receivedAt":"2018-01-22T12:35:33Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 21 Jan 2018, at 15:22, Simon Ruderich <simon@ruderich.org> wrote:\n> \n> On Sat, Jan 20, 2018 at 04:24:17PM +0100, lars.schneider@autodesk.com wrote:\n>> +static struct encoding *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(struct convert_driver));\n> \n> I think this should be \"sizeof(struct encoding)\" but I prefer\n> \"sizeof(*enc)\" which prevents these kind of mistakes.\n\nGreat catch! Thank you!\n\nOther code paths are at risk of this problem too. Consider this:\n\n$ git grep 'sizeof(\\*' | wc -l\n     303\n$ git grep 'sizeof(struct ' | wc -l\n     208\n\nE.g. even in the same file (likely where I got the code from):\nhttps://github.com/git/git/blob/59c276cf4da0705064c32c9dba54baefa282ea55/convert.c#L780\n\n@Junio: \nWould you welcome a patch that replaces \"struct foo\" with \"*foo\"\nif applicable?\n\n\n>> +\tenc->name = xstrdup_toupper(value);  /* aways use upper case names! */\n> \n> \"aways\" -> \"always\" and I think the comment should say why\n> uppercase is important.\n\nWould that be better?\n\n\t/* Aways use upper case names to simplify subsequent string comparison. */\n\tenc->name = xstrdup_toupper(value);\n\nAFAIK uppercase and lowercase names are both valid. I just wanted to\nensure that we use one consistent casing. That reads better in error messages\nand I don't need to check for the letter case in has_prohibited_utf_bom()\nand friends in utf8.c\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>> +\trm test.utf8.raw test.utf16.git\n>> +'\n>> +\n>> +test_expect_success 're-encode to UTF-16 on checkout' '\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> \n> Micro-nit: For consistency with the previous test, remove the\n> empty line and comment (or just keep the files generated from the\n> \"setup test repo\" phase and don't explicitly delete them)?\n\nI would rather add a new line and a comment to the previous test \nto be consistent.\n\nI know we could leave the files but these lingering files could\nalways surprise writers of future tests (at least they surprised\nme in other tests).\n\n\nThank you very much for the review,\nLars"},{"id":"337095","messageId":"20180122180042.70101-1-lars.schneider@autodesk.com","threadId":"47654","inReplyTo":"20180120152418.52859-6-lars.schneider@autodesk.com","subject":"SQUASH convert: add tracing for 'working-tree-encoding' attribute","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-01-22T18:00:42Z","receivedAt":"2018-01-22T18:01:14Z","isPatch":false,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\nHi Junio,\n\nthis attached patch addresses Simon's review comments.\n\nCan you squash the patch if you apply \"[PATCH v4 5/6] convert: add\n'working-tree-encoding' attribute\"?\n\nhttps://public-inbox.org/git/20180120152418.52859-6-lars.schneider@autodesk.com/\n\nThanks,\nLars\n\n\n convert.c                        | 5 +++--\n t/t0028-working-tree-encoding.sh | 2 ++\n 2 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 13fad490ce..f5e0cfc352 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1165,8 +1165,9 @@ static struct encoding *git_path_check_encoding(struct attr_check_item *check)\n \tif (!strcasecmp(value, default_encoding))\n \t\treturn NULL;\n\n-\tenc = xcalloc(1, sizeof(struct convert_driver));\n-\tenc->name = xstrdup_toupper(value);  /* aways use upper case names! */\n+\tenc = xcalloc(1, sizeof(*enc));\n+\t/* Aways use upper case names to simplify subsequent string comparison. */\n+\tenc->name = xstrdup_toupper(value);\n \t*encoding_tail = enc;\n \tencoding_tail = &(enc->next);\n\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex 0f36d4990a..a6da61280d 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -22,6 +22,8 @@ test_expect_success 'setup test repo' '\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+\n+\t# cleanup\n \trm test.utf8.raw test.utf16.git\n '\n\n\nbase-commit: 21f4dac5aba07a6109285c57a0478bf502e09009\n--\n2.16.0\n\n"},{"id":"337100","messageId":"CAPig+cRMHmFuMxQmSeGoK9hhUVEVVVLQs0j10Lo8bE5t8-V9OA@mail.gmail.com","threadId":"47654","inReplyTo":"20180122180042.70101-1-lars.schneider@autodesk.com","subject":"Re: SQUASH convert: add tracing for 'working-tree-encoding' attribute","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-22T19:37:46Z","receivedAt":"2018-01-22T19:37:52Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jan 22, 2018 at 1:00 PM,  <lars.schneider@autodesk.com> wrote:\n> diff --git a/convert.c b/convert.c\n> @@ -1165,8 +1165,9 @@ static struct encoding *git_path_check_encoding(struct attr_check_item *check)\n> -       enc = xcalloc(1, sizeof(struct convert_driver));\n> -       enc->name = xstrdup_toupper(value);  /* aways use upper case names! */\n> +       enc = xcalloc(1, sizeof(*enc));\n> +       /* Aways use upper case names to simplify subsequent string comparison. */\n\ns/Aways/Always/\n\nhttps://public-inbox.org/git/20180121142222.GA10248@ruderich.org/\n\n> +       enc->name = xstrdup_toupper(value);\n>         *encoding_tail = enc;\n>         encoding_tail = &(enc->next);\n"},{"id":"337138","messageId":"20180123005401.GG26357@sigill.intra.peff.net","threadId":"47654","inReplyTo":"05265803-BD74-4667-ABB5-9752E55A5015@gmail.com","subject":"Re: [PATCH v4 5/6] convert: add 'working-tree-encoding' attribute","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-23T00:54:01Z","receivedAt":"2018-01-23T00:54:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 22, 2018 at 01:35:25PM +0100, Lars Schneider wrote:\n\n> >> +\tenc = xcalloc(1, sizeof(struct convert_driver));\n> > \n> > I think this should be \"sizeof(struct encoding)\" but I prefer\n> > \"sizeof(*enc)\" which prevents these kind of mistakes.\n> \n> Great catch! Thank you!\n> \n> Other code paths are at risk of this problem too. Consider this:\n> \n> $ git grep 'sizeof(\\*' | wc -l\n>      303\n> $ git grep 'sizeof(struct ' | wc -l\n>      208\n> \n> E.g. even in the same file (likely where I got the code from):\n> https://github.com/git/git/blob/59c276cf4da0705064c32c9dba54baefa282ea55/convert.c#L780\n> \n> @Junio: \n> Would you welcome a patch that replaces \"struct foo\" with \"*foo\"\n> if applicable?\n\nThis is part of the reason we've been moving to helpers like\nALLOC_ARRAY(), which make it harder to get this wrong.\n\nWe don't have an ALLOC_OBJECT(), which is what you would want here. I'm\nnot sure if that is helpful or crossing the line of \"you're obscuring it\nto the point that people familiar with C have trouble reading the code\".\nThe ALLOC_ARRAY() macros have been sort of an experiment there (I tend\nto like them, but I also work with Git's code often enough that I am not\nlikely to be confused by our bespoke macros).\n\nBut anyway, that was a bit of a tangent. Certainly the smaller change is\njust standardizing on sizeof(*foo), which I think most people agree on\nat this point. It might be worth putting in CodingGuidelines.\n\n-Peff\n"},{"id":"337148","messageId":"20180123092749.GA6308@ruderich.org","threadId":"47654","inReplyTo":"05265803-BD74-4667-ABB5-9752E55A5015@gmail.com","subject":"Re: [PATCH v4 5/6] convert: add 'working-tree-encoding' attribute","fromName":"Simon Ruderich","fromEmail":"simon@ruderich.org","sentAt":"2018-01-23T09:27:49Z","receivedAt":"2018-01-23T09:27:56Z","isPatch":true,"sender":{"key":"simon@ruderich.org","avatar":"https://avatars.githubusercontent.com/u/390994?v=4"},"body":"On Mon, Jan 22, 2018 at 01:35:25PM +0100, Lars Schneider wrote:\n>>> +\tenc->name = xstrdup_toupper(value);  /* aways use upper case names! */\n>>\n>> \"aways\" -> \"always\" and I think the comment should say why\n>> uppercase is important.\n>\n> Would that be better?\n>\n> \t/* Aways use upper case names to simplify subsequent string comparison. */\n> \tenc->name = xstrdup_toupper(value);\n>\n> AFAIK uppercase and lowercase names are both valid. I just wanted to\n> ensure that we use one consistent casing. That reads better in error messages\n> and I don't need to check for the letter case in has_prohibited_utf_bom()\n> and friends in utf8.c\n\nSounds good (minus the \"Aways\" typo Eric already noticed).\n\n>> Micro-nit: For consistency with the previous test, remove the\n>> empty line and comment (or just keep the files generated from the\n>> \"setup test repo\" phase and don't explicitly delete them)?\n>\n> I would rather add a new line and a comment to the previous test\n> to be consistent.\n>\n> I know we could leave the files but these lingering files could\n> always surprise writers of future tests (at least they surprised\n> me in other tests).\n\nSure, that sounds good. Just noticed the inconsistency and wanted\nto mention it.\n\nRegards\nSimon\n-- \n+ privacy is necessary\n+ using gnupg http://gnupg.org\n+ public key id: 0x92FEFDB7E44C32F9\n"},{"id":"337150","messageId":"20180123101739.72232-1-lars.schneider@autodesk.com","threadId":"47654","inReplyTo":"20180120152418.52859-6-lars.schneider@autodesk.com","subject":"[PATCH v2] SQUASH convert: add tracing for 'working-tree-encoding' attribute","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-01-23T10:17:39Z","receivedAt":"2018-01-23T10:19:13Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\nHi Junio,\n\nI overlooked a typo pointed out in Simon's review. Here is a new patch\nfor squashing. Sorry for the trouble!\n\n@Eric: Thanks for spotting this!\n\nCheers,\nLars\n\n\n convert.c                        | 8 ++++++--\n t/t0028-working-tree-encoding.sh | 2 ++\n 2 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 13fad490ce..dce2f6e201 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1165,8 +1165,12 @@ static struct encoding *git_path_check_encoding(struct attr_check_item *check)\n \tif (!strcasecmp(value, default_encoding))\n \t\treturn NULL;\n\n-\tenc = xcalloc(1, sizeof(struct convert_driver));\n-\tenc->name = xstrdup_toupper(value);  /* aways use upper case names! */\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\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex 0f36d4990a..a6da61280d 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -22,6 +22,8 @@ test_expect_success 'setup test repo' '\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+\n+\t# cleanup\n \trm test.utf8.raw test.utf16.git\n '\n\n\nbase-commit: 21f4dac5aba07a6109285c57a0478bf502e09009\n--\n2.16.0\n\n"},{"id":"337154","messageId":"20180123102558.GA3878@ruderich.org","threadId":"47654","inReplyTo":"20180123005401.GG26357@sigill.intra.peff.net","subject":"Re: [PATCH v4 5/6] convert: add 'working-tree-encoding' attribute","fromName":"Simon Ruderich","fromEmail":"simon@ruderich.org","sentAt":"2018-01-23T10:25:58Z","receivedAt":"2018-01-23T10:26:04Z","isPatch":true,"sender":{"key":"simon@ruderich.org","avatar":"https://avatars.githubusercontent.com/u/390994?v=4"},"body":"On Mon, Jan 22, 2018 at 07:54:01PM -0500, Jeff King wrote:\n> But anyway, that was a bit of a tangent. Certainly the smaller change is\n> just standardizing on sizeof(*foo), which I think most people agree on\n> at this point. It might be worth putting in CodingGuidelines.\n\nPersonally I prefer sizeof(*foo) which is a well non-idiom, used\nin many projects and IMHO easy to read and understand.\n\nI've played a little with coccinelle and the following spatch\nseems to catch many occurrences of sizeof(struct ..) (the first\nhunk seems to expand multiple times causing conflicts in the\ngenerated patch). Cases like a->b = xcalloc() are not matched, I\ndon't know enough coccinelle for that. If there's interest I\ncould prepare patches, but it will create quite some code churn.\n\nRegards\nSimon\n\n@@\ntype T;\nidentifier x;\n@@\n- T *x = xmalloc(sizeof(T));\n+ T *x = xmalloc(sizeof(*x));\n\n@@\ntype T;\nT *x;\n@@\n- x = xmalloc(sizeof(T));\n+ x = xmalloc(sizeof(*x));\n\n\n@@\ntype T;\nidentifier x;\nexpression n;\n@@\n- T *x = xcalloc(n, sizeof(T));\n+ T *x = xcalloc(n, sizeof(*x));\n\n@@\ntype T;\nT *x;\nexpression n;\n@@\n- x = xcalloc(n, sizeof(T));\n+ x = xcalloc(n, sizeof(*x));\n\n\n@@\ntype T;\nT x;\n@@\n- memset(&x, 0, sizeof(T));\n+ memset(&x, 0, sizeof(x));\n\n@@\ntype T;\nT *x;\n@@\n- memset(x, 0, sizeof(T));\n+ memset(x, 0, sizeof(*x));\n\n-- \n+ privacy is necessary\n+ using gnupg http://gnupg.org\n+ public key id: 0x92FEFDB7E44C32F9\n"},{"id":"337157","messageId":"20180123162016.GD13068@sigill.intra.peff.net","threadId":"47654","inReplyTo":"20180123102558.GA3878@ruderich.org","subject":"Re: [PATCH v4 5/6] convert: add 'working-tree-encoding' attribute","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-23T16:20:16Z","receivedAt":"2018-01-23T16:20:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 23, 2018 at 11:25:58AM +0100, Simon Ruderich wrote:\n\n> On Mon, Jan 22, 2018 at 07:54:01PM -0500, Jeff King wrote:\n> > But anyway, that was a bit of a tangent. Certainly the smaller change is\n> > just standardizing on sizeof(*foo), which I think most people agree on\n> > at this point. It might be worth putting in CodingGuidelines.\n> \n> Personally I prefer sizeof(*foo) which is a well non-idiom, used\n> in many projects and IMHO easy to read and understand.\n\nMe too.\n\n> I've played a little with coccinelle and the following spatch\n> seems to catch many occurrences of sizeof(struct ..) (the first\n> hunk seems to expand multiple times causing conflicts in the\n> generated patch). Cases like a->b = xcalloc() are not matched, I\n> don't know enough coccinelle for that. If there's interest I\n> could prepare patches, but it will create quite some code churn.\n\nYeah, I'm not sure what our current policy is there. Traditionally our\nstrategy was not to churn code, but to update old idioms as they were\ntouched. Especially if the change was not urgent, but mostly stylistic\n(which this one is).\n\nBut with Coccinelle, it's a lot easier to apply the change tree-wide, and\nto convert topics in flight as they get merged. The maintainer still\ngets conflicts with topics-in-flight that touch converted areas, though.\nSo I'd be curious to hear if Junio's opinion has changed at all.\n\n-Peff\n"},{"id":"337178","messageId":"20180123201930.GA23019@tor.lan","threadId":"47654","inReplyTo":"20180120152418.52859-1-lars.schneider@autodesk.com","subject":"Re: [PATCH v4 0/6] convert: add support for different encodings","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-01-23T20:19:30Z","receivedAt":"2018-01-23T20:19:55Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Sat, Jan 20, 2018 at 04:24:12PM +0100, lars.schneider@autodesk.com wrote:\n> From: Lars Schneider <larsxschneider@gmail.com>\n> \n> Hi,\n> \n> Patches 1-4 and 6 are preparation and helper functions.\n> Patch 5 is the actual change.\n\nI (still) have 2 remarks on convert.c - to make live easier,\nI will send a small \"on top\" patch the next days.\n"},{"id":"337183","messageId":"xmqqshawfgaa.fsf@gitster.mtv.corp.google.com","threadId":"47654","inReplyTo":"20180123201930.GA23019@tor.lan","subject":"Re: [PATCH v4 0/6] convert: add support for different encodings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-23T20:53:33Z","receivedAt":"2018-01-23T20:53:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> On Sat, Jan 20, 2018 at 04:24:12PM +0100, lars.schneider@autodesk.com wrote:\n>> From: Lars Schneider <larsxschneider@gmail.com>\n>> \n>> Hi,\n>> \n>> Patches 1-4 and 6 are preparation and helper functions.\n>> Patch 5 is the actual change.\n>\n> I (still) have 2 remarks on convert.c - to make live easier,\n> I will send a small \"on top\" patch the next days.\n\nThanks, both.  I'll stay on the sideline ;-) and deal with other\ntopics first.\n"},{"id":"337184","messageId":"xmqqo9lkffe0.fsf@gitster.mtv.corp.google.com","threadId":"47654","inReplyTo":"20180123162016.GD13068@sigill.intra.peff.net","subject":"Re: [PATCH v4 5/6] convert: add 'working-tree-encoding' attribute","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-23T21:12:55Z","receivedAt":"2018-01-23T21:13:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> But with Coccinelle, it's a lot easier to apply the change tree-wide, and\n> to convert topics in flight as they get merged. The maintainer still\n> gets conflicts with topics-in-flight that touch converted areas, though.\n> So I'd be curious to hear if Junio's opinion has changed at all.\n\nThere are two distinct kinds of cost on such a tree-wide change.\nConflicts with in-flight topic cannot be avoided other than truly\navoiding, i.e. refraining from touching the areas in flux, but it is\nprimarily what the maintainer does, and with help with rerere it can\nbe reasonably well automated ;-)\n\nBut the cost of reviewing could become a lot smaller when our tools\nare trustworthy.  As long as we can be reasonably certain that the\ntree-wide change patch does one thing it is intended to do and\nnothing else (e.g. comes with mechanical reproduction recipe that\nallows the patch to be independently audited), I do not have much\nproblem with such a clean-up.\n\nThe \"avoid tree-wide change\" rule still applies for things that\nallows a lot of subjective judgment and discretion.  I do not know\nof a good way to reduce reviewer costs on those kind of changes.\n\nThanks.\n"},{"id":"337684","messageId":"20180129201855.9182-1-tboegi@web.de","threadId":"47654","inReplyTo":"xmqqshawfgaa.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v5 0/7] convert: add support for different encodings","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2018-01-29T20:18:55Z","receivedAt":"2018-01-29T20:19:25Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen <tboegi@web.de>\n\nTake V4 from Lars, manually integrated the V2 squash patch,\nso a review would be good.\n\nAdd my \"comments\" as a patch, see 7/7 (and this is more like an RFC)\n\nThis needs to go on top of tb/crlf-conv-flags\n\n\nLars Schneider (6):\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: add tracing for 'working-tree-encoding' attribute\n\nTorsten Bögershausen (1):\n  Careful with CRLF when using e.g. UTF-16 for working-tree-encoding\n\n Documentation/gitattributes.txt  |  63 +++++++++++\n convert.c                        | 233 ++++++++++++++++++++++++++++++++++++++-\n convert.h                        |   1 +\n sha1_file.c                      |   2 +-\n strbuf.c                         |  13 ++-\n strbuf.h                         |   1 +\n t/t0028-working-tree-encoding.sh | 198 +++++++++++++++++++++++++++++++++\n utf8.c                           |  37 +++++++\n utf8.h                           |  25 +++++\n 9 files changed, 567 insertions(+), 6 deletions(-)\n create mode 100755 t/t0028-working-tree-encoding.sh\n\n-- \n2.16.0.rc0.2.g64d3e4d0cc.dirty\n\n"},{"id":"337685","messageId":"20180129201901.9269-1-tboegi@web.de","threadId":"47654","inReplyTo":"xmqqshawfgaa.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v5 2/7] strbuf: add xstrdup_toupper()","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2018-01-29T20:19:01Z","receivedAt":"2018-01-29T20:19:33Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\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 <larsxschneider@gmail.com>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\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 490f7850e..a20af696b 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 14c8c10d6..df7ced53e 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.0.rc0.2.g64d3e4d0cc.dirty\n\n"},{"id":"337686","messageId":"20180129201903.9312-1-tboegi@web.de","threadId":"47654","inReplyTo":"xmqqshawfgaa.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v5 3/7] utf8: add function to detect prohibited UTF-16/32 BOM","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2018-01-29T20:19:03Z","receivedAt":"2018-01-29T20:19:36Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\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 <larsxschneider@gmail.com>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\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 2c27ce013..914881cd1 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 6bbcf31a8..4711429af 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+ * 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+ *\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.0.rc0.2.g64d3e4d0cc.dirty\n\n"},{"id":"337687","messageId":"20180129201909.9441-1-tboegi@web.de","threadId":"47654","inReplyTo":"xmqqshawfgaa.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v5 6/7] convert: add tracing for 'working-tree-encoding' attribute","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2018-01-29T20:19:09Z","receivedAt":"2018-01-29T20:19:38Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\nAdd the GIT_TRACE_CHECKOUT_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 <larsxschneider@gmail.com>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n convert.c                        | 28 ++++++++++++++++++++++++++++\n t/t0028-working-tree-encoding.sh |  2 ++\n 2 files changed, 30 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex 0c372069b..13fad490c 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -266,6 +266,29 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n \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(CHECKOUT_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 struct encoding {\n \tconst char *name;\n \tstruct encoding *next;\n@@ -325,6 +348,7 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \t\t\terror(error_msg, path, enc->name);\n \t}\n \n+\ttrace_encoding(\"source\", path, enc->name, src, src_len);\n \tdst = reencode_string_len(src, src_len, default_encoding, enc->name,\n \t\t\t\t  &dst_len);\n \tif (!dst) {\n@@ -340,6 +364,7 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \t\telse\n \t\t\terror(msg, path, enc->name, default_encoding);\n \t}\n+\ttrace_encoding(\"destination\", path, default_encoding, dst, dst_len);\n \n \t/*\n \t * UTF supports lossless round tripping [1]. UTF to other encoding are\n@@ -365,6 +390,9 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \t\t\t\t\t     enc->name, default_encoding,\n \t\t\t\t\t     &re_src_len);\n \n+\t\ttrace_encoding(\"reencoded source\", path, enc->name,\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 \"\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex 4d85b4277..0f36d4990 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_CHECKOUT_ENCODING=1 && export GIT_TRACE_CHECKOUT_ENCODING\n+\n test_expect_success 'setup test repo' '\n \tgit config core.eol lf &&\n \n-- \n2.16.0.rc0.2.g64d3e4d0cc.dirty\n\n"},{"id":"337688","messageId":"20180129201905.9355-1-tboegi@web.de","threadId":"47654","inReplyTo":"xmqqshawfgaa.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v5 4/7] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2018-01-29T20:19:05Z","receivedAt":"2018-01-29T20:19:40Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\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\nhas_missing_utf_bom() function returns true if a required BOM is\nmissing.\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 <larsxschneider@gmail.com>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n utf8.c | 13 +++++++++++++\n utf8.h | 16 ++++++++++++++++\n 2 files changed, 29 insertions(+)\n\ndiff --git a/utf8.c b/utf8.c\nindex 914881cd1..f033fec1c 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 has_missing_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 4711429af..26b5e9185 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -79,4 +79,20 @@ 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\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+ *\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 has_missing_utf_bom(const char *enc, const char *data, size_t len);\n+\n #endif\n-- \n2.16.0.rc0.2.g64d3e4d0cc.dirty\n\n"},{"id":"337689","messageId":"20180129201908.9398-1-tboegi@web.de","threadId":"47654","inReplyTo":"xmqqshawfgaa.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v5 5/7] convert: add 'working-tree-encoding' attribute","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2018-01-29T20:19:08Z","receivedAt":"2018-01-29T20:19:42Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\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 <larsxschneider@gmail.com>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n Documentation/gitattributes.txt  |  60 ++++++++++++\n convert.c                        | 190 ++++++++++++++++++++++++++++++++++++-\n convert.h                        |   1 +\n sha1_file.c                      |   2 +-\n t/t0028-working-tree-encoding.sh | 196 +++++++++++++++++++++++++++++++++++++++\n 5 files changed, 447 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 30687de81..a8dbf4be3 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -272,6 +272,66 @@ few exceptions.  Even though...\n   catch potential problems early, safety triggers.\n \n \n+`working-tree-encoding`\n+^^^^^^^^^^^^^^^^^^^^^^^\n+\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+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+attributes is added to Git, then Git reencodes the content from the\n+specified encoding to UTF-8 and stores the result in its internal data\n+structure (called \"the index\"). On checkout the content is encoded\n+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+- Git clients that do not support the `working-tree-encoding` attribute\n+  will checkout the respective files UTF-8 encoded and not in the\n+  expected encoding. Consequently, these files will appear different\n+  which typically causes trouble. This is in particular the case for\n+  older Git versions and alternative Git implementations such as JGit\n+  or libgit2 (as of January 2018).\n+\n+- Reencoding content to non-UTF encodings (e.g. SHIFT-JIS) can cause\n+  errors as the conversion might not be round trip safe.\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 in\n+UTF-8 encoding and if you want Git to be able to process the content as\n+text.\n+\n+Use the following attributes if your '*.txt' files are UTF-16 encoded\n+with byte order mark (BOM) and you want Git to perform automatic line\n+ending conversion based on your platform.\n+\n+------------------------\n+*.txt\t\ttext working-tree-encoding=UTF-16\n+------------------------\n+\n+Use the following attributes if your '*.txt' files are UTF-16 little\n+endian encoded without BOM and you want Git to use Windows line endings\n+in the working directory.\n+\n+------------------------\n+*.txt \t\tworking-tree-encoding=UTF-16LE text 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+\n `ident`\n ^^^^^^^\n \ndiff --git a/convert.c b/convert.c\nindex b976eb968..0c372069b 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,147 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n \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+{\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+\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+\tif (has_prohibited_utf_bom(enc->name, src, src_len)) {\n+\t\tconst char *error_msg = _(\n+\t\t\t\"BOM is prohibited for '%s' if encoded as %s\");\n+\t\tconst char *advise_msg = _(\n+\t\t\t\"You told Git to treat '%s' as %s. A byte order mark \"\n+\t\t\t\"(BOM) is prohibited with this encoding. Either use \"\n+\t\t\t\"%.6s as working tree encoding or remove the BOM from the \"\n+\t\t\t\"file.\");\n+\n+\t\tadvise(advise_msg, path, enc->name, 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+\n+\n+\t} else if (has_missing_utf_bom(enc->name, src, src_len)) {\n+\t\tconst char *error_msg = _(\n+\t\t\t\"BOM is required for '%s' if encoded as %s\");\n+\t\tconst char *advise_msg = _(\n+\t\t\t\"You told Git to treat '%s' as %s. A byte order mark \"\n+\t\t\t\"(BOM) is required with this encoding. Either use \"\n+\t\t\t\"%sBE/%sLE as working tree encoding or add a BOM to the \"\n+\t\t\t\"file.\");\n+\t\tadvise(advise_msg, path, enc->name, 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+\n+\tdst = reencode_string_len(src, src_len, default_encoding, enc->name,\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 (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}\n+\n+\t/*\n+\t * UTF supports lossless round tripping [1]. UTF to other encoding are\n+\t * mostly round trip safe as Unicode aims to be a superset of all other\n+\t * character encodings. However, the SHIFT-JIS (Japanese character set)\n+\t * is an exception as some codes are not round trip safe [2].\n+\t *\n+\t * Reverse the transformation of 'dst' and check the result with 'src'\n+\t * if content is written to Git. This ensures no information is lost\n+\t * during conversion to/from UTF-8.\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 iconv error.\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 ((conv_flags & CONV_WRITE_OBJECT) && !strcmp(enc->name, \"SHIFT-JIS\")) {\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     &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\tif (conv_flags & CONV_WRITE_OBJECT)\n+\t\t\t\tdie(msg, path, enc->name, default_encoding);\n+\t\t\telse\n+\t\t\t\terror(msg, path, enc->name, 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+}\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+{\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->name, 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\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 +1120,31 @@ 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+{\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(struct convert_driver));\n+\tenc->name = xstrdup_toupper(value);  /* aways use upper case names! */\n+\t*encoding_tail = enc;\n+\tencoding_tail = &(enc->next);\n+\n+\treturn enc;\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 +1200,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 *checkout_encoding; /* Supported encoding or default encoding if NULL */\n };\n \n static void convert_attrs(struct conv_attrs *ca, const char *path)\n@@ -1041,8 +1209,10 @@ 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\tencoding_tail = &encoding;\n \t\tgit_config(read_convert_config, NULL);\n \t}\n \n@@ -1064,6 +1234,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->checkout_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 +1315,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.checkout_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 +1345,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.checkout_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 +1377,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.checkout_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 +1849,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.checkout_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 65ab3e516..1d9539ed0 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 6bc7c6ada..e2f319d67 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 000000000..4d85b4277\n--- /dev/null\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -0,0 +1,196 @@\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 repo' '\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+\tcp test.utf16.raw test.utf16 &&\n+\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+\trm test.utf8.raw test.utf16.git\n+'\n+\n+test_expect_success 're-encode to UTF-16 on checkout' '\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+'\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+\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+\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+\n+test_expect_success 'check required UTF BOM' '\n+\techo \"*.utf32 text working-tree-encoding=utf-32\" >>.gitattributes &&\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+\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+\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+\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+\n+\t# cleanup\n+\trm nobom.utf16 nobom.utf32 &&\n+\tgit reset --hard HEAD\n+'\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+\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+\n+\tgit add eol.utf16 &&\n+\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+\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+\n+\trm crlf.utf16.raw crlf.utf8.raw lf.utf16.raw lf.utf8.raw &&\n+\n+\t# cleanup\n+\tgit reset --hard HEAD^\n+'\n+\n+test_expect_success 'check unsupported encodings' '\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+\t# cleanup\n+\trm err.out &&\n+\tgit reset --hard HEAD\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+\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+'\n+\n+test_expect_success 'error if encoding garbage is already in Git' '\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 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+\t# cleanup\n+\trm err.out &&\n+\tgit reset --hard $BEFORE_STATE\n+'\n+\n+test_done\n-- \n2.16.0.rc0.2.g64d3e4d0cc.dirty\n\n"},{"id":"337690","messageId":"20180129201911.9484-1-tboegi@web.de","threadId":"47654","inReplyTo":"xmqqshawfgaa.fsf@gitster.mtv.corp.google.com","subject":"[PATCH/RFC v5 7/7] Careful with CRLF when using e.g. UTF-16 for working-tree-encoding","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2018-01-29T20:19:11Z","receivedAt":"2018-01-29T20:19:43Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen <tboegi@web.de>\n\nUTF-16 encoded files are treated as \"binary\" by Git, and no CRLF\nconversion is done.\nWhen the UTF-16 encoded files are converted into UF-8 using the new\n\"working-tree-encoding\", the CRLF are converted if core.autocrlf is true.\n\nThis may lead to confusion:\nA tool writes an UTF-16 encoded file with CRLF.\nThe file is commited with core.autocrlf=true, the CLRF are converted into LF.\nThe repo is pushed somewhere and cloned by a different user, who has\ndecided to use core.autocrlf=false.\nHe uses the same tool, and now the CRLF are not there as expected, but LF,\nmake the file useless for the tool.\n\nAvoid this (possible) confusion by ignoring core.autocrlf for all files\nwhich have \"working-tree-encoding\" defined.\n\nThe user can still use a .gitattributes file and specify the line endings\nlike \"text=auto\", \"text\", or \"text eol=crlf\" and let that .gitattribute\nfile travel together with push and clone.\n\nChange convert.c to e more careful, simplify the initialization when\nattributes are retrived (and none are specified) and update the documentation.\n\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n Documentation/gitattributes.txt |  9 ++++++---\n convert.c                       | 15 ++++++++++++---\n 2 files changed, 18 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex a8dbf4be3..3665c4677 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -308,12 +308,15 @@ Use the `working-tree-encoding` attribute only if you cannot store a file in\n UTF-8 encoding and if you want Git to be able to process the content as\n text.\n \n+Note that when `working-tree-encoding` is defined, core.autocrlf is ignored.\n+Set the `text` attribute (or `text=auto`) to enable CRLF conversions.\n+\n Use the following attributes if your '*.txt' files are UTF-16 encoded\n-with byte order mark (BOM) and you want Git to perform automatic line\n-ending conversion based on your platform.\n+with byte order mark (BOM) and you want Git to perform line\n+ending conversion based on core.eol.\n \n ------------------------\n-*.txt\t\ttext working-tree-encoding=UTF-16\n+*.txt\t\tworking-tree-encoding=UTF-16 text\n ------------------------\n \n Use the following attributes if your '*.txt' files are UTF-16 little\ndiff --git a/convert.c b/convert.c\nindex 13fad490c..e7f11d1db 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1264,15 +1264,24 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)\n \t\t}\n \t\tca->checkout_encoding = git_path_check_encoding(ccheck + 5);\n \t} else {\n-\t\tca->drv = NULL;\n-\t\tca->crlf_action = CRLF_UNDEFINED;\n-\t\tca->ident = 0;\n+\t\tmemset(ca, 0, sizeof(*ca));\n \t}\n \n \t/* Save attr and make a decision for action */\n \tca->attr_action = ca->crlf_action;\n \tif (ca->crlf_action == CRLF_TEXT)\n \t\tca->crlf_action = text_eol_is_crlf() ? CRLF_TEXT_CRLF : CRLF_TEXT_INPUT;\n+\t/*\n+\t * Often UTF-16 encoded files are read and written by programs which\n+\t * really need CRLF, and it is important to keep the CRLF \"as is\" when\n+\t * files are committed with core.autocrlf=true and the repo is pushed.\n+\t * The CRLF would be converted into LF when the repo is cloned to\n+\t * a machine with core.autocrlf=false.\n+\t * Obey the \"text\" and \"eol\" attributes and be independent on the\n+\t * local core.autocrlf for all \"encoded\" files.\n+\t */\n+\tif ((ca->crlf_action == CRLF_UNDEFINED) && ca->checkout_encoding)\n+\t\tca->crlf_action = CRLF_BINARY;\n \tif (ca->crlf_action == CRLF_UNDEFINED && auto_crlf == AUTO_CRLF_FALSE)\n \t\tca->crlf_action = CRLF_BINARY;\n \tif (ca->crlf_action == CRLF_UNDEFINED && auto_crlf == AUTO_CRLF_TRUE)\n-- \n2.16.0.rc0.2.g64d3e4d0cc.dirty\n\n"},{"id":"337691","messageId":"20180129201859.9226-1-tboegi@web.de","threadId":"47654","inReplyTo":"xmqqshawfgaa.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v5 1/7] strbuf: remove unnecessary NUL assignment in xstrdup_tolower()","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2018-01-29T20:18:59Z","receivedAt":"2018-01-29T20:19:46Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\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 <larsxschneider@gmail.com>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n strbuf.c | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 8007be8fb..490f7850e 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.0.rc0.2.g64d3e4d0cc.dirty\n\n"},{"id":"337783","messageId":"55B6C3D5-4131-4636-AD0E-20759EDBE8CD@gmail.com","threadId":"47654","inReplyTo":"20180129201911.9484-1-tboegi@web.de","subject":"Re: [PATCH/RFC v5 7/7] Careful with CRLF when using e.g. UTF-16 for working-tree-encoding","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-01-30T11:23:47Z","receivedAt":"2018-01-30T11:23:55Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 29 Jan 2018, at 21:19, tboegi@web.de wrote:\n> \n> From: Torsten Bögershausen <tboegi@web.de>\n> \n> UTF-16 encoded files are treated as \"binary\" by Git, and no CRLF\n> conversion is done.\n> When the UTF-16 encoded files are converted into UF-8 using the new\ns/UF-8/UTF-8/\n\n\n> \"working-tree-encoding\", the CRLF are converted if core.autocrlf is true.\n> \n> This may lead to confusion:\n> A tool writes an UTF-16 encoded file with CRLF.\n> The file is commited with core.autocrlf=true, the CLRF are converted into LF.\n> The repo is pushed somewhere and cloned by a different user, who has\n> decided to use core.autocrlf=false.\n> He uses the same tool, and now the CRLF are not there as expected, but LF,\n> make the file useless for the tool.\n> \n> Avoid this (possible) confusion by ignoring core.autocrlf for all files\n> which have \"working-tree-encoding\" defined.\n\nMaybe I don't understand your use case but I think this will generate even \nmore confusion because that's not what I would expect as a user. I think Git \nshould behave consistently independent of the used encoding. Here are my arguments:\n\n  (1) Legacy users are *not* affected. If you don't use the \"working-tree-encoding\"\n      attribute then nothing changes for you.\n\n  (2) If you use the \"working-tree-encoding\" attribute *and* you want to ensure \n      your file keeps CRLF then you can define that in the attributes too. E.g.:\n      \n      *.proj textworking-tree-encoding=UTF-16 eol=crlf\n\n- Lars\n\n\n\n> The user can still use a .gitattributes file and specify the line endings\n> like \"text=auto\", \"text\", or \"text eol=crlf\" and let that .gitattribute\n> file travel together with push and clone.\n> \n> Change convert.c to e more careful, simplify the initialization when\n> attributes are retrived (and none are specified) and update the documentation.\n> \n> Signed-off-by: Torsten Bögershausen <tboegi@web.de>\n> ---\n> Documentation/gitattributes.txt |  9 ++++++---\n> convert.c                       | 15 ++++++++++++---\n> 2 files changed, 18 insertions(+), 6 deletions(-)\n> \n> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\n> index a8dbf4be3..3665c4677 100644\n> --- a/Documentation/gitattributes.txt\n> +++ b/Documentation/gitattributes.txt\n> @@ -308,12 +308,15 @@ Use the `working-tree-encoding` attribute only if you cannot store a file in\n> UTF-8 encoding and if you want Git to be able to process the content as\n> text.\n> \n> +Note that when `working-tree-encoding` is defined, core.autocrlf is ignored.\n> +Set the `text` attribute (or `text=auto`) to enable CRLF conversions.\n> +\n> Use the following attributes if your '*.txt' files are UTF-16 encoded\n> -with byte order mark (BOM) and you want Git to perform automatic line\n> -ending conversion based on your platform.\n> +with byte order mark (BOM) and you want Git to perform line\n> +ending conversion based on core.eol.\n> \n> ------------------------\n> -*.txt\t\ttext working-tree-encoding=UTF-16\n> +*.txt\t\tworking-tree-encoding=UTF-16 text\n> ------------------------\n> \n> Use the following attributes if your '*.txt' files are UTF-16 little\n> diff --git a/convert.c b/convert.c\n> index 13fad490c..e7f11d1db 100644\n> --- a/convert.c\n> +++ b/convert.c\n> @@ -1264,15 +1264,24 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)\n> \t\t}\n> \t\tca->checkout_encoding = git_path_check_encoding(ccheck + 5);\n> \t} else {\n> -\t\tca->drv = NULL;\n> -\t\tca->crlf_action = CRLF_UNDEFINED;\n> -\t\tca->ident = 0;\n> +\t\tmemset(ca, 0, sizeof(*ca));\n> \t}\n> \n> \t/* Save attr and make a decision for action */\n> \tca->attr_action = ca->crlf_action;\n> \tif (ca->crlf_action == CRLF_TEXT)\n> \t\tca->crlf_action = text_eol_is_crlf() ? CRLF_TEXT_CRLF : CRLF_TEXT_INPUT;\n> +\t/*\n> +\t * Often UTF-16 encoded files are read and written by programs which\n> +\t * really need CRLF, and it is important to keep the CRLF \"as is\" when\n> +\t * files are committed with core.autocrlf=true and the repo is pushed.\n> +\t * The CRLF would be converted into LF when the repo is cloned to\n> +\t * a machine with core.autocrlf=false.\n> +\t * Obey the \"text\" and \"eol\" attributes and be independent on the\n> +\t * local core.autocrlf for all \"encoded\" files.\n> +\t */\n> +\tif ((ca->crlf_action == CRLF_UNDEFINED) && ca->checkout_encoding)\n> +\t\tca->crlf_action = CRLF_BINARY;\n> \tif (ca->crlf_action == CRLF_UNDEFINED && auto_crlf == AUTO_CRLF_FALSE)\n> \t\tca->crlf_action = CRLF_BINARY;\n> \tif (ca->crlf_action == CRLF_UNDEFINED && auto_crlf == AUTO_CRLF_TRUE)\n> -- \n> 2.16.0.rc0.2.g64d3e4d0cc.dirty\n> \n\n"},{"id":"337787","messageId":"20180130144002.GA30211@tor.lan","threadId":"47654","inReplyTo":"55B6C3D5-4131-4636-AD0E-20759EDBE8CD@gmail.com","subject":"Re: [PATCH/RFC v5 7/7] Careful with CRLF when using e.g. UTF-16 for working-tree-encoding","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-01-30T14:40:02Z","receivedAt":"2018-01-30T14:40:24Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Tue, Jan 30, 2018 at 12:23:47PM +0100, Lars Schneider wrote:\n> \n> > On 29 Jan 2018, at 21:19, tboegi@web.de wrote:\n> > \n> > From: Torsten Bögershausen <tboegi@web.de>\n> > \n> > UTF-16 encoded files are treated as \"binary\" by Git, and no CRLF\n> > conversion is done.\n> > When the UTF-16 encoded files are converted into UF-8 using the new\n> s/UF-8/UTF-8/\n> \n> \n> > \"working-tree-encoding\", the CRLF are converted if core.autocrlf is true.\n> > \n> > This may lead to confusion:\n> > A tool writes an UTF-16 encoded file with CRLF.\n> > The file is commited with core.autocrlf=true, the CLRF are converted into LF.\n> > The repo is pushed somewhere and cloned by a different user, who has\n> > decided to use core.autocrlf=false.\n> > He uses the same tool, and now the CRLF are not there as expected, but LF,\n> > make the file useless for the tool.\n> > \n> > Avoid this (possible) confusion by ignoring core.autocrlf for all files\n> > which have \"working-tree-encoding\" defined.\n> \n> Maybe I don't understand your use case but I think this will generate even \n> more confusion because that's not what I would expect as a user. I think Git \n> should behave consistently independent of the used encoding. Here are my arguments:\n\nTo start with: I have probably seen too many repos with CRLF messed up.\n\n> \n>   (1) Legacy users are *not* affected. If you don't use the \"working-tree-encoding\"\n>       attribute then nothing changes for you.\n\nPeople who don't use \"working-tree-encoding\" are not affected,\nI never ment to state that.\n\nI am thinking about people who use \"working-tree-encoding\" without thinking\nabout line endings.\nOr the ones that have in mind that core.autocrlf=true will leave the\nline endings for UTF-16 encoded files as is, but that changes as soon as they\nare converted into UTF-8 and the \"auto\" check is now done\n-after- the conversion. I would find that confusing.\n\n> \n>   (2) If you use the \"working-tree-encoding\" attribute *and* you want to ensure \n>       your file keeps CRLF then you can define that in the attributes too. E.g.:\n>       \n>       *.proj textworking-tree-encoding=UTF-16 eol=crlf\n\nThat is a good one.\nIf you ever plan a re-roll (I don't at the moment) the *.proj extemsion\nmake much more sense in Documentation/gitattributes that *.tx\nThere no text files encoded in UTF-16 wich are called xxx.txt, but those\nare non-ideal examples. *.proj makes good sense as an example.\n\n\n> \n> - Lars\n> \n> \n> \n> > The user can still use a .gitattributes file and specify the line endings\n> > like \"text=auto\", \"text\", or \"text eol=crlf\" and let that .gitattribute\n> > file travel together with push and clone.\n> > \n> > Change convert.c to e more careful, simplify the initialization when\n> > attributes are retrived (and none are specified) and update the documentation.\n> > \n> > Signed-off-by: Torsten Bögershausen <tboegi@web.de>\n> > ---\n> > Documentation/gitattributes.txt |  9 ++++++---\n> > convert.c                       | 15 ++++++++++++---\n> > 2 files changed, 18 insertions(+), 6 deletions(-)\n> > \n> > diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\n> > index a8dbf4be3..3665c4677 100644\n> > --- a/Documentation/gitattributes.txt\n> > +++ b/Documentation/gitattributes.txt\n> > @@ -308,12 +308,15 @@ Use the `working-tree-encoding` attribute only if you cannot store a file in\n> > UTF-8 encoding and if you want Git to be able to process the content as\n> > text.\n> > \n> > +Note that when `working-tree-encoding` is defined, core.autocrlf is ignored.\n> > +Set the `text` attribute (or `text=auto`) to enable CRLF conversions.\n> > +\n> > Use the following attributes if your '*.txt' files are UTF-16 encoded\n> > -with byte order mark (BOM) and you want Git to perform automatic line\n> > -ending conversion based on your platform.\n> > +with byte order mark (BOM) and you want Git to perform line\n> > +ending conversion based on core.eol.\n> > \n> > ------------------------\n> > -*.txt\t\ttext working-tree-encoding=UTF-16\n> > +*.txt\t\tworking-tree-encoding=UTF-16 text\n> > ------------------------\n> > \n> > Use the following attributes if your '*.txt' files are UTF-16 little\n> > diff --git a/convert.c b/convert.c\n> > index 13fad490c..e7f11d1db 100644\n> > --- a/convert.c\n> > +++ b/convert.c\n> > @@ -1264,15 +1264,24 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)\n> > \t\t}\n> > \t\tca->checkout_encoding = git_path_check_encoding(ccheck + 5);\n> > \t} else {\n> > -\t\tca->drv = NULL;\n> > -\t\tca->crlf_action = CRLF_UNDEFINED;\n> > -\t\tca->ident = 0;\n> > +\t\tmemset(ca, 0, sizeof(*ca));\n> > \t}\n> > \n> > \t/* Save attr and make a decision for action */\n> > \tca->attr_action = ca->crlf_action;\n> > \tif (ca->crlf_action == CRLF_TEXT)\n> > \t\tca->crlf_action = text_eol_is_crlf() ? CRLF_TEXT_CRLF : CRLF_TEXT_INPUT;\n> > +\t/*\n> > +\t * Often UTF-16 encoded files are read and written by programs which\n> > +\t * really need CRLF, and it is important to keep the CRLF \"as is\" when\n> > +\t * files are committed with core.autocrlf=true and the repo is pushed.\n> > +\t * The CRLF would be converted into LF when the repo is cloned to\n> > +\t * a machine with core.autocrlf=false.\n> > +\t * Obey the \"text\" and \"eol\" attributes and be independent on the\n> > +\t * local core.autocrlf for all \"encoded\" files.\n> > +\t */\n> > +\tif ((ca->crlf_action == CRLF_UNDEFINED) && ca->checkout_encoding)\n> > +\t\tca->crlf_action = CRLF_BINARY;\n> > \tif (ca->crlf_action == CRLF_UNDEFINED && auto_crlf == AUTO_CRLF_FALSE)\n> > \t\tca->crlf_action = CRLF_BINARY;\n> > \tif (ca->crlf_action == CRLF_UNDEFINED && auto_crlf == AUTO_CRLF_TRUE)\n> > -- \n> > 2.16.0.rc0.2.g64d3e4d0cc.dirty\n> > \n> \n"},{"id":"337789","messageId":"10091BA4-1069-4A65-9057-CAAD87F9B55F@gmail.com","threadId":"47654","inReplyTo":"20180130144002.GA30211@tor.lan","subject":"Re: [PATCH/RFC v5 7/7] Careful with CRLF when using e.g. UTF-16 for working-tree-encoding","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-01-30T15:14:03Z","receivedAt":"2018-01-30T15:14:13Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 30 Jan 2018, at 15:40, Torsten Bögershausen <tboegi@web.de> wrote:\n> \n> On Tue, Jan 30, 2018 at 12:23:47PM +0100, Lars Schneider wrote:\n>> \n>>> On 29 Jan 2018, at 21:19, tboegi@web.de wrote:\n>>> \n>>> From: Torsten Bögershausen <tboegi@web.de>\n>>> \n>>> UTF-16 encoded files are treated as \"binary\" by Git, and no CRLF\n>>> conversion is done.\n>>> When the UTF-16 encoded files are converted into UF-8 using the new\n>> s/UF-8/UTF-8/\n>> \n>> \n>>> \"working-tree-encoding\", the CRLF are converted if core.autocrlf is true.\n>>> \n>>> This may lead to confusion:\n>>> A tool writes an UTF-16 encoded file with CRLF.\n>>> The file is commited with core.autocrlf=true, the CLRF are converted into LF.\n>>> The repo is pushed somewhere and cloned by a different user, who has\n>>> decided to use core.autocrlf=false.\n>>> He uses the same tool, and now the CRLF are not there as expected, but LF,\n>>> make the file useless for the tool.\n>>> \n>>> Avoid this (possible) confusion by ignoring core.autocrlf for all files\n>>> which have \"working-tree-encoding\" defined.\n>> \n>> Maybe I don't understand your use case but I think this will generate even \n>> more confusion because that's not what I would expect as a user. I think Git \n>> should behave consistently independent of the used encoding. Here are my arguments:\n> \n> To start with: I have probably seen too many repos with CRLF messed up.\n> \n>> \n>>  (1) Legacy users are *not* affected. If you don't use the \"working-tree-encoding\"\n>>      attribute then nothing changes for you.\n> \n> People who don't use \"working-tree-encoding\" are not affected,\n> I never ment to state that.\n> \n> I am thinking about people who use \"working-tree-encoding\" without thinking\n> about line endings.\n> Or the ones that have in mind that core.autocrlf=true will leave the\n> line endings for UTF-16 encoded files as is, but that changes as soon as they\n> are converted into UTF-8 and the \"auto\" check is now done\n> -after- the conversion. I would find that confusing.\n> \n>> \n>>  (2) If you use the \"working-tree-encoding\" attribute *and* you want to ensure \n>>      your file keeps CRLF then you can define that in the attributes too. E.g.:\n>> \n>>      *.proj textworking-tree-encoding=UTF-16 eol=crlf\n> \n> That is a good one.\n> If you ever plan a re-roll (I don't at the moment) the *.proj extemsion\n> make much more sense in Documentation/gitattributes that *.tx\n> There no text files encoded in UTF-16 wich are called xxx.txt, but those\n> are non-ideal examples. *.proj makes good sense as an example.\n\nOK, I'll do that. Would that fix the problem which this patch tries to address for you?\n(I would also explicitly add a paragraph to discuss line endings)\n\n- Lars"},{"id":"337798","messageId":"xmqq4ln3upiv.fsf@gitster-ct.c.googlers.com","threadId":"47654","inReplyTo":"20180129201905.9355-1-tboegi@web.de","subject":"Re: [PATCH v5 4/7] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-30T19:15:20Z","receivedAt":"2018-01-30T19:15:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n> From: Lars Schneider <larsxschneider@gmail.com>\n>\n> If the endianness is not defined in the encoding name, then let's\n> be strict and require a BOM to avoid any encoding confusion. The\n> has_missing_utf_bom() function returns true if a required BOM is\n> missing.\n>\n> The Unicode standard instructs to assume big-endian if there in no BOM\n> for UTF-16/32 [1][2]. However, the W3C/WHATWG encoding standard used\n> in HTML5 recommends to assume little-endian to \"deal with deployed\n> content\" [3]. Strictly requiring a BOM seems to be the safest option\n> for content in Git.\n\nI do not have strong opinion on encoding such policy-ish behaviour\nas our default, but am I alone to find that \"has missing X\" is a\nconfusing name for a helper function?  \"is missing X\" (or \"lacks\nX\") is a bit more understandable, I guess.\n\n> +int has_missing_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"},{"id":"337801","messageId":"xmqqzi4vt8n1.fsf@gitster-ct.c.googlers.com","threadId":"47654","inReplyTo":"20180129201908.9398-1-tboegi@web.de","subject":"Re: [PATCH v5 5/7] convert: add 'working-tree-encoding' attribute","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-30T20:05:22Z","receivedAt":"2018-01-30T20:05:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n> +\tif ((conv_flags & CONV_WRITE_OBJECT) && !strcmp(enc->name, \"SHIFT-JIS\")) {\n> +\t\tchar *re_src;\n> +\t\tint re_src_len;\n\nI think it is a bad idea to \n\n (1) not check without CONV_WRITE_OBJECT here.\n (2) hardcode SJIS and do this always and to SJIS alone.\n\nFor (1), a fix would be obvious (and that will resurrect the dead\ncode below).\n\nFor (2), perhaps introduce a multi-value configuration variable\ncore.checkRoundtripEncoding, whose default value consists of just\nSJIS, but allow users to add or clear it?\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     &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\tif (conv_flags & CONV_WRITE_OBJECT)\n> +\t\t\t\tdie(msg, path, enc->name, default_encoding);\n> +\t\t\telse\n> +\t\t\t\terror(msg, path, enc->name, default_encoding);\n\nThe \"error\" side of this inner if() is dead code, I think.\n"},{"id":"337805","messageId":"396FBDFA-606F-41D9-988C-D6886089BC15@gmail.com","threadId":"47654","inReplyTo":"xmqqzi4vt8n1.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v5 5/7] convert: add 'working-tree-encoding' attribute","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-01-30T20:31:58Z","receivedAt":"2018-01-30T20:32:08Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 30 Jan 2018, at 21:05, Junio C Hamano <gitster@pobox.com> wrote:\n> \n> tboegi@web.de writes:\n> \n>> +\tif ((conv_flags & CONV_WRITE_OBJECT) && !strcmp(enc->name, \"SHIFT-JIS\")) {\n>> +\t\tchar *re_src;\n>> +\t\tint re_src_len;\n> \n> I think it is a bad idea to \n> \n> (1) not check without CONV_WRITE_OBJECT here.\n\nThe idea is to perform the roundtrip check *only* if we \nactually write to Git. In all other cases we don't care\nif the encoding roundtrips.\n\n\"git checkout\" is such a case where we don't care as \nnoted by Peff here:\nhttps://public-inbox.org/git/20171215095838.GA3567@sigill.intra.peff.net/\n\nDo you agree?\n\n\n> (2) hardcode SJIS and do this always and to SJIS alone.\n> \n> ...\n> \n> For (2), perhaps introduce a multi-value configuration variable\n> core.checkRoundtripEncoding, whose default value consists of just\n> SJIS, but allow users to add or clear it?\n\nWell, in that case I would make it simpler and make\ncore.checkRoundtripEncoding a boolean that applies to all encodings\nif enabled. We could make even simpler than that by removing the entire \nroundtrip check. The thing is, I was not able to come up with a\nsequence that would not generate a iconv error *and* not round trip.\nWould that be ok for you to remove all that roundtrip checking code?\n\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     &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\tif (conv_flags & CONV_WRITE_OBJECT)\n>> +\t\t\t\tdie(msg, path, enc->name, default_encoding);\n>> +\t\t\telse\n>> +\t\t\t\terror(msg, path, enc->name, default_encoding);\n> \n> The \"error\" side of this inner if() is dead code, I think.\n\nGood catch. I think this code should go away if we keep the roundtrip\ncode and you agree with my statement above.\n\n\nThanks a lot for the review,\nLars"},{"id":"337812","messageId":"BEE9E5DB-AB1A-4119-90E6-700186739C59@gmail.com","threadId":"47654","inReplyTo":"xmqq4ln3upiv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v5 4/7] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-01-30T20:58:55Z","receivedAt":"2018-01-30T20:59:02Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 30 Jan 2018, at 20:15, Junio C Hamano <gitster@pobox.com> wrote:\n> \n> tboegi@web.de writes:\n> \n>> From: Lars Schneider <larsxschneider@gmail.com>\n>> \n>> If the endianness is not defined in the encoding name, then let's\n>> be strict and require a BOM to avoid any encoding confusion. The\n>> has_missing_utf_bom() function returns true if a required BOM is\n>> missing.\n>> \n>> The Unicode standard instructs to assume big-endian if there in no BOM\n>> for UTF-16/32 [1][2]. However, the W3C/WHATWG encoding standard used\n>> in HTML5 recommends to assume little-endian to \"deal with deployed\n>> content\" [3]. Strictly requiring a BOM seems to be the safest option\n>> for content in Git.\n> \n> I do not have strong opinion on encoding such policy-ish behaviour\n> as our default, but am I alone to find that \"has missing X\" is a\n> confusing name for a helper function?  \"is missing X\" (or \"lacks\n> X\") is a bit more understandable, I guess.\n\nThat might be a german/english translation thingy but I think I get\nyour point. \"has\" implies there is something and \"missing\" implies\nthere is nothing :)\n\n\"is_missing_utf_bom()\" might be even a bit unspecific as UTF-8\nis usually missing a UTF BOM but the function would still return \n\"false\". Therefore, \"is_missing_required_utf_bom()\" might be \nlengthy but should fit.\n\nOK for you?\n\n- Lars\n\n\n> \n>> +int has_missing_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"},{"id":"337844","messageId":"xmqqk1vzrp0l.fsf@gitster-ct.c.googlers.com","threadId":"47654","inReplyTo":"BEE9E5DB-AB1A-4119-90E6-700186739C59@gmail.com","subject":"Re: [PATCH v5 4/7] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-30T21:54:34Z","receivedAt":"2018-01-30T21:54:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider <larsxschneider@gmail.com> writes:\n\n> \"false\". Therefore, \"is_missing_required_utf_bom()\" might be \n> lengthy but should fit.\n\nThanks, sounds understandable a lot better than the original ;-)\n"},{"id":"337846","messageId":"xmqqfu6nrowm.fsf@gitster-ct.c.googlers.com","threadId":"47654","inReplyTo":"396FBDFA-606F-41D9-988C-D6886089BC15@gmail.com","subject":"Re: [PATCH v5 5/7] convert: add 'working-tree-encoding' attribute","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-30T21:56:57Z","receivedAt":"2018-01-30T21:57:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider <larsxschneider@gmail.com> writes:\n\n>> On 30 Jan 2018, at 21:05, Junio C Hamano <gitster@pobox.com> wrote:\n>> \n>> tboegi@web.de writes:\n>> \n>>> +\tif ((conv_flags & CONV_WRITE_OBJECT) && !strcmp(enc->name, \"SHIFT-JIS\")) {\n>>> +\t\tchar *re_src;\n>>> +\t\tint re_src_len;\n>> \n>> I think it is a bad idea to \n>> \n>> (1) not check without CONV_WRITE_OBJECT here.\n>\n> The idea is to perform the roundtrip check *only* if we \n> actually write to Git. In all other cases we don't care\n> if the encoding roundtrips.\n>\n> \"git checkout\" is such a case where we don't care as \n> noted by Peff here:\n> https://public-inbox.org/git/20171215095838.GA3567@sigill.intra.peff.net/\n>\n> Do you agree?\n\nI am not sure why this is special cased and other codepaths have \"if\nWRITE_OBJECT then die, otherwise error\" checks, so no, I do not\nagree with your reasoning, at least not yet.\n\n"},{"id":"337966","messageId":"20180131172837.GA32723@tor.lan","threadId":"47654","inReplyTo":"10091BA4-1069-4A65-9057-CAAD87F9B55F@gmail.com","subject":"Re: [PATCH/RFC v5 7/7] Careful with CRLF when using e.g. UTF-16 for working-tree-encoding","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-01-31T17:28:37Z","receivedAt":"2018-01-31T17:29:01Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"[]\n> > That is a good one.\n> > If you ever plan a re-roll (I don't at the moment) the *.proj extemsion\n> > make much more sense in Documentation/gitattributes that *.tx\n> > There no text files encoded in UTF-16 wich are called xxx.txt, but those\n> > are non-ideal examples. *.proj makes good sense as an example.\n> \n> OK, I'll do that. Would that fix the problem which this patch tries to address for you?\n> (I would also explicitly add a paragraph to discuss line endings)\n\nPlease let me see the patch first, before I can have a comment.\n\nBut back to the more general question:\n\nHow should Git handle the line endings of UTF-16 files in the woring-tree,\nwhich are UTF-8 in the index?\n\n\nThere are 2 opposite opionions/user expectations here:\n\na) They are binary in the working tree, so git should leave the line endings\n   as is. (Unless specified otherwise in the .attributes file)\nb) They are text files in the index. Git will convert line endings\n   if core.autocrlf is true (or the .gitattributes file specifies \"-text\")\n\nMy feeling is that both arguments are valid, so let's ask for opinions\nand thoughts of others.\nErik, Junio, Johannes, Johannes, Jeff, Ramsay, everybody:\nWhat do yo think ?\n"},{"id":"337970","messageId":"57086A32-6A2A-4B66-A355-10408C9FE0B0@gmail.com","threadId":"47654","inReplyTo":"xmqqfu6nrowm.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v5 5/7] convert: add 'working-tree-encoding' attribute","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-01-31T19:12:44Z","receivedAt":"2018-01-31T19:12:52Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 30 Jan 2018, at 22:56, Junio C Hamano <gitster@pobox.com> wrote:\n> \n> Lars Schneider <larsxschneider@gmail.com> writes:\n> \n>>> On 30 Jan 2018, at 21:05, Junio C Hamano <gitster@pobox.com> wrote:\n>>> \n>>> tboegi@web.de writes:\n>>> \n>>>> +\tif ((conv_flags & CONV_WRITE_OBJECT) && !strcmp(enc->name, \"SHIFT-JIS\")) {\n>>>> +\t\tchar *re_src;\n>>>> +\t\tint re_src_len;\n>>> \n>>> I think it is a bad idea to \n>>> \n>>> (1) not check without CONV_WRITE_OBJECT here.\n>> \n>> The idea is to perform the roundtrip check *only* if we \n>> actually write to Git. In all other cases we don't care\n>> if the encoding roundtrips.\n>> \n>> \"git checkout\" is such a case where we don't care as \n>> noted by Peff here:\n>> https://public-inbox.org/git/20171215095838.GA3567@sigill.intra.peff.net/\n>> \n>> Do you agree?\n> \n> I am not sure why this is special cased and other codepaths have \"if\n> WRITE_OBJECT then die, otherwise error\" checks, so no, I do not\n> agree with your reasoning, at least not yet.\n\nThe convert_to_git()/encode_to_git() machinery is used in two different\nkinds of code paths:\n\nSome code paths actually write to the Git database (indicated by the \nCONV_WRITE_OBJECT flag). I consider these the \"critical/important\" code \npaths and I don't want to tolerate any encoding errors in these cases as \nthe errors would be \"forever\" in the Git database. That's why I call \ndie() on errors for these cases to abort whatever we are doing. \n\nOther code paths do not write to the Git database (e.g. during \"git \ncheckout\" we use the code to ensure that we are moving away from the \nexact state that we think we are moving away). In these code paths I am \nless concerned about encoding errors. I also don't want to abort the \noperation (e.g. \"git checkout\") in these cases. That's why I only inform\nthe user about the problem with an error message.\n\nThe encoding round-trip check can be expensive. That's why I decided \ninitially to only execute the check in the \"critical/important\" \nwrite-to-Git-database situations (CONV_WRITE_OBJECT flag!). I also \ndecided to run it only if the \"SHIFT-JIS\" encoding is used as this was\nthe only encoding that I could find which reportedly does not round-trip\nwith UTF-8 (although I was not able to replicate the round-trip \nproblems). \n\nI want to change the current implementation as follows:\n\nI want to check the round-trip encoding only if the the environment \nvariable \"GIT_WORKING_TREE_ENCODING_ROUNDTRIP_CHECK\" is set. This way\na user can check the round-trip if necessary for *any* encoding. I\ndon't want to make it a git config because that setting should only \nrarely be used for debugging purposes.\n\nPerforming the round-trip check every time is not necessary from my \npoint of view because it can be expensive and I was not able to generate\na test case which *does not* round-trip without triggering any other\niconv error.\n\n- Lars"},{"id":"337971","messageId":"F3D39E4F-D467-4A6E-95A4-A76BFEDF14BF@gmail.com","threadId":"47654","inReplyTo":"20180131172837.GA32723@tor.lan","subject":"Re: [PATCH/RFC v5 7/7] Careful with CRLF when using e.g. UTF-16 for working-tree-encoding","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-01-31T19:37:30Z","receivedAt":"2018-01-31T19:37:37Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 31 Jan 2018, at 18:28, Torsten Bögershausen <tboegi@web.de> wrote:\n> \n> []\n>>> That is a good one.\n>>> If you ever plan a re-roll (I don't at the moment) the *.proj extemsion\n>>> make much more sense in Documentation/gitattributes that *.tx\n>>> There no text files encoded in UTF-16 wich are called xxx.txt, but those\n>>> are non-ideal examples. *.proj makes good sense as an example.\n>> \n>> OK, I'll do that. Would that fix the problem which this patch tries to address for you?\n>> (I would also explicitly add a paragraph to discuss line endings)\n> \n> Please let me see the patch first, before I can have a comment.\n\nSure! I'll have it ready tomorrow.\n\n\n> But back to the more general question:\n> \n> How should Git handle the line endings of UTF-16 files in the woring-tree,\n> which are UTF-8 in the index?\n> \n> \n> There are 2 opposite opionions/user expectations here:\n> \n> a) They are binary in the working tree, so git should leave the line endings\n>   as is. (Unless specified otherwise in the .attributes file)\n\nWell, if you consider your UTF-16 files binary then you would not change\n*anything*. You would not enable the new \"working-tree-encoding\" attribute.\nAs a consequence, Git's behavior would not change. Git would leave all line \nendings as they are for the UTF-16 files.\n\n\n> b) They are text files in the index. Git will convert line endings\n>   if core.autocrlf is true (or the .gitattributes file specifies \"-text\")\n\nThis would *only* happen if you enable the new \"working-tree-encoding\"\nattribute. In this case a user has already made the conscious decision to\ntreat these files as text files. Therefore, the user expects Git to handle\nthem in the same way other text files are handled.\n\n\n> My feeling is that both arguments are valid, so let's ask for opinions\n> and thoughts of others.\n> Erik, Junio, Johannes, Johannes, Jeff, Ramsay, everybody:\n> What do yo think ?\n\n- Lars"},{"id":"337981","messageId":"xmqqtvv1r6kr.fsf@gitster-ct.c.googlers.com","threadId":"47654","inReplyTo":"57086A32-6A2A-4B66-A355-10408C9FE0B0@gmail.com","subject":"Re: [PATCH v5 5/7] convert: add 'working-tree-encoding' attribute","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-31T22:45:08Z","receivedAt":"2018-01-31T22:45:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider <larsxschneider@gmail.com> writes:\n\n>> I am not sure why this is special cased and other codepaths have \"if\n>> WRITE_OBJECT then die, otherwise error\" checks, so no, I do not\n>> agree with your reasoning, at least not yet.\n>\n> The convert_to_git()/encode_to_git() machinery is used in two different\n> kinds of code paths:\n>\n> Some code paths actually write to the Git database (indicated by the \n> CONV_WRITE_OBJECT flag). I consider these the \"critical/important\" code \n> paths and I don't want to tolerate any encoding errors in these cases as \n> the errors would be \"forever\" in the Git database. That's why I call \n> die() on errors for these cases to abort whatever we are doing. \n>\n> Other code paths do not write to the Git database (e.g. during \"git \n> checkout\" we use the code to ensure that we are moving away from the \n> exact state that we think we are moving away). In these code paths I am \n> less concerned about encoding errors. I also don't want to abort the \n> operation (e.g. \"git checkout\") in these cases. That's why I only inform\n> the user about the problem with an error message.\n\nWarning the users early while they are doing non-writing\noperation to give them chance to adjust the contents, before they\nactually need to register the contents as objects by writing, at\nwhich point we need to die.  That's a reasonable distinction and all\nof that I already agree with.\n\nWhat was questionable and left unexplained was why this roundtrip\nthing needs to be different.\n\n> The encoding round-trip check can be expensive. That's why I decided \n> initially to only execute the check in the \"critical/important\" \n> write-to-Git-database situations (CONV_WRITE_OBJECT flag!). I also \n> decided to run it only if the \"SHIFT-JIS\" encoding is used as this was\n> the only encoding that I could find which reportedly does not round-trip\n> with UTF-8 (although I was not able to replicate the round-trip \n> problems). \n\nI still do not see why you have problems with the approach of\nmaintaining a configurable set of \"iffy\" encodings (and throw SJIS\ninto the default list) to achieve all of the above and more.  For\nSJIS users, instead of having to set environment variables to obtain\nsafe behaviour, they automatically get safe behaviour.  When using\nencodings that are not problematic, they do not need to spend cycles\nchecking round-trip.  And when SJIS users know they do not care about\nroundtrip checks, they can just configure SJIS away from the list.\n"},{"id":"338112","messageId":"xmqqtvuzcibz.fsf@gitster-ct.c.googlers.com","threadId":"47654","inReplyTo":"20180131172837.GA32723@tor.lan","subject":"Re: [PATCH/RFC v5 7/7] Careful with CRLF when using e.g. UTF-16 for working-tree-encoding","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-02T19:17:04Z","receivedAt":"2018-02-02T19:17:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> There are 2 opposite opionions/user expectations here:\n>\n> a) They are binary in the working tree, so git should leave the line endings\n>    as is. (Unless specified otherwise in the .attributes file)\n> ...\n> b) They are text files in the index. Git will convert line endings\n>    if core.autocrlf is true (or the .gitattributes file specifies \"-text\")\n\nI sense that you seem to be focusing on the distinction between \"in\nthe working tree\" vs \"in the index\" while contrasting.  The \"binary\nvs text\" in your \"binary in wt, text in index\" is based on the\ndefault heuristics without any input from end-users or the project\nthat uses Git that happens to contain such files.  If the users and\nthe project that uses Git want to treat contents in a path as text,\nit is text even when it is (re-)encoded to UTF-16, no?\n\nSuch files may be (mis)classified as binary with the default\nheuristics when there is no help from what is written in the\n.gitattributes file, but here we are talking about the case where\nthe user explicitly tells us it is in UTF-16, right?  Is there such a\nthing as UTF-16 binary?\n"},{"id":"338589","messageId":"20180207063147.GA22714@tor.lan","threadId":"47654","inReplyTo":"xmqqtvuzcibz.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH/RFC v5 7/7] Careful with CRLF when using e.g. UTF-16 for working-tree-encoding","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-02-07T06:31:47Z","receivedAt":"2018-02-07T06:32:12Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Fri, Feb 02, 2018 at 11:17:04AM -0800, Junio C Hamano wrote:\n> Torsten Bögershausen <tboegi@web.de> writes:\n> \n> > There are 2 opposite opionions/user expectations here:\n> >\n> > a) They are binary in the working tree, so git should leave the line endings\n> >    as is. (Unless specified otherwise in the .attributes file)\n> > ...\n> > b) They are text files in the index. Git will convert line endings\n> >    if core.autocrlf is true (or the .gitattributes file specifies \"-text\")\n> \n> I sense that you seem to be focusing on the distinction between \"in\n> the working tree\" vs \"in the index\" while contrasting.  The \"binary\n> vs text\" in your \"binary in wt, text in index\" is based on the\n> default heuristics without any input from end-users or the project\n> that uses Git that happens to contain such files.  If the users and\n> the project that uses Git want to treat contents in a path as text,\n> it is text even when it is (re-)encoded to UTF-16, no?\n> \n> Such files may be (mis)classified as binary with the default\n> heuristics when there is no help from what is written in the\n> .gitattributes file, but here we are talking about the case where\n> the user explicitly tells us it is in UTF-16, right?  Is there such a\n> thing as UTF-16 binary?\n\nI don't think so, by definiton UTF-16 is ment to be text.\n(this means that git ls-files --eol needs some update, I can have a look)\n\nDo we agree that UTF-16 is text ?\nIf yes, could Git assume that the \"text\" attribute is set when\nworking-tree-encoding is set ?\n\nI would even go a step further and demand that the user -must- make a decision\nabout the line endings for working-tree-encoded files:\nworking-tree-encoding=UTF-16                 # illegal, die()\nworking-tree-encoding=UTF-16 text=auto       # illegal, die()\nworking-tree-encoding=UTF-16 -text           # no eol conversion\nworking-tree-encoding=UTF-16 text            # eol according to core.eol\nworking-tree-encoding=UTF-16 text eol=lf     # LF\nworking-tree-encoding=UTF-16 text eol=crlf   # CRLF\n\nWhat do you think ?\n\n\n\n"},{"id":"338645","messageId":"xmqq372c8y9n.fsf@gitster-ct.c.googlers.com","threadId":"47654","inReplyTo":"20180207063147.GA22714@tor.lan","subject":"Re: [PATCH/RFC v5 7/7] Careful with CRLF when using e.g. UTF-16 for working-tree-encoding","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-07T18:12:20Z","receivedAt":"2018-02-07T18:12:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n>> the user explicitly tells us it is in UTF-16, right?  Is there such a\n>> thing as UTF-16 binary?\n>\n> I don't think so, by definiton UTF-16 is ment to be text.\n> (this means that git ls-files --eol needs some update, I can have a look)\n>\n> Do we agree that UTF-16 is text ?\n> If yes, could Git assume that the \"text\" attribute is set when\n> working-tree-encoding is set ?\n\nThese are two different questions.  It seems that between the two of\nus, we established that \"UTF-16 binary\" is a nonsense, and a path\nthat is given working-tree-encoding=UTF-16 must be treated as\nholding a text file.  But that does not have direct relevance to the\nother question you are asking: \"is a question 'does this path have\ntext attribute set?' be answered with 'yes' if the path has wte\nattribute set to UTF-16?\"  I think the answer to that latter\nquestion ought to be \"no\".\n\nBy the way, a related tangent is if it makes sense to give\nworking-tree-encoding to anything that is binary, regardless of the\nvalue---I am inclined to say it is not, so the issue here is not \"by\ndefinition UTF-16 is text\", but \"any path that has wte set to some\nenconding should be treated the same way as if the path also has\ntext attribute set as far as convert machinery is concerned.\".\n\n"}]}