{"thread":{"id":"39548","subject":"[PATCH] utf8.c: print warning about disabled iconv","startedAt":"2015-06-06T21:02:31Z","lastAt":"2015-08-17T19:49:05Z","messageCount":8,"participants":["Max Kirillov","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"263132","messageId":"1433624551-20730-1-git-send-email-max@max630.net","threadId":"39548","inReplyTo":null,"subject":"[PATCH] utf8.c: print warning about disabled iconv","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-06-06T21:02:31Z","receivedAt":"2015-06-06T21:02:31Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"It is an allowed compile-time option to build git without iconv\nsupport. Resulting build almost always functions correctly, and\nnever displays that it is missing anything, but reencode_string_len()\njust never modifies its input. This gives undesirable result that\nreturned data or even data written into repository is incorrect\nand user is not aware about it.\n\nShow warning there is non-trivial reencoding requested. Show it only once\nduring program run.\n\nSigned-off-by: Max Kirillov <max@max630.net>\n---\nHi.\n\nI have noticed there is some activity around the option, so it's not completely\nabandoned. Maybe then this patch could be useful?\n utf8.c | 13 +++++++++++++\n utf8.h |  5 ++---\n 2 files changed, 15 insertions(+), 3 deletions(-)\n\ndiff --git a/utf8.c b/utf8.c\nindex 520fbb4..b1b836e 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -521,6 +521,19 @@ char *reencode_string_len(const char *in, int insz,\n \ticonv_close(conv);\n \treturn out;\n }\n+#else\n+static int noiconv_warning_shown = 0;\n+\n+char *reencode_string_len(const char *in, int insz,\n+\t\t\t  const char *out_encoding, const char *in_encoding,\n+\t\t\t  int *outsz)\n+{\n+\tif (!same_encoding(in_encoding, out_encoding) && !noiconv_warning_shown) {\n+\t\twarning(\"Iconv support is disabled at compile time. It is likely that\\nincorrect data will be printed or stored in repository.\\nConsider using other build for this task.\");\n+\t\tnoiconv_warning_shown = 1;\n+\t}\n+\treturn NULL;\n+}\n #endif\n \n /*\ndiff --git a/utf8.h b/utf8.h\nindex e4d9183..3d900f1 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -23,13 +23,12 @@ void strbuf_utf8_replace(struct strbuf *sb, int pos, int width,\n #ifndef NO_ICONV\n char *reencode_string_iconv(const char *in, size_t insz,\n \t\t\t    iconv_t conv, int *outsz);\n+#endif\n+\n char *reencode_string_len(const char *in, int insz,\n \t\t\t  const char *out_encoding,\n \t\t\t  const char *in_encoding,\n \t\t\t  int *outsz);\n-#else\n-#define reencode_string_len(a,b,c,d,e) NULL\n-#endif\n \n static inline char *reencode_string(const char *in,\n \t\t\t\t    const char *out_encoding,\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"263241","messageId":"xmqqfv62ch0v.fsf@gitster.dls.corp.google.com","threadId":"39548","inReplyTo":"1433624551-20730-1-git-send-email-max@max630.net","subject":"Re: [PATCH] utf8.c: print warning about disabled iconv","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-08T16:16:16Z","receivedAt":"2015-06-08T16:16:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov <max@max630.net> writes:\n\n> It is an allowed compile-time option to build git without iconv\n> support. Resulting build almost always functions correctly, and\n> never displays that it is missing anything, but reencode_string_len()\n> just never modifies its input.\n\nCorrect.\n\n> This gives undesirable result that\n> returned data or even data written into repository is incorrect\n> and user is not aware about it.\n\nI do not necessarily agree with that.  The user knows what s/he is\ndoing, data written to or shown from the repository is correct as\nfar as the user is concerned, and the user takes the full\nrespoinsibility when compiling out certain features.\n\n> +\tif (!same_encoding(in_encoding, out_encoding) && !noiconv_warning_shown) {\n> +\t\twarning(\"Iconv support is disabled at compile time. It is likely that\\nincorrect data will be printed or stored in repository.\\nConsider using other build for this task.\");\n> +\t\tnoiconv_warning_shown = 1;\n> +\t}\n\nI actually am OK if the user gets exactly the same warning between\nthe two cases:\n\n - iconv failed to convert in the real reencode_string_len()\n\n - we compiled out iconv() and real conversion was asked.\n\nand this patch is about the latter; I do not think it is reasonable\nto give noise only for the latter but not for the former.  The\nlatter is expected by users who compile out the feature, but the\nformer is not and deserves the wranings more, so the patch is\nbackwards in that sense.\n\nThanks.\n"},{"id":"263310","messageId":"20150608210705.GA22850@wheezy.local","threadId":"39548","inReplyTo":"xmqqfv62ch0v.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] utf8.c: print warning about disabled iconv","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-06-08T21:07:05Z","receivedAt":"2015-06-08T21:07:05Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Hi.\n\nOn Mon, Jun 08, 2015 at 09:16:16AM -0700, Junio C Hamano wrote:\n> Max Kirillov <max@max630.net> writes:\n\n>> This gives undesirable result that returned data or even\n>> data written into repository is incorrect and user is not\n>> aware about it.\n> \n> I do not necessarily agree with that.  The user knows what\n> s/he is doing, data written to or shown from the\n> repository is correct as far as the user is concerned, and\n> the user takes the full respoinsibility when compiling out\n> certain features.\n\nUser, in theory, can be not the same person who builds, or\ncan be not aware that the case needs recoding. It actually\nstarted when I compiled git without iconv support and got\nabout 10 failed tests, and only 2 of them mentioned i18n in\ntheir name.\n\nCompiling out other features is not exactly the same. If\nuser compiles out curl, for example, git will not be able to\npush or fetch through http, but it is not going to pretend\nto be working, it will fail visibly.\n\n> I actually am OK if the user gets exactly the same warning between\n> the two cases:\n> \n>  - iconv failed to convert in the real reencode_string_len()\n> \n>  - we compiled out iconv() and real conversion was asked.\n\nDoes 'exactly the same' mean the same text? Shouldn't it\ndescribe the reason? I can see 2 possible failures in case\nof real iconv: unknown or unsupported encoding and invalid\ninput. Wouldn't them better to be detailed in warning?\n\n-- \nMax\n"},{"id":"263311","messageId":"xmqqoakp9a2f.fsf@gitster.dls.corp.google.com","threadId":"39548","inReplyTo":"20150608210705.GA22850@wheezy.local","subject":"Re: [PATCH] utf8.c: print warning about disabled iconv","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-08T21:14:48Z","receivedAt":"2015-06-08T21:14:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov <max@max630.net> writes:\n\n> User, in theory, can be not the same person who builds, or\n> can be not aware that the case needs recoding.\n\nBecause you can pretty much say the same for build with iconv\nenabled, I think that line of argument is futile.  The users do not\nhave control over platform's iconv (and sysadmin's choice of locale\npackages) what charset/encoding can be converted to what other ones.\n\n>> I actually am OK if the user gets exactly the same warning between\n>> the two cases:\n>> \n>>  - iconv failed to convert in the real reencode_string_len()\n>> \n>>  - we compiled out iconv() and real conversion was asked.\n>\n> Does 'exactly the same' mean the same text?\n\nNo, I was trying to point out the total lack of corresponding\nwarnings in the iconv-enabled build.\n\nAfter all, if you had to convert between UTF-8 and ISO-2022-JP, the\nlatter of which your system does not support, whether you use\niconv-disabled build of Git or iconv-enabled build of Git, we pass\nthe bytestream through, right?  Your patch gives warning for the\nformer (which is a good starting point if we want to warn \"user\nexpected them to be converted, we didn't\" case) but does not do\nanything to the latter, even though users of the iconv-disabled\nbuild is more likely to be aware of the potential issue (and are\nlikely to be willing to accept that) than the ones with\niconv-enabled build that runs on a sysmet that cannot convert the\nspecific encoding.\n"},{"id":"268099","messageId":"1439589334-32318-1-git-send-email-max@max630.net","threadId":"39548","inReplyTo":"1433624551-20730-1-git-send-email-max@max630.net","subject":"[PATCH v2] utf8.c: print warning about iconv errors","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-08-14T21:55:34Z","receivedAt":"2015-08-14T21:55:34Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"If reencoding a text data from one encoding to another fails, the original\nversion is used insted. Currently there is no warning about failed reencoding,\nwhich can have an undesired outcome that returned data is incorrect but user\nis not aware about it.\n\nAdd printing warning when conversion fails.\n\nAlso add test script to assert that warning is actually printed and output is\nnot changed, as expected.\n\nSigned-off-by: Max Kirillov <max@max630.net>\n---\nChanges since v1:\n* rebase to recent changes\n* add handling runtime errors\n* add test\n* do not limit number of warnings - does not worth complicating the code\n* noticed that incomplete utf8 sequence in input silently treated as latin1.\n  so mark the testcase as expect_failure. Actually, it's quite surprising,\n  would be nice if somebody tries it in various environments\nActually, as far as I could grep, all uses of the resoding happen\nonly for printing, so probably it is not that important.\n t/t3911-show-reencode.sh | 46 ++++++++++++++++++++++++++++++++++++++++++++++\n utf8.c                   | 24 +++++++++++++++++++++++-\n utf8.h                   |  7 ++-----\n 3 files changed, 71 insertions(+), 6 deletions(-)\n create mode 100755 t/t3911-show-reencode.sh\n\ndiff --git a/t/t3911-show-reencode.sh b/t/t3911-show-reencode.sh\nnew file mode 100755\nindex 0000000..061d820\n--- /dev/null\n+++ b/t/t3911-show-reencode.sh\n@@ -0,0 +1,46 @@\n+#!/bin/sh\n+\n+test_description='reencoding'\n+\n+. ./test-lib.sh\n+\n+printf '\\304\\201\\n' >a_macron_utf8\n+printf '\\303\\244\\n' >a_diaeresis_utf8\n+printf '\\303\\244\\304\\n' >incomplete_utf8\n+printf '\\344\\n' >a_diaeresis_latin1\n+\n+test_expect_success 'setup' '\n+\tgit commit --allow-empty -F a_diaeresis_utf8 &&\n+\tgit tag latin1_utf8 &&\n+\tgit commit --allow-empty -F a_macron_utf8 &&\n+\tgit tag extended_utf8 &&\n+\tgit commit --allow-empty -F incomplete_utf8 &&\n+\tgit tag invalid_utf8\n+'\n+\n+test_expect_success 'encoding to latin1' '\n+\tgit log --encoding=latin1 --pretty=format:%B -1 latin1_utf8 >out 2>err &&\n+\ttest_must_be_empty err &&\n+\ttest_cmp out a_diaeresis_latin1\n+'\n+\n+test_expect_success 'unknown encoding' '\n+\tgit log --encoding=no-encoding --pretty=format:%B -1 latin1_utf8 >out 2>err &&\n+\tgrep -q \"not supported\" err &&\n+\ttest_cmp out a_diaeresis_utf8\n+'\n+\n+# apparently incomplete UTF8 byte sequences silently treated as latin1\n+test_expect_failure 'incomplete utf8' '\n+\tgit log --encoding=latin1 --pretty=format:%B -1 invalid_utf8 >out 2>err &&\n+\tgrep -q \"Invalid input\" err &&\n+\ttest_cmp out incomplete_utf8\n+'\n+\n+test_expect_success 'does not fit into latin1' '\n+\tgit log --encoding=latin1 --pretty=format:%B -1 extended_utf8 >out 2>err &&\n+\tgrep -q \"Invalid input\" err &&\n+\ttest_cmp out a_macron_utf8\n+'\n+\n+test_done\ndiff --git a/utf8.c b/utf8.c\nindex 28e6d76..d284bb0 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -465,7 +465,9 @@ char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv, int *outs\n \t\tif (cnt == (size_t) -1) {\n \t\t\tsize_t sofar;\n \t\t\tif (errno != E2BIG) {\n+\t\t\t\tint failure_errno = errno;\n \t\t\t\tfree(out);\n+\t\t\t\terrno = failure_errno;\n \t\t\t\treturn NULL;\n \t\t\t}\n \t\t\t/* insz has remaining number of bytes.\n@@ -513,14 +515,34 @@ char *reencode_string_len(const char *in, int insz,\n \t\tif (is_encoding_utf8(out_encoding))\n \t\t\tout_encoding = \"UTF-8\";\n \t\tconv = iconv_open(out_encoding, in_encoding);\n-\t\tif (conv == (iconv_t) -1)\n+\t\tif (conv == (iconv_t) -1) {\n+\t\t\tif (errno == EINVAL)\n+\t\t\t\twarning(\"Conversion from %s to %s not supported, falling back to verbatim copy\", in_encoding, out_encoding);\n+\t\t\telse\n+\t\t\t\twarning(\"Conversion from %s to %s failed: %s, falling back to verbatim copy\", in_encoding, out_encoding, strerror(errno));\n \t\t\treturn NULL;\n+\t\t}\n \t}\n \n \tout = reencode_string_iconv(in, insz, conv, outsz);\n+\tif (out == NULL) {\n+\t\tif (errno == EILSEQ || errno == EINVAL)\n+\t\t\twarning(\"Invalid input for conversion from %s to %s, falling back to verbatim copy\", in_encoding, out_encoding);\n+\t\telse\n+\t\t\twarning(\"Conversion from %s to %s failed: %s, falling back to verbatim copy\", in_encoding, out_encoding, strerror(errno));\n+\t}\n \ticonv_close(conv);\n \treturn out;\n }\n+#else\n+char *reencode_string_len(const char *in, int insz,\n+\t\t\t  const char *out_encoding, const char *in_encoding,\n+\t\t\t  int *outsz)\n+{\n+\tif (!same_encoding(in_encoding, out_encoding))\n+\t\twarning(\"Iconv support is disabled at compile time. It is likely that\\nincorrect data will be printed or stored in repository.\\nConsider using other build for this task.\");\n+\treturn NULL;\n+}\n #endif\n \n /*\ndiff --git a/utf8.h b/utf8.h\nindex 5a9e94b..c72998b 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -26,15 +26,12 @@ void strbuf_utf8_replace(struct strbuf *sb, int pos, int width,\n #ifndef NO_ICONV\n char *reencode_string_iconv(const char *in, size_t insz,\n \t\t\t    iconv_t conv, int *outsz);\n+#endif\n+\n char *reencode_string_len(const char *in, int insz,\n \t\t\t  const char *out_encoding,\n \t\t\t  const char *in_encoding,\n \t\t\t  int *outsz);\n-#else\n-static inline char *reencode_string_len(const char *a, int b,\n-\t\t\t\t\tconst char *c, const char *d, int *e)\n-{ if (e) *e = 0; return NULL; }\n-#endif\n \n static inline char *reencode_string(const char *in,\n \t\t\t\t    const char *out_encoding,\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"268104","messageId":"xmqqvbchfqgh.fsf@gitster.dls.corp.google.com","threadId":"39548","inReplyTo":"1439589334-32318-1-git-send-email-max@max630.net","subject":"Re: [PATCH v2] utf8.c: print warning about iconv errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-14T22:35:58Z","receivedAt":"2015-08-14T22:35:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov <max@max630.net> writes:\n\n> * do not limit number of warnings - does not worth complicating the code\n\nUnless the warning leads to a quick \"die()\", wouldn't this make Git\nunusable by spewing a \"falling back to verbatim copy\" for each and\nevery line of the message of a commit that has 'encoding' element in\nits header in the \"git log\" output, no?\n\nI suspect that this may be a huge mistake.\n\n> +char *reencode_string_len(const char *in, int insz,\n> +\t\t\t  const char *out_encoding, const char *in_encoding,\n> +\t\t\t  int *outsz)\n> +{\n> +\tif (!same_encoding(in_encoding, out_encoding))\n> +\t\twarning(\"Iconv support is disabled at compile time. It is likely that\\nincorrect data will be printed or stored in repository.\\nConsider using other build for this task.\");\n> +\treturn NULL;\n> +}\n\nHmmm, I suspect this may be seen as regression by those who build\nGit without ICONV for performance, knowing that there is nothing in\ntheir data that requires character set conversion.\n\nWe'd call same_encoding() every time, which would involve a few\nstrcasecmp() calls.  Originally, we didn't even have a function call\noverhead.\n"},{"id":"268206","messageId":"20150817190238.GA3594@sigill.intra.peff.net","threadId":"39548","inReplyTo":"xmqqvbchfqgh.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] utf8.c: print warning about iconv errors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-08-17T19:02:38Z","receivedAt":"2015-08-17T19:02:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 14, 2015 at 03:35:58PM -0700, Junio C Hamano wrote:\n\n> Max Kirillov <max@max630.net> writes:\n> \n> > * do not limit number of warnings - does not worth complicating the code\n> \n> Unless the warning leads to a quick \"die()\", wouldn't this make Git\n> unusable by spewing a \"falling back to verbatim copy\" for each and\n> every line of the message of a commit that has 'encoding' element in\n> its header in the \"git log\" output, no?\n\nWe only do the reencode once per commit. So it would be once per commit\nrather than once per line. Which still sounds kind of annoying, if you\nare using \"git log --oneline\" or similar.\n\nI think I'd favor a single warning in general, along the lines of\n\"some encodings could not be converted\". But of course if you are trying\nto figure out _which_ encodings your system doesn't have, that's not\nvery helpful. Maybe we could have an advice.encodingFailure config flag\nwith a tristate:\n\n  - false: don't spew any warnings\n\n  - true: give a generic warning once per program\n\n  - all: give a specific warning for each case, like \"unable to convert\n    EUC-JP to UTF-8: iconv_open: Invalid argument\". (Sadly EINVAL is\n    what iconv_open seems to return when you it doesn't know about a\n    particular encoding; it may be nicer to translate to something more\n    reasonable than what strerror() provides).\n\n> > +char *reencode_string_len(const char *in, int insz,\n> > +\t\t\t  const char *out_encoding, const char *in_encoding,\n> > +\t\t\t  int *outsz)\n> > +{\n> > +\tif (!same_encoding(in_encoding, out_encoding))\n> > +\t\twarning(\"Iconv support is disabled at compile time. It is likely that\\nincorrect data will be printed or stored in repository.\\nConsider using other build for this task.\");\n> > +\treturn NULL;\n> > +}\n> \n> Hmmm, I suspect this may be seen as regression by those who build\n> Git without ICONV for performance, knowing that there is nothing in\n> their data that requires character set conversion.\n\nI don't think it matters that much. The obvious tight loop is\nlogmsg_reencode, and it already checks same_encoding (because it really\nwants to avoid reallocation in the first place if it can). So anybody\nwho cares about the performance of reencode_string_len would do better\nto optimize out any calls to it. :)\n\nIf anything, we could make same_encoding faster by memo-izing its ptrs,\nlike:\n\ndiff --git a/utf8.c b/utf8.c\nindex 28e6d76..50a8ac0 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -409,13 +409,26 @@ int is_encoding_utf8(const char *name)\n \treturn 0;\n }\n \n-int same_encoding(const char *src, const char *dst)\n+static int same_encoding_1(const char *src, const char *dst)\n {\n+\twarning(\"actually checking same_encoding(%s, %s)\", src, dst);\n \tif (is_encoding_utf8(src) && is_encoding_utf8(dst))\n \t\treturn 1;\n \treturn !strcasecmp(src, dst);\n }\n \n+int same_encoding(const char *src, const char *dst)\n+{\n+\tstatic const char *cached_src, *cached_dst;\n+\tstatic int cached_ret = -1;\n+\n+\tif (src == cached_src && dst == cached_dst && cached_ret >= 0)\n+\t\treturn cached_ret;\n+\tcached_src = src;\n+\tcached_dst = dst;\n+\treturn cached_ret = same_encoding_1(src, dst);\n+}\n+\n /*\n  * Wrapper for fprintf and returns the total number of columns required\n  * for the printed string, assuming that the string is utf8.\n\n\nBut I couldn't measure any real speedup on \"git log --oneline\" from\ndoing so. It's also kind of gross (it will yield the wrong answer if you\nwrite a different encoding to the same buffer; I don't think we do that,\nbut it's quite a gotcha).\n\nAnother approach would be to preserve NULL encodings (which we treat as\nutf8) through the code base more. The common case of utf8 should be a\nquick check for two NULLs, then. Unfortunately just teaching\nget_commit_encoding and get_log_output_encoding to return NULL isn't\nenough. Some parts of the code want to output the actual value (e.g.,\nformat-patch for a charset header), and would need to be adjusted. Given\nthat I couldn't measure any speedup, I don't think it's worth pursuing,\nthough.\n\n-Peff\n"},{"id":"268216","messageId":"xmqqbne5d7bi.fsf@gitster.dls.corp.google.com","threadId":"39548","inReplyTo":"20150817190238.GA3594@sigill.intra.peff.net","subject":"Re: [PATCH v2] utf8.c: print warning about iconv errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-17T19:49:05Z","receivedAt":"2015-08-17T19:49:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, Aug 14, 2015 at 03:35:58PM -0700, Junio C Hamano wrote:\n>\n>> Max Kirillov <max@max630.net> writes:\n>> \n>> > * do not limit number of warnings - does not worth complicating the code\n>> \n>> Unless the warning leads to a quick \"die()\", wouldn't this make Git\n>> unusable by spewing a \"falling back to verbatim copy\" for each and\n>> every line of the message of a commit that has 'encoding' element in\n>> its header in the \"git log\" output, no?\n>\n> We only do the reencode once per commit. So it would be once per commit\n> rather than once per line. Which still sounds kind of annoying, if you\n> are using \"git log --oneline\" or similar.\n>\n> I think I'd favor a single warning in general, along the lines of\n> \"some encodings could not be converted\". But of course if you are trying\n> to figure out _which_ encodings your system doesn't have, that's not\n> very helpful. Maybe we could have an advice.encodingFailure config flag\n> with a tristate:\n>\n>   - false: don't spew any warnings\n>\n>   - true: give a generic warning once per program\n>\n>   - all: give a specific warning for each case, like \"unable to convert\n>     EUC-JP to UTF-8: iconv_open: Invalid argument\". (Sadly EINVAL is\n>     what iconv_open seems to return when you it doesn't know about a\n>     particular encoding; it may be nicer to translate to something more\n>     reasonable than what strerror() provides).\n\nSounds sensible.\n\n>> > +char *reencode_string_len(const char *in, int insz,\n>> > +\t\t\t  const char *out_encoding, const char *in_encoding,\n>> > +\t\t\t  int *outsz)\n>> > +{\n>> > +\tif (!same_encoding(in_encoding, out_encoding))\n>> > + warning(\"Iconv support is disabled at compile time. It is likely\n>> > that\\nincorrect data will be printed or stored in\n>> > repository.\\nConsider using other build for this task.\");\n>> > +\treturn NULL;\n>> > +}\n>> \n>> Hmmm, I suspect this may be seen as regression by those who build\n>> Git without ICONV for performance, knowing that there is nothing in\n>> their data that requires character set conversion.\n>\n> I don't think it matters that much.\n\nYeah, I think I agree.  Thanks.\n"}]}