This revision was automatically updated to reflect the committed changes.
Closed by commit rG00bb397b0dc7: [lldb] Support Python imports relative the to 
the current file being sourced (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D89334?vs=300803&id=301030#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89334/new/

https://reviews.llvm.org/D89334

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/Options.td
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/test/Shell/ScriptInterpreter/Python/Inputs/hello.split
  lldb/test/Shell/ScriptInterpreter/Python/Inputs/relative.split
  lldb/test/Shell/ScriptInterpreter/Python/command_relative_import.test

Index: lldb/test/Shell/ScriptInterpreter/Python/command_relative_import.test
===================================================================
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/command_relative_import.test
@@ -0,0 +1,31 @@
+# REQUIRES: python
+
+# RUN: rm -rf %t && mkdir -p %t/foo/bar/baz
+# RUN: split-file %S/Inputs/relative.split %t/foo
+# RUN: split-file %S/Inputs/hello.split %t/foo/bar
+# RUN: mv %t/foo/bar/hello.py %t/foo/bar/baz
+# RUN: echo 'command source %t/foo/bar/hello.in' >> %t/foo/zip.in
+
+# RUN: %lldb --script-language python \
+# RUN:    -o 'command source %t/foo/magritte.in' \
+# RUN:    -o 'command source %t/foo/zip.in' \
+# RUN:    -o 'command source %t/foo/magritte.in' \
+# RUN;    -o 'zip' \
+# RUN:    -o 'hello'
+# RUN     -o 'magritte' 2>&1 | FileCheck %s
+
+# The first time importing 'magritte' fails because we didn't pass -c.
+# CHECK: ModuleNotFoundError: No module named 'magritte'
+# CHECK-NOT: Ceci n'est pas une pipe
+# CHECK: 95126
+# CHECK: Hello, World!
+# The second time importing 'magritte' works, even without passing -c because
+# we added '%t/foo' to the Python path when importing 'zip'.
+# CHECK: Ceci n'est pas une pipe
+
+# Cannot use `-o` here because the driver puts the commands in a file and
+# sources them.
+command script import -c %t/foo/magritte.py
+quit
+# RUN: cat %s | %lldb --script-language python 2>&1 | FileCheck %s --check-prefix ERROR
+# ERROR: error: command script import -c can only be specified from a command file
Index: lldb/test/Shell/ScriptInterpreter/Python/Inputs/relative.split
===================================================================
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/Inputs/relative.split
@@ -0,0 +1,20 @@
+#--- magritte.in
+command script import magritte
+#--- magritte.py
+import lldb
+
+def magritte(debugger, command, result, internal_dict):
+    print("Ceci n'est pas une pipe")
+
+def __lldb_init_module(debugger, internal_dict):
+    debugger.HandleCommand('command script add -f magritte.magritte magritte')
+#--- zip.in
+command script import -c zip
+#--- zip.py
+import lldb
+
+def zip(debugger, command, result, internal_dict):
+    print("95126")
+
+def __lldb_init_module(debugger, internal_dict):
+    debugger.HandleCommand('command script add -f zip.zip zip')
Index: lldb/test/Shell/ScriptInterpreter/Python/Inputs/hello.split
===================================================================
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/Inputs/hello.split
@@ -0,0 +1,10 @@
+#--- hello.in
+command script import -c baz.hello
+#--- hello.py
+import lldb
+
+def hello(debugger, command, result, internal_dict):
+    print("Hello, World!")
+
+def __lldb_init_module(debugger, internal_dict):
+    debugger.HandleCommand('command script add -f baz.hello.hello hello')
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -231,10 +231,10 @@
   bool RunScriptFormatKeyword(const char *impl_function, ValueObject *value,
                               std::string &output, Status &error) override;
 
-  bool
-  LoadScriptingModule(const char *filename, bool init_session,
-                      lldb_private::Status &error,
-                      StructuredData::ObjectSP *module_sp = nullptr) override;
+  bool LoadScriptingModule(const char *filename, bool init_session,
+                           lldb_private::Status &error,
+                           StructuredData::ObjectSP *module_sp = nullptr,
+                           FileSpec extra_search_dir = {}) override;
 
   bool IsReservedWord(const char *word) override;
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2733,8 +2733,9 @@
 
 bool ScriptInterpreterPythonImpl::LoadScriptingModule(
     const char *pathname, bool init_session, lldb_private::Status &error,
-    StructuredData::ObjectSP *module_sp) {
+    StructuredData::ObjectSP *module_sp, FileSpec extra_search_dir) {
   namespace fs = llvm::sys::fs;
+  namespace path = llvm::sys::path;
 
   if (!pathname || !pathname[0]) {
     error.SetErrorString("invalid pathname");
@@ -2743,44 +2744,23 @@
 
   lldb::DebuggerSP debugger_sp = m_debugger.shared_from_this();
 
-  FileSpec target_file(pathname);
-  FileSystem::Instance().Resolve(target_file);
-  FileSystem::Instance().Collect(target_file);
-  std::string basename(target_file.GetFilename().GetCString());
-
-  StreamString command_stream;
-
   // Before executing Python code, lock the GIL.
   Locker py_lock(this,
                  Locker::AcquireLock |
                      (init_session ? Locker::InitSession : 0) | Locker::NoSTDIN,
                  Locker::FreeAcquiredLock |
                      (init_session ? Locker::TearDownSession : 0));
-  fs::file_status st;
-  std::error_code ec = status(target_file.GetPath(), st);
-
-  if (ec || st.type() == fs::file_type::status_error ||
-      st.type() == fs::file_type::type_unknown ||
-      st.type() == fs::file_type::file_not_found) {
-    // if not a valid file of any sort, check if it might be a filename still
-    // dot can't be used but / and \ can, and if either is found, reject
-    if (strchr(pathname, '\\') || strchr(pathname, '/')) {
-      error.SetErrorString("invalid pathname");
-      return false;
-    }
-    basename = pathname; // not a filename, probably a package of some sort,
-                         // let it go through
-  } else if (is_directory(st) || is_regular_file(st)) {
-    if (target_file.GetDirectory().IsEmpty()) {
-      error.SetErrorString("invalid directory name");
-      return false;
+
+  auto ExtendSysPath = [this](std::string directory) -> llvm::Error {
+    if (directory.empty()) {
+      return llvm::make_error<llvm::StringError>(
+          "invalid directory name", llvm::inconvertibleErrorCode());
     }
 
-    std::string directory = target_file.GetDirectory().GetCString();
     replace_all(directory, "\\", "\\\\");
     replace_all(directory, "'", "\\'");
 
-    // now make sure that Python has "directory" in the search path
+    // Make sure that Python has "directory" in the search path.
     StreamString command_stream;
     command_stream.Printf("if not (sys.path.__contains__('%s')):\n    "
                           "sys.path.insert(1,'%s');\n\n",
@@ -2792,27 +2772,68 @@
                                  .SetSetLLDBGlobals(false))
             .Success();
     if (!syspath_retval) {
-      error.SetErrorString("Python sys.path handling failed");
-      return false;
+      return llvm::make_error<llvm::StringError>(
+          "Python sys.path handling failed", llvm::inconvertibleErrorCode());
     }
 
+    return llvm::Error::success();
+  };
+
+  std::string module_name(pathname);
+
+  if (extra_search_dir) {
+    if (llvm::Error e = ExtendSysPath(extra_search_dir.GetPath())) {
+      error = std::move(e);
+      return false;
+    }
   } else {
-    error.SetErrorString("no known way to import this module specification");
-    return false;
+    FileSpec module_file(pathname);
+    FileSystem::Instance().Resolve(module_file);
+    FileSystem::Instance().Collect(module_file);
+
+    fs::file_status st;
+    std::error_code ec = status(module_file.GetPath(), st);
+
+    if (ec || st.type() == fs::file_type::status_error ||
+        st.type() == fs::file_type::type_unknown ||
+        st.type() == fs::file_type::file_not_found) {
+      // if not a valid file of any sort, check if it might be a filename still
+      // dot can't be used but / and \ can, and if either is found, reject
+      if (strchr(pathname, '\\') || strchr(pathname, '/')) {
+        error.SetErrorString("invalid pathname");
+        return false;
+      }
+      // Not a filename, probably a package of some sort, let it go through.
+    } else if (is_directory(st) || is_regular_file(st)) {
+      if (module_file.GetDirectory().IsEmpty()) {
+        error.SetErrorString("invalid directory name");
+        return false;
+      }
+      if (llvm::Error e =
+              ExtendSysPath(module_file.GetDirectory().GetCString())) {
+        error = std::move(e);
+        return false;
+      }
+      module_name = module_file.GetFilename().GetCString();
+    } else {
+      error.SetErrorString("no known way to import this module specification");
+      return false;
+    }
   }
 
   // Strip .py or .pyc extension
-  llvm::StringRef extension = target_file.GetFileNameExtension().GetCString();
+  llvm::StringRef extension = llvm::sys::path::extension(module_name);
   if (!extension.empty()) {
     if (extension == ".py")
-      basename.resize(basename.length() - 3);
+      module_name.resize(module_name.length() - 3);
     else if (extension == ".pyc")
-      basename.resize(basename.length() - 4);
+      module_name.resize(module_name.length() - 4);
   }
 
   // check if the module is already import-ed
+  StreamString command_stream;
   command_stream.Clear();
-  command_stream.Printf("sys.modules.__contains__('%s')", basename.c_str());
+  command_stream.Printf("sys.modules.__contains__('%s')", module_name.c_str());
   bool does_contain = false;
   // this call will succeed if the module was ever imported in any Debugger
   // in the lifetime of the process in which this LLDB framework is living
@@ -2827,9 +2848,9 @@
   // this call will fail if the module was not imported in this Debugger
   // before
   command_stream.Clear();
-  command_stream.Printf("sys.getrefcount(%s)", basename.c_str());
+  command_stream.Printf("sys.getrefcount(%s)", module_name.c_str());
   bool was_imported_locally = GetSessionDictionary()
-                                  .GetItemForKey(PythonString(basename))
+                                  .GetItemForKey(PythonString(module_name))
                                   .IsAllocated();
 
   bool was_imported = (was_imported_globally || was_imported_locally);
@@ -2839,12 +2860,12 @@
 
   if (was_imported) {
     if (!was_imported_locally)
-      command_stream.Printf("import %s ; reload_module(%s)", basename.c_str(),
-                            basename.c_str());
+      command_stream.Printf("import %s ; reload_module(%s)",
+                            module_name.c_str(), module_name.c_str());
     else
-      command_stream.Printf("reload_module(%s)", basename.c_str());
+      command_stream.Printf("reload_module(%s)", module_name.c_str());
   } else
-    command_stream.Printf("import %s", basename.c_str());
+    command_stream.Printf("import %s", module_name.c_str());
 
   error = ExecuteMultipleLines(command_stream.GetData(),
                                ScriptInterpreter::ExecuteScriptOptions()
@@ -2855,8 +2876,8 @@
 
   // if we are here, everything worked
   // call __lldb_init_module(debugger,dict)
-  if (!LLDBSwigPythonCallModuleInit(basename.c_str(), m_dictionary_name.c_str(),
-                                    debugger_sp)) {
+  if (!LLDBSwigPythonCallModuleInit(module_name.c_str(),
+                                    m_dictionary_name.c_str(), debugger_sp)) {
     error.SetErrorString("calling __lldb_init_module failed");
     return false;
   }
@@ -2864,7 +2885,7 @@
   if (module_sp) {
     // everything went just great, now set the module object
     command_stream.Clear();
-    command_stream.Printf("%s", basename.c_str());
+    command_stream.Printf("%s", module_name.c_str());
     void *module_pyobj = nullptr;
     if (ExecuteOneLineWithReturn(
             command_stream.GetData(),
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -25,10 +25,10 @@
 
   void ExecuteInterpreterLoop() override;
 
-  bool
-  LoadScriptingModule(const char *filename, bool init_session,
-                      lldb_private::Status &error,
-                      StructuredData::ObjectSP *module_sp = nullptr) override;
+  bool LoadScriptingModule(const char *filename, bool init_session,
+                           lldb_private::Status &error,
+                           StructuredData::ObjectSP *module_sp = nullptr,
+                           FileSpec extra_search_dir = {}) override;
 
   // Static Functions
   static void Initialize();
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -124,7 +124,7 @@
 
 bool ScriptInterpreterLua::LoadScriptingModule(
     const char *filename, bool init_session, lldb_private::Status &error,
-    StructuredData::ObjectSP *module_sp) {
+    StructuredData::ObjectSP *module_sp, FileSpec extra_search_dir) {
 
   FileSystem::Instance().Collect(filename);
   if (llvm::Error e = m_lua->LoadModule(filename)) {
Index: lldb/source/Interpreter/ScriptInterpreter.cpp
===================================================================
--- lldb/source/Interpreter/ScriptInterpreter.cpp
+++ lldb/source/Interpreter/ScriptInterpreter.cpp
@@ -47,9 +47,11 @@
       "This script interpreter does not support watchpoint callbacks.");
 }
 
-bool ScriptInterpreter::LoadScriptingModule(
-    const char *filename, bool init_session, lldb_private::Status &error,
-    StructuredData::ObjectSP *module_sp) {
+bool ScriptInterpreter::LoadScriptingModule(const char *filename,
+                                            bool init_session,
+                                            lldb_private::Status &error,
+                                            StructuredData::ObjectSP *module_sp,
+                                            FileSpec extra_search_dir) {
   error.SetErrorString(
       "This script interpreter does not support importing modules.");
   return false;
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===================================================================
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2554,11 +2554,15 @@
     debugger.SetAsyncExecution(false);
 
   m_command_source_depth++;
+  m_command_source_dirs.push_back(cmd_file.CopyByRemovingLastPathComponent());
 
   debugger.RunIOHandlerSync(io_handler_sp);
   if (!m_command_source_flags.empty())
     m_command_source_flags.pop_back();
+
+  m_command_source_dirs.pop_back();
   m_command_source_depth--;
+
   result.SetStatus(eReturnStatusSuccessFinishNoResult);
   debugger.SetAsyncExecution(old_async_execution);
 }
@@ -2964,6 +2968,12 @@
   return true;
 }
 
+FileSpec CommandInterpreter::GetCurrentSourceDir() {
+  if (m_command_source_dirs.empty())
+    return {};
+  return m_command_source_dirs.back();
+}
+
 void CommandInterpreter::GetLLDBCommandsFromIOHandler(
     const char *prompt, IOHandlerDelegate &delegate, void *baton) {
   Debugger &debugger = GetDebugger();
Index: lldb/source/Commands/Options.td
===================================================================
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -704,6 +704,10 @@
     Desc<"Allow the script to be loaded even if it was already loaded before. "
     "This argument exists for backwards compatibility, but reloading is always "
     "allowed, whether you specify it or not.">;
+  def relative_to_command_file : Option<"relative-to-command-file", "c">,
+    Group<1>, Desc<"Resolve non-absolute paths relative to the location of the "
+    "current command file. This argument can only be used when the command is "
+    "being sourced from a file.">;
 }
 
 let Command = "script add" in {
Index: lldb/source/Commands/CommandObjectCommands.cpp
===================================================================
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -1272,6 +1272,9 @@
       case 'r':
         // NO-OP
         break;
+      case 'c':
+        relative_to_command_file = true;
+        break;
       default:
         llvm_unreachable("Unimplemented option");
       }
@@ -1280,11 +1283,13 @@
     }
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
+      relative_to_command_file = false;
     }
 
     llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
       return llvm::makeArrayRef(g_script_import_options);
     }
+    bool relative_to_command_file = false;
   };
 
   bool DoExecute(Args &command, CommandReturnObject &result) override {
@@ -1294,6 +1299,17 @@
       return false;
     }
 
+    FileSpec source_dir = {};
+    if (m_options.relative_to_command_file) {
+      source_dir = GetDebugger().GetCommandInterpreter().GetCurrentSourceDir();
+      if (!source_dir) {
+        result.AppendError("command script import -c can only be specified "
+                           "from a command file");
+        result.SetStatus(eReturnStatusFailed);
+        return false;
+      }
+    }
+
     for (auto &entry : command.entries()) {
       Status error;
 
@@ -1308,7 +1324,7 @@
       // more)
       m_exe_ctx.Clear();
       if (GetDebugger().GetScriptInterpreter()->LoadScriptingModule(
-              entry.c_str(), init_session, error)) {
+              entry.c_str(), init_session, error, nullptr, source_dir)) {
         result.SetStatus(eReturnStatusSuccessFinishNoResult);
       } else {
         result.AppendErrorWithFormat("module importing failed: %s",
Index: lldb/include/lldb/Interpreter/ScriptInterpreter.h
===================================================================
--- lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -507,7 +507,8 @@
   virtual bool
   LoadScriptingModule(const char *filename, bool init_session,
                       lldb_private::Status &error,
-                      StructuredData::ObjectSP *module_sp = nullptr);
+                      StructuredData::ObjectSP *module_sp = nullptr,
+                      FileSpec extra_search_dir = {});
 
   virtual bool IsReservedWord(const char *word) { return false; }
 
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===================================================================
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -551,6 +551,8 @@
   bool SaveTranscript(CommandReturnObject &result,
                       llvm::Optional<std::string> output_file = llvm::None);
 
+  FileSpec GetCurrentSourceDir();
+
 protected:
   friend class Debugger;
 
@@ -637,7 +639,13 @@
   ChildrenTruncatedWarningStatus m_truncation_warning; // Whether we truncated
                                                        // children and whether
                                                        // the user has been told
+
+  // FIXME: Stop using this to control adding to the history and then replace
+  // this with m_command_source_dirs.size().
   uint32_t m_command_source_depth;
+  /// A stack of directory paths. When not empty, the last one is the directory
+  /// of the file that's currently sourced.
+  std::vector<FileSpec> m_command_source_dirs;
   std::vector<uint32_t> m_command_source_flags;
   CommandInterpreterRunResult m_result;
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to