<rdar://problem/12586188> Make ImportError a special case for "command script import", such that the error message for the exception becomes the error for the entire import operation

and silence the backtrace printout

In the process, refactor the Execute* commands in ScriptInterpreter to take an options object, and add a new setting to not mask out errors so that the callers can handle them directly
instead of having the default behavior

llvm-svn: 167067

10 files changed, 170 insertions, 43 deletions

diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index fe2096222b3..09b80b7e238 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -128,11 +128,65 @@ public:
 
     virtual ~ScriptInterpreter ();
 
+    struct ExecuteScriptOptions
+    {
+    public:
+        ExecuteScriptOptions () :
+            m_enable_io(true),
+            m_set_lldb_globals(true),
+            m_maskout_errors(true)
+        {
+        }
+        
+        bool
+        GetEnableIO () const
+        {
+            return m_enable_io;
+        }
+        
+        bool
+        GetSetLLDBGlobals () const
+        {
+            return m_set_lldb_globals;
+        }
+        
+        bool
+        GetMaskoutErrors () const
+        {
+            return m_maskout_errors;
+        }
+        
+        ExecuteScriptOptions&
+        SetEnableIO (bool enable)
+        {
+            m_enable_io = enable;
+            return *this;
+        }
+
+        ExecuteScriptOptions&
+        SetSetLLDBGlobals (bool set)
+        {
+            m_set_lldb_globals = set;
+            return *this;
+        }
+
+        ExecuteScriptOptions&
+        SetMaskoutErrors (bool maskout)
+        {
+            m_maskout_errors = maskout;
+            return *this;
+        }
+        
+    private:
+        bool m_enable_io;
+        bool m_set_lldb_globals;
+        bool m_maskout_errors;
+    };
+    
     virtual bool
     ExecuteOneLine (const char *command,
                     CommandReturnObject *result,
-                    bool enable_io,
-                    bool set_lldb_globals = true) = 0;
+                    const ExecuteScriptOptions &options = ExecuteScriptOptions()) = 0;
 
     virtual void
     ExecuteInterpreterLoop () = 0;
@@ -141,16 +195,14 @@ public:
     ExecuteOneLineWithReturn (const char *in_string,
                               ScriptReturnType return_type,
                               void *ret_value,
-                              bool enable_io,
-                              bool set_lldb_globals = true)
+                              const ExecuteScriptOptions &options = ExecuteScriptOptions())
     {
         return true;
     }
 
     virtual bool
     ExecuteMultipleLines (const char *in_string,
-                          bool enable_io,
-                          bool set_lldb_globals = true)
+                          const ExecuteScriptOptions &options = ExecuteScriptOptions())
     {
         return true;
     }
diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreterNone.h b/lldb/include/lldb/Interpreter/ScriptInterpreterNone.h
index 46beef60e1d..6c82b60b0bc 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreterNone.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreterNone.h
@@ -23,7 +23,7 @@ public:
     ~ScriptInterpreterNone ();
 
     bool
-    ExecuteOneLine (const char *command, CommandReturnObject *result, bool enable_io, bool set_lldb_globals = true);
+    ExecuteOneLine (const char *command, CommandReturnObject *result, const ExecuteScriptOptions &options = ExecuteScriptOptions());
 
     void
     ExecuteInterpreterLoop ();
diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreterPython.h b/lldb/include/lldb/Interpreter/ScriptInterpreterPython.h
index e94eb368408..9268867ea1d 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreterPython.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreterPython.h
@@ -41,8 +41,7 @@ public:
     bool
     ExecuteOneLine (const char *command,
                     CommandReturnObject *result,
-                    bool enable_io,
-                    bool set_lldb_globals = true);
+                    const ExecuteScriptOptions &options = ExecuteScriptOptions());
 
     void
     ExecuteInterpreterLoop ();
@@ -51,13 +50,11 @@ public:
     ExecuteOneLineWithReturn (const char *in_string, 
                               ScriptInterpreter::ScriptReturnType return_type,
                               void *ret_value,
-                              bool enable_io,
-                              bool set_lldb_globals = true);
+                              const ExecuteScriptOptions &options = ExecuteScriptOptions());
 
     bool
     ExecuteMultipleLines (const char *in_string,
-                          bool enable_io,
-                          bool set_lldb_globals = true);
+                          const ExecuteScriptOptions &options = ExecuteScriptOptions());
 
     bool
     ExportFunctionDefinitionToInterpreter (StringList &function_def);
diff --git a/lldb/source/Interpreter/CommandObjectScript.cpp b/lldb/source/Interpreter/CommandObjectScript.cpp
index ce097b79670..6dd2f7aba76 100644
--- a/lldb/source/Interpreter/CommandObjectScript.cpp
+++ b/lldb/source/Interpreter/CommandObjectScript.cpp
@@ -68,7 +68,7 @@ CommandObjectScript::DoExecute
     }
 
     // We can do better when reporting the status of one-liner script execution.
-    if (script_interpreter->ExecuteOneLine (command, &result, true))
+    if (script_interpreter->ExecuteOneLine (command, &result))
         result.SetStatus(eReturnStatusSuccessFinishNoResult);
     else
         result.SetStatus(eReturnStatusFailed);
diff --git a/lldb/source/Interpreter/ScriptInterpreterNone.cpp b/lldb/source/Interpreter/ScriptInterpreterNone.cpp
index a23ce37f038..d178011d004 100644
--- a/lldb/source/Interpreter/ScriptInterpreterNone.cpp
+++ b/lldb/source/Interpreter/ScriptInterpreterNone.cpp
@@ -26,7 +26,7 @@ ScriptInterpreterNone::~ScriptInterpreterNone ()
 }
 
 bool
-ScriptInterpreterNone::ExecuteOneLine (const char *command, CommandReturnObject *, bool enable_io, bool set_lldb_globals)
+ScriptInterpreterNone::ExecuteOneLine (const char *command, CommandReturnObject *, const ExecuteScriptOptions&)
 {
     m_interpreter.GetDebugger().GetErrorStream().PutCString ("error: there is no embedded script interpreter in this mode.\n");
     return false;
diff --git a/lldb/source/Interpreter/ScriptInterpreterPython.cpp b/lldb/source/Interpreter/ScriptInterpreterPython.cpp
index 9457ef9e24c..ced46459878 100644
--- a/lldb/source/Interpreter/ScriptInterpreterPython.cpp
+++ b/lldb/source/Interpreter/ScriptInterpreterPython.cpp
@@ -709,7 +709,7 @@ GenerateUniqueName (const char* base_name_wanted,
 }
 
 bool
-ScriptInterpreterPython::ExecuteOneLine (const char *command, CommandReturnObject *result, bool enable_io, bool set_lldb_globals)
+ScriptInterpreterPython::ExecuteOneLine (const char *command, CommandReturnObject *result, const ExecuteScriptOptions &options)
 {
     if (!m_valid_session)
         return false;
@@ -720,8 +720,8 @@ ScriptInterpreterPython::ExecuteOneLine (const char *command, CommandReturnObjec
     // method to pass the command string directly down to Python.
 
     Locker locker(this,
-                  ScriptInterpreterPython::Locker::AcquireLock | (set_lldb_globals ? ScriptInterpreterPython::Locker::InitSession : 0),
-                  ScriptInterpreterPython::Locker::FreeAcquiredLock | (set_lldb_globals ? ScriptInterpreterPython::Locker::TearDownSession : 0));
+                  ScriptInterpreterPython::Locker::AcquireLock | (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::InitSession : 0),
+                  ScriptInterpreterPython::Locker::FreeAcquiredLock | (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::TearDownSession : 0));
 
     bool success = false;
 
@@ -764,7 +764,7 @@ ScriptInterpreterPython::ExecuteOneLine (const char *command, CommandReturnObjec
                         {
                             PyObject *pvalue = NULL;
                             { // scope for PythonInputReaderManager
-                                PythonInputReaderManager py_input(enable_io ? this : NULL);
+                                PythonInputReaderManager py_input(options.GetEnableIO() ? this : NULL);
                                 pvalue = PyObject_CallObject (pfunc, pargs);
                             }
                             Py_DECREF (pargs);
@@ -773,7 +773,7 @@ ScriptInterpreterPython::ExecuteOneLine (const char *command, CommandReturnObjec
                                 Py_DECREF (pvalue);
                                 success = true;
                             }
-                            else if (PyErr_Occurred ())
+                            else if (options.GetMaskoutErrors() && PyErr_Occurred ())
                             {
                                 PyErr_Print();
                                 PyErr_Clear();
@@ -988,13 +988,12 @@ bool
 ScriptInterpreterPython::ExecuteOneLineWithReturn (const char *in_string,
                                                    ScriptInterpreter::ScriptReturnType return_type,
                                                    void *ret_value,
-                                                   bool enable_io,
-                                                   bool set_lldb_globals)
+                                                   const ExecuteScriptOptions &options)
 {
 
     Locker locker(this,
-                  ScriptInterpreterPython::Locker::AcquireLock | (set_lldb_globals ? ScriptInterpreterPython::Locker::InitSession : 0),
-                  ScriptInterpreterPython::Locker::FreeAcquiredLock | (set_lldb_globals ? ScriptInterpreterPython::Locker::TearDownSession : 0));
+                  ScriptInterpreterPython::Locker::AcquireLock | (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::InitSession : 0),
+                  ScriptInterpreterPython::Locker::FreeAcquiredLock | (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::TearDownSession : 0));
 
     PyObject *py_return = NULL;
     PyObject *mainmod = PyImport_AddModule ("__main__");
@@ -1026,7 +1025,7 @@ ScriptInterpreterPython::ExecuteOneLineWithReturn (const char *in_string,
     if (in_string != NULL)
     {
         { // scope for PythonInputReaderManager
-            PythonInputReaderManager py_input(enable_io ? this : NULL);
+            PythonInputReaderManager py_input(options.GetEnableIO() ? this : NULL);
             py_return = PyRun_String (in_string, Py_eval_input, globals, locals);
             if (py_return == NULL)
             { 
@@ -1144,23 +1143,26 @@ ScriptInterpreterPython::ExecuteOneLineWithReturn (const char *in_string,
     py_error = PyErr_Occurred();
     if (py_error != NULL)
     {
-        if (PyErr_GivenExceptionMatches (py_error, PyExc_SyntaxError))
-            PyErr_Print ();
-        PyErr_Clear();
         ret_success = false;
+        if (options.GetMaskoutErrors())
+        {
+            if (PyErr_GivenExceptionMatches (py_error, PyExc_SyntaxError))
+                PyErr_Print ();
+            PyErr_Clear();
+        }
     }
 
     return ret_success;
 }
 
 bool
-ScriptInterpreterPython::ExecuteMultipleLines (const char *in_string, bool enable_io, bool set_lldb_globals)
+ScriptInterpreterPython::ExecuteMultipleLines (const char *in_string, const ExecuteScriptOptions &options)
 {
     
     
     Locker locker(this,
-                  ScriptInterpreterPython::Locker::AcquireLock | (set_lldb_globals ? ScriptInterpreterPython::Locker::InitSession : 0),
-                  ScriptInterpreterPython::Locker::FreeAcquiredLock | (set_lldb_globals ? ScriptInterpreterPython::Locker::TearDownSession : 0));
+                  ScriptInterpreterPython::Locker::AcquireLock | (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::InitSession : 0),
+                  ScriptInterpreterPython::Locker::FreeAcquiredLock | (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::TearDownSession : 0));
 
     bool success = false;
     PyObject *py_return = NULL;
@@ -1197,7 +1199,7 @@ ScriptInterpreterPython::ExecuteMultipleLines (const char *in_string, bool enabl
             if (compiled_code)
             {
                 { // scope for PythonInputReaderManager
-                    PythonInputReaderManager py_input(enable_io ? this : NULL);
+                    PythonInputReaderManager py_input(options.GetEnableIO() ? this : NULL);
                     py_return = PyEval_EvalCode (compiled_code, globals, locals);
                 }
                 if (py_return != NULL)
@@ -1214,10 +1216,13 @@ ScriptInterpreterPython::ExecuteMultipleLines (const char *in_string, bool enabl
     py_error = PyErr_Occurred ();
     if (py_error != NULL)
     {
-        if (PyErr_GivenExceptionMatches (py_error, PyExc_SyntaxError))
-            PyErr_Print ();
-        PyErr_Clear();
         success = false;
+        if (options.GetMaskoutErrors())
+        {
+            if (PyErr_GivenExceptionMatches (py_error, PyExc_SyntaxError))
+                PyErr_Print ();
+            PyErr_Clear();
+        }
     }
 
     return success;
@@ -1555,7 +1560,7 @@ ScriptInterpreterPython::ExportFunctionDefinitionToInterpreter (StringList &func
     // Convert StringList to one long, newline delimited, const char *.
     std::string function_def_string(function_def.CopyList());
 
-    return ExecuteMultipleLines (function_def_string.c_str(), false);
+    return ExecuteMultipleLines (function_def_string.c_str(), ScriptInterpreter::ExecuteScriptOptions().SetEnableIO(false));
 }
 
 bool
@@ -2422,7 +2427,7 @@ ScriptInterpreterPython::LoadScriptingModule (const char* pathname,
         command_stream.Printf("if not (sys.path.__contains__('%s')):\n    sys.path.append('%s');\n\n",
                               directory,
                               directory);
-        bool syspath_retval = ExecuteMultipleLines(command_stream.GetData(), false, false);
+        bool syspath_retval = ExecuteMultipleLines(command_stream.GetData(), ScriptInterpreter::ExecuteScriptOptions().SetEnableIO(false).SetSetLLDBGlobals(false));
         if (!syspath_retval)
         {
             error.SetErrorString("Python sys.path handling failed");
@@ -2445,8 +2450,7 @@ ScriptInterpreterPython::LoadScriptingModule (const char* pathname,
         bool was_imported = (ExecuteOneLineWithReturn(command_stream.GetData(),
                                                       ScriptInterpreterPython::eScriptReturnTypeInt,
                                                       &refcount,
-                                                      false,
-                                                      false) && refcount > 0);
+                                                      ScriptInterpreter::ExecuteScriptOptions().SetEnableIO(false).SetSetLLDBGlobals(false)) && refcount > 0);
         if (was_imported == true && can_reload == false)
         {
             error.SetErrorString("module already imported");
@@ -2456,13 +2460,43 @@ ScriptInterpreterPython::LoadScriptingModule (const char* pathname,
         // now actually do the import
         command_stream.Clear();
         command_stream.Printf("import %s",basename.c_str());
-        bool import_retval = ExecuteOneLine(command_stream.GetData(), NULL, false, false);
-        if (!import_retval)
+        bool import_retval = ExecuteMultipleLines(command_stream.GetData(), ScriptInterpreter::ExecuteScriptOptions().SetEnableIO(false).SetSetLLDBGlobals(false).SetMaskoutErrors(false));
+        PyObject* py_error = PyErr_Occurred(); // per Python docs: "you do not need to Py_DECREF()" the return of this function
+        
+        if (py_error || !import_retval) // check for failure of the import
         {
-            error.SetErrorString("Python import statement failed");
+            if (py_error) // if we have a Python error..
+            {
+                if (PyErr_GivenExceptionMatches (py_error, PyExc_ImportError)) // and it is an ImportError
+                {
+                    PyObject *type,*value,*traceback;
+                    PyErr_Fetch (&type,&value,&traceback);
+                    
+                    if (value && value != Py_None)
+                        error.SetErrorString(PyString_AsString(PyObject_Str(value)));
+                    else
+                        error.SetErrorString("ImportError raised by imported module");
+                    
+                    Py_XDECREF(type);
+                    Py_XDECREF(value);
+                    Py_XDECREF(traceback);
+                }
+                else // any other error
+                {
+                    error.SetErrorString("Python raised an error while importing module");
+                }
+            }
+            else // we failed but have no error to explain why
+            {
+                error.SetErrorString("unknown error while importing module");
+            }
+            
+            // anyway, clear the error indicator and return false
+            PyErr_Clear();
             return false;
         }
         
+        // if we are here, everything worked
         // call __lldb_init_module(debugger,dict)
         if (!g_swig_call_module_init (basename,
                                         m_dictionary_name.c_str(),
@@ -2569,7 +2603,7 @@ ScriptInterpreterPython::GetDocumentationForItem(const char* item, std::string&
     
     if (ExecuteOneLineWithReturn (command.c_str(),
                                  ScriptInterpreter::eScriptReturnTypeCharStrOrNone,
-                                 &result_ptr, false))
+                                  &result_ptr, ScriptInterpreter::ExecuteScriptOptions().SetEnableIO(false) /*.SetSetLLDBGlobals(false)*/))
     {
         if (result_ptr)
             dest.assign(result_ptr);
diff --git a/lldb/test/functionalities/command_script/import/rdar-12586188/Makefile b/lldb/test/functionalities/command_script/import/rdar-12586188/Makefile
new file mode 100644
index 00000000000..7913aaa4b74
--- /dev/null
+++ b/lldb/test/functionalities/command_script/import/rdar-12586188/Makefile
@@ -0,0 +1,3 @@
+LEVEL = ../../../../make
+
+include $(LEVEL)/Makefile.rules
diff --git a/lldb/test/functionalities/command_script/import/rdar-12586188/TestRdar12586188.py b/lldb/test/functionalities/command_script/import/rdar-12586188/TestRdar12586188.py
new file mode 100644
index 00000000000..6e79bb2b577
--- /dev/null
+++ b/lldb/test/functionalities/command_script/import/rdar-12586188/TestRdar12586188.py
@@ -0,0 +1,33 @@
+"""Check that we handle an ImportError in a special way when command script importing files."""
+
+import os, sys, time
+import unittest2
+import lldb
+from lldbtest import *
+
+class Rdar12586188TestCase(TestBase):
+
+    mydir = os.path.join("functionalities", "command_script", "import", "rdar-12586188")
+
+    @python_api_test
+    def test_rdar12586188_command(self):
+        """Check that we handle an ImportError in a special way when command script importing files."""
+        self.run_test()
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+
+    def run_test(self):
+        """Check that we handle an ImportError in a special way when command script importing files."""
+
+        self.expect("command script import ./fail12586188.py --allow-reload",
+                error=True, substrs = ['error: module importing failed: I do not want to be imported'])
+        self.expect("command script import ./fail212586188.py --allow-reload",
+                error=True, substrs = ['error: module importing failed: Python raised an error while importing module'])
+
+if __name__ == '__main__':
+    import atexit
+    lldb.SBDebugger.Initialize()
+    atexit.register(lambda: lldb.SBDebugger.Terminate())
+    unittest2.main()
diff --git a/lldb/test/functionalities/command_script/import/rdar-12586188/fail12586188.py b/lldb/test/functionalities/command_script/import/rdar-12586188/fail12586188.py
new file mode 100644
index 00000000000..add85a73f85
--- /dev/null
+++ b/lldb/test/functionalities/command_script/import/rdar-12586188/fail12586188.py
@@ -0,0 +1,4 @@
+def f(x):
+	return x + 1
+
+raise ImportError("I do not want to be imported")
diff --git a/lldb/test/functionalities/command_script/import/rdar-12586188/fail212586188.py b/lldb/test/functionalities/command_script/import/rdar-12586188/fail212586188.py
new file mode 100644
index 00000000000..1549a036590
--- /dev/null
+++ b/lldb/test/functionalities/command_script/import/rdar-12586188/fail212586188.py
@@ -0,0 +1,4 @@
+def f(x):
+	return x + 1
+
+raise ValueError("I do not want to be imported")