Ticket #3629 (closed defect: fixed)

Opened 9 years ago

Last modified 5 years ago

[PATCH] Linking fix for non-default gettext package

Reported by: and Owned by: andrew_b
Priority: minor Milestone: 4.8.24
Component: mc-core Version: master
Keywords: Cc: slyfox
Blocked By: Blocking:
Branch state: merged Votes for changeset: committed-master

Description

mc lost the linking game if gettext package pulled from outside of system paths.
(Pulling can be done by suitable C-/CPP/LDFLAGS settings)

Furthermore gettext package depends on libintl (if system lib
don't provide needed functions) then gettext prepare INITLIBS/LIBINTL variable
for working libintl pull in from gettext libdir.

https://www.gnu.org/software/gettext/FAQ.html#integrating_undefined

Let respect LIBINTL variable at linking (it is empty if not needed).

Failure example for Solaris 10 with non-system-default gettext package:

Undefined first referenced

symbol in file
libintl_bind_textdomain_codeset ./.libs/libinternal.a(args.o)
libintl_gettext main.o
libintl_textdomain main.o
libintl_bindtextdomain main.o
libintl_ngettext ./.libs/libinternal.a(midnight.o)

Signed-off-by: Andreas Mohr <and@…>

Attachments

mc-3629-Linking-fix-for-gettext.patch (1.6 KB) - added by and 9 years ago.
0001-Ticket-3629-configure.ac-drop-bundled-gettext.patch (3.5 KB) - added by slyfox 5 years ago.
0001-Ticket-3629-configure.ac-drop-bundled-gettext.patch

Change History

Changed 9 years ago by and

comment:1 Changed 9 years ago by zaytsev

  • Owner set to zaytsev
  • Status changed from new to accepted

comment:2 Changed 9 years ago by zaytsev

I think that this patch is not correct.

It seems to me that the real problem is that we still use an outdated variable $(INTLLIBS) to link to gettext, see source:lib/Makefile.am#L77. The right way is to use the "new" @LTLIBINTL@ substitution in libmc, and the problem should go away:

https://www.gnu.org/software/gettext/manual/html_node/src_002fMakefile.html

My understanding is that it's currently done for libmc, so that we don't need to do it separately for mc binaries, tests, etc.

comment:3 Changed 9 years ago by zaytsev

  • Milestone changed from 4.8.17 to 4.8.18

comment:4 Changed 9 years ago by and

Honestly, libintl_* undefined in libinternal.a .

/bin/bash ../libtool  --tag=CC   --mode=link cc -std=c99  -xtarget=haswell -xchip=haswell -xarch=avx -O2 -O2 -xcheck=%all -I/opt/ncursesw-5.9/include/ncursesw -I/opt/ncursesw-5.9/include -Wl,-rpath,/opt/ncursesw-5.9/lib/  -lsocket -L/opt/ncursesw-5.9/lib -R/opt/ncursesw-5.9/lib -lncursesw -L/opt/gettext-0.19.7/lib -R/opt/gettext-0.19.7/lib -o mc main.o libinternal.la ../lib/libmc.la  -lnsl
libtool: link: cc -std=c99 -xtarget=haswell -xchip=haswell -xarch=avx -O2 -O2 -xcheck=%all -I/opt/ncursesw-5.9/include/ncursesw -I/opt/ncursesw-5.9/include -Wl,-rpath -Wl,/opt/ncursesw-5.9/lib/ -o mc main.o  -L/opt/ncursesw-5.9/lib -L/opt/gettext-0.19.7/lib ./.libs/libinternal.a -L/opt/glib-2.43.2/lib ../lib/.libs/libmc.a -lsocket -lncursesw /opt/glib-2.43.2/lib/libglib-2.0.so -lpthread -lthread -lrt -lc -lnsl -R/opt/glib-2.43.2/lib -R/opt/glib-2.43.2/lib -R/opt/ncursesw-5.9/lib -R/opt/gettext-0.19.7/lib
Undefined                       first referenced
 symbol                             in file
libintl_bind_textdomain_codeset     ./.libs/libinternal.a(args.o)
libintl_gettext                     main.o
libintl_textdomain                  main.o
libintl_bindtextdomain              main.o
libintl_ngettext                    ./.libs/libinternal.a(midnight.o)

comment:5 Changed 9 years ago by and

Your for finding of libmc $(INTLLIBS) I attached a cleanup patch at #3607.

comment:6 Changed 8 years ago by zaytsev

  • Milestone changed from 4.8.18 to 4.8.19

comment:7 Changed 7 years ago by zaytsev

  • Milestone changed from 4.8.20 to 4.8.21

comment:8 Changed 7 years ago by zaytsev

  • Milestone changed from 4.8.21 to 4.8.22

comment:9 Changed 6 years ago by zaytsev

  • Milestone changed from 4.8.22 to 4.8.23

comment:10 Changed 6 years ago by zaytsev

  • Milestone changed from 4.8.23 to 4.8.24

comment:11 Changed 5 years ago by slyfox

  • Cc slyfox added

I think we are seeing similar (the same?) bug on musl (https://bugs.gentoo.org/693850) and on glibc. Simple reproducer:

$ git describe
4.8.23-72-g5990b06c9
$ ./configure --with-included-gettext
$ make
...
  CCLD     mc
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: ./.libs/libinternal.a(filegui.o): in function `file_frmt_time':
/home/slyfox/dev/git/mc/src/filemanager/filegui.c:344: undefined reference to `libintl_gettext'
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: ./.libs/libinternal.a(filegui.o): in function `file_eta_prepare_for_show':
/home/slyfox/dev/git/mc/src/filemanager/filegui.c:363: undefined reference to `libintl_gettext'
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: ./.libs/libinternal.a(filegui.o): in function `file_bps_prepare_for_show':
/home/slyfox/dev/git/mc/src/filemanager/filegui.c:374: undefined reference to `libintl_gettext'

comment:12 Changed 5 years ago by slyfox

Tl;DR: lib/Makefile.am:libmc_la_LIBADD += ... $(LIBINTL) is invalid because $(LIBINTL) can only be used against executable programs to link against, not shared libraries and not even static libraries.

Looking at why it happens:

$ /bin/sh ../libtool  --tag=CC   --mode=link gcc  -fdiagnostics-show-option -Wbad-function-cast -Wcomment -Wdeclaration-after-statement -Wfloat-conversion -Wfloat-equal -Wformat -Wformat-security -Wformat-signedness -Wimplicit -Wimplicit-fallthrough -Wignored-qualifiers -Wlogical-not-parentheses -Wmaybe-uninitialized -Wmissing-braces -Wmissing-declarations -Wmissing-field-initializers -Wmissing-format-attribute -Wmissing-parameter-type -Wmissing-prototypes -Wnested-externs -Wno-long-long -Wno-unreachable-code -Wparentheses -Wpointer-arith -Wpointer-sign -Wredundant-decls -Wreturn-type -Wsequence-point -Wshadow -Wsign-compare -Wstrict-prototypes -Wswitch -Wswitch-default -Wtype-limits -Wundef -Wuninitialized -Wunreachable-code -Wunused-but-set-variable -Wunused-function -Wunused-label -Wunused-parameter -Wunused-result -Wunused-value -Wunused-variable -Wwrite-strings  -g -O2   -o libmc.la   utilunix.lo util.lo hook.lo glibcompat.lo global.lo keybind.lo lock.lo serialize.lo shell.lo timefmt.lo timer.lo  charsets.lo event/libmcevent.la filehighlight/libmcfilehighlight.la mcconfig/libmcconfig.la search/libsearch.la strutil/libmcstrutil.la skin/libmcskin.la tty/libmctty.la vfs/libmcvfs.la widget/libmcwidget.la -lslang  -lgpm -lssh2  -pthread -lgmodule-2.0 -lglib-2.0    ../intl/libintl.a   -lutil

*** Warning: Linking the shared library libmc.la against the
*** static library ../intl/libintl.a is not portable!
libtool: link: rm -fr  .libs/libmc.a .libs/libmc.la
libtool: link: (cd .libs/libmc.lax/libmcevent.a && ar x "/home/slyfox/dev/git/mc/lib/event/.libs/libmcevent.a")
libtool: link: (cd .libs/libmc.lax/libmcfilehighlight.a && ar x "/home/slyfox/dev/git/mc/lib/filehighlight/.libs/libmcfilehighlight.a")
libtool: link: (cd .libs/libmc.lax/libmcconfig.a && ar x "/home/slyfox/dev/git/mc/lib/mcconfig/.libs/libmcconfig.a")
libtool: link: (cd .libs/libmc.lax/libsearch.a && ar x "/home/slyfox/dev/git/mc/lib/search/.libs/libsearch.a")
libtool: link: (cd .libs/libmc.lax/libmcstrutil.a && ar x "/home/slyfox/dev/git/mc/lib/strutil/.libs/libmcstrutil.a")
libtool: link: (cd .libs/libmc.lax/libmcskin.a && ar x "/home/slyfox/dev/git/mc/lib/skin/.libs/libmcskin.a")
libtool: link: (cd .libs/libmc.lax/libmctty.a && ar x "/home/slyfox/dev/git/mc/lib/tty/.libs/libmctty.a")
libtool: link: (cd .libs/libmc.lax/libmcvfs.a && ar x "/home/slyfox/dev/git/mc/lib/vfs/.libs/libmcvfs.a")
libtool: link: (cd .libs/libmc.lax/libmcwidget.a && ar x "/home/slyfox/dev/git/mc/lib/widget/.libs/libmcwidget.a")
copying selected object files to avoid basename conflicts...
libtool: link: ln .libs/libmc.lax/libmcconfig.a/common.o .libs/libmc.lax/lt1-common.o || cp .libs/libmc.lax/libmcconfig.a/common.o .libs/libmc.lax/lt1-common.o
libtool: link: ln .libs/libmc.lax/libmcskin.a/common.o .libs/libmc.lax/lt2-common.o || cp .libs/libmc.lax/libmcskin.a/common.o .libs/libmc.lax/lt2-common.o
libtool: link: ln .libs/libmc.lax/libmcwidget.a/history.o .libs/libmc.lax/lt3-history.o || cp .libs/libmc.lax/libmcwidget.a/history.o .libs/libmc.lax/lt3-history.o
libtool: link: ln .libs/libmc.lax/libmcwidget.a/mouse.o .libs/libmc.lax/lt4-mouse.o || cp .libs/libmc.lax/libmcwidget.a/mouse.o .libs/libmc.lax/lt4-mouse.o
libtool: link: ar cru .libs/libmc.a .libs/utilunix.o .libs/util.o .libs/hook.o .libs/glibcompat.o .libs/global.o .libs/keybind.o .libs/lock.o .libs/serialize.o .libs/shell.o .libs/timefmt.o .libs/timer.o .libs/charsets.o .libs/libmc.lax/libmcevent.a/event.o .libs/libmc.lax/libmcevent.a/manage.o .libs/libmc.lax/libmcevent.a/raise.o .libs/libmc.lax/libmcfilehighlight.a/common.o .libs/libmc.lax/libmcfilehighlight.a/get-color.o .libs/libmc.lax/libmcfilehighlight.a/ini-file-read.o .libs/libmc.lax/lt1-common.o .libs/libmc.lax/libmcconfig.a/get.o .libs/libmc.lax/libmcconfig.a/history.o .libs/libmc.lax/libmcconfig.a/paths.o .libs/libmc.lax/libmcconfig.a/set.o .libs/libmc.lax/libsearch.a/glob.o .libs/libmc.lax/libsearch.a/hex.o .libs/libmc.lax/libsearch.a/lib.o .libs/libmc.lax/libsearch.a/normal.o .libs/libmc.lax/libsearch.a/regex.o .libs/libmc.lax/libsearch.a/search.o .libs/libmc.lax/libmcstrutil.a/filevercmp.o .libs/libmc.lax/libmcstrutil.a/replace.o .libs/libmc.lax/libmcstrutil.a/strescape.o .libs/libmc.lax/libmcstrutil.a/strutil.o .libs/libmc.lax/libmcstrutil.a/strutil8bit.o .libs/libmc.lax/libmcstrutil.a/strutilascii.o .libs/libmc.lax/libmcstrutil.a/strutilutf8.o .libs/libmc.lax/libmcstrutil.a/strverscmp.o .libs/libmc.lax/libmcstrutil.a/xstrtol.o .libs/libmc.lax/libmcskin.a/colors-old.o .libs/libmc.lax/libmcskin.a/colors.o .libs/libmc.lax/lt2-common.o .libs/libmc.lax/libmcskin.a/hc-skins.o .libs/libmc.lax/libmcskin.a/ini-file.o .libs/libmc.lax/libmcskin.a/lines.o .libs/libmc.lax/libmctty.a/color-internal.o .libs/libmc.lax/libmctty.a/color-slang.o .libs/libmc.lax/libmctty.a/color.o .libs/libmc.lax/libmctty.a/key.o .libs/libmc.lax/libmctty.a/keyxdef.o .libs/libmc.lax/libmctty.a/mouse.o .libs/libmc.lax/libmctty.a/tty-internal.o .libs/libmc.lax/libmctty.a/tty-slang.o .libs/libmc.lax/libmctty.a/tty.o .libs/libmc.lax/libmctty.a/win.o .libs/libmc.lax/libmctty.a/x11conn.o .libs/libmc.lax/libmcvfs.a/direntry.o .libs/libmc.lax/libmcvfs.a/gc.o .libs/libmc.lax/libmcvfs.a/interface.o .libs/libmc.lax/libmcvfs.a/netutil.o .libs/libmc.lax/libmcvfs.a/parse_ls_vga.o .libs/libmc.lax/libmcvfs.a/path.o .libs/libmc.lax/libmcvfs.a/utilvfs.o .libs/libmc.lax/libmcvfs.a/vfs.o .libs/libmc.lax/libmcwidget.a/button.o .libs/libmc.lax/libmcwidget.a/buttonbar.o .libs/libmc.lax/libmcwidget.a/check.o .libs/libmc.lax/libmcwidget.a/dialog-switch.o .libs/libmc.lax/libmcwidget.a/dialog.o .libs/libmc.lax/libmcwidget.a/gauge.o .libs/libmc.lax/libmcwidget.a/groupbox.o .libs/libmc.lax/lt3-history.o .libs/libmc.lax/libmcwidget.a/hline.o .libs/libmc.lax/libmcwidget.a/input.o .libs/libmc.lax/libmcwidget.a/input_complete.o .libs/libmc.lax/libmcwidget.a/label.o .libs/libmc.lax/libmcwidget.a/listbox-window.o .libs/libmc.lax/libmcwidget.a/listbox.o .libs/libmc.lax/libmcwidget.a/menu.o .libs/libmc.lax/lt4-mouse.o .libs/libmc.lax/libmcwidget.a/quick.o .libs/libmc.lax/libmcwidget.a/radio.o .libs/libmc.lax/libmcwidget.a/widget-common.o .libs/libmc.lax/libmcwidget.a/wtools.o
libtool: link: ranlib .libs/libmc.a
libtool: link: rm -fr .libs/libmc.lax .libs/libmc.lax
libtool: link: ( cd ".libs" && rm -f "libmc.la" && ln -s "../libmc.la" "libmc.la" )
$ cat libmc.la

# libmc.la - a libtool library file
# Generated by libtool (GNU libtool) 2.4.6
#
# Please DO NOT delete this file!
# It is necessary for linking the library.

# The name that we can dlopen(3).
dlname=''

# Names of this library.
library_names=''

# The name of the static archive.
old_library='libmc.a'

# Linker flags that cannot go in dependency_libs.
inherited_linker_flags=' -pthread'

# Libraries that this one depends upon.
dependency_libs=' -lslang -lgpm -lssh2 -lgmodule-2.0 -lglib-2.0 -lutil'

# Names of additional weak libraries provided by this library
weak_library_names=''

# Version information for libmc.
current=
age=
revision=

# Is this an already installed library?
installed=no

# Should we warn about portability when linking against -modules?
shouldnotlink=no

# Files to dlopen/dlpreopen
dlopen=''
dlpreopen=''

# Directory that this library needs to be installed in:
libdir=''

Note how lobtool builds libmc library:

  • it does not refer local static .la libraries as external entities. It just repackages .la files into .a files.
  • .a libraries are not referred in .la file at all. Those are just skipped.

Reading through https://www.gnu.org/software/libtool/manual/html_node/Inter_002dlibrary-dependencies.html it looks like libtool understands only *.la and -l<foo> arguments as library dependencies.

Thus one of the workarounds for users would be to rely on linker for shared libraries to link libintl into libmc.so: ./configure --with-included-gettext --enable-mclib.

Unfortunately .a files are not built in a PIC mode because they are not intended for linking into shared libraries:

$ ./configure --with-included-gettext --enable-mclib
...
  CCLD     libmc.la

*** Warning: Linking the shared library libmc.la against the
*** static library ../intl/libintl.a is not portable!
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: ../intl/libintl.a(dcigettext.o): relocation R_X86_64_PC32 against symbol `_nl_msg_cat_cntr' can not be used when making a shared object; recompile with -fPIC
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: final link failed: nonrepresentable section on output
collect2: error: ld returned 1 exit status

Thus attached patch is correct at least for --disable-mclib case. --enable-mclib requires even more work.

Changed 5 years ago by slyfox

0001-Ticket-3629-configure.ac-drop-bundled-gettext.patch

comment:13 Changed 5 years ago by slyfox

0001-Ticket-3629-configure.ac-drop-bundled-gettext.patch​ just drops bundled gettext.

comment:14 follow-up: ↓ 17 Changed 5 years ago by andrew_b

  CCLD     mc
/usr/bin/ld: cannot find -liconv
collect2: error: ld returned 1 exit status

comment:15 follow-up: ↓ 16 Changed 5 years ago by andrew_b

Linkage fail can be fixed using AM_ICONV:

  • configure.ac

    a b dnl ############################################################################ 
    272272dnl Internationalization 
    273273dnl ############################################################################ 
    274274 
     275AM_ICONV 
    275276AM_GNU_GETTEXT([external], [need-ngettext]) 
    276277AM_GNU_GETTEXT_VERSION([0.18.1]) 
    277278 

But anyway, i18n is broken. There no cyrillic in the TUI: question marks are shown instead of cyrillic letters.

comment:16 in reply to: ↑ 15 Changed 5 years ago by slyfox

Replying to andrew_b:

But anyway, i18n is broken. There no cyrillic in the TUI: question marks are shown instead of cyrillic letters.

It's not clear which ./configure options you are using thus I'm not sure how to reproduce. It's probably due to src/main.c's conditional:

#ifdef HAVE_SETLOCALE
    (void) setlocale (LC_ALL, "");
#endif

and HAVE_SETLOCALE is not set.

The fix would be to add a

AC_CHECK_FUNCS([setlocale])

in configure.ac.

Before mc implicitly relied on intl.m4 to detect the same.

comment:17 in reply to: ↑ 14 Changed 5 years ago by slyfox

Replying to andrew_b:

  CCLD     mc
/usr/bin/ld: cannot find -liconv
collect2: error: ld returned 1 exit status

Can you share your config.log? It does not happen for me. It feels like mc does not really use any iconv_* symbols directly and gettext.m4 should sort all out itself.

I wonder if the -liconv flag comes from $(LIBICONV) at

lib/Makefile.am:libmc_la_LIBADD += $(PCRE_LIBS) $(LIBICONV)

and should just be removed.

comment:18 Changed 5 years ago by andrew_b

Indeed,

AC_CHECK_FUNCS([setlocale])

fixes the cyrillic in the TUI and

libmc_la_LIBADD += $(PCRE_LIBS)

fixes the linkage error.

The patch on top on your one is following:

  • configure.ac

    diff --git a/configure.ac b/configure.ac
    index 79b63b39f..bd1f06b26 100644
    a b dnl ############################################################################ 
    272272dnl Internationalization 
    273273dnl ############################################################################ 
    274274 
     275AC_CHECK_FUNCS([setlocale]) 
     276 
    275277AM_GNU_GETTEXT([external], [need-ngettext]) 
    276278AM_GNU_GETTEXT_VERSION([0.18.1]) 
    277279 
  • lib/Makefile.am

    diff --git a/lib/Makefile.am b/lib/Makefile.am
    index 69c47601f..455f9ddf7 100644
    a b else 
    7474    libmc_la_LIBADD += $(GLIB_LIBS) 
    7575endif 
    7676 
    77 libmc_la_LIBADD += $(PCRE_LIBS) $(LIBICONV) 
     77libmc_la_LIBADD += $(PCRE_LIBS) 

It feels like mc does not really use any iconv_* symbols directly

Yes. mc uses g_iconv_* from GLib only.

comment:19 Changed 5 years ago by andrew_b

  • Owner changed from zaytsev to andrew_b
  • Branch state changed from no branch to on review

Branch: 3629_external_gettext
Initial changeset:6f6c1ca36e13e205302ae2ae1f1db842a52b84f7

comment:20 Changed 5 years ago by andrew_b

  • Votes for changeset set to andrew_b
  • Branch state changed from on review to approved

comment:21 Changed 5 years ago by andrew_b

  • Status changed from accepted to testing
  • Votes for changeset changed from andrew_b to committed-master
  • Resolution set to fixed
  • Branch state changed from approved to merged

comment:22 Changed 5 years ago by andrew_b

  • Status changed from testing to closed
Note: See TracTickets for help on using tickets.