Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-python extension dependencies with external dependencies are not recursively patched #136

Closed
ehashman opened this issue Jan 24, 2019 · 14 comments · Fixed by #298
Closed

Comments

@ehashman
Copy link
Member

As reported by @thomaslima:

Our wheel has a complicated dependency structure. But the gist of it is the following:

dbcore.so (py_ext) -> lib_tl.so (non-py_ext) -> libcurl.so (external library to be grafted)

Because lib_tl is a non-py extension of dbcore, it was not added to the full_external_refs dictionary, which is used by repair.py to patch the sonames. So the final result is that lib_tl was not patched, and the package was broken.

See also #134.

@dsteinmo
Copy link

Is there confirmation that the PR proposed by @thomaslima fixes this problem? My project also has a complicated dependency tree, and it would be ideal if all external dependencies that are bundled get patched to include $ORIGIN in rpath. Thanks.

@grisuthedragon
Copy link

We have the same problem that our python extension c_interface.so depends on a library libcode.so which we also ship in the .libs folder. This this is require since the build process of libcode.so does not depend on python and we only create the c_interface.so using setuptools. libcode.so has all dependencies to the external libaries inside.

@dsteinmo With the PR #139 integrated in the auditwheel script, the problem is fixed and the wheels work well.

@dsteinmo
Copy link

Sounds good. It does not appear that @thomaslima has engaged the upstream maintainers by rebasing his branch. If I find some time, I'll attempt a fork + new PR.

@grisuthedragon
Copy link

grisuthedragon commented Aug 29, 2019

I extracted the crucual part out of @thomaslima commit, which we use to build our wheels.

Here is the patch: auditwheel.txt

@dsteinmo
Copy link

Thanks; I might have spoken too soon, it looks like this is still active, but in a separate branch alongside a separate issue: #134. I'll leave it in the hands of @thomaslima and the maintainers.

@thomaslima
Copy link
Contributor

thomaslima commented Aug 29, 2019 via email

@delarosatrevin
Copy link

Hi everyone,

Is there any progress on this issue?
I'm facing the same problem to build a wheel for my package emcore.

It is a scientific image processing library that depends on external libraries such as fftw3, libpng, libtiff, and sqlite3 among others. We build libemcore that does not depends on Python and then the Python extension using pybind11.

Thanks in advance.

@myselfhimself
Copy link

myselfhimself commented Dec 7, 2020

It seems that this command: (relying on the copydeps package)

copydeps gmic.cpython-37m-x86_64-linux-gnu.so --exclude exclude-list-gmic

with exclude-list-gmic's contents here

finds and copies shared libraries deeper than PR #139 ... Still struggling with auditwheel+ PR #139's shallow shared libraries lookup and copy into a repaired wheel archive... (libcurl)

@myselfhimself
Copy link

My copydeps-based script did not work well.

I am giving up on depending on libcurl for linux wheel building and will rely instead of automatic OS execution of the curl executable by the gmic library.

I am getting back to a standard auditwheel version

@bdice
Copy link
Contributor

bdice commented Feb 3, 2021

This problem is happening for me as well (see issue glotzerlab/freud#720). I have confirmed that PR #139 does solve the problem for our use case. We have a few Python extensions that link to libfreud.so and libtbb.so. libfreud.so is also linked to libtbb.so. auditwheel correctly relocates the RPATHs of our Python extensions, but does not alter the RPATH of libfreud.so to point to the relocated libtbb.so.

It looks like PR #139 was stalled because the original author needed to update it. I will update it and file a new PR so that this can hopefully be fixed in the next release of auditwheel.

@thomaslima
Copy link
Contributor

thomaslima commented Feb 3, 2021 via email

@bdice
Copy link
Contributor

bdice commented Feb 3, 2021

@thomaslima No problem! Thanks for your initial work on it. I am updating the tests, too. PR #134 with the corresponding tests was reverted in #190, so I will try to improve it according to the maintainers’ comments on why it was reverted.

@bdice
Copy link
Contributor

bdice commented Feb 6, 2021

@thomaslima I created a new pull request #283 to solve this issue, based on your previous work. Thanks for your initial efforts in #139, it just took a little bit of cleanup and updating tests! 👍 🚀

psavery added a commit to OpenChemistry/stempy that referenced this issue Feb 19, 2021
pypa/auditwheel#136 results in pure C++ libraries not getting patched
by auditwheel. This is a problem for us, since our pure C++ library,
libstem.so, needs its rpath patched for hdf5.

pypa/auditwheel#283 fixes this issue, but is not yet merged. Until
it gets merged and added to quay.io/pypa/manylinux2010_x86_64,
we will patch auditwheel inside quay.io/pypa/manylinux2010_x86_64 to
include this fix.

Signed-off-by: Patrick Avery <[email protected]>
psavery added a commit to OpenChemistry/stempy that referenced this issue Feb 19, 2021
pypa/auditwheel#136 results in pure C++ libraries not getting patched
by auditwheel. This is a problem for us, since our pure C++ library,
libstem.so, needs its rpath patched for hdf5.

pypa/auditwheel#283 fixes this issue, but is not yet merged. Until
it gets merged and added to quay.io/pypa/manylinux2010_x86_64,
we will patch auditwheel inside quay.io/pypa/manylinux2010_x86_64 to
include this fix.

Signed-off-by: Patrick Avery <[email protected]>
@molpopgen
Copy link

Am I mistaken, or is this a duplicate of #48? Came across both after running into this issue myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment