labath added a reviewer: ted.
labath added a comment.
+ 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...
================
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)
----------------
missing `$` in {default_relocatable_python}
================
Comment at: lldb/cmake/modules/LLDBConfig.cmake:153
+ set(LLDB_PYTHON_HOME "${PYTHON_HOME}" CACHE STRING
+ "Path to use as PYTHONHONE in lldb. If a relative path is specified, \
+ it will be resolved at runtime relative to liblldb directory.")
----------------
typo PYTHONHONE
================
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();
----------------
Is the `remove_dot_dots` really necessary? It can produce unexpected results
when `..`s back up over symlinks...
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
