Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

fallback on lsof if otool not found in libedit detection #525

Merged
merged 2 commits into from

4 participants

@minrk
Owner

This is necessary for Macs without XCode.

It also prevents a crash in the unlikely event that neither is found.

lsof detection is vulnerable to false positives if other (non-readline)
modules are linked against libedit and have been imported prior to
the first import of rlineimpl, but this is also unlikely.

closes #524

@minrk minrk fallback on lsof if otool not found in libedit detection
This is necessary for Macs without XCode.
It also prevents a crash in the unlikely event that neither is found.
lsof detection is vulnerable to false positives if other (non-readline)
modules are linked against libedit, but this is also unlikely.

closes gh-524
f1278cb
@fperez
Owner

I don't have a mac to test on, but I did read it fairly carefully, and it looks clean to me. Thanks!

@OliDa

line 48, not compatible with python3, prefer parenthesis :
print ('Failed GetOutputFile')

IPython/utils/rlineimpl.py
((31 lines not shown))
warnings.warn("libedit detection failed: %s"%err)
break
else:
break
- if p.returncode == 0 and re.search(r'/libedit[\.\d+]*\.dylib\s', otool):
+ if p is not None and p.returncode == 0 and re.search(r'/libedit[\.\d+]*\.dylib\s', out):
@OliDa
OliDa added a note

end of line : out is not decoded so re.search() fails.
=> could you check this (seems to work here):
if p is not None and p.returncode == 0 and re.search(r'/libedit[.\d+]*.dylib\s', out.decode()):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@takluyver
Owner

@OliDa: Note that this is the version to run on Python 2. It will get run through 2to3 to make the version for Python 3.

@OliDa

ok, right.
should I write something here https://github.com/ipython/ipython-py3k/blob/master/IPython/utils/rlineimpl.py
in the meantime ?

@takluyver
Owner

If you could check that this works with Python 2, we'll get it merged in to master here, then I'll transfer the changes across to the Python 3 port. The aim is for the difference between the two to be as small as possible.

@minrk minrk use a bytes regex to check for libedit
this eases the work of 2to3, which doesn't catch that `out` is bytes.

Also use warnings.warn for 'Failed GetOutputFile' message instead of print.
ec698f9
@minrk
Owner

@takluyver, it looks like 2to3 doesn't handle the regex str->bytes fix, so I made it a bytes pattern like you have. This still works fine on 2.6.

And the print statement should really go through warnings.warn anyway, so I fixed that too.

@OliDa

thanks guys :-)
@minrk: it's ok here.

@minrk
Owner

Okay, I'll go ahead and merge, then.

Thanks!

@minrk minrk merged commit ec698f9 into from
@takluyver
Owner

And I've updated ipython-py3k: ipython/ipython-py3k@340fc1e

@damianavila damianavila referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 18, 2011
  1. @minrk

    fallback on lsof if otool not found in libedit detection

    minrk authored
    This is necessary for Macs without XCode.
    It also prevents a crash in the unlikely event that neither is found.
    lsof detection is vulnerable to false positives if other (non-readline)
    modules are linked against libedit, but this is also unlikely.
    
    closes gh-524
Commits on Jun 19, 2011
  1. @minrk

    use a bytes regex to check for libedit

    minrk authored
    this eases the work of 2to3, which doesn't catch that `out` is bytes.
    
    Also use warnings.warn for 'Failed GetOutputFile' message instead of print.
This page is out of date. Refresh to see the latest.
Showing with 23 additions and 6 deletions.
  1. +23 −6 IPython/utils/rlineimpl.py
View
29 IPython/utils/rlineimpl.py
@@ -9,6 +9,7 @@
boolean and _outputfile variable used in IPython.utils.
"""
+import os
import re
import sys
import time
@@ -44,7 +45,7 @@ def add_history(line):
try:
_outputfile=_rl.GetOutputFile()
except AttributeError:
- print "Failed GetOutputFile"
+ warnings.warn("Failed GetOutputFile")
have_readline = False
# Test to see if libedit is being used instead of GNU readline.
@@ -56,21 +57,36 @@ def add_history(line):
# seems to make this better behaved in environments such as PyQt and gdb
dt = 1e-3
while dt < 1:
- p = Popen(['otool', '-L', _rl.__file__], stdout=PIPE, stderr=PIPE)
- otool,err = p.communicate()
+ try:
+ p = Popen(['otool', '-L', _rl.__file__], stdout=PIPE, stderr=PIPE)
+ except OSError:
+ try:
+ # otool not available (no XCode), use lsof instead.
+ # This *could* have a false positive
+ # if another package that uses libedit explicitly
+ # has been imported prior to this test.
+ p = Popen(['lsof', '-p', str(os.getpid())], stdout=PIPE, stderr=PIPE)
+ except OSError:
+ # This is highly unlikely, but let's be sure
+ # we don't crash IPython just because we can't find lsof
+ p = out = err = None
+ warnings.warn("libedit detection failed")
+ break
+
+ out,err = p.communicate()
if p.returncode == 4:
# EINTR
time.sleep(dt)
dt *= 2
continue
- elif p.returncode:
+ elif p is None or p.returncode:
warnings.warn("libedit detection failed: %s"%err)
break
else:
break
- if p.returncode == 0 and re.search(r'/libedit[\.\d+]*\.dylib\s', otool):
+ if p is not None and p.returncode == 0 and re.search(br'/libedit[\.\d+]*\.dylib\s', out):
# we are bound to libedit - new in Leopard
_rl.parse_and_bind("bind ^I rl_complete")
warnings.warn("Leopard libedit detected - readline will not be well behaved "
@@ -79,7 +95,8 @@ def add_history(line):
"which is easy_installable with: 'easy_install readline'",
RuntimeWarning)
uses_libedit = True
-
+ # cleanup names
+ del dt,p,out,err
# the clear_history() function was only introduced in Python 2.4 and is
# actually optional in the readline API, so we must explicitly check for its
Something went wrong with that request. Please try again.