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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to