Allow customized relative PYTHONHOME

[hhb]

Summary

This change allows a hard coded relative PYTHONHOME setting. So that python can easily be packaged together with lldb.

The change includes:

1. Extend LLDB_RELOCATABLE_PYTHON to all platforms. It defaults to ON for platforms other than Windows, to keep the behavior compatible.
2. Allows to customize LLDB_PYTHON_HOME. But still defaults to PYTHON_HOME.
3. LLDB_PYTHON_HOME can be a path relative to liblldb. If it is relative, we will resolve it before send it to Py_DecodeLocale.

[labath]

+ ted, who I believe has a stake in the relocatable python game

Your use case seems reasonable, but I find it hard to understand the meanings of the options in this patch. IIUC, there are now two ways to create an lldb with a "relocatable" python

1. set LLDB_RELOCATABLE_PYTHON to ON, and set the PYTHONHOME environment variable before launching lldb
2. set LLDB_RELOCATABLE_PYTHON to OFF, and also set LLDB_PYTHON_HOME to a relative path

It took me like five minutes to understand how setting LLDB_RELOCATABLE_PYTHON=OFF can help you ship python side-by-side with lldb (i.e. make it position indepent/relocatable). I think the main cause of the confusion is that the name LLDB_RELOCATABLE_PYTHON does not express very well what that option does -- it really should be something like LLDB_EMBED_PYTHON_HOME. Can we maybe rename it to something like that? Or maybe even just a single LLDB_PYTHON_HOME variable, with the empty value meaning we use the default python mechanism (PYTHONHOME env var, or the baked-in python default)?

The other thing which bugs me is that the relative python path chosen here will likely only be correct once lldb is installed. Will it be possible to run lldb (and its test suite) configured in this way directly from the build tree? I don't know if there's anything we can or should do about that, but this situation seems less than ideal...

lldb/cmake/modules/LLDBConfig.cmake

67

missing $ in {default_relocatable_python}

155

typo PYTHONHONE

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

298

Is the remove_dot_dots really necessary? It can produce unexpected results when ..s back up over symlinks...

[hhb]

Fix comments

[hhb]

Fix comments.

[hhb]

Can we maybe rename it to something like that?

LLDB_RELOCATABLE_PYTHON is misleading. I'd be happy to rename it. But that may break people who already set this flag.

Is there a standard procedure to do this? Maybe add the new one and remove the old one a year later? Or maybe fail when the old is set. So that at least people know how to fix?

Or maybe even just a single LLDB_PYTHON_HOME variable, with the empty value meaning we use the default python mechanism (PYTHONHOME env var, or the baked-in python default)?

To do that, we have to make LLDB_PYTHON_HOME default to PYTHON_HOME for Windows. And if windows users want to make it "relocatable", they should set LLDB_PYTHON_HOME back to empty. That's also somehow weird...

The other thing which bugs me is that the relative python path chosen here will likely only be correct once lldb is installed. Will it be possible to run lldb (and its test suite) configured in this way directly from the build tree? I don't know if there's anything we can or should do about that, but this situation seems less than ideal...

If someone uses relative python, that mean they should be responsible to put Python at the right place. No matter it is build tree or install tree. Or in other words, the install tree won't work by default either. People still need to put a symlink to python at the right place.

lldb/cmake/modules/LLDBConfig.cmake

67

Good catch. Thanks!

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

298

I was just trying to make the string shorter. It is not necessary. Removed.

But just curious what is the unexpected result? Say if I have a symlink "/src/python" -> "/usr/local/lib/python3.7", a "/src/python/../" should be resolved to /src, with or without remove_dot_dots?

Well I guess things get more interesting when liblldb is a symlink... That doesn't affect my use case though.

[hhb]

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

298

Kind of understood after searching around. For me remove_dot_dots is the expected behavior. But yea let's remove it to reduce some confusion...

[JDevlieghere]

In D74727#1881055, @hhb wrote:

Can we maybe rename it to something like that?

LLDB_RELOCATABLE_PYTHON is misleading. I'd be happy to rename it. But that may break people who already set this flag.

Is there a standard procedure to do this? Maybe add the new one and remove the old one a year later? Or maybe fail when the old is set. So that at least people know how to fix?

We've made breaking changes like that in the past. This might even be a good thing when we're changing behavior.

Or maybe even just a single LLDB_PYTHON_HOME variable, with the empty value meaning we use the default python mechanism (PYTHONHOME env var, or the baked-in python default)?

To do that, we have to make LLDB_PYTHON_HOME default to PYTHON_HOME for Windows. And if windows users want to make it "relocatable", they should set LLDB_PYTHON_HOME back to empty. That's also somehow weird...

Another alternative would be to default to "Auto", like we do with the optional dependencies, in which case it would default to PYTHON_HOME. Empty would always signal a relocatable python.

The other thing which bugs me is that the relative python path chosen here will likely only be correct once lldb is installed. Will it be possible to run lldb (and its test suite) configured in this way directly from the build tree? I don't know if there's anything we can or should do about that, but this situation seems less than ideal...

If someone uses relative python, that mean they should be responsible to put Python at the right place. No matter it is build tree or install tree. Or in other words, the install tree won't work by default either. People still need to put a symlink to python at the right place.

[labath]

In D74727#1881962, @JDevlieghere wrote:

In D74727#1881055, @hhb wrote:

Can we maybe rename it to something like that?

LLDB_RELOCATABLE_PYTHON is misleading. I'd be happy to rename it. But that may break people who already set this flag.

Is there a standard procedure to do this? Maybe add the new one and remove the old one a year later? Or maybe fail when the old is set. So that at least people know how to fix?

We've made breaking changes like that in the past. This might even be a good thing when we're changing behavior.

Yeah, I wouldn't worry about that. When we were doing some dependency refactorings two months ago, we left in some compat code. However, this was only because some buildbots depended on the old options, and the code was removed as soon as those were updated.

But you don't need to worry about that -- I don't see any bot using this option.

Or maybe even just a single LLDB_PYTHON_HOME variable, with the empty value meaning we use the default python mechanism (PYTHONHOME env var, or the baked-in python default)?

To do that, we have to make LLDB_PYTHON_HOME default to PYTHON_HOME for Windows. And if windows users want to make it "relocatable", they should set LLDB_PYTHON_HOME back to empty. That's also somehow weird...

Yes, I can see how that could be confusing. Then let's stick with two options, but rename LLDB_RELOCATABLE_PYTHON to indicate that is really about hardcoding the PYTHONHOME path?

The other thing which bugs me is that the relative python path chosen here will likely only be correct once lldb is installed. Will it be possible to run lldb (and its test suite) configured in this way directly from the build tree? I don't know if there's anything we can or should do about that, but this situation seems less than ideal...

If someone uses relative python, that mean they should be responsible to put Python at the right place. No matter it is build tree or install tree. Or in other words, the install tree won't work by default either. People still need to put a symlink to python at the right place.

Yeah, that makes kind of sense. I think I can go with that...

[labath]

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

298

Yeah, a lot of people think that is the expected behavior, and then get surprised when something different happens. Unfortunately, the surprise does not happen frequently enough to for people to become aware of that...

[hhb]

Rename LLDB_RELOCATABLE_PYTHON to LLDB_EMBED_PYTHON_HOME.

[labath]

This looks good to me.

lldb/cmake/modules/LLDBConfig.cmake

67–69

Maybe this could also be in the if(LLDB_ENABLE_PYTHON) block below?

[hhb]

Move option() into if

[hhb]

The code should check on LLDB_EMBED_PYTHON_HOME

[stella.stamenova]

It looks like this broke the Windows LLDB Buildbot:

http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/14048/steps/build/logs/stdio

[hans]

In D74727#1887325, @stella.stamenova wrote:

It looks like this broke the Windows LLDB Buildbot:

http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/14048/steps/build/logs/stdio

Seems it may have also broken the Windows LLDB releases is general: https://github.com/llvm/llvm-project/issues/54589

and should the docs be updated too? https://github.com/llvm/llvm-project/blob/llvmorg-14.0.4/lldb/docs/resources/build.rst#id7

lldb/cmake/modules/LLDBConfig.cmake

 
 add_optional_dependency(LLDB_ENABLE_LIBEDIT "Enable editline support in LLDB" LibEdit LibEdit_FOUND)
 add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support in LLDB" CursesAndPanel CURSESANDPANEL_FOUND)
 add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA compression support in LLDB" LibLZMA LIBLZMA_FOUND)
 add_optional_dependency(LLDB_ENABLE_LUA "Enable Lua scripting support in LLDB" LuaAndSwig LUAANDSWIG_FOUND)
 add_optional_dependency(LLDB_ENABLE_PYTHON "Enable Python scripting support in LLDB" PythonInterpAndLibs PYTHONINTERPANDLIBS_FOUND)
 add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in LLDB" LibXml2 LIBXML2_FOUND VERSION 2.8)
 
-option(LLDB_RELOCATABLE_PYTHON "Use the PYTHONHOME environment variable to locate Python." OFF)
 option(LLDB_USE_SYSTEM_SIX "Use six.py shipped with system and do not install a copy of it" OFF)
 option(LLDB_USE_ENTITLEMENTS "When codesigning, use entitlements if available" ON)
 option(LLDB_BUILD_FRAMEWORK "Build LLDB.framework (Darwin only)" OFF)
 option(LLDB_NO_INSTALL_DEFAULT_RPATH "Disable default RPATH settings in binaries" OFF)
 option(LLDB_USE_SYSTEM_DEBUGSERVER "Use the system's debugserver for testing (Darwin only)." OFF)
 option(LLDB_SKIP_STRIP "Whether to skip stripping of binaries when installing lldb." OFF)
 
 if (LLDB_USE_SYSTEM_DEBUGSERVER)
   # The custom target for the system debugserver has no install target, so we
   # need to remove it from the LLVM_DISTRIBUTION_COMPONENTS list.
   if (LLVM_DISTRIBUTION_COMPONENTS)
     list(REMOVE_ITEM LLVM_DISTRIBUTION_COMPONENTS debugserver)
     set(LLVM_DISTRIBUTION_COMPONENTS ${LLVM_DISTRIBUTION_COMPONENTS} CACHE STRING "" FORCE)
   endif()
 endif()
 
else()
     set(LLDB_HAVE_EL_RFUNC_T 1)
   endif()
   set(CMAKE_REQUIRED_LIBRARIES)
   set(CMAKE_REQUIRED_INCLUDES)
   set(CMAKE_EXTRA_INCLUDE_FILES)
 endif()
 
 if (LLDB_ENABLE_PYTHON)
+  if(CMAKE_SYSTEM_NAME MATCHES "Windows")
+    set(default_embed_python_home ON)
+  else()
+    set(default_embed_python_home OFF)
+  endif()
+  option(LLDB_EMBED_PYTHON_HOME
+    "Embed PYTHONHOME in the binary. If set to OFF, PYTHONHOME environment variable will be used to to locate Python."
+    ${default_embed_python_home})
+
   include_directories(${PYTHON_INCLUDE_DIRS})
-  if ("${CMAKE_SYSTEM_NAME}" STREQUAL "Windows" AND NOT LLDB_RELOCATABLE_PYTHON)
+  if (LLDB_EMBED_PYTHON_HOME)
     get_filename_component(PYTHON_HOME "${PYTHON_EXECUTABLE}" DIRECTORY)
-    file(TO_CMAKE_PATH "${PYTHON_HOME}" LLDB_PYTHON_HOME)
+    set(LLDB_PYTHON_HOME "${PYTHON_HOME}" CACHE STRING
+      "Path to use as PYTHONHOME in lldb. If a relative path is specified, it will be resolved at runtime relative to liblldb directory.")
   endif()
 endif()
 
 if (LLVM_EXTERNAL_CLANG_SOURCE_DIR)
   include_directories(${LLVM_EXTERNAL_CLANG_SOURCE_DIR}/include)
 else ()
   include_directories(${CMAKE_SOURCE_DIR}/tools/clang/include)
 endif ()

lldb/include/lldb/Host/Config.h.cmake

 #cmakedefine01 LLDB_ENABLE_LIBEDIT
 
 #cmakedefine01 LLDB_ENABLE_LIBXML2
 
 #cmakedefine01 LLDB_ENABLE_LUA
 
 #cmakedefine01 LLDB_ENABLE_PYTHON
 
+#cmakedefine01 LLDB_EMBED_PYTHON_HOME
+
 #cmakedefine LLDB_PYTHON_HOME "${LLDB_PYTHON_HOME}"
 
 #define LLDB_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}"
 
 #endif // #ifndef LLDB_HOST_CONFIG_H

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

if (m_was_already_initialized) {
       PyEval_SaveThread();
     }
 
     m_stdin_tty_state.Restore();
   }
 
 private:
   void InitializePythonHome() {
-#if defined(LLDB_PYTHON_HOME)
+#if LLDB_EMBED_PYTHON_HOME
+#if PY_MAJOR_VERSION >= 3
+    typedef const wchar_t* str_type;
+#else
+    typedef char* str_type;
+#endif
+    static str_type g_python_home = []() -> str_type {
+      const char *lldb_python_home = LLDB_PYTHON_HOME;
+      const char *absolute_python_home = nullptr;
+      llvm::SmallString<64> path;
+      if (llvm::sys::path::is_absolute(lldb_python_home)) {
+        absolute_python_home = lldb_python_home;
+      } else {
+        FileSpec spec = HostInfo::GetShlibDir();
+        if (!spec)
+          return nullptr;
+        spec.GetPath(path);
+        llvm::sys::path::append(path, lldb_python_home);
+        absolute_python_home = path.c_str();
+      }
 #if PY_MAJOR_VERSION >= 3
       size_t size = 0;
-    static wchar_t *g_python_home = Py_DecodeLocale(LLDB_PYTHON_HOME, &size);
+      return Py_DecodeLocale(absolute_python_home, &size);
 #else
-    static char g_python_home[] = LLDB_PYTHON_HOME;
+      return strdup(absolute_python_home);
 #endif
+    }();
+    if (g_python_home != nullptr) {
       Py_SetPythonHome(g_python_home);
+    }
 #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 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.
     static char path[] =