Pcbnew: Python scripting support clean up.

Always check the return value of PyRun_SimpleString() for errors when a
Python script is run and show an error message rather than fail silently.

Enable Python interpreter I/O redirection in debug builds so that script
string errors will be shown when PyRun_SimpleString() is called.

Do not call PyErr_Print() after a PyRun_SimpleString() call failure.  It
doesn't do anything useful.

Do not call Py_Finalize() after a PyRun_SimpleString() call failure.  It
seems to cause Pcbnew to crash.
This commit is contained in:
Wayne Stambaugh 2019-05-23 16:55:27 -04:00
parent 4ae5a006d0
commit 816f6db310
3 changed files with 81 additions and 65 deletions

View File

@ -303,19 +303,22 @@ static bool scriptingSetup()
void PythonPluginsReloadBase() void PythonPluginsReloadBase()
{ {
#if defined(KICAD_SCRIPTING) #if defined( KICAD_SCRIPTING )
//Reload plugin list: reload Python plugins if they are newer than // Reload plugin list: reload Python plugins if they are newer than
// the already loaded, and load new plugins // the already loaded, and load new plugins
char cmd[1024]; char cmd[1024];
snprintf( cmd, sizeof(cmd), snprintf( cmd, sizeof( cmd ),
"pcbnew.LoadPlugins(\"%s\")", TO_UTF8( PyScriptingPath() ) ); "pcbnew.LoadPlugins(\"%s\")", TO_UTF8( PyScriptingPath() ) );
PyLOCK lock; PyLOCK lock;
// ReRun the Python method pcbnew.LoadPlugins // ReRun the Python method pcbnew.LoadPlugins
// (already called when starting Pcbnew) // (already called when starting Pcbnew)
PyRun_SimpleString( cmd ); int retv = PyRun_SimpleString( cmd );
if( retv != 0 )
wxLogError( "Python error %d occurred running command:\n\n`%s`", retv, cmd );
#endif #endif
} }

View File

@ -2,7 +2,7 @@
* This program source code file is part of KiCad, a free EDA CAD application. * This program source code file is part of KiCad, a free EDA CAD application.
* *
* Copyright (C) 2012 NBEE Embedded Systems, Miguel Angel Ajo <miguelangel@nbee.es> * Copyright (C) 2012 NBEE Embedded Systems, Miguel Angel Ajo <miguelangel@nbee.es>
* Copyright (C) 1992-2018 KiCad Developers, see AUTHORS.txt for contributors. * Copyright (C) 1992-2019 KiCad Developers, see AUTHORS.txt for contributors.
* *
* This program is free software; you can redistribute it and/or * This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License * modify it under the terms of the GNU General Public License
@ -62,7 +62,9 @@ extern "C" void init_pcbnew( void );
struct _inittab* SwigImportInittab; struct _inittab* SwigImportInittab;
static int SwigNumModules = 0; static int SwigNumModules = 0;
static bool wxPythonLoaded = false; // true if the wxPython scripting layer was successfully loaded /// True if the wxPython scripting layer was successfully loaded.
static bool wxPythonLoaded = false;
bool IsWxPythonLoaded() bool IsWxPythonLoaded()
{ {
@ -70,7 +72,9 @@ bool IsWxPythonLoaded()
} }
/* Add a name + initfuction to our SwigImportInittab */ /**
* Add a name + initfuction to our SwigImportInittab
*/
#if PY_MAJOR_VERSION >= 3 #if PY_MAJOR_VERSION >= 3
static void swigAddModule( const char* name, PyObject* (* initfunc)() ) static void swigAddModule( const char* name, PyObject* (* initfunc)() )
@ -86,8 +90,9 @@ static void swigAddModule( const char* name, void (* initfunc)() )
} }
/* Add the builtin python modules */ /**
* Add the builtin python modules
*/
static void swigAddBuiltin() static void swigAddBuiltin()
{ {
int i = 0; int i = 0;
@ -111,12 +116,9 @@ static void swigAddBuiltin()
} }
/* Function swigAddModules /**
* adds the internal modules we offer to the python scripting, so they will be * Add the internal modules to the python scripting so they will be available to the scripts.
* available to the scripts we run.
*
*/ */
static void swigAddModules() static void swigAddModules()
{ {
#if PY_MAJOR_VERSION >= 3 #if PY_MAJOR_VERSION >= 3
@ -132,27 +134,28 @@ static void swigAddModules()
} }
/* Function swigSwitchPythonBuiltin /**
* switches python module table to our built one . * Switch the python module table to the Pcbnew built one.
*
*/ */
static void swigSwitchPythonBuiltin() static void swigSwitchPythonBuiltin()
{ {
PyImport_Inittab = SwigImportInittab; PyImport_Inittab = SwigImportInittab;
} }
/* Function pcbnewInitPythonScripting
* Initializes all the python environment and publish our interface inside it
* initializes all the wxpython interface, and returns the python thread control structure
*
*/
PyThreadState* g_PythonMainTState; PyThreadState* g_PythonMainTState;
/**
* Initialize the python environment and publish the Pcbnew interface inside it.
*
* This initializes all the wxPython interface and returns the python thread control structure
*/
bool pcbnewInitPythonScripting( const char * aUserScriptingPath ) bool pcbnewInitPythonScripting( const char * aUserScriptingPath )
{ {
int retv;
char cmd[1024];
swigAddBuiltin(); // add builtin functions swigAddBuiltin(); // add builtin functions
swigAddModules(); // add our own modules swigAddModules(); // add our own modules
swigSwitchPythonBuiltin(); // switch the python builtin modules to our new list swigSwitchPythonBuiltin(); // switch the python builtin modules to our new list
@ -164,20 +167,17 @@ bool pcbnewInitPythonScripting( const char * aUserScriptingPath )
PyEval_InitThreads(); PyEval_InitThreads();
#ifndef KICAD_SCRIPTING_WXPYTHON_PHOENIX #ifndef KICAD_SCRIPTING_WXPYTHON_PHOENIX
#ifndef __WINDOWS__ // import wxversion.py currently not working under winbuilder, and not useful. #ifndef __WINDOWS__ // import wxversion.py currently not working under winbuilder, and not useful.
char cmd[1024];
// Make sure that that the correct version of wxPython is loaded. In systems where there // Make sure that that the correct version of wxPython is loaded. In systems where there
// are different versions of wxPython installed this can lead to select wrong wxPython // are different versions of wxPython installed this can lead to select wrong wxPython
// version being selected. // version being selected.
snprintf( cmd, sizeof( cmd ), "import wxversion; wxversion.select( '%s' )", WXPYTHON_VERSION ); snprintf( cmd, sizeof( cmd ), "import wxversion; wxversion.select( '%s' )", WXPYTHON_VERSION );
int retv = PyRun_SimpleString( cmd ); retv = PyRun_SimpleString( cmd );
if( retv != 0 ) if( retv != 0 )
{ {
wxLogError( wxT( "Python error %d occurred running string `%s`" ), retv, cmd ); wxLogError( "Python error %d occurred running command:\n\n`%s`", retv, cmd );
PyErr_Print();
Py_Finalize();
return false; return false;
} }
#endif // ifndef __WINDOWS__ #endif // ifndef __WINDOWS__
@ -187,13 +187,17 @@ bool pcbnewInitPythonScripting( const char * aUserScriptingPath )
// internally by the rest of the API functions. // internally by the rest of the API functions.
if( !wxPyCoreAPI_IMPORT() ) if( !wxPyCoreAPI_IMPORT() )
{ {
wxLogError( wxT( "***** Error importing the wxPython API! *****" ) ); wxLogError( "***** Error importing the wxPython API! *****" );
PyErr_Print(); PyErr_Print();
Py_Finalize(); Py_Finalize();
return false; return false;
} }
#endif #endif
#if defined( DEBUG )
RedirectStdio();
#endif
wxPythonLoaded = true; wxPythonLoaded = true;
// Save the current Python thread state and release the // Save the current Python thread state and release the
@ -202,15 +206,18 @@ bool pcbnewInitPythonScripting( const char * aUserScriptingPath )
#endif // ifdef KICAD_SCRIPTING_WXPYTHON #endif // ifdef KICAD_SCRIPTING_WXPYTHON
// load pcbnew inside python, and load all the user plugins, TODO: add system wide plugins // Load pcbnew inside Python and load all the user plugins, TODO: add system wide plugins
{ {
char loadCmd[1024];
PyLOCK lock; PyLOCK lock;
snprintf( loadCmd, sizeof(loadCmd), "import sys, traceback\n"
"sys.path.append(\".\")\n" snprintf( cmd, sizeof( cmd ), "import sys, traceback\n"
"import pcbnew\n" "sys.path.append(\".\")\n"
"pcbnew.LoadPlugins(\"%s\")", aUserScriptingPath ); "import pcbnew\n"
PyRun_SimpleString( loadCmd ); "pcbnew.LoadPlugins(\"%s\")", aUserScriptingPath );
retv = PyRun_SimpleString( cmd );
if( retv != 0 )
wxLogError( "Python error %d occurred running command:\n\n`%s`", retv, cmd );
} }
return true; return true;
@ -218,9 +225,10 @@ bool pcbnewInitPythonScripting( const char * aUserScriptingPath )
/** /**
* this function runs a python method from pcbnew module, which returns a string * Run a python method from the pcbnew module.
*
* @param aMethodName is the name of the method (like "pcbnew.myfunction" ) * @param aMethodName is the name of the method (like "pcbnew.myfunction" )
* @param aNames will contains the returned string * @param aNames will contain the returned string
*/ */
static void pcbnewRunPythonMethodWithReturnedString( const char* aMethodName, wxString& aNames ) static void pcbnewRunPythonMethodWithReturnedString( const char* aMethodName, wxString& aNames )
{ {
@ -253,9 +261,11 @@ static void pcbnewRunPythonMethodWithReturnedString( const char* aMethodName, wx
PyObject* str = PyDict_GetItemString(localDict, "result" ); PyObject* str = PyDict_GetItemString(localDict, "result" );
#if PY_MAJOR_VERSION >= 3 #if PY_MAJOR_VERSION >= 3
const char* str_res = NULL; const char* str_res = NULL;
if(str) if(str)
{ {
PyObject* temp_bytes = PyUnicode_AsEncodedString( str, "UTF-8", "strict" ); PyObject* temp_bytes = PyUnicode_AsEncodedString( str, "UTF-8", "strict" );
if( temp_bytes != NULL ) if( temp_bytes != NULL )
{ {
str_res = PyBytes_AS_STRING( temp_bytes ); str_res = PyBytes_AS_STRING( temp_bytes );
@ -277,7 +287,7 @@ static void pcbnewRunPythonMethodWithReturnedString( const char* aMethodName, wx
Py_DECREF( localDict ); Py_DECREF( localDict );
if( PyErr_Occurred() ) if( PyErr_Occurred() )
wxLogMessage(PyErrStringWithTraceback()); wxLogMessage( PyErrStringWithTraceback() );
} }
@ -292,6 +302,7 @@ void pcbnewGetScriptsSearchPaths( wxString& aNames )
pcbnewRunPythonMethodWithReturnedString( "pcbnew.GetWizardsSearchPaths", aNames ); pcbnewRunPythonMethodWithReturnedString( "pcbnew.GetWizardsSearchPaths", aNames );
} }
void pcbnewGetWizardsBackTrace( wxString& aNames ) void pcbnewGetWizardsBackTrace( wxString& aNames )
{ {
pcbnewRunPythonMethodWithReturnedString( "pcbnew.GetWizardsBackTrace", aNames ); pcbnewRunPythonMethodWithReturnedString( "pcbnew.GetWizardsBackTrace", aNames );
@ -308,7 +319,6 @@ void pcbnewFinishPythonScripting()
#if defined( KICAD_SCRIPTING_WXPYTHON ) #if defined( KICAD_SCRIPTING_WXPYTHON )
void RedirectStdio() void RedirectStdio()
{ {
// This is a helpful little tidbit to help debugging and such. It // This is a helpful little tidbit to help debugging and such. It
@ -322,7 +332,10 @@ void RedirectStdio()
PyLOCK lock; PyLOCK lock;
PyRun_SimpleString( python_redirect ); int retv = PyRun_SimpleString( python_redirect );
if( retv != 0 )
wxLogError( "Python error %d occurred running command:\n\n`%s`", retv, python_redirect );
} }
@ -370,7 +383,8 @@ wxWindow* CreatePythonShellWindow( wxWindow* parent, const wxString& aFramenameI
auto app_ptr = wxTheApp; auto app_ptr = wxTheApp;
#endif #endif
// Execute the code to make the makeWindow function we defined above // Execute the code to make the makeWindow function we defined above
PyObject* result = PyRun_String( pcbnew_pyshell_one_step.str().c_str(), Py_file_input, globals, globals ); PyObject* result = PyRun_String( pcbnew_pyshell_one_step.str().c_str(), Py_file_input,
globals, globals );
#ifdef KICAD_SCRIPTING_WXPYTHON_PHOENIX #ifdef KICAD_SCRIPTING_WXPYTHON_PHOENIX
// This absolutely ugly hack is to work around the pyshell re-writing the global // This absolutely ugly hack is to work around the pyshell re-writing the global
@ -395,7 +409,7 @@ wxWindow* CreatePythonShellWindow( wxWindow* parent, const wxString& aFramenameI
if( !PyInt_Check( result ) ) if( !PyInt_Check( result ) )
#endif #endif
{ {
wxLogError("creation of scripting window didn't return a number"); wxLogError( "creation of scripting window didn't return a number" );
return NULL; return NULL;
} }
@ -414,16 +428,15 @@ wxWindow* CreatePythonShellWindow( wxWindow* parent, const wxString& aFramenameI
if( !window ) if( !window )
{ {
wxLogError("unable to find pyshell window with id %d", windowId); wxLogError( "unable to find pyshell window with id %d", windowId );
return NULL; return NULL;
} }
return window; return window;
} }
#endif #endif
wxString PyStringToWx( PyObject* aString ) wxString PyStringToWx( PyObject* aString )
{ {
wxString ret; wxString ret;
@ -434,6 +447,7 @@ wxString PyStringToWx( PyObject* aString )
#if PY_MAJOR_VERSION >= 3 #if PY_MAJOR_VERSION >= 3
const char* str_res = NULL; const char* str_res = NULL;
PyObject* temp_bytes = PyUnicode_AsEncodedString( aString, "UTF-8", "strict" ); PyObject* temp_bytes = PyUnicode_AsEncodedString( aString, "UTF-8", "strict" );
if( temp_bytes != NULL ) if( temp_bytes != NULL )
{ {
str_res = PyBytes_AS_STRING( temp_bytes ); str_res = PyBytes_AS_STRING( temp_bytes );
@ -471,6 +485,7 @@ wxArrayString PyArrayStringToWx( PyObject* aArrayString )
#if PY_MAJOR_VERSION >= 3 #if PY_MAJOR_VERSION >= 3
const char* str_res = NULL; const char* str_res = NULL;
PyObject* temp_bytes = PyUnicode_AsEncodedString( element, "UTF-8", "strict" ); PyObject* temp_bytes = PyUnicode_AsEncodedString( element, "UTF-8", "strict" );
if( temp_bytes != NULL ) if( temp_bytes != NULL )
{ {
str_res = PyBytes_AS_STRING( temp_bytes ); str_res = PyBytes_AS_STRING( temp_bytes );
@ -505,6 +520,7 @@ wxString PyErrStringWithTraceback()
PyErr_Fetch( &type, &value, &traceback ); PyErr_Fetch( &type, &value, &traceback );
PyErr_NormalizeException( &type, &value, &traceback ); PyErr_NormalizeException( &type, &value, &traceback );
if( traceback == NULL ) if( traceback == NULL )
{ {
traceback = Py_None; traceback = Py_None;
@ -545,14 +561,15 @@ wxString PyErrStringWithTraceback()
return err; return err;
} }
/** /**
* Find the Python scripting path * Find the Python scripting path.
*/ */
wxString PyScriptingPath() wxString PyScriptingPath()
{ {
wxString path; wxString path;
//TODO should this be a user configurable variable eg KISCRIPT ? //@todo This should this be a user configurable variable eg KISCRIPT?
#if defined( __WXMAC__ ) #if defined( __WXMAC__ )
path = GetOSXKicadDataDir() + wxT( "/scripting" ); path = GetOSXKicadDataDir() + wxT( "/scripting" );
#else #else
@ -571,6 +588,7 @@ wxString PyScriptingPath()
return path; return path;
} }
wxString PyPluginsPath() wxString PyPluginsPath()
{ {
// Note we are using unix path separator, because window separator sometimes // Note we are using unix path separator, because window separator sometimes

View File

@ -1,7 +1,7 @@
/* /*
* This program source code file is part of KiCad, a free EDA CAD application. * This program source code file is part of KiCad, a free EDA CAD application.
* *
* Copyright (C) 1992-2016 KiCad Developers, see AUTHORS.txt for contributors. * Copyright (C) 1992-2019 KiCad Developers, see AUTHORS.txt for contributors.
* *
* This program is free software; you can redistribute it and/or * This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License * modify it under the terms of the GNU General Public License
@ -50,31 +50,30 @@
#include <wx/string.h> #include <wx/string.h>
#include <wx/arrstr.h> #include <wx/arrstr.h>
/** /**
* Function pcbnewInitPythonScripting * Initialize the Python engine inside pcbnew.
* Initializes the Python engine inside pcbnew
*/ */
bool pcbnewInitPythonScripting( const char * aUserScriptingPath ); bool pcbnewInitPythonScripting( const char * aUserScriptingPath );
void pcbnewFinishPythonScripting(); void pcbnewFinishPythonScripting();
/** /**
* Function pcbnewGetUnloadableScriptNames * Collect the list of python scripts which could not be loaded.
* collects the list of python scripts which could not be loaded because *
* some error (synthax error) happened
* @param aNames is a wxString which will contain the filenames (separated by '\n') * @param aNames is a wxString which will contain the filenames (separated by '\n')
*/ */
void pcbnewGetUnloadableScriptNames( wxString& aNames ); void pcbnewGetUnloadableScriptNames( wxString& aNames );
/** /**
* Function pcbnewGetScriptsSearchPaths * Collect the list of paths where python scripts are searched.
* collects the list of paths where python scripts are searched *
* @param aNames is a wxString which will contain the paths (separated by '\n') * @param aNames is a wxString which will contain the paths (separated by '\n')
*/ */
void pcbnewGetScriptsSearchPaths( wxString& aNames ); void pcbnewGetScriptsSearchPaths( wxString& aNames );
/** /**
* Function pcbnewGetWizardsBackTrace * Return the backtrace of errors (if any) when wizard python scripts are loaded.
* returns the backtrace of errors (if any) when wizard python scripts are loaded *
* @param aNames is a wxString which will contain the trace * @param aNames is a wxString which will contain the trace
*/ */
void pcbnewGetWizardsBackTrace( wxString& aNames ); void pcbnewGetWizardsBackTrace( wxString& aNames );
@ -84,7 +83,6 @@ void RedirectStdio();
wxWindow* CreatePythonShellWindow( wxWindow* parent, const wxString& aFramenameId ); wxWindow* CreatePythonShellWindow( wxWindow* parent, const wxString& aFramenameId );
#endif #endif
#if 0 && defined (KICAD_SCRIPTING_WXPYTHON) #if 0 && defined (KICAD_SCRIPTING_WXPYTHON)
// This definition of PyLOCK crashed Pcbnew under some conditions (JPC), // This definition of PyLOCK crashed Pcbnew under some conditions (JPC),
// especially reloading plugins // especially reloading plugins
@ -98,8 +96,6 @@ public:
PyLOCK() { b = wxPyBeginBlockThreads(); } PyLOCK() { b = wxPyBeginBlockThreads(); }
~PyLOCK() { wxPyEndBlockThreads( b ); } ~PyLOCK() { wxPyEndBlockThreads( b ); }
}; };
#else #else
class PyLOCK class PyLOCK
{ {
@ -108,7 +104,6 @@ public:
PyLOCK() { gil_state = PyGILState_Ensure(); } PyLOCK() { gil_state = PyGILState_Ensure(); }
~PyLOCK() { PyGILState_Release( gil_state ); } ~PyLOCK() { PyGILState_Release( gil_state ); }
}; };
#endif #endif
wxString PyStringToWx( PyObject* str ); wxString PyStringToWx( PyObject* str );