I’m thinking about finishing this work off, because it’s still a big hindrance for running the unit tests on windows. (not dotest, but the gtest stuff). The reason is that gtest works by outputting a separate executable for every test suite, which means each of those executables needs to link against python. Windows doesn’t have a good alternative to LD_LIBRARY_PATH (it only has PATH) so this means it’s cumbersome to run the unittest suite without modifying your PATH variable, which interferes with the system installed python.
I have a patch locally which removes some unnecessary #includes of ScriptInterpreterPython.h, and after I submit this patch I think that will be the last of the references to ScriptInterpreterPython hanging around somewhere they don’t really belong. The remaining occurrences look like this:
Find all “ScriptInterpreterPython.h”, Match case, Subfolders, Find Results 1, “d:\src\llvm\tools\lldb”, ".cpp; .h"
D:\src\llvm\tools\lldb\include\lldb\Interpreter\ScriptInterpreterPython.h(1)://===-- ScriptInterpreterPython.h -------------------------------- C++ --===//
Matching lines: 6 Matching files: 6 Total files searched: 1864
I know we had talked about moving ScriptInterpreterPython and PythonDataObjects to source/API/bindings. I think that will be sufficient for us, but that still leaves SystemInitializerCommon, SystemInitializerFull, and CommandInterpreter to worry about.
For the first two I think I can solve this by removing the dependency from the unit test executables to lldbInitialization (which is why I created this library in the first place). After all, unit tests are only testing very simple data structures, they don’t need to initialize anything.
But I’m still a little unsure about what to do about the final reference in CommandInterpreter.
It all boils down to this function
ScriptInterpreter *CommandInterpreter::GetScriptInterpreter (bool can_create)
I think that what is necessary is to invert the creation responsibility here. Instead of having CommandInterpreter::GetScriptInterpreter create the thing, instead have SystemInitialierCommon or SystemInitializerFull call SBCommandInterpreter::InitializeScriptInterpreter(), and SBCommandInterpreter will call some function like m_spCommandInterpreter->AddScriptInterpreter(ScriptLanguage, ScriptInterpreter*)
With the above 3 fixes (creating source/API/bindings, removing the dependency from unit test executables → lldbInitialization, and pushing ScriptInterpreter creation from CommandInterpreter to SBCommandInterpreter), I think this work will finally be done.
Thoughts? Does this seem like a reasonable approach?