[ScriptInterpreter] Make sure that PYTHONHOME is right.

[davide]

Summary

For the only version of Python actually supported on Darwin.

rdar://problem/40961425

[JDevlieghere]

I think this is okay, although I would prefer if there was a way to deduct the correct python home from the Python library we link against. I guess if that was possible we didn't need to set the PythonHome in the first place. We also can't deduct it from the Python interpreter (PYTHON_EXECUTABLE) because as we've seen in the past, it's possible that the one in your path (e.g. when installed with HomeBrew or from python.org) is different from the system one we link against on Darwin.

TL;DR LGTM

[aprantl]

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

181 ↗

(On Diff #191953)

Just to don't break hypothetical users building their own LLDB on ancient versions of macOS let's also change the minor version.

[davide]

Adrian's feedback

[labath]

It sounds to me like you could achieve the same thing by generalizing the LLDB_PYTHON_HOME logic in LLDBConfig.cmake. This would have the advantage of centralizing the way we manage python-finding logic (instead of each OS doing it's own thing) and also enable those users, who know what they are doing, to override this logic and point lldb to a different python. (I don't know if there are any such users, but it does not sounds like an impossible scenario).

I think all it would take is to do something like:

• move LLDB_RELOCATABLE_PYTHON handling outside of if(WINDOWS)
• have the default value of LLDB_RELOCATABLE_PYTHON be false for darwin
• possibly tweak the python-finding logic so that it prefers the one in /System/Library/Frameworks/...

[ted]

This doesn't look correct to me - it looks like there are 1 too many #endifs. I think the one at line 179 should be removed - it should have been replaced by the #else that is at line 180.

[davide]

In D59719#1442181, @ted wrote:

This doesn't look correct to me - it looks like there are 1 too many #endifs. I think the one at line 179 should be removed - it should have been replaced by the #else that is at line 180.

Update your checkout.

[davide]

In D59719#1441254, @labath wrote:

It sounds to me like you could achieve the same thing by generalizing the LLDB_PYTHON_HOME logic in LLDBConfig.cmake. This would have the advantage of centralizing the way we manage python-finding logic (instead of each OS doing it's own thing) and also enable those users, who know what they are doing, to override this logic and point lldb to a different python. (I don't know if there are any such users, but it does not sounds like an impossible scenario).

I think all it would take is to do something like:

• move LLDB_RELOCATABLE_PYTHON handling outside of if(WINDOWS)
• have the default value of LLDB_RELOCATABLE_PYTHON be false for darwin
• possibly tweak the python-finding logic so that it prefers the one in /System/Library/Frameworks/...

I'll try to make this more generic. Thanks for the input.

[davide]

In D59719#1441254, @labath wrote:

It sounds to me like you could achieve the same thing by generalizing the LLDB_PYTHON_HOME logic in LLDBConfig.cmake. This would have the advantage of centralizing the way we manage python-finding logic (instead of each OS doing it's own thing) and also enable those users, who know what they are doing, to override this logic and point lldb to a different python. (I don't know if there are any such users, but it does not sounds like an impossible scenario).

I think all it would take is to do something like:

• move LLDB_RELOCATABLE_PYTHON handling outside of if(WINDOWS)
• have the default value of LLDB_RELOCATABLE_PYTHON be false for darwin
• possibly tweak the python-finding logic so that it prefers the one in /System/Library/Frameworks/...

Oh, this won't work with xcodebuild, I'm afraid.

[labath]

In D59719#1442563, @davide wrote:

In D59719#1441254, @labath wrote:

It sounds to me like you could achieve the same thing by generalizing the LLDB_PYTHON_HOME logic in LLDBConfig.cmake. This would have the advantage of centralizing the way we manage python-finding logic (instead of each OS doing it's own thing) and also enable those users, who know what they are doing, to override this logic and point lldb to a different python. (I don't know if there are any such users, but it does not sounds like an impossible scenario).

I think all it would take is to do something like:

• move LLDB_RELOCATABLE_PYTHON handling outside of if(WINDOWS)
• have the default value of LLDB_RELOCATABLE_PYTHON be false for darwin
• possibly tweak the python-finding logic so that it prefers the one in /System/Library/Frameworks/...

Oh, this won't work with xcodebuild, I'm afraid.

Why not? The xcode build could hardcode the value for LLDB_PYTHON_HOME, just like you do here now. That could be done either in lldb/Host/Config.h, or by passing the appropriate -D flag to the compiler via the xcode project file. My preference would be to use lldb/Host/Config.h, and to also move the cmake build off of the explicit -D, and put this variable into Config.h.cmake as well.

source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

 #if PY_MAJOR_VERSION >= 3
     size_t size = 0;
     static wchar_t *g_python_home = Py_DecodeLocale(LLDB_PYTHON_HOME, &size);
 #else
     static char g_python_home[] = LLDB_PYTHON_HOME;
 #endif
     Py_SetPythonHome(g_python_home);
 #endif
+#else
+#if defined(__APPLE__) && PY_MAJOR_VERSION == 2 && PY_MINOR_VERSION == 7
+    // For Darwin, the only Python version supported is the one shipped in the OS
+    // and linked with lldb. Other installation of Python may have higher priorities
+    // in the path, overriding PYTHONHOME and causing problems/incompatibilities.
+    // In order to avoid confusion, always hardcode the PythonHome to be right,
+    // as it's not going to change.
+    Py_SetPythonHome("/System/Library/Frameworks/Python.framework/Versions/2.7");
+#endif
+#endif
   }
 
   void InitializeThreadsPrivate() {
 // Since Python 3.7 `Py_Initialize` calls `PyEval_InitThreads` inside itself,
 // so there is no way to determine whether the embedded interpreter
 // was already initialized by some external code. `PyEval_ThreadsInitialized`
 // would always return `true` and `PyGILState_Ensure/Release` flow would be
 // executed instead of unlocking GIL with `PyEval_SaveThread`. When