{"thread":{"id":"48000","subject":"[PATCH v10 1/9] strbuf: remove unnecessary NUL assignment in xstrdup_tolower()","startedAt":"2018-03-07T17:31:04Z","lastAt":"2018-03-09T19:06:12Z","messageCount":32,"participants":["lars.schneider@autodesk.com","Eric Sunshine","Junio C Hamano","Lars Schneider","Duy Nguyen","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":10,"patchTotal":9},"messages":[{"id":"341209","messageId":"20180307173026.30058-2-lars.schneider@autodesk.com","threadId":"48000","inReplyTo":"20180307173026.30058-1-lars.schneider@autodesk.com","subject":"[PATCH v10 1/9] strbuf: remove unnecessary NUL assignment in xstrdup_tolower()","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-07T17:30:18Z","receivedAt":"2018-03-07T17:31:04Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nSince 3733e69464 (use xmallocz to avoid size arithmetic, 2016-02-22) we\nallocate the buffer for the lower case string with xmallocz(). This\nalready ensures a NUL at the end of the allocated buffer.\n\nRemove the unnecessary assignment.\n\nSigned-off-by: Lars Schneider \n---\n strbuf.c | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 1df674e919..55b7daeb35 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -781,7 +781,6 @@ char *xstrdup_tolower(const char *string)\n \tresult = xmallocz(len);\n \tfor (i = 0; i < len; i++)\n \t\tresult[i] = tolower(string[i]);\n-\tresult[i] = '\\0';\n \treturn result;\n }\n \n-- \n2.16.2\n\n"},{"id":"341210","messageId":"20180307173026.30058-1-lars.schneider@autodesk.com","threadId":"48000","inReplyTo":null,"subject":"[PATCH v10 0/9] convert: add support for different encodings","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-07T17:30:17Z","receivedAt":"2018-03-07T17:31:07Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nHi,\n\nPatches 1-5,8 are preparation and helper functions. Patch 3 is new.\nPatch 6,7,9 are the actual change.\n\nThis series depends on Torsten's 8462ff43e4 (convert_to_git():\nsafe_crlf/checksafe becomes int conv_flags, 2018-01-13) which is\nalready in master.\n\nChanges since v9:\n\n* make has_bom_prefix() / is_missing_required_utf_bom() more lenient in\n what they accept (ignore casing, accept UTF?? and UTF-?? , Junio)\n* replace memcmp() which does not check the length of the strings with\n a case insensitive variant of starts_with() (Junio)\n* do not convert encoding names to uppercase\n (this fixes a leak introduced in the last iteration, Eric)\n* do not cleanup test files that the test did not create (Eric)\n* do not cleanup err.out files in tests (Eric)\n\nI did not address Eric's feedback to make validate_encoding()\ncleaner [1] as I want to stabilize the series and Eric wrote\nthat we can clean this up later:\nhttp://public-inbox.org/git/CAPig+cSoka-yBTYBz42JGQTyCH7LDWnToeOvdZfG0_64o9QnBQ@mail.gmail.com\n\nThanks,\nLars\n\n\n RFC: https://public-inbox.org/git/BDB9B884-6D17-4BE3-A83C-F67E2AFA2B46@gmail.com/\n v1: https://public-inbox.org/git/20171211155023.1405-1-lars.schneider@autodesk.com/\n v2: https://public-inbox.org/git/20171229152222.39680-1-lars.schneider@autodesk.com/\n v3: https://public-inbox.org/git/20180106004808.77513-1-lars.schneider@autodesk.com/\n v4: https://public-inbox.org/git/20180120152418.52859-1-lars.schneider@autodesk.com/\n v5: https://public-inbox.org/git/20180129201855.9182-1-tboegi@web.de/\n v6: https://public-inbox.org/git/20180209132830.55385-1-lars.schneider@autodesk.com/\n v7: https://public-inbox.org/git/20180215152711.158-1-lars.schneider@autodesk.com/\n v8: https://public-inbox.org/git/20180224162801.98860-1-lars.schneider@autodesk.com/\n v9: https://public-inbox.org/git/20180304201418.60958-1-lars.schneider@autodesk.com/\n\n\n\nBase Ref:\nWeb-Diff: https://github.com/larsxschneider/git/commit/a602b8dcef\nCheckout: git fetch https://github.com/larsxschneider/git encoding-v10 && git checkout a602b8dcef\n\n\n### Interdiff (v9..v10):\n\ndiff --git a/convert.c b/convert.c\nindex 6cbb2b2618..e861f1abbc 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -269,7 +269,8 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n static int validate_encoding(const char *path, const char *enc,\n \t\t const char *data, size_t len, int die_on_error)\n {\n-\tif (!memcmp(\"UTF-\", enc, 4)) {\n+\t/* We only check for UTF here as UTF?? can be an alias for UTF-?? */\n+\tif (startscase_with(enc, \"UTF\")) {\n \t\t/*\n \t\t * Check for detectable errors in UTF encodings\n \t\t */\n@@ -277,16 +278,18 @@ static int validate_encoding(const char *path, const char *enc,\n \t\t\tconst char *error_msg = _(\n \t\t\t\t\"BOM is prohibited in '%s' if encoded as %s\");\n \t\t\t/*\n-\t\t\t * This advice is shown for UTF-??BE and UTF-??LE\n-\t\t\t * encodings. We truncate the encoding name to 6\n-\t\t\t * chars with %.6s to cut off the last two \"byte\n-\t\t\t * order\" characters.\n+\t\t\t * This advice is shown for UTF-??BE and UTF-??LE encodings.\n+\t\t\t * We cut off the last two characters of the encoding name\n+\t\t\t # to generate the encoding name suitable for BOMs.\n \t\t\t */\n \t\t\tconst char *advise_msg = _(\n \t\t\t\t\"The file '%s' contains a byte order \"\n-\t\t\t\t\"mark (BOM). Please use %.6s as \"\n+\t\t\t\t\"mark (BOM). Please use %s as \"\n \t\t\t\t\"working-tree-encoding.\");\n-\t\t\tadvise(advise_msg, path, enc);\n+\t\t\tchar *upper_enc = xstrdup_toupper(enc);\n+\t\t\tupper_enc[strlen(upper_enc)-2] = '\\0';\n+\t\t\tadvise(advise_msg, path, upper_enc);\n+\t\t\tfree(upper_enc);\n \t\t\tif (die_on_error)\n \t\t\t\tdie(error_msg, path, enc);\n \t\t\telse {\n@@ -301,7 +304,9 @@ static int validate_encoding(const char *path, const char *enc,\n \t\t\t\t\"mark (BOM). Please use %sBE or %sLE \"\n \t\t\t\t\"(depending on the byte order) as \"\n \t\t\t\t\"working-tree-encoding.\");\n-\t\t\tadvise(advise_msg, path, enc, enc);\n+\t\t\tchar *upper_enc = xstrdup_toupper(enc);\n+\t\t\tadvise(advise_msg, path, upper_enc, upper_enc);\n+\t\t\tfree(upper_enc);\n \t\t\tif (die_on_error)\n \t\t\t\tdie(error_msg, path, enc);\n \t\t\telse {\n@@ -1216,11 +1221,7 @@ static const char *git_path_check_encoding(struct attr_check_item *check)\n \tif (!strcasecmp(value, default_encoding))\n \t\treturn NULL;\n\n-\t/*\n-\t * Ensure encoding names are always upper case (e.g. UTF-8) to\n-\t * simplify subsequent string comparisons.\n-\t */\n-\treturn xstrdup_toupper(value);\n+\treturn value;\n }\n\n static enum crlf_action git_path_check_crlf(struct attr_check_item *check)\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 68b2ad531e..f648da0c11 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -455,6 +455,7 @@ extern void (*get_warn_routine(void))(const char *warn, va_list params);\n extern void set_die_is_recursing_routine(int (*routine)(void));\n\n extern int starts_with(const char *str, const char *prefix);\n+extern int startscase_with(const char *str, const char *prefix);\n\n /*\n * If the string \"str\" begins with the string found in \"prefix\", return 1.\ndiff --git a/strbuf.c b/strbuf.c\nindex b635f0bdc4..5779a2d591 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -11,6 +11,15 @@ int starts_with(const char *str, const char *prefix)\n \t\t\treturn 0;\n }\n\n+int startscase_with(const char *str, const char *prefix)\n+{\n+\tfor (; ; str++, prefix++)\n+\t\tif (!*prefix)\n+\t\t\treturn 1;\n+\t\telse if (tolower(*str) != tolower(*prefix))\n+\t\t\treturn 0;\n+}\n+\n int skip_to_optional_arg_default(const char *str, const char *prefix,\n \t\t\t\t const char **arg, const char *def)\n {\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex 23e89ae623..7cff41a350 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -52,7 +52,7 @@ test_expect_success 're-encode to UTF-16 on checkout' '\n '\n\n test_expect_success 'check $GIT_DIR/info/attributes support' '\n-\ttest_when_finished \"rm -f test.utf8.raw test.utf32.raw test.utf32.git\" &&\n+\ttest_when_finished \"rm -f test.utf32.git\" &&\n \ttest_when_finished \"git reset --hard HEAD\" &&\n\n \techo \"*.utf32 text working-tree-encoding=utf-32\" >.git/info/attributes &&\n@@ -75,19 +75,19 @@ do\n \t\t# In these cases the BOM is prohibited.\n \t\tcp bebom.utf${i}be.raw bebom.utf${i}be &&\n \t\ttest_must_fail git add bebom.utf${i}be 2>err.out &&\n-\t\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-${i}BE\" err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* utf-${i}be\" err.out &&\n\n \t\tcp lebom.utf${i}le.raw lebom.utf${i}be &&\n \t\ttest_must_fail git add lebom.utf${i}be 2>err.out &&\n-\t\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-${i}BE\" err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* utf-${i}be\" err.out &&\n\n \t\tcp bebom.utf${i}be.raw bebom.utf${i}le &&\n \t\ttest_must_fail git add bebom.utf${i}le 2>err.out &&\n-\t\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-${i}LE\" err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* utf-${i}le\" err.out &&\n\n \t\tcp lebom.utf${i}le.raw lebom.utf${i}le &&\n \t\ttest_must_fail git add lebom.utf${i}le 2>err.out &&\n-\t\ttest_i18ngrep \"fatal: BOM is prohibited .* UTF-${i}LE\" err.out\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* utf-${i}le\" err.out\n \t'\n\n \ttest_expect_success \"check required UTF-${i} BOM\" '\n@@ -97,11 +97,11 @@ do\n\n \t\tcp nobom.utf${i}be.raw nobom.utf${i} &&\n \t\ttest_must_fail git add nobom.utf${i} 2>err.out &&\n-\t\ttest_i18ngrep \"fatal: BOM is required .* UTF-${i}\" err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is required .* utf-${i}\" err.out &&\n\n \t\tcp nobom.utf${i}le.raw nobom.utf${i} &&\n \t\ttest_must_fail git add nobom.utf${i} 2>err.out &&\n-\t\ttest_i18ngrep \"fatal: BOM is required .* UTF-${i}\" err.out\n+\t\ttest_i18ngrep \"fatal: BOM is required .* utf-${i}\" err.out\n \t'\n\n \ttest_expect_success \"eol conversion for UTF-${i} encoded files on checkout\" '\n@@ -141,7 +141,6 @@ do\n done\n\n test_expect_success 'check unsupported encodings' '\n-\ttest_when_finished \"rm -f err.out\" &&\n \ttest_when_finished \"git reset --hard HEAD\" &&\n\n \techo \"*.nothing text working-tree-encoding=\" >>.gitattributes &&\n@@ -156,7 +155,6 @@ test_expect_success 'check unsupported encodings' '\n\n test_expect_success 'error if encoding round trip is not the same during refresh' '\n \tBEFORE_STATE=$(git rev-parse HEAD) &&\n-\ttest_when_finished \"rm -f err.out\" &&\n \ttest_when_finished \"git reset --hard $BEFORE_STATE\" &&\n\n \t# Add and commit a UTF-16 file but skip the \"working-tree-encoding\"\n@@ -176,7 +174,6 @@ test_expect_success 'error if encoding round trip is not the same during refresh\n\n test_expect_success 'error if encoding garbage is already in Git' '\n \tBEFORE_STATE=$(git rev-parse HEAD) &&\n-\ttest_when_finished \"rm -f err.out\" &&\n \ttest_when_finished \"git reset --hard $BEFORE_STATE\" &&\n\n \t# Skip the UTF-16 filter for the added file\n@@ -219,14 +216,14 @@ test_expect_success 'check roundtrip encoding' '\n \t# ... unless we tell Git to check it!\n \tGIT_TRACE=1 git -c core.checkRoundtripEncoding=\"UTF-16, UTF-32\" \\\n \t\tadd roundtrip.utf16 2>&1 |\n-\t\tgrep \"Checking roundtrip encoding for UTF-16\" &&\n+\t\tgrep \"Checking roundtrip encoding for utf-16\" &&\n \tgit reset &&\n\n \t# ... unless we tell Git to check it!\n \t# (here we also check that the casing of the encoding is irrelevant)\n \tGIT_TRACE=1 git -c core.checkRoundtripEncoding=\"UTF-32, utf-16\" \\\n \t\tadd roundtrip.utf16 2>&1 |\n-\t\tgrep \"Checking roundtrip encoding for UTF-16\" &&\n+\t\tgrep \"Checking roundtrip encoding for utf-16\" &&\n \tgit reset\n '\n\ndiff --git a/utf8.c b/utf8.c\nindex 5113d26e56..81c6678df1 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -552,11 +552,13 @@ static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\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 (!strcasecmp(enc, \"UTF-16BE\") || !strcasecmp(enc, \"UTF-16LE\") ||\n+\t !strcasecmp(enc, \"UTF16BE\") || !strcasecmp(enc, \"UTF16LE\")) &&\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 (!strcasecmp(enc, \"UTF-32BE\") || !strcasecmp(enc, \"UTF-32LE\") ||\n+\t !strcasecmp(enc, \"UTF32BE\") || !strcasecmp(enc, \"UTF32LE\")) &&\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@@ -565,11 +567,11 @@ int has_prohibited_utf_bom(const char *enc, const char *data, size_t len)\n int is_missing_required_utf_bom(const char *enc, const char *data, size_t len)\n {\n \treturn (\n-\t !strcmp(enc, \"UTF-16\") &&\n+\t (!strcasecmp(enc, \"UTF-16\") || !strcasecmp(enc, \"UTF16\")) &&\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 (!strcasecmp(enc, \"UTF-32\") || !strcasecmp(enc, \"UTF32\")) &&\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### Patches\n\nLars Schneider (9):\n strbuf: remove unnecessary NUL assignment in xstrdup_tolower()\n strbuf: add xstrdup_toupper()\n strbuf: add a case insensitive starts_with()\n utf8: add function to detect prohibited UTF-16/32 BOM\n utf8: add function to detect a missing UTF-16/32 BOM\n convert: add 'working-tree-encoding' attribute\n convert: check for detectable errors in UTF encodings\n convert: add tracing for 'working-tree-encoding' attribute\n convert: add round trip check based on 'core.checkRoundtripEncoding'\n\n Documentation/config.txt | 6 +\n Documentation/gitattributes.txt | 88 +++++++++++++\n config.c | 5 +\n convert.c | 268 ++++++++++++++++++++++++++++++++++++++-\n convert.h | 2 +\n environment.c | 1 +\n git-compat-util.h | 1 +\n sha1_file.c | 2 +-\n strbuf.c | 22 +++-\n strbuf.h | 1 +\n t/t0028-working-tree-encoding.sh | 230 +++++++++++++++++++++++++++++++++\n utf8.c | 39 ++++++\n utf8.h | 28 ++++\n 13 files changed, 690 insertions(+), 3 deletions(-)\n create mode 100755 t/t0028-working-tree-encoding.sh\n\n\nbase-commit: 8a2f0888555ce46ac87452b194dec5cb66fb1417\n--\n2.16.2\n\n"},{"id":"341211","messageId":"20180307173026.30058-3-lars.schneider@autodesk.com","threadId":"48000","inReplyTo":"20180307173026.30058-1-lars.schneider@autodesk.com","subject":"[PATCH v10 2/9] strbuf: add xstrdup_toupper()","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-07T17:30:19Z","receivedAt":"2018-03-07T17:31:09Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nCreate a copy of an existing string and make all characters upper case.\nSimilar xstrdup_tolower().\n\nThis function is used in a subsequent commit.\n\nSigned-off-by: Lars Schneider \n---\n strbuf.c | 12 ++++++++++++\n strbuf.h | 1 +\n 2 files changed, 13 insertions(+)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 55b7daeb35..b635f0bdc4 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -784,6 +784,18 @@ char *xstrdup_tolower(const char *string)\n \treturn result;\n }\n \n+char *xstrdup_toupper(const char *string)\n+{\n+\tchar *result;\n+\tsize_t len, i;\n+\n+\tlen = strlen(string);\n+\tresult = xmallocz(len);\n+\tfor (i = 0; i < len; i++)\n+\t\tresult[i] = toupper(string[i]);\n+\treturn result;\n+}\n+\n char *xstrvfmt(const char *fmt, va_list ap)\n {\n \tstruct strbuf buf = STRBUF_INIT;\ndiff --git a/strbuf.h b/strbuf.h\nindex 14c8c10d66..df7ced53ed 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -607,6 +607,7 @@ __attribute__((format (printf,2,3)))\n extern int fprintf_ln(FILE *fp, const char *fmt, ...);\n \n char *xstrdup_tolower(const char *);\n+char *xstrdup_toupper(const char *);\n \n /**\n * Create a newly allocated string using printf format. You can do this easily\n-- \n2.16.2\n\n"},{"id":"341212","messageId":"20180307173026.30058-4-lars.schneider@autodesk.com","threadId":"48000","inReplyTo":"20180307173026.30058-1-lars.schneider@autodesk.com","subject":"[PATCH v10 3/9] strbuf: add a case insensitive starts_with()","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-07T17:30:20Z","receivedAt":"2018-03-07T17:31:15Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nCheck in a case insensitive manner if one string is a prefix of another\nstring.\n\nThis function is used in a subsequent commit.\n\nSigned-off-by: Lars Schneider \n---\n git-compat-util.h | 1 +\n strbuf.c | 9 +++++++++\n 2 files changed, 10 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 68b2ad531e..f648da0c11 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -455,6 +455,7 @@ extern void (*get_warn_routine(void))(const char *warn, va_list params);\n extern void set_die_is_recursing_routine(int (*routine)(void));\n \n extern int starts_with(const char *str, const char *prefix);\n+extern int startscase_with(const char *str, const char *prefix);\n \n /*\n * If the string \"str\" begins with the string found in \"prefix\", return 1.\ndiff --git a/strbuf.c b/strbuf.c\nindex b635f0bdc4..5779a2d591 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -11,6 +11,15 @@ int starts_with(const char *str, const char *prefix)\n \t\t\treturn 0;\n }\n \n+int startscase_with(const char *str, const char *prefix)\n+{\n+\tfor (; ; str++, prefix++)\n+\t\tif (!*prefix)\n+\t\t\treturn 1;\n+\t\telse if (tolower(*str) != tolower(*prefix))\n+\t\t\treturn 0;\n+}\n+\n int skip_to_optional_arg_default(const char *str, const char *prefix,\n \t\t\t\t const char **arg, const char *def)\n {\n-- \n2.16.2\n\n"},{"id":"341213","messageId":"20180307173026.30058-5-lars.schneider@autodesk.com","threadId":"48000","inReplyTo":"20180307173026.30058-1-lars.schneider@autodesk.com","subject":"[PATCH v10 4/9] utf8: add function to detect prohibited UTF-16/32 BOM","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-07T17:30:21Z","receivedAt":"2018-03-07T17:31:20Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nWhenever a data stream is declared to be UTF-16BE, UTF-16LE, UTF-32BE\nor UTF-32LE a BOM must not be used [1]. The function returns true if\nthis is the case.\n\nThis function is used in a subsequent commit.\n\n[1] http://unicode.org/faq/utf_bom.html#bom10\n\nSigned-off-by: Lars Schneider \n---\n utf8.c | 26 ++++++++++++++++++++++++++\n utf8.h | 9 +++++++++\n 2 files changed, 35 insertions(+)\n\ndiff --git a/utf8.c b/utf8.c\nindex 2c27ce0137..e4b99580f0 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -538,6 +538,32 @@ 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 (!strcasecmp(enc, \"UTF-16BE\") || !strcasecmp(enc, \"UTF-16LE\") ||\n+\t !strcasecmp(enc, \"UTF16BE\") || !strcasecmp(enc, \"UTF16LE\")) &&\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 (!strcasecmp(enc, \"UTF-32BE\") || !strcasecmp(enc, \"UTF-32LE\") ||\n+\t !strcasecmp(enc, \"UTF32BE\") || !strcasecmp(enc, \"UTF32LE\")) &&\n+\t (has_bom_prefix(data, len, utf32_be_bom, sizeof(utf32_be_bom)) ||\n+\t has_bom_prefix(data, len, utf32_le_bom, sizeof(utf32_le_bom)))\n+\t);\n+}\n+\n /*\n * Returns first character length in bytes for multi-byte `text` according to\n * `encoding`.\ndiff --git a/utf8.h b/utf8.h\nindex 6bbcf31a83..0db1db4519 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -70,4 +70,13 @@ typedef enum {\n void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int width,\n \t\t const char *s);\n \n+/*\n+ * If a data stream is declared as UTF-16BE or UTF-16LE, then a UTF-16\n+ * BOM must not be used [1]. The same applies for the UTF-32 equivalents.\n+ * The function returns true if this rule is violated.\n+ *\n+ * [1] http://unicode.org/faq/utf_bom.html#bom10\n+ */\n+int has_prohibited_utf_bom(const char *enc, const char *data, size_t len);\n+\n #endif\n-- \n2.16.2\n\n"},{"id":"341214","messageId":"20180307173026.30058-6-lars.schneider@autodesk.com","threadId":"48000","inReplyTo":"20180307173026.30058-1-lars.schneider@autodesk.com","subject":"[PATCH v10 5/9] utf8: add function to detect a missing UTF-16/32 BOM","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-07T17:30:22Z","receivedAt":"2018-03-07T17:31:22Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nIf the endianness is not defined in the encoding name, then let's\nbe strict and require a BOM to avoid any encoding confusion. The\nis_missing_required_utf_bom() function returns true if a required BOM\nis missing.\n\nThe Unicode standard instructs to assume big-endian if there in no BOM\nfor UTF-16/32 [1][2]. However, the W3C/WHATWG encoding standard used\nin HTML5 recommends to assume little-endian to \"deal with deployed\ncontent\" [3]. Strictly requiring a BOM seems to be the safest option\nfor content in Git.\n\nThis function is used in a subsequent commit.\n\n[1] http://unicode.org/faq/utf_bom.html#gen6\n[2] http://www.unicode.org/versions/Unicode10.0.0/ch03.pdf\n Section 3.10, D98, page 132\n[3] https://encoding.spec.whatwg.org/#utf-16le\n\nSigned-off-by: Lars Schneider \n---\n utf8.c | 13 +++++++++++++\n utf8.h | 19 +++++++++++++++++++\n 2 files changed, 32 insertions(+)\n\ndiff --git a/utf8.c b/utf8.c\nindex e4b99580f0..81c6678df1 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -564,6 +564,19 @@ int has_prohibited_utf_bom(const char *enc, const char *data, size_t len)\n \t);\n }\n \n+int is_missing_required_utf_bom(const char *enc, const char *data, size_t len)\n+{\n+\treturn (\n+\t (!strcasecmp(enc, \"UTF-16\") || !strcasecmp(enc, \"UTF16\")) &&\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 (!strcasecmp(enc, \"UTF-32\") || !strcasecmp(enc, \"UTF32\")) &&\n+\t !(has_bom_prefix(data, len, utf32_be_bom, sizeof(utf32_be_bom)) ||\n+\t has_bom_prefix(data, len, utf32_le_bom, sizeof(utf32_le_bom)))\n+\t);\n+}\n+\n /*\n * Returns first character length in bytes for multi-byte `text` according to\n * `encoding`.\ndiff --git a/utf8.h b/utf8.h\nindex 0db1db4519..cce654a64a 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -79,4 +79,23 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid\n */\n int has_prohibited_utf_bom(const char *enc, const char *data, size_t len);\n \n+/*\n+ * If the endianness is not defined in the encoding name, then we\n+ * require a BOM. The function returns true if a required BOM is missing.\n+ *\n+ * The Unicode standard instructs to assume big-endian if there in no\n+ * BOM for UTF-16/32 [1][2]. However, the W3C/WHATWG encoding standard\n+ * used in HTML5 recommends to assume little-endian to \"deal with\n+ * deployed content\" [3].\n+ *\n+ * Therefore, strictly requiring a BOM seems to be the safest option for\n+ * content in Git.\n+ *\n+ * [1] http://unicode.org/faq/utf_bom.html#gen6\n+ * [2] http://www.unicode.org/versions/Unicode10.0.0/ch03.pdf\n+ * Section 3.10, D98, page 132\n+ * [3] https://encoding.spec.whatwg.org/#utf-16le\n+ */\n+int is_missing_required_utf_bom(const char *enc, const char *data, size_t len);\n+\n #endif\n-- \n2.16.2\n\n"},{"id":"341215","messageId":"20180307173026.30058-7-lars.schneider@autodesk.com","threadId":"48000","inReplyTo":"20180307173026.30058-1-lars.schneider@autodesk.com","subject":"[PATCH v10 6/9] convert: add 'working-tree-encoding' attribute","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-07T17:30:23Z","receivedAt":"2018-03-07T17:31:31Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nGit recognizes files encoded with ASCII or one of its supersets (e.g.\nUTF-8 or ISO-8859-1) as text files. All other encodings are usually\ninterpreted as binary and consequently built-in Git text processing\ntools (e.g. 'git diff') as well as most Git web front ends do not\nvisualize the content.\n\nAdd an attribute to tell Git what encoding the user has defined for a\ngiven file. If the content is added to the index, then Git converts the\ncontent to a canonical UTF-8 representation. On checkout Git will\nreverse the conversion.\n\nSigned-off-by: Lars Schneider \n---\n Documentation/gitattributes.txt | 80 +++++++++++++++++++++++\n convert.c | 110 +++++++++++++++++++++++++++++++-\n convert.h | 1 +\n sha1_file.c | 2 +-\n t/t0028-working-tree-encoding.sh | 133 +++++++++++++++++++++++++++++++++++++++\n 5 files changed, 324 insertions(+), 2 deletions(-)\n create mode 100755 t/t0028-working-tree-encoding.sh\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 30687de81a..31a4f92840 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -272,6 +272,86 @@ few exceptions. Even though...\n catch potential problems early, safety triggers.\n \n \n+`working-tree-encoding`\n+^^^^^^^^^^^^^^^^^^^^^^^\n+\n+Git recognizes files encoded in ASCII or one of its supersets (e.g.\n+UTF-8, ISO-8859-1, ...) as text files. Files encoded in certain other\n+encodings (e.g. UTF-16) are interpreted as binary and consequently\n+built-in Git text processing tools (e.g. 'git diff') as well as most Git\n+web front ends do not visualize the contents of these files by default.\n+\n+In these cases you can tell Git the encoding of a file in the working\n+directory with the `working-tree-encoding` attribute. If a file with this\n+attribute is added to Git, then Git reencodes the content from the\n+specified encoding to UTF-8. Finally, Git stores the UTF-8 encoded\n+content in its internal data structure (called \"the index\"). On checkout\n+the content is reencoded back to the specified encoding.\n+\n+Please note that using the `working-tree-encoding` attribute may have a\n+number of pitfalls:\n+\n+- Alternative Git implementations (e.g. JGit or libgit2) and older Git\n+ versions (as of March 2018) do not support the `working-tree-encoding`\n+ attribute. If you decide to use the `working-tree-encoding` attribute\n+ in your repository, then it is strongly recommended to ensure that all\n+ clients working with the repository support it.\n+\n+ For example, Microsoft Visual Studio resources files (`*.rc`) or\n+ PowerShell script files (`*.ps1`) are sometimes encoded in UTF-16.\n+ If you declare `*.ps1` as files as UTF-16 and you add `foo.ps1` with\n+ a `working-tree-encoding` enabled Git client, then `foo.ps1` will be\n+ stored as UTF-8 internally. A client without `working-tree-encoding`\n+ support will checkout `foo.ps1` as UTF-8 encoded file. This will\n+ typically cause trouble for the users of this file.\n+\n+ If a Git client, that does not support the `working-tree-encoding`\n+ attribute, adds a new file `bar.ps1`, then `bar.ps1` will be\n+ stored \"as-is\" internally (in this example probably as UTF-16).\n+ A client with `working-tree-encoding` support will interpret the\n+ internal contents as UTF-8 and try to convert it to UTF-16 on checkout.\n+ That operation will fail and cause an error.\n+\n+- Reencoding content requires resources that might slow down certain\n+ Git operations (e.g 'git checkout' or 'git add').\n+\n+Use the `working-tree-encoding` attribute only if you cannot store a file\n+in UTF-8 encoding and if you want Git to be able to process the content\n+as text.\n+\n+As an example, use the following attributes if your '*.ps1' files are\n+UTF-16 encoded with byte order mark (BOM) and you want Git to perform\n+automatic line ending conversion based on your platform.\n+\n+------------------------\n+*.ps1\t\ttext working-tree-encoding=UTF-16\n+------------------------\n+\n+Use the following attributes if your '*.ps1' files are UTF-16 little\n+endian encoded without BOM and you want Git to use Windows line endings\n+in the working directory. Please note, it is highly recommended to\n+explicitly define the line endings with `eol` if the `working-tree-encoding`\n+attribute is used to avoid ambiguity.\n+\n+------------------------\n+*.ps1\t\ttext working-tree-encoding=UTF-16LE eol=CRLF\n+------------------------\n+\n+You can get a list of all available encodings on your platform with the\n+following command:\n+\n+------------------------\n+iconv --list\n+------------------------\n+\n+If you do not know the encoding of a file, then you can use the `file`\n+command to guess the encoding:\n+\n+------------------------\n+file foo.ps1\n+------------------------\n+\n+\n `ident`\n ^^^^^^^\n \ndiff --git a/convert.c b/convert.c\nindex b976eb968c..80549ff2b5 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -7,6 +7,7 @@\n #include \"sigchain.h\"\n #include \"pkt-line.h\"\n #include \"sub-process.h\"\n+#include \"utf8.h\"\n \n /*\n * convert.c - convert a file when checking it out and checking it in.\n@@ -265,6 +266,78 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n \n }\n \n+static const char *default_encoding = \"UTF-8\";\n+\n+static int encode_to_git(const char *path, const char *src, size_t src_len,\n+\t\t\t struct strbuf *buf, const char *enc, int conv_flags)\n+{\n+\tchar *dst;\n+\tint dst_len;\n+\tint die_on_error = conv_flags & CONV_WRITE_OBJECT;\n+\n+\t/*\n+\t * No encoding is specified or there is nothing to encode.\n+\t * Tell the caller that the content was not modified.\n+\t */\n+\tif (!enc || (src && !src_len))\n+\t\treturn 0;\n+\n+\t/*\n+\t * Looks like we got called from \"would_convert_to_git()\".\n+\t * This means Git wants to know if it would encode (= modify!)\n+\t * the content. Let's answer with \"yes\", since an encoding was\n+\t * specified.\n+\t */\n+\tif (!buf && !src)\n+\t\treturn 1;\n+\n+\tdst = reencode_string_len(src, src_len, default_encoding, enc,\n+\t\t\t\t &dst_len);\n+\tif (!dst) {\n+\t\t/*\n+\t\t * We could add the blob \"as-is\" to Git. However, on checkout\n+\t\t * we would try to reencode to the original encoding. This\n+\t\t * would fail and we would leave the user with a messed-up\n+\t\t * working tree. Let's try to avoid this by screaming loud.\n+\t\t */\n+\t\tconst char* msg = _(\"failed to encode '%s' from %s to %s\");\n+\t\tif (die_on_error)\n+\t\t\tdie(msg, path, enc, default_encoding);\n+\t\telse {\n+\t\t\terror(msg, path, enc, default_encoding);\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\n+\tstrbuf_attach(buf, dst, dst_len, dst_len + 1);\n+\treturn 1;\n+}\n+\n+static int encode_to_worktree(const char *path, const char *src, size_t src_len,\n+\t\t\t struct strbuf *buf, const char *enc)\n+{\n+\tchar *dst;\n+\tint dst_len;\n+\n+\t/*\n+\t * No encoding is specified or there is nothing to encode.\n+\t * Tell the caller that the content was not modified.\n+\t */\n+\tif (!enc || (src && !src_len))\n+\t\treturn 0;\n+\n+\tdst = reencode_string_len(src, src_len, enc, default_encoding,\n+\t\t\t\t &dst_len);\n+\tif (!dst) {\n+\t\terror(\"failed to encode '%s' from %s to %s\",\n+\t\t\tpath, default_encoding, enc);\n+\t\treturn 0;\n+\t}\n+\n+\tstrbuf_attach(buf, dst, dst_len, dst_len + 1);\n+\treturn 1;\n+}\n+\n static int crlf_to_git(const struct index_state *istate,\n \t\t const char *path, const char *src, size_t len,\n \t\t struct strbuf *buf,\n@@ -978,6 +1051,21 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,\n \treturn 1;\n }\n \n+static const char *git_path_check_encoding(struct attr_check_item *check)\n+{\n+\tconst char *value = check->value;\n+\n+\tif (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value) ||\n+\t !strlen(value))\n+\t\treturn NULL;\n+\n+\t/* Don't encode to the default encoding */\n+\tif (!strcasecmp(value, default_encoding))\n+\t\treturn NULL;\n+\n+\treturn value;\n+}\n+\n static enum crlf_action git_path_check_crlf(struct attr_check_item *check)\n {\n \tconst char *value = check->value;\n@@ -1033,6 +1121,7 @@ struct conv_attrs {\n \tenum crlf_action attr_action; /* What attr says */\n \tenum crlf_action crlf_action; /* When no attr is set, use core.autocrlf */\n \tint ident;\n+\tconst char *working_tree_encoding; /* Supported encoding or default encoding if NULL */\n };\n \n static void convert_attrs(struct conv_attrs *ca, const char *path)\n@@ -1041,7 +1130,8 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)\n \n \tif (!check) {\n \t\tcheck = attr_check_initl(\"crlf\", \"ident\", \"filter\",\n-\t\t\t\t\t \"eol\", \"text\", NULL);\n+\t\t\t\t\t \"eol\", \"text\", \"working-tree-encoding\",\n+\t\t\t\t\t NULL);\n \t\tuser_convert_tail = &user_convert;\n \t\tgit_config(read_convert_config, NULL);\n \t}\n@@ -1064,6 +1154,7 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)\n \t\t\telse if (eol_attr == EOL_CRLF)\n \t\t\t\tca->crlf_action = CRLF_TEXT_CRLF;\n \t\t}\n+\t\tca->working_tree_encoding = git_path_check_encoding(ccheck + 5);\n \t} else {\n \t\tca->drv = NULL;\n \t\tca->crlf_action = CRLF_UNDEFINED;\n@@ -1144,6 +1235,13 @@ int convert_to_git(const struct index_state *istate,\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n+\n+\tret |= encode_to_git(path, src, len, dst, ca.working_tree_encoding, conv_flags);\n+\tif (ret && dst) {\n+\t\tsrc = dst->buf;\n+\t\tlen = dst->len;\n+\t}\n+\n \tif (!(conv_flags & CONV_EOL_KEEP_CRLF)) {\n \t\tret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, conv_flags);\n \t\tif (ret && dst) {\n@@ -1167,6 +1265,7 @@ void convert_to_git_filter_fd(const struct index_state *istate,\n \tif (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN, NULL))\n \t\tdie(\"%s: clean filter '%s' failed\", path, ca.drv->name);\n \n+\tencode_to_git(path, dst->buf, dst->len, dst, ca.working_tree_encoding, conv_flags);\n \tcrlf_to_git(istate, path, dst->buf, dst->len, dst, ca.crlf_action, conv_flags);\n \tident_to_git(path, dst->buf, dst->len, dst, ca.ident);\n }\n@@ -1198,6 +1297,12 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t\t}\n \t}\n \n+\tret |= encode_to_worktree(path, src, len, dst, ca.working_tree_encoding);\n+\tif (ret) {\n+\t\tsrc = dst->buf;\n+\t\tlen = dst->len;\n+\t}\n+\n \tret_filter = apply_filter(\n \t\tpath, src, len, -1, dst, ca.drv, CAP_SMUDGE, dco);\n \tif (!ret_filter && ca.drv && ca.drv->required)\n@@ -1664,6 +1769,9 @@ struct stream_filter *get_stream_filter(const char *path, const unsigned char *s\n \tif (ca.drv && (ca.drv->process || ca.drv->smudge || ca.drv->clean))\n \t\treturn NULL;\n \n+\tif (ca.working_tree_encoding)\n+\t\treturn NULL;\n+\n \tif (ca.crlf_action == CRLF_AUTO || ca.crlf_action == CRLF_AUTO_CRLF)\n \t\treturn NULL;\n \ndiff --git a/convert.h b/convert.h\nindex 65ab3e5167..1d9539ed0b 100644\n--- a/convert.h\n+++ b/convert.h\n@@ -12,6 +12,7 @@ struct index_state;\n #define CONV_EOL_RNDTRP_WARN (1<<1) /* Warn if CRLF to LF to CRLF is different */\n #define CONV_EOL_RENORMALIZE (1<<2) /* Convert CRLF to LF */\n #define CONV_EOL_KEEP_CRLF (1<<3) /* Keep CRLF line endings as is */\n+#define CONV_WRITE_OBJECT (1<<4) /* Content is written to the index */\n \n extern int global_conv_flags_eol;\n \ndiff --git a/sha1_file.c b/sha1_file.c\nindex 6bc7c6ada9..e2f319d677 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -138,7 +138,7 @@ static int get_conv_flags(unsigned flags)\n \tif (flags & HASH_RENORMALIZE)\n \t\treturn CONV_EOL_RENORMALIZE;\n \telse if (flags & HASH_WRITE_OBJECT)\n-\t return global_conv_flags_eol;\n+\t\treturn global_conv_flags_eol | CONV_WRITE_OBJECT;\n \telse\n \t\treturn 0;\n }\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nnew file mode 100755\nindex 0000000000..89b4dbee1d\n--- /dev/null\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -0,0 +1,133 @@\n+#!/bin/sh\n+\n+test_description='working-tree-encoding conversion via gitattributes'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup test files' '\n+\tgit config core.eol lf &&\n+\n+\ttext=\"hallo there!\\ncan you read me?\" &&\n+\techo \"*.utf16 text working-tree-encoding=utf-16\" >.gitattributes &&\n+\tprintf \"$text\" >test.utf8.raw &&\n+\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&\n+\tprintf \"$text\" | iconv -f UTF-8 -t UTF-32 >test.utf32.raw &&\n+\n+\t# Line ending tests\n+\tprintf \"one\\ntwo\\nthree\\n\" >lf.utf8.raw &&\n+\tprintf \"one\\r\\ntwo\\r\\nthree\\r\\n\" >crlf.utf8.raw &&\n+\n+\t# BOM tests\n+\tprintf \"\\0a\\0b\\0c\" >nobom.utf16be.raw &&\n+\tprintf \"a\\0b\\0c\\0\" >nobom.utf16le.raw &&\n+\tprintf \"\\376\\777\\0a\\0b\\0c\" >bebom.utf16be.raw &&\n+\tprintf \"\\777\\376a\\0b\\0c\\0\" >lebom.utf16le.raw &&\n+\tprintf \"\\0\\0\\0a\\0\\0\\0b\\0\\0\\0c\" >nobom.utf32be.raw &&\n+\tprintf \"a\\0\\0\\0b\\0\\0\\0c\\0\\0\\0\" >nobom.utf32le.raw &&\n+\tprintf \"\\0\\0\\376\\777\\0\\0\\0a\\0\\0\\0b\\0\\0\\0c\" >bebom.utf32be.raw &&\n+\tprintf \"\\777\\376\\0\\0a\\0\\0\\0b\\0\\0\\0c\\0\\0\\0\" >lebom.utf32le.raw &&\n+\n+\t# Add only UTF-16 file, we will add the UTF-32 file later\n+\tcp test.utf16.raw test.utf16 &&\n+\tcp test.utf32.raw test.utf32 &&\n+\tgit add .gitattributes test.utf16 &&\n+\tgit commit -m initial\n+'\n+\n+test_expect_success 'ensure UTF-8 is stored in Git' '\n+\ttest_when_finished \"rm -f test.utf16.git\" &&\n+\n+\tgit cat-file -p :test.utf16 >test.utf16.git &&\n+\ttest_cmp_bin test.utf8.raw test.utf16.git\n+'\n+\n+test_expect_success 're-encode to UTF-16 on checkout' '\n+\ttest_when_finished \"rm -f test.utf16.raw\" &&\n+\n+\trm test.utf16 &&\n+\tgit checkout test.utf16 &&\n+\ttest_cmp_bin test.utf16.raw test.utf16\n+'\n+\n+test_expect_success 'check $GIT_DIR/info/attributes support' '\n+\ttest_when_finished \"rm -f test.utf32.git\" &&\n+\ttest_when_finished \"git reset --hard HEAD\" &&\n+\n+\techo \"*.utf32 text working-tree-encoding=utf-32\" >.git/info/attributes &&\n+\tgit add test.utf32 &&\n+\n+\tgit cat-file -p :test.utf32 >test.utf32.git &&\n+\ttest_cmp_bin test.utf8.raw test.utf32.git\n+'\n+\n+for i in 16 32\n+do\n+\ttest_expect_success \"eol conversion for UTF-${i} encoded files on checkout\" '\n+\t\ttest_when_finished \"rm -f crlf.utf${i}.raw lf.utf${i}.raw\" &&\n+\t\ttest_when_finished \"git reset --hard HEAD^\" &&\n+\n+\t\tcat lf.utf8.raw | iconv -f UTF-8 -t UTF-${i} >lf.utf${i}.raw &&\n+\t\tcat crlf.utf8.raw | iconv -f UTF-8 -t UTF-${i} >crlf.utf${i}.raw &&\n+\t\tcp crlf.utf${i}.raw eol.utf${i} &&\n+\n+\t\tcat >expectIndexLF <<-EOF &&\n+\t\t\ti/lf w/-text attr/text \teol.utf${i}\n+\t\tEOF\n+\n+\t\tgit add eol.utf${i} &&\n+\t\tgit commit -m eol &&\n+\n+\t\t# UTF-${i} with CRLF (Windows line endings)\n+\t\trm eol.utf${i} &&\n+\t\tgit -c core.eol=crlf checkout eol.utf${i} &&\n+\t\ttest_cmp_bin crlf.utf${i}.raw eol.utf${i} &&\n+\n+\t\t# Although the file has CRLF in the working tree,\n+\t\t# ensure LF in the index\n+\t\tgit ls-files --eol eol.utf${i} >actual &&\n+\t\ttest_cmp expectIndexLF actual &&\n+\n+\t\t# UTF-${i} with LF (Unix line endings)\n+\t\trm eol.utf${i} &&\n+\t\tgit -c core.eol=lf checkout eol.utf${i} &&\n+\t\ttest_cmp_bin lf.utf${i}.raw eol.utf${i} &&\n+\n+\t\t# The file LF in the working tree, ensure LF in the index\n+\t\tgit ls-files --eol eol.utf${i} >actual &&\n+\t\ttest_cmp expectIndexLF actual\n+\t'\n+done\n+\n+test_expect_success 'check unsupported encodings' '\n+\ttest_when_finished \"git reset --hard HEAD\" &&\n+\n+\techo \"*.nothing text working-tree-encoding=\" >>.gitattributes &&\n+\tprintf \"nothing\" >t.nothing &&\n+\tgit add t.nothing &&\n+\n+\techo \"*.garbage text working-tree-encoding=garbage\" >>.gitattributes &&\n+\tprintf \"garbage\" >t.garbage &&\n+\ttest_must_fail git add t.garbage 2>err.out &&\n+\ttest_i18ngrep \"fatal: failed to encode\" err.out\n+'\n+\n+test_expect_success 'error if encoding round trip is not the same during refresh' '\n+\tBEFORE_STATE=$(git rev-parse HEAD) &&\n+\ttest_when_finished \"git reset --hard $BEFORE_STATE\" &&\n+\n+\t# Add and commit a UTF-16 file but skip the \"working-tree-encoding\"\n+\t# filter. Consequently, the in-repo representation is UTF-16 and not\n+\t# UTF-8. This simulates a Git version that has no working tree encoding\n+\t# support.\n+\techo \"*.utf16le text working-tree-encoding=utf-16le\" >.gitattributes &&\n+\techo \"hallo\" >nonsense.utf16le &&\n+\tTEST_HASH=$(git hash-object --no-filters -w nonsense.utf16le) &&\n+\tgit update-index --add --cacheinfo 100644 $TEST_HASH nonsense.utf16le &&\n+\tCOMMIT=$(git commit-tree -p $(git rev-parse HEAD) -m \"plain commit\" $(git write-tree)) &&\n+\tgit update-ref refs/heads/master $COMMIT &&\n+\n+\ttest_must_fail git checkout HEAD^ 2>err.out &&\n+\ttest_i18ngrep \"error: .* overwritten by checkout:\" err.out\n+'\n+\n+test_done\n-- \n2.16.2\n\n"},{"id":"341216","messageId":"20180307173026.30058-8-lars.schneider@autodesk.com","threadId":"48000","inReplyTo":"20180307173026.30058-1-lars.schneider@autodesk.com","subject":"[PATCH v10 7/9] convert: check for detectable errors in UTF encodings","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-07T17:30:24Z","receivedAt":"2018-03-07T17:31:35Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nCheck that new content is valid with respect to the user defined\n'working-tree-encoding' attribute.\n\nSigned-off-by: Lars Schneider \n---\n convert.c | 55 +++++++++++++++++++++++++++++++++++++++\n t/t0028-working-tree-encoding.sh | 56 ++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 111 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex 80549ff2b5..f19a15dd02 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -266,6 +266,58 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n \n }\n \n+static int validate_encoding(const char *path, const char *enc,\n+\t\t const char *data, size_t len, int die_on_error)\n+{\n+\t/* We only check for UTF here as UTF?? can be an alias for UTF-?? */\n+\tif (startscase_with(enc, \"UTF\")) {\n+\t\t/*\n+\t\t * Check for detectable errors in UTF encodings\n+\t\t */\n+\t\tif (has_prohibited_utf_bom(enc, data, len)) {\n+\t\t\tconst char *error_msg = _(\n+\t\t\t\t\"BOM is prohibited in '%s' if encoded as %s\");\n+\t\t\t/*\n+\t\t\t * This advice is shown for UTF-??BE and UTF-??LE encodings.\n+\t\t\t * We cut off the last two characters of the encoding name\n+\t\t\t # to generate the encoding name suitable for BOMs.\n+\t\t\t */\n+\t\t\tconst char *advise_msg = _(\n+\t\t\t\t\"The file '%s' contains a byte order \"\n+\t\t\t\t\"mark (BOM). Please use %s as \"\n+\t\t\t\t\"working-tree-encoding.\");\n+\t\t\tchar *upper_enc = xstrdup_toupper(enc);\n+\t\t\tupper_enc[strlen(upper_enc)-2] = '\\0';\n+\t\t\tadvise(advise_msg, path, upper_enc);\n+\t\t\tfree(upper_enc);\n+\t\t\tif (die_on_error)\n+\t\t\t\tdie(error_msg, path, enc);\n+\t\t\telse {\n+\t\t\t\treturn error(error_msg, path, enc);\n+\t\t\t}\n+\n+\t\t} else if (is_missing_required_utf_bom(enc, data, len)) {\n+\t\t\tconst char *error_msg = _(\n+\t\t\t\t\"BOM is required in '%s' if encoded as %s\");\n+\t\t\tconst char *advise_msg = _(\n+\t\t\t\t\"The file '%s' is missing a byte order \"\n+\t\t\t\t\"mark (BOM). Please use %sBE or %sLE \"\n+\t\t\t\t\"(depending on the byte order) as \"\n+\t\t\t\t\"working-tree-encoding.\");\n+\t\t\tchar *upper_enc = xstrdup_toupper(enc);\n+\t\t\tadvise(advise_msg, path, upper_enc, upper_enc);\n+\t\t\tfree(upper_enc);\n+\t\t\tif (die_on_error)\n+\t\t\t\tdie(error_msg, path, enc);\n+\t\t\telse {\n+\t\t\t\treturn error(error_msg, path, enc);\n+\t\t\t}\n+\t\t}\n+\n+\t}\n+\treturn 0;\n+}\n+\n static const char *default_encoding = \"UTF-8\";\n \n static int encode_to_git(const char *path, const char *src, size_t src_len,\n@@ -291,6 +343,9 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \tif (!buf && !src)\n \t\treturn 1;\n \n+\tif (validate_encoding(path, enc, src, src_len, die_on_error))\n+\t\treturn 0;\n+\n \tdst = reencode_string_len(src, src_len, default_encoding, enc,\n \t\t\t\t &dst_len);\n \tif (!dst) {\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex 89b4dbee1d..aa05f82166 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -62,6 +62,46 @@ test_expect_success 'check $GIT_DIR/info/attributes support' '\n \n for i in 16 32\n do\n+\ttest_expect_success \"check prohibited UTF-${i} BOM\" '\n+\t\ttest_when_finished \"git reset --hard HEAD\" &&\n+\n+\t\techo \"*.utf${i}be text working-tree-encoding=utf-${i}be\" >>.gitattributes &&\n+\t\techo \"*.utf${i}le text working-tree-encoding=utf-${i}le\" >>.gitattributes &&\n+\n+\t\t# Here we add a UTF-16 (resp. UTF-32) files with BOM (big/little-endian)\n+\t\t# but we tell Git to treat it as UTF-16BE/UTF-16LE (resp. UTF-32).\n+\t\t# In these cases the BOM is prohibited.\n+\t\tcp bebom.utf${i}be.raw bebom.utf${i}be &&\n+\t\ttest_must_fail git add bebom.utf${i}be 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* utf-${i}be\" err.out &&\n+\n+\t\tcp lebom.utf${i}le.raw lebom.utf${i}be &&\n+\t\ttest_must_fail git add lebom.utf${i}be 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* utf-${i}be\" err.out &&\n+\n+\t\tcp bebom.utf${i}be.raw bebom.utf${i}le &&\n+\t\ttest_must_fail git add bebom.utf${i}le 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* utf-${i}le\" err.out &&\n+\n+\t\tcp lebom.utf${i}le.raw lebom.utf${i}le &&\n+\t\ttest_must_fail git add lebom.utf${i}le 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is prohibited .* utf-${i}le\" err.out\n+\t'\n+\n+\ttest_expect_success \"check required UTF-${i} BOM\" '\n+\t\ttest_when_finished \"git reset --hard HEAD\" &&\n+\n+\t\techo \"*.utf${i} text working-tree-encoding=utf-${i}\" >>.gitattributes &&\n+\n+\t\tcp nobom.utf${i}be.raw nobom.utf${i} &&\n+\t\ttest_must_fail git add nobom.utf${i} 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is required .* utf-${i}\" err.out &&\n+\n+\t\tcp nobom.utf${i}le.raw nobom.utf${i} &&\n+\t\ttest_must_fail git add nobom.utf${i} 2>err.out &&\n+\t\ttest_i18ngrep \"fatal: BOM is required .* utf-${i}\" err.out\n+\t'\n+\n \ttest_expect_success \"eol conversion for UTF-${i} encoded files on checkout\" '\n \t\ttest_when_finished \"rm -f crlf.utf${i}.raw lf.utf${i}.raw\" &&\n \t\ttest_when_finished \"git reset --hard HEAD^\" &&\n@@ -130,4 +170,20 @@ test_expect_success 'error if encoding round trip is not the same during refresh\n \ttest_i18ngrep \"error: .* overwritten by checkout:\" err.out\n '\n \n+test_expect_success 'error if encoding garbage is already in Git' '\n+\tBEFORE_STATE=$(git rev-parse HEAD) &&\n+\ttest_when_finished \"git reset --hard $BEFORE_STATE\" &&\n+\n+\t# Skip the UTF-16 filter for the added file\n+\t# This simulates a Git version that has no checkoutEncoding support\n+\tcp nobom.utf16be.raw nonsense.utf16 &&\n+\tTEST_HASH=$(git hash-object --no-filters -w nonsense.utf16) &&\n+\tgit update-index --add --cacheinfo 100644 $TEST_HASH nonsense.utf16 &&\n+\tCOMMIT=$(git commit-tree -p $(git rev-parse HEAD) -m \"plain commit\" $(git write-tree)) &&\n+\tgit update-ref refs/heads/master $COMMIT &&\n+\n+\tgit diff 2>err.out &&\n+\ttest_i18ngrep \"error: BOM is required\" err.out\n+'\n+\n test_done\n-- \n2.16.2\n\n"},{"id":"341217","messageId":"20180307173026.30058-10-lars.schneider@autodesk.com","threadId":"48000","inReplyTo":"20180307173026.30058-1-lars.schneider@autodesk.com","subject":"[PATCH v10 9/9] convert: add round trip check based on 'core.checkRoundtripEncoding'","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-07T17:30:26Z","receivedAt":"2018-03-07T17:31:37Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nUTF supports lossless conversion round tripping and conversions between\nUTF and other encodings are mostly round trip safe as Unicode aims to be\na superset of all other character encodings. However, certain encodings\n(e.g. SHIFT-JIS) are known to have round trip issues [1].\n\nAdd 'core.checkRoundtripEncoding', which contains a comma separated\nlist of encodings, to define for what encodings Git should check the\nconversion round trip if they are used in the 'working-tree-encoding'\nattribute.\n\nSet SHIFT-JIS as default value for 'core.checkRoundtripEncoding'.\n\n[1] https://support.microsoft.com/en-us/help/170559/prb-conversion-problem-between-shift-jis-and-unicode\n\nSigned-off-by: Lars Schneider \n---\n Documentation/config.txt | 6 ++++\n Documentation/gitattributes.txt | 8 +++++\n config.c | 5 +++\n convert.c | 78 ++++++++++++++++++++++++++++++++++++++++\n convert.h | 1 +\n environment.c | 1 +\n t/t0028-working-tree-encoding.sh | 39 ++++++++++++++++++++\n 7 files changed, 138 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 0e25b2c92b..d7a56054a5 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -530,6 +530,12 @@ core.autocrlf::\n \tThis variable can be set to 'input',\n \tin which case no output conversion is performed.\n \n+core.checkRoundtripEncoding::\n+\tA comma separated list of encodings that Git performs UTF-8 round\n+\ttrip checks on if they are used in an `working-tree-encoding`\n+\tattribute (see linkgit:gitattributes[5]). The default value is\n+\t`SHIFT-JIS`.\n+\n core.symlinks::\n \tIf false, symbolic links are checked out as small plain files that\n \tcontain the link text. linkgit:git-update-index[1] and\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 31a4f92840..aa3deae392 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -312,6 +312,14 @@ number of pitfalls:\n internal contents as UTF-8 and try to convert it to UTF-16 on checkout.\n That operation will fail and cause an error.\n \n+- Reencoding content to non-UTF encodings can cause errors as the\n+ conversion might not be UTF-8 round trip safe. If you suspect your\n+ encoding to not be round trip safe, then add it to\n+ `core.checkRoundtripEncoding` to make Git check the round trip\n+ encoding (see linkgit:git-config[1]). SHIFT-JIS (Japanese character\n+ set) is known to have round trip issues with UTF-8 and is checked by\n+ default.\n+\n - Reencoding content requires resources that might slow down certain\n Git operations (e.g 'git checkout' or 'git add').\n \ndiff --git a/config.c b/config.c\nindex 1f003fbb90..d0ada9fcd4 100644\n--- a/config.c\n+++ b/config.c\n@@ -1172,6 +1172,11 @@ static int git_default_core_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.checkroundtripencoding\")) {\n+\t\tcheck_roundtrip_encoding = xstrdup(value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"core.notesref\")) {\n \t\tnotes_ref_name = xstrdup(value);\n \t\treturn 0;\ndiff --git a/convert.c b/convert.c\nindex 5776dfbc99..e861f1abbc 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -341,6 +341,43 @@ static void trace_encoding(const char *context, const char *path,\n \tstrbuf_release(&trace);\n }\n \n+static int check_roundtrip(const char* enc_name)\n+{\n+\t/*\n+\t * check_roundtrip_encoding contains a string of space and/or\n+\t * comma separated encodings (eg. \"UTF-16, ASCII, CP1125\").\n+\t * Search for the given encoding in that string.\n+\t */\n+\tconst char *found = strcasestr(check_roundtrip_encoding, enc_name);\n+\tconst char *next;\n+\tint len;\n+\tif (!found)\n+\t\treturn 0;\n+\tnext = found + strlen(enc_name);\n+\tlen = strlen(check_roundtrip_encoding);\n+\treturn (found && (\n+\t\t\t/*\n+\t\t\t * check that the found encoding is at the\n+\t\t\t * beginning of check_roundtrip_encoding or\n+\t\t\t * that it is prefixed with a space or comma\n+\t\t\t */\n+\t\t\tfound == check_roundtrip_encoding || (\n+\t\t\t\tfound > check_roundtrip_encoding &&\n+\t\t\t\t(*(found-1) == ' ' || *(found-1) == ',')\n+\t\t\t)\n+\t\t) && (\n+\t\t\t/*\n+\t\t\t * check that the found encoding is at the\n+\t\t\t * end of check_roundtrip_encoding or\n+\t\t\t * that it is suffixed with a space or comma\n+\t\t\t */\n+\t\t\tnext == check_roundtrip_encoding + len || (\n+\t\t\t\tnext < check_roundtrip_encoding + len &&\n+\t\t\t\t(*next == ' ' || *next == ',')\n+\t\t\t)\n+\t\t));\n+}\n+\n static const char *default_encoding = \"UTF-8\";\n \n static int encode_to_git(const char *path, const char *src, size_t src_len,\n@@ -389,6 +426,47 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \t}\n \ttrace_encoding(\"destination\", path, default_encoding, dst, dst_len);\n \n+\t/*\n+\t * UTF supports lossless conversion round tripping [1] and conversions\n+\t * between UTF and other encodings are mostly round trip safe as\n+\t * Unicode aims to be a superset of all other character encodings.\n+\t * However, certain encodings (e.g. SHIFT-JIS) are known to have round\n+\t * trip issues [2]. Check the round trip conversion for all encodings\n+\t * listed in core.checkRoundtripEncoding.\n+\t *\n+\t * The round trip check is only performed if content is written to Git.\n+\t * This ensures that no information is lost during conversion to/from\n+\t * the internal UTF-8 representation.\n+\t *\n+\t * Please note, the code below is not tested because I was not able to\n+\t * generate a faulty round trip without an iconv error. Iconv errors\n+\t * are already caught above.\n+\t *\n+\t * [1] http://unicode.org/faq/utf_bom.html#gen2\n+\t * [2] https://support.microsoft.com/en-us/help/170559/prb-conversion-problem-between-shift-jis-and-unicode\n+\t */\n+\tif (die_on_error && check_roundtrip(enc)) {\n+\t\tchar *re_src;\n+\t\tint re_src_len;\n+\n+\t\tre_src = reencode_string_len(dst, dst_len,\n+\t\t\t\t\t enc, default_encoding,\n+\t\t\t\t\t &re_src_len);\n+\n+\t\ttrace_printf(\"Checking roundtrip encoding for %s...\\n\", enc);\n+\t\ttrace_encoding(\"reencoded source\", path, enc,\n+\t\t\t re_src, re_src_len);\n+\n+\t\tif (!re_src || src_len != re_src_len ||\n+\t\t memcmp(src, re_src, src_len)) {\n+\t\t\tconst char* msg = _(\"encoding '%s' from %s to %s and \"\n+\t\t\t\t\t \"back is not the same\");\n+\t\t\tdie(msg, path, enc, default_encoding);\n+\t\t}\n+\n+\t\tfree(re_src);\n+\t}\n+\n \tstrbuf_attach(buf, dst, dst_len, dst_len + 1);\n \treturn 1;\n }\ndiff --git a/convert.h b/convert.h\nindex 1d9539ed0b..765abfbd60 100644\n--- a/convert.h\n+++ b/convert.h\n@@ -56,6 +56,7 @@ struct delayed_checkout {\n };\n \n extern enum eol core_eol;\n+extern char *check_roundtrip_encoding;\n extern const char *get_cached_convert_stats_ascii(const struct index_state *istate,\n \t\t\t\t\t\t const char *path);\n extern const char *get_wt_convert_stats_ascii(const char *path);\ndiff --git a/environment.c b/environment.c\nindex 10a32c20ac..5bae9131ad 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -50,6 +50,7 @@ int check_replace_refs = 1;\n char *git_replace_ref_base;\n enum eol core_eol = EOL_UNSET;\n int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;\n+char *check_roundtrip_encoding = \"SHIFT-JIS\";\n unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;\n enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;\n enum rebase_setup_type autorebase = AUTOREBASE_NEVER;\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex 6f3a82f61b..7cff41a350 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -188,4 +188,43 @@ test_expect_success 'error if encoding garbage is already in Git' '\n \ttest_i18ngrep \"error: BOM is required\" err.out\n '\n \n+test_expect_success 'check roundtrip encoding' '\n+\ttest_when_finished \"rm -f roundtrip.shift roundtrip.utf16\" &&\n+\ttest_when_finished \"git reset --hard HEAD\" &&\n+\n+\ttext=\"hallo there!\\nroundtrip test here!\" &&\n+\tprintf \"$text\" | iconv -f UTF-8 -t SHIFT-JIS >roundtrip.shift &&\n+\tprintf \"$text\" | iconv -f UTF-8 -t UTF-16 >roundtrip.utf16 &&\n+\techo \"*.shift text working-tree-encoding=SHIFT-JIS\" >>.gitattributes &&\n+\n+\t# SHIFT-JIS encoded files are round-trip checked by default...\n+\tGIT_TRACE=1 git add .gitattributes roundtrip.shift 2>&1 |\n+\t\tgrep \"Checking roundtrip encoding for SHIFT-JIS\" &&\n+\tgit reset &&\n+\n+\t# ... unless we overwrite the Git config!\n+\t! GIT_TRACE=1 git -c core.checkRoundtripEncoding=garbage \\\n+\t\tadd .gitattributes roundtrip.shift 2>&1 |\n+\t\tgrep \"Checking roundtrip encoding for SHIFT-JIS\" &&\n+\tgit reset &&\n+\n+\t# UTF-16 encoded files should not be round-trip checked by default...\n+\t! GIT_TRACE=1 git add roundtrip.utf16 2>&1 |\n+\t\tgrep \"Checking roundtrip encoding for UTF-16\" &&\n+\tgit reset &&\n+\n+\t# ... unless we tell Git to check it!\n+\tGIT_TRACE=1 git -c core.checkRoundtripEncoding=\"UTF-16, UTF-32\" \\\n+\t\tadd roundtrip.utf16 2>&1 |\n+\t\tgrep \"Checking roundtrip encoding for utf-16\" &&\n+\tgit reset &&\n+\n+\t# ... unless we tell Git to check it!\n+\t# (here we also check that the casing of the encoding is irrelevant)\n+\tGIT_TRACE=1 git -c core.checkRoundtripEncoding=\"UTF-32, utf-16\" \\\n+\t\tadd roundtrip.utf16 2>&1 |\n+\t\tgrep \"Checking roundtrip encoding for utf-16\" &&\n+\tgit reset\n+'\n+\n test_done\n-- \n2.16.2\n\n"},{"id":"341218","messageId":"20180307173026.30058-9-lars.schneider@autodesk.com","threadId":"48000","inReplyTo":"20180307173026.30058-1-lars.schneider@autodesk.com","subject":"[PATCH v10 8/9] convert: add tracing for 'working-tree-encoding' attribute","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2018-03-07T17:30:25Z","receivedAt":"2018-03-07T17:31:48Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nAdd the GIT_TRACE_WORKING_TREE_ENCODING environment variable to enable\ntracing for content that is reencoded with the 'working-tree-encoding'\nattribute. This is useful to debug encoding issues.\n\nSigned-off-by: Lars Schneider \n---\n convert.c | 25 +++++++++++++++++++++++++\n t/t0028-working-tree-encoding.sh | 2 ++\n 2 files changed, 27 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex f19a15dd02..5776dfbc99 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -318,6 +318,29 @@ static int validate_encoding(const char *path, const char *enc,\n \treturn 0;\n }\n \n+static void trace_encoding(const char *context, const char *path,\n+\t\t\t const char *encoding, const char *buf, size_t len)\n+{\n+\tstatic struct trace_key coe = TRACE_KEY_INIT(WORKING_TREE_ENCODING);\n+\tstruct strbuf trace = STRBUF_INIT;\n+\tint i;\n+\n+\tstrbuf_addf(&trace, \"%s (%s, considered %s):\\n\", context, path, encoding);\n+\tfor (i = 0; i < len && buf; ++i) {\n+\t\tstrbuf_addf(\n+\t\t\t&trace,\"| \\e[2m%2i:\\e[0m %2x \\e[2m%c\\e[0m%c\",\n+\t\t\ti,\n+\t\t\t(unsigned char) buf[i],\n+\t\t\t(buf[i] > 32 && buf[i] < 127 ? buf[i] : ' '),\n+\t\t\t((i+1) % 8 && (i+1) < len ? ' ' : '\\n')\n+\t\t);\n+\t}\n+\tstrbuf_addchars(&trace, '\\n', 1);\n+\n+\ttrace_strbuf(&coe, &trace);\n+\tstrbuf_release(&trace);\n+}\n+\n static const char *default_encoding = \"UTF-8\";\n \n static int encode_to_git(const char *path, const char *src, size_t src_len,\n@@ -346,6 +369,7 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \tif (validate_encoding(path, enc, src, src_len, die_on_error))\n \t\treturn 0;\n \n+\ttrace_encoding(\"source\", path, enc, src, src_len);\n \tdst = reencode_string_len(src, src_len, default_encoding, enc,\n \t\t\t\t &dst_len);\n \tif (!dst) {\n@@ -363,6 +387,7 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,\n \t\t\treturn 0;\n \t\t}\n \t}\n+\ttrace_encoding(\"destination\", path, default_encoding, dst, dst_len);\n \n \tstrbuf_attach(buf, dst, dst_len, dst_len + 1);\n \treturn 1;\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex aa05f82166..6f3a82f61b 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -4,6 +4,8 @@ test_description='working-tree-encoding conversion via gitattributes'\n \n . ./test-lib.sh\n \n+GIT_TRACE_WORKING_TREE_ENCODING=1 && export GIT_TRACE_WORKING_TREE_ENCODING\n+\n test_expect_success 'setup test files' '\n \tgit config core.eol lf &&\n \n-- \n2.16.2\n\n"},{"id":"341220","messageId":"CAPig+cSvC1a5EaDfFZdJU74mZz84M=CkeOJLSDxFdXKcJYyzEw@mail.gmail.com","threadId":"48000","inReplyTo":"20180307173026.30058-7-lars.schneider@autodesk.com","subject":"Re: [PATCH v10 6/9] convert: add 'working-tree-encoding' attribute","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-03-07T17:54:51Z","receivedAt":"2018-03-07T17:54:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Mar 7, 2018 at 12:30 PM, wrote:\n> [...]\n> Add an attribute to tell Git what encoding the user has defined for a\n> given file. If the content is added to the index, then Git converts the\n> content to a canonical UTF-8 representation. On checkout Git will\n> reverse the conversion.\n>\n> Signed-off-by: Lars Schneider \n> ---\n> Documentation/gitattributes.txt | 80 +++++++++++++++++++++++\n> diff --git a/convert.c b/convert.c\n> @@ -265,6 +266,78 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n> +static const char *default_encoding = \"UTF-8\";\n> @@ -978,6 +1051,21 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,\n> +static const char *git_path_check_encoding(struct attr_check_item *check)\n> +{\n> + const char *value = check->value;\n> +\n> + if (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value) ||\n> + !strlen(value))\n> + return NULL;\n> +\n> + /* Don't encode to the default encoding */\n> + if (!strcasecmp(value, default_encoding))\n> + return NULL;\n\nAs of v10, the rest of the code accepts encoding names \"UTF-xx\" and\n\"UTFxx\" (case insensitive), however, this check recognizes only\n\"UTF-8\" (case insensitive). For consistency, one would expect this\nalso to recognize \"UTF8\" (case insensitive).\n\n> +\n> + return value;\n> +}\n"},{"id":"341221","messageId":"CAPig+cTsRufRNZbKJS-02fvbtaLB30FO3Em6HR_LZrsK+CfOjw@mail.gmail.com","threadId":"48000","inReplyTo":"20180307173026.30058-8-lars.schneider@autodesk.com","subject":"Re: [PATCH v10 7/9] convert: check for detectable errors in UTF encodings","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-03-07T18:04:44Z","receivedAt":"2018-03-07T18:04:50Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Mar 7, 2018 at 12:30 PM, wrote:\n> Check that new content is valid with respect to the user defined\n> 'working-tree-encoding' attribute.\n>\n> Signed-off-by: Lars Schneider \n> ---\n> diff --git a/convert.c b/convert.c\n> @@ -266,6 +266,58 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n> +static int validate_encoding(const char *path, const char *enc,\n> + const char *data, size_t len, int die_on_error)\n> +{\n> + /* We only check for UTF here as UTF?? can be an alias for UTF-?? */\n> + if (startscase_with(enc, \"UTF\")) {\n> + /*\n> + * Check for detectable errors in UTF encodings\n> + */\n> + if (has_prohibited_utf_bom(enc, data, len)) {\n> + const char *error_msg = _(\n> + \"BOM is prohibited in '%s' if encoded as %s\");\n> + /*\n> + * This advice is shown for UTF-??BE and UTF-??LE encodings.\n> + * We cut off the last two characters of the encoding name\n> + # to generate the encoding name suitable for BOMs.\n\ns/#/*/\n\n> + */\n> + const char *advise_msg = _(\n> + \"The file '%s' contains a byte order \"\n> + \"mark (BOM). Please use %s as \"\n> + \"working-tree-encoding.\");\n> + char *upper_enc = xstrdup_toupper(enc);\n> + upper_enc[strlen(upper_enc)-2] = '\\0';\n\nDue to startscase_with(...,\"UTF\"), we know at this point that the\nstring is at least 3 characters long, thus it's safe to back up by 2.\nGood.\n\n> + advise(advise_msg, path, upper_enc);\n> + free(upper_enc);\n> + if (die_on_error)\n> + die(error_msg, path, enc);\n> + else {\n> + return error(error_msg, path, enc);\n> + }\n> diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\n> @@ -62,6 +62,46 @@ test_expect_success 'check $GIT_DIR/info/attributes support' '\n> for i in 16 32\n> do\n> + test_expect_success \"check prohibited UTF-${i} BOM\" '\n> + test_when_finished \"git reset --hard HEAD\" &&\n> +\n> + echo \"*.utf${i}be text working-tree-encoding=utf-${i}be\" >>.gitattributes &&\n> + echo \"*.utf${i}le text working-tree-encoding=utf-${i}le\" >>.gitattributes &&\n\nv10 is checking only hyphenated lowercase encoding name; earlier\nversions checked uppercase. For better coverage, it would be nice to\ncheck several combinations: all uppercase, all lowercase, mixed case,\nhyphenated, not hyphenated.\n\nI'm not suggesting running all the tests repeatedly but rather just\nvarying the format of the encoding name in these tests you're adding.\nFor instance, the above could instead be:\n\n echo \"*.utf${i}be text working-tree-encoding=UTF-${i}be\" >>.gitattributes &&\n echo \"*.utf${i}le text working-tree-encoding=utf${i}LE\" >>.gitattributes &&\n\nor something.\n\n> + # Here we add a UTF-16 (resp. UTF-32) files with BOM (big/little-endian)\n> + # but we tell Git to treat it as UTF-16BE/UTF-16LE (resp. UTF-32).\n> + # In these cases the BOM is prohibited.\n> + cp bebom.utf${i}be.raw bebom.utf${i}be &&\n> + test_must_fail git add bebom.utf${i}be 2>err.out &&\n> + test_i18ngrep \"fatal: BOM is prohibited .* utf-${i}be\" err.out &&\n> +\n> + cp lebom.utf${i}le.raw lebom.utf${i}be &&\n> + test_must_fail git add lebom.utf${i}be 2>err.out &&\n> + test_i18ngrep \"fatal: BOM is prohibited .* utf-${i}be\" err.out &&\n> +\n> + cp bebom.utf${i}be.raw bebom.utf${i}le &&\n> + test_must_fail git add bebom.utf${i}le 2>err.out &&\n> + test_i18ngrep \"fatal: BOM is prohibited .* utf-${i}le\" err.out &&\n> +\n> + cp lebom.utf${i}le.raw lebom.utf${i}le &&\n> + test_must_fail git add lebom.utf${i}le 2>err.out &&\n> + test_i18ngrep \"fatal: BOM is prohibited .* utf-${i}le\" err.out\n> + '\n> +\n> + test_expect_success \"check required UTF-${i} BOM\" '\n> + test_when_finished \"git reset --hard HEAD\" &&\n> +\n> + echo \"*.utf${i} text working-tree-encoding=utf-${i}\" >>.gitattributes &&\n\nThis is another opportunity for checking a variation (case,\nhyphenation) of the encoding name rather than using only hyphenated\nlowercase.\n\n> +\n> + cp nobom.utf${i}be.raw nobom.utf${i} &&\n> + test_must_fail git add nobom.utf${i} 2>err.out &&\n> + test_i18ngrep \"fatal: BOM is required .* utf-${i}\" err.out &&\n> +\n> + cp nobom.utf${i}le.raw nobom.utf${i} &&\n> + test_must_fail git add nobom.utf${i} 2>err.out &&\n> + test_i18ngrep \"fatal: BOM is required .* utf-${i}\" err.out\n> + '\n"},{"id":"341227","messageId":"xmqqvae7znkj.fsf@gitster-ct.c.googlers.com","threadId":"48000","inReplyTo":"20180307173026.30058-7-lars.schneider@autodesk.com","subject":"Re: [PATCH v10 6/9] convert: add 'working-tree-encoding' attribute","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-07T19:35:08Z","receivedAt":"2018-03-07T19:35:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"lars.schneider@autodesk.com writes:\n\n> +static const char *git_path_check_encoding(struct attr_check_item *check)\n> +{\n> +\tconst char *value = check->value;\n> +\n> +\tif (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value) ||\n> +\t !strlen(value))\n> +\t\treturn NULL;\n\nThis means that having \"* working-tree-encoding\" (without \"=value\")\nin your .gitattributes file is silently ignored. \n\nThinking aloud. I wonder if that is something we would want to warn\nabout so that the project can fix it, perhaps? Or would that become\ntoo noisy to use an older version of Git that includes this series\nwhen a newer version that defines new meanings to boolean\n(set/unset) values for w-t-e attribute becomes available and\nprojects adopt it? On the other hand, with this code as-is or with\nan additional warning, such an \"older version\" of Git by definition\nbehaves differently from such a \"newer version\" that does something\ndifferent when the attribute is not a non-empty string, so it is\nquite likely that we won't be able to redefine or extend the meaning\nof w-t-e in a way like that.\n\nWhich means that the only sensible way to make sure we _could_ later\nextend the meaning of w-t-e and make it behave differently when set\nto a non-empty string is to make it an error in _this_ \"older\"\nversion. That way, of course people cannot use the \"older\" version\non a newer project that depends on the way how \"newer\" Git treats\nw-t-e that is set to, say, \"true\", but we won't silently (or loudly)\ndo wrong things to the project's data.\n\n"},{"id":"341228","messageId":"xmqqr2ovzmwo.fsf@gitster-ct.c.googlers.com","threadId":"48000","inReplyTo":"20180307173026.30058-8-lars.schneider@autodesk.com","subject":"Re: [PATCH v10 7/9] convert: check for detectable errors in UTF encodings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-07T19:49:27Z","receivedAt":"2018-03-07T19:49:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"lars.schneider@autodesk.com writes:\n\n> +static int validate_encoding(const char *path, const char *enc,\n> +\t\t const char *data, size_t len, int die_on_error)\n> +{\n> +\t/* We only check for UTF here as UTF?? can be an alias for UTF-?? */\n> +\tif (startscase_with(enc, \"UTF\")) {\n> +\t\t/*\n> +\t\t * Check for detectable errors in UTF encodings\n> +\t\t */\n> +\t\tif (has_prohibited_utf_bom(enc, data, len)) {\n> +\t\t\tconst char *error_msg = _(\n> +\t\t\t\t\"BOM is prohibited in '%s' if encoded as %s\");\n> +\t\t\t/*\n> +\t\t\t * This advice is shown for UTF-??BE and UTF-??LE encodings.\n> +\t\t\t * We cut off the last two characters of the encoding name\n> +\t\t\t # to generate the encoding name suitable for BOMs.\n> +\t\t\t */\n\nYuck. The code pretends to abstract away the details in a helper\nhas_prohibited_x() yet the caller still knows quite a lot.\n\n> +\t\t\tconst char *advise_msg = _(\n> +\t\t\t\t\"The file '%s' contains a byte order \"\n> +\t\t\t\t\"mark (BOM). Please use %s as \"\n> +\t\t\t\t\"working-tree-encoding.\");\n> +\t\t\tchar *upper_enc = xstrdup_toupper(enc);\n> +\t\t\tupper_enc[strlen(upper_enc)-2] = '\\0';\n> +\t\t\tadvise(advise_msg, path, upper_enc);\n> +\t\t\tfree(upper_enc);\n\nI think this up-casing is more problematic than without, not from\nthe point of view of the internal code, but from the point of view\nof the end user experience. When the user writes utf16le or\nutf-16le and the data does not trigger the BOM check, we are likely\nto successfully convert it. I do not see the merit of suggesting\nUTF16 or UTF-16 in such a case, over telling them to just drop the\nbyte-order suffix from the encoding names (i.e. utf16 or utf-16).\n\nIf you are trying to force/nudge people in the direction of\ncanonical way of spelling things (which may not be a bad idea), then\n\"utf16le\" as the original input would want to result in \"UTF-16\"\nwith dash in the advise, no?\n\nOn the other hand, if we are not enforcing such a policy decision\nbut merely explaining a way to work around this check, then it may\nbe better to give a variant with the smaller difference from the\noriginal (i.e. without up-casing).\n"},{"id":"341229","messageId":"xmqqmuzjzmfi.fsf@gitster-ct.c.googlers.com","threadId":"48000","inReplyTo":"20180307173026.30058-10-lars.schneider@autodesk.com","subject":"Re: [PATCH v10 9/9] convert: add round trip check based on 'core.checkRoundtripEncoding'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-07T19:59:45Z","receivedAt":"2018-03-07T19:59:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"lars.schneider@autodesk.com writes:\n\n> +static int check_roundtrip(const char* enc_name)\n\nThe asterisk sticks to the variable, not type.\n\n> +{\n> +\t/*\n> +\t * check_roundtrip_encoding contains a string of space and/or\n> +\t * comma separated encodings (eg. \"UTF-16, ASCII, CP1125\").\n> +\t * Search for the given encoding in that string.\n> +\t */\n> +\tconst char *found = strcasestr(check_roundtrip_encoding, enc_name);\n> +\tconst char *next;\n> +\tint len;\n> +\tif (!found)\n> +\t\treturn 0;\n> +\tnext = found + strlen(enc_name);\n> +\tlen = strlen(check_roundtrip_encoding);\n> +\treturn (found && (\n> +\t\t\t/*\n> +\t\t\t * check that the found encoding is at the\n> +\t\t\t * beginning of check_roundtrip_encoding or\n> +\t\t\t * that it is prefixed with a space or comma\n> +\t\t\t */\n> +\t\t\tfound == check_roundtrip_encoding || (\n> +\t\t\t\tfound > check_roundtrip_encoding &&\n> +\t\t\t\t(*(found-1) == ' ' || *(found-1) == ',')\n> +\t\t\t)\n\nThe second line is unneeded, as we know a non-NULL found either\npoints at check_roundtrip_encoding or somewhere to the right, and\nthe first test already checked the \"points exactly at\" case.\n\nThis is defined to be a comma separated list, so it is unnecessary\nto accept == <\"FOO, SHIFT-JIS, BAR\", \"SHIFT-JIS\">; if you\nallow SP, perhaps \"isspace(found[-1]) || found[-1] == ','\" to also\nallow HT may also be appropriate. I think \"comma or whitespace\nseparated list\" is fine; in any case, the comment near the beginning\nof this function does not match new text in Documentation/config.txt\nadded by this patch.\n"},{"id":"341236","messageId":"B001E8E2-9C98-4136-87A3-4425C50E90DB@gmail.com","threadId":"48000","inReplyTo":"xmqqr2ovzmwo.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v10 7/9] convert: check for detectable errors in UTF encodings","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-07T22:12:38Z","receivedAt":"2018-03-07T22:12:48Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 07 Mar 2018, at 20:49, Junio C Hamano wrote:\n> \n> lars.schneider@autodesk.com writes:\n> \n>> +static int validate_encoding(const char *path, const char *enc,\n>> +\t\t const char *data, size_t len, int die_on_error)\n>> +{\n>> +\t/* We only check for UTF here as UTF?? can be an alias for UTF-?? */\n>> +\tif (startscase_with(enc, \"UTF\")) {\n>> +\t\t/*\n>> +\t\t * Check for detectable errors in UTF encodings\n>> +\t\t */\n>> +\t\tif (has_prohibited_utf_bom(enc, data, len)) {\n>> +\t\t\tconst char *error_msg = _(\n>> +\t\t\t\t\"BOM is prohibited in '%s' if encoded as %s\");\n>> +\t\t\t/*\n>> +\t\t\t * This advice is shown for UTF-??BE and UTF-??LE encodings.\n>> +\t\t\t * We cut off the last two characters of the encoding name\n>> +\t\t\t # to generate the encoding name suitable for BOMs.\n>> +\t\t\t */\n> \n> Yuck. The code pretends to abstract away the details in a helper\n> has_prohibited_x() yet the caller still knows quite a lot.\n\nTrue, but has_prohibited_x() cannot create a proper error/advise\nmessage unless we give it more parameters (e.g. path name).\nTherefore, I don't see a better way right now.\n\n\n>> +\t\t\tconst char *advise_msg = _(\n>> +\t\t\t\t\"The file '%s' contains a byte order \"\n>> +\t\t\t\t\"mark (BOM). Please use %s as \"\n>> +\t\t\t\t\"working-tree-encoding.\");\n>> +\t\t\tchar *upper_enc = xstrdup_toupper(enc);\n>> +\t\t\tupper_enc[strlen(upper_enc)-2] = '\\0';\n>> +\t\t\tadvise(advise_msg, path, upper_enc);\n>> +\t\t\tfree(upper_enc);\n> \n> I think this up-casing is more problematic than without, not from\n> the point of view of the internal code, but from the point of view\n> of the end user experience. When the user writes utf16le or\n> utf-16le and the data does not trigger the BOM check, we are likely\n> to successfully convert it. I do not see the merit of suggesting\n> UTF16 or UTF-16 in such a case, over telling them to just drop the\n> byte-order suffix from the encoding names (i.e. utf16 or utf-16).\n> \n> If you are trying to force/nudge people in the direction of\n> canonical way of spelling things (which may not be a bad idea), then\n> \"utf16le\" as the original input would want to result in \"UTF-16\"\n> with dash in the advise, no?\n\nCorrect. In the error messages I kept the encoding name \"as-is\" and\nonly in the advise message I used the uppercase variant to steer\npeople into the canonical direction. My initial reason for this was\nthat in is_missing_required_utf_bom() we add \"BE/LE\" to the encoding\nin the advise message. Let's say the user used \"Utf-16\" as encoding.\n Should \"BE/LE\" be upper case or lower case? To avoid that question \nI made it always upper case.\n\nI also would have liked to advise \"UTF-16\" instead of \"UTF16\" as\nyou suggested. However, that required a few more lines and I wanted\nto keep the change to a minimum. I feel this could be added in a\nfollow up patch.\n\n\n> On the other hand, if we are not enforcing such a policy decision\n> but merely explaining a way to work around this check, then it may\n> be better to give a variant with the smaller difference from the\n> original (i.e. without up-casing).\n\nSee example mentioned above: \"Utf-16\". How would you handle that?\n\n\nThanks,\nLars\n\n"},{"id":"341238","messageId":"xmqqa7vjzfcp.fsf@gitster-ct.c.googlers.com","threadId":"48000","inReplyTo":"B001E8E2-9C98-4136-87A3-4425C50E90DB@gmail.com","subject":"Re: [PATCH v10 7/9] convert: check for detectable errors in UTF encodings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-07T22:32:38Z","receivedAt":"2018-03-07T22:32:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n> I also would have liked to advise \"UTF-16\" instead of \"UTF16\" as\n> you suggested. However, that required a few more lines and I wanted\n> to keep the change to a minimum. I feel this could be added in a\n> follow up patch.\n\nI'd say the whole upcase thing belongs to such a follow-up patch if\nthat is the case.\n\n>> On the other hand, if we are not enforcing such a policy decision\n>> but merely explaining a way to work around this check, then it may\n>> be better to give a variant with the smaller difference from the\n>> original (i.e. without up-casing).\n>\n> See example mentioned above: \"Utf-16\". How would you handle that?\n\nDropping LE suffix from \"Utf-16LE\" or \"Utf16LE\" would yield \"Utf-16\"\nor \"Utf16\" if the advise message does not force policy, or \"UTF-16\"\nin the canoical form if it does. Is there a problem?\n"},{"id":"341239","messageId":"40C38F6A-E536-4C7E-981F-B15746C103B5@gmail.com","threadId":"48000","inReplyTo":"xmqqmuzjzmfi.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v10 9/9] convert: add round trip check based on 'core.checkRoundtripEncoding'","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-07T22:44:06Z","receivedAt":"2018-03-07T22:44:16Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 07 Mar 2018, at 20:59, Junio C Hamano wrote:\n> \n> lars.schneider@autodesk.com writes:\n> \n>> +static int check_roundtrip(const char* enc_name)\n> \n> The asterisk sticks to the variable, not type.\n\nArgh. I need to put this check into Travis CI ;-)\n\n\n>> +{\n>> +\t/*\n>> +\t * check_roundtrip_encoding contains a string of space and/or\n>> +\t * comma separated encodings (eg. \"UTF-16, ASCII, CP1125\").\n>> +\t * Search for the given encoding in that string.\n>> +\t */\n>> +\tconst char *found = strcasestr(check_roundtrip_encoding, enc_name);\n>> +\tconst char *next;\n>> +\tint len;\n>> +\tif (!found)\n>> +\t\treturn 0;\n>> +\tnext = found + strlen(enc_name);\n>> +\tlen = strlen(check_roundtrip_encoding);\n>> +\treturn (found && (\n>> +\t\t\t/*\n>> +\t\t\t * check that the found encoding is at the\n>> +\t\t\t * beginning of check_roundtrip_encoding or\n>> +\t\t\t * that it is prefixed with a space or comma\n>> +\t\t\t */\n>> +\t\t\tfound == check_roundtrip_encoding || (\n>> +\t\t\t\tfound > check_roundtrip_encoding &&\n>> +\t\t\t\t(*(found-1) == ' ' || *(found-1) == ',')\n>> +\t\t\t)\n> \n> The second line is unneeded, as we know a non-NULL found either\n> points at check_roundtrip_encoding or somewhere to the right, and\n> the first test already checked the \"points exactly at\" case.\n\nEric objected that too [1], but noted the documentation value:\n\n Although the 'found > check_roundtrip_encoding' expression is\n effectively dead code in this case, it does document that the\n programmer didn't just blindly use '*(found-1)' without taking\n boundary conditions into consideration.\n\n (It's dead code because, at this point, we know that strcasestr()\n didn't return NULL and we know that 'found' doesn't equal\n 'check_roundtrip_encoding', therefore it _must_ be greater than\n 'check_roundtrip_encoding'.)\n\n[1] https://public-inbox.org/git/CAPig+cR81J3fTOtrgAumAs=RC5hqYFfSmeb-ru-Yf_ahFuBiew@mail.gmail.com/\n\nAlthough the line is unnecessary, I felt it is safer/easier to \nunderstand and maintain. Since both of you tripped over it, I will\nremove it though.\n\n\n> This is defined to be a comma separated list, so it is unnecessary\n> to accept == <\"FOO, SHIFT-JIS, BAR\", \"SHIFT-JIS\">; if you\n> allow SP, perhaps \"isspace(found[-1]) || found[-1] == ','\" to also\n> allow HT may also be appropriate.\n\nI don't think HT makes too much sense. However, isspace() is nice\nand I will use it. Being more permissive on the inputs should hurt.\n\n\n> I think \"comma or whitespace\n> separated list\" is fine; in any case, the comment near the beginning\n> of this function does not match new text in Documentation/config.txt\n> added by this patch.\n\nNice catch. Will fix.\n\n\nThanks,\nLars\n\n"},{"id":"341241","messageId":"766B302C-446B-4730-A5D9-909C6FCF6A84@gmail.com","threadId":"48000","inReplyTo":"xmqqa7vjzfcp.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v10 7/9] convert: check for detectable errors in UTF encodings","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-07T22:49:08Z","receivedAt":"2018-03-07T22:49:15Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 07 Mar 2018, at 23:32, Junio C Hamano wrote:\n> \n> Lars Schneider writes:\n> \n>> I also would have liked to advise \"UTF-16\" instead of \"UTF16\" as\n>> you suggested. However, that required a few more lines and I wanted\n>> to keep the change to a minimum. I feel this could be added in a\n>> follow up patch.\n> \n> I'd say the whole upcase thing belongs to such a follow-up patch if\n> that is the case.\n> \n>>> On the other hand, if we are not enforcing such a policy decision\n>>> but merely explaining a way to work around this check, then it may\n>>> be better to give a variant with the smaller difference from the\n>>> original (i.e. without up-casing).\n>> \n>> See example mentioned above: \"Utf-16\". How would you handle that?\n> \n> Dropping LE suffix from \"Utf-16LE\" or \"Utf16LE\" would yield \"Utf-16\"\n> or \"Utf16\" if the advise message does not force policy, or \"UTF-16\"\n> in the canoical form if it does. Is there a problem?\n\nIn the case of has_prohibited_utf_bom() you are right as we are \ndropping the BE/LE suffix in the advise. However, look at the \nis_missing_required_utf_bom() advise. Here we *add* BE/LE.\n\nAt this point I thought it would make sense to make the advised\nencoding name uppercase in both situations. OK with you?\n\n- Lars\n\n\n\n\n"},{"id":"341242","messageId":"xmqqzi3jxzus.fsf@gitster-ct.c.googlers.com","threadId":"48000","inReplyTo":"40C38F6A-E536-4C7E-981F-B15746C103B5@gmail.com","subject":"Re: [PATCH v10 9/9] convert: add round trip check based on 'core.checkRoundtripEncoding'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-07T22:52:43Z","receivedAt":"2018-03-07T22:52:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n> Although the line is unnecessary, I felt it is safer/easier to \n> understand and maintain. Since both of you tripped over it, I will\n> remove it though.\n\nI didn't actually trip over it. It made it look as if the coder\ndidn't understand what the code is doing to have that extra line.\n\n> I don't think HT makes too much sense. However, isspace() is nice\n> and I will use it. Being more permissive on the inputs should hurt.\n\nYou are being incoherent in these three sentences. If you want to\nbe more strict and only allow SP, then isspace() is already too\nbroad, as it does allow HT (and even CR and LF).\n\nI do think HT makes just as much sense as SP, though, so if you use\nisspace(), that would be perfectly fine.\n"},{"id":"341243","messageId":"66370A41-A048-44E7-9BF8-4631C50AAE63@gmail.com","threadId":"48000","inReplyTo":"CAPig+cSvC1a5EaDfFZdJU74mZz84M=CkeOJLSDxFdXKcJYyzEw@mail.gmail.com","subject":"Re: [PATCH v10 6/9] convert: add 'working-tree-encoding' attribute","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-07T22:56:33Z","receivedAt":"2018-03-07T22:56:40Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 07 Mar 2018, at 18:54, Eric Sunshine wrote:\n> \n> On Wed, Mar 7, 2018 at 12:30 PM, wrote:\n>> [...]\n>> Add an attribute to tell Git what encoding the user has defined for a\n>> given file. If the content is added to the index, then Git converts the\n>> content to a canonical UTF-8 representation. On checkout Git will\n>> reverse the conversion.\n>> \n>> Signed-off-by: Lars Schneider \n>> ---\n>> Documentation/gitattributes.txt | 80 +++++++++++++++++++++++\n>> diff --git a/convert.c b/convert.c\n>> @@ -265,6 +266,78 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n>> +static const char *default_encoding = \"UTF-8\";\n>> @@ -978,6 +1051,21 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,\n>> +static const char *git_path_check_encoding(struct attr_check_item *check)\n>> +{\n>> + const char *value = check->value;\n>> +\n>> + if (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value) ||\n>> + !strlen(value))\n>> + return NULL;\n>> +\n>> + /* Don't encode to the default encoding */\n>> + if (!strcasecmp(value, default_encoding))\n>> + return NULL;\n> \n> As of v10, the rest of the code accepts encoding names \"UTF-xx\" and\n> \"UTFxx\" (case insensitive), however, this check recognizes only\n> \"UTF-8\" (case insensitive). For consistency, one would expect this\n> also to recognize \"UTF8\" (case insensitive).\n\nNice catch. What do you think about this solution using is_encoding_utf8()\nfrom utf.c?\n\t\n\tif (is_encoding_utf8(value) && is_encoding_utf8(default_encoding)) \n\n- Lars"},{"id":"341244","messageId":"xmqqvae7xznk.fsf@gitster-ct.c.googlers.com","threadId":"48000","inReplyTo":"766B302C-446B-4730-A5D9-909C6FCF6A84@gmail.com","subject":"Re: [PATCH v10 7/9] convert: check for detectable errors in UTF encodings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-07T22:57:03Z","receivedAt":"2018-03-07T22:57:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n> In the case of has_prohibited_utf_bom() you are right as we are \n> dropping the BE/LE suffix in the advise. However, look at the \n> is_missing_required_utf_bom() advise. Here we *add* BE/LE.\n\nSo? Then add BE/LE like \"Utf-16BE\" or \"utf16BE\". You do not have\nenough information to decide what is best for the user anyway (which\nis why I am not convinced that it is even a good idea to always\nupcase in the first place).\n\n> At this point I thought it would make sense to make the advised\n> encoding name uppercase in both situations. OK with you?\n\nIn the endgame, if upcased and properly dashed form is always used,\nthat would be good (if we are enforcing the policy, which I am not\nonboard 100% but it's your code and I do not care too strongly about\nit). I do not see much point in an interim step that only upcases\nwithout doing the dash insertion.\n\n\n"},{"id":"341245","messageId":"xmqqr2ovxzmj.fsf@gitster-ct.c.googlers.com","threadId":"48000","inReplyTo":"66370A41-A048-44E7-9BF8-4631C50AAE63@gmail.com","subject":"Re: [PATCH v10 6/9] convert: add 'working-tree-encoding' attribute","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-07T22:57:40Z","receivedAt":"2018-03-07T22:57:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n> Nice catch. What do you think about this solution using is_encoding_utf8()\n> from utf.c?\n\nThat helper was invented exactly for a case like this, I would\nthink.\n"},{"id":"341246","messageId":"F974DE28-D4EE-4BC6-9AE8-BF5F79FACC86@gmail.com","threadId":"48000","inReplyTo":"xmqqzi3jxzus.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v10 9/9] convert: add round trip check based on 'core.checkRoundtripEncoding'","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-07T22:58:58Z","receivedAt":"2018-03-07T22:59:06Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 07 Mar 2018, at 23:52, Junio C Hamano wrote:\n> \n> Lars Schneider writes:\n> \n>> I don't think HT makes too much sense. However, isspace() is nice\n>> and I will use it. Being more permissive on the inputs should hurt.\n> \n> You are being incoherent in these three sentences. If you want to\n> be more strict and only allow SP, then isspace() is already too\n> broad, as it does allow HT (and even CR and LF).\n\nI meant \"Being more permissive on the inputs shouldN'T hurt.\" :-)\n\n\n> I do think HT makes just as much sense as SP, though, so if you use\n> isspace(), that would be perfectly fine.\n\nOK!\n\n- Lars\n\n"},{"id":"341247","messageId":"4465AF18-EBE8-4BA3-8F95-B51630A41B86@gmail.com","threadId":"48000","inReplyTo":"xmqqvae7xznk.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v10 7/9] convert: check for detectable errors in UTF encodings","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-07T23:19:44Z","receivedAt":"2018-03-07T23:19:52Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 07 Mar 2018, at 23:57, Junio C Hamano wrote:\n> \n> Lars Schneider writes:\n> \n>> At this point I thought it would make sense to make the advised\n>> encoding name uppercase in both situations. OK with you?\n> \n> In the endgame, if upcased and properly dashed form is always used,\n> that would be good (if we are enforcing the policy, which I am not\n> onboard 100% but it's your code and I do not care too strongly about\n> it). I do not see much point in an interim step that only upcases\n> without doing the dash insertion.\n\nI would like to advise the dashed form as this seems to be the\ncanonical form and it avoids cross platform issues. My macOS\niconv does not support the form without dashes.\n\nWould this approach work for you?\n\n\t\t\tconst char *advise_msg = _(\n\t\t\t\t\"The file '%s' contains a byte order \"\n\t\t\t\t\"mark (BOM). Please use UTF-%s as \"\n\t\t\t\t\"working-tree-encoding.\");\n\t\t\tconst char *stripped;\n\t\t\tchar *upper = xstrdup_toupper(enc);\n\t\t\tupper[strlen(upper)-2] = '\\0';\n\t\t\tskip_prefix(upper, \"UTF-\", &stripped) ||\n\t\t\tskip_prefix(stripped, \"UTF\", &stripped);\n\t\t\tadvise(advise_msg, path, stripped);\n\nThanks,\nLars\n"},{"id":"341249","messageId":"xmqqmuzjxxxc.fsf@gitster-ct.c.googlers.com","threadId":"48000","inReplyTo":"4465AF18-EBE8-4BA3-8F95-B51630A41B86@gmail.com","subject":"Re: [PATCH v10 7/9] convert: check for detectable errors in UTF encodings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-07T23:34:23Z","receivedAt":"2018-03-07T23:34:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n> I would like to advise the dashed form as this seems to be the\n> canonical form and it avoids cross platform issues. My macOS\n> iconv does not support the form without dashes.\n\nSure, that is why I said canonicalization without inserting dash\ndoes not make much sense, hence an interim step with only upcasing\nis not a good idea. A possible interim solution would be to do\nnothing (no dash insertion, no upcasing) and fixing both in a later\nfollow-up patch, but as I said, I do not care too strongly either\nway.\n\n> Would this approach work for you?\n>\n> \t\t\tconst char *advise_msg = _(\n> \t\t\t\t\"The file '%s' contains a byte order \"\n> \t\t\t\t\"mark (BOM). Please use UTF-%s as \"\n> \t\t\t\t\"working-tree-encoding.\");\n> \t\t\tconst char *stripped;\n> \t\t\tchar *upper = xstrdup_toupper(enc);\n> \t\t\tupper[strlen(upper)-2] = '\\0';\n> \t\t\tskip_prefix(upper, \"UTF-\", &stripped) ||\n> \t\t\tskip_prefix(stripped, \"UTF\", &stripped);\n> \t\t\tadvise(advise_msg, path, stripped);\n\nAre you now interested in not having any interim step and jump\ndirectly to the endgame solution? If so, that is fine by me, too,\nbut as I already said earlier (i.e. not doing this BOM check for an\nencoding that is not spelled in your canonical upcase-with-dash form\nmight be a feature that leaves an escape hatch), I am not all that\ninterested in enforcing policy at this point in the codepath to\nbegin with, so...\n\n"},{"id":"341250","messageId":"CACsJy8DWMmC9mvz783XQFHUopbVMH00LoqpW-CQunzg0qgiEEA@mail.gmail.com","threadId":"48000","inReplyTo":"20180307173026.30058-4-lars.schneider@autodesk.com","subject":"Re: [PATCH v10 3/9] strbuf: add a case insensitive starts_with()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-03-08T00:31:52Z","receivedAt":"2018-03-08T00:32:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Mar 8, 2018 at 12:30 AM, wrote:\n> From: Lars Schneider \n>\n> Check in a case insensitive manner if one string is a prefix of another\n> string.\n>\n> This function is used in a subsequent commit.\n>\n> Signed-off-by: Lars Schneider \n> ---\n> git-compat-util.h | 1 +\n> strbuf.c | 9 +++++++++\n> 2 files changed, 10 insertions(+)\n>\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index 68b2ad531e..f648da0c11 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -455,6 +455,7 @@ extern void (*get_warn_routine(void))(const char *warn, va_list params);\n> extern void set_die_is_recursing_routine(int (*routine)(void));\n>\n> extern int starts_with(const char *str, const char *prefix);\n> +extern int startscase_with(const char *str, const char *prefix);\n\nThis name is a bit hard to read. Boost [1] goes with istarts_with. I\nwonder if it's better. If not I guess either starts_with_case or\nstarts_case_with will improve readability.\n\n[1] http://www.boost.org/doc/libs/1_41_0/doc/html/boost/algorithm/istarts_with.html\n\n>\n> /*\n> * If the string \"str\" begins with the string found in \"prefix\", return 1.\n> diff --git a/strbuf.c b/strbuf.c\n> index b635f0bdc4..5779a2d591 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -11,6 +11,15 @@ int starts_with(const char *str, const char *prefix)\n> return 0;\n> }\n>\n> +int startscase_with(const char *str, const char *prefix)\n> +{\n> + for (; ; str++, prefix++)\n> + if (!*prefix)\n> + return 1;\n> + else if (tolower(*str) != tolower(*prefix))\n> + return 0;\n> +}\n> +\n> int skip_to_optional_arg_default(const char *str, const char *prefix,\n> const char **arg, const char *def)\n> {\n> --\n> 2.16.2\n>\n\n\n\n-- \nDuy\n"},{"id":"341323","messageId":"xmqq4llq88ms.fsf@gitster-ct.c.googlers.com","threadId":"48000","inReplyTo":"CACsJy8DWMmC9mvz783XQFHUopbVMH00LoqpW-CQunzg0qgiEEA@mail.gmail.com","subject":"Re: [PATCH v10 3/9] strbuf: add a case insensitive starts_with()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-08T23:12:11Z","receivedAt":"2018-03-08T23:12:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n>> extern int starts_with(const char *str, const char *prefix);\n>> +extern int startscase_with(const char *str, const char *prefix);\n>\n> This name is a bit hard to read. Boost [1] goes with istarts_with. I\n> wonder if it's better. If not I guess either starts_with_case or\n> starts_case_with will improve readability.\n\nstarts_with_case() sounds quite strange even though\nstarts_with_icase() may make it clear that it is \"a variant of\nstarts_with() function that ignores case\". I dunno.\ndir.c::cmp_icase() takes the _icase suffix for not quite the way\nthat is consistent with that understanding, though.\n\n"},{"id":"341344","messageId":"7920610F-8B51-4E8F-83C6-7B29D0EBF852@gmail.com","threadId":"48000","inReplyTo":"xmqq4llq88ms.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v10 3/9] strbuf: add a case insensitive starts_with()","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-09T15:54:30Z","receivedAt":"2018-03-09T15:54:35Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 09 Mar 2018, at 00:12, Junio C Hamano wrote:\n> \n> Duy Nguyen writes:\n> \n>>> extern int starts_with(const char *str, const char *prefix);\n>>> +extern int startscase_with(const char *str, const char *prefix);\n>> \n>> This name is a bit hard to read. Boost [1] goes with istarts_with. I\n>> wonder if it's better. If not I guess either starts_with_case or\n>> starts_case_with will improve readability.\n> \n> starts_with_case() sounds quite strange even though\n> starts_with_icase() may make it clear that it is \"a variant of\n> starts_with() function that ignores case\". I dunno.\n> dir.c::cmp_icase() takes the _icase suffix for not quite the way\n> that is consistent with that understanding, though.\n\nI think following the boost lib makes most sense. Therefore,\nI would like to go with \"istarts_with\". OK with you?\n\n- Lars\n"},{"id":"341345","messageId":"43D0E2A3-E265-4503-9E2D-4058437A8FCA@gmail.com","threadId":"48000","inReplyTo":"CAPig+cTsRufRNZbKJS-02fvbtaLB30FO3Em6HR_LZrsK+CfOjw@mail.gmail.com","subject":"Re: [PATCH v10 7/9] convert: check for detectable errors in UTF encodings","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-09T17:02:46Z","receivedAt":"2018-03-09T17:02:58Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 07 Mar 2018, at 19:04, Eric Sunshine wrote:\n> \n> On Wed, Mar 7, 2018 at 12:30 PM, wrote:\n>> Check that new content is valid with respect to the user defined\n>> 'working-tree-encoding' attribute.\n>> \n>> Signed-off-by: Lars Schneider \n>> ---\n>> diff --git a/convert.c b/convert.c\n>> @@ -266,6 +266,58 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,\n>> +static int validate_encoding(const char *path, const char *enc,\n>> + const char *data, size_t len, int die_on_error)\n>> +{\n>> + /* We only check for UTF here as UTF?? can be an alias for UTF-?? */\n>> + if (startscase_with(enc, \"UTF\")) {\n>> + /*\n>> + * Check for detectable errors in UTF encodings\n>> + */\n>> + if (has_prohibited_utf_bom(enc, data, len)) {\n>> + const char *error_msg = _(\n>> + \"BOM is prohibited in '%s' if encoded as %s\");\n>> + /*\n>> + * This advice is shown for UTF-??BE and UTF-??LE encodings.\n>> + * We cut off the last two characters of the encoding name\n>> + # to generate the encoding name suitable for BOMs.\n> \n> s/#/*/\n\nOf course!\n\n\n>> diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\n>> @@ -62,6 +62,46 @@ test_expect_success 'check $GIT_DIR/info/attributes support' '\n>> for i in 16 32\n>> do\n>> + test_expect_success \"check prohibited UTF-${i} BOM\" '\n>> + test_when_finished \"git reset --hard HEAD\" &&\n>> +\n>> + echo \"*.utf${i}be text working-tree-encoding=utf-${i}be\" >>.gitattributes &&\n>> + echo \"*.utf${i}le text working-tree-encoding=utf-${i}le\" >>.gitattributes &&\n> \n> v10 is checking only hyphenated lowercase encoding name; earlier\n> versions checked uppercase. For better coverage, it would be nice to\n> check several combinations: all uppercase, all lowercase, mixed case,\n> hyphenated, not hyphenated.\n> \n> I'm not suggesting running all the tests repeatedly but rather just\n> varying the format of the encoding name in these tests you're adding.\n> For instance, the above could instead be:\n> \n> echo \"*.utf${i}be text working-tree-encoding=UTF-${i}be\" >>.gitattributes &&\n> echo \"*.utf${i}le text working-tree-encoding=utf${i}LE\" >>.gitattributes &&\n> \n> or something.\n\nThe casing is a good idea - I will do that. I don't want to do \"hyphenated, not \nhyphenated\" as this would make the tests fail on macOS (and I believe on Windows).\n\n\nThanks,\nLars"},{"id":"341349","messageId":"xmqqmuzh6u8g.fsf@gitster-ct.c.googlers.com","threadId":"48000","inReplyTo":"7920610F-8B51-4E8F-83C6-7B29D0EBF852@gmail.com","subject":"Re: [PATCH v10 3/9] strbuf: add a case insensitive starts_with()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-09T17:20:47Z","receivedAt":"2018-03-09T17:20:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n> I think following the boost lib makes most sense. Therefore,\n> I would like to go with \"istarts_with\". OK with you?\n\nI don't care too deeply; if we took starts_with() from there, where\nwhat we now want is defined as istarts_with(), then that sounds like\na good thing to do.\n\nThanks.\n"},{"id":"341365","messageId":"87h8ppqdb6.fsf@evledraar.gmail.com","threadId":"48000","inReplyTo":"xmqqmuzh6u8g.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v10 3/9] strbuf: add a case insensitive starts_with()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-03-09T19:06:05Z","receivedAt":"2018-03-09T19:06:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Mar 09 2018, Junio C. Hamano jotted:\n\n> Lars Schneider writes:\n>\n>> I think following the boost lib makes most sense. Therefore,\n>> I would like to go with \"istarts_with\". OK with you?\n>\n> I don't care too deeply; if we took starts_with() from there, where\n> what we now want is defined as istarts_with(), then that sounds like\n> a good thing to do.\n\nI don't care either, but just a note that we had this exact discussion\naround this time last year when I added a starts_with_icase() [1][2]\nwhich eventually got dropped.\n\n1. https://public-inbox.org/git/xmqqpohao2hw.fsf@gitster.mtv.corp.google.com/\n2. https://public-inbox.org/git/20170326121654.22035-4-avarab@gmail.com/\n"}]}