Distribute PySoundFile as wheels #116

bastibe · 2015-03-11T10:23:41Z

This is a prototype of a project structure that allows for wheel distribution.

Run python setup.py bdist_wheel --universal to create the wheel, then pip install {filename}.whl to install it. I don't know how this interacts with PyPi yet.

This should also fix most of the issues in #47, and remove the copying of files. It doesn't solve the need to ship binaries with the repository yet.

I don't particularly like the change to soundfile.py, but I haven't found a better solution yet.

bastibe · 2015-03-12T08:45:34Z

If pysoundfile is not installed as a wheel, and libsndfile is not present on the system, this will fail in the exception handler where it tries to dlopen the wheel library, which is kind of unfortunate. I don't see how to improve this while still trying for system libraries first though.

In an alternative implementation, we could change pysoundfile into a package, and use package_data instead of data_files. As far as I can tell this won't improve things much, though.

mgeier · 2015-03-12T13:15:49Z

setup.py


 if platform == 'win32' and architecture()[0] == '32bit':
-    shutil.copy2('win/sndfile32.dll', 'win/sndfile.dll')
-    sndfile = [('', ['win/sndfile.dll', 'win/sndfile_license'])]
+    sndfile = [('lib', ['win32/libsndfile.dll', 'win32/sndfile_license'])]


why not keep the DLL install directory as before (i.e. '')? If you don't use a subdirectory, you don't need to do the path manipulation abomination in dlopen().

'' is the Python root, not the PySoundFile root. It would be the only binary in that path. At least in lib, it is among other libraries.

See https://github.com/bastibe/PySoundFile/pull/116#issuecomment-78554177

mgeier · 2015-03-12T13:30:04Z

That looks promising, but I have a few comments:

why not keep the DLL install directory as before? Then you don't need the ugly changes to soundfile.py (see https://github.com/bastibe/PySoundFile/pull/116/files#r26299565).
the Windows wheel doesn't seem to be "universal", it contains cp33-none-win_amd64. Shouldn't this be py2.py3-none-win_amd64?

If pysoundfile is not installed as a wheel, and libsndfile is not present on the system, this will fail in the exception handler where it tries to dlopen the wheel library, which is kind of unfortunate. I don't see how to improve this while still trying for system libraries first though.

You could catch the second exception, set a flag and ignore the exception.
Then, back in the exception handler for the first exception you could check the flag and raise.
This should re-raise the first exception (but it would be still quite ugly).

In an alternative implementation, we could change pysoundfile into a package, and use package_data instead of data_files. As far as I can tell this won't improve things much, though.

Would this actually change anything?

bastibe · 2015-03-12T16:14:17Z

why not keep the DLL install directory as before?

Because the Windows installer puts it in a different place than the wheel. The Windows installer puts the library in the PySoundFile root, whereas the wheel puts it into the environment root. The environment root is not a good place for the library.

the Windows wheel doesn't seem to be "universal",

True. Just don't use the --universal flag then.

mgeier · 2015-03-12T18:18:40Z

Because the Windows installer puts it in a different place than the wheel.

When I tried it, it was put into the exact same directory (Windows 7, Pyzo 2013c).
There isn't really a PySoundFile root, the file soundfile.py is just thrown into the lib\site-packages directory, among many other files, the DLL ends up one directory higher, in lib or another directory higher, in the Python root.
This (the latter) happens for both the .exe installer and the wheel.

The environment root is not a good place for the library.

I wouldn't really care. It's Windows, after all. Nothing in Windows is in a good place.
I'd say if it makes our code simple, it's fair game.

The real problem, though, is if the option --user is given with pip, the DLL ends up somewhere where it isn't found (neither with nor without lib).

I guess with the .exe installer a similar option wasn't even possible, right?

the Windows wheel doesn't seem to be "universal",

True. Just don't use the --universal flag then.

Does that mean we have to provide many binary wheels?
Separate for all combinations of CPython and PyPy and Python 2.6, 2.7, 3.3, 3.4 and 32bit and 64bit?

bastibe · 2015-03-13T07:52:37Z

Does that mean we have to provide many binary wheels?

We'll have to provide a Windows64, Windows32, and Mac wheel. I think I got confused with the --universal flag. Both Windows and Mac should be --universal.

The real problem, though, is the option --user

This can actually be solved by converting PySoundFile to a package, since packages can ship package_data in their own directory.

bastibe · 2015-03-13T08:13:36Z

Here's a different idea: I moved all binaries into their own package. This installs them alongside sndfile.py. The downside is that they now have a platform-dependent name, since package_data does not change their relative path as data_files does.

bastibe · 2015-03-13T08:35:23Z

This idea leads to a pretty nice side benefit: We can move the cdef from soundfile.py into the pysoundfile_binaries package. I actually like this!

dlopen is still very specific about what kinds of paths it can and cannot open. Apparently, it can't open relative paths? So the path manipulation abominations are back. But at least they are not in soundfile.py.

mgeier · 2015-03-15T17:22:58Z

pysoundfile_binaries/__init__.py

+    elif platform == 'darwin':
+        snd = ffi.dlopen(join(dirname(__file__), 'darwin/libsndfile'))
+    else:
+        raise err


You should just use raise.
Otherwise, you're losing the original traceback information.

True. Thank you.

mgeier · 2015-03-18T16:00:40Z

I wasn't aware that setup() supports both packages and py_modules at the same time.
That's great, because this indeed solves the --user problem!

I like the idea with the separate pysoundfile_binaries package, but I would wait with moving the CFFI stuff out of soundfile.py.
This is clearly a separate issue and should be discussed separately (and in the current state of things, I'm against it).
Also, it's very dubious to call it something with "binaries" and then put Python code in it, but that's part of a future discussion ...

I still don't know how a wheel can be created that works for all Python interpreters on Win64.

mgeier · 2015-03-18T16:06:43Z

pysoundfile_binaries/__init__.py

+    from os.path import join, dirname
+
+    if platform == 'win32' and architecture()[0] == '32bit':
+        snd = ffi.dlopen(join(dirname(__file__), 'win32/libsndfile'))


You should rename the directories to win32_32bit, win32_64bit and darwin_64bit, respectively.
Then you can avoid most of the repetition:

if platform in ('darwin', 'win32'): ... platform + '_' + architecture()[0] ... else: raise

mgeier · 2015-03-18T16:42:58Z

dlopen is still very specific about what kinds of paths it can and cannot open. Apparently, it can't open relative paths?

I think it can, but the base directory must still be in the system path (?), which it probably normally isn't.
Actually, the magic name __file__ generally evaluates to a relative path.

When I tried it on Windows, it didn't work because of this, so you should add os.path.abspath().

I suggest something like this

    if platform in ('darwin', 'win32'):
        _snd = _ffi.dlopen(_os.path.join(
            _os.path.dirname(_os.path.abspath(__file__)),
            'pysoundfile_binaries',
            _platform + '_' + _architecture()[0],
            'libsndfile'))
    else:
        raise

So the path manipulation abominations are back.

Indeed!

But for now, I don't see a better way ...

mgeier · 2015-03-19T13:19:12Z

After some experimenting and some web browsing, I think I'm starting to understand what a "pure" wheel is: there is no such thing!

There is only the setting Root-Is-Purelib in *.dist-info/WHEEL, which means, according to PEP 427:

Root-Is-Purelib is true if the top level directory of the archive should be installed into purelib; otherwise the root should be installed into platlib.

The funny thing is, if we extend Distribution and make is_pure() return False, all files are moved from the root directory to purelib/!
All files including the DLLs!
So we might as well just make it "pure", as it will have exactly the same effect!

I think we should just create a presumably "pure" wheel for each supported platform (win32, win64, mac64) which will initially have a generic name (e.g. PySoundFile-0.7.0-py3-none-any.whl). Then, we should simply rename it like a "non-pure" wheel (e.g. in PySoundFile-0.7.0-cp26.cp27.cp33.cp34-none-win_amd64.whl).
We might have to add a few more tags to support PyPy (probably py2.py3?).

Note that this, contrary to what I expected, doesn't work with pip: PySoundFile-0.7.0-py2.py3-none-win_amd64.whl ("... is not a supported wheel on this platform").

For OSX, we'll have to add a whole lot of different tags to the file name.
See those links for a (probably not complete) collection of tags:
https://pypi.python.org/pypi/cffi
https://pypi.python.org/pypi/scipy
https://pypi.python.org/pypi/pandas
https://pypi.python.org/pypi/coverage/4.0a5

Once we upload our 3 wheels (plus probably an actually "pure" wheel without any binaries that can be used on Linux: PySoundFile-0.7.0-py2.py3-none-any.whl), I'd hope that pip install PySoundFile gets the right wheel for each platform. I didn't try that, though.

I think we should use something like this in setup.py:

if platform in ('darwin', 'win32'):
    subdir = platform + '_' + architecture()[0]
    extension = '.dylib' if platform == 'darwin' else '.dll'
    packages = ['pysoundfile_binaries']
    package_data = {'pysoundfile_binaries': [
        os.path.join(subdir, 'libsndfile' + extension),
        os.path.join(subdir, 'libsndfile_license')
    ]}
else:
    packages = None
    package_data = None

...

setup(
    ...
    packages=packages,
    package_data=package_data,
    ...
)

This should also put the license into the right subdirectory (and make the superfluous copy of it obsolete).

mgeier · 2015-03-19T13:21:54Z

Hmmm, there is still the question how we get the renamed wheels uploaded to PyPI ...

mgeier · 2015-03-19T13:34:58Z

OK, a few more findings:

With pip >= 6.0 the OSX tags probably become a bit less of a mess:
pypa/pip#1465
https://mail.python.org/pipermail/distutils-sig/2014-March/023977.html

We can probably use "twine" to upload the renamed wheels: https://pypi.python.org/pypi/twine

All this is from the very interesting page https://github.com/MacPython/wiki/wiki/Spinning-wheels.

bastibe · 2015-03-26T10:07:30Z

I'll look into this this weekend. But it seems that "pure" really means "not a CPython extension", which would be applicable to PySoundFile.

mgeier · 2015-03-26T20:18:04Z

What was the reason for the 32/64-bit subdirectories?
Couldn't we just use differently named files?

Then we could use this in setup.py:

if platform in ('darwin', 'win32'):
    extension = '.dylib' if platform == 'darwin' else '.dll'
    packages = ['pysoundfile_data']
    package_data = {'pysoundfile_data': [
        'libsndfile_' + architecture()[0] + extension,
        'libsndfile_license'
    ]}
else:
    packages = None
    package_data = None

And in soundfile.py:

try:
    _snd = _ffi.dlopen('sndfile')
except OSError as err:
    if _sys.platform in ('darwin', 'win32'):
        from platform import architecture as _architecture
        _snd = _ffi.dlopen(_os.path.join(
            _os.path.dirname(_os.path.abspath(__file__)),
            'pysoundfile_data', 'libsndfile_' + _architecture()[0]))
    else:
        raise

mgeier · 2015-03-26T21:28:52Z

To allow cross-platform wheel builds, we could add something like this to setup.py:

platform = os.environ.get('PYSOUNDFILE_PLATFORM', platform)
architecture0 = os.environ.get('PYSOUNDFILE_ARCHITECTURE', architecture()[0])

To be used like this:

$ PYSOUNDFILE_PLATFORM=win32 PYSOUNDFILE_ARCHITECTURE=64bit python setup.py bdist_wheel

bastibe · 2015-04-01T09:29:56Z

So far, I was unable to find a way for including a full set of libraries in the wheel. The homebrew-provided libsndfile depends on system libraries, but copying those system libraries into pysoundfile_binaries doesn't help. I have not found a way of producing a statically-linked version of libsndfile, either (there doesn't seem to be a switch in configure or makefile to allow this).

I could build a version of libsndfile that does not link to flac, ogg, or vorbis.

Also, delocate can't fix the problem, since it only patches python extensions, but not the included libsndfile library.

bastibe · 2015-04-01T09:41:23Z

The current solution should work for Windows, though.

Except it doesn't, because python setup.py bdist_wheel throws an error deep inside the bowels of setuptools. This is becoming rather frustrating.

mgeier · 2015-04-03T18:57:49Z

setup.py

+if platform in ('darwin', 'win32'):
+    packages = ['pysoundfile_binaries']
+    package_data = {'pysoundfile_binaries':
+                    ['pysoundfile_binaries/libsndfile_license']}


Is the repetition of pysoundfile_binaries intentional?
For me it doesn't work like this.

mgeier · 2015-04-03T19:06:32Z

python setup.py bdist_wheel throws an error deep inside the bowels of setuptools.

Could you please share the error message?
There are two minor issues (https://github.com/bastibe/PySoundFile/pull/116/files#r27748229 and https://github.com/bastibe/PySoundFile/pull/116/files#r27748314), does it work once they are fixed?

BTW, you don't need to use Windows at all (see https://github.com/bastibe/PySoundFile/pull/116#issuecomment-86726184).

Can now be built as wheel on Windows and OSX

c22fe37

bastibe added this to the 0.7.0 milestone Mar 11, 2015

bastibe self-assigned this Mar 11, 2015

improved library path handling

7d627d5

mgeier reviewed Mar 12, 2015
View reviewed changes

mgeier reviewed Mar 15, 2015
View reviewed changes

mgeier reviewed Mar 18, 2015
View reviewed changes

bastibe force-pushed the wheels branch 2 times, most recently from 8f3869c to d08139e Compare April 1, 2015 08:48

refactored path mangling a bit

d08139e

mgeier reviewed Apr 3, 2015
View reviewed changes

mgeier mentioned this pull request Apr 9, 2015

Move DLLs to submodule #125

Merged

bastibe closed this Apr 10, 2015

bastibe deleted the wheels branch April 25, 2016 08:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Distribute PySoundFile as wheels #116

Distribute PySoundFile as wheels #116

bastibe commented Mar 11, 2015

bastibe commented Mar 12, 2015

mgeier Mar 12, 2015

bastibe Mar 12, 2015

mgeier Mar 12, 2015

mgeier commented Mar 12, 2015

bastibe commented Mar 12, 2015

mgeier commented Mar 12, 2015

bastibe commented Mar 13, 2015

bastibe commented Mar 13, 2015

bastibe commented Mar 13, 2015

mgeier Mar 15, 2015

bastibe Mar 17, 2015

mgeier commented Mar 18, 2015

mgeier Mar 18, 2015

mgeier commented Mar 18, 2015

mgeier commented Mar 19, 2015

mgeier commented Mar 19, 2015

mgeier commented Mar 19, 2015

bastibe commented Mar 26, 2015

mgeier commented Mar 26, 2015

mgeier commented Mar 26, 2015

bastibe commented Apr 1, 2015

bastibe commented Apr 1, 2015

mgeier Apr 3, 2015

mgeier commented Apr 3, 2015

Distribute PySoundFile as wheels #116

Distribute PySoundFile as wheels #116

Conversation

bastibe commented Mar 11, 2015

bastibe commented Mar 12, 2015

mgeier Mar 12, 2015

Choose a reason for hiding this comment

bastibe Mar 12, 2015

Choose a reason for hiding this comment

mgeier Mar 12, 2015

Choose a reason for hiding this comment

mgeier commented Mar 12, 2015

bastibe commented Mar 12, 2015

mgeier commented Mar 12, 2015

bastibe commented Mar 13, 2015

bastibe commented Mar 13, 2015

bastibe commented Mar 13, 2015

mgeier Mar 15, 2015

Choose a reason for hiding this comment

bastibe Mar 17, 2015

Choose a reason for hiding this comment

mgeier commented Mar 18, 2015

mgeier Mar 18, 2015

Choose a reason for hiding this comment

mgeier commented Mar 18, 2015

mgeier commented Mar 19, 2015

mgeier commented Mar 19, 2015

mgeier commented Mar 19, 2015

bastibe commented Mar 26, 2015

mgeier commented Mar 26, 2015

mgeier commented Mar 26, 2015

bastibe commented Apr 1, 2015

bastibe commented Apr 1, 2015

mgeier Apr 3, 2015

Choose a reason for hiding this comment

mgeier commented Apr 3, 2015