This is an archive of the discontinued LLVM Phabricator instance.

Pass enviroment variables to python scripts.
Needs RevisionPublic

Authored by rdhindsa on Feb 9 2021, 2:05 PM.

Details

Reviewers
labath
jingham
Summary

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.

Diff Detail

Event Timeline

rdhindsa requested review of this revision.Feb 9 2021, 2:05 PM
rdhindsa created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2021, 2:05 PM
rdhindsa updated this revision to Diff 322510.Feb 9 2021, 2:47 PM
jingham requested changes to this revision.Feb 9 2021, 3:29 PM
jingham added a subscriber: jingham.

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.

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
3280

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.

This revision now requires changes to proceed.Feb 9 2021, 3:29 PM

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', '.']