Opened 3 years ago

Closed 18 months ago

#29000 closed defect (invalid)

make libs/readline know where headers/libs are

Reported by: dimpase Owned by:
Priority: critical Milestone: sage-duplicate/invalid/wontfix
Component: build: configure Keywords:
Cc: dimpase, dcoudert, isuruf, mjo Merged in:
Authors: Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: public/packages/readline_underlink (Commits, GitHub, GitLab) Commit: af7cec6bed75df362f8914df8990e298491c12b0
Dependencies: #32073 Stopgaps:

Status badges

Description (last modified by mkoeppe)

To allow non-standard locations for readline (e.g. with MacOS Homebrew) one should use # distutils: include_dirs = etc in sage/libs/readline.pxd to make it look in non-standard locations.

readline/spkg-configure.m4 should set up appropriate variables to be filled in and exported in src/bin/sage-env-config.in; in turn they should make it into cython_aliases in sage/env.py.

This came up in this thread on sage-devel.

See also (readline-related):

  • #29573 qepcad build fails on slackware-14.2-maximal
  • #29572 Fix perl_term_readline_gnu build error on conda-forge-macos-maximal

See also (similar failure mode):

Change History (33)

comment:1 Changed 3 years ago by gh-mwageringel

Also note that several packages seem to link against the macOS readline instead of the Homebrew one. In fact, the only one I am sure links against the Homebrew readline is rpy2. What would be needed to solve this? The Homebrew version provides a pkg-config file, in case that is useful.

I am hesitant to permanently install Homebrew's readline to /usr/local as this is against what is intended by Homebrew. In the case of readline though, I am not sure how much of a problem this really is.

comment:2 Changed 3 years ago by dimpase

GAP needs a "real" readline, not a MacOS's libedit.

IIRC, sagelib's readline also needs "real" readline.

I did try installing Homebrew's readline into /usr/local, it seems to cause no harm, and allowed builds to work on MacOS 10.13.

comment:3 in reply to:  2 Changed 3 years ago by gh-mwageringel

Replying to dimpase:

GAP needs a "real" readline, not a MacOS's libedit.

This might explain why I currently see several persistent doctest timeouts with this setup. Giac seems to be affected, as well. I am a bit surprised the build does not fail with this incompatible readline (other than for sage.libs.readline).

comment:4 Changed 3 years ago by gh-mwageringel

Description: modified (diff)

comment:5 Changed 3 years ago by mkoeppe

Thanks for adding the link to the problem description.

What is happening here is that you are relying on an automake feature that our build system (unfortunately) does not have.

It is true that one can set variables by passing them to configure, and they will be respected during the configure run and remembered in config.status. However, in our build system they are not saved in any configure-generated output file:

$ ./configure LDFLAGS="-L/xyzzy1" CPPFLAGS="-I/xyzzy2" PKG_CONFIG_PATH="/xyzzy3"
$ grep xyzzy build/make/Makefile-auto build/make/Makefile src/Makefile src/bin/sage-env-config build/bin/sage-build-env-config build/pkgs/sage_conf/src/sage_conf.py build/pkgs/sage_conf/src/setup.cfg

... except in the automake-generated file build/make/Makefile-auto, which is not used at all in our build process.

build/make/Makefile-auto:CPPFLAGS =  -I/xyzzy2
build/make/Makefile-auto:LDFLAGS = -L/xyzzy1
build/make/Makefile-auto:PKG_CONFIG_PATH = /xyzzy3

comment:6 Changed 3 years ago by gh-mwageringel

Thank you for explaining this. This clears up a lot. I think I will avoid the keg-only packages for now.

comment:7 Changed 3 years ago by mkoeppe

Also PARI's spkg-install.in hardcodes readline to come from SAGE_LOCAL

bash ./Configure --prefix="$SAGE_LOCAL" \
    --with-readline="$SAGE_LOCAL" $SAGE_CONFIGURE_GMP \
    --with-runtime-perl="/usr/bin/env perl" \
    --kernel=gmp $PARI_CONFIGURE 

which is not correct when readline/spkg-configure.m4 found a system readline.

On local-homebrew-macos-standard (#29417), this leads to:

Hi-Res Graphics: none
###
### Editline wrapper detected, building without readline support
###
### Building without GNU readline support

comment:8 Changed 3 years ago by dimpase

I believe these are missing for packages for which readline is not a dependency (but a feature). On the other hand, perhaps it can potentially pollute the compiling/linking calls.

Let's fix it...

comment:9 Changed 3 years ago by mkoeppe

Cc: dimpase dcoudert added
Priority: majorblocker

comment:10 Changed 3 years ago by mkoeppe

Cc: isuruf added

Another case:

  • #29572 Fix perl_term_readline_gnu build error on conda-forge-macos-maximal

comment:11 Changed 3 years ago by dimpase

Authors: Dima Pasechnik

comment:12 Changed 3 years ago by mkoeppe

Description: modified (diff)

comment:13 Changed 3 years ago by mkoeppe

Description: modified (diff)

comment:14 Changed 3 years ago by dimpase

It appears this will need AX_ABSOLUTE_HEADER, as we use for GMP/MPIR - I don't see another way around. :-(

comment:15 Changed 3 years ago by mkoeppe

Then it has to be.

comment:16 Changed 3 years ago by dimpase

underlinking of libreadline needs to be tested in readline's spkg-configure. And the name of the library has to be found from ac_cv_search_wresize set by AC_SEARCH_LIBS in ncurses' spkg-configure, or the result of pkg-config --libs ncurses (more precisely, the autoconf equivalent of the latter).

comment:17 Changed 3 years ago by isuruf

Or we should make sure CFLAGS and LDFLAGS are respected by all the spkgs

comment:18 Changed 3 years ago by mkoeppe

How's this coming along?

comment:19 Changed 3 years ago by dimpase

hope to fix today (SG time)

comment:20 Changed 3 years ago by mkoeppe

Anything I can help?

comment:21 Changed 3 years ago by dimpase

Branch: public/packages/readline_underlink
Commit: af7cec6bed75df362f8914df8990e298491c12b0

feel free to take over - I won't have much time for it in the coming 48 hours. The branch I pushed is only a start.


New commits:

af7cec6compute NCURSESLIBNAMES, swicth to dependence macro

comment:22 Changed 3 years ago by mkoeppe

Cc: mjo added

comment:23 Changed 3 years ago by mkoeppe

Hm, not tonight

comment:24 Changed 3 years ago by mkoeppe

Description: modified (diff)

comment:25 Changed 3 years ago by mkoeppe

Priority: blockercritical

comment:26 Changed 3 years ago by dimpase

I think slackware problem (#29573) is a different one - it ships an underlinked readline (and does not provide .pc file for readline, to make everything harder than needed).

comment:27 Changed 3 years ago by mkoeppe

Milestone: sage-9.1sage-9.2

comment:28 Changed 2 years ago by mkoeppe

Milestone: sage-9.2sage-9.3

comment:29 Changed 21 months ago by mkoeppe

Milestone: sage-9.3sage-9.4

Moving to 9.4, as 9.3 has been released.

comment:30 Changed 19 months ago by dimpase

should we proceed with this - in view of recent readline changes?

comment:31 Changed 19 months ago by mkoeppe

Authors: Dima Pasechnik
Dependencies: #32073
Milestone: sage-9.4sage-duplicate/invalid/wontfix
Status: newneeds_review

The issue in the ticket description is indeed no longer relevant after #32073, so let's close this.

There are still issues with readline in other packages, as mentioned in https://trac.sagemath.org/ticket/29000#comment:7 above, but it does not help to keep the ticket open.

comment:32 Changed 19 months ago by dimpase

Reviewers: Dima Pasechnik
Status: needs_reviewpositive_review

OK

comment:33 Changed 18 months ago by mkoeppe

Resolution: invalid
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.