Opened 2 years ago
Closed 22 months ago
#27277 closed enhancement (duplicate)
spkg-configure.m4 for ncurses and readline
Reported by: | dimpase | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | build | Keywords: | spkg-configure |
Cc: | embray, fbissey | Merged in: | |
Authors: | Dima Pasechnik | Reviewers: | Matthias Koeppe, François Bissey |
Report Upstream: | Reported upstream. No feedback yet. | Work issues: | |
Branch: | u/dimpase/packages/ncurses_readline-config (Commits, GitHub, GitLab) | Commit: | 6828f7ad4fc5533ba7987f4806440b90f9a08856 |
Dependencies: | Stopgaps: |
Description (last modified by )
use pkg-config and test that the versions are as new as in Sage:
SAGE_SPKG_CONFIGURE([ncurses], [ dnl First try checking for ncurses with pkg-config PKG_CHECK_MODULES([NCURSES], [ncurses >= 6.0], [sage_spkg_install_ncurses=no], [sage_spkg_install_ncurses=yes]) ])
and
SAGE_SPKG_CONFIGURE([readline], [ dnl First try checking for readline with pkg-config PKG_CHECK_MODULES([READLINE], [readline >= 6.3], [sage_spkg_install_readline=no], [sage_spkg_install_readline=yes]) ])
readline 8.0 tarball: ftp://ftp.cwru.edu/pub/bash/readline-8.0.tar.gz
Change History (27)
comment:1 Changed 2 years ago by
- Milestone changed from sage-8.7 to sage-8.8
comment:2 Changed 2 years ago by
test for use_tioctl to recognise ncurses version 6.
comment:3 Changed 2 years ago by
? Do we even need that if we can use PKG_CHECK_MODULES? Or do you mean for OSX?
For the time being I don't even really care that much about this working on OSX but if you or someone else wants to make it work I certainly won't complain as long as it doesn't make things enormously more complex.
comment:4 Changed 2 years ago by
- Branch set to u/dimpase/packages/ncurses-config
- Commit set to 3dbdddff063fe2743f69d10ca5e2a9b94623c107
New commits:
3dbdddf | spkg-configure for ncurses, request version >= 5.7
|
comment:5 Changed 2 years ago by
macos native ncurses is OK, but readline is a faked one, libedit, with many functions from readline missing. Homebrew provides readline 8.0 linked with native macos ncurses. So we can try to replicate this.
comment:6 Changed 2 years ago by
- Commit changed from 3dbdddff063fe2743f69d10ca5e2a9b94623c107 to 33a1ae4735abd402ae204018ecc2be7cceaf5d36
comment:7 Changed 2 years ago by
- Description modified (diff)
comment:8 Changed 2 years ago by
- Status changed from new to needs_review
OK, so this works on OSX - with native ncurses and readline built by Sage.
comment:9 Changed 2 years ago by
- Status changed from needs_review to needs_work
Perhaps the readline version bump is not needed.
On Fedora I get this error building python2
gcc -pthread -shared -L. -L/home/scratch2/dimpase/sage/sage/local/lib -Wl,-rpath,/home/scratch2/dimpase/sage/sage/local/lib -L. -L/home/scratch2/dimpase/sage/sage/local/lib -Wl,-rpath,/home/scratch2/dimpase/sage/sage/local/lib build/temp.linux-x86_64-2.7/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/python2-2.7.15.p1/src/Modules/readline.o -L/usr/lib/termcap -L/home/scratch2/dimpase/sage/sage/local/lib -L/usr/local/lib -L. -lreadline -lpython2.7 -o build/lib.linux-x86_64-2.7/readline.so *** WARNING: renaming "readline" since importing it failed: /home/scratch2/dimpase/sage/sage/local/lib/libreadline.so.8: undefined symbol: UP
comment:10 Changed 2 years ago by
- Cc fbissey added
- Report Upstream changed from N/A to Not yet reported upstream; Will do shortly.
a patch related to libtinfo
is still needed. E.g.
--- a/support/shobj-conf +++ b/support/shobj-conf @@ -130,6 +130,7 @@ linux*-*|gnu*-*|k*bsd*-gnu-*|freebsd*-gentoo) SHLIB_XLDFLAGS='-Wl,-rpath,$(libdir) -Wl,-soname,`basename $@ $(SHLIB_MINOR)`' SHLIB_LIBVERSION='$(SHLIB_LIBSUFF).$(SHLIB_MAJOR)$(SHLIB_MINOR)' + SHLIB_LIBS='-ltinfo' ;; freebsd2*)
fixes the build on Fedora.
Actually, I have spotted the bug (2, in fact) in the readline source.
- The first is in the macro BASH_CHECK_LIB_TERMCAP in readline's
aclocal.m4
(this is not an autogenerated file, it's hand-written). If--with-curses
is given to./configure
then it still first checkslibtermcap
, and if it's installed on the system thenlibtinfo
is skipped. After fixing this, one sees-ltinfo
inTERMCAP_LIB
variable, as it should be.
- It's not enough to fix the above, as
TERMCAP_LIB
is only used on cygwin/mingw, for a reason I fail to comprehend. Setting SHLIB_LIBS to'$(TERMCAP_LIB)'
unconditionally seems to be the right thing.
The thing works on OSX as there there is an unconditional SHLIB_LIBS='-lncurses'
.
Should this be reported upstream?
comment:11 follow-up: ↓ 13 Changed 2 years ago by
It feels like an upstream work around mostly but the stuff on fedora is probably worth reporting. I very much doubt it is fedora specific. Looking at the gentoo ebuild we have that telling section for readline 8.0
# Force ncurses linking. #71420 # Use pkg-config to get the right values. #457558 local ncurses_libs=$($(tc-getPKG_CONFIG) ncurses --libs) sed -i \ -e "/^SHLIB_LIBS=/s:=.*:='${ncurses_libs}':" \ support/shobj-conf || die sed -i \ -e "/^[[:space:]]*LIBS=.-lncurses/s:-lncurses:${ncurses_libs}:" \ examples/rlfe/configure || die # fix building under Gentoo/FreeBSD; upstream FreeBSD deprecated # objformat for years, so we don't want to rely on that. sed -i -e '/objformat/s:if .*; then:if true; then:' support/shobj-conf || die ln -s ../.. examples/rlfe/readline || die # for local readline headers
comment:12 Changed 2 years ago by
No, I really don't see how the upstream stuff can work correctly---of course one can like an idiot insert -ltinfo
everywhere -lreadline
appears, but this does not make it right.
comment:13 in reply to: ↑ 11 Changed 2 years ago by
- Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.
Replying to fbissey:
It feels like an upstream work around mostly but the stuff on fedora is probably worth reporting. I very much doubt it is fedora specific.
No, it's readline's "not overlinking" (I'd rather call it "blatant underlinking) policy, plus "always try termcap first", see http://lists.gnu.org/archive/html/bug-readline/2014-04/msg00024.html (underlinking is very bad IMHO, it potentially means that a wrong termcap lib is used, resulting in all sorts of hiccups).
I just sent a followup there, but I don't have much hope for accepting this as bugs on upstream side.
comment:14 Changed 2 years ago by
comment:15 Changed 2 years ago by
please support me arguing against underlinking here: http://lists.gnu.org/archive/html/bug-readline/2019-04/msg00013.html :-)
comment:16 Changed 2 years ago by
And with readline version 4 GAP doesn't build:
ld: error: undefined symbol: rl_bind_keyseq >>> referenced by sysfiles.c:2343 (src/sysfiles.c:2343) >>> obj/src/.libs/sysfiles.o:(initreadline)
So we need AC_SEARCH_LIBS
on rl_bind_keyseq
.
comment:17 Changed 2 years ago by
- Branch changed from u/dimpase/packages/ncurses-config to u/dimpase/packages/ncurses_readline-config
- Commit changed from 33a1ae4735abd402ae204018ecc2be7cceaf5d36 to 79e37a0e4c53ab24dd33a8f8f5ddc4e8c82984ce
- Status changed from needs_work to needs_review
I'd like to postpone update to readline 8, see if we can do with what we have now.
New commits:
8d4157b | spkg-configure for ncurses, request version >= 5.7
|
7b78256 | readline try on osx
|
2b3a5c7 | new readline 8.0 and checks in its spkg-configure
|
228bba0 | Revert "new readline 8.0 and checks in its spkg-configure"
|
79e37a0 | test for rl_bind_keyseq to detect old or fake readlines
|
comment:18 Changed 2 years ago by
- Status changed from needs_review to needs_work
oops,now I recall that I forgot to update readline's patches, as a result it won't build with external ncurses (unless there is a library named libtinfo
).
comment:19 Changed 2 years ago by
- Commit changed from 79e37a0e4c53ab24dd33a8f8f5ddc4e8c82984ce to 6828f7ad4fc5533ba7987f4806440b90f9a08856
Branch pushed to git repo; I updated commit sha1. New commits:
6828f7a | get termcap library name from the readline's configure
|
comment:20 Changed 2 years ago by
- Status changed from needs_work to needs_review
OK, this works now, external (n)curses or not.
comment:21 Changed 2 years ago by
- Reviewers set to Matthias Koeppe
Works well on macOS Mojave: ncurses is not built, readline is built.
comment:22 Changed 2 years ago by
ping?
comment:23 Changed 2 years ago by
- Reviewers changed from Matthias Koeppe to Matthias Koeppe, François Bissey
- Status changed from needs_review to positive_review
LGTM
comment:24 follow-up: ↓ 25 Changed 2 years ago by
- Status changed from positive_review to needs_review
I still haven't had a chance to test this one.
comment:25 in reply to: ↑ 24 Changed 2 years ago by
comment:26 Changed 23 months ago by
- Milestone changed from sage-8.8 to sage-duplicate/invalid/wontfix
- Status changed from needs_review to positive_review
changing the milestone to concentrate the rest of this work on #27825
comment:27 Changed 22 months ago by
- Resolution set to duplicate
- Status changed from positive_review to closed
Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)