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

Extra space with tab completion in ipython #42

Closed
isofer opened this issue Nov 27, 2013 · 65 comments
Closed

Extra space with tab completion in ipython #42

isofer opened this issue Nov 27, 2013 · 65 comments
Assignees

Comments

@isofer
Copy link

isofer commented Nov 27, 2013

This issue still persists.
see details: https://groups.google.com/a/continuum.io/d/topic/anaconda/oRV1Yc-H3IY/discussion

@jjhelmus
Copy link
Contributor

Here is a work around if anyone is interested. Works on Linux, untested on other systems.

Download the readline library source code from http://cnswww.cns.cwru.edu/php/chet/readline/rltop.html.

Unpack the tarball, edit lines 340 and 471 (these might change for later version, this is for 6.2)

rl_completion_append_character = ' ';

to

rl_completion_append_character = '\0';

build the source with shared libraries

./configure --enable-shared
make

Now copy libreadline.a and shlib/libreadline.so.6.2 into the anaconda/lib directory overwriting the files with the same name (you may want to make backups of the originals in case something goes wrong).

Now IPython (and other anaconda programs) will no longer will add a space after a tab completion.

This works for me, but use at your own risk, I am not responsible if it brakes anything!

@asmeurer
Copy link
Contributor

So apparently the real fix is to update the Anaconda readline to 6.2.1.

@jjhelmus
Copy link
Contributor

Just updating to readline 6.2.1 will still insert the space. You need to make the change to the rl_completion_append_character lines to suppress it. I would expect the same behavior in early version of readline but have not tested it.

@asmeurer
Copy link
Contributor

Is this change a good idea to apply for everyone? Should we patch the readline we build in Anaconda to have it?

@jjhelmus
Copy link
Contributor

I don't know if this should be pushed out to everyone. The problem seems to be present only on the Linux version of Anaconda. A default install on my Mac did not produce the extra space after a tab completion, I haven't checked the Windows version. The above works for me, but should probably be tested and checked a bit better before pushing out to everyone.

@pchanial
Copy link

pchanial commented Feb 8, 2014

This bug drives me nuts! Nobody's using Linux at continuum?
I tried to investigate what's happening by downloading readline's git repo (http://git.savannah.gnu.org/cgit/readline.git) and building it by applying jjhelmus's suggestion of changing rl_completion_append_character. I'm running ubuntu 13.10 x64.

I first made sure that no libreadline.so library could get in the way (removing all of them indicates that the readline functionality is not present in ipython, which tells us with very high probability that this lib is not statically linked) and then copied the patched libs to anaconda/lib. The annoying space is still appended, so it looks like the variable rl_completion_append_character is set by the library caller...

@asmeurer
Copy link
Contributor

I'm not sure if the append character is the issue. The append character is also ' ' on Mac OS X, but that issue doesn't appear there. You can check what the character is by using the rl module (there is a recipe in conda-recipes but I didn't have luck building it on Linux), and doing rl.readline.get_completion_append_character().

@asmeurer
Copy link
Contributor

Also, this is not IPython specific. I can reproduce the same thing by using a PYTHONSTARTUP with

import readline, rlcompleter
readline.parse_and_bind("tab: complete")

in the regular Python interpreter.

@alendit
Copy link

alendit commented Feb 20, 2014

1.9 is out an no fix in sight. I guess, Linux isn't just that imporant.

Python version of readline comes with rl_completion_append_character = '\0' by default (See https://github.com/ludwigschwardt/python-gnureadline/blob/master/Modules/3.x/readline.c line 890). The reason it works on MacOS is that ipython contains a hack which skips the system readline discovery and goes straight to the pip installed version (see https://github.com/ipython/ipython/blob/master/IPython/utils/rlineimpl.py).

Please, just compile readline with a correct rl_completion_append_character.

Until then users can copy their system readline.so into anaconda environment, which also does the trick.

Even better way:

  1. Remove readline.so from lib/python{version}/lib-dynload in your anaconda install and environment.
  2. install pip with conda install pip
  3. pip install readline

@asmeurer
Copy link
Contributor

The readline recipe in the conda-recipes repo has this change, but for me, it doesn't fix the problem. My test is, in IPython, from sys <TAB>. This types import with two spaces. Note that for whatever reason, the version in that recipe is listed as 6.2, but it's really 6.2.5. @tpn will have to comment further.

@alendit
Copy link

alendit commented Feb 20, 2014

@asmeurer please make sure, that the referred recepy indeed overwrites all instances of readline.so.

In my case, ipython tryied to import readline.so from ~/anaconda/lib/python2.7/lib-dynload first, then from ~/anaconda/envs/myenv/lib/python2.7/lib-dynload when I removed the first one. Maybe only the second instance is overwritten by the recepy.

EDIT: to check the path, your readline was imported from do the following:

  1. Start ipython
  2. import readline
  3. readline._rt

@asmeurer
Copy link
Contributor

Oh, you're right. The readline being loaded is part of the python package, not the readline package.

@asmeurer
Copy link
Contributor

Oh but that doesn't make sense. The readline in Python has the right append character.

@alendit
Copy link

alendit commented Feb 20, 2014

I am not sure which readline you mean. According to my knowledge:

  • readline which comes with anaconda (in lib-dynload) is buggy and doesn't have the right append character
  • readline which comes with python-readline is patched and fixes the issue. You can see the patches and the source code in their repository

@asmeurer
Copy link
Contributor

(with the patched readline installed)

In [4]: readline._rl 
Out[4]: <module 'readline' from '/home/aaronmeurer/anaconda/lib/python3.3/lib-dynload/readline.cpython-33m.so'>

In [5]: 
Do you really want to exit ([y]/n)? 
aaronmeurer@ubuntu:~/Documents/conda-recipes(master %=)$conda package --which /home/aaronmeurer/anaconda/lib/python3.3/lib-dynload/readline.cpython-33m.so
/home/aaronmeurer/anaconda/lib/python3.3/lib-dynload/readline.cpython-33m.so  python-3.3.4-0

@alendit
Copy link

alendit commented Feb 20, 2014

The patch in the recepy is not complete. It should also change the second occurence of rl_completion_append_character like this

--- readline-6.2.5/complete.c   2014-01-04 02:29:06.000000000 +0100
+++ complete.c  2014-02-20 23:40:20.084564457 +0100
@@ -337,7 +337,7 @@

 /* Character appended to completed words when at the end of the line.  The
    default is a space. */
-int rl_completion_append_character = ' ';
+int rl_completion_append_character = '\0';

 /* If non-zero, the completion functions don't append any closing quote.
    This is set to 0 by rl_complete_internal and may be changed by an
@@ -468,7 +468,7 @@
   rl_filename_quoting_desired = 1;
   rl_completion_type = what_to_do;
   rl_completion_suppress_append = rl_completion_suppress_quote = 0;
-  rl_completion_append_character = ' ';
+  rl_completion_append_character = '\0';

   /* The completion entry function may optionally change this. */
   rl_completion_mark_symlink_dirs = _rl_complete_mark_symlink_dirs;

asmeurer added a commit to conda-archive/conda-recipes that referenced this issue Feb 21, 2014
@asmeurer
Copy link
Contributor

OK, I built it with that patch, and it seems to fix it. You can get it with conda install -c asmeurer -f readline (note that the -f is needed because python currently marks itself as depending on readline 6.2, but this is version 6.2.5).

@isofer
Copy link
Author

isofer commented Feb 22, 2014

the patch does not work for me.
It installs readline, but does not fix the problem.

I have the latest conda version, but not the latest anaconda.

@mb3152
Copy link

mb3152 commented Mar 28, 2014

I am having this same problem on 64bit unix

@kbg
Copy link

kbg commented Mar 31, 2014

The problem has nothing to do with the GNU readline library, but rather with an erroneous build of the Python interpreter (or more specifically the Python readline module).

Just to make it clear (because there seemed to be some confusion in the previous postings between GNU readline library and the Python readline module), the file libreadline.so is the shared version of the GNU readline library, while the readline.so is the Python extension module which comes with Python and which is linked against the actual readline library. The gnureadline package from PyPI on the other hand is just a copy of the readline module that comes with Python, statically linked against the GNU readline library, so installing this package is only a workaround and not a solution to the original problem.

The problem with the Anaconda Python package is visible inside the pyconfig.h file that comes with Anaconda, where the HAVE_LIBREADLINE and all the HAVE_RL_* entries are not defined. Apparently the Python readline module was built anyway, but without all the code inside the HAVE_RL_* ifdefs, which already handles the trailing spaces.

So if the Python interpreter that comes with Anaconda is built correctly, there is no need to add another "remove-trailing-spaces" patch to libreadline or to install gnureadline from PyPI. I would have tried to fix the build problem directly, but unfortunately the "recipe" for the Python package that comes with Anaconda does not seem to be publicly available.

On the linux machines where I have installed Anaconda (running CentOS 6.5 and Fedora 20) the problem could be easily fixed by compiling Python from sources (linking against the system version of libreadline), replacing the readline.so of the Anaconda installation with the newly created one and deinstalling the Anaconda readline package using conda. I even could reproduce the bug by undefining the HAVE_LIBREADLINE and HAVE_RL_* entries in pyconfig.h (after running the configure script) and compiling the Python sources again (curiously, the readline module was built in this case, even though the HAVE_LIBREADLINE was not defined). So the bug must be definitely caused by a build error in the Python package.

After that I installed the Anaconda readline package again (conda install readline) and tried to compile Python 2.7.6 using the libreadline provided by Anaconda, with the commands

./configure --enable-shared --enable-ipv6 --enable-unicode=ucs4 \
    CFLAGS=-I/opt/anaconda/include \
    LDFLAGS="-L/opt/anaconda/lib -Wl,-rpath=/opt/anaconda/lib"
make

which failed to build the readline.so:

[...]
building 'readline' extension
gcc -pthread -fPIC -fno-strict-aliasing -I/opt/anaconda/include -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I. -IInclude -I./Include -I/usr/local/include -I/home/kolja/build/Python-2.7.6/Include -I/home/kolja/build/Python-2.7.6 -c /home/kolja/build/Python-2.7.6/Modules/readline.c -o build/temp.linux-x86_64-2.7/home/kolja/build/Python-2.7.6/Modules/readline.o
[...]
gcc -pthread -shared -L/opt/anaconda/lib -Wl,-rpath=/opt/anaconda/lib build/temp.linux-x86_64-2.7/home/kolja/build/Python-2.7.6/Modules/readline.o -L/usr/lib/termcap -L/opt/anaconda/lib -L/usr/local/lib -L. -lreadline -lpython2.7 -o build/lib.linux-x86_64-2.7/readline.so
*** WARNING: renaming "readline" since importing it failed: /opt/anaconda/lib/libreadline.so.6: undefined symbol: PC
[...]
Failed to build these modules:
readline
[...]

The reason for this was, that the readline extension module is built by a Python setup.py script, which tries to figure out how to link libreadline and which additional libraries are needed to resolve all symbols. In this case the script did not pick the right libreadline.so (the one in /opt/anaconda/lib) but the system's libreadline.so.

Now the libreadline.so that comes with CentOS 6 and Fedora has a DT_NEEDED entry for libtinfo.so:

$ readelf -d /lib64/libreadline.so.6 | grep NEEDED
 0x0000000000000001 (NEEDED)             Shared library: [libtinfo.so.5]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]

while the libreadline.so from Anaconda is underlinked:

$ readelf -d /opt/anaconda/lib/libreadline.so.6 | grep NEEDED
 0x0000000000000001 (NEEDED)

and thus needs to be linked together with libtinfo.so (or libncursesw.so on systems where ncurses is not split into two libraries).

So because the build script picks the wrong library, it comes to the conclusion that it does not need to link any other library, which then results in missing symbols when importing the module.

Luckily this behaviour can be easily fixed by making sure that the library search order is correct (first look into directories specified by -L and then into the system library directories):

--- setup.py.orig       2013-11-10 08:36:41.000000000 +0100
+++ setup.py    2014-03-31 01:56:36.101247755 +0200
@@ -35,12 +35,16 @@
 # This global variable is used to hold the list of modules to be disabled.
 disabled_module_list = []

-def add_dir_to_list(dirlist, dir):
-    """Add the directory 'dir' to the list 'dirlist' (at the front) if
+def add_dir_to_list(dirlist, dir, append=False):
+    """Add the directory 'dir' to the list 'dirlist' (at the front if append
+    is False; at the back if append is True) if
     1) 'dir' is not already in 'dirlist'
     2) 'dir' actually exists, and is a directory."""
     if dir is not None and os.path.isdir(dir) and dir not in dirlist:
-        dirlist.insert(0, dir)
+        if append:
+            dirlist.append(dir)
+        else:
+            dirlist.insert(0, dir)

 def macosx_sdk_root():
     """
@@ -508,7 +512,7 @@
                 '/lib64', '/usr/lib64',
                 '/lib', '/usr/lib',
                 ):
-                add_dir_to_list(lib_dirs, d)
+                add_dir_to_list(lib_dirs, d, append=True)
         exts = []
         missing = []

After this modification the build went through without errors and the readline.so was linked against libncursesw.so and libtinfo.so (even if libtinfo.so alone would have sufficed on CentOS 6 or Fedora). But at least there were no more missing symbols in the newly created readline.so and it was linked against the Anaconda version of libreadline.

After making the file relocatable with

patchelf --set-rpath '$ORIGIN/../..' readline.so

I successfully used it to replace the broken Anaconda readline modules on different installations (Fedora, Ubuntu 12.04, Arch Linux).

On Arch Linux I had a little problem, because the readline.so I created was built on a CentOS 6 system, where the ncurses library is split in libtinfo.so and libncursesw.so (which AFAIK most distros do these days), while Arch Linux still uses only a single libncursesw.so library and has no separate libtinfo.so. A quick fix for this problem was to create a symlink of the libncursesw.so.5 into the Anaconda directory as libtinfo.so.5:

ln -s /lib/libncursesw.so.5 /opt/anaconda/lib/libtinfo.so.5

I think this tinfo/ncurses problem would not appear in an official Anaconda build, because I think I read somewhere that Anaconda is built using CentOS 5, where ncurses is also not split into two libraries. So the readline.so created on this system should work on all other systems without such a workaround.

So I think all that needs to be done to get finally rid of this annoying trailing-space bug is to fix the Anaconda Python package by making sure that autoconf script finds (and uses) the Anaconda version of the GNU readline library and to make sure that the readline.so extension is linked correctly (for example by applying the above patch to the setup.py file).

Also, I think it would maybe be a good idea to include the ncurses library into the Anaconda distribution. This would make it possible to link libreadline.so directly against libtinfo.so and avoid underlinking. And it could help to prevent possible tinfo/ncurses problems, if the build system comes with a ncurses installation that is split into two libraries.

@asmeurer
Copy link
Contributor

asmeurer commented Apr 2, 2014

Thanks for the write up. We'll look into fixing this.

Couldn't you have used --prefix=/path/to/anaconda with your configure command to get it to find the right libreadline.so?

@kbg
Copy link

kbg commented Apr 5, 2014

The `--prefix`` option can be used to specify the installation directory - not the location of additional header and library files.

The -I and -L flags, on the other hand, tell the compiler/linker that it should look into these directories first to find headers and libraries, before trying to find them inside the system directories.

@asmeurer
Copy link
Contributor

Sorry, we never figured out how to get around the issues that arose when compiling ncurses (conda-archive/conda-recipes#112 (comment)). If you know how to fix it, let us know. If you can provide a working recipe for ncurses that doesn't have these issues, that would help a lot. Otherwise, we don't have the resources to spend much more time on this right now.

@kbg
Copy link

kbg commented Jun 12, 2014

Strange... @timothydmorton's posting, you just replied to, doesn't seem to exist anymore.

Anyway, I still think that something went wrong during the build of the Python readline extension module. Have you checked, that the setup.py from the Python tarball uses the right libraries? It should link against your custom libreadline.so and libtinfo.so and not against any other readline, curses or termcap library that is installed on your system.

@ostrokach
Copy link

I agree with kbg.

Furthermore, the extra tabs are not the only problem with the anaconda readline package. Compiling R using the anaconda/include folder as the C_INCLUDE_PATH and anaconda/lib folder as the LIBRARY_PATH does not work on Ubuntu because of a problem with the anaconda readline library.

Running ./configure in the R source directory leads to the following error:
checking for readline/readline.h... yes
checking for rl_callback_read_char in -lreadline... no
checking for main in -lncurses... yes
checking for rl_callback_read_char in -lreadline... no
checking for history_truncate_file... no
configure: error: --with-readline=yes (default) and headers/libs are not available

Oddly enough, R compiles just fine using the anaconda readline library on CentOS...

@benjaminmgross
Copy link

I tried to make a simplified instructional gist for scenarios like mine:

https://gist.github.com/benjaminmgross/0fffd01f9037aaa76a9e

@wesm
Copy link

wesm commented Nov 10, 2014

I'm having this problem with Anaconda 2.1.0 and stock Debian 7. Any better solutions?

@orodbhen
Copy link

I've read over this thread at least a couple times, and I not sure if anyone found a solution or even a reliable workaround. Could someone who understands the issue well please provide a summary of what's causing it? For my benefit, and the benefit of anyone else who reads this in the future.

Is it the readline package? Is it the built-in python module? Is it ncurses? Is it the way Continuum compiles readline, prior to compiling Python?

We've been using a custom-built Python 2.7 distribution that I created on our CentOS 6 machines, and I haven't seen this problem there; or in the Python packages installed on Fedora. Installing ipython with pip in a virtualenv doesn't have this issue either. So I'm a little confused about what makes the anaconda version different.

I was hoping to migrate our systems to use anaconda, so I don't have to keep maintaining my custom distribution. I've tried Canopy, but found it to be too buggy to justify paying the $200 licensing fee. Are these kinds of issues typical with Anaconda, or is it normally quite stable?

@orodbhen
Copy link

Ok. I'm going to give it a try. First this, from @kbg:

"The problem with the Anaconda Python package is visible inside the pyconfig.h file that comes with Anaconda, where the HAVE_LIBREADLINE and all the HAVE_RL_* entries are not defined. Apparently the Python readline module was built anyway, but without all the code inside the HAVE_RL_* ifdefs, which already handles the trailing spaces."

So the python readline module is compiled with a bad pyconfig.h. Why is it that way?

What I don't understand is why there were problems compiling Python 2.7.6 against the system readline library. I was able to compile 2.7.6 on CentOS 6, and readline.so was created without problems. I used:

LDFLAGS="-Wl,-rpath,/opt/python/intel64/lib" ./configure --enable-unicode=ucs4 --enable-shared --prefix=/opt/python/intel64 --with-valgrind

Is it because you were using the anaconda library path? Is it a conflict between a library in that path, and the system libreadline?

So then what's needed, If understand correctly, is for Continuum to fix the bad pyconfig.h, and package ncurses with Anaconda to satisfy the readline.so dependency. Correct?

@immerrr
Copy link

immerrr commented Mar 28, 2015

Is it a conflict between a library in that path, and the system libreadline?

There is a conflict between system ncurses & anaconda one: common linux distros build separate ncurses & libtinfo libs (with a special ncurses build flag) while conda ships a single ncurses binary. Python build script detects system-wide libtinfo.so and makes readline.so dependent on it, but since the library is not installed in the env, it is not found and this breaks readline.so at runtime in many ways, including producing broken pyconfig.h.

There are two fixes: the better one, from @kbg, patches python build script so that ncurses/tinfo autodetection runs on conda-supplied library, and mine, which mitigates the issue patching ncurses build script to build separate libtinfo library so that it is installed into the env and resolved properly.

Alas, Continuum guys seem to disregard the issue have spent quite some time on the issue and reluctant to spend more even if it means fixing the problem.

@immerrr
Copy link

immerrr commented Mar 28, 2015

@orodbhen , addressing your other question

Are these kinds of issues typical with Anaconda, or is it normally quite stable?

Conda employs some non-100%-reliable techniques, like patching strings that look like paths in the binaries. Most of the packages are thoroughly tested, so they are quite stable as long as you don't do something unusual. At least they worked quite well for me.

However, there are some corner cases. For example I have run into a case when such binary patching corrupted the debug info and made valgrind choke.

Another issue I have run into is that conda-shipped qt library is built without ssl support (#140). Even though I'd love to help, it is far from trivial to find out what prevents qt from using openssl package without seeing the qt recipe, and I'm not sure if the publicly available recipe is compatible with the one Continuum uses for building packages in the official channel.

@orodbhen
Copy link

Thanks @immerrr . It makes a lot more sense now. I have a Qt 4 build in my custom Python install with SSL enabled, that works quite well. But I'm not very familiar with the conda build system.

I was actually in the process of creating a tool for managing python packages and virtual envs, as well as non-python packages. I was looking at making a python clone of homebrew for Linux, with some changes. But then I discovered that conda does all of the above. Even if Continuum's packages prove unreliable, it at least provides a management framework that may work for us.

I had a heck of a time getting Python to build 32 bit on a 64 bit system. It's build scripts don't do a great job at honoring CFLAGS and LDFLAGS.

Anyways, I'll try the patch and see if it solves the problem for me.

Why does "conda search ncurses" not return any results?

@orodbhen
Copy link

The patch from @kbg fixed the issue for me.

It also fixed a segfault I was getting with a Traitsui script, when trying to run it in the Anaconda env. I wonder what other differences there are in Continuum's build recipe for Python.

@orodbhen
Copy link

orodbhen commented Apr 4, 2015

How do I keep conda from downgrading Python to the original version every time I install something? Do I have to give the patched Python the same version number as the original?

@ccordoba12 ccordoba12 self-assigned this Apr 6, 2015
@ccordoba12
Copy link

I'll try to review and merge the mentioned patch this week

@ccordoba12
Copy link

Ok, so I went through the entire thread trying to understand its main points. I have one question for @kbg:

The configure step in your Python recipe is

./configure --prefix=$PREFIX \
    --enable-shared --enable-ipv6 --enable-unicode=ucs4 \
    --with-tcltk-includes="-I$PREFIX/include" \
    --with-tcltk-libs="-L$PREFIX/lib -ltcl8.5 -ltk8.5" \
    CPPFLAGS="-I$PREFIX/include" \
    LDFLAGS="-L$PREFIX/lib -Wl,-rpath=$PREFIX/lib,--no-as-needed"

How important are the tcltk lines? I mean, are they really necessary?

@ccordoba12
Copy link

Just as a side note to @immerrr: the error you mentioned about Qt is solved now, as almost all other important issues related to it :-)

@ccordoba12
Copy link

I tested @kbg suggestion and everything seems to be working as expected!!

I submitted a PR to our internal repo, so let's hope this is fixed soon :-)

@immerrr
Copy link

immerrr commented Apr 12, 2015

@ccordoba12 that is so awesome, thank you!

@ccordoba12
Copy link

Ok guys, we just released a new version of Python 2 (specifically 2.7.9-3) which finally fixes this problem.

Sorry for the big delay in addressing this issue, and thanks a lot to @kbg for providing a very informed and clear solution.

Happy coding!! :-)

@immerrr
Copy link

immerrr commented May 8, 2015

@ccordoba12, will python3 be updated soon too?

@jjhelmus
Copy link
Contributor

jjhelmus commented May 8, 2015

Thanks for fixing this! I can verify the new build, 2.7.9-3, no longer inserts an extra tab.

@ccordoba12
Copy link

@immerrr, I thought Python 3 doesn't have this problem. I'll verify and report back :-)

@ccordoba12
Copy link

Ok, I tested it in Python 3 and @immerrr is right, the extra space is a problem there too.

We'll try to release a new package soon to fix this problem :-)

@ccordoba12 ccordoba12 reopened this May 14, 2015
@jjhelmus
Copy link
Contributor

jjhelmus commented Jun 8, 2015

The extra space issue seems to have been fixed in Python 3 with the release of python 3.4.3-1 and python 3.3.5-4. Thanks!

@ccordoba12
Copy link

@jjhelmus, thanks for testing!! So this infamous bug can finally be closed , yay!! \o/

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

No branches or pull requests