Re: [PATCH 1/1] configure.ac: Properly check for libintl

From: Junio C Hamano <gitster@pobox.com>
To: Vadim Kochan <vadim4j@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/1] configure.ac: Properly check for libintl
Date: Thu, 18 Apr 2019 14:45:56 +0900	[thread overview]
Message-ID: <xmqqftqfhnmz.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20190418050419.21114-1-vadim4j@gmail.com> (Vadim Kochan's message of "Thu, 18 Apr 2019 08:04:19 +0300")

Vadim Kochan <vadim4j@gmail.com> writes:

> Some libc implementations like uclibc or musl provides
> gettext stubs via libintl library but this case is not checked
> by AC_CHECK_LIB(c, gettext ...) because gcc has gettext as builtin
> which passess the check.
>
> So check it with included libintl.h where gettext may unfold into
> libintl_gettext which will cause check to fail if libintl_gettext are
> needed to be linked with -lintl.

Let me make sure I can understand the above correctly (otherwise,
that is a sign that the proposed log message is lacking) by trying
to rephrase (iow, this is not me saying "your log message should be
rewritten like so"; it is "if I read what you wrote above correctly,
I think what I am going to write here is also correct"):

    Some libc implementations have function called gettext() that
    can be linked via -lc without -lintl, but these are mere stubs
    and do not do useful i18n.  On these systems, if a program that
    calls gettext() is built _with_ "#include <libintl.h>", the
    linker calls for the real version (e.g. libintl_gettext()) and
    that can be satisfied only by linking with -lintl.

    The current check to see if -lc provides with gettext() is
    sufficient for libc implementations like GNU libc that actually
    has full fledged gettext(); to detect libc with stub gettext()
    and libintl with real gettext(), aliased via <libintl.h>, the
    check to see if -lintl is necessary must be done with a sample
    source that #include's the header file.

Is that what is going on and why this patch is needed?

I think the only possibile kind of system this change could break
that currently is working is the one that has a usable gettext()
in -lc, but does not offer <libintl.h>, as the new test program
added by this patch will fail to compile, but I do not think that
is possible in practice---our own gettext.c #include's <libintl.h>
so there is no way such a hypothetical system that would be broken
by this change could possibly have built Git successfully.

Assuming that the way I read your log message is in line with what
you wanted to say, I think the patch looks good.

Thanks.

> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> ---
>  configure.ac | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index e0d0da3c0c..be3b55f1cc 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -763,9 +763,19 @@ AC_CHECK_LIB([c], [basename],
>  GIT_CONF_SUBST([NEEDS_LIBGEN])
>  test -n "$NEEDS_LIBGEN" && LIBS="$LIBS -lgen"
>  
> -AC_CHECK_LIB([c], [gettext],
> -[LIBC_CONTAINS_LIBINTL=YesPlease],
> -[LIBC_CONTAINS_LIBINTL=])
> +AC_DEFUN([LIBINTL_SRC], [
> +AC_LANG_PROGRAM([[
> +#include <libintl.h>
> +]],[[
> +char *msg = gettext("test");
> +]])])
> +
> +AC_MSG_CHECKING([if libc contains libintl])
> +AC_LINK_IFELSE([LIBINTL_SRC],
> +	[AC_MSG_RESULT([yes])
> +	LIBC_CONTAINS_LIBINTL=YesPlease],
> +	[AC_MSG_RESULT([no])
> +	LIBC_CONTAINS_LIBINTL=])
>  GIT_CONF_SUBST([LIBC_CONTAINS_LIBINTL])
>  
>  #