[Buildroot] [PATCH] package/lshw: fix the build with glibc without NLS

[Buildroot] [PATCH] package/lshw: fix the build with glibc without NLS

[Romain Naour]

Since [1], NONLS is added to LSHW_CFLAGS even for glibc based toolchain.
Previously it was not the case since BR2_ENABLE_LOCALE is always
selected for glibc based toolchain.

With NONLS, config.h try to redefine two Intl function (textdomain and
bindtextdomain) that are used by glibc internally.

This break the build with the following error:
sysroot/usr/include/libintl.h:82:52: error: expected unqualified-id before 'throw'
 extern char *textdomain (const char *__domainname) __THROW;

Fixes:
http://autobuild.buildroot.net/results/9bf/9bf5437b4348ea8077013b80a51ce05fa328247d

[1] 3acd9f845de0489071719d52d774aab48d7f9c33

Signed-off-by: Romain Naour <romain.naour@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Gustavo Zacarias <gustavo@zacarias.com.ar>
---
 ...nt-t-try-to-redefine-intl-functions-when-.patch | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 package/lshw/0004-config.h-dont-t-try-to-redefine-intl-functions-when-.patch

diff --git a/package/lshw/0004-config.h-dont-t-try-to-redefine-intl-functions-when-.patch b/package/lshw/0004-config.h-dont-t-try-to-redefine-intl-functions-when-.patch
new file mode 100644
index 0000000..5618d49
--- /dev/null
+++ b/package/lshw/0004-config.h-dont-t-try-to-redefine-intl-functions-when-.patch
@@ -0,0 +1,38 @@
+From e7550f317ac88957a2127cc1bedb7c5b4dd918bd Mon Sep 17 00:00:00 2001
+From: Romain Naour <romain.naour@gmail.com>
+Date: Thu, 13 Jul 2017 22:44:20 +0200
+Subject: [PATCH] config.h: dont't try to redefine intl functions when NONLS is
+ set
+
+When NONLS is set and a glibc based toolchain is used to build lshw,
+config.h try to redefine two Intl function (textdomain and
+bindtextdomain) that are used by glibc internally.
+
+This break the build with the following error:
+sysroot/usr/include/libintl.h:82:52: error: expected unqualified-id before 'throw'
+ extern char *textdomain (const char *__domainname) __THROW;
+
+Fixes:
+http://autobuild.buildroot.net/results/9bf/9bf5437b4348ea8077013b80a51ce05fa328247d
+
+Signed-off-by: Romain Naour <romain.naour@gmail.com>
+---
+ src/core/config.h | 2 --
+ 1 file changed, 2 deletions(-)
+
+diff --git a/src/core/config.h b/src/core/config.h
+index 69023fd..e7bdc18 100644
+--- a/src/core/config.h
++++ b/src/core/config.h
+@@ -4,8 +4,6 @@
+ #ifdef NONLS
+ #define _(String) (String)
+ #define N_(String) String
+-#define textdomain(Domain)
+-#define bindtextdomain(Package, Directory)
+ #else
+ #include <libintl.h>
+ #define _(String) gettext (String)
+-- 
+2.9.4
+
-- 
2.9.4

[Buildroot] [PATCH] package/lshw: fix the build with glibc without NLS

[Thomas Petazzoni]

Hello,

On Thu, 13 Jul 2017 23:04:24 +0200, Romain Naour wrote:
> Since [1], NONLS is added to LSHW_CFLAGS even for glibc based toolchain.
> Previously it was not the case since BR2_ENABLE_LOCALE is always
> selected for glibc based toolchain.
> 
> With NONLS, config.h try to redefine two Intl function (textdomain and
> bindtextdomain) that are used by glibc internally.
> 
> This break the build with the following error:
> sysroot/usr/include/libintl.h:82:52: error: expected unqualified-id before 'throw'
>  extern char *textdomain (const char *__domainname) __THROW;
> 
> Fixes:
> http://autobuild.buildroot.net/results/9bf/9bf5437b4348ea8077013b80a51ce05fa328247d
> 
> [1] 3acd9f845de0489071719d52d774aab48d7f9c33
> 
> Signed-off-by: Romain Naour <romain.naour@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Gustavo Zacarias <gustavo@zacarias.com.ar>

I also had a look at this issue, and I'm not sure this is the most
appropriate fix.

Indeed, the NONLS mode is really for cases where the C library does not
provide *any* gettext implementation, as would be the case with uClibc
without intl stubs enabled.

But in the context of Buildroot, all C libraries provide a gettext
implementation. It might be full-featured or minimal, but it always
exists.

Therefore, I believe the appropriate fix is to in fact never pass
-DNONLS, because we are never in a situation where we don't have any
NLS support at all. So I believe the right fix is to remove the
following three lines:

ifeq ($(BR2_SYSTEM_ENABLE_NLS),)
LSHW_CFLAGS += -DNONLS
endif

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH] package/lshw: fix the build with glibc without NLS

[Romain Naour]

Hi Thomas,

Le 13/07/2017 ? 23:28, Thomas Petazzoni a ?crit :
> Hello,
> 
> On Thu, 13 Jul 2017 23:04:24 +0200, Romain Naour wrote:
>> Since [1], NONLS is added to LSHW_CFLAGS even for glibc based toolchain.
>> Previously it was not the case since BR2_ENABLE_LOCALE is always
>> selected for glibc based toolchain.
>>
>> With NONLS, config.h try to redefine two Intl function (textdomain and
>> bindtextdomain) that are used by glibc internally.
>>
>> This break the build with the following error:
>> sysroot/usr/include/libintl.h:82:52: error: expected unqualified-id before 'throw'
>>  extern char *textdomain (const char *__domainname) __THROW;
>>
>> Fixes:
>> http://autobuild.buildroot.net/results/9bf/9bf5437b4348ea8077013b80a51ce05fa328247d
>>
>> [1] 3acd9f845de0489071719d52d774aab48d7f9c33
>>
>> Signed-off-by: Romain Naour <romain.naour@gmail.com>
>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Cc: Gustavo Zacarias <gustavo@zacarias.com.ar>
> 
> I also had a look at this issue, and I'm not sure this is the most
> appropriate fix.
> 
> Indeed, the NONLS mode is really for cases where the C library does not
> provide *any* gettext implementation, as would be the case with uClibc
> without intl stubs enabled.
> 
> But in the context of Buildroot, all C libraries provide a gettext
> implementation. It might be full-featured or minimal, but it always
> exists.
> 
> Therefore, I believe the appropriate fix is to in fact never pass
> -DNONLS, because we are never in a situation where we don't have any
> NLS support at all. So I believe the right fix is to remove the
> following three lines:
> 
> ifeq ($(BR2_SYSTEM_ENABLE_NLS),)
> LSHW_CFLAGS += -DNONLS
> endif

Indeed, that's the first thing I removed during investigation.

I find misleading that BR2_SYSTEM_ENABLE_NLS not set does not mean NONLS for lshw.

I'm wondering if lshw build fine with a toolchain with wchar but without locale
(ok it's a corner case).

See src/lshw.cc:#ifndef NONLS

#ifndef NONLS
#include <locale.h>
#endif

...

#ifndef NONLS
  setlocale (LC_ALL, "");
  ...
#endif

Best regards,
Romain

> 
> Best regards,
> 
> Thomas
> 

[Buildroot] [PATCH] package/lshw: fix the build with glibc without NLS

[Thomas Petazzoni]

Hello,

On Fri, 14 Jul 2017 00:10:39 +0200, Romain Naour wrote:

> > ifeq ($(BR2_SYSTEM_ENABLE_NLS),)
> > LSHW_CFLAGS += -DNONLS
> > endif  
> 
> Indeed, that's the first thing I removed during investigation.
> 
> I find misleading that BR2_SYSTEM_ENABLE_NLS not set does not mean NONLS for lshw.

I think that's OK. For lshw, NONLS really means "there is absolutely no
gettext functions provided by the C library", which is never the case
in the context of Buildroot.

Can you send an updated patch that simply removes the -DNONLS ?

> I'm wondering if lshw build fine with a toolchain with wchar but without locale
> (ok it's a corner case).
> 
> See src/lshw.cc:#ifndef NONLS
> 
> #ifndef NONLS
> #include <locale.h>
> #endif
> 
> ...
> 
> #ifndef NONLS
>   setlocale (LC_ALL, "");

Not a problem I believe. From libc/misc/locale/Makefile.in in uClibc-ng:

# multi source locale.c
CSRC-y := setlocale.c localeconv.c _locale_init.c nl_langinfo.c
CSRC-$(UCLIBC_HAS_LOCALE) += newlocale.c __locale_mbrtowc_l.c
CSRC-$(UCLIBC_HAS_XLOCALE) += nl_langinfo_l.c duplocale.c freelocale.c \
        uselocale.c __curlocale.c

Hence, it seems like a uClibc-ng built with UCLIBC_HAS_LOCALE disabled
will still have a setlocale() function.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com