[Buildroot] [PATCH] vim: fix gettext check

[Buildroot] [PATCH] vim: fix gettext check

[Thomas De Schampheleire]

Fixes http://autobuild.buildroot.net/results/21b5a910e6a27fa1cb12d0002ffed7e6ed9d6c83/

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

---
 package/vim/vim-0001-fix-nls-check.patch |  120 +++++++++++++++++++++++++++
 1 files changed, 120 insertions(+), 0 deletions(-)

diff --git a/package/vim/vim-0001-fix-nls-check.patch b/package/vim/vim-0001-fix-nls-check.patch
new file mode 100644
--- /dev/null
+++ b/package/vim/vim-0001-fix-nls-check.patch
@@ -0,0 +1,120 @@
+configure: use minimal LIBS during gettext check
+
+In some configurations, vim fails to link correctly with messages such as:
+edit.c:(.text+0x1614): undefined reference to `libintl_gettext'
+
+In particular, this has been seen by the buildroot autobuilds (see [1]) but
+has also been reported at [2] and [3].
+
+In the bad case, the configure script says:
+checking for NLS... gettext() works
+In the good case, it says:
+checking for NLS... gettext() works with -lintl
+
+In the bad case, the system has libelf, vim detects that and takes it along
+in $LIBS. Libelf needs libintl on this system, and so linking the test
+program with -lelf will automatically take -lintl too. This causes configure
+to think gettext() does not need libintl, while in reality it does.
+
+In the good case, libelf is not present and thus not used. The first
+configure test for gettext fails, and then configure retries with -lintl
+(which succeeds).
+
+Until now, there isn't really a problem. In both cases, you could link
+correctly. In the 'bad' case, libintl is implicitly taken through libelf, in
+the second case it is explicitly taken.
+
+The real problem occurs because configure also tests whether the linker
+supports --as-needed and uses it when possible, instead of the link.sh
+script. However, --as-needed seems to cause libintl NOT to be taken in the
+bad case, causing the undefined references to libintl_gettext.
+
+This patch changes the configure script so that the gettext check does not
+use additional libraries such as libelf. The test program is linked either
+with nothing, or with libintl alone. This will cause libintl to
+be added explicitly to LIBS, when needed.
+
+[1] http://autobuild.buildroot.net/results/21b5a910e6a27fa1cb12d0002ffed7e6ed9d6c83/
+[2] http://lists.freebsd.org/pipermail/freebsd-ports-bugs/2012-November/243930.html
+[3] http://lists.freebsd.org/pipermail/freebsd-ports-bugs/2012-May/234184.html
+
+
+Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
+Upstream-status: submitted
+
+---
+ src/auto/configure |  11 ++++++-----
+ src/configure.in   |  13 +++++++++----
+ 2 files changed, 15 insertions(+), 9 deletions(-)
+
+diff --git a/src/auto/configure b/src/auto/configure
+--- a/src/auto/configure
++++ b/src/auto/configure
+@@ -12271,6 +12271,8 @@ fi
+   if test -f po/Makefile; then
+     have_gettext="no"
+     if test -n "$MSGFMT"; then
++      olibs=$LIBS
++      LIBS=""
+       cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+ /* end confdefs.h.  */
+ #include <libintl.h>
+@@ -12284,10 +12286,9 @@ gettext("Test");
+ _ACEOF
+ if ac_fn_c_try_link "$LINENO"; then :
+   { $as_echo "$as_me:${as_lineno-$LINENO}: result: gettext() works" >&5
+-$as_echo "gettext() works" >&6; }; have_gettext="yes"
+-else
+-  olibs=$LIBS
+-	  LIBS="$LIBS -lintl"
++$as_echo "gettext() works" >&6; }; have_gettext="yes"; LIBS=$olibs;
++else
++	  LIBS="-lintl"
+ 	  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+ /* end confdefs.h.  */
+ #include <libintl.h>
+@@ -12301,7 +12302,7 @@ gettext("Test");
+ _ACEOF
+ if ac_fn_c_try_link "$LINENO"; then :
+   { $as_echo "$as_me:${as_lineno-$LINENO}: result: gettext() works with -lintl" >&5
+-$as_echo "gettext() works with -lintl" >&6; }; have_gettext="yes"
++$as_echo "gettext() works with -lintl" >&6; }; have_gettext="yes"; LIBS="$olibs -lintl";
+ else
+   { $as_echo "$as_me:${as_lineno-$LINENO}: result: gettext() doesn't work" >&5
+ $as_echo "gettext() doesn't work" >&6; };
+diff --git a/src/configure.in b/src/configure.in
+--- a/src/configure.in
++++ b/src/configure.in
+@@ -3484,6 +3484,9 @@ if test "$MANDEF" = "man -s"; then
+ fi
+ 
+ dnl Check if gettext() is working and if it needs -lintl
++dnl We take care to base this on an empty LIBS: on some systems libelf would be
++dnl in LIBS and implicitly take along libintl. The final LIBS would then not
++dnl contain libintl, and the link step would fail due to -Wl,--as-needed.
+ AC_MSG_CHECKING(--disable-nls argument)
+ AC_ARG_ENABLE(nls,
+ 	[  --disable-nls           Don't support NLS (gettext()).], ,
+@@ -3502,16 +3505,18 @@ if test "$enable_nls" = "yes"; then
+   if test -f po/Makefile; then
+     have_gettext="no"
+     if test -n "$MSGFMT"; then
++      olibs=$LIBS
++      LIBS=""
+       AC_TRY_LINK(
+ 	[#include <libintl.h>],
+ 	[gettext("Test");],
+-	AC_MSG_RESULT([gettext() works]); have_gettext="yes",
+-	  olibs=$LIBS
+-	  LIBS="$LIBS -lintl"
++	AC_MSG_RESULT([gettext() works]); have_gettext="yes"; LIBS=$olibs,
++	  LIBS="-lintl"
+ 	  AC_TRY_LINK(
+ 	      [#include <libintl.h>],
+ 	      [gettext("Test");],
+-	      AC_MSG_RESULT([gettext() works with -lintl]); have_gettext="yes",
++	      AC_MSG_RESULT([gettext() works with -lintl]); have_gettext="yes";
++	      LIBS="$olibs -lintl",
+ 	      AC_MSG_RESULT([gettext() doesn't work]);
+ 	      LIBS=$olibs))
+     else

[Buildroot] [PATCH] vim: fix gettext check

[Peter Korsgaard]

>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:

 > Fixes http://autobuild.buildroot.net/results/21b5a910e6a27fa1cb12d0002ffed7e6ed9d6c83/

As that URL doesn't currently work, could you explain a bit what the
problem is?

I've tried building it with a number of different toolchain
configurations, and don't seem to be able to trigger the issue.

I see that vim.mk doesn't do anything with gettext. Shouldn't it atleast
make sure it gets built after gettext if enabled and/or select gettext
if BR2_NEEDS_GETTEXT_IF_LOCALE?

 > Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

 > ---
 >  package/vim/vim-0001-fix-nls-check.patch |  120 +++++++++++++++++++++++++++
 >  1 files changed, 120 insertions(+), 0 deletions(-)

 > diff --git a/package/vim/vim-0001-fix-nls-check.patch b/package/vim/vim-0001-fix-nls-check.patch
 > new file mode 100644
 > --- /dev/null
 > +++ b/package/vim/vim-0001-fix-nls-check.patch
 > @@ -0,0 +1,120 @@
 > +configure: use minimal LIBS during gettext check
 > +
 > +In some configurations, vim fails to link correctly with messages such as:
 > +edit.c:(.text+0x1614): undefined reference to `libintl_gettext'
 > +
 > +In particular, this has been seen by the buildroot autobuilds (see [1]) but
 > +has also been reported at [2] and [3].
 > +
 > +In the bad case, the configure script says:
 > +checking for NLS... gettext() works
 > +In the good case, it says:
 > +checking for NLS... gettext() works with -lintl
 > +
 > +In the bad case, the system has libelf, vim detects that and takes it along
 > +in $LIBS. Libelf needs libintl on this system, and so linking the test
 > +program with -lelf will automatically take -lintl too. This causes configure
 > +to think gettext() does not need libintl, while in reality it does.
 > +
 > +In the good case, libelf is not present and thus not used. The first
 > +configure test for gettext fails, and then configure retries with -lintl
 > +(which succeeds).
 > +
 > +Until now, there isn't really a problem. In both cases, you could link
 > +correctly. In the 'bad' case, libintl is implicitly taken through libelf, in
 > +the second case it is explicitly taken.
 > +
 > +The real problem occurs because configure also tests whether the linker
 > +supports --as-needed and uses it when possible, instead of the link.sh
 > +script. However, --as-needed seems to cause libintl NOT to be taken in the
 > +bad case, causing the undefined references to libintl_gettext.
 > +
 > +This patch changes the configure script so that the gettext check does not
 > +use additional libraries such as libelf. The test program is linked either
 > +with nothing, or with libintl alone. This will cause libintl to
 > +be added explicitly to LIBS, when needed.
 > +
 > +[1] http://autobuild.buildroot.net/results/21b5a910e6a27fa1cb12d0002ffed7e6ed9d6c83/
 > +[2] http://lists.freebsd.org/pipermail/freebsd-ports-bugs/2012-November/243930.html
 > +[3] http://lists.freebsd.org/pipermail/freebsd-ports-bugs/2012-May/234184.html

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] vim: fix gettext check

[Thomas De Schampheleire]

Hi Peter,

On Sun, Nov 17, 2013 at 6:27 PM, Peter Korsgaard <jacmet@uclibc.org> wrote:
>>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:
>
>  > Fixes http://autobuild.buildroot.net/results/21b5a910e6a27fa1cb12d0002ffed7e6ed9d6c83/
>
> As that URL doesn't currently work, could you explain a bit what the
> problem is?
>
> I've tried building it with a number of different toolchain
> configurations, and don't seem to be able to trigger the issue.

I attached the defconfig file.
This situation was seen on the SH architecture. This seems to contain
libelf, which also depends on libintl. If you can create a similar
configuration, you should see the problem too (it's not SH specific).

The following does it on the supplied config:
make clean toolchain elfutils vim

The bottom line is that vim tries to detect how to link with gettext,
but while doing so takes along other libraries as well. It then thinks
that gettext is available directly, without libintl (because it is
implicitly taken by libelf). Later, it uses as-needed in the link
step, which strips the implicit libintl dependency. The explanation in
the patch itself is probably much clearer about this.

By the way, in the mean time the patch has been accepted upstream:
http://article.gmane.org/gmane.editors.vim.devel/43528

>
> I see that vim.mk doesn't do anything with gettext. Shouldn't it atleast
> make sure it gets built after gettext if enabled and/or select gettext
> if BR2_NEEDS_GETTEXT_IF_LOCALE?

I think you're right that there should be a gettext dependency to vim.
Currently, even if you have locale support, you may or may not get it
in vim, depending on the order of packages.
However, that is another improvement, it has no direct relation to the
autobuild fix.

I will submit a new patch (along with the updated 'upstream-status' in
the first patch).

Thanks for your feedback,
Thomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: defconfig
Type: application/octet-stream
Size: 7144 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20131118/4794f173/attachment.obj>