[Lldb-commits] [PATCH] D74727: Allow customized relative PYTHONHOME

[Haibo Huang via Phabricator via lldb-commits]

hhb added a comment.

> 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.



================
Comment at: lldb/cmake/modules/LLDBConfig.cmake:68
+
+option(LLDB_RELOCATABLE_PYTHON "Use the PYTHONHOME environment variable to locate Python." {default_relocatable_python})
 option(LLDB_USE_SYSTEM_SIX "Use six.py shipped with system and do not install a copy of it" OFF)
----------------
labath wrote:
> missing `$` in {default_relocatable_python}
Good catch. Thanks!


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:298
+        llvm::sys::path::append(path, lldb_python_home);
+        llvm::sys::path::remove_dots(path, /* remove_dot_dots = */ true);
+        absolute_python_home = path.c_str();
----------------
labath wrote:
> Is the `remove_dot_dots` really necessary? It can produce unexpected results when `..`s back up over symlinks...
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74727/new/

https://reviews.llvm.org/D74727