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

From: Vadym Kochan <vadim4j@gmail.com>
Date: Apr 18, 2019, 08:28 UTC
Message-ID: <20190418082844.GA10068@lwo1-lhp-f71841>
In-Reply-To: <xmqqftqfhnmz.fsf@gitster-ct.c.googlers.com>

Hi Junio,

On Thu, Apr 18, 2019 at 02:45:56PM +0900, Junio C Hamano wrote:

Show 45 quoted lines

> 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.

Yes you are correct. 'gettext' even might be defined as libintl_gettext. When I got build error I checked config.log and I saw that gcc claims that gettext builtin is used, so I think that was the reason why test passed for non-glibc. So, if to use <libintl.h> then gettext might be preprocessed to libintl_gettext in the test. The original error which I was fixing is:

	http://autobuild.buildroot.net/results/8eeac7f7ddd97576eaeb87311bf0988d59d8b132/build-end.log

Show 34 quoted lines

> 
> 
> > 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])
> >  
> >  #