readline configuration for shared libs w/o curses dependencies

[mbeachy]

BPO	1204
Nosy	@akuchling, @gpshead
Files	full.patch python-release25-readline.patch configure-readline-libs-64bit.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/gpshead'
closed_at = <Date 2008-09-07.19:26:45.474>
created_at = <Date 2007-09-25.19:34:08.015>
labels = ['deferred-blocker', 'build', 'expert-installation']
title = 'readline configuration for shared libs w/o curses dependencies'
updated_at = <Date 2008-09-07.19:26:45.473>
user = 'https://bugs.python.org/mbeachy'

bugs.python.org fields:

activity = <Date 2008-09-07.19:26:45.473>
actor = 'gregory.p.smith'
assignee = 'gregory.p.smith'
closed = True
closed_date = <Date 2008-09-07.19:26:45.474>
closer = 'gregory.p.smith'
components = ['Build', 'Installation']
creation = <Date 2007-09-25.19:34:08.015>
creator = 'mbeachy'
dependencies = []
files = ['8471', '11272', '11413']
hgrepos = []
issue_num = 1204
keywords = ['patch', '64bit']
message_count = 13.0
messages = ['56140', '60188', '62807', '72039', '72712', '72723', '72728', '72733', '72734', '72744', '72746', '72747', '72748']
nosy_count = 5.0
nosy_names = ['akuchling', 'gregory.p.smith', 'mbeachy', 'henry.precheur', 'rpetrov']
pr_nums = []
priority = 'deferred blocker'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'compile error'
url = 'https://bugs.python.org/issue1204'
versions = ['Python 2.5', 'Python 3.0']

[mbeachy]

For RHEL 3 (and it also appears RHEL 4 and 5) the libreadline shared lib
has no specified lib requirement that satisfies the tgetent and related
symbols. (These symbols are provided by ncursesw, ncurses, curses,
termcap as noted in the python setup.py.) The configure script does not
add these required libs in for the readline tests, and so the autoconf
tests will fail and it will incorrectly determine that readline is not
present (and so not define HAVE_RL_COMPLETION_MATCHES etc.)

I guess this generally does not prevent the readline module from being
compiled since setup.py does its own search for readline and adds in the
needed curses library. It does prevent proper declaration of the
completion_matches function, however. On 32 bit systems this doesn't
matter but on 64 bit ones it does as the undeclared (but present in
libreadline) completion_matches returns a char **.

The fix checked in with r54874 after the 2.5.1 release (bpo-1703270)
to Modules/readline.c fixes the problem for completion_matches
specifically, but the problem of incorrect determination of readline
presence still exists.

Attached is a patch to fix the problem: it adds the necessary additional
library to the temporary LIBS definition in the readline tests, using
the same order of curses libs specified in setup.py. (The patch includes
the changes for the configure script in addition to configure.in.)

[mbeachy]

Urgh. Re-reading this, I could barely understand what the hell I was saying.

The problem: 64 bit compiles will dump core when readline is used if
they don't properly identify the presence of readline and use the system
headers.

The solution: fix autoconf to properly recognize when readline is
present by providing configure's readline test program with all
necessary prerequisites.

caveat: The fix for bpo-1703270 (already checked in post-2.5.1) works
around this by hard coding the completion_matches prototype in
Modules/readline.c, but the autoconf fix seems better long term.

[akuchling]

The patch looks reasonable. I'll commit it on Monday when I'm on a
machine with a more up-to-date autoconf.

[rpetrov]

In the configure{.in} exist another bug:
--------------

AC_CHECK_LIB(readline, readline)
if test "$ac_cv_have_readline_readline" = no
then
  AC_CHECK_LIB(termcap, readline)
fi

---

but "grep _readline_readline configure" show that variable in use is
$ac_cv_lib_readline_readline, so the check for function termcap in
readline can be removed safely - it is never reached.

I would like to propose another patch that don't use possible unresolved
symbol to detect presence of library. The new patch will define
HAVE_LIBREADLINE. If necessary will check for dependent library
and link it in correct order. The script print messages like next:
------
checking how to link readline libs... -lreadline -lncursesw
checking for rl_callback_handler_install in -lreadline... yes
checking for rl_pre_input_hook in -lreadline... yes
checking for rl_completion_matches in -lreadline... yes
------

The patch is for branch release25-maint.
It is without changes in configure script, i.e autoconf has to run to
recreate it after applying.
The patch can be applied to trunk as well.

[gpshead]

when committing this one, the platform specific openbsd/amd64 fix I
committed for this in bpo-3645 should probably be verified as unneeded
and undone.

marking as release blocker as we should make sure this autoconf update
goes in before the 2.6/3.0 release. compiling for 64bit with readline
should work out of the box withing requiring patches on the most common
OSes.

I'm verifying rpetrov's patch in my centos4 VM now...

[gpshead]

I've attached an updated patch. Not much changed other than the better
diff -u format, a couple grammar errors in comments and inclusion of the
new autoconf generated configure script.

It works fine for me on ubuntu hardy 32bit, CentOS 5.x x86_64, MacOS X
10.5 32-bit and 64-bit.

Could someone confirm on a platform where linking with 64bit readline
was broken before that this fixes it. I added Henry Precheur to the
nosy list as he should be able to confirm on OpenBSD/amd64.

Lowering this to deferred blocker as it need not hold up -rc1 so long as
we get it in before -rc2.

[henryprecheur]

According to config.log the readline functions are correctly detected. I
tested the patch with Python 2.5 & 2.6 and both work with the test I
posted on bpo-3645.

[gpshead]

fixed in trunk r66283 (followed by rerunning autoconf to generate a new
configure script in r66284).

[gpshead]

merged into py3k branch in r66285.

backported to release25-maint in r66288 and r66289.

[rpetrov]

I realize too late that in my patch line "if test $py_cv_lib_readline =
!yes; then" is not correct. :(
It has to be "if test $py_cv_lib_readline = no; then", i.e. s/!yes/no/'.

[gpshead]

eek. not quite fixed then :)

i'll retest and take care of it.

[gpshead]

fixed in trunk r66295/r66296.

merging and backporting now...

[gpshead]

merged to py3k r66297 + r66298

backported to release25-maint r66299 + r66300.