[Buildroot] [PATCH 1/1] git: Fix libintl linking if there is no full gettext support

[Buildroot] [PATCH 1/1] git: Fix libintl linking if there is no full gettext support

[Vadim Kochan]

From: Vadim Kochan <vadim.kochan@petcube.com>

Git could not resolve libntl_xxx symbols if toolchain does no provide
full gettext support but the gettext package is selected.

In case of ulibc/musl toolchain there is no full getttext support, but
they still provides libintl stub which makes git think there is full
gettext support hence -lintl is not passed to the linker.

Added workaround which checks if toolchain provides full gettext
support, if no then -lintl will be added to EXTLIBS variable generated
within config.mak file.

Signed-off-by: Vadim Kochan <vadim.kochan@petcube.com>
---
 package/git/git.mk | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/package/git/git.mk b/package/git/git.mk
index ac7b8f2e8c..2d3c3cc2c1 100644
--- a/package/git/git.mk
+++ b/package/git/git.mk
@@ -63,8 +63,18 @@ endif
 
 ifeq ($(BR2_SYSTEM_ENABLE_NLS),)
 GIT_MAKE_OPTS += NO_GETTEXT=1
+else
+ifeq ($(BR2_TOOLCHAIN_HAS_FULL_GETTEXT),)
+GIT_EXTLIBS += -lintl
+endif
 endif
 
+define GIT_GEN_EXT_CONFIG_CMDS
+	echo "EXTLIBS = $(GIT_EXTLIBS)" >> $(@D)/config.mak
+endef
+
+GIT_POST_CONFIGURE_HOOKS += GIT_GEN_EXT_CONFIG_CMDS
+
 GIT_INSTALL_TARGET_OPTS = $(GIT_MAKE_OPTS) DESTDIR=$(TARGET_DIR) install
 
 # assume yes for these tests, configure will bail out otherwise
-- 
2.14.1

[Buildroot] [PATCH 1/1] git: Fix libintl linking if there is no full gettext support

[Thomas Petazzoni]

Hello Vadim,

On Fri, 24 Aug 2018 16:20:49 +0300, Vadim Kochan wrote:
> From: Vadim Kochan <vadim.kochan@petcube.com>
> 
> Git could not resolve libntl_xxx symbols if toolchain does no provide
> full gettext support but the gettext package is selected.
> 
> In case of ulibc/musl toolchain there is no full getttext support, but
> they still provides libintl stub which makes git think there is full
> gettext support hence -lintl is not passed to the linker.
> 
> Added workaround which checks if toolchain provides full gettext
> support, if no then -lintl will be added to EXTLIBS variable generated
> within config.mak file.
> 
> Signed-off-by: Vadim Kochan <vadim.kochan@petcube.com>

Could you give some minimal Buildroot configuration that exhibits the
issue ?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] git: Fix libintl linking if there is no full gettext support

[Vadim Kochan]

Hi Thomas,

On Sat, Aug 25, 2018 at 12:09 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Vadim,
>
> On Fri, 24 Aug 2018 16:20:49 +0300, Vadim Kochan wrote:
> > From: Vadim Kochan <vadim.kochan@petcube.com>
> >
> > Git could not resolve libntl_xxx symbols if toolchain does no provide
> > full gettext support but the gettext package is selected.
> >
> > In case of ulibc/musl toolchain there is no full getttext support, but
> > they still provides libintl stub which makes git think there is full
> > gettext support hence -lintl is not passed to the linker.
> >
> > Added workaround which checks if toolchain provides full gettext
> > support, if no then -lintl will be added to EXTLIBS variable generated
> > within config.mak file.
> >
> > Signed-off-by: Vadim Kochan <vadim.kochan@petcube.com>
>
> Could you give some minimal Buildroot configuration that exhibits the
> issue ?

So I just did:
1) make menuconfig
2) Toolchain -> WCHAR
3) System -> NLS
4) Target packages -> Development -> git

I attached also config file w/o '#' and empty lines.

P.S.
Not sure if the fix is good, will try to cook v2 ...

Regards,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: config
Type: application/octet-stream
Size: 8859 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20180825/be8b1632/attachment.obj>

[Buildroot] [PATCH 1/1] git: Fix libintl linking if there is no full gettext support

[Thomas Petazzoni]

Hello Vadim,

On Sat, 25 Aug 2018 14:24:37 +0300, Vadim Kochan wrote:

> > Could you give some minimal Buildroot configuration that exhibits the
> > issue ?  
> 
> So I just did:
> 1) make menuconfig
> 2) Toolchain -> WCHAR
> 3) System -> NLS
> 4) Target packages -> Development -> git
> 
> I attached also config file w/o '#' and empty lines.
> 
> P.S.
> Not sure if the fix is good, will try to cook v2 ...

OK, I can reproduce (we should really add some testing for NLS enabled
in our autobuilders, let's do this after the 2018.08 release). However,
the proper fix is:

 ifeq ($(BR2_SYSTEM_ENABLE_NLS),)
 GIT_MAKE_OPTS += NO_GETTEXT=1
+else
+GIT_EXTLIBS += $(TARGET_NLS_LIBS)
 endif

TARGET_NLS_LIBS is automaticaly set to -lintl when gettext provides the
full libintl library we need to link against.

Could you test this, and send an updated patch ?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] git: Fix libintl linking if there is no full gettext support

[Vadim Kochan]

Hi Thomas,

On Sat, Aug 25, 2018 at 3:12 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Vadim,
>
> On Sat, 25 Aug 2018 14:24:37 +0300, Vadim Kochan wrote:
>
> > > Could you give some minimal Buildroot configuration that exhibits the
> > > issue ?
> >
> > So I just did:
> > 1) make menuconfig
> > 2) Toolchain -> WCHAR
> > 3) System -> NLS
> > 4) Target packages -> Development -> git
> >
> > I attached also config file w/o '#' and empty lines.
> >
> > P.S.
> > Not sure if the fix is good, will try to cook v2 ...
>
> OK, I can reproduce (we should really add some testing for NLS enabled
> in our autobuilders, let's do this after the 2018.08 release). However,
> the proper fix is:
>
>  ifeq ($(BR2_SYSTEM_ENABLE_NLS),)
>  GIT_MAKE_OPTS += NO_GETTEXT=1
> +else
> +GIT_EXTLIBS += $(TARGET_NLS_LIBS)
>  endif
>
> TARGET_NLS_LIBS is automaticaly set to -lintl when gettext provides the
> full libintl library we need to link against.
>
> Could you test this, and send an updated patch ?
>
> Thanks!

Sure! Thanks for the hint! I was also thinking about to just remove
LIBC_CONTAINS_LIBINTL
line from the config.mak.autogen file via $(SED), so this will
indicate for the git's Makefile
to add -lintl to the linker.

Regards,

[Buildroot] [PATCH 1/1] git: Fix libintl linking if there is no full gettext support

[Thomas Petazzoni]

Hello,

On Sat, 25 Aug 2018 15:25:50 +0300, Vadim Kochan wrote:

> Sure! Thanks for the hint! I was also thinking about to just remove
> LIBC_CONTAINS_LIBINTL
> line from the config.mak.autogen file via $(SED), so this will
> indicate for the git's Makefile
> to add -lintl to the linker.

That is another option. I haven't looked in detail at the git build
system.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com