Bug 211850
|
Description
lightside
2016-08-14 23:37:27 UTC
Looks good, awaiting maintainer's feedback While here, is the patch under files/ really needed any longer? Seems like it can be upstreamed or removed. Also I'd suggest to add MAKE_ARGS= V=1 to verbosify build. Otherwise looks good and builds fine, awaiting maintainer's feedback. Created attachment 173823 [details] Proposed patch (since 412346 revision) (In reply to comment #2) > While here, is the patch under files/ really needed any longer? The files/patch-config.make.in applies still. It was added in ports r353967. The purpose of it was explained in bug 189539 comment 0: > - Fix display wide character problem that cause by readline, Change LDLIBS > order to resolve. I reproduced this. For example, try to build devel/tig without mentioned patch. Run tig for some repository and press "SHIFT + :", then try to type UTF-8 (non-english) character(s). It will display correct character(s) with patch. (In reply to comment #2) > Also I'd suggest to add MAKE_ARGS= V=1 to verbosify build. Ok, added. (In reply to lightside from comment #3) > > While here, is the patch under files/ really needed any longer? > The files/patch-config.make.in applies still. It was added in ports r353967. > The purpose of it was explained in bug 189539 comment 0: > > - Fix display wide character problem that cause by readline, Change LDLIBS > > order to resolve. > I reproduced this. For example, try to build devel/tig without mentioned > patch. Run tig for some repository and press "SHIFT + :", then try to type > UTF-8 (non-english) character(s). It will display correct character(s) with > patch. OK then, leaving the patch there. I'd prefer this investigated further though and, if possible, fixed/upstreamed. This, for instance, smells fishy: /usr/local/bin/tig: libncursesw.so.8 => /lib/libncursesw.so.8 (0x80085d000) libreadline.so.6 => /usr/local/lib/libreadline.so.6 (0x800ab9000) libncurses.so.8 => /lib/libncurses.so.8 (0x800d05000) libc.so.7 => /lib/libc.so.7 (0x800f57000) libncurses comes from libreadline, but I'm not sure why symbols from it are picked up, as we have disabled recursive linking. Created attachment 173839 [details] Proposed patch (since 412346 revision) with bash_cv_termcap_lib=libncursesw (In reply to comment #2) > OK then, leaving the patch there. The proposed method in ports r353967 is similar to https://github.com/jonas/tig/blob/tig-2.2/contrib/config.make, which used as example in https://github.com/jonas/tig/blob/tig-2.2/INSTALL.adoc#build-configuration, where "-lncursesw" added before "-lreadline" for LDLIBS variable. (In reply to comment #4) > libncurses comes from libreadline, but I'm not sure why symbols from it are > picked up, as we have disabled recursive linking. This issue somehow related to termcap library, which linked, but not used, according to ldd output. The tools/ax_lib_readline.m4 script, which included in generated configure file, selects bash_cv_termcap_lib=libtermcap first: https://github.com/jonas/tig/blob/tig-2.2/tools/ax_lib_readline.m4#L36 % readelf -Ds /usr/lib/libtermcap.so | grep tgetent 214 331: 0000000000015590 1415 FUNC GLOBAL DEFAULT 11 tgetent If explicitly define bash_cv_termcap_lib variable to libncursesw value: % readelf -Ds /usr/lib/libncursesw.so | grep tgetent 377 331: 00000000000177b0 1415 FUNC GLOBAL DEFAULT 11 tgetent then this issue may be fixed: % make build && ldd -a work/tig-2.2/src/tig | sed -e '1d ; s| (.*)$||' libreadline.so.6 => /usr/local/lib/libreadline.so.6 libncursesw.so.8 => /lib/libncursesw.so.8 libc.so.7 => /lib/libc.so.7 /usr/local/lib/libreadline.so.6: libncurses.so.8 => /lib/libncurses.so.8 libc.so.7 => /lib/libc.so.7 /lib/libncursesw.so.8: libc.so.7 => /lib/libc.so.7 /lib/libncurses.so.8: libc.so.7 => /lib/libc.so.7 Created attachment 173840 [details] Proposed patch (since 412346 revision) with bash_cv_termcap_lib=libc The same effect (as in comment #5) with bash_cv_termcap_lib=libc, which removes second "-lncursesw" link on FreeBSD, but libc.so didn't have tgetent function on first check. Created attachment 173841 [details]
Proposed patch (since 412346 revision) with LIBS=-lncursesw
Also possible to use LIBS=-lncursesw.
I attached many variants to check.
(In reply to comment #5) > % make build && ldd -a work/tig-2.2/src/tig | sed -e '1d ; s| (.*)$||' Output for attachment #173840 [details] in comment #6: libreadline.so.6 => /usr/local/lib/libreadline.so.6 libncursesw.so.8 => /lib/libncursesw.so.8 libc.so.7 => /lib/libc.so.7 /usr/local/lib/libreadline.so.6: libncurses.so.8 => /lib/libncurses.so.8 libc.so.7 => /lib/libc.so.7 /lib/libncursesw.so.8: libc.so.7 => /lib/libc.so.7 /lib/libncurses.so.8: libc.so.7 => /lib/libc.so.7 Output for attachment #173841 [details] in comment #7: libncursesw.so.8 => /lib/libncursesw.so.8 libreadline.so.6 => /usr/local/lib/libreadline.so.6 libc.so.7 => /lib/libc.so.7 /lib/libncursesw.so.8: libc.so.7 => /lib/libc.so.7 /usr/local/lib/libreadline.so.6: libncurses.so.8 => /lib/libncurses.so.8 libc.so.7 => /lib/libc.so.7 /lib/libncurses.so.8: libc.so.7 => /lib/libc.so.7 (In reply to comment #5) > This issue somehow related to termcap library This is true, because libtermcap.so is a link to libncurses.so on FreeBSD. For example, on FreeBSD 10.2: % readlink /usr/lib/libtermcap.so libncurses.so % sha256 /usr/lib/libtermcap.so SHA256 (/usr/lib/libtermcap.so) = 39fd11d3e0fab753e51848bacad0eef4b84567f01f737b0f2663bae71124b2ed % sha256 /usr/lib/libncurses.so SHA256 (/usr/lib/libncurses.so) = 39fd11d3e0fab753e51848bacad0eef4b84567f01f737b0f2663bae71124b2ed (In reply to comment #5) > which linked, but not used, according to ldd output. this is why it was linked and used, but with different library name (libncurses.so.8). Comment on attachment 173840 [details] Proposed patch (since 412346 revision) with bash_cv_termcap_lib=libc There is bash_cv_termcap_lib=libc method, which does not add selected termcap library for linking: https://github.com/jonas/tig/blob/tig-2.2/tools/ax_lib_readline.m4#L67 I set maintainer-approval request for it, currently. Other methods are almost equal on effect. Created attachment 173856 [details] Proposed patch (since 412346 revision) with bash_cv_termcap_lib=libc Looks like, the tgetent function is not available in libc.so or libncurses*.so libraries on DragonFlyBSD. It's available in tinfo library: https://github.com/jonas/tig/blob/tig-2.2/tools/ax_lib_readline.m4#L37 % readelf -s /usr/local/lib/libtinfo.so | grep -w tgetent 132: 0000000000014880 21 FUNC GLOBAL DEFAULT 11 tgetent This may require to restrict bash_cv_termcap_lib=libc method to FreeBSD only. Created attachment 173857 [details]
Proposed patch (since 412346 revision) with readline stub
Possible to add readline stub for tools/ax_lib_readline.m4 script. The configure file will be regenerated in this case.
Comment on attachment 173857 [details] Proposed patch (since 412346 revision) with readline stub (In reply to comment #12) > Possible to add readline stub for tools/ax_lib_readline.m4 script. No, the implementation is wrong. Created attachment 173858 [details]
Proposed patch (since 412346 revision) with reordering of termcap's library checks
Possible to reorder termcap's library checks for tools/ax_lib_readline.m4 script. Trying to check *curses* libraries first, based on previous detection of $ax_cv_curses_which.
The used patch is possible candidate to propose for upstream.
Created attachment 173860 [details]
Proposed patch (since 412346 revision) with reordering of termcap's library checks
Some fixes.
(In reply to comment #14) > The used patch is possible candidate to propose for upstream. There is related (committed) pull request, which fixed similar issue for Mac OS X: https://github.com/jonas/tig/pull/268 Created attachment 173868 [details]
Proposed patch (since 412346 revision) with reordering of termcap's library checks
Other attachments were obsoleted to not create a possible ambiguity. Possible to return concrete attachment or propose other variant. A commit references this bug: Author: amdmi3 Date: Wed Oct 5 19:57:03 UTC 2016 New revision: 423383 URL: https://svnweb.freebsd.org/changeset/ports/423383 Log: - Update to 2.2 PR: 211850 Submitted by: lightside@gmx.com Approved by: maintainer timeout (darcsis@gmail.com, 6 weeks) Changes: head/devel/tig/Makefile head/devel/tig/distinfo head/devel/tig/files/patch-config.make.in head/devel/tig/files/patch-src_ui.c head/devel/tig/files/patch-tools_ax__lib__readline.m4 |
