Repository navigation
enable compilation of readline module on Mac OS X 10.5 and 10.6 #51126
Description
| BPO | 6877 |
|---|---|
| Nosy | @freddrake, @warsaw, @brettcannon, @birkenfeld, @ronaldoussoren, @mdickinson, @benjaminp, @ned-deily, @zvezdan, @meadori |
| Files |
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/ronaldoussoren'
closed_at = <Date 2010-02-11.13:16:42.701>
created_at = <Date 2009-09-10.12:38:28.291>
labels = ['build', 'release-blocker']
title = 'enable compilation of readline module on Mac OS X 10.5 and 10.6'
updated_at = <Date 2011-10-28.20:57:54.717>
user = 'https://github.com/zvezdan'bugs.python.org fields:
activity = <Date 2011-10-28.20:57:54.717>
actor = 'ned.deily'
assignee = 'ronaldoussoren'
closed = True
closed_date = <Date 2010-02-11.13:16:42.701>
closer = 'ronaldoussoren'
components = ['Build']
creation = <Date 2009-09-10.12:38:28.291>
creator = 'zvezdan'
dependencies = []
files = ['14871', '14895', '14899', '14938', '16136', '16159', '16162', '16163']
hgrepos = []
issue_num = 6877
keywords = ['patch', '26backport', 'needs review']
message_count = 44.0
messages = ['92480', '92481', '92483', '92487', '92489', '92654', '92694', '92747', '92750', '92894', '92895', '92968', '92972', '92988', '92989', '92991', '92996', '92998', '93001', '93015', '93018', '98818', '98819', '98858', '98859', '98904', '98908', '98912', '98913', '98942', '98953', '98977', '98978', '98979', '98980', '98987', '99011', '99012', '99019', '99031', '99204', '99207', '100464', '100697']
nosy_count = 12.0
nosy_names = ['fdrake', 'barry', 'brett.cannon', 'georg.brandl', 'ronaldoussoren', 'mark.dickinson', 'benjamin.peterson', 'ned.deily', 'zvezdan', 'srid', 'meador.inge', 'Alexander.Belopolsky']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'compile error'
url = 'https://bugs.python.org/issue6877'
versions = ['Python 2.6', 'Python 3.1', 'Python 2.7', 'Python 3.2']
Activity
zvezdan commentedon Sep 10, 2009
The attached patch enables compilation and use of the readline module on
Mac OS X 10.5 (Leopard) and 10.6 (Snow Leopard).
It utilizes the native editline library (used for emulation of the
readline library on Mac).
I used the patch for almost two years already with Python 2.4 and since
December 2008 with Python 2.6. The only difference is that Python 2.4
did not need the setup.py changes.
The patch is written in such a way that it does *not* affect the
compilation on systems that use GNU readline library (e.g., Linux).
However, I don't have access to any other system that uses editline
emulation of readline library besides Mac. I believe it should work the
same but some testing would be welcome if anyone is aware of such a
system (NetBSD?).
With the readline module compiled in, it is enough to put in .editrc
and one gets a vi emulation in the python interactive interpreter.
You can also try it directly from the shell:
It would be nice if we could get this included into Python-2.6.3
release.
zvezdan commentedon Sep 10, 2009
Changed type to "crash" because compilation of readline module without
this patch was causing Python to crash with BusError.
zvezdan commentedon Sep 10, 2009
When testing the patch make sure that your readline module has been
actually linked against editline library rather then some copy of GNU
readline from MacPorts or Fink.
Assuming --prefix=${HOME}/opt, the output of
should contain /usr/lib/libedit.2.dylib.
ronaldoussoren commentedon Sep 10, 2009
I'm +1 on merging this functionality.
See also: bpo-6872
As I mentioned there we should ensure that readline linked to libedit
has the same semantics as readline linked to GNU readline, and because
the configuration file of libedit has a different format as the one of
readline we should mention that in the documentation as well.
It would also be nice if one could programmaticly detect if readline is
linked to libedit, that way tools like ipython can load the right
configuration files without user interaction.
BTW. I'm pretty sure that the readline emultation on Leopard was pretty
broken, ipython used to cause hard crashes with /usr/bin/python on
Leopard.
zvezdan commentedon Sep 10, 2009
There's this rl_library_version constant defined in editline/readline C
libraries that the attached patch uses.
Perhaps, if we can expose its value from the Python readline module, one
could check whether the value startswith("EditLine wrapper") in ipython
and similar programs.
Regarding your comment about editline being broken on Leopard: The
patch worked just fine for me and other people who used it in my
company. We used line editing and history in interactive interpreter
with both Python 2.4 and Python 2.6 on Leopard.
One person used it with Python 2.4 and ipython. He did not have a
readline functionality until he compiled with the patch. After applying
the patch the line editing, history and TAB completion worked for him in
ipython. He's now running Snow Leopard so we couldn't check one more
time.
Perhaps you meant it was broken on Tiger (10.4; Darwin 8).
ronaldoussoren commentedon Sep 15, 2009
The attached patch is a slightly cleaner version of your patch. What I
don't like is that I'm also using runtime detection of libedit, I'd
prefer compile-time detection but that doesn't seem possible without
doing that dtection in setup.py.
My changes w.r.t. your patch:
work with other libedit's (such as the one in NetBSD) as well,
but I cannot test that and don't want to risk breaking other
platforms.
The patch seems to work on fine, including browsing in ipython's history
on 10.6.
PS. I mentioned ipython because I know there were issues with ipython
and Apple's python when Leopard was just out. I don't use ipython myself
and hence don't know if they have added workarounds in their code (or if
Apple fixed the issues the ran into)
Marking this issue as 'needs review' for two reasons:
zvezdan commentedon Sep 16, 2009
The patch
readline-libedit.patchhas the following problems:It fails to compile
readline.c. ::function)
Modules/readline.c:1059:1: error: unterminated #ifdef
The fixed patch is attached as
readline-libedit-2.patchabove.Please review it.
There are also a few minor stylistic nits I took a liberty to fix.
In readline.c
=============
Line 496
Formatted a block comment to a usual C style (used by other comments
it the patch, FWIW).
Line 505
The standard style in C (and C-like) languages is to *not* write a
space between a variable and a unary operator (e.g., "C++").
Changed it to::
Lines 1085/86
Removed two unnecessary empty lines added by the first patch.
There's already one empty line in the original code to space an if
statement from the next line.
Line 1070
Line was longer than 79 characters (violates PEP-7). Reformatted.
In setup.py
===========
Line 561
A comment block was indented in the first patch and had lines longer
than 79 characters (violates PEP-8). I refactored the code to avoid
repeating the same else statement twice. This obviated the need to
indent the said comment block.
Just out of curiosity: does anyone know why the first if statement that
tests for 'darwin' uses
platformand the second one usessys.platform? Couldn't we useplatformin both of them?mdickinson commentedon Sep 17, 2009
Do the remove_history_item and replace_history_item functions also need
the off-by-one adjustment?
ronaldoussoren commentedon Sep 17, 2009
Mark: yes those functions need to be changed as well.
ronaldoussoren commentedon Sep 20, 2009
Mark: it turns out that GNU readline has a rather odd interface, only
the index of get_history_item is 1-based, all others are 0-based. This
is not mentioned in the documentation (neither that of the readline
module or the GNU documentation).
I've attached a very minimal testcase for the history manipulation
functions in the readline module that passes with GNU readline.
The tests now also pass using libedit emulation (after fixing the
compilation issues noted by Zvezdan).
I'll file a new issue about the readline documentation, that's unrelated
to getting libedit support into the repository.
ronaldoussoren commentedon Sep 20, 2009
Committed my latest version of the patch as r74970 (trunk) and r74971
(3.2)
Barry: what's your opinion on a backport of this to 2.6?
26 remaining items
zvezdan commentedon Feb 7, 2010
OK, so bpo-4204 patch is causing problems.
As I mentioned before we could choose to #ifdef __FreeBSD__ or check whether __APPLE__ is not defined.
I'm attaching a new patch for 2.6.5 (2.6 branch) that uses the __APPLE__ check.
zvezdan commentedon Feb 7, 2010
The patch with the same correction against trunk is attached so that Meador can test it.
zvezdan commentedon Feb 7, 2010
Meador, I just looked at your patch (it seems we were posting patches at about the same time).
That's a good fix too.
Barry it's your call.
ronaldoussoren commentedon Feb 7, 2010
re: msg98913
The addition of an #include of stdlib.h is unnecessary because Python.h already does that.
re: msg98908
Building against with a 10.3 deployment target should be harmless. A universal build targets 10.3 on purpose because that results in binaries that work on all supported OSX platforms.
BTW. I've pruned the list of python versions, this patch will never be applied to 2.4.
BTW2. I'll do the backport later today.
warsaw commentedon Feb 7, 2010
Zvezdan, your "fix1" patch works perfectly for me in Python 2.6 on both OS X 10.5 and 10.6. Great work!
I approve applying this patch for Python 2.6.5. Ronald, would you like to do the honors?
ronaldoussoren commentedon Feb 7, 2010
I have a question about this bit of the patch:
That bit is not in the trunk, should it be forward ported to the trunk?
The actual prototype on 10.5 and 10.6 is:
Wouldn't it be better to change the prototype in readline.c to match?
As that's not a critical change I've committed the fix1 patch as is in r78096.
meadori commentedon Feb 7, 2010
FWIW, I would really like to have it.
I may have missed something, but the patch is actually *excluding* the
prototype on OS X, i.e. #if *not* defined. Thus the 'const char*' prototype
that comes in with 'readline.h' is the one that will be used.
On the other hand, if you are proposing to make them 'match' just to avoid
the redefinition conflict, then this may break on other systems where 'char
*' is used.
Regards,
-- Meador
zvezdan commentedon Feb 7, 2010
Ronald,
Yes, that should be applied to trunk and 3.x to make it work on Mac OS X 10.5 (Leopard). I indicated that in msg98979.
The explanation why that part is needed is given in msg98978 and msg98913.
That ultimately points to re-declaration of completion_matches() inserted in a patch in bpo-4204 (see my comments in the last two messages there).
We must *avoid* the duplicate declaration on Mac OS X, because the same declaration is already in /usr/include/readline/readline.h.
Hence #if !defined(APPLE) is used.
So, the readline-libedit-trunk.patch I attached yesterday should be applied to trunk (2.7) and 3.x.
It seems that you forgot to
before committing.
If you run svn status on that checkout I bet it will show with '?' in the left column.
P.S.
FWIW, you are right about #include <stdlib.h>. I removed it from the patch yesterday (notice attachment, removal, and a new attachment in the history).
ronaldoussoren commentedon Feb 11, 2010
I've added the tests to the 2.6 branch and have ported the #ifdef guard around the prototype for completion_matches to the trunk and 3.2.
I'm therefore closing the issue.
meadori commentedon Feb 11, 2010
On Thu, Feb 11, 2010 at 7:16 AM, Ronald Oussoren <report@bugs.python.org>wrote:
Verified in trunk. Thanks Ronald!
ned-deily commentedon Mar 5, 2010
See bpo-8066 for additional patches to correct problems when building on 10.5 or 10.6 for deployment targets of 10.4 or earlier.
ned-deily commentedon Mar 9, 2010
Also note that this feature has not been backported to 3.1 which means it will not be in the upcoming 3.1.2 release. (It will not be an issue for the python.org 3.1.2 installer which will be built with GNU readline as usual to support older systems.)