-
Notifications
You must be signed in to change notification settings - Fork 149
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
Repair does not handle recursive libraries correctly #48
Comments
Ok, the problem boils down to If I replace the Before I do a PR that removes this if-statement: Anyone remembers why it is there in the first place? |
Man, I wish we had a better test suite. I don't remember why that |
May be |
Then I vote we go ahead and rip it out :-). (And add a test for this case, of course.) Either everything will be fine, or else, something will break and then we'll know what test case we need to add. |
We can always rebuild the manylinux wheels for numpy, scipy and a couple other projects and check that their own tests still pass. |
I believe the if statement works as follows: when hunting through various required libraries, we use the Python extension as a sort of anchor point to start looking for dependency chains of the various ELF files. If we don't do this, we get the test failure identified in #107, where repaired libs that were included in the wheel don't have any paths set and our processing logic chokes. (@geofft to correct me if I'm remembering this wrong.) The good news is, I think this issue should be addressed in #110, which fixes this bug and safely processes binary files included in wheels that are not Python extensions. |
I feel like any time we get a report like this we should add it to our tests 😄 Let's see if we can add one for building pyarrow if possible. |
|
In that case I will just close this. |
So the issue is now libraries downstream from pyarrow fail audit wheel in trying to find pyarrow |
We just also ran into this issue building python bindings for a c++ shared library using pybind11. It would be great if auditwheel would also fix the rpath of included libraries. The situation is like this:
libfoo is build by setup.py, auditwheel correctly only fixes the library name for the python module, not the same library dependency of ldd output of the module:
Observe the ldd output of the shared library also linking vs protobuf:
|
To make it clearer, the situation in the unrepaired wheel is this:
After repairing,
Observe that |
Here is an example project (very artificial, but it displays the problem) which has https://github.com/maxnoe/example_repair_fail ldd outputs after
this is correct. But
|
We encounter this problem too: https://github.com/tomlegkov/marine-python Everything works well when I change: https://github.com/pypa/auditwheel/blob/master/auditwheel/wheel_abi.py#L76 to |
Could it be that this line is the culprit: auditwheel/auditwheel/wheel_abi.py Line 119 in 70e167d
Being in needed libs does not mean the dependencies have been checked if I read the code correctly. |
This is the script I am using now to fix the wheels after the intial auditwheel run: '''
This script fixes missing patches to the library names and rpaths
after running `auditwheel repair`.
It should no longer be needed after this auditwheel issue is fixed:
https://github.com/pypa/auditwheel/issues/48
'''
import os
from zipfile import ZipFile, ZIP_DEFLATED
import subprocess as sp
import tempfile
import argparse
from pathlib import Path
import re
parser = argparse.ArgumentParser()
parser.add_argument('inwheel')
parser.add_argument('outdir')
def zip_dir(zip_file, path):
for root, _, files in os.walk(path):
for file in files:
file = os.path.join(root, file)
zip_file.write(file, os.path.relpath(file, path))
def lib_name(lib):
return Path(lib).stem.partition('.')[0].partition('-')[0]
def get_needed_libs(lib):
result = sp.run(['readelf', '-d', str(lib)], stdout=sp.PIPE, encoding='utf-8')
out = result.stdout
libs = re.findall(r'\(NEEDED\)\s+Shared library: \[(.*)\]', out)
return libs
def fix_wheel(inwheel, outdir):
with tempfile.TemporaryDirectory(prefix='fix_wheel_') as tmpdir:
tmpdir = Path(tmpdir)
print('Extracting wheel')
ZipFile(inwheel).extractall(path=tmpdir)
libsdir = next(tmpdir.glob('*.libs'))
package_name = libsdir.stem
libs = [p.name for p in libsdir.glob('*.so*')]
without_hash = [lib_name(lib) for lib in libs]
vendored_libs = dict(zip(without_hash, libs))
for lib in tmpdir.glob('**/*.so'):
print('Patching ', lib)
rpath = os.path.relpath(libsdir, lib.parent)
print(rpath)
rpath = f'$ORIGIN:$ORIGIN/{rpath}'
print('Using RPATH', rpath)
sp.run(['patchelf', '--force-rpath', '--set-rpath', rpath , str(lib)], check=True)
needed_libs = get_needed_libs(lib)
for needed in needed_libs:
name = lib_name(needed)
new = vendored_libs.get(name)
if new and new != needed:
print(f'replacing {needed} with {new} in {lib.name}')
sp.run(['patchelf', '--remove-needed', needed, str(lib)], check=True)
sp.run(['patchelf', '--add-needed', new, str(lib)], check=True)
outdir = Path(outdir)
outdir.mkdir(exist_ok=True, parents=True)
outwheel = outdir / Path(inwheel).name
print(f'Writing fixed wheel to {outwheel}')
with ZipFile(outwheel, 'w', compression=ZIP_DEFLATED, compresslevel=7) as zip_file:
zip_dir(zip_file, tmpdir)
def main():
args = parser.parse_args()
fix_wheel(args.inwheel, args.outdir)
if __name__ == '__main__':
main() |
#136 seems to duplicate this issue.
|
Using the latest manylinux image (and so hopefully the latest auditwheel version) I have the problem that
repair
does not correctly relink all libraries. The dependency hierarchy is as follows:parquet.so
is a Python module (Cython generated) that depends on the C++ librarieslibpyarrow.so
andlibarrow.so
directly.libpyarrow.so
is a pure C++ helper library but also depends onlibarrow.so
directly.parquet.so
has the correct links after the repair butlibpyarrow.so
does not.Both libs
parquet.so
andlibpyarrow.so
are part of the wheel,libarrow.so
is in this case an external one).Console output:
Library linkages:
Happy to fix this on my own but I would need some pointers where I should start to investigate!
The text was updated successfully, but these errors were encountered: