Build error: "ENABLE_NLS" is not defined

Build error: "ENABLE_NLS" is not defined

[Grégoire Sutre]

Hi,

When NLS is disabled or not supported, the macro ENABLE_NLS is not 
defined, and this breaks compilation of GRUB modules as they are 
compiled with -Werror -Wundef by default.  This is with bazaar trunk.

$ ./autogen.sh && ./configure && gmake
[...]
gcc -Ikern -I./kern -nostdinc -isystem /usr/include -I./include -I. 
-I./include -Wall -W  -Os -DGRUB_MACHINE_PCBIOS=1 -Wall -W -Wshadow 
-Wpointer-arith -Wmissing-prototypes                -Wundef 
-Wstrict-prototypes -g -falign-jumps=1 -falign-loops=1 
-falign-functions=1 -mno-mmx -mno-sse -mno-sse2 -mno-3dnow -m32 
-fno-stack-protector -mno-stack-arg-probe -Werror -fno-builtin -mrtd 
-mregparm=3 -m32   -MD -c -o kernel_img-kern_err.o kern/err.c
In file included from kern/err.c:23:
./include/grub/i18n.h:29:5: error: "ENABLE_NLS" is not defined
gmake: *** [kernel_img-kern_err.o] Error 1

A possible fix consists in replacing the #if ENABLE_NLS by #if 
defined(ENABLE_NLS).  There are not many instances, so they can be 
changed manually, but the following command did the trick for me 
(assuming sed is GNU sed):

find . -name '*.[ch]' -exec sed -i -e 's, ENABLE_NLS, 
defined(ENABLE_NLS),g' '{}' ';'

Best,

Grégoire

Re: Build error: "ENABLE_NLS" is not defined

[richardvoigt]

2010/1/12 Grégoire Sutre <gregoire.sutre@gmail.com>:
> Hi,
>
> When NLS is disabled or not supported, the macro ENABLE_NLS is not defined,
> and this breaks compilation of GRUB modules as they are compiled with
> -Werror -Wundef by default.  This is with bazaar trunk.
>
> $ ./autogen.sh && ./configure && gmake
> [...]
> gcc -Ikern -I./kern -nostdinc -isystem /usr/include -I./include -I.
> -I./include -Wall -W  -Os -DGRUB_MACHINE_PCBIOS=1 -Wall -W -Wshadow
> -Wpointer-arith -Wmissing-prototypes                -Wundef
> -Wstrict-prototypes -g -falign-jumps=1 -falign-loops=1 -falign-functions=1
> -mno-mmx -mno-sse -mno-sse2 -mno-3dnow -m32 -fno-stack-protector
> -mno-stack-arg-probe -Werror -fno-builtin -mrtd -mregparm=3 -m32   -MD -c -o
> kernel_img-kern_err.o kern/err.c
> In file included from kern/err.c:23:
> ./include/grub/i18n.h:29:5: error: "ENABLE_NLS" is not defined
> gmake: *** [kernel_img-kern_err.o] Error 1
>
> A possible fix consists in replacing the #if ENABLE_NLS by #if
> defined(ENABLE_NLS).  There are not many instances, so they can be changed
> manually, but the following command did the trick for me (assuming sed is
> GNU sed):
>
> find . -name '*.[ch]' -exec sed -i -e 's, ENABLE_NLS, defined(ENABLE_NLS),g'
> '{}' ';'

This fix breaks when ENABLE_NLS is defined as 0.

This gcc bug is blocking a better solution:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38105

>
> Best,
>
> Grégoire
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>

Re: Build error: "ENABLE_NLS" is not defined

[Grégoire Sutre]

richardvoigt@gmail.com wrote:

> This fix breaks when ENABLE_NLS is defined as 0.

AFAICS, in the implementation of AM_GNU_GETTEXT, ENABLE_NLS is either 
not defined or set to 1.

But I agree that my proposed fix is too dependent on the implementation 
of AM_GNU_GETTEXT, which may change.

Another option would be to replace #if ENABLE_NLS by #if 
defined(ENABLE_NLS) && ENABLE_NLS.

> This gcc bug is blocking a better solution:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38105

I didn't know that this was a gcc bug.  It felt like a normal behavior 
to me to get an error with both -Wundef and -Werror in that case.  From 
the gcc man page:

  -Werror
      Make all warnings into errors.

  -Wundef
      Warn if an undefined identifier is evaluated in an #if directive.


Grégoire

Re: Build error: "ENABLE_NLS" is not defined

[richardvoigt]

2010/1/12 Grégoire Sutre <gregoire.sutre@gmail.com>:
> richardvoigt@gmail.com wrote:
>
>> This fix breaks when ENABLE_NLS is defined as 0.
>
> AFAICS, in the implementation of AM_GNU_GETTEXT, ENABLE_NLS is either not
> defined or set to 1.
>
> But I agree that my proposed fix is too dependent on the implementation of
> AM_GNU_GETTEXT, which may change.
>
> Another option would be to replace #if ENABLE_NLS by #if defined(ENABLE_NLS)
> && ENABLE_NLS.

I know the C compiler short-circuits &&, if the preprocessor does also
then this looks like the best solution.  If not, then nested #if.

>
>> This gcc bug is blocking a better solution:
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38105
>
> I didn't know that this was a gcc bug.  It felt like a normal behavior to me
> to get an error with both -Wundef and -Werror in that case.  From the gcc
> man page:
>
>  -Werror
>     Make all warnings into errors.
>
>  -Wundef
>     Warn if an undefined identifier is evaluated in an #if directive.

And -Wno-error=undef is supposed to change the severity of that class
of warning back to non-fatal.  But it currently doesn't.

>
>
> Grégoire
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>

Re: Build error: "ENABLE_NLS" is not defined

[Grégoire Sutre]

richardvoigt@gmail.com wrote:

>> Another option would be to replace #if ENABLE_NLS by #if defined(ENABLE_NLS)
>> && ENABLE_NLS.
> 
> I know the C compiler short-circuits &&, if the preprocessor does also
> then this looks like the best solution.  If not, then nested #if.

Yes the preprocessor also short-circuits (I tested a small example to be 
sure, with gcc 4.1 and gcc 4.4).  So the automatic replacement command 
becomes:

find . -name '*.[ch]' -exec sed -i -e 's, ENABLE_NLS, 
(defined(ENABLE_NLS) \&\& ENABLE_NLS),g' '{}' ';'

Grégoire

p.s. By the way, this kind of contruct appears in the autoconf manual 
for a similar problem (middle of page):

http://www.gnu.org/software/autoconf/manual/html_node/Generic-Declarations.html#Generic-Declarations

Re: Build error: "ENABLE_NLS" is not defined

[Robert Millan]

On Wed, Jan 13, 2010 at 02:02:22AM +0100, Grégoire Sutre wrote:
> richardvoigt@gmail.com wrote:
>
>>> Another option would be to replace #if ENABLE_NLS by #if defined(ENABLE_NLS)
>>> && ENABLE_NLS.
>>
>> I know the C compiler short-circuits &&, if the preprocessor does also
>> then this looks like the best solution.  If not, then nested #if.
>
> Yes the preprocessor also short-circuits (I tested a small example to be  
> sure, with gcc 4.1 and gcc 4.4).  So the automatic replacement command  
> becomes:
>
> find . -name '*.[ch]' -exec sed -i -e 's, ENABLE_NLS,  
> (defined(ENABLE_NLS) \&\& ENABLE_NLS),g' '{}' ';'

This affects gnulib/error.c and gnulib/gettext.h which would be much better
not to change, as they're being imported semi-automatically.

Perhaps you could solve this at its source?  (i.e. by defining ENABLE_NLS to
0 when gettext is unavailable).

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi

Re: Build error: "ENABLE_NLS" is not defined

[Grégoire Sutre]

Robert Millan wrote:

> This affects gnulib/error.c and gnulib/gettext.h which would be much better
> not to change, as they're being imported semi-automatically.

I understand.

But could this be actually a bug in gnulib?  The problem only occurs 
when gettext is not found and when compiling with -Wundef -Werror, so 
maybe it went unnoticed?

> Perhaps you could solve this at its source?  (i.e. by defining ENABLE_NLS to
> 0 when gettext is unavailable).

Indeed, the problem is gone when the following line is added before the 
call to AM_GNU_GETTEXT in configure.ac:

AC_DEFINE([ENABLE_NLS], [0])

It does not really solve the problem at its source, though.  ENABLE_NLS 
is defined in AM_GNU_GETTEXT and the documentation of this macro [1] 
does not require ENABLE_NLS to be defined when gettext is not available.

Best regards,

Grégoire

[1] 
http://www.gnu.org/software/hello/manual/gettext/AM_005fGNU_005fGETTEXT.html