matthew-brett / delocate

Find and copy needed dynamic libraries into python wheels
BSD 2-Clause "Simplified" License
262 stars 59 forks source link

Support fixing top-level extension modules #72

Closed McSinyx closed 2 years ago

McSinyx commented 4 years ago

Potential duplication of GH-12, GH-15, GH-22, GH-45, GH-63 and possibly many others. I open this to discuss the strategy to fix this quite common issue.

Rationale

delocate-listdeps works perfectly for these wheels, while delocate-wheel fails silently. This is really confusing to the users. It'd be a lot better if any detected missing lib can be added to the wheel.

Proposal

auditwheel support top-level extension modules by copying the shared libraries to {package-name}.libs and I suggest delocate follow that convention. With package-name being canonicalized to snake_case, this should never cause name collision with the package ending with libs. For example

$ unzip palace-0.1.5-cp38-cp38-manylinux2014_x86_64.whl 
Archive:  palace-0.1.5-cp38-cp38-manylinux2014_x86_64.whl
  inflating: palace.cpython-38-x86_64-linux-gnu.so  
  inflating: palace-0.1.5.dist-info/top_level.txt  
  inflating: palace-0.1.5.dist-info/RECORD  
  inflating: palace-0.1.5.dist-info/METADATA  
  inflating: palace-0.1.5.dist-info/LICENSE  
  inflating: palace-0.1.5.dist-info/WHEEL  
  inflating: palace.libs/libopus-abf1ee53.so.0.3.0  
  inflating: palace.libs/libvorbisfile-c5d289a9.so.3.3.5  
  inflating: palace.libs/libalure2-9f93a6b1.so  
  inflating: palace.libs/libvorbis-02341991.so.0.4.6  
  inflating: palace.libs/libFLAC-b0904aee.so.8.3.0  
  inflating: palace.libs/libgsm-ed01cacd.so.1.0.12  
  inflating: palace.libs/libopusfile-a90fc0fd.so.0.3.0  
  inflating: palace.libs/libopenal-ce7fd2e6.so.1.20.1  
  inflating: palace.libs/libvorbisenc-e32e3ab5.so.2.0.9  
  inflating: palace.libs/libsndfile-7c9b3dac.so.1.0.25  
  inflating: palace.libs/libogg-24eaa32b.so.0.8.0  

I'm looking forward to see your response on this. Happy labor day BTW :fireworks:

McSinyx commented 4 years ago

In our project, we work around it by running the following script:

#!/bin/sh
set -ex
cd $(mktemp -d)
unzip $abspath_to_wheel
delocate-path -L $package_name.dylibs .
wheel=$(basename $abspath_to_wheel)
zip -r $wheel *
mkdir -p $output_dir
mv $wheel $output_dir
tempdir=$(pwd)
cd -
rm -rf $tempdir

The actual setup is available at McSinyx/palace@5406740. Thank you for providing the lower-level utility for hacks like this to be possible, though I still think it'd be nice if this is supported by delocate-wheel.

matthew-brett commented 4 years ago

I'd really like a solution - do you have any interest in trying to make PR for this? Or refactor the current one? @natefoo did a brave effort at this in https://github.com/matthew-brett/delocate/pull/39 - but I had some problems with the various API refactorings - see that PR for details. I'm really happy to guide if you do want to have a go.

McSinyx commented 4 years ago

I'd really like a solution - do you have any interest in trying to make PR for this? Or refactor the current one?

I'd love too, since it turns out the work-around script I posted above had trouble relinking @rpath, as showed in https://github.com/McSinyx/palace/issues/63#issuecomment-623869184.

I'm really happy to guide if you do want to have a go.

It'd be an honor, although I can't promise about the timing since I've just (physically) got back to school this week and they're bombarding us with 6h of lectures everyday for 6 days a week. What I mean is please don't wait for me if you (or any other) figure out how to do it properly, although I'll try my best coordinating my time to help.

bsolomon1124 commented 4 years ago

This is currently preventing bundling from properly occurring with PyYAML ☹️

$ ls dist
PyYAML-5.3.1-cp37-cp37m-macosx_10_15_x86_64.whl
$ delocate-listdeps dist/*.whl
/usr/local/Cellar/libyaml/0.2.4/lib/libyaml-0.2.dylib
$ delocate-wheel -w wheelhouse -v dist/PyYAML-5.3.1-cp37-cp37m-macosx_10_15_x86_64.whl 
Fixing: dist/PyYAML-5.3.1-cp37-cp37m-macosx_10_15_x86_64.whl
$ unzip -l dist/PyYAML-5.3.1-cp37-cp37m-macosx_10_15_x86_64.whl 
Archive:  dist/PyYAML-5.3.1-cp37-cp37m-macosx_10_15_x86_64.whl
  Length      Date    Time    Name
---------  ---------- -----   ----
   318428  05-24-2020 20:35   _yaml.cpython-37m-darwin.so
    13170  05-23-2020 13:33   yaml/__init__.py
     4883  05-23-2020 13:33   yaml/composer.py
    28627  05-23-2020 13:33   yaml/constructor.py
     3846  05-23-2020 13:33   yaml/cyaml.py
     2837  05-23-2020 13:33   yaml/dumper.py
    43006  05-23-2020 13:33   yaml/emitter.py
     2533  05-23-2020 13:33   yaml/error.py
     2445  05-23-2020 13:33   yaml/events.py
     2061  05-23-2020 13:33   yaml/loader.py
     1440  05-23-2020 13:33   yaml/nodes.py
    25495  05-23-2020 13:33   yaml/parser.py
     6794  05-23-2020 13:33   yaml/reader.py
    14184  05-23-2020 13:33   yaml/representer.py
     8970  05-23-2020 13:33   yaml/resolver.py
    51277  05-23-2020 13:33   yaml/scanner.py
     4165  05-23-2020 13:33   yaml/serializer.py
     2573  05-23-2020 13:33   yaml/tokens.py
     1101  05-24-2020 20:36   PyYAML-5.3.1.dist-info/LICENSE
     1690  05-24-2020 20:36   PyYAML-5.3.1.dist-info/METADATA
      111  05-24-2020 20:36   PyYAML-5.3.1.dist-info/WHEEL
     1609  05-24-2020 20:36   PyYAML-5.3.1.dist-info/RECORD

And with 170k downstream deps, I highly doubt the PyYAML maintainers are going to be interested in the "just change your package layout" solution.

Considering manually putting the output paths from delocate-listdeps dist/*.whl into a wheel dylib directory, but that feels like quite a shaky workaround.

sarnold commented 3 years ago

I'm in the same boat as @bsolomon1124 and I ended up using this fork: https://github.com/Chia-Network/delocate/commits/master which merged #39 and adds a (force) file insertion option. Since this seems to work (and builds macos wheels with the right bundled libs directory) I'm wondering what's still holding this up? If I actually had a Mac I would certainly poke at this more and make sure the tests work, etc, but I don't have a Mac (or any "extra" time for that matter). Thanks! (my example is here: https://github.com/freepn/py-re2/actions/runs/487166636)