[PATCH] Fix use of ENABLE_NLS (which is not always defined)

[PATCH] Fix use of ENABLE_NLS (which is not always defined)

[Grégoire Sutre]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

Hi,

This message concerns both gnulib and grub.  As discussed on irc and on 
the list [1], ENABLE_NLS is not used correctly, which leads to a build 
failure when gettext is not detected (or with configure option 
--disable-nls).

ENABLE_NLS is defined in AM_GNU_GETTEXT and the documentation of this 
macro [2] does not require ENABLE_NLS to be defined when gettext is not 
available.  However, uses of ENABLE_NLS assume it to be defined, which 
is not correct.

The attached patch fixes the problem for grub.

For gnulib, running the following command on the source tree should 
solve the problem:

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

Best regards,

Grégoire

[1] http://lists.gnu.org/archive/html/grub-devel/2010-01/msg00288.html
[2] 
http://www.gnu.org/software/hello/manual/gettext/AM_005fGNU_005fGETTEXT.html 


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-enable-nls.diff --]
[-- Type: text/x-patch; name="patch-enable-nls.diff", Size: 3965 bytes --]

=== modified file 'ChangeLog'
--- ChangeLog	2010-01-25 09:06:55 +0000
+++ ChangeLog	2010-01-25 09:24:09 +0000
@@ -1,5 +1,18 @@
 2010-01-25  Grégoire Sutre  <gregoire.sutre@gmail.com>
 
+	Fix a build failure (-Wundef -Werror) when ENABLE_NLS is not defined,
+	which is the case with --disabled-nls.
+
+	* gnulib/error.c: use (defined(ENABLE_NLS) && ENABLE_NLS) instead of
+	ENABLE_NLS in all #if preprocessor macros.
+	* gnulib/gettext.h: likewise.
+	* include/grub/i18n.h: likewise.
+	* util/misc.c: likewise.
+	* util/mkisofs/mkisofs.c: likewise.
+	* util/mkisofs/mkisofs.h: likewise.
+
+2010-01-25  Grégoire Sutre  <gregoire.sutre@gmail.com>
+
 	* configure.ac: Check for `limits.h'.
 	* util/misc.c: Include `<limits.h>' (for PATH_MAX).
 

=== modified file 'gnulib/error.c'
--- gnulib/error.c	2009-11-16 18:49:44 +0000
+++ gnulib/error.c	2010-01-25 09:22:09 +0000
@@ -28,7 +28,7 @@
 #include <stdlib.h>
 #include <string.h>
 
-#if !_LIBC && ENABLE_NLS
+#if !_LIBC && (defined(ENABLE_NLS) && ENABLE_NLS)
 # include "gettext.h"
 # define _(msgid) gettext (msgid)
 #endif

=== modified file 'gnulib/gettext.h'
--- gnulib/gettext.h	2009-11-09 19:16:09 +0000
+++ gnulib/gettext.h	2010-01-25 09:22:09 +0000
@@ -19,7 +19,7 @@
 #define _LIBGETTEXT_H 1
 
 /* NLS can be disabled through the configure --disable-nls option.  */
-#if ENABLE_NLS
+#if (defined(ENABLE_NLS) && ENABLE_NLS)
 
 /* Get declarations of GNU message catalog functions.  */
 # include <libintl.h>

=== modified file 'include/grub/i18n.h'
--- include/grub/i18n.h	2010-01-01 20:32:30 +0000
+++ include/grub/i18n.h	2010-01-25 09:22:08 +0000
@@ -26,7 +26,7 @@
 extern const char *(*EXPORT_VAR(grub_gettext)) (const char *s);
 
 /* NLS can be disabled through the configure --disable-nls option.  */
-#if ENABLE_NLS
+#if (defined(ENABLE_NLS) && ENABLE_NLS)
 
 # ifdef GRUB_UTIL
 
@@ -35,7 +35,7 @@
 
 # endif /* GRUB_UTIL */
 
-#else /* ! ENABLE_NLS */
+#else /* ! (defined(ENABLE_NLS) && ENABLE_NLS) */
 
 /* Disabled NLS.
    The casts to 'const char *' serve the purpose of producing warnings
@@ -48,7 +48,7 @@
 #  define grub_gettext(str) ((const char *) (str))
 # endif /* GRUB_UTIL */
 
-#endif /* ENABLE_NLS */
+#endif /* (defined(ENABLE_NLS) && ENABLE_NLS) */
 
 #ifdef GRUB_UTIL
 # define _(str) gettext(str)

=== modified file 'util/misc.c'
--- util/misc.c	2010-01-25 09:06:55 +0000
+++ util/misc.c	2010-01-25 09:22:07 +0000
@@ -598,9 +598,9 @@
 void
 grub_util_init_nls (void)
 {
-#if ENABLE_NLS
+#if (defined(ENABLE_NLS) && ENABLE_NLS)
   setlocale (LC_ALL, "");
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);
-#endif /* ENABLE_NLS */
+#endif /* (defined(ENABLE_NLS) && ENABLE_NLS) */
 }

=== modified file 'util/mkisofs/mkisofs.c'
--- util/mkisofs/mkisofs.c	2010-01-08 15:22:40 +0000
+++ util/mkisofs/mkisofs.c	2010-01-25 09:22:06 +0000
@@ -640,11 +640,11 @@
   char *log_file = 0;
 
   set_program_name (argv[0]);
-#if ENABLE_NLS
+#if (defined(ENABLE_NLS) && ENABLE_NLS)
   setlocale (LC_ALL, "");
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);
-#endif /* ENABLE_NLS */
+#endif /* (defined(ENABLE_NLS) && ENABLE_NLS) */
 
   if (argc < 2)
     usage();

=== modified file 'util/mkisofs/mkisofs.h'
--- util/mkisofs/mkisofs.h	2010-01-01 20:32:30 +0000
+++ util/mkisofs/mkisofs.h	2010-01-25 09:22:06 +0000
@@ -30,12 +30,12 @@
 #include <prototyp.h>
 #include <sys/stat.h>
 
-#if ENABLE_NLS
+#if (defined(ENABLE_NLS) && ENABLE_NLS)
 
 #  include <locale.h>
 #  include <libintl.h>
 
-#else /* ! ENABLE_NLS */
+#else /* ! (defined(ENABLE_NLS) && ENABLE_NLS) */
 
 /* Disabled NLS.
    The casts to 'const char *' serve the purpose of producing warnings
@@ -43,7 +43,7 @@
    On pre-ANSI systems without 'const', the config.h file is supposed to
    contain "#define const".  */
 #  define gettext(Msgid) ((const char *) (Msgid))
-#endif /* ENABLE_NLS */
+#endif /* (defined(ENABLE_NLS) && ENABLE_NLS) */
 
 #define _(str) gettext(str)
 #define N_(str) str

Re: [PATCH] Fix use of ENABLE_NLS (which is not always defined)

[Yves Blusseau]

[-- Attachment #1: Type: text/plain, Size: 349 bytes --]

Le 25/01/2010 10:53, Grégoire Sutre a écrit :
> ENABLE_NLS is defined in AM_GNU_GETTEXT and the documentation of this 
> macro [2] does not require ENABLE_NLS to be defined when gettext is 
> not available.  However, uses of ENABLE_NLS assume it to be defined, 
> which is not correct.

Seems good for me, any objections ?

Yves Blusseau


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3326 bytes --]

Re: [PATCH] Fix use of ENABLE_NLS (which is not always defined)

[Grégoire Sutre]

Hi,

I just discovered your answers [1, 2] on this subject (I'm not
registered to the bug-gnulib mailing list).  Thanks for the
explanations.  I take note that Gnulib does not support use of -Wundef.

I just want to add that I find it good practice to avoid evaluating
undefined identifiers in #if directives.

> In fact, any package which makes good use of Autoconf cannot support 
> -Wundef.

I don't see why.  Could you please elaborate?

Best regards,

Grégoire Sutre

[1] http://lists.gnu.org/archive/html/bug-gnulib/2010-01/msg00316.html
[2] http://lists.gnu.org/archive/html/bug-gnulib/2010-01/msg00319.html