mgorny marked an inline comment as done.
mgorny added inline comments.

================
Comment at: lldb/scripts/get_relative_lib_dir.py:26
     split_libdir = arch_specific_libdir.split(os.sep)
-    lib_re = re.compile(r"^lib.+$")
+    lib_re = re.compile(r"^lib.*$")
 
----------------
hhb wrote:
> hhb wrote:
> > mgorny wrote:
> > > hhb wrote:
> > > > mgorny wrote:
> > > > > hhb wrote:
> > > > > > mgorny wrote:
> > > > > > > hhb wrote:
> > > > > > > > If we go this way, should we always use 
> > > > > > > > LLDB_PYTHON_RELATIVE_LIBDIR in ScriptInterpreterPython.cpp, and 
> > > > > > > > add some code to make sure it is defined? Because all 
> > > > > > > > assumption of the path can be wrong.
> > > > > > > > 
> > > > > > > > After the change here, I think POSIX will always use 
> > > > > > > > LLDB_PYTHON_RELATIVE_LIBDIR. But for windows, the path is still 
> > > > > > > > hard coded to lib/site-packages.
> > > > > > > > 
> > > > > > > > (maybe finishSwigPythonLLDB.py / make_symlink() can also be 
> > > > > > > > updated to use os.path.relpath? )
> > > > > > > Actually, I think we can kill all this logic by simply passing 
> > > > > > > `''` as prefix, as I did in the CMake part.
> > > > > > I'm not sure. On my machine:
> > > > > > 
> > > > > > $ python3
> > > > > > Python 3.6.8 (default, Jan  3 2019, 03:42:36)
> > > > > > [GCC 8.2.0] on linux
> > > > > > Type "help", "copyright", "credits" or "license" for more 
> > > > > > information.
> > > > > > \>>> import distutils.sysconfig
> > > > > > \>>> distutils.sysconfig.get_python_lib(True, False)
> > > > > > '/usr/lib/python3/dist-packages'
> > > > > > \>>> distutils.sysconfig.get_python_lib(True, False, '')
> > > > > > 'lib/python3/dist-packages'
> > > > > > \>>> distutils.sysconfig.get_python_lib(True, False, '/src/lib')
> > > > > > '/src/lib/lib/python3.6/site-packages'
> > > > > > 
> > > > > Hm, that's interesting. The documentation says:
> > > > > 
> > > > > > If 'prefix' is supplied, use it instead of sys.base_prefix or 
> > > > > > sys.base_exec_prefix -- i.e., ignore 'plat_specific'.
> > > > > 
> > > > > So apparently first arg being true is meaningless then. Maybe we 
> > > > > should go for `get_python_lib(False, False, '')`?
> > > > Well I checked the code. plat_specific IS ignored. But it will test 
> > > > whether prefix "is default".
> > > > 
> > > > ```
> > > > is_default_prefix = not prefix or os.path.normpath(prefix) in ('/usr', 
> > > > '/usr/local')
> > > > ```
> > > > 
> > > > And do things differently based on that. Sigh..
> > > I'm looking through the code of CPython and I don't see `dist-packages` 
> > > anywhere. Is this some local distro patching or something?
> > > 
> > > What I'm really wondering is whether we need to split `.so` and `.py` 
> > > modules. Technically distutils does that but it seems to use the same 
> > > path for both on all platforms I see in `INSTALL_SCHEMES`.
> > It is an Debian patch. 
> > https://bugs.launchpad.net/ubuntu/+source/python3-defaults/+bug/1814653
> > 
> > What make things worse, the path returned by get_python_lib() may not be in 
> > sys.path(). But I guess that's their issue.
> > https://bugs.launchpad.net/ubuntu/+source/python3-defaults/+bug/1814653
> Sorry first link is wrong. Debian patch:
> https://salsa.debian.org/cpython-team/python3-stdlib/blob/master/debian/patches/3.6/distutils-install-layout.diff
Looking at it, it still uses the same location for `.py` and `.so`, just uses 
`dist-packages` instead of `site-packages` for stuff that's supposed to be 
installed by .deb packages. Do you expect LLDB to use dist-packages or 
site-packages by default? I think their intent is the former.


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

https://reviews.llvm.org/D67890



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

Reply via email to