All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] RFC: musl: Fix gettext support
@ 2016-01-30 22:51 Bernd Kuhls
  2016-01-31  7:33 ` Thomas Petazzoni
  2016-02-01 16:11 ` Peter Korsgaard
  0 siblings, 2 replies; 8+ messages in thread
From: Bernd Kuhls @ 2016-01-30 22:51 UTC (permalink / raw)
  To: buildroot

The issue is discussed here:
http://thread.gmane.org/gmane.comp.lib.uclibc.buildroot/127196

I consider this patch an ugly hack but I could compile ~900 packages
without a gettext-related error, so I put this patch up for discussion.

Fixes
http://autobuild.buildroot.net/results/911/9115ddb411ec15eb8bfa1d4f03a804f48e77ff76/
and many similar errors

To test use the defconfig mentioned above, then

make clean
make dos2unix (build will succeed either with or without this patch)

make clean
make gettext
make dos2unix (build fails because gettext replaced musl's libintl.h)

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
---
 package/Makefile.in        |  6 ++++++
 package/gettext/gettext.mk | 14 ++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/package/Makefile.in b/package/Makefile.in
index c5652af..16ab17f 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -340,6 +340,11 @@ else
 BR2_AC_CV_C_BIGENDIAN = ac_cv_c_bigendian=no
 endif
 
+# configure script misdetects musl gettext support
+ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y)
+BR2_GT_CV_FUNC_GNUGETTEXT1_LIBC = gt_cv_func_gnugettext1_libc=yes
+endif
+
 TARGET_CONFIGURE_ARGS = \
 	$(BR2_AC_CV_TRAP_CHECK) \
 	ac_cv_func_mmap_fixed_mapped=yes \
@@ -350,6 +355,7 @@ TARGET_CONFIGURE_ARGS = \
 	ac_cv_func_calloc_0_nonnull=yes \
 	ac_cv_func_realloc_0_nonnull=yes \
 	lt_cv_sys_lib_search_path_spec="" \
+	$(BR2_GT_CV_FUNC_GNUGETTEXT1_LIBC) \
 	$(BR2_AC_CV_C_BIGENDIAN)
 
 ################################################################################
diff --git a/package/gettext/gettext.mk b/package/gettext/gettext.mk
index 0b4e5dd..f388ea8 100644
--- a/package/gettext/gettext.mk
+++ b/package/gettext/gettext.mk
@@ -27,6 +27,20 @@ GETTEXT_CONF_OPTS += \
 	--disable-relocatable \
 	--without-emacs
 
+# musl provides a gettext implementation, but gettext does not
+# recognize it as being a valid implementation.
+ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y)
+GETTEXT_CONF_ENV += \
+	ac_cv_gnu_library_2_1=yes \
+	ac_cv_gnu_library_2=yes
+
+# prevent gettext overwriting musl's libintl.h
+define GETTEXT_DO_NOT_INSTALL_LIBINTL_H
+	$(SED) '/\$$(INSTALL_DATA) libintl.h/d' $(@D)/gettext-runtime/intl/Makefile.in
+endef
+GETTEXT_POST_PATCH_HOOKS = GETTEXT_DO_NOT_INSTALL_LIBINTL_H
+endif
+
 HOST_GETTEXT_CONF_OPTS = \
 	--disable-libasprintf \
 	--disable-acl \
-- 
2.7.0.rc3

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH 1/1] RFC: musl: Fix gettext support
  2016-01-30 22:51 [Buildroot] [PATCH 1/1] RFC: musl: Fix gettext support Bernd Kuhls
@ 2016-01-31  7:33 ` Thomas Petazzoni
  2016-01-31  7:49   ` Bernd Kuhls
  2016-01-31 20:17   ` Bernd Kuhls
  2016-02-01 16:11 ` Peter Korsgaard
  1 sibling, 2 replies; 8+ messages in thread
From: Thomas Petazzoni @ 2016-01-31  7:33 UTC (permalink / raw)
  To: buildroot

Dear Bernd Kuhls,

On Sat, 30 Jan 2016 23:51:33 +0100, Bernd Kuhls wrote:
> The issue is discussed here:
> http://thread.gmane.org/gmane.comp.lib.uclibc.buildroot/127196
> 
> I consider this patch an ugly hack but I could compile ~900 packages
> without a gettext-related error, so I put this patch up for discussion.

I've looked at your patch, and I don't think it's such a hack. It
really fixes the root of the problem: 1/ convince gettext that musl has
a valid gettext implementation so that it doesn't build/install its own
libintl library and 2/ convince the gettext.m4 logic in all packages
that the provided libintl implementation is good enough for them.

I have just one question.

> +# prevent gettext overwriting musl's libintl.h
> +define GETTEXT_DO_NOT_INSTALL_LIBINTL_H
> +	$(SED) '/\$$(INSTALL_DATA) libintl.h/d' $(@D)/gettext-runtime/intl/Makefile.in
> +endef
> +GETTEXT_POST_PATCH_HOOKS = GETTEXT_DO_NOT_INSTALL_LIBINTL_H
> +endif

Why is this part needed ? By passing ac_cv_gnu_library_2_1=yes and
ac_cv_gnu_library_2=yes, aren't you convincing gettext to not
build/install its libintl library ? If so, why is gettext still
installing its own libintl.h ?

Thanks!

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH 1/1] RFC: musl: Fix gettext support
  2016-01-31  7:33 ` Thomas Petazzoni
@ 2016-01-31  7:49   ` Bernd Kuhls
  2016-01-31 20:17   ` Bernd Kuhls
  1 sibling, 0 replies; 8+ messages in thread
From: Bernd Kuhls @ 2016-01-31  7:49 UTC (permalink / raw)
  To: buildroot

Am Sun, 31 Jan 2016 08:33:25 +0100 schrieb Thomas Petazzoni:

> Why is this part needed ? By passing ac_cv_gnu_library_2_1=yes and
> ac_cv_gnu_library_2=yes, aren't you convincing gettext to not
> build/install its libintl library ? If so, why is gettext still
> installing its own libintl.h ?

Hi Thomas,

gettext-0.19.7/gettext-runtime/intl/Makefile.in, line 403 installs 
libintl.h to $DESTDIR if @USE_INCLUDED_LIBINTL@ was set to "yes" by the 
configure script. Despite the ac_cv* variables, which are necessary 
anyway, at the end they fail to prevent installing libintl.h, therefore I 
hacked gettext-0.19.7/gettext-runtime/intl/Makefile.in by removing line 
403. I did not spent hours in analyzing the package, maybe there is a way 
to set @USE_INCLUDED_LIBINTL@ to "no", but I did not find it. Therefore I 
consider my patch as an ugly hack. I tried to set other variables in 
_CONF_ENV as well, like
	nls_cv_use_gnu_gettext
	nls_cv_force_use_gnu_gettext
	gt_included_intl
but they did not fix the problem.

Regards, Bernd

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH 1/1] RFC: musl: Fix gettext support
  2016-01-31  7:33 ` Thomas Petazzoni
  2016-01-31  7:49   ` Bernd Kuhls
@ 2016-01-31 20:17   ` Bernd Kuhls
  1 sibling, 0 replies; 8+ messages in thread
From: Bernd Kuhls @ 2016-01-31 20:17 UTC (permalink / raw)
  To: buildroot

Am Sun, 31 Jan 2016 08:33:25 +0100 schrieb Thomas Petazzoni:

> I've looked at your patch, and I don't think it's such a hack.

Hi Thomas,

ftr, this patch can be reverted when a general solution for gettext/musl 
is found: https://git.busybox.net/buildroot/commit/package/madplay/
madplay.mk?id=a0a244d26d8e7e7e5465c3e6e9fcd1c31e2c178d

Madplay compiles without it using my patch.

Regards, Bernd

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH 1/1] RFC: musl: Fix gettext support
  2016-01-30 22:51 [Buildroot] [PATCH 1/1] RFC: musl: Fix gettext support Bernd Kuhls
  2016-01-31  7:33 ` Thomas Petazzoni
@ 2016-02-01 16:11 ` Peter Korsgaard
  2016-02-01 17:45   ` Bernd Kuhls
  2016-02-01 21:05   ` Bernd Kuhls
  1 sibling, 2 replies; 8+ messages in thread
From: Peter Korsgaard @ 2016-02-01 16:11 UTC (permalink / raw)
  To: buildroot

>>>>> "Bernd" == Bernd Kuhls <bernd.kuhls@t-online.de> writes:

 > The issue is discussed here:
 > http://thread.gmane.org/gmane.comp.lib.uclibc.buildroot/127196

 > I consider this patch an ugly hack but I could compile ~900 packages
 > without a gettext-related error, so I put this patch up for discussion.

 > Fixes
 > http://autobuild.buildroot.net/results/911/9115ddb411ec15eb8bfa1d4f03a804f48e77ff76/
 > and many similar errors

 > To test use the defconfig mentioned above, then

 > make clean
 > make dos2unix (build will succeed either with or without this patch)

 > make clean
 > make gettext
 > make dos2unix (build fails because gettext replaced musl's libintl.h)

 > Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>

Hi,

I now spent some quality time (!) with gettext.m4 and figured out what
we really going on here with musl and libintl.h, and I've pushed a
simpler / more complete patch fixing the issue.

I have therefore marked this patch as superseeded.

I've also reverted the madplay fix now that it is handled globally in
TARGET_CONFIGURE_ARGS.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH 1/1] RFC: musl: Fix gettext support
  2016-02-01 16:11 ` Peter Korsgaard
@ 2016-02-01 17:45   ` Bernd Kuhls
  2016-02-01 21:05   ` Bernd Kuhls
  1 sibling, 0 replies; 8+ messages in thread
From: Bernd Kuhls @ 2016-02-01 17:45 UTC (permalink / raw)
  To: buildroot

Am Mon, 01 Feb 2016 17:11:57 +0100 schrieb Peter Korsgaard:

> I now spent some quality time (!) with gettext.m4 and figured out what
> we really going on here with musl and libintl.h, and I've pushed a
> simpler / more complete patch fixing the issue.

Hi Peter,

thanks, the code looks good, usr/include/libintl.h is not overwritten by 
gettext anymore, my dos2unix testcase works!

Regards, Bernd

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH 1/1] RFC: musl: Fix gettext support
  2016-02-01 16:11 ` Peter Korsgaard
  2016-02-01 17:45   ` Bernd Kuhls
@ 2016-02-01 21:05   ` Bernd Kuhls
  2016-02-01 22:42     ` Peter Korsgaard
  1 sibling, 1 reply; 8+ messages in thread
From: Bernd Kuhls @ 2016-02-01 21:05 UTC (permalink / raw)
  To: buildroot

Am Mon, 01 Feb 2016 17:11:57 +0100 schrieb Peter Korsgaard:

> I now spent some quality time (!) with gettext.m4 and figured out what
> we really going on here with musl and libintl.h, and I've pushed a
> simpler / more complete patch fixing the issue.
> 
> I have therefore marked this patch as superseeded.
> 
> I've also reverted the madplay fix now that it is handled globally in
> TARGET_CONFIGURE_ARGS.

Hi Peter,

there is still a problem left, binutils contains its own intl library 
with a GNU libintl.h, similar to the one provided by gettext. It changes 
function names in libbfd.so, this leads to a link error at least with 
dropwatch:

http://autobuild.buildroot.net/results/c90/
c90f7688b3b2e07f3232db163dc747c954604442//

/home/test/autobuild/instance-2/output/host/usr/x86_64-buildroot-linux-
musl/sysroot/usr/lib/../lib64/libbfd.so: undefined reference to 
`libintl_dgettext'

This situation is still present today after your commit:

output/host/usr/bin/x86_64-linux-readelf -a output/staging/usr/lib64/
libbfd-2.24.so | grep gettext
0000002e4d50  012400000007 R_X86_64_JUMP_SLO 0000000000000000 
libintl_dgettext + 0
   292: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND 
libintl_dgettext
  1275: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND 
libintl_dgettext

Again I hacked a bit to get the problem solved in a way you will never 
accept it ;)

$ cat package/binutils/2.24/200-musl-gettext.patch
diff -uNr binutils-2.24.org/Makefile.in binutils-2.24/Makefile.in
--- binutils-2.24.org/Makefile.in	2013-12-02 10:31:21.000000000 
+0100
+++ binutils-2.24/Makefile.in	2016-02-01 21:54:49.286655138 +0100
@@ -16938,6 +16938,7 @@
 	esac; \
 	srcdiroption="--srcdir=$${topdir}/intl"; \
 	libsrcdir="$$s/intl"; \
+	gt_cv_func_gnugettext1_libc=yes \
 	$(SHELL) $${libsrcdir}/configure \
 	  $(HOST_CONFIGARGS) --build=${build_alias} --host=${host_alias} \
 	  --target=${target_alias} $${srcdiroption}  \

Using this patch libbfd-2.24.so will look like this:

$ output/host/usr/bin/x86_64-linux-readelf -a output/staging/usr/lib64/
libbfd-2.24.so | grep gettext
0000002e4be0  00bb00000007 R_X86_64_JUMP_SLO 0000000000000000 dgettext + 0
   187: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND dgettext
  1171: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND dgettext

and dropwatch will compile fine. I tried to get the content of our 
$gt_cv_func_gnugettext1_libc variable from Makefile.in down to binutils/
intl/configure and failed :( I hope this will help anyway.

Regards, Bernd

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH 1/1] RFC: musl: Fix gettext support
  2016-02-01 21:05   ` Bernd Kuhls
@ 2016-02-01 22:42     ` Peter Korsgaard
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Korsgaard @ 2016-02-01 22:42 UTC (permalink / raw)
  To: buildroot

>>>>> "Bernd" == Bernd Kuhls <bernd.kuhls@t-online.de> writes:

Hi,

 > Am Mon, 01 Feb 2016 17:11:57 +0100 schrieb Peter Korsgaard:
 >> I now spent some quality time (!) with gettext.m4 and figured out what
 >> we really going on here with musl and libintl.h, and I've pushed a
 >> simpler / more complete patch fixing the issue.
 >> 
 >> I have therefore marked this patch as superseeded.
 >> 
 >> I've also reverted the madplay fix now that it is handled globally in
 >> TARGET_CONFIGURE_ARGS.

 > Hi Peter,

 > there is still a problem left, binutils contains its own intl library 
 > with a GNU libintl.h, similar to the one provided by gettext. It changes 
 > function names in libbfd.so, this leads to a link error at least with 
 > dropwatch:

Thanks. The problem is really that the binutils build system runs
configure in the sub directories from make, and we only pass
TARGET_CONFIGURE_ARGS in the environment of the configure step and not
the make step.

I'll send a fix shortly.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-02-01 22:42 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-30 22:51 [Buildroot] [PATCH 1/1] RFC: musl: Fix gettext support Bernd Kuhls
2016-01-31  7:33 ` Thomas Petazzoni
2016-01-31  7:49   ` Bernd Kuhls
2016-01-31 20:17   ` Bernd Kuhls
2016-02-01 16:11 ` Peter Korsgaard
2016-02-01 17:45   ` Bernd Kuhls
2016-02-01 21:05   ` Bernd Kuhls
2016-02-01 22:42     ` Peter Korsgaard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.