This implements basic line editing for the interactive python interpreter on linux.
No tab completion, but at least backspace works.
Details
Details
Diff Detail
Diff Detail
- Repository
- rL LLVM
Event Timeline
| scripts/Python/modules/readline/readline.cpp | ||
|---|---|---|
| 17 | Given the code you are adding, this comment seems out of date. | |
Comment Actions
It might need to be conditionalized for libedit support if the CMakeLists doesn't already cover that. I think we had it set up to allow conditional lldb building without libedit support.
Comment Actions
You'll need to guard against exclusion of libedit:
#ifndef LLDB_DISABLE_LIBEDIT ... #endif
We can't exclude this whole file if libedit is disabled, because if we don't have libedit, we still need this null stub in on Linux. (i.e. the existing behavior needs to be present if no libedit).
Comment Actions
I'm still testing, but this seems to do the trick:
diff --git a/CMakeLists.txt b/CMakeLists.txt index 55fdf77..dd2c440 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,7 +4,14 @@ include(cmake/modules/LLDBStandalone.cmake) include(cmake/modules/LLDBConfig.cmake) include(cmake/modules/AddLLDB.cmake) -#add_subdirectory(include) +# We need libedit support to go down both the source and +# the scripts directories. +set(LLDB_DISABLE_LIBEDIT ${LLDB_DEFAULT_DISABLE_LIBEDIT} CACHE BOOL "Disables the use of editline.") +if (LLDB_DISABLE_LIBEDIT) + add_definitions( -DLLDB_DISABLE_LIBEDIT ) +endif() + +# add_subdirectory(include) add_subdirectory(docs) if (NOT LLDB_DISABLE_PYTHON) add_subdirectory(scripts) diff --git a/scripts/Python/modules/readline/CMakeLists.txt b/scripts/Python/modules/readline/CMakeLists.txt index 11089c6..0a4376c 100644 --- a/scripts/Python/modules/readline/CMakeLists.txt +++ b/scripts/Python/modules/readline/CMakeLists.txt @@ -7,7 +7,11 @@ SET(PYTHON_DIRECTORY python${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR}/site- include_directories(${PYTHON_INCLUDE_DIR}) add_library(readline SHARED readline.cpp) -target_link_libraries(readline ${PYTHON_LIBRARY}) +if (NOT LLDB_DISABLE_LIBEDIT) + target_link_libraries(readline ${PYTHON_LIBRARY} edit) +else() + target_link_libraries(readline ${PYTHON_LIBRARY}) +endif() # FIXME: the LIBRARY_OUTPUT_PATH seems to be ignored - this is not a # functional issue for the build dir, though, since the shared lib dir diff --git a/scripts/Python/modules/readline/readline.cpp b/scripts/Python/modules/readline/readline.cpp index b0d6b74..38268f8 100644 --- a/scripts/Python/modules/readline/readline.cpp +++ b/scripts/Python/modules/readline/readline.cpp @@ -1,23 +1,68 @@ #include <stdio.h> #include "Python.h" -// Python readline module intentionally built to not implement the -// readline module interface. This is meant to work around llvm -// pr18841 to avoid seg faults in the stock Python readline.so linked -// against GNU readline. +#ifndef LLDB_DISABLE_LIBEDIT +#include <editline/readline.h> +#endif + +// Simple implementation of the Python readline module using libedit. +// In the event that libedit is excluded from the build, this turns +// back into a null implementation that blocks the module from pulling +// in the GNU readline shared lib, which causes linkage confusion when +// both readline and libedit's readline compatibility symbols collide. +// +// Currently it only installs a PyOS_ReadlineFunctionPointer, without +// implementing any of the readline module methods. This is meant to +// work around LLVM pr18841 to avoid seg faults in the stock Python +// readline.so linked against GNU readline. static struct PyMethodDef moduleMethods[] = { {nullptr, nullptr, 0, nullptr} }; +#ifndef LLDB_DISABLE_LIBEDIT +PyDoc_STRVAR( + moduleDocumentation, + "Simple readline module implementation based on libedit."); +#else PyDoc_STRVAR( moduleDocumentation, - "Stub module meant to effectively disable readline support."); + "Stub module meant to avoid linking GNU readline."); +#endif + +#ifndef LLDB_DISABLE_LIBEDIT +static char* +simple_readline(FILE *stdin, FILE *stdout, char *prompt) +{ + rl_instream = stdin; + rl_outstream = stdout; + char* line = readline(prompt); + if (!line) + { + char* ret = (char*)PyMem_Malloc(1); + if (ret != NULL) + *ret = '\0'; + return ret; + } + if (*line) + add_history(line); + int n = strlen(line); + char* ret = (char*)PyMem_Malloc(n + 2); + strncpy(ret, line, n); + free(line); + ret[n] = '\n'; + ret[n+1] = '\0'; + return ret; +} +#endif PyMODINIT_FUNC initreadline(void) { +#ifndef LLDB_DISABLE_LIBEDIT + PyOS_ReadlineFunctionPointer = simple_readline; +#endif Py_InitModule4( "readline", moduleMethods, diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt index 0470e51..8e85498 100644 --- a/source/CMakeLists.txt +++ b/source/CMakeLists.txt @@ -6,11 +6,6 @@ else() set(LLDB_DEFAULT_DISABLE_LIBEDIT 0) endif () -set(LLDB_DISABLE_LIBEDIT ${LLDB_DEFAULT_DISABLE_LIBEDIT} CACHE BOOL "Disables the use of editline.") -if (LLDB_DISABLE_LIBEDIT) - add_definitions( -DLLDB_DISABLE_LIBEDIT ) -endif() - if ( CMAKE_SYSTEM_NAME MATCHES "Linux" ) include_directories( Plugins/Process/Linux
Comment Actions
Yep, that modified patch worked for both:
cmake -DLLDB_DISABLE_LIBEDIT=1
where libedit is disabled from the build, and for the normal case where libedit is available. In both cases it did the right thing - simple readline support with libedit and your change (minor tweaks by me solely on comments and conditional libedit), and for the no-libedit case (where it behaves as before, including the no-crash part :-) ).
If you can update the patch with that, it looks good to me.
Comment Actions
(And nice work on the underlying change - with libedit it works quite nicely for basic line editing and history!)
Comment Actions
And if you'd prefer, I'd be happy to put in that patch as is (the one I tested with both libedit included and libedit excluded).
Given the code you are adding, this comment seems out of date.