BUG#29897911 UNABLE TO DEFINE A COMPLEX PYTHON PLUGIN: 'IMPORT' DOES NOT WORK

This patch fixes the plugin load logic so it:
- Properly adds the plugin path to the sys.path
- Properly defines the plugin __file__ variable

Plugin Group Support

Now, a plugin folder may contain either:
- A single plugin definition
- Multiple plugin definitions

This is determined by the presence of the initialization file, if it is
present, the plugin is assumed to be a single plugin. If it is not present
the plugin is assumed to be a plugin group, this is, it may contain the
definition of multiple plugins.

A plugin group is defined by the main plugin folder, containing sub-folders.
Each subfolder will be scanned for a standard plugin definition, and loaded
following the standard plugin load logic.

Only the main plugin folders support multiple plugin definitions, this is,
this logic is not recursive.

Change-Id: I06d550bb323db07ec3c1d0c641ed9bdc143bd021

Author: rennox

modules/mod_extensible_object.cc

@@ -341,6 +341,13 @@ void Extensible_object::register_member(
                                             name + "' member.");
   }
 
+  std::string target_object;
+  if (m_name.empty())
+    target_object = "an unregistered extension object";
+  else
+    target_object =
+        shcore::str_format("the '%s' extension object", m_name.c_str());
+
   if (s_allowed_member_types.find(value.type) == s_allowed_member_types.end()) {
     throw shcore::Exception::argument_error("Unsupported member type.");
   } else if (value.type == shcore::Object) {
@@ -353,15 +360,22 @@ void Extensible_object::register_member(
     object->m_definition = parse_member_definition(definition);
     object->m_definition->name = name;
     register_object(object);
+    log_debug(
+        "The '%s' extension object has been registered as a member into %s.",
+        name.c_str(), target_object.c_str());
   } else {
     if (value.type == shcore::Function) {
       auto fd = parse_function_definition(definition);
       fd->name = name;
       register_function(fd, value.as_function(), false);
+      log_debug("The '%s' function has been registered into %s.", name.c_str(),
+                target_object.c_str());
     } else {
       auto md = parse_member_definition(definition);
       md->name = name;
       register_property(md, value, false);
+      log_debug("The '%s' property has been registered into %s.", name.c_str(),
+                target_object.c_str());
     }
   }
 }
@@ -776,10 +790,10 @@ void Extensible_object::register_help(const std::string &brief,
       // Creates the help topic for the object
       auto type = is_global ? shcore::Topic_type::GLOBAL_OBJECT
                             : shcore::Topic_type::OBJECT;
-      help->add_help_topic(m_name, type, m_name, parent, mask);
+      help->add_help_topic(m_name, type, m_qualified_name, parent, mask);
 
       // Now registers the object brief, parameters and details
-      auto prefix = shcore::str_upper(m_name);
+      auto prefix = shcore::str_upper(m_qualified_name);
 
       help->add_help(prefix, "BRIEF", brief);
       help->add_help(prefix, "DETAIL", &m_detail_sequence, details);
@@ -1098,4 +1112,14 @@ void Extensible_object::disable_help() {
   }
 }
 
+std::string Extensible_object::get_help_id() const {
+  auto tokens = shcore::split_string(m_qualified_name, ".");
+  std::vector<std::string> id_tokens;
+  for (const auto &token : tokens) {
+    id_tokens.push_back(
+        shcore::get_member_name(token, shcore::current_naming_style()));
+  }
+  return shcore::str_join(id_tokens, ".");
+}
+
 }  // namespace mysqlsh

modules/mod_extensible_object.h

@@ -138,6 +138,7 @@ class Extensible_object
   std::string class_name() const override {
     return m_name.empty() ? "ExtensionObject" : m_name;
   }
+  std::string get_help_id() const override;
   bool operator==(const Object_bridge &other) const override;
 
   shcore::Value get_member(const std::string &prop) const override;

modules/mod_shell.cc

@@ -1495,6 +1495,9 @@ void Shell::register_report(const std::string &name, const std::string &type,
                             const shcore::Function_base_ref &report,
                             const shcore::Dictionary_t &description) {
   m_reports->register_report(name, type, report, description);
+
+  log_debug("The '%s' report of type '%s' has been registered.", name.c_str(),
+            type.c_str());
 }
 
 REGISTER_HELP_FUNCTION(createExtensionObject, shell);
@@ -1744,6 +1747,9 @@ void Shell::register_global(const std::string &name,
 
     object->set_registered(name);
 
+    log_debug(
+        "The '%s' extension object has been registered as a global object.",
+        name.c_str());
   } else {
     throw shcore::Exception::type_error(
         "Argument #2 is expected to be an extension object.");

mysqlshdk/include/scripting/common.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License, version 2.0,
@@ -53,4 +53,13 @@
 
 #define MYSH_FULL_VERSION MYSH_VERSION EXTRA_NAME_SUFFIX
 
+namespace shcore {
+// A plugin file contains the path, and bool indicating whether it is
+// A 1st level plugin (true) or a child plugin (false)
+struct Plugin_definition {
+  Plugin_definition(const std::string f, bool m) : file(f), main(m) {}
+  std::string file;
+  bool main;
+};
+}  // namespace shcore
 #endif

mysqlshdk/include/scripting/jscript_context.h

@@ -84,7 +84,7 @@ class SHCORE_PUBLIC JScript_context {
   std::string to_string(v8::Local<v8::Value> obj);
 
   std::string translate_exception(const v8::TryCatch &exc, bool interactive);
-  bool load_plugin(const std::string &file_name);
+  bool load_plugin(const Plugin_definition &plugin);
 
   std::weak_ptr<JScript_function_storage> store(
       v8::Local<v8::Function> function);

mysqlshdk/include/scripting/python_context.h

@@ -117,7 +117,7 @@ class TYPES_COMMON_PUBLIC Python_context {
   bool is_module(const std::string &file_name);
   Value execute_module(const std::string &file_name,
                        const std::vector<std::string> &argv);
-  bool load_plugin(const std::string &file_name);
+  bool load_plugin(const Plugin_definition &plugin);
 
   Value pyobj_to_shcore_value(PyObject *value);
   PyObject *shcore_value_to_pyobj(const Value &value);

mysqlshdk/include/scripting/types_cpp.h

@@ -577,6 +577,7 @@ class SHCORE_PUBLIC Cpp_object_bridge : public Object_bridge {
   virtual std::string &append_repr(std::string &s_out) const;
 
   virtual shcore::Value help(const shcore::Argument_list &args);
+  virtual std::string get_help_id() const { return class_name(); }
 
  protected:
   void detect_overload_conflicts(const std::string &name,

mysqlshdk/include/shellcore/shell_core.h

@@ -129,9 +129,9 @@ class SHCORE_PUBLIC Shell_language {
    * run-time will be available, however the global scope is not going to be
    * affected by the specified script.
    *
-   * @param file_name - full path to the file which is going to be loaded.
+   * @param plugin - the information of the plugin to be loaded.
    */
-  virtual bool load_plugin(const std::string &UNUSED(file_name)) {
+  virtual bool load_plugin(const Plugin_definition &UNUSED(plugin)) {
     return true;
   }
 
@@ -200,9 +200,9 @@ class SHCORE_PUBLIC Shell_core : public shcore::IShell_core {
    * Loads the specified plugin file using the current scripting language.
    *
    * @param mode - The language in which the plugin should be loaded.
-   * @param file_name - full path to the file which is going to be loaded.
+   * @param plugin - The information of the plugin to be loaded.
    */
-  bool load_plugin(Mode mode, const std::string &file_name);
+  bool load_plugin(Mode mode, const Plugin_definition &plugin);
 
   virtual void clear_input();
 

mysqlshdk/include/shellcore/shell_jscript.h

@@ -51,7 +51,7 @@ class Shell_javascript : public Shell_language {
   void clear_input() override;
   std::string get_continued_input_context() override;
 
-  bool load_plugin(const std::string &file_name) override;
+  bool load_plugin(const Plugin_definition &plugin) override;
 
  private:
   void abort() noexcept;

mysqlshdk/include/shellcore/shell_python.h

@@ -49,7 +49,7 @@ class Shell_python : public Shell_language {
 
   bool is_module(const std::string &file_name) override;
   void execute_module(const std::string &file_name) override;
-  bool load_plugin(const std::string &file_name) override;
+  bool load_plugin(const Plugin_definition &plugin) override;
 
   std::shared_ptr<Python_context> python_context() { return _py; }
 

mysqlshdk/scripting/jscript_context.cc

@@ -1339,13 +1339,14 @@ std::string JScript_context::translate_exception(const v8::TryCatch &exc,
   return format_exception(get_v8_exception_data(exc, interactive));
 }
 
-bool JScript_context::load_plugin(const std::string &file_name) {
+bool JScript_context::load_plugin(const Plugin_definition &plugin) {
   bool ret_val = true;
   // load the file
   std::string source;
 
   shcore::Scoped_naming_style style(NamingStyle::LowerCamelCase);
 
+  std::string file_name = plugin.file;
   if (load_text_file(file_name, source)) {
     const auto _isolate = isolate();
 

mysqlshdk/scripting/python_context.cc

@@ -631,7 +631,6 @@ void Python_context::set_python_error(const shcore::Exception &exc,
 
 void Python_context::set_python_error(PyObject *obj,
                                       const std::string &location) {
-  log_info("Python error: %s", location.c_str());
   PyErr_SetString(obj, location.c_str());
 }
 
@@ -1139,38 +1138,123 @@ Value Python_context::execute_module(const std::string &file_name,
   return ret_val;
 }
 
-bool Python_context::load_plugin(const std::string &file_name) {
-  bool ret_val = true;
+bool Python_context::load_plugin(const Plugin_definition &plugin) {
   WillEnterPython lock;
+  bool ret_val = false;
   shcore::Scoped_naming_style ns(shcore::NamingStyle::LowerCaseUnderscores);
 
-  const auto file = fopen(file_name.c_str(), "r");
+  PyObject *plugin_file = nullptr;
+  PyObject *plugin_path = nullptr;
+  PyObject *parent_plugin_path = nullptr;
+  PyObject *result = nullptr;
+  PyObject *globals = nullptr;
+  PyObject *sys_path_backup = nullptr;
+  FILE *file = nullptr;
+  bool executed = false;
+  std::string file_name = plugin.file;
+  try {
+    file = fopen(file_name.c_str(), "r");
+    if (nullptr == file) {
+      throw std::runtime_error(shcore::str_format(
+          "Failed opening the file: %s", errno_to_string(errno).c_str()));
+    }
+
+    plugin_file = PyString_FromString(file_name.c_str());
+    if (nullptr == plugin_file) {
+      throw std::runtime_error("Unable to get plugin file");
+    }
+
+    std::string plugin_dir = shcore::path::dirname(file_name);
+    plugin_path = PyString_FromString(plugin_dir.c_str());
+    if (nullptr == plugin_path) {
+      throw std::runtime_error("Unable to get plugin path");
+    }
+
+    std::string parent_plugin_dir;
+    if (!plugin.main) {
+      parent_plugin_dir = shcore::path::dirname(plugin_dir);
+      parent_plugin_path = PyString_FromString(parent_plugin_dir.c_str());
+      if (nullptr == parent_plugin_path) {
+        throw std::runtime_error("Unable to get the parent plugin path");
+      }
+    }
+
+    PyObject *sys_path = PySys_GetObject(const_cast<char *>("path"));
+    if (nullptr == sys_path) {
+      throw std::runtime_error("Failed getting sys.path");
+    }
+
+    // copy globals, so executed scripts does not pollute global
+    // namespace
+    globals = PyDict_Copy(_globals);
+    if (PyDict_SetItemString(globals, "__file__", plugin_file)) {
+      throw std::runtime_error("Failed setting __file__");
+    }
+
+    // Backups the original sys.path, in case the plugin load fails sys.path
+    // will be set back to the original content before the plugin load
+    sys_path_backup = PyList_GetSlice(sys_path, 0, PyList_Size(sys_path));
+    if (nullptr == sys_path_backup) {
+      std::string error = fetch_and_clear_exception();
+
+      throw std::runtime_error(
+          shcore::str_format("Failed backing up sys.path: %s", error.c_str()));
+    }
+
+    if (PyList_Insert(sys_path, 0, plugin_path)) {
+      throw std::runtime_error("Failed adding plugin path to sys.path");
+    }
+
+    if (!plugin.main) {
+      if (PyList_Insert(sys_path, 0, parent_plugin_path)) {
+        throw std::runtime_error(
+            "Failed adding parent plugin path to sys.path");
+      }
+    }
 
-  if (nullptr == file) {
-    ret_val = false;
-    log_error("Error loading Python file '%s':\n\tFailed opening the file: %s",
-              file_name.c_str(), errno_to_string(errno).c_str());
-  } else {
-    // copy globals, so executed scripts does not pollute global namespace
-    PyObject *globals = PyDict_Copy(_globals);
     // PyRun_FileEx() will close the file
-    // plugins are loaded in sandboxed environment, we're not using the
-    // compiler flags here
-    const auto result =
-        PyRun_FileEx(file, shcore::path::basename(file_name).c_str(),
-                     Py_file_input, globals, nullptr, true);
+    // plugins are loaded in sandboxed environment, we're not using
+    // the compiler flags here
+    result = PyRun_FileEx(file, shcore::path::basename(file_name).c_str(),
+                          Py_file_input, globals, nullptr, true);
+
+    executed = true;
 
+    // In case of an error, gets the description and clears the error condition
+    // in python to be able to restore the sys.path
+    std::string error;
     if (nullptr == result) {
-      ret_val = false;
-      log_error("Error loading Python file '%s':\n\tExecution failed: %s",
-                file_name.c_str(), fetch_and_clear_exception().c_str());
+      error = fetch_and_clear_exception();
     }
 
-    Py_XDECREF(result);
-    Py_XDECREF(globals);
+    // The plugin paths are required on sys.path at the moment the plugin is
+    // loaded, after that they are not required so the original sys.path is
+    // restored
+    PySys_SetObject(const_cast<char *>("path"), sys_path_backup);
+
+    // If there was an error, raises the corresponding exception
+    if (nullptr == result) {
+      throw std::runtime_error(
+          shcore::str_format("Execution failed: %s", error.c_str()));
+    } else {
+      ret_val = true;
+    }
+  } catch (const std::runtime_error &error) {
+    log_error("Error loading Python file '%s':\n\t%s", file_name.c_str(),
+              error.what());
+
+    if (file && !executed) {
+      fclose(file);
+    }
   }
+  Py_XDECREF(plugin_file);
+  Py_XDECREF(result);
+  Py_XDECREF(globals);
+  Py_XDECREF(plugin_path);
+  Py_XDECREF(sys_path_backup);
+
   return ret_val;
-}
+}  // namespace shcore
 
 std::string Python_context::fetch_and_clear_exception() {
   std::string exception;

mysqlshdk/scripting/types_cpp.cc

@@ -617,7 +617,7 @@ shcore::Value Cpp_object_bridge::help(const shcore::Argument_list &args) {
   help.set_mode(mode);
 
   shcore::Topic_mask mask;
-  std::string pattern = class_name();
+  std::string pattern = get_help_id();
   if (!item.empty()) {
     // This group represents the API topics that can childs
     // of another API topic

mysqlshdk/shellcore/shell_core.cc

@@ -310,12 +310,12 @@ void Shell_core::execute_module(const std::string &file_name,
   _langs[_mode]->execute_module(file_name);
 }
 
-bool Shell_core::load_plugin(Mode mode, const std::string &file_name) {
+bool Shell_core::load_plugin(Mode mode, const Plugin_definition &plugin) {
   assert(mode != Mode::None);
 
   init_mode(mode);
 
-  return _langs[mode]->load_plugin(file_name);
+  return _langs[mode]->load_plugin(plugin);
 }
 
 //------------------ COMMAND HANDLER FUNCTIONS ------------------//

mysqlshdk/shellcore/shell_jscript.cc

@@ -87,6 +87,6 @@ std::string Shell_javascript::get_continued_input_context() {
   return m_last_input_state == Input_state::Ok ? "" : "-";
 }
 
-bool Shell_javascript::load_plugin(const std::string &file_name) {
-  return _js->load_plugin(file_name);
+bool Shell_javascript::load_plugin(const Plugin_definition &plugin) {
+  return _js->load_plugin(plugin);
 }

mysqlshdk/shellcore/shell_python.cc

@@ -176,8 +176,8 @@ void Shell_python::execute_module(const std::string &file_name) {
   }
 }
 
-bool Shell_python::load_plugin(const std::string &file_name) {
-  return _py->load_plugin(file_name);
+bool Shell_python::load_plugin(const Plugin_definition &plugin) {
+  return _py->load_plugin(plugin);
 }
 
 void Shell_python::clear_input() {