diff mbox series

glibc: Install iconvconfig on target with gconv libs

Message ID 20180802063830.19364-1-abrodkin@synopsys.com
State Changes Requested
Headers show
Series glibc: Install iconvconfig on target with gconv libs | expand

Commit Message

Alexey Brodkin Aug. 2, 2018, 6:38 a.m. UTC
In some cases we need to have gconv-modules.cache on target,
for example see [1] or glibc's test-suite (iconv/test-iconvconfig).

We do have iconvconfig installed in Buildroot's staging folder but
it gets never installed on targe because instead of calling glibc's
"install" target we simply copy a list of libs.

So then let's copy oven iconvconfig there as well in a similar manner.
Still we do it only if gconv libs are being copied otherwise it makes no
sense I guess.

[1] http://lists.busybox.net/pipermail/buildroot/2013-July/075097.html

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Romain Naour <romain.naour@gmail.com>
Cc: Baruch Siach <baruch@tkos.co.il>
---

I do realize my implementation might look not super correct as
appending *binary* to the list of *libs* is not good. But:

1. It's very tiny yet working fix or even better "improvement"
2. That might at least be a good starting pont for a discussion on
   what would be a better way :)

 package/glibc/glibc.mk | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Romain Naour Oct. 20, 2018, 9:07 p.m. UTC | #1
Hi Alexey,

Le 02/08/2018 à 08:38, Alexey Brodkin a écrit :
> In some cases we need to have gconv-modules.cache on target,
> for example see [1] or glibc's test-suite (iconv/test-iconvconfig).
> 
> We do have iconvconfig installed in Buildroot's staging folder but
> it gets never installed on targe because instead of calling glibc's
> "install" target we simply copy a list of libs.
> 
> So then let's copy oven iconvconfig there as well in a similar manner.
> Still we do it only if gconv libs are being copied otherwise it makes no
> sense I guess.
> 
> [1] http://lists.busybox.net/pipermail/buildroot/2013-July/075097.html
> 
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Romain Naour <romain.naour@gmail.com>
> Cc: Baruch Siach <baruch@tkos.co.il>
> ---
> 
> I do realize my implementation might look not super correct as
> appending *binary* to the list of *libs* is not good. But:
> 
> 1. It's very tiny yet working fix or even better "improvement"
> 2. That might at least be a good starting pont for a discussion on
>    what would be a better way :)

Well, we don't require copy_toolchain_lib_root to install iconvconfig binary on
the target. Libraries needs a complicated logic to be deployed on the target
hence copy_toolchain_lib_root.

Maybe just add GLIBC_INSTALL_ICONVCONFIG:

define GLIBC_INSTALL_ICONVCONFIG
	$(INSTALL) -m 0755 -D $(@D)/build/iconv/iconvconfig \
		$(TARGET_DIR)/usr/sbin/iconvconfig
endef

define GLIBC_INSTALL_TARGET_CMDS
	for libpattern in $(GLIBC_LIBS_LIB); do \
		$(call copy_toolchain_lib_root,$$libpattern) ; \
	done
	$(GLIBC_INSTALL_ICONVCONFIG)
endef

Best regards,
Romain


> 
>  package/glibc/glibc.mk | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/package/glibc/glibc.mk b/package/glibc/glibc.mk
> index 6d21ae7ac07d..9366b3ec4199 100644
> --- a/package/glibc/glibc.mk
> +++ b/package/glibc/glibc.mk
> @@ -119,6 +119,10 @@ ifeq ($(BR2_PACKAGE_GDB),y)
>  GLIBC_LIBS_LIB += libthread_db.so.*
>  endif
>  
> +ifeq ($(BR2_TOOLCHAIN_GLIBC_GCONV_LIBS_COPY),y)
> +GLIBC_LIBS_LIB += iconvconfig
> +endif
> +
>  define GLIBC_INSTALL_TARGET_CMDS
>  	for libpattern in $(GLIBC_LIBS_LIB); do \
>  		$(call copy_toolchain_lib_root,$$libpattern) ; \
>
Thomas Petazzoni April 7, 2019, 9:05 p.m. UTC | #2
Hello Alexey,

On Thu,  2 Aug 2018 09:38:30 +0300
Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote:

> diff --git a/package/glibc/glibc.mk b/package/glibc/glibc.mk
> index 6d21ae7ac07d..9366b3ec4199 100644
> --- a/package/glibc/glibc.mk
> +++ b/package/glibc/glibc.mk
> @@ -119,6 +119,10 @@ ifeq ($(BR2_PACKAGE_GDB),y)
>  GLIBC_LIBS_LIB += libthread_db.so.*
>  endif
>  
> +ifeq ($(BR2_TOOLCHAIN_GLIBC_GCONV_LIBS_COPY),y)
> +GLIBC_LIBS_LIB += iconvconfig
> +endif

This solution only solves the problem for the internal toolchain
backend, and leaves external toolchain unsupported.

Instead, this should be done in toolchain/toolchain.mk, as part of the
TOOLCHAIN_GLIBC_COPY_GCONV_LIBS macro.

A possible (but separate!) improvement would be to move this macro from
being called as a TOOLCHAIN_TARGET_FINALIZE_HOOKS to a
TOOLCHAIN_POST_INSTALL_TARGET_HOOKS.

However, overall it would be a lot nicer to not install iconvconfig to
the target at all, and be able to generate the gconv-modules.cache at
build time. This would make the thing work nicely for read-only
filesystem configurations, for example.

Best regards,

Thomas
diff mbox series

Patch

diff --git a/package/glibc/glibc.mk b/package/glibc/glibc.mk
index 6d21ae7ac07d..9366b3ec4199 100644
--- a/package/glibc/glibc.mk
+++ b/package/glibc/glibc.mk
@@ -119,6 +119,10 @@  ifeq ($(BR2_PACKAGE_GDB),y)
 GLIBC_LIBS_LIB += libthread_db.so.*
 endif
 
+ifeq ($(BR2_TOOLCHAIN_GLIBC_GCONV_LIBS_COPY),y)
+GLIBC_LIBS_LIB += iconvconfig
+endif
+
 define GLIBC_INSTALL_TARGET_CMDS
 	for libpattern in $(GLIBC_LIBS_LIB); do \
 		$(call copy_toolchain_lib_root,$$libpattern) ; \