[Lldb-commits] [PATCH] D89334: [lldb] Support Python imports relative the to the current file being sourced

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The main question on my mind is should we be adding the directory of the python file to the path (which is what I believe is happening now), or if we should add the directory of the command file that is being sourced (and adjust the way we import the file). The main impact of this is how will the imported module "see" itself (what will it's name be), and how it will be able to import other modules.

Imagine the user has the following (reasonable, I think) file structure.

  $ROOT/utils/consult_oracle.py
  $ROOT/automatic_bug_finder/main.py # uses consult_oracle.py
  $ROOT/awesome_backtrace_analyzer/main.py # uses consult_oracle.py
  $ROOT/install_super_scripts.lldbinit # calls command script import awesome_backtrace_analyzer/main.py

If "command script import awesome_backtrace_analyzer/main.py" ends up adding `$ROOT/awesome_backtrace_analyzer`  to the path, then this module will have a hard time importing the modules it depends on (it would either have to use weird relative imports, or mess with sys.path itself. If we add just `$ROOT` then it could simply `import utils.consult_oracle`.

I think setting the import path to $ROOT would actually make the sys.path manipulation serve some useful purpose (and also reduce the number of sys.path entries we add). If, on the other hand, we are not interested making cross-module imports work "out of the box" (like, we could say that it's the responsibility of individual modules to ensure that), we could also try to import the file without messing with sys.path at all (https://stackoverflow.com/questions/67631/how-to-import-a-module-given-the-full-path gives one way to do that).



================
Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:554
 
+  FileSpec GetCurrentSourceDir();
+
----------------
A comment would be very useful here.


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

https://reviews.llvm.org/D89334