{"thread":{"id":"25274","subject":"[PATCH] gettext: use libcharset when available","startedAt":"2010-09-28T16:05:26Z","lastAt":"2010-09-29T13:43:34Z","messageCount":12,"participants":["Erik Faye-Lund","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"151929","messageId":"1285689926-5048-1-git-send-email-kusmabite@gmail.com","threadId":"25274","inReplyTo":null,"subject":"[PATCH] gettext: use libcharset when available","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-09-28T16:05:26Z","receivedAt":"2010-09-28T16:05:26Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"libcharset provides an even more portable way of quering the charset\nof the current locale.\n\nUse that instead of nl_langinfo unless NO_LIBCHARSET is set.\n\nSigned-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n---\n\nWindows doesn't have langinfo.h and nl_langinfo(), but libcharset was\ninvented for this very purpose. With this patch on top, ab/i18n\ncompiles without errors in msysGit.\n\nThere's still a bunch of lower-level issues on Windows, like gettext\nending up overloading our winansi-wrappings for printf and friends,\nbut let's take thinks one step at the time :)\n\n configure.ac |    6 ++++++\n gettext.c    |   10 +++++++++-\n 2 files changed, 15 insertions(+), 1 deletions(-)\n\ndiff --git a/configure.ac b/configure.ac\nindex 1821d89..d3139cd 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -810,6 +810,12 @@ AC_CHECK_HEADER([libintl.h],\n [NO_GETTEXT=YesPlease])\n AC_SUBST(NO_GETTEXT)\n #\n+# Define NO_LIBCHARSET if you don't have libcharset.h\n+AC_CHECK_HEADER([libcharset.h],\n+[NO_LIBCHARSET=],\n+[NO_LIBCHARSET=YesPlease])\n+AC_SUBST(NO_LIBCHARSET)\n+#\n # Define NO_STRCASESTR if you don't have strcasestr.\n GIT_CHECK_FUNC(strcasestr,\n [NO_STRCASESTR=],\ndiff --git a/gettext.c b/gettext.c\nindex 8644098..902268c 100644\n--- a/gettext.c\n+++ b/gettext.c\n@@ -1,13 +1,17 @@\n #include \"exec_cmd.h\"\n #include <locale.h>\n #include <libintl.h>\n+#ifndef NO_LIBCHARSET\n+#include <libcharset.h>\n+#else\n #include <langinfo.h>\n+#endif\n #include <stdlib.h>\n \n extern void git_setup_gettext(void) {\n \tchar *podir;\n \tchar *envdir = getenv(\"GIT_TEXTDOMAINDIR\");\n-\tchar *charset;\n+\tconst char *charset;\n \n \tif (envdir) {\n \t\t(void)bindtextdomain(\"git\", envdir);\n@@ -20,7 +24,11 @@ extern void git_setup_gettext(void) {\n \n \t(void)setlocale(LC_MESSAGES, \"\");\n \t(void)setlocale(LC_CTYPE, \"\");\n+#ifndef NO_LIBCHARSET\n+\tcharset = locale_charset();\n+#else\n \tcharset = nl_langinfo(CODESET);\n+#endif\n \t(void)bind_textdomain_codeset(\"git\", charset);\n \t(void)setlocale(LC_CTYPE, \"C\");\n \t(void)textdomain(\"git\");\n-- \n1.7.3.165.gdfe39.dirty\n"},{"id":"151934","messageId":"AANLkTin_fu02QN0cUX15+02iO7xJ+tii5aVBqR2V_AVc@mail.gmail.com","threadId":"25274","inReplyTo":"1285689926-5048-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH] gettext: use libcharset when available","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-28T17:07:51Z","receivedAt":"2010-09-28T17:07:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Sep 28, 2010 at 16:05, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n> libcharset provides an even more portable way of quering the charset\n> of the current locale.\n>\n> Use that instead of nl_langinfo unless NO_LIBCHARSET is set.\n>\n> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n> ---\n>\n> Windows doesn't have langinfo.h and nl_langinfo(), but libcharset was\n> invented for this very purpose. With this patch on top, ab/i18n\n> compiles without errors in msysGit.\n>\n> There's still a bunch of lower-level issues on Windows, like gettext\n> ending up overloading our winansi-wrappings for printf and friends,\n> but let's take thinks one step at the time :)\n>\n>  configure.ac |    6 ++++++\n>  gettext.c    |   10 +++++++++-\n>  2 files changed, 15 insertions(+), 1 deletions(-)\n>\n> diff --git a/configure.ac b/configure.ac\n> index 1821d89..d3139cd 100644\n> --- a/configure.ac\n> +++ b/configure.ac\n> @@ -810,6 +810,12 @@ AC_CHECK_HEADER([libintl.h],\n>  [NO_GETTEXT=YesPlease])\n>  AC_SUBST(NO_GETTEXT)\n>  #\n> +# Define NO_LIBCHARSET if you don't have libcharset.h\n> +AC_CHECK_HEADER([libcharset.h],\n> +[NO_LIBCHARSET=],\n> +[NO_LIBCHARSET=YesPlease])\n> +AC_SUBST(NO_LIBCHARSET)\n> +#\n>  # Define NO_STRCASESTR if you don't have strcasestr.\n>  GIT_CHECK_FUNC(strcasestr,\n>  [NO_STRCASESTR=],\n> diff --git a/gettext.c b/gettext.c\n> index 8644098..902268c 100644\n> --- a/gettext.c\n> +++ b/gettext.c\n> @@ -1,13 +1,17 @@\n>  #include \"exec_cmd.h\"\n>  #include <locale.h>\n>  #include <libintl.h>\n> +#ifndef NO_LIBCHARSET\n> +#include <libcharset.h>\n> +#else\n>  #include <langinfo.h>\n> +#endif\n>  #include <stdlib.h>\n>\n>  extern void git_setup_gettext(void) {\n>        char *podir;\n>        char *envdir = getenv(\"GIT_TEXTDOMAINDIR\");\n> -       char *charset;\n> +       const char *charset;\n>\n>        if (envdir) {\n>                (void)bindtextdomain(\"git\", envdir);\n> @@ -20,7 +24,11 @@ extern void git_setup_gettext(void) {\n>\n>        (void)setlocale(LC_MESSAGES, \"\");\n>        (void)setlocale(LC_CTYPE, \"\");\n> +#ifndef NO_LIBCHARSET\n> +       charset = locale_charset();\n> +#else\n>        charset = nl_langinfo(CODESET);\n> +#endif\n>        (void)bind_textdomain_codeset(\"git\", charset);\n>        (void)setlocale(LC_CTYPE, \"C\");\n>        (void)textdomain(\"git\");\n\nThanks for porting it to Windows. Some points:\n\n * Nit: Should be NEEDS_LIBCHARSET instead of NO_LIBCHARSET, all the\n   variables that set library inclusions in the Makefile use the\n   NEED_* names.\n\n * GHC had a patch like this, it seems it affects NetBSD and OpenBSD\n   too, can anyone with these systems confirm:\n   http://hackage.haskell.org/trac/ghc/ticket/4080\n\n * Their patch compiles a program that includes libcharset.h and\n   compiles \"const char* charset = locale_charset();\". I don't know if\n   this is needed, or whether just checking the header name like\n   you've done will do.\n\n * They also have a HAVE_LANGINFO_H define and fall back on just\n   returning \"\", which works on GNU iconv. Maybe we should do this\n   too?\n\nI'm not sure about any of this, since I've just been testing on\nSolaris, Linux and FreeBSD.\n"},{"id":"151937","messageId":"AANLkTinHCETsaM=ytHuE9S5A+uAb=e3YLETsJJLga+DM@mail.gmail.com","threadId":"25274","inReplyTo":"AANLkTin_fu02QN0cUX15+02iO7xJ+tii5aVBqR2V_AVc@mail.gmail.com","subject":"Re: [PATCH] gettext: use libcharset when available","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-09-28T17:42:31Z","receivedAt":"2010-09-28T17:42:31Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Sep 28, 2010 at 7:07 PM, Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> On Tue, Sep 28, 2010 at 16:05, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n>> libcharset provides an even more portable way of quering the charset\n>> of the current locale.\n>>\n>> Use that instead of nl_langinfo unless NO_LIBCHARSET is set.\n>>\n>> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n>> ---\n>>\n>> Windows doesn't have langinfo.h and nl_langinfo(), but libcharset was\n>> invented for this very purpose. With this patch on top, ab/i18n\n>> compiles without errors in msysGit.\n>>\n>> There's still a bunch of lower-level issues on Windows, like gettext\n>> ending up overloading our winansi-wrappings for printf and friends,\n>> but let's take thinks one step at the time :)\n>>\n>>  configure.ac |    6 ++++++\n>>  gettext.c    |   10 +++++++++-\n>>  2 files changed, 15 insertions(+), 1 deletions(-)\n>>\n>> diff --git a/configure.ac b/configure.ac\n>> index 1821d89..d3139cd 100644\n>> --- a/configure.ac\n>> +++ b/configure.ac\n>> @@ -810,6 +810,12 @@ AC_CHECK_HEADER([libintl.h],\n>>  [NO_GETTEXT=YesPlease])\n>>  AC_SUBST(NO_GETTEXT)\n>>  #\n>> +# Define NO_LIBCHARSET if you don't have libcharset.h\n>> +AC_CHECK_HEADER([libcharset.h],\n>> +[NO_LIBCHARSET=],\n>> +[NO_LIBCHARSET=YesPlease])\n>> +AC_SUBST(NO_LIBCHARSET)\n>> +#\n>>  # Define NO_STRCASESTR if you don't have strcasestr.\n>>  GIT_CHECK_FUNC(strcasestr,\n>>  [NO_STRCASESTR=],\n>> diff --git a/gettext.c b/gettext.c\n>> index 8644098..902268c 100644\n>> --- a/gettext.c\n>> +++ b/gettext.c\n>> @@ -1,13 +1,17 @@\n>>  #include \"exec_cmd.h\"\n>>  #include <locale.h>\n>>  #include <libintl.h>\n>> +#ifndef NO_LIBCHARSET\n>> +#include <libcharset.h>\n>> +#else\n>>  #include <langinfo.h>\n>> +#endif\n>>  #include <stdlib.h>\n>>\n>>  extern void git_setup_gettext(void) {\n>>        char *podir;\n>>        char *envdir = getenv(\"GIT_TEXTDOMAINDIR\");\n>> -       char *charset;\n>> +       const char *charset;\n>>\n>>        if (envdir) {\n>>                (void)bindtextdomain(\"git\", envdir);\n>> @@ -20,7 +24,11 @@ extern void git_setup_gettext(void) {\n>>\n>>        (void)setlocale(LC_MESSAGES, \"\");\n>>        (void)setlocale(LC_CTYPE, \"\");\n>> +#ifndef NO_LIBCHARSET\n>> +       charset = locale_charset();\n>> +#else\n>>        charset = nl_langinfo(CODESET);\n>> +#endif\n>>        (void)bind_textdomain_codeset(\"git\", charset);\n>>        (void)setlocale(LC_CTYPE, \"C\");\n>>        (void)textdomain(\"git\");\n>\n> Thanks for porting it to Windows. Some points:\n>\n>  * Nit: Should be NEEDS_LIBCHARSET instead of NO_LIBCHARSET, all the\n>   variables that set library inclusions in the Makefile use the\n>   NEED_* names.\n>\n\nThat's not true, at least NO_OPENSSL, NO_PTHREADS, NO_ICONV,\nNO_LIBGEN_H, NO_MMAP and NO_SYS_SELECT_H use the NO_-prefix to include\nlibraries (as opposed to the NEEDS_-prefix). And to be honest, I think\nthe NEEDS_-prefix would be a lie in this case; we don't NEED it, we\ntake advantage of it if it's there.\n\nTo be honest, I just mimicked what was done for detection of gettext.\nPerhaps HAVE_LIBCHARSET_H would be the appropriate define? It's what\nwe use for paths.h (HAVE_PATHS_H)...\n\n>  * Their patch compiles a program that includes libcharset.h and\n>   compiles \"const char* charset = locale_charset();\". I don't know if\n>   this is needed, or whether just checking the header name like\n>   you've done will do.\n>\n\nI don't think there's any point to it - libcharset.h is pretty much\njust about that one function. And if it turns out I'm wrong, we can\nadd a check like that in the future.\n\n>  * They also have a HAVE_LANGINFO_H define and fall back on just\n>   returning \"\", which works on GNU iconv. Maybe we should do this\n>   too?\n>\n\nPerhaps, but can't that wait until we encounter a system that needs it?\n\nWhat's interesting there is what you say about GNU iconv handling \"\".\nThis means that the minimal fix for Windows could be even smaller -\nwe're using GNU iconv :)\n\n> I'm not sure about any of this, since I've just been testing on\n> Solaris, Linux and FreeBSD.\n\nI think Solaris, Linux, FreeBSD and Windows is a pretty wide selection\nof platforms, so I hope it should be pretty painless once this hits\n'next'. Sure, there might be some platforms that needs a fix-up, but\nthere always is with new code. And besides, there's even time to test\nmore platforms before merging to 'next', no? :)\n"},{"id":"151941","messageId":"1285698577-28395-1-git-send-email-avarab@gmail.com","threadId":"25274","inReplyTo":"AANLkTinHCETsaM=ytHuE9S5A+uAb=e3YLETsJJLga+DM@mail.gmail.com","subject":"[PATCH/RFC 0/2] use libcharset.h with gettext if available","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-28T18:29:35Z","receivedAt":"2010-09-28T18:29:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Sep 28, 2010 at 17:42, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n> On Tue, Sep 28, 2010 at 7:07 PM, Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n>> On Tue, Sep 28, 2010 at 16:05, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n>>> libcharset provides an even more portable way of quering the charset\n>>> of the current locale.\n>>>\n>>> Use that instead of nl_langinfo unless NO_LIBCHARSET is set.\n>>>\n>>> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n>>> ---\n>>>\n>>> Windows doesn't have langinfo.h and nl_langinfo(), but libcharset was\n>>> invented for this very purpose. With this patch on top, ab/i18n\n>>> compiles without errors in msysGit.\n>>>\n>>> There's still a bunch of lower-level issues on Windows, like gettext\n>>> ending up overloading our winansi-wrappings for printf and friends,\n>>> but let's take thinks one step at the time :)\n>>>\n>>>  configure.ac |    6 ++++++\n>>>  gettext.c    |   10 +++++++++-\n>>>  2 files changed, 15 insertions(+), 1 deletions(-)\n>>>\n>>> diff --git a/configure.ac b/configure.ac\n>>> index 1821d89..d3139cd 100644\n>>> --- a/configure.ac\n>>> +++ b/configure.ac\n>>> @@ -810,6 +810,12 @@ AC_CHECK_HEADER([libintl.h],\n>>>  [NO_GETTEXT=YesPlease])\n>>>  AC_SUBST(NO_GETTEXT)\n>>>  #\n>>> +# Define NO_LIBCHARSET if you don't have libcharset.h\n>>> +AC_CHECK_HEADER([libcharset.h],\n>>> +[NO_LIBCHARSET=],\n>>> +[NO_LIBCHARSET=YesPlease])\n>>> +AC_SUBST(NO_LIBCHARSET)\n>>> +#\n>>>  # Define NO_STRCASESTR if you don't have strcasestr.\n>>>  GIT_CHECK_FUNC(strcasestr,\n>>>  [NO_STRCASESTR=],\n>>> diff --git a/gettext.c b/gettext.c\n>>> index 8644098..902268c 100644\n>>> --- a/gettext.c\n>>> +++ b/gettext.c\n>>> @@ -1,13 +1,17 @@\n>>>  #include \"exec_cmd.h\"\n>>>  #include <locale.h>\n>>>  #include <libintl.h>\n>>> +#ifndef NO_LIBCHARSET\n>>> +#include <libcharset.h>\n>>> +#else\n>>>  #include <langinfo.h>\n>>> +#endif\n>>>  #include <stdlib.h>\n>>>\n>>>  extern void git_setup_gettext(void) {\n>>>        char *podir;\n>>>        char *envdir = getenv(\"GIT_TEXTDOMAINDIR\");\n>>> -       char *charset;\n>>> +       const char *charset;\n>>>\n>>>        if (envdir) {\n>>>                (void)bindtextdomain(\"git\", envdir);\n>>> @@ -20,7 +24,11 @@ extern void git_setup_gettext(void) {\n>>>\n>>>        (void)setlocale(LC_MESSAGES, \"\");\n>>>        (void)setlocale(LC_CTYPE, \"\");\n>>> +#ifndef NO_LIBCHARSET\n>>> +       charset = locale_charset();\n>>> +#else\n>>>        charset = nl_langinfo(CODESET);\n>>> +#endif\n>>>        (void)bind_textdomain_codeset(\"git\", charset);\n>>>        (void)setlocale(LC_CTYPE, \"C\");\n>>>        (void)textdomain(\"git\");\n>>\n>> Thanks for porting it to Windows. Some points:\n>>\n>>  * Nit: Should be NEEDS_LIBCHARSET instead of NO_LIBCHARSET, all the\n>>   variables that set library inclusions in the Makefile use the\n>>   NEED_* names.\n>>\n>\n> That's not true, at least NO_OPENSSL, NO_PTHREADS, NO_ICONV,\n> NO_LIBGEN_H, NO_MMAP and NO_SYS_SELECT_H use the NO_-prefix to include\n> libraries (as opposed to the NEEDS_-prefix). And to be honest, I think\n> the NEEDS_-prefix would be a lie in this case; we don't NEED it, we\n> take advantage of it if it's there.\n>\n> To be honest, I just mimicked what was done for detection of gettext.\n> Perhaps HAVE_LIBCHARSET_H would be the appropriate define? It's what\n> we use for paths.h (HAVE_PATHS_H)...\n\nYou're right, a NO_* makes more sense here.\n\n>>  * Their patch compiles a program that includes libcharset.h and\n>>   compiles \"const char* charset = locale_charset();\". I don't know if\n>>   this is needed, or whether just checking the header name like\n>>   you've done will do.\n>>\n>\n> I don't think there's any point to it - libcharset.h is pretty much\n> just about that one function. And if it turns out I'm wrong, we can\n> add a check like that in the future.\n>\n>>  * They also have a HAVE_LANGINFO_H define and fall back on just\n>>   returning \"\", which works on GNU iconv. Maybe we should do this\n>>   too?\n>>\n>\n> Perhaps, but can't that wait until we encounter a system that needs it?\n\nTo both of the above: It's completely reasonable as-is, I just wanted\nto mention it in case a list member knew of a reason to do it like GHC\ndid it. But just checking for the header will do fine until we\nencounter issues with that.\n\n> What's interesting there is what you say about GNU iconv handling \"\".\n> This means that the minimal fix for Windows could be even smaller -\n> we're using GNU iconv :)\n\nYeah, but probably better to do it by using libcharset.h instead.\n\n>> I'm not sure about any of this, since I've just been testing on\n>> Solaris, Linux and FreeBSD.\n>\n> I think Solaris, Linux, FreeBSD and Windows is a pretty wide selection\n> of platforms, so I hope it should be pretty painless once this hits\n> 'next'. Sure, there might be some platforms that needs a fix-up, but\n> there always is with new code. And besides, there's even time to test\n> more platforms before merging to 'next', no? :)\n\nYeah, I'm hoping it'll get into next soon so we can get more\nreports/fixes like these. Anyway, I amended your patches a bit, here\nare the changes:\n\n * Split up the s/char*/const char*/ change into its own patch, or is\n   there a reason for why this needs to be there along with the\n   libcharset.h change?\n\n * Added docs about the define to the Makefile\n\n * Added defaults for NO_LIBCHARSET to the default, I only changed the\n   defaults for the MINGW entry, maybe it should be changed on Cygwin\n   and Windows too? And probably on OpenBSD and NetBSD too.\n\nErik Faye-Lund (2):\n  gettext: use const char* instead of char*\n  gettext: use libcharset when available\n\n Makefile     |   17 +++++++++++++++++\n configure.ac |    6 ++++++\n gettext.c    |   10 +++++++++-\n 3 files changed, 32 insertions(+), 1 deletions(-)\n\n-- \n1.7.3.159.g610493\n"},{"id":"151942","messageId":"1285698577-28395-2-git-send-email-avarab@gmail.com","threadId":"25274","inReplyTo":"AANLkTinHCETsaM=ytHuE9S5A+uAb=e3YLETsJJLga+DM@mail.gmail.com","subject":"[PATCH/RFC 1/2] gettext: use const char* instead of char*","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-28T18:29:36Z","receivedAt":"2010-09-28T18:29:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Erik Faye-Lund <kusmabite@gmail.com>\n\nChange the charset variable in git_setup_gettext from char* to const\nchar*.\n\nSigned-off-by: Erik Faye-Lund <kusmabite@gmail.com>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n gettext.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/gettext.c b/gettext.c\nindex 8644098..d20bb39 100644\n--- a/gettext.c\n+++ b/gettext.c\n@@ -7,7 +7,7 @@\n extern void git_setup_gettext(void) {\n \tchar *podir;\n \tchar *envdir = getenv(\"GIT_TEXTDOMAINDIR\");\n-\tchar *charset;\n+\tconst char *charset;\n \n \tif (envdir) {\n \t\t(void)bindtextdomain(\"git\", envdir);\n-- \n1.7.3.159.g610493\n"},{"id":"151968","messageId":"AANLkTinxSqPD8H4Giigb1vbi7DC2wpDVqWmXGVZXmeKQ@mail.gmail.com","threadId":"25274","inReplyTo":"1285698577-28395-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH/RFC 0/2] use libcharset.h with gettext if available","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-09-28T21:47:38Z","receivedAt":"2010-09-28T21:47:38Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Sep 28, 2010 at 8:29 PM, Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> Yeah, I'm hoping it'll get into next soon so we can get more\n> reports/fixes like these. Anyway, I amended your patches a bit, here\n> are the changes:\n>\n>  * Split up the s/char*/const char*/ change into its own patch, or is\n>   there a reason for why this needs to be there along with the\n>   libcharset.h change?\n>\n\nThe reason was that my version of locale_charset() returns a const\nchar *, so I got a warning if I didn't. nl_langinfo() returns a char\n*, so I don't think that constness-fix patch makes sense in itself.\nBut what might make more sense would be to squash it into the original\ncommit for that line.\n\n>  * Added docs about the define to the Makefile\n>\n\nNice!\n\n>  * Added defaults for NO_LIBCHARSET to the default, I only changed the\n>   defaults for the MINGW entry, maybe it should be changed on Cygwin\n>   and Windows too? And probably on OpenBSD and NetBSD too.\n>\n\nI don't think NO_LIBCHARSET should be the default. libcharset is\nreported to be a bit better than nl_langinfo at normalizing the\nencoding, and GNU gettext depends on libcharset (through libiconv,\nwhich libcharset is distributed with). So in the case of a GNU\ngettext, libcharset should really be present.\n\n> Erik Faye-Lund (2):\n>  gettext: use const char* instead of char*\n>  gettext: use libcharset when available\n>\n>  Makefile     |   17 +++++++++++++++++\n>  configure.ac |    6 ++++++\n>  gettext.c    |   10 +++++++++-\n>  3 files changed, 32 insertions(+), 1 deletions(-)\n>\n> --\n> 1.7.3.159.g610493\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"152036","messageId":"AANLkTin90pKaVGVaG-4PW50TAhgfp1QVtrKpvyVxpnW0@mail.gmail.com","threadId":"25274","inReplyTo":"AANLkTinxSqPD8H4Giigb1vbi7DC2wpDVqWmXGVZXmeKQ@mail.gmail.com","subject":"Re: [PATCH/RFC 0/2] use libcharset.h with gettext if available","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-29T10:00:14Z","receivedAt":"2010-09-29T10:00:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Sep 28, 2010 at 21:47, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n> On Tue, Sep 28, 2010 at 8:29 PM, Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n>> Yeah, I'm hoping it'll get into next soon so we can get more\n>> reports/fixes like these. Anyway, I amended your patches a bit, here\n>> are the changes:\n>>\n>>  * Split up the s/char*/const char*/ change into its own patch, or is\n>>   there a reason for why this needs to be there along with the\n>>   libcharset.h change?\n>>\n>\n> The reason was that my version of locale_charset() returns a const\n> char *, so I got a warning if I didn't. nl_langinfo() returns a char\n> *, so I don't think that constness-fix patch makes sense in itself.\n> But what might make more sense would be to squash it into the original\n> commit for that line.\n\nAh, then it should be part of the patch. I'll add a note about the\ndifferent prototypes to it.\n\n>>  * Added docs about the define to the Makefile\n>>\n>\n> Nice!\n>\n>>  * Added defaults for NO_LIBCHARSET to the default, I only changed the\n>>   defaults for the MINGW entry, maybe it should be changed on Cygwin\n>>   and Windows too? And probably on OpenBSD and NetBSD too.\n>>\n>\n> I don't think NO_LIBCHARSET should be the default. libcharset is\n> reported to be a bit better than nl_langinfo at normalizing the\n> encoding, and GNU gettext depends on libcharset (through libiconv,\n> which libcharset is distributed with). So in the case of a GNU\n> gettext, libcharset should really be present.\n\nI can't find any package (with apt-file) on Debian or Ubuntu that\nprovides libcharset.h, but I have langinfo.h on those systems.\n\nThe GNU gettext manual also reccomends the use of nl_langinfo in\n\"11.2.4 How to specify the output character set `gettext' uses\", so it\nseems that using that and not libiconv is the default way of doing\nthings on GNU gettext + GNU libc systems.\n"},{"id":"152043","messageId":"AANLkTikMWy2a0M6bFMj+Jb+2QewUGsUjMsbv=XyR1Tr9@mail.gmail.com","threadId":"25274","inReplyTo":"AANLkTin90pKaVGVaG-4PW50TAhgfp1QVtrKpvyVxpnW0@mail.gmail.com","subject":"Re: [PATCH/RFC 0/2] use libcharset.h with gettext if available","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-09-29T11:41:37Z","receivedAt":"2010-09-29T11:41:37Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Sep 29, 2010 at 12:00 PM, Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> On Tue, Sep 28, 2010 at 21:47, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n>> On Tue, Sep 28, 2010 at 8:29 PM, Ævar Arnfjörð Bjarmason\n>>>  * Added defaults for NO_LIBCHARSET to the default, I only changed the\n>>>   defaults for the MINGW entry, maybe it should be changed on Cygwin\n>>>   and Windows too? And probably on OpenBSD and NetBSD too.\n>>>\n>>\n>> I don't think NO_LIBCHARSET should be the default. libcharset is\n>> reported to be a bit better than nl_langinfo at normalizing the\n>> encoding, and GNU gettext depends on libcharset (through libiconv,\n>> which libcharset is distributed with). So in the case of a GNU\n>> gettext, libcharset should really be present.\n>\n> I can't find any package (with apt-file) on Debian or Ubuntu that\n> provides libcharset.h, but I have langinfo.h on those systems.\n>\n\nStrange. A 'make install' on libiconv installed libcharset.h to\n$prefix/include on my system. But looking a bit deeper, it seems that\nglibc supplies it's own iconv implementation (perhaps based on\nlibiconv, I don't know). So yes, I tend to agree with you. GNU\nplatforms should not be expected to have libcharset.\n\n> The GNU gettext manual also reccomends the use of nl_langinfo in\n> \"11.2.4 How to specify the output character set `gettext' uses\", so it\n> seems that using that and not libiconv is the default way of doing\n> things on GNU gettext + GNU libc systems.\n>\n\nOK, fair enough. I based my comment on some comment by the GNU gettext\nmaintainer (who is also the libcharset maintainer - libcharset does in\nfact use nl_langinfo if present), but since this is in the manual I\nfully withdraw my comment.\n\nThen again, if this is an opt-in rather than an opt-out, perhaps we\nshould change the switch to HAVE_LIBCHARSET? I don't mean to go in\ncircles here, but it sounds more self-documenting to me.\n"},{"id":"152049","messageId":"1285765637-16381-1-git-send-email-avarab@gmail.com","threadId":"25274","inReplyTo":"AANLkTikMWy2a0M6bFMj+Jb+2QewUGsUjMsbv=XyR1Tr9@mail.gmail.com","subject":"[PATCH] gettext: use libcharset when available","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-29T13:07:17Z","receivedAt":"2010-09-29T13:07:17Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Erik Faye-Lund <kusmabite@gmail.com>\n\nChange the git_setup_gettext function to use libcharset to query the\ncharacter set of the current locale if it's available. I.e. use it\ninstead of nl_langinfo if HAVE_LIBCHARSET_H is set.\n\nThe GNU gettext manual recommends using langinfo.h's\nnl_langinfo(CODESET) to acquire the current character set, but on\nsystems that have libcharset.h's locale_charset() using the latter is\neither saner, or the only option on those systems.\n\nGNU and Solaris have a nl_langinfo(CODESET), FreeBSD can use either,\nbut MingW and some others need to use libcharset.h's locale_charset()\ninstead.\n\nSince locale_charset returns a const char* instead of char* as\nnl_langinfo does the type of the variable we're using to store the\ncharset in git_setup_gettext has been changed.\n\nSigned-off-by: Erik Faye-Lund <kusmabite@gmail.com>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nJunio, this goes on top of ab/i18n.\n\nOn Wed, Sep 29, 2010 at 11:41, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n> On Wed, Sep 29, 2010 at 12:00 PM, Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n>> On Tue, Sep 28, 2010 at 21:47, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n>>> On Tue, Sep 28, 2010 at 8:29 PM, Ævar Arnfjörð Bjarmason\n>>>>  * Added defaults for NO_LIBCHARSET to the default, I only changed the\n>>>>   defaults for the MINGW entry, maybe it should be changed on Cygwin\n>>>>   and Windows too? And probably on OpenBSD and NetBSD too.\n>>>>\n>>>\n>>> I don't think NO_LIBCHARSET should be the default. libcharset is\n>>> reported to be a bit better than nl_langinfo at normalizing the\n>>> encoding, and GNU gettext depends on libcharset (through libiconv,\n>>> which libcharset is distributed with). So in the case of a GNU\n>>> gettext, libcharset should really be present.\n>>\n>> I can't find any package (with apt-file) on Debian or Ubuntu that\n>> provides libcharset.h, but I have langinfo.h on those systems.\n>>\n>\n> Strange. A 'make install' on libiconv installed libcharset.h to\n> $prefix/include on my system. But looking a bit deeper, it seems that\n> glibc supplies it's own iconv implementation (perhaps based on\n> libiconv, I don't know). So yes, I tend to agree with you. GNU\n> platforms should not be expected to have libcharset.\n\nI asked around and none of Debian/Ubuntu/Fedora and other Linux\nsystems have libcharset.h, and using the gettext branch without\nlibcharset.h on FreeBSD works fine.\n\n>> The GNU gettext manual also reccomends the use of nl_langinfo in\n>> \"11.2.4 How to specify the output character set `gettext' uses\", so it\n>> seems that using that and not libiconv is the default way of doing\n>> things on GNU gettext + GNU libc systems.\n>>\n>\n> OK, fair enough. I based my comment on some comment by the GNU gettext\n> maintainer (who is also the libcharset maintainer - libcharset does in\n> fact use nl_langinfo if present), but since this is in the manual I\n> fully withdraw my comment.\n>\n> Then again, if this is an opt-in rather than an opt-out, perhaps we\n> should change the switch to HAVE_LIBCHARSET? I don't mean to go in\n> circles here, but it sounds more self-documenting to me.\n\nAgreed, changed in this version. I also added a bit to config.mak.in\nto make the configure.ac change actually do something, and changed the\ndocs & commit message.\n\n Makefile      |   17 +++++++++++++++++\n config.mak.in |    1 +\n configure.ac  |    6 ++++++\n gettext.c     |   10 +++++++++-\n 4 files changed, 33 insertions(+), 1 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 680e578..a05396b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -43,6 +43,12 @@ all::\n # on platforms where we don't expect glibc (Linux, Hurd,\n # GNU/kFreeBSD), which includes libintl.\n #\n+# Define HAVE_LIBCHARSET_H if you haven't set NO_GETTEXT and you can't\n+# trust the langinfo.h's nl_langinfo(CODESET) function to return the\n+# current character set. GNU and Solaris have a nl_langinfo(CODESET),\n+# FreeBSD can use either, but MingW and some others need to use\n+# libcharset.h's locale_charset() instead.\n+#\n # Define GNU_GETTEXT if you're using the GNU implementation of\n # libintl. We define this everywhere except on Solaris, which has its\n # own gettext implementation. If GNU_GETTEXT is set we'll use GNU\n@@ -792,6 +798,10 @@ ifndef NO_GETTEXT\n \t# Systems that don't use GNU gettext are the exception. Only\n \t# Solaris has a mature non-GNU gettext implementation.\n \tGNU_GETTEXT = YesPlease\n+\n+\t# Since we assume a GNU gettext by default we also assume a\n+\t# GNU-like langinfo.h by default\n+\tHAVE_LIBCHARSET_H =\n endif\n \n # We choose to avoid \"if .. else if .. else .. endif endif\"\n@@ -1180,6 +1190,9 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT))\n \tEXTLIBS += /mingw/lib/libz.a\n \tNO_R_TO_GCC_LINKER = YesPlease\n \tINTERNAL_QSORT = YesPlease\n+ifndef NO_GETTEXT\n+\tHAVE_LIBCHARSET_H = YesPlease\n+endif\n else\n \tNO_CURL = YesPlease\n endif\n@@ -1964,6 +1977,10 @@ config.s config.o: EXTRA_CPPFLAGS = -DETC_GITCONFIG='\"$(ETC_GITCONFIG_SQ)\"'\n \n http.s http.o: EXTRA_CPPFLAGS = -DGIT_HTTP_USER_AGENT='\"git/$(GIT_VERSION)\"'\n \n+ifdef HAVE_LIBCHARSET_H\n+gettext.s gettext.o: EXTRA_CPPFLAGS = -DHAVE_LIBCHARSET_H\n+endif\n+\n ifdef NO_EXPAT\n http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT\n endif\ndiff --git a/config.mak.in b/config.mak.in\nindex 9f47aa5..969cbaa 100644\n--- a/config.mak.in\n+++ b/config.mak.in\n@@ -34,6 +34,7 @@ NO_CURL=@NO_CURL@\n NO_EXPAT=@NO_EXPAT@\n NO_LIBGEN_H=@NO_LIBGEN_H@\n HAVE_PATHS_H=@HAVE_PATHS_H@\n+HAVE_LIBCHARSET_H=@HAVE_LIBCHARSET_H@\n NO_GETTEXT=@NO_GETTEXT@\n NEEDS_LIBICONV=@NEEDS_LIBICONV@\n NEEDS_SOCKET=@NEEDS_SOCKET@\ndiff --git a/configure.ac b/configure.ac\nindex 1821d89..b06bad1 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -810,6 +810,12 @@ AC_CHECK_HEADER([libintl.h],\n [NO_GETTEXT=YesPlease])\n AC_SUBST(NO_GETTEXT)\n #\n+# Define HAVE_LIBCHARSET_H if have libcharset.h\n+AC_CHECK_HEADER([libcharset.h],\n+[HAVE_LIBCHARSET_H=YesPlease],\n+[HAVE_LIBCHARSET_H=])\n+AC_SUBST(HAVE_LIBCHARSET_H)\n+#\n # Define NO_STRCASESTR if you don't have strcasestr.\n GIT_CHECK_FUNC(strcasestr,\n [NO_STRCASESTR=],\ndiff --git a/gettext.c b/gettext.c\nindex 8644098..9bdac56 100644\n--- a/gettext.c\n+++ b/gettext.c\n@@ -1,13 +1,17 @@\n #include \"exec_cmd.h\"\n #include <locale.h>\n #include <libintl.h>\n+#ifdef HAVE_LIBCHARSET_H\n+#include <libcharset.h>\n+#else\n #include <langinfo.h>\n+#endif\n #include <stdlib.h>\n \n extern void git_setup_gettext(void) {\n \tchar *podir;\n \tchar *envdir = getenv(\"GIT_TEXTDOMAINDIR\");\n-\tchar *charset;\n+\tconst char *charset;\n \n \tif (envdir) {\n \t\t(void)bindtextdomain(\"git\", envdir);\n@@ -20,7 +24,11 @@ extern void git_setup_gettext(void) {\n \n \t(void)setlocale(LC_MESSAGES, \"\");\n \t(void)setlocale(LC_CTYPE, \"\");\n+#ifdef HAVE_LIBCHARSET_H\n+\tcharset = locale_charset();\n+#else\n \tcharset = nl_langinfo(CODESET);\n+#endif\n \t(void)bind_textdomain_codeset(\"git\", charset);\n \t(void)setlocale(LC_CTYPE, \"C\");\n \t(void)textdomain(\"git\");\n-- \n1.7.3.159.g610493\n"},{"id":"152054","messageId":"AANLkTinRsryfdnj_uUH++yZrV1r_M+NNoXLm9TSO9N+J@mail.gmail.com","threadId":"25274","inReplyTo":"1285765637-16381-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH] gettext: use libcharset when available","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-09-29T13:34:54Z","receivedAt":"2010-09-29T13:34:54Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Sep 29, 2010 at 3:07 PM, Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> From: Erik Faye-Lund <kusmabite@gmail.com>\n>\n> Change the git_setup_gettext function to use libcharset to query the\n> character set of the current locale if it's available. I.e. use it\n> instead of nl_langinfo if HAVE_LIBCHARSET_H is set.\n>\n> The GNU gettext manual recommends using langinfo.h's\n> nl_langinfo(CODESET) to acquire the current character set, but on\n> systems that have libcharset.h's locale_charset() using the latter is\n> either saner, or the only option on those systems.\n>\n> GNU and Solaris have a nl_langinfo(CODESET), FreeBSD can use either,\n> but MingW and some others need to use libcharset.h's locale_charset()\n> instead.\n\nVery minor nit: It's officially spelled MinGW, with an upper-case G.\n\n>\n> Since locale_charset returns a const char* instead of char* as\n> nl_langinfo does the type of the variable we're using to store the\n> charset in git_setup_gettext has been changed.\n>\n> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>\n> Junio, this goes on top of ab/i18n.\n>\n\nWow, thanks for taking care of this!\n"},{"id":"152055","messageId":"1285767632-24852-1-git-send-email-avarab@gmail.com","threadId":"25274","inReplyTo":"AANLkTinRsryfdnj_uUH++yZrV1r_M+NNoXLm9TSO9N+J@mail.gmail.com","subject":"[PATCH v2] gettext: use libcharset when available","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-29T13:40:32Z","receivedAt":"2010-09-29T13:40:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Erik Faye-Lund <kusmabite@gmail.com>\n\nChange the git_setup_gettext function to use libcharset to query the\ncharacter set of the current locale if it's available. I.e. use it\ninstead of nl_langinfo if HAVE_LIBCHARSET_H is set.\n\nThe GNU gettext manual recommends using langinfo.h's\nnl_langinfo(CODESET) to acquire the current character set, but on\nsystems that have libcharset.h's locale_charset() using the latter is\neither saner, or the only option on those systems.\n\nGNU and Solaris have a nl_langinfo(CODESET), FreeBSD can use either,\nbut MinGW and some others need to use libcharset.h's locale_charset()\ninstead.\n\nSince locale_charset returns a const char* instead of char* as\nnl_langinfo does the type of the variable we're using to store the\ncharset in git_setup_gettext has been changed.\n\nSigned-off-by: Erik Faye-Lund <kusmabite@gmail.com>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nOn Wed, Sep 29, 2010 at 13:34, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n> Very minor nit: It's officially spelled MinGW, with an upper-case G.\n\nDid s/MingW/MinGW/ in the content & message in this v2.\n\n Makefile      |   19 ++++++++++++++++++-\n config.mak.in |    1 +\n configure.ac  |    6 ++++++\n gettext.c     |   10 +++++++++-\n 4 files changed, 34 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 680e578..00cff39 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -43,6 +43,12 @@ all::\n # on platforms where we don't expect glibc (Linux, Hurd,\n # GNU/kFreeBSD), which includes libintl.\n #\n+# Define HAVE_LIBCHARSET_H if you haven't set NO_GETTEXT and you can't\n+# trust the langinfo.h's nl_langinfo(CODESET) function to return the\n+# current character set. GNU and Solaris have a nl_langinfo(CODESET),\n+# FreeBSD can use either, but MinGW and some others need to use\n+# libcharset.h's locale_charset() instead.\n+#\n # Define GNU_GETTEXT if you're using the GNU implementation of\n # libintl. We define this everywhere except on Solaris, which has its\n # own gettext implementation. If GNU_GETTEXT is set we'll use GNU\n@@ -268,7 +274,7 @@ uname_P := $(shell sh -c 'uname -p 2>/dev/null || echo not')\n uname_V := $(shell sh -c 'uname -v 2>/dev/null || echo not')\n \n ifdef MSVC\n-\t# avoid the MingW and Cygwin configuration sections\n+\t# avoid the MinGW and Cygwin configuration sections\n \tuname_S := Windows\n \tuname_O := Windows\n endif\n@@ -792,6 +798,10 @@ ifndef NO_GETTEXT\n \t# Systems that don't use GNU gettext are the exception. Only\n \t# Solaris has a mature non-GNU gettext implementation.\n \tGNU_GETTEXT = YesPlease\n+\n+\t# Since we assume a GNU gettext by default we also assume a\n+\t# GNU-like langinfo.h by default\n+\tHAVE_LIBCHARSET_H =\n endif\n \n # We choose to avoid \"if .. else if .. else .. endif endif\"\n@@ -1180,6 +1190,9 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT))\n \tEXTLIBS += /mingw/lib/libz.a\n \tNO_R_TO_GCC_LINKER = YesPlease\n \tINTERNAL_QSORT = YesPlease\n+ifndef NO_GETTEXT\n+\tHAVE_LIBCHARSET_H = YesPlease\n+endif\n else\n \tNO_CURL = YesPlease\n endif\n@@ -1964,6 +1977,10 @@ config.s config.o: EXTRA_CPPFLAGS = -DETC_GITCONFIG='\"$(ETC_GITCONFIG_SQ)\"'\n \n http.s http.o: EXTRA_CPPFLAGS = -DGIT_HTTP_USER_AGENT='\"git/$(GIT_VERSION)\"'\n \n+ifdef HAVE_LIBCHARSET_H\n+gettext.s gettext.o: EXTRA_CPPFLAGS = -DHAVE_LIBCHARSET_H\n+endif\n+\n ifdef NO_EXPAT\n http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT\n endif\ndiff --git a/config.mak.in b/config.mak.in\nindex 9f47aa5..969cbaa 100644\n--- a/config.mak.in\n+++ b/config.mak.in\n@@ -34,6 +34,7 @@ NO_CURL=@NO_CURL@\n NO_EXPAT=@NO_EXPAT@\n NO_LIBGEN_H=@NO_LIBGEN_H@\n HAVE_PATHS_H=@HAVE_PATHS_H@\n+HAVE_LIBCHARSET_H=@HAVE_LIBCHARSET_H@\n NO_GETTEXT=@NO_GETTEXT@\n NEEDS_LIBICONV=@NEEDS_LIBICONV@\n NEEDS_SOCKET=@NEEDS_SOCKET@\ndiff --git a/configure.ac b/configure.ac\nindex 1821d89..b06bad1 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -810,6 +810,12 @@ AC_CHECK_HEADER([libintl.h],\n [NO_GETTEXT=YesPlease])\n AC_SUBST(NO_GETTEXT)\n #\n+# Define HAVE_LIBCHARSET_H if have libcharset.h\n+AC_CHECK_HEADER([libcharset.h],\n+[HAVE_LIBCHARSET_H=YesPlease],\n+[HAVE_LIBCHARSET_H=])\n+AC_SUBST(HAVE_LIBCHARSET_H)\n+#\n # Define NO_STRCASESTR if you don't have strcasestr.\n GIT_CHECK_FUNC(strcasestr,\n [NO_STRCASESTR=],\ndiff --git a/gettext.c b/gettext.c\nindex 8644098..9bdac56 100644\n--- a/gettext.c\n+++ b/gettext.c\n@@ -1,13 +1,17 @@\n #include \"exec_cmd.h\"\n #include <locale.h>\n #include <libintl.h>\n+#ifdef HAVE_LIBCHARSET_H\n+#include <libcharset.h>\n+#else\n #include <langinfo.h>\n+#endif\n #include <stdlib.h>\n \n extern void git_setup_gettext(void) {\n \tchar *podir;\n \tchar *envdir = getenv(\"GIT_TEXTDOMAINDIR\");\n-\tchar *charset;\n+\tconst char *charset;\n \n \tif (envdir) {\n \t\t(void)bindtextdomain(\"git\", envdir);\n@@ -20,7 +24,11 @@ extern void git_setup_gettext(void) {\n \n \t(void)setlocale(LC_MESSAGES, \"\");\n \t(void)setlocale(LC_CTYPE, \"\");\n+#ifdef HAVE_LIBCHARSET_H\n+\tcharset = locale_charset();\n+#else\n \tcharset = nl_langinfo(CODESET);\n+#endif\n \t(void)bind_textdomain_codeset(\"git\", charset);\n \t(void)setlocale(LC_CTYPE, \"C\");\n \t(void)textdomain(\"git\");\n-- \n1.7.3.159.g610493\n"},{"id":"152056","messageId":"1285767814-25857-1-git-send-email-avarab@gmail.com","threadId":"25274","inReplyTo":"1285767632-24852-1-git-send-email-avarab@gmail.com","subject":"[PATCH v3] gettext: use libcharset when available","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-29T13:43:34Z","receivedAt":"2010-09-29T13:43:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Erik Faye-Lund <kusmabite@gmail.com>\n\nChange the git_setup_gettext function to use libcharset to query the\ncharacter set of the current locale if it's available. I.e. use it\ninstead of nl_langinfo if HAVE_LIBCHARSET_H is set.\n\nThe GNU gettext manual recommends using langinfo.h's\nnl_langinfo(CODESET) to acquire the current character set, but on\nsystems that have libcharset.h's locale_charset() using the latter is\neither saner, or the only option on those systems.\n\nGNU and Solaris have a nl_langinfo(CODESET), FreeBSD can use either,\nbut MinGW and some others need to use libcharset.h's locale_charset()\ninstead.\n\nSince locale_charset returns a const char* instead of char* as\nnl_langinfo does the type of the variable we're using to store the\ncharset in git_setup_gettext has been changed.\n\nSigned-off-by: Erik Faye-Lund <kusmabite@gmail.com>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nOn Wed, Sep 29, 2010 at 13:40, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> From: Erik Faye-Lund <kusmabite@gmail.com>\n> On Wed, Sep 29, 2010 at 13:34, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n>> Very minor nit: It's officially spelled MinGW, with an upper-case G.\n>\n> Did s/MingW/MinGW/ in the content & message in this v2.\n\nDiscard that one, I accidentally changed some unrelated mention of\nMingW to MinGW.\n\n Makefile      |   17 +++++++++++++++++\n config.mak.in |    1 +\n configure.ac  |    6 ++++++\n gettext.c     |   10 +++++++++-\n 4 files changed, 33 insertions(+), 1 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 680e578..49a3386 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -43,6 +43,12 @@ all::\n # on platforms where we don't expect glibc (Linux, Hurd,\n # GNU/kFreeBSD), which includes libintl.\n #\n+# Define HAVE_LIBCHARSET_H if you haven't set NO_GETTEXT and you can't\n+# trust the langinfo.h's nl_langinfo(CODESET) function to return the\n+# current character set. GNU and Solaris have a nl_langinfo(CODESET),\n+# FreeBSD can use either, but MinGW and some others need to use\n+# libcharset.h's locale_charset() instead.\n+#\n # Define GNU_GETTEXT if you're using the GNU implementation of\n # libintl. We define this everywhere except on Solaris, which has its\n # own gettext implementation. If GNU_GETTEXT is set we'll use GNU\n@@ -792,6 +798,10 @@ ifndef NO_GETTEXT\n \t# Systems that don't use GNU gettext are the exception. Only\n \t# Solaris has a mature non-GNU gettext implementation.\n \tGNU_GETTEXT = YesPlease\n+\n+\t# Since we assume a GNU gettext by default we also assume a\n+\t# GNU-like langinfo.h by default\n+\tHAVE_LIBCHARSET_H =\n endif\n \n # We choose to avoid \"if .. else if .. else .. endif endif\"\n@@ -1180,6 +1190,9 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT))\n \tEXTLIBS += /mingw/lib/libz.a\n \tNO_R_TO_GCC_LINKER = YesPlease\n \tINTERNAL_QSORT = YesPlease\n+ifndef NO_GETTEXT\n+\tHAVE_LIBCHARSET_H = YesPlease\n+endif\n else\n \tNO_CURL = YesPlease\n endif\n@@ -1964,6 +1977,10 @@ config.s config.o: EXTRA_CPPFLAGS = -DETC_GITCONFIG='\"$(ETC_GITCONFIG_SQ)\"'\n \n http.s http.o: EXTRA_CPPFLAGS = -DGIT_HTTP_USER_AGENT='\"git/$(GIT_VERSION)\"'\n \n+ifdef HAVE_LIBCHARSET_H\n+gettext.s gettext.o: EXTRA_CPPFLAGS = -DHAVE_LIBCHARSET_H\n+endif\n+\n ifdef NO_EXPAT\n http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT\n endif\ndiff --git a/config.mak.in b/config.mak.in\nindex 9f47aa5..969cbaa 100644\n--- a/config.mak.in\n+++ b/config.mak.in\n@@ -34,6 +34,7 @@ NO_CURL=@NO_CURL@\n NO_EXPAT=@NO_EXPAT@\n NO_LIBGEN_H=@NO_LIBGEN_H@\n HAVE_PATHS_H=@HAVE_PATHS_H@\n+HAVE_LIBCHARSET_H=@HAVE_LIBCHARSET_H@\n NO_GETTEXT=@NO_GETTEXT@\n NEEDS_LIBICONV=@NEEDS_LIBICONV@\n NEEDS_SOCKET=@NEEDS_SOCKET@\ndiff --git a/configure.ac b/configure.ac\nindex 1821d89..b06bad1 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -810,6 +810,12 @@ AC_CHECK_HEADER([libintl.h],\n [NO_GETTEXT=YesPlease])\n AC_SUBST(NO_GETTEXT)\n #\n+# Define HAVE_LIBCHARSET_H if have libcharset.h\n+AC_CHECK_HEADER([libcharset.h],\n+[HAVE_LIBCHARSET_H=YesPlease],\n+[HAVE_LIBCHARSET_H=])\n+AC_SUBST(HAVE_LIBCHARSET_H)\n+#\n # Define NO_STRCASESTR if you don't have strcasestr.\n GIT_CHECK_FUNC(strcasestr,\n [NO_STRCASESTR=],\ndiff --git a/gettext.c b/gettext.c\nindex 8644098..9bdac56 100644\n--- a/gettext.c\n+++ b/gettext.c\n@@ -1,13 +1,17 @@\n #include \"exec_cmd.h\"\n #include <locale.h>\n #include <libintl.h>\n+#ifdef HAVE_LIBCHARSET_H\n+#include <libcharset.h>\n+#else\n #include <langinfo.h>\n+#endif\n #include <stdlib.h>\n \n extern void git_setup_gettext(void) {\n \tchar *podir;\n \tchar *envdir = getenv(\"GIT_TEXTDOMAINDIR\");\n-\tchar *charset;\n+\tconst char *charset;\n \n \tif (envdir) {\n \t\t(void)bindtextdomain(\"git\", envdir);\n@@ -20,7 +24,11 @@ extern void git_setup_gettext(void) {\n \n \t(void)setlocale(LC_MESSAGES, \"\");\n \t(void)setlocale(LC_CTYPE, \"\");\n+#ifdef HAVE_LIBCHARSET_H\n+\tcharset = locale_charset();\n+#else\n \tcharset = nl_langinfo(CODESET);\n+#endif\n \t(void)bind_textdomain_codeset(\"git\", charset);\n \t(void)setlocale(LC_CTYPE, \"C\");\n \t(void)textdomain(\"git\");\n-- \n1.7.3.159.g610493\n"}]}