Re: [PATCH] gettext: use libcharset when available

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Tue, Sep 28, 2010 at 16:05, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> libcharset provides an even more portable way of quering the charset
> of the current locale.
>
> Use that instead of nl_langinfo unless NO_LIBCHARSET is set.
>
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> ---
>
> Windows doesn't have langinfo.h and nl_langinfo(), but libcharset was
> invented for this very purpose. With this patch on top, ab/i18n
> compiles without errors in msysGit.
>
> There's still a bunch of lower-level issues on Windows, like gettext
> ending up overloading our winansi-wrappings for printf and friends,
> but let's take thinks one step at the time :)
>
>  configure.ac |    6 ++++++
>  gettext.c    |   10 +++++++++-
>  2 files changed, 15 insertions(+), 1 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 1821d89..d3139cd 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -810,6 +810,12 @@ AC_CHECK_HEADER([libintl.h],
>  [NO_GETTEXT=YesPlease])
>  AC_SUBST(NO_GETTEXT)
>  #
> +# Define NO_LIBCHARSET if you don't have libcharset.h
> +AC_CHECK_HEADER([libcharset.h],
> +[NO_LIBCHARSET=],
> +[NO_LIBCHARSET=YesPlease])
> +AC_SUBST(NO_LIBCHARSET)
> +#
>  # Define NO_STRCASESTR if you don't have strcasestr.
>  GIT_CHECK_FUNC(strcasestr,
>  [NO_STRCASESTR=],
> diff --git a/gettext.c b/gettext.c
> index 8644098..902268c 100644
> --- a/gettext.c
> +++ b/gettext.c
> @@ -1,13 +1,17 @@
>  #include "exec_cmd.h"
>  #include <locale.h>
>  #include <libintl.h>
> +#ifndef NO_LIBCHARSET
> +#include <libcharset.h>
> +#else
>  #include <langinfo.h>
> +#endif
>  #include <stdlib.h>
>
>  extern void git_setup_gettext(void) {
>        char *podir;
>        char *envdir = getenv("GIT_TEXTDOMAINDIR");
> -       char *charset;
> +       const char *charset;
>
>        if (envdir) {
>                (void)bindtextdomain("git", envdir);
> @@ -20,7 +24,11 @@ extern void git_setup_gettext(void) {
>
>        (void)setlocale(LC_MESSAGES, "");
>        (void)setlocale(LC_CTYPE, "");
> +#ifndef NO_LIBCHARSET
> +       charset = locale_charset();
> +#else
>        charset = nl_langinfo(CODESET);
> +#endif
>        (void)bind_textdomain_codeset("git", charset);
>        (void)setlocale(LC_CTYPE, "C");
>        (void)textdomain("git");

Thanks for porting it to Windows. Some points:

 * Nit: Should be NEEDS_LIBCHARSET instead of NO_LIBCHARSET, all the
   variables that set library inclusions in the Makefile use the
   NEED_* names.

 * GHC had a patch like this, it seems it affects NetBSD and OpenBSD
   too, can anyone with these systems confirm:
   http://hackage.haskell.org/trac/ghc/ticket/4080

 * Their patch compiles a program that includes libcharset.h and
   compiles "const char* charset = locale_charset();". I don't know if
   this is needed, or whether just checking the header name like
   you've done will do.

 * They also have a HAVE_LANGINFO_H define and fall back on just
   returning "", which works on GNU iconv. Maybe we should do this
   too?

I'm not sure about any of this, since I've just been testing on
Solaris, Linux and FreeBSD.