Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make libs/readline know where headers/libs are #29000

Closed
dimpase opened this issue Jan 13, 2020 · 32 comments
Closed

make libs/readline know where headers/libs are #29000

dimpase opened this issue Jan 13, 2020 · 32 comments

Comments

@dimpase
Copy link
Member

dimpase commented Jan 13, 2020

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):

See also (similar failure mode):

Depends on #32073

CC: @dimpase @dcoudert @isuruf @orlitzky

Component: build: configure

Branch/Commit: public/packages/readline_underlink @ af7cec6

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/29000

@dimpase dimpase added this to the sage-9.1 milestone Jan 13, 2020
@mwageringel
Copy link

comment:1

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.

@dimpase
Copy link
Member Author

dimpase commented Jan 16, 2020

comment:2

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.

@mwageringel
Copy link

comment:3

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).

@mwageringel

This comment has been minimized.

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 21, 2020

comment:5

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

@mwageringel
Copy link

comment:6

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 2, 2020

comment:7

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

@dimpase
Copy link
Member Author

dimpase commented Apr 3, 2020

comment:8

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...

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 25, 2020

comment:10

Another case:

@dimpase
Copy link
Member Author

dimpase commented Apr 25, 2020

Author: Dima Pasechnik

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@dimpase
Copy link
Member Author

dimpase commented Apr 25, 2020

comment:14

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 25, 2020

comment:15

Then it has to be.

@dimpase
Copy link
Member Author

dimpase commented Apr 25, 2020

comment:16

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).

@isuruf
Copy link
Member

isuruf commented Apr 25, 2020

comment:17

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 26, 2020

comment:18

How's this coming along?

@dimpase
Copy link
Member Author

dimpase commented Apr 26, 2020

comment:19

hope to fix today (SG time)

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 27, 2020

comment:20

Anything I can help?

@dimpase
Copy link
Member Author

dimpase commented Apr 28, 2020

Branch: public/packages/readline_underlink

@dimpase
Copy link
Member Author

dimpase commented Apr 28, 2020

Commit: af7cec6

@dimpase
Copy link
Member Author

dimpase commented Apr 28, 2020

comment:21

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 28, 2020

comment:23

Hm, not tonight

@mkoeppe

This comment has been minimized.

@dimpase
Copy link
Member Author

dimpase commented May 1, 2020

comment:26

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).

@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 May 5, 2020
@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Oct 24, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented May 10, 2021

comment:29

Moving to 9.4, as 9.3 has been released.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 May 10, 2021
@dimpase
Copy link
Member Author

dimpase commented Jul 5, 2021

comment:30

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 5, 2021

Dependencies: #32073

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 5, 2021

comment:31

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 #29000 comment:7 above, but it does not help to keep the ticket open.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 5, 2021

Changed author from Dima Pasechnik to none

@mkoeppe mkoeppe removed this from the sage-9.4 milestone Jul 5, 2021
@dimpase
Copy link
Member Author

dimpase commented Jul 5, 2021

comment:32

OK

@dimpase
Copy link
Member Author

dimpase commented Jul 5, 2021

Reviewer: Dima Pasechnik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants