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

Sometime readline do not have set_completer_delims #63

Merged
merged 1 commit into from
Apr 8, 2016
Merged

Sometime readline do not have set_completer_delims #63

merged 1 commit into from
Apr 8, 2016

Conversation

tebeka
Copy link
Contributor

@tebeka tebeka commented Mar 1, 2016

No description provided.

@minrk
Copy link
Member

minrk commented Mar 1, 2016

@tebeka can you give an example of when this is unavailable?

@tebeka
Copy link
Contributor Author

tebeka commented Mar 1, 2016

@minrk Every time I run Anaconda jupyter console on my Archlinux machine

@Carreau
Copy link
Member

Carreau commented Mar 3, 2016

set_completer_delims is supposed to be available on readline module, if it's not it's:

  • either a bug of the readline implementation on arch/anaconda-arch (in which case it should be fixed/ reported upstream)
  • a missing statement in the docs (in which case it also need to be fixed)
  • a bug in our code that fail to find set_completer_delims (we have a shim around readline)

can you import readline, and give us the result of readline.__file__, so that we can try to track that down ?

In [1]: import readline
In [2]: readline.__file__
Out[2]: '/Users/bussonniermatthias/anaconda3/lib/python3.5/lib-dynload/readline.cpython-35m-darwin.so'

@tebeka
Copy link
Contributor Author

tebeka commented Mar 4, 2016

IMO this is a version mismatch between the installed readline on arch vs the one anaconda expects.
If I run the arch system python you'll see

Python 3.5.1 (default, Dec  7 2015, 12:58:09) 
[GCC 5.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import readline
>>> readline.__file__
'/usr/lib/python3.5/lib-dynload/readline.cpython-35m-x86_64-linux-gnu.so'

However if you run the Anaconda Python:

Python 3.5.1 |Anaconda 2.5.0 (64-bit)| (default, Dec  7 2015, 11:16:01) 
[GCC 4.4.7 20120313 (Red Hat 4.4.7-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import readline
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: libncursesw.so.5: cannot open shared object file: No such file or directory
>>> 

There is /opt/anaconda/lib/python3.5/lib-dynload/readline.cpython-35m-x86_64-linux-gnu.so however:

ldd /opt/anaconda/lib/python3.5/lib-dynload/readline.cpython-35m-x86_64-linux-gnu.so
    linux-vdso.so.1 (0x00007ffdbd1fd000)
    libreadline.so.6 => /opt/anaconda/lib/python3.5/lib-dynload/../../libreadline.so.6 (0x00007f5d7ba11000)
    libncursesw.so.5 => not found
    libpython3.5m.so.1.0 => /opt/anaconda/lib/python3.5/lib-dynload/../../libpython3.5m.so.1.0 (0x00007f5d7b4e8000)
    libpthread.so.0 => /usr/lib/libpthread.so.0 (0x00007f5d7b2cb000)
    libc.so.6 => /usr/lib/libc.so.6 (0x00007f5d7af2a000)
    libdl.so.2 => /usr/lib/libdl.so.2 (0x00007f5d7ad25000)
    libutil.so.1 => /usr/lib/libutil.so.1 (0x00007f5d7ab22000)
    librt.so.1 => /usr/lib/librt.so.1 (0x00007f5d7a91a000)
    libm.so.6 => /usr/lib/libm.so.6 (0x00007f5d7a614000)
    /usr/lib64/ld-linux-x86-64.so.2 (0x0000560a7aa0a000)

You'll see it depends on libncursesw.so.5 which is not in the system. Where's the system one uses libncursesw.so.6

ldd /usr/lib/python3.5/lib-dynload/readline.cpython-35m-x86_64-linux-gnu.so
    linux-vdso.so.1 (0x00007ffd9fbf9000)
    libreadline.so.6 => /usr/lib/libreadline.so.6 (0x00007f8646b8d000)
    libpython3.5m.so.1.0 => /usr/lib/libpython3.5m.so.1.0 (0x00007f86466bf000)
    libpthread.so.0 => /usr/lib/libpthread.so.0 (0x00007f86464a2000)
    libc.so.6 => /usr/lib/libc.so.6 (0x00007f8646100000)
    libncursesw.so.6 => /usr/lib/libncursesw.so.6 (0x00007f8645e93000)
    libdl.so.2 => /usr/lib/libdl.so.2 (0x00007f8645c8f000)
    libutil.so.1 => /usr/lib/libutil.so.1 (0x00007f8645a8b000)
    libm.so.6 => /usr/lib/libm.so.6 (0x00007f8645786000)
    /usr/lib64/ld-linux-x86-64.so.2 (0x0000558c0f766000)

@minrk
Copy link
Member

minrk commented Mar 4, 2016

That seems like a significant problem to bring up with Continuum, since it suggests that the readline isn't built correctly.

@Carreau
Copy link
Member

Carreau commented Mar 4, 2016

ping @damianavila, @bollwyvl do you know who we should ping on the continuum team about that ?

@bollwyvl
Copy link

bollwyvl commented Mar 4, 2016

@ilanschnell is the man with the plan on this one. Certainly worth raising on https://github.com/ContinuumIO/anaconda-issues

@Carreau
Copy link
Member

Carreau commented Mar 4, 2016

Thanks !

Might be related to ContinuumIO/anaconda-issues#455

@tebeka
Copy link
Contributor Author

tebeka commented Mar 5, 2016

I agree that Continuum should address the problem their end. But IMO they can't cover all the issue with readline. This small patch guards against such problems in the future.

@Carreau
Copy link
Member

Carreau commented Mar 8, 2016

This small patch guards against such problems in the future.

You can see that through the opposite view which is this patch hides an obvious bug in an underlying implementation or build of a specific package. It also confuses other dev into thinking set_completer_delims is not mandatory.

If there is not fixes in conda that get released we can weight the cost of adding that in, which could make sens, but I prefer (for now) to wait on wether ContinuumIO/anaconda-issues#455 will be fixed.

@takluyver
Copy link
Member

(N.B. We're also ditching readline for 5.0, so this won't be a longstanding problem)

@Carreau Carreau added this to the 4.2 milestone Apr 8, 2016
@Carreau
Copy link
Member

Carreau commented Apr 8, 2016

Thanks, merging, if we do a 4.2 release this should be backported, not sure there will be a 4.2 though.

Sorry for the delay,

@Carreau Carreau merged commit ddbd771 into jupyter:master Apr 8, 2016
@bollwyvl
Copy link

bollwyvl commented Apr 8, 2016

Thanks, merging, if we do a 4.2 release this should be backported, not sure there will be a 4.2 though.

News to me: what's the development that suggests otherwise? Love to help out if I can!

@takluyver
Copy link
Member

takluyver commented Apr 8, 2016 via email

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.

5 participants