This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Don't build Interpreter unittest if python is disabled
Needs ReviewPublic

Authored by loladiro on Jul 12 2015, 2:31 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None

Diff Detail

Repository
rL LLVM

Event Timeline

loladiro updated this revision to Diff 29525.Jul 12 2015, 2:31 PM
loladiro retitled this revision from to [CMake] Don't build Interpreter unittest if python is disabled.
loladiro updated this object.
loladiro added a reviewer: labath.
loladiro set the repository for this revision to rL LLVM.
loladiro added a subscriber: lldb-commits.
labath edited edge metadata.Jul 13 2015, 2:39 AM
labath added a subscriber: zturner.

What kind of error were you getting here? For me the test seems to run fine even with LLDB_DISABLE_PYTHON and the resulting executable does not seem to depend on it. However, it seems that unittests/Interpreter/CMakeLists.txt includes the python library unconditionally. Could you check if the tests will run for you if we just disable the ${PYTHON_LIBRARY} dependency in that file?

I was getting a link error with python. I just assumed it was required since it was explicitly linked. Do you mean remove it entirely, independent of whether LLDB_DISABLE_PYTHON is set, or just in that case?

I was getting a link error with python. I just assumed it was required since it was explicitly linked. Do you mean remove it entirely, independent of whether LLDB_DISABLE_PYTHON is set, or just in that case?

I am pretty sure that the dependency is required if the whole lldb is built with python. I am proposing we make the just the dependency on python conditional on the LLDB_DISABLE_PYTHON setting. Could you check if that works for you?

Hi, I have been gone for the past 5 weeks, but I have an unfinished patch
which will make Interpreter independent from Python so you can build the
interpreter unittests regardless of your setting of LLDB_DISABLE_PYTHON.
This actually makes more sense, because even without my patch Python is
only a small subset of the functionality contained in Interpreter, and the
other stuff should still be able to be unit tested.

Can you try this again (without your change)? Building Interpreter unittest should no longer cause a link against python. (What OS are you on btw?)

This was a fresh Ubuntu VM. I'll try again when I get the chance.

You might run into one remaining issue. In HostPosix (or maybe HostLinux) it manually #includes "lldb-python.h" and there is 1 or 2 lines of code that that gets the python version using some #define from the python header files. This is the last piece of the puzzle that I did not fix.

The way to fix this is to update ScriptInterpreter base class to provide an abstract GetInterpreterVersion() method. Have ScriptInterpreterPython override this to pull the value from the python header. Then have HostLinux or HostPosix or whatever it is use the PluginManager to get the Plugin for eLanguagePython, and call the GetInterpreterVersion method.

I didn't fix this yet because my primary focus has been Windows, but AFAIK that's all that's necessary to remove the last implicit link dependency on Python.

labath resigned from this revision.Nov 25 2015, 8:34 AM
labath removed a reviewer: labath.