Re: Python C-library import paths
On 2022-04-02 16:04 +0100, Simon McVittie wrote: > On Sat, 02 Apr 2022 at 12:55:37 +0100, Wookey wrote: > > On 2022-04-01 00:30 -0400, M. Zhou wrote: > > > They have written > > > their own ffi loader, so I think it is an upstream bug. The upstream > > > should detect and add multiarch directory to the paths. > > > > A correct implemntation really should use the full ldconfig set of search > > paths. > > I think what they should actually be doing on Linux (at least in distro > packages) is taking a step back from all these attempts to reproduce > the system's search path for public shared libraries, and instead doing > this in https://github.com/apache/tvm/blob/main/python/tvm/_ffi/base.py: > > ctypes.CDLL('libtvm.so.0') > > which will (ask glibc to) do the correct path search, in something like > 99% less code. Aha. That sounds much more the answer to the query in my original mail 'or is there a way to turn on 'just use the system paths' mode?'. This does indeed work to load the library without a lot of manual search-path generation, but it's a bit tricky to use in the existing codebase which wants the loader function to return both a name and a path, and with this magic loading, I don't know the path. _LIB, _LIB_NAME = _load_lib() The second parameter only seems to be used to determine whether libtvm or libtvm_runtime was loaded. I think I can work round that. OK. This patch appears to basically work: -- tvm-0.8.0.orig/python/tvm/_ffi/base.py +++ tvm-0.8.0/python/tvm/_ffi/base.py @@ -48,15 +48,21 @@ else: def _load_lib(): """Load libary by searching possible path.""" -lib_path = libinfo.find_lib_path() # The dll search path need to be added explicitly in # windows after python 3.8 if sys.platform.startswith("win32") and sys.version_info >= (3, 8): for path in libinfo.get_dll_directories(): os.add_dll_directory(path) -lib = ctypes.CDLL(lib_path[0], ctypes.RTLD_GLOBAL) + +use_runtime = os.environ.get("TVM_USE_RUNTIME_LIB", False) +if use_runtime: +lib = ctypes.CDLL('libtvm_runtime.so.0', ctypes.RTLD_GLOBAL) +sys.stderr.write("Loading runtime library %s... exec only\n" % lib._name) +sys.stderr.flush() +else: +lib = ctypes.CDLL('libtvm.so.0', ctypes.RTLD_GLOBAL) lib.TVMGetLastError.restype = ctypes.c_char_p -return lib, os.path.basename(lib_path[0]) +return lib try: @@ -68,10 +74,10 @@ except ImportError: # version number __version__ = libinfo.__version__ # library instance -_LIB, _LIB_NAME = _load_lib() +_LIB = _load_lib() # Whether we are runtime only -_RUNTIME_ONLY = "runtime" in _LIB_NAME +_RUNTIME_ONLY = "runtime" in _LIB._name Yay! _Unless_ you ask to use the runtime version - then it blows up. $ TVM_USE_RUNTIME_LIB=1 tvmc ... File "/usr/lib/python3/dist-packages/tvm/auto_scheduler/cost_model/cost_model.py", line 37, in __init__ self.__init_handle_by_constructor__(_ffi_api.RandomModel) AttributeError: module 'tvm.auto_scheduler._ffi_api' has no attribute 'RandomModel' I've not looked into that yet. Back to the issue of getting the path of the loaded library. Which I no longer obviously _need_, but I would like to know how to do it. There is ctypes.util.find_library(name) which says it returns a path but ctypes.util.find_library('tvm') just returns 'libtvm.so.0' I can't see an attribute in the returned lib object to get the path: lib=ctypes.CDLL('libtvm.so.0') >>> print(lib) >> print(lib.__dir__()) ['_name', '_FuncPtr', '_handle', '__module__', '__doc__', '_func_flags_', '_func_restype_', '__init__', '__repr__', '__getattr__', '__getitem__', '__dict__', '__weakref__', '__hash__', '__str__', '__getattribute__', '__setattr__', '__delattr__', '__lt__', '__le__', '__eq__', '__ne__', '__gt__', '__ge__', '__new__', '__reduce_ex__', '__reduce__', '__subclasshook__', '__init_subclass__', '__format__', '__sizeof__', '__dir__', '__class__'] But _something_ knows, because if I ask for an incorrect thing it prints it out as an 'AtributeError': >>> print(lib.wibble) Traceback (most recent call last): File "", line 1, in File "/usr/lib/python3.9/ctypes/__init__.py", line 387, in __getattr__ func = self.__getitem__(name) File "/usr/lib/python3.9/ctypes/__init__.py", line 392, in __getitem__ func = self._FuncPtr((name_or_ordinal, self)) AttributeError: /usr/lib/x86_64-linux-gnu/libtvm.so.0: undefined symbol: wibble Again, my weak python-foo is holding me back here. Also, you suggest that for a disto package we just need to do lib = ctypes.CDLL('libtvm.so.0', ctypes.RTLD_GLOBAL) and scrap 99% of the code. However, some of the complicated logic in the special _load_lib code seems like maybe it shouldn't be thrown away. Like allowing to override the path with TVM_LIBRARY_PATH envvar, and setting TVM_USE_RUNTIME_LIB to load libtvm_runtime rather than libtvm (I've currently preserved the latter but not the former). I don't know why this is useful, but
Re: Python C-library import paths
On Sat, 02 Apr 2022 at 12:55:37 +0100, Wookey wrote: > On 2022-04-01 00:30 -0400, M. Zhou wrote: > > They have written > > their own ffi loader, so I think it is an upstream bug. The upstream > > should detect and add multiarch directory to the paths. > > A correct implemntation really should use the full ldconfig set of search > paths. I think what they should actually be doing on Linux (at least in distro packages) is taking a step back from all these attempts to reproduce the system's search path for public shared libraries, and instead doing this in https://github.com/apache/tvm/blob/main/python/tvm/_ffi/base.py: ctypes.CDLL('libtvm.so.0') which will (ask glibc to) do the correct path search, in something like 99% less code. Maybe all this complexity is needed on Windows or in a "relocatable" package, but for a distro package it's completely unnecessary and sometimes harmful. > I also don't think it should use the $PATH paths for finding > libraries (but maybe upstream have some reason for doing that) I suspect the reason is: on Windows, PATH is the equivalent of Linux PATH, but it also has a dual role as the equivalent of Linux LD_LIBRARY_PATH. smcv
Re: Python C-library import paths
On 2022-04-01 00:30 -0400, M. Zhou wrote: > On Fri, 2022-04-01 at 02:32 +0100, Wookey wrote: > > > > > > So it tries quite hard to find it, but doesn't know about multiarch > > and thus fails to look in the right place: > > /usr/lib// (/usr/lib/x86_64-linux-gnu/ on this box) > > dlopen should know the multiarch triplet on debian. They have written > their own ffi loader, so I think it is an upstream bug. The upstream > should detect and add multiarch directory to the paths. Agreed. I also don't think it should use the $PATH paths for finding libraries (but maybe upstream have some reason for doing that) I made this patch but it's debian-specific, using dpkg-architecture. @@ -48,7 +49,8 @@ def get_dll_directories(): # $PREFIX/lib/python3.6/site-packages/tvm/_ffi ffi_dir = os.path.dirname(os.path.realpath(os.path.expanduser(__file__))) source_dir = os.path.join(ffi_dir, "..", "..", "..") -install_lib_dir = os.path.join(ffi_dir, "..", "..", "..", "..") +multiarch_name = subprocess.run(['dpkg-architecture', '-q', 'DEB_HOST_MULTIARCH'], stdout=subprocess.PIPE).stdout.decode('utf-8').rstrip() +install_lib_dir = os.path.join(ffi_dir, "..", "..", "..", "..", multiarch_name) (and it took me _ages_ to work out that suprocess.run without that .rstrip() leaves the trailing newline in the string which stops it working!) A correct implemntation really should use the full ldconfig set of search paths. > > OK, but that mostly reveals a second issue: it's looking for > > libtvm.so, but that unversioned link is only provoded in the dev > > package > > libtvm-dev. The library package has the versioned filenames > > /usr/lib/x86_64-linux-gnu/libtvm.so.0 > > /usr/lib/x86_64-linux-gnu/libtvm_runtime.so.0 > > I think it is fine to let it dlopen the libtvm.so, as it says > itself as some sort of "compiler". > > Take pytorch as example, python3-torch has some functionalities > for extending itself with C++. As a result, libtorch-dev is > a dependency of python3-torch. OK. I see there is also a find_include_path in libinfo.py so I guess if it needs the headers too then depending on the -dev package is indeed correct. I've reverted the change to look for libtvm.so.0. > > What it should actually be adding is what's in /etc/ld.so.conf.d/* > > That can be listed with > > /sbin/ldconfig -v 2>/dev/null | grep -v ^$'\t' | cut -d: -f1 > > (yuk? is there really no better way?) OK. I tried this, and given that I don't know any python it went better than I expected. So this code makes an array of paths (as strings) from ldconfig -v output. However I fell at the last hurble of joining the lib_search_dirs array to the dll_paths list such that I get one list of all the paths, not a list where the first entry still has multiple entries. My reading of the docs says that using extend() instead of append() should merge the lists, but it isn't for some reason. I made them both strings, rather than one lot of byte array and one lot of strings, but it still doesn't work. I'm sure this is trivial to fix for someone who actually knows some python, hence this mail. So I get this nice set of paths: search_dirs [['/usr/lib/x86_64-linux-gnu/libfakeroot:', '/usr/local/lib:', '/lib/x86_64-linux-gnu:', '/usr/lib/x86_64-linux-gnu:', '/lib:', '/usr/lib:']] which is combined with the other paths to get this incorrect data structure: dll_path: [['/usr/lib/x86_64-linux-gnu/libfakeroot:', '/usr/local/lib:', '/lib/x86_64-linux-gnu:', '/usr/lib/x86_64-linux-gnu:', '/lib:', '/usr/lib:'], '/usr/lib/python3/dist-packages/tvm/_ffi/..', '/usr/lib/python3/dist-packages/tvm/_ffi/../../../build', '/usr/lib/python3/dist-packages/tvm/_ffi/../../../build/Release', '/usr/lib/python3/dist-packages/tvm/_ffi/../../../lib'] Here is the code: def get_lib_search_dirs(): """Get unix library search path from ldconfig -v""" # loads of output, only lines starting with / are relevant output = subprocess.run(["/sbin/ldconfig","-v"],capture_output=True) paths = output.stdout.split(b'\n') filtered = [] for path in paths: if path[:1] == b'/': filtered.append(path.split()[0].decode()) return [filtered] def get_dll_directories(): """Get the possible dll directories""" # NB: This will either be the source directory (if TVM is run # inplace) or the install directory (if TVM is installed). # An installed TVM's curr_path will look something like: # $PREFIX/lib/python3.6/site-packages/tvm/_ffi ffi_dir = os.path.dirname(os.path.realpath(os.path.expanduser(__file__))) source_dir = os.path.join(ffi_dir, "..", "..", "..") lib_search_dirs = get_lib_search_dirs() print ("search_dirs",lib_search_dirs) dll_path = [] if os.environ.get("TVM_LIBRARY_PATH", None): dll_path.append(os.environ["TVM_LIBRARY_PATH"]) if sys.platform.startswith("linux") or sys.platform.startswith("freebsd"): dll_path.extend(split_env_var("LD_LIBRARY_PATH", ":"))