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

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

Merged
merged 2 commits into from Jun 19, 2011

Conversation

minrk
Copy link
Member

@minrk minrk commented Jun 18, 2011

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

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 ipythongh-524
@fperez
Copy link
Member

fperez commented Jun 19, 2011

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

@OliDa
Copy link

OliDa commented Jun 19, 2011

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@takluyver
Copy link
Member

@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
Copy link

OliDa commented Jun 19, 2011

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

@takluyver
Copy link
Member

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.

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.
@minrk
Copy link
Member Author

minrk commented Jun 19, 2011

@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
Copy link

OliDa commented Jun 19, 2011

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

@minrk
Copy link
Member Author

minrk commented Jun 19, 2011

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

Thanks!

@minrk minrk merged commit ec698f9 into ipython:master Jun 19, 2011
@takluyver
Copy link
Member

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

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

Successfully merging this pull request may close these issues.

otool should not be unconditionally called on osx
4 participants