Skip to content

Commit

Permalink
python: use processplugin
Browse files Browse the repository at this point in the history 
+ add tests
  • Loading branch information
Markus Raab committed Jul 28, 2018
1 parent 27828a9 commit 532666e
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 50 deletions.
2 changes: 1 addition & 1 deletion doc/news/_preparation_next_release.md
View file
Expand Up @@ -160,7 +160,7 @@ Thanks to Daniel Bugl.
### Interpreter Plugins

- The plugins ruby, python and jni can now also be mounted as global plugin.
- Fix crash in python plugin. *(Markus Raab)*
- Fix crash in python plugin by using process plugin. *(Markus Raab)*

### HexNumber

Expand Down
33 changes: 17 additions & 16 deletions src/plugins/python/CMakeLists.txt
View file
Expand Up @@ -2,10 +2,11 @@ include (LibAddBinding)

if (DEPENDENCY_PHASE)
find_package (PythonLibs 3 QUIET)
find_package (Pluginprocess)
find_swig ()
check_binding_included ("swig_python" BINDING_WAS_ADDED SUBDIRECTORY "swig/python" SILENT)

if (PYTHONLIBS_FOUND AND SWIG_FOUND AND BINDING_WAS_ADDED)
if (PYTHONLIBS_FOUND AND SWIG_FOUND AND BINDING_WAS_ADDED AND PLUGINPROCESS_FOUND)
include (LibAddMacros)

add_custom_command (OUTPUT runtime.h
Expand All @@ -23,6 +24,8 @@ if (DEPENDENCY_PHASE)
remove_plugin (python "python 3 libs (libpython3-dev) not found")
elseif (NOT BINDING_WAS_ADDED)
remove_plugin (python "swig_python binding is required")
elseif (NOT PLUGINPROCESS_FOUND)
remove_plugin (python "Elektra's pluginprocess library is required")
else ()
remove_plugin (python "swig not found")
endif ()
Expand All @@ -35,28 +38,26 @@ add_plugin (python
${CMAKE_CURRENT_BINARY_DIR}/runtime.h
INCLUDE_DIRECTORIES ${PYTHON_INCLUDE_DIR}
LINK_LIBRARIES ${PYTHON_LIBRARIES}
LINK_ELEKTRA elektra-pluginprocess
COMPILE_DEFINITIONS SWIG_TYPE_TABLE=kdb
SWIG_RUNTIME=\"runtime.h\"
PYTHON_PLUGIN_NAME=python
PYTHON_PLUGIN_SYMBOL_NAME=Python)

if (ADDTESTING_PHASE)
if (ENABLE_BROKEN_TESTS)
# bindings are required for tests
check_binding_was_added ("swig_python" BINDING_WAS_ADDED)
if (BUILD_TESTING AND BINDING_WAS_ADDED)

# bindings are required for tests
check_binding_was_added ("swig_python" BINDING_WAS_ADDED)
if (BUILD_TESTING AND BINDING_WAS_ADDED)
# test environment clears env so setting PYTHONPATH is no option we workaround this by changing the current directory to our
# bindings output directory + our test adds the current directory to pythons sys.path
add_plugintest (python MEMLEAK WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/../../bindings/swig/python/)

# test environment clears env so setting PYTHONPATH is no option we workaround this by changing the current
# directory to our bindings output directory + our test adds the current directory to pythons sys.path
add_plugintest (python MEMLEAK WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/../../bindings/swig/python/)

if (INSTALL_TESTING)
install (DIRECTORY python DESTINATION ${TARGET_TEST_DATA_FOLDER})
endif ()

else ()
message (WARNING "BUILD_STATIC or BUILD_FULL and swig_python bindings are required for testing, test deactivated")
if (INSTALL_TESTING)
install (DIRECTORY python DESTINATION ${TARGET_TEST_DATA_FOLDER})
endif ()
endif (ENABLE_BROKEN_TESTS)

else ()
message (WARNING "The swig_python binding is required for testing, test deactivated")
endif ()
endif ()
82 changes: 64 additions & 18 deletions src/plugins/python/python.cpp
View file
Expand Up @@ -20,6 +20,7 @@
#include SWIG_RUNTIME
#include "python.hpp"

#include <kdbpluginprocess.h>
#include <key.hpp>
#include <keyset.hpp>
#include <libgen.h>
Expand Down Expand Up @@ -69,8 +70,10 @@ typedef struct
{
PyThreadState * tstate;
PyObject * instance;
Key * script;
int printError;
int shutdown;
ElektraPluginProcess * pp;
} moduleData;

static int Python_AppendToSysPath (const char * path)
Expand Down Expand Up @@ -99,7 +102,8 @@ static int Python_CallFunction_Int (moduleData * data, PyObject * object, PyObje
PyObject * res = Python_CallFunction (object, args);
if (!res)
{
ELEKTRA_SET_ERROR (111, errorKey, "Error while calling python function");
ELEKTRA_SET_ERRORF (111, errorKey, "Error while calling python function of script %s%s", keyString (data->script),
data->printError ? "" : ", use /print to print error messages");
if (data->printError) PyErr_Print ();
}
else
Expand Down Expand Up @@ -180,31 +184,56 @@ static void Python_Shutdown (moduleData * data)
}
}

extern "C" {
int PYTHON_PLUGIN_FUNCTION (Open) (ckdb::Plugin * handle, ckdb::Key * errorKey)
static moduleData * createModuleData (ckdb::Plugin * handle)
{
KeySet * config = elektraPluginGetConfig (handle);

Key * script = ksLookupByName (config, "/script", 0);
if (script == nullptr || !strlen (keyString (script)))
{
if (ksLookupByName (config, "/module", 0) != nullptr)
{
return 0; // by convention: success if /module exists
}
ELEKTRA_SET_ERROR (111, errorKey, "No python script set");
return -1;
return 0;
}

/* create module data */
auto data = new moduleData;
data->tstate = nullptr;
data->instance = nullptr;
data->script = script;
data->printError = (ksLookupByName (config, "/print", 0) != nullptr);
/* shutdown flag is integer by design. This way users can set the
* expected behaviour without worring about default values
*/
data->shutdown = (ksLookupByName (config, "/shutdown", 0) && !!strcmp (keyString (ksLookupByName (config, "/shutdown", 0)), "0"));
return data;
}

extern "C" {
int PYTHON_PLUGIN_FUNCTION (Open) (ckdb::Plugin * handle, ckdb::Key * errorKey)
{
KeySet * config = elektraPluginGetConfig (handle);
if (ksLookupByName (config, "/module", 0) != nullptr)
{
return 0; // by convention: success if /module exists
}

if (!elektraPluginGetData (handle))
{
ElektraPluginProcess * pp = elektraPluginProcessInit (errorKey);
if (pp == nullptr) return ELEKTRA_PLUGIN_STATUS_ERROR;
moduleData * md = createModuleData (handle);
if (!md)
{
ELEKTRA_SET_ERROR (111, errorKey, "No python script set, please pass a filename via /script");
return -1;
}
md->pp = pp;
elektraPluginSetData (handle, md);
if (!elektraPluginProcessIsParent (pp)) elektraPluginProcessStart (handle, pp);
}

moduleData * data = static_cast<moduleData *> (elektraPluginGetData (handle));
if (elektraPluginProcessIsParent (data->pp)) return elektraPluginProcessOpen (data->pp, errorKey);


{
/* initialize python interpreter if necessary */
Expand Down Expand Up @@ -248,7 +277,7 @@ int PYTHON_PLUGIN_FUNCTION (Open) (ckdb::Plugin * handle, ckdb::Key * errorKey)
Py_XDECREF (kdbModule);

/* extend sys path */
char * tmpScript = elektraStrDup (keyString (script));
char * tmpScript = elektraStrDup (keyString (data->script));
const char * dname = dirname (tmpScript);
if (!Python_AppendToSysPath (dname))
{
Expand All @@ -259,15 +288,15 @@ int PYTHON_PLUGIN_FUNCTION (Open) (ckdb::Plugin * handle, ckdb::Key * errorKey)
elektraFree (tmpScript);

/* import module/script */
tmpScript = elektraStrDup (keyString (script));
tmpScript = elektraStrDup (keyString (data->script));
char * bname = basename (tmpScript);
size_t bname_len = strlen (bname);
if (bname_len >= 4 && strcmp (bname + bname_len - 3, ".py") == 0) bname[bname_len - 3] = '\0';

PyObject * pModule = PyImport_ImportModule (bname);
if (pModule == nullptr)
{
ELEKTRA_SET_ERRORF (111, errorKey, "Unable to import python script %s", keyString (script));
ELEKTRA_SET_ERRORF (111, errorKey, "Unable to import python script %s", keyString (data->script));
elektraFree (tmpScript);
goto error_print;
}
Expand Down Expand Up @@ -307,6 +336,7 @@ int PYTHON_PLUGIN_FUNCTION (Open) (ckdb::Plugin * handle, ckdb::Key * errorKey)
/* destroy python */
Python_Shutdown (data);
delete data;
elektraPluginSetData (handle, nullptr);
return -1;
}

Expand All @@ -315,11 +345,21 @@ int PYTHON_PLUGIN_FUNCTION (Close) (ckdb::Plugin * handle, ckdb::Key * errorKey)
moduleData * data = static_cast<moduleData *> (elektraPluginGetData (handle));
if (data == nullptr) return 0;

ElektraPluginProcess * pp = data->pp;
if (elektraPluginProcessIsParent (pp))
{
int result = elektraPluginProcessSend (pp, ELEKTRA_PLUGINPROCESS_CLOSE, NULL, errorKey);
if (elektraPluginProcessClose (pp, errorKey)) elektraPluginSetData (handle, NULL);
return result;
}


int ret = Python_CallFunction_Helper1 (data, "close", errorKey);

/* destroy python */
Python_Shutdown (data);
delete data;
elektraPluginSetData (handle, nullptr);
return ret;
}

Expand All @@ -343,22 +383,28 @@ int PYTHON_PLUGIN_FUNCTION (Get) (ckdb::Plugin * handle, ckdb::KeySet * returned
}

moduleData * data = static_cast<moduleData *> (elektraPluginGetData (handle));
if (data != nullptr) return Python_CallFunction_Helper2 (data, "get", returned, parentKey);
return 0;
if (data == nullptr) return 0;
if (elektraPluginProcessIsParent (data->pp))
return elektraPluginProcessSend (data->pp, ELEKTRA_PLUGINPROCESS_GET, returned, parentKey);
return Python_CallFunction_Helper2 (data, "get", returned, parentKey);
}

int PYTHON_PLUGIN_FUNCTION (Set) (ckdb::Plugin * handle, ckdb::KeySet * returned, ckdb::Key * parentKey)
{
moduleData * data = static_cast<moduleData *> (elektraPluginGetData (handle));
if (data != nullptr) return Python_CallFunction_Helper2 (data, "set", returned, parentKey);
return 0;
if (data == nullptr) return 0;
if (elektraPluginProcessIsParent (data->pp))
return elektraPluginProcessSend (data->pp, ELEKTRA_PLUGINPROCESS_GET, returned, parentKey);
return Python_CallFunction_Helper2 (data, "set", returned, parentKey);
}

int PYTHON_PLUGIN_FUNCTION (Error) (ckdb::Plugin * handle, ckdb::KeySet * returned, ckdb::Key * parentKey)
{
moduleData * data = static_cast<moduleData *> (elektraPluginGetData (handle));
if (data != nullptr) return Python_CallFunction_Helper2 (data, "error", returned, parentKey);
return 0;
if (data == nullptr) return 0;
if (elektraPluginProcessIsParent (data->pp))
return elektraPluginProcessSend (data->pp, ELEKTRA_PLUGINPROCESS_GET, returned, parentKey);
return Python_CallFunction_Helper2 (data, "error", returned, parentKey);
}

ckdb::Plugin * PYTHON_PLUGIN_EXPORT (PYTHON_PLUGIN_NAME)
Expand Down
31 changes: 16 additions & 15 deletions src/plugins/python2/CMakeLists.txt
View file
Expand Up @@ -2,12 +2,13 @@ include (LibAddBinding)

if (DEPENDENCY_PHASE)
find_package (Python2Libs 2.7 QUIET)
find_package (Pluginprocess)
find_swig ()
check_binding_included ("swig_python2" BINDING_WAS_ADDED SUBDIRECTORY "swig/python2" SILENT)

if (PYTHON2LIBS_VERSION_STRING MATCHES "^3\\.[0-9]+")
remove_plugin (python2 "python2 is for python 2.7 only and not ${PYTHON2LIBS_VERSION_STRING}")
elseif (PYTHON2LIBS_FOUND AND SWIG_FOUND AND BINDING_WAS_ADDED)
elseif (PYTHON2LIBS_FOUND AND SWIG_FOUND AND BINDING_WAS_ADDED AND PLUGINPROCESS_FOUND)
include (LibAddMacros)

add_custom_command (OUTPUT runtime.h
Expand All @@ -25,6 +26,8 @@ if (DEPENDENCY_PHASE)
remove_plugin (python2 "python 2 libs (libpython-dev) not found")
elseif (NOT BINDING_WAS_ADDED)
remove_plugin (python2 "swig_python2 binding is required")
elseif (NOT PLUGINPROCESS_FOUND)
remove_plugin (python "Elektra's pluginprocess library is required")
else ()
remove_plugin (python2 "swig not found")
endif ()
Expand All @@ -37,6 +40,7 @@ add_plugin (python2
${CMAKE_CURRENT_BINARY_DIR}/runtime.h
INCLUDE_DIRECTORIES ${PYTHON2_INCLUDE_DIR}
LINK_LIBRARIES ${PYTHON2_LIBRARIES}
LINK_ELEKTRA elektra-pluginprocess
COMPILE_DEFINITIONS SWIG_TYPE_TABLE=kdb
SWIG_RUNTIME=\"runtime.h\"
PYTHON_PLUGIN_NAME=python2
Expand All @@ -58,22 +62,19 @@ if (DEPENDENCY_PHASE)
endif ()

if (ADDTESTING_PHASE)
if (ENABLE_BROKEN_TESTS)
check_binding_was_added ("swig_python2" BINDING_WAS_ADDED) # bindings are required for tests
check_binding_was_added ("swig_python2" BINDING_WAS_ADDED) # bindings are required for tests

if (BUILD_TESTING AND BINDING_WAS_ADDED)
if (BUILD_TESTING AND BINDING_WAS_ADDED)

# test environment clears env so setting PYTHONPATH is no option we workaround this by changing the current
# directory to our bindings output directory + our test adds the current directory to pythons sys.path
add_plugintest (python2 MEMLEAK WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/../../bindings/swig/python2/)
# test environment clears env so setting PYTHONPATH is no option we workaround this by changing the current directory to our
# bindings output directory + our test adds the current directory to pythons sys.path
add_plugintest (python2 MEMLEAK WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/../../bindings/swig/python2/)

if (INSTALL_TESTING)
install (DIRECTORY ${CMAKE_SOURCE_DIR}/src/plugins/python/python/
DESTINATION "${TARGET_TEST_DATA_FOLDER}/python2")
endif ()

else ()
message (WARNING "BUILD_STATIC or BUILD_FULL and swig_python2 bindings are required for testing, test deactivated")
if (INSTALL_TESTING)
install (DIRECTORY ${CMAKE_SOURCE_DIR}/src/plugins/python/python/ DESTINATION "${TARGET_TEST_DATA_FOLDER}/python2")
endif ()
endif (ENABLE_BROKEN_TESTS)

else ()
message (WARNING "The swig_python2 binding is required for testing, test deactivated")
endif ()
endif ()

0 comments on commit 532666e

Please sign in to comment.