| Message ID | 20180824132049.20330-1-vadim4j@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] git: Fix libintl linking if there is no full gettext support | expand |
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
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,
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
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,
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
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