Re: [PATCH] lib/gettext.h: fix warning if gettext is already present

From: Giulio Benetti <giulio.benetti@benettiengineering.com>
To: Bruno Haible <bruno@clisp.org>
Cc: bug-gnulib@gnu.org
Subject: Re: [PATCH] lib/gettext.h: fix warning if gettext is already present
Date: Mon, 3 Feb 2020 00:15:35 +0100	[thread overview]
Message-ID: <999942d4-69af-6c17-6dc3-d2d7732f0162@benettiengineering.com> (raw)
In-Reply-To: <7353597.sKBQ19CNnG@omega>

Hi Bruno,

On 1/27/20 8:29 PM, Bruno Haible wrote:
> Giulio Benetti wrote:
>>> There should not be a <gettext.h> in public include file directories!
>>
>> Indeed there is not, there is <libintl.h> and it is part of uclibc:
>> https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/include/libintl.h
> 
> Ah! Most of the contents of this <libintl.h> is OK. But
>    - gettext_noop
>    - gettext_printf
> don't belong there.

I see that usually glibc #define gettext_noop() in regex_internal.h that 
in order #include libintl.h[1] so, the same usage as gnulib does.

>> What I was proposing at the beginning was to add:
>> #undef gettext_noop
>>
>> the same way uClibc does.
> 
> The compiler warning about a redefined macros is meant to catch unintentional
> overriding definitions. For example, when a program defines gettext_noop
> and afterwards #includes "gettext.h". It is reasonable to keep this warning
> in the general case.

Yes, but we get problems when somebody passes -Werror and this can occur.

> Therefore the right action is that the uClibc people remove their definition
> of gettext_noop and gettext_printf from this file.

uClibc-ng embeds gettext-tiny which has that libintl.h flavour where 
they define gettext_noop() and gettext_printf() [2]. But their purpose 
is exaclty to provide a stub for gettext. So I end up with the 
conclusion that libbytesize has the problem, none of you all(gnulib, 
uClibc-ng, glibc). Basically the problem is that they define 
-DENABLE_NLS in any case without taking into account if the build system 
provide or not a gettext lib(gettext, gettext-tiny), so they enable 
gnulib's gettext.h by default then we get to double #define.

The only fix to do is in libbytesize/configure.ac: append -DENABLE_NLS 
to CFLAGS only if gettext is not found in build system.

In the meanwhile I've added the workaround I've pointed here with this 
patch, but this is not the solution.

Writing this e-mail everything got clear to me :-) so sorry for the 
noise, anyway it was also to give you an answer.

[1]:https://sourceware.org/git/?p=glibc.git;a=blob;f=posix/regex_internal.h;h=6f761c6ed66940b5b3670060200eeb8e42641559;hb=HEAD#l90
[2]:https://github.com/sabotage-linux/gettext-tiny/blob/master/include/libintl.h#L34

Kind regards
-- 
Giulio Benetti
Benetti Engineering sas