[Lldb-commits] [PATCH] D68442: [lldb] Unifying lldb python path

Sat Oct 5 10:29:00 PDT 2019

mgorny added inline comments.

================
Comment at: lldb/CMakeLists.txt:42
+    COMMAND ${PYTHON_EXECUTABLE}
+        -c "import distutils.sysconfig, sys; print(distutils.sysconfig.get_python_lib(True, False, sys.argv[1]))"
+        ${CMAKE_INSTALL_PREFIX}
----------------
hhb wrote:
> hhb wrote:
> > mgorny wrote:
> > > I still like my `(False, False, '')` version better than having to recalculate path afterwards.
> > I don't have a strong opinion here. Let's see what labath@ think.
> > 
> > That been said, I did this because some distribution modified get_python_lib() to return differently based on prefix. One example is Debian/Ubuntu, where 'dist-packages' will be used if prefix is '', '/usr' or '/usr/local'.
> > 
> > In reality, that only makes difference when CMAKE_INSTALL_PREFIX is set. But I guess it doesn't really matter whether we use 'dist-packages' or 'site-packages' that time, as long as it is consistent everywhere.
> Considering DESTINT, maybe empty string is better...
> 
> By the way, what's the first parameter plat_specific? In all platforms I have, it doesn't make any difference...
Technically, it's for arch-dependent vs arch-independent modules, i.e. should be True for .so extensions and False for .py modules.

Judging by the documentation, it's only used to switch between `sys.base_prefix` (i.e. `--prefix` given to build Python) and `sys.base_exec_prefix` (`--exec-prefix`). However, I'm not aware of any platform where two different prefixes are used for Python.

When a prefix is given as third argument, its value is ignored. So True vs False shouldn't really matter here, hence I've left it at the default (False).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68442