[Lldb-commits] [PATCH] D61090: [SBHostOS} Remove getting the python script interpreter path

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I'm going to throw one more opinion into the mix here. :)

While I agree that sprinkling `#ifdef PYTHON` everywhere is bad, and it's great to be rid of it, I also think that it is good for the user to be able to know how a particular build of lldb was configured. So, I wouldn't say this goes "against the  notion of a generic pluggable interpreter". In fact I would say the opposite. In an imaginary future where lldb can be built with either python or say javascript support (or both?), it may be very important for programs to detect whether the library they are using is compatible with them.

The question on my mind is "do we ever want/anticipate the python path to change at runtime?". If we don't, then getting the path value through a command interpreter *instance* seems odd. In fact, I would say that the SBHostOS is the best place to provide this info, as it is really a property of the host, which does not depend on any runtime configuration of anything. So, if the only  reason you made this go through the command interpreter class was to break the build dependency, then I think we should do this another way. One way to handle this would be to add one more argument to the PluginManager::RegisterPlugin function which is called from ScriptInterpreterPython::Initialize. This argument could be either a callback which computes the "module path" for the given language, or even the path itself. Then SBHostOS could just get the path by asking the PluginManager.

So, with both backward and forward compatibility in mind, what I'd to is this:

- don't add any new SB interfaces
- implement `SBHostOS::GetLLDBPythonPath` via a call to `PluginManager::GetModulePathFor(eScriptLanguagePython)` as described previously
- In the future, once multiple interpreters become a reality, consider a new SBHostOS function, which takes a language type as an argument. This will allow the user to determine which scripting languages are available and leave the door open for a single build of lldb to support multiple languages (which the current proposal doesn't). `SBHostOS::GetLLDBPythonPath` then just becomes a deprecated variant of this new function.


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

https://reviews.llvm.org/D61090