This is an archive of the discontinued LLVM Phabricator instance.

Remove lldb_private library implicit dependencies on python
AbandonedPublic

Authored by zturner on Jun 2 2015, 10:53 AM.

Details

Reviewers
clayborg
Summary

After this patch, the link dependency on python will only be present for liblldb. Linking against a .a or .lib file will not cause an implicit link dependency on python.

The way this works is by moving ScriptInterpreterPython and any other files which interface directly with the Python C API to source\API\bindings\Python. Now, as far as lldb_private is concerned, there is no such thing as ScriptInterpreterPython, there is only ScriptInterpreterBase.

Some work needs to happen on the Xcode project to get it to recognize the new directory structure, due to the interaction between Python and LLDB.framework stuff. I can do simple Xcode stuff, but I'm not quite sure how to handle this, so hopefully someone can help out.

Diff Detail

Event Timeline

zturner updated this revision to Diff 26987.Jun 2 2015, 10:53 AM
zturner retitled this revision from to Remove lldb_private library implicit dependencies on python.
zturner updated this object.
zturner edited the test plan for this revision. (Show Details)
zturner added a reviewer: clayborg.
zturner added a subscriber: Unknown Object (MLST).
zturner updated this revision to Diff 27240.Jun 5 2015, 3:53 PM

Rebase against ToT and update the Xcode project.

Greg, would you mind taking a look at this? I'm leaving next Friday and will be gone for 5 weeks, I don't want to leave such a large patch in limbo for such a long period of time.

I updated the Xcode project as best I could. For some reason I'm getting some errors related to an STL header on one of the files I introduced. I'm not sure exactly why it's causing a problem, and I think it might be a configuration problem with the way the files were added and something to do with LLDB.Framework rather than something I've done wrong in the code.

Bump. Greg, is there any way you could give me an update on this? I'm currently blocked on the Xcode project, as this seems to be fine everywhere else. As I mentioned before, I will be gone for an extended period after Friday, and I would prefer for this not to linger while I'm gone.

+Greg's other email address for visibility.

Ping. Is there anything I can do to help speed this along or get someone to look at it? It's been over a week with no acknowledgement.

klimek added a subscriber: klimek.Jun 11 2015, 1:44 AM

Should work again - made it into our sendgrid's bounce cache due to an smtp
problem...

clayborg requested changes to this revision.Jun 15 2015, 3:47 PM
clayborg edited edge metadata.

Why do we need a ScriptInterpreterFactory class and then pass around a unique pointer to it? Why don't we just add the script interpreters to PluginManager.h and PluginManager.cpp and have all script interpreter languages add themselves as supported script interpreter plug-ins with a create method. Seems more consistent with all of the other plug-in based things we support.

This revision now requires changes to proceed.Jun 15 2015, 3:47 PM

The goal of ScriptInterpreterFactory was to remove the dependency from
CommandInterpreter -> ScriptInterpreterPython. If it can be done with a
plugin as you suggest, then that should work fine.

In any case, I'm OOO for another 5 weeks, so it won't happen until then.

So I can make a Plugin out of it, but I don't think it removes the need for some kind of factory. The issue is that I need to break the dependency between Interpreter and libpython. In other words, nothing in Interpreter can reference (implicitly or explicitly) anything from libpython. Since ScriptInterpreterPython obviously will reference libpython, it means CommandInterpreter cannot just create a ScriptInterpreterPython based on the value of the language enum. It has to be given a factory which lives in library X so that X has dependencies on Interpreter and libpython, but Interpreter does not have a dependency on X.

zturner abandoned this revision.Oct 15 2015, 1:54 PM
source/Interpreter/CommandInterpreter.cpp