matthew-brett / delocate

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

Location of bundled libraries is non-deterministic #192

Closed mwoehlke-kitware closed 6 months ago

mwoehlke-kitware commented 7 months ago

Describe the bug delocate-wheel picks a non-deterministic directory for bundled .dylibs.

To Reproduce This is reproducible by building the macOS wheels of Drake. I'm not sure what would be required for a simpler test case.

Expected behavior Bundled .dylibs should always be in the same directory.

Platform (please complete the following information):

Additional context This behavior is caused by wheels for which tools.find_package_dirs returns multiple entries of the form {'a', 'ba'}, where a is the package name. For example, when delocating the Drake wheel, the package directories are .../drake and .../pydrake. Because _decide_dylib_bundle_directory uses directory.endswith(package_name) to select a result, and because the order of the items in the set is non-deterministic (at least with respect to their final component), the result of _decide_dylib_bundle_directory is non-deterministic.

A correct implementation is to split the final path component from directory and compare it to package_name, e.g. directory.rsplit('/', 1)[-1] == package_name.

HexDecimal commented 7 months ago

I would prefer pathlib to .rsplit, especially since string handling methods caused this problem in the first place.

Relevant code to change: https://github.com/matthew-brett/delocate/blob/d0fba91a4118513d4f8632cf4adc8148a3b47e49/delocate/delocating.py#L532-L536

mwoehlke-kitware commented 7 months ago

There are a number of ways this could be "correctly" implemented; I just gave as an example the one that was quickest/shortest to write.