[Lldb-commits] [PATCH] D96370: Pass enviroment variables to python scripts.

2021-02-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Yep, I agree with what Jim said.

In addition it seems that lldb already honors the PYTHONPATH variable (as 
passed to the lldb process), so that would be another way to control the script 
location.

  PYTHONPATH=/tmp/foobar lldb -o "script print(sys.path)" -b
  (lldb) script print(sys.path)
  ['/usr/lib64', '/usr/lib/python3.8/site-packages', '/tmp/foobar', 
'/usr/lib/python38.zip', '/usr/lib/python3.8', 
'/usr/lib/python3.8/lib-dynload', '/usr/lib/python3.8/site-packages', '.']


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

https://reviews.llvm.org/D96370

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D96370: Pass enviroment variables to python scripts.

2021-02-09 Thread Jim Ingham via Phabricator via lldb-commits
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

Modifying the target environment in order to add something to the host lldb's 
PYTHONPATH seems very counterintuitive to me.  It is misusing the purpose of 
the target environment setting.  And it could cause confusion if you set this 
up (say in your .lldbinit) and then months later wondered why some Python 
program you were debugging ended up finding python modules in weird places...  
If you want to have a way to add to lldb's python interpreter's path when 
starting lldb, then there should be a setting for that specific purpose.

Also, this would need some sort of documention.  There's no way anybody could 
discover this feature without reading the lldb sources.

Also also, if I put:

script import sys; sys.path.append("/some/directory")

in my .lldbinit, wouldn't that do the same thing your patch does, but in a much 
more straightforward way?  I don't see the advantage of doing this in a 
setting, and certainly not in the target env-vars setting.




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:3280
+  PyRun_SimpleString("import os");
+  for (const auto  : env) {
+if (strcmp(KV.getKey().data(), "PYTHONPATH") == 0 ||

lldb local variables should always be lower case, and should be words.  I'm 
assuming the this is called KV because it's a key/value entry, but it took me 
too much time to guess that. "key_value" or something would make this easier to 
read.


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

https://reviews.llvm.org/D96370

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D96370: Pass enviroment variables to python scripts.

2021-02-09 Thread Rumeet Dhindsa via Phabricator via lldb-commits
rdhindsa updated this revision to Diff 322510.

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

https://reviews.llvm.org/D96370

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/test/Shell/ScriptInterpreter/Python/Inputs/environ.py
  lldb/test/Shell/ScriptInterpreter/Python/pass_environment.test


Index: lldb/test/Shell/ScriptInterpreter/Python/pass_environment.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/pass_environment.test
@@ -0,0 +1,10 @@
+# REQUIRES: python
+
+# RUN: %lldb --batch \
+# RUN:--script-language python \
+# RUN:-O 'settings append target.env-vars PYTHONPATH=/tmp' \
+# RUN:-O 'command script import %S/Inputs/environ.py' \
+# RUN:-O 'test' 2>&1  | FileCheck %s
+
+# CHECK: :/tmp
+# CHECK: '/tmp'
Index: lldb/test/Shell/ScriptInterpreter/Python/Inputs/environ.py
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/Inputs/environ.py
@@ -0,0 +1,10 @@
+import os
+import sys
+
+def test(debugger, command, result, internal_dict):
+  print(os.environ["PYTHONPATH"])
+  print(sys.path)
+
+def __lldb_init_module(debugger, internal_dict):
+  debugger.HandleCommand('command script add -f environ.test test')
+
Index: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -341,6 +341,8 @@
  lldb::user_id_t watch_id);
   static void InitializePrivate();
 
+  static void SetPythonEnvironment(Debugger );
+
   class SynchronicityHandler {
   private:
 lldb::DebuggerSP m_debugger_sp;
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -537,6 +537,8 @@
   PyRun_SimpleString(run_string.GetData());
   run_string.Clear();
 
+  SetPythonEnvironment(m_debugger);
+
   run_string.Printf("run_one_line (%s, 'import lldb.embedded_interpreter; from 
"
 "lldb.embedded_interpreter import run_python_interpreter; "
 "from lldb.embedded_interpreter import run_one_line')",
@@ -3269,6 +3271,26 @@
  "from lldb.embedded_interpreter import run_one_line");
 }
 
+void ScriptInterpreterPythonImpl::SetPythonEnvironment(Debugger ) {
+  StreamString run_string;
+  lldb::TargetSP target_get = debugger.GetTargetList().GetSelectedTarget();
+  const Environment  = target_get->GetGlobalProperties()->GetEnvironment();
+
+  PyRun_SimpleString("import os");
+  for (const auto  : env) {
+if (strcmp(KV.getKey().data(), "PYTHONPATH") == 0 ||
+strcmp(KV.getKey().data(), "PATH") == 0) {
+  run_string.Clear();
+  run_string.Printf(
+  "os.environ[\"%s\"] = os.environ.get(\"%s\",\"\")+ os.pathsep +"
+  "\"%s\"",
+  KV.getKey().data(), KV.getKey().data(), KV.getValue().data());
+  PyRun_SimpleString(run_string.GetData());
+  AddToSysPath(AddLocation::End, KV.getValue().data());
+}
+  }
+}
+
 void ScriptInterpreterPythonImpl::AddToSysPath(AddLocation location,
std::string path) {
   std::string path_copy;


Index: lldb/test/Shell/ScriptInterpreter/Python/pass_environment.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/pass_environment.test
@@ -0,0 +1,10 @@
+# REQUIRES: python
+
+# RUN: %lldb --batch \
+# RUN:--script-language python \
+# RUN:-O 'settings append target.env-vars PYTHONPATH=/tmp' \
+# RUN:-O 'command script import %S/Inputs/environ.py' \
+# RUN:-O 'test' 2>&1  | FileCheck %s
+
+# CHECK: :/tmp
+# CHECK: '/tmp'
Index: lldb/test/Shell/ScriptInterpreter/Python/Inputs/environ.py
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/Inputs/environ.py
@@ -0,0 +1,10 @@
+import os
+import sys
+
+def test(debugger, command, result, internal_dict):
+  print(os.environ["PYTHONPATH"])
+  print(sys.path)
+
+def __lldb_init_module(debugger, internal_dict):
+  debugger.HandleCommand('command script add -f environ.test test')
+
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ 

[Lldb-commits] [PATCH] D96370: Pass enviroment variables to python scripts.

2021-02-09 Thread Rumeet Dhindsa via Phabricator via lldb-commits
rdhindsa created this revision.
rdhindsa added a reviewer: labath.
rdhindsa requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

It enables environment variables set for lldb targets to be passed to python 
scripts. This allows the user to put Python scripts at different locations, but 
being able to import them by setting pythonpath.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96370

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/test/Shell/ScriptInterpreter/Python/Inputs/environ.py
  lldb/test/Shell/ScriptInterpreter/Python/pass_environment.test


Index: lldb/test/Shell/ScriptInterpreter/Python/pass_environment.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/pass_environment.test
@@ -0,0 +1,10 @@
+# REQUIRES: python
+
+# RUN: %lldb --batch \
+# RUN:--script-language python \
+# RUN:-O 'settings append target.env-vars PYTHONPATH=/tmp' \
+# RUN:-O 'command script import %S/Inputs/environ.py' \
+# RUN:-O 'test' 2>&1  | FileCheck %s
+
+# CHECK: :/tmp
+# CHECK: '/tmp'
Index: lldb/test/Shell/ScriptInterpreter/Python/Inputs/environ.py
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/Inputs/environ.py
@@ -0,0 +1,10 @@
+import os
+import sys
+
+def test(debugger, command, result, internal_dict):
+  print(os.environ["PYTHONPATH"])
+  print(sys.path)
+
+def __lldb_init_module(debugger, internal_dict):
+  debugger.HandleCommand('command script add -f environ.test test')
+
Index: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -341,6 +341,8 @@
  lldb::user_id_t watch_id);
   static void InitializePrivate();
 
+  static void SetPythonEnvironment(Debugger& debugger);
+
   class SynchronicityHandler {
   private:
 lldb::DebuggerSP m_debugger_sp;
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -537,6 +537,8 @@
   PyRun_SimpleString(run_string.GetData());
   run_string.Clear();
 
+  SetPythonEnvironment(m_debugger);
+
   run_string.Printf("run_one_line (%s, 'import lldb.embedded_interpreter; from 
"
 "lldb.embedded_interpreter import run_python_interpreter; "
 "from lldb.embedded_interpreter import run_one_line')",
@@ -3269,6 +3271,26 @@
  "from lldb.embedded_interpreter import run_one_line");
 }
 
+void ScriptInterpreterPythonImpl::SetPythonEnvironment(Debugger ) {
+  StreamString run_string;
+  lldb::TargetSP target_get = debugger.GetTargetList().GetSelectedTarget();
+  const Environment  = target_get->GetGlobalProperties()->GetEnvironment();
+
+  PyRun_SimpleString("import os");
+  for (const auto  : env) {
+if (strcmp(KV.getKey().data(), "PYTHONPATH") == 0 ||
+strcmp(KV.getKey().data(), "PATH") == 0) {
+  run_string.Clear();
+  run_string.Printf(
+  "os.environ[\"%s\"] = os.environ.get(\"%s\",\"\")+ os.pathsep +"
+  "\"%s\"",
+  KV.getKey().data(), KV.getKey().data(), KV.getValue().data());
+  PyRun_SimpleString(run_string.GetData());
+  AddToSysPath(AddLocation::End, KV.getValue().data());
+}
+  }
+}
+
 void ScriptInterpreterPythonImpl::AddToSysPath(AddLocation location,
std::string path) {
   std::string path_copy;


Index: lldb/test/Shell/ScriptInterpreter/Python/pass_environment.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/pass_environment.test
@@ -0,0 +1,10 @@
+# REQUIRES: python
+
+# RUN: %lldb --batch \
+# RUN:--script-language python \
+# RUN:-O 'settings append target.env-vars PYTHONPATH=/tmp' \
+# RUN:-O 'command script import %S/Inputs/environ.py' \
+# RUN:-O 'test' 2>&1  | FileCheck %s
+
+# CHECK: :/tmp
+# CHECK: '/tmp'
Index: lldb/test/Shell/ScriptInterpreter/Python/Inputs/environ.py
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/Inputs/environ.py
@@ -0,0 +1,10 @@
+import os
+import sys
+
+def test(debugger, command, result, internal_dict):
+  print(os.environ["PYTHONPATH"])
+  print(sys.path)
+
+def __lldb_init_module(debugger, internal_dict):
+