search

vpelletier / python-libusb1

Python ctype-based wrapper around libusb1
GNU Lesser General Public License v2.1
168 stars 65 forks source link

Add a script for building Windows binary wheels #61

Closed whitequark closed 3 years ago

whitequark commented 3 years ago

The wheels contain the official libusb1 binaries, so the additional risk from shipping them is negligible.

See #33.

vpelletier commented 3 years ago

Remove obsolete code. NFC.

Pushed to master, sorry that you had to do this change. I had a similar one myself that I realize I did not push. So I rebased mine over your and pushed it to master (along with some very minor changes I had), and this seems to be making github conflict detection unhappy. But there is no conflict for the second commit.

whitequark commented 3 years ago

Let me just resolve the conflict.

whitequark commented 3 years ago

Done.

vpelletier commented 3 years ago

Quick update: I was trying to fold buildWheels.sh into setup.py to not have to remember to use a different command to release this project, and I fell into a rabbit hole. Now I'm angry at distutils.

whitequark commented 3 years ago

Yup, I have also fell into that rabbit hole, and I recommend sticking with the script.

vpelletier commented 3 years ago

Looking in the produced wheels, I'm not finding a reference to 2to3... Does this mean I have to build 6 bdist_wheels ? Maybe 5 if I replace the "all" arch with an sdist, assuming install-time code prioritizes platform-specific bdists over generic sdists, and given the rabbit hole I'm not sure if this is a safe assumption.

whitequark commented 3 years ago

Looking in the produced wheels, I'm not finding a reference to 2to3... Does this mean I have to build 6 bdist_wheels ?

Wheels do not provide a capability to run arbitrary code during installation (very much on purpose), so yes, you need separate wheels for Python 2 and 3.

Wouldn't it be four, btw? You have to upload the sdist regardless. And yes, to be clear, the "all" arch does not provide much value. I think you can safely replace it with an sdist.

assuming install-time code prioritizes platform-specific bdists over generic sdists, and given the rabbit hole I'm not sure if this is a safe assumption.

No, it is in fact a safe assumption, it is documented behavior of pip, and many other projects rely on it as well. (Wasmtime for example.)

vpelletier commented 3 years ago

Wouldn't it be four, btw?

I was mixing wheels and sdist counts: {py2,py3},{w32,w64} (4) + either sdist (1) or {py2,py3},all (2), hence "5 or 6".

I pushed a proposal over your commit on a new windows-wheels on this repository. Could you give it a try and tell me if this works (...and makes sense) for you ?

No, it is in fact a safe assumption

\o/

whitequark commented 3 years ago

I'm thoroughly impressed with your patience with Python 2.7 and distutils. I would have given up 1/3 of the way into that massive amount of support code. And yes, it all looks reasonable to me. Trying now.

whitequark commented 3 years ago

I've confirmed that both of the Py3 wheels work with a real application/device.

vpelletier commented 3 years ago

And this is now merged in master. I'm working on releasing it, it should become version 1.8.1 . Thanks a lot for your contributions !

whitequark commented 3 years ago

Thank you for your work. It''s always a pleasure :)

whitequark commented 3 years ago

@vpelletier Do you think you could publish arch-independent wheels too? Without them, pip install libusb1 fails on a completely fresh Python installation because there's no wheel package. There are other benefits too, but this is the main one.

vpelletier commented 3 years ago

Oh, I thought it would fall back to the sdist, as it does when no wheel exist :( .

So I need to release 2 more wheels, right, one per python version ? Something like:

for PYTHON in python2 python3; do
  "${PYTHON}" setup.py --quiet bdist_wheel --plat-name any
done
vpelletier commented 3 years ago

...or is it actually falling back to the sdist, but then setup.py fails because of the declared setup_requires on wheel ?

That was something which was not in your proposed patch, and which I added just because I discovered it existed and seemed to make sense. But maybe it was not just unnecessary but also counter-productive...

whitequark commented 3 years ago

I think it would be best to just release wheels. There are other reasons to do that. In any case, I think that even if you remove wheel from setup_requires it'll still fail, I never got to the bottom of it, but I think wheel is an undeclared dependency of sdists when they are built by pip because pip always calls build_wheel or something like that.

vpelletier commented 3 years ago

Done, I uploaded py2-any and py3-any wheels on the same version and pushed the change to my setup.sh for future releases.

Thanks for the report !

whitequark commented 3 years ago

Thanks for the quick fix!

vpelletier commented 3 years ago

Actually, I've been too quick, and forgot I had already committed 2 source changes... so these wheels do not actually contain the same code as other 1.8.1 wheels. So I re-released all as 1.9 and will delete these inconsistent 1.8.1 wheels.

vpelletier commented 3 years ago

Aaand it's broken already: libusb 1.0.24 changed the windows release archive to also include libusb versions built by various compilers:

$ 7z l build/download-cache/libusb-1.0.24.7z | grep '\.dll$'
2020-12-11 08:00:00 ....A       262758               MinGW32/dll/libusb-1.0.dll
2020-12-11 08:00:00 ....A       284336               MinGW64/dll/libusb-1.0.dll
2020-12-11 08:00:00 ....A       117760               VS2013/MS32/dll/libusb-1.0.dll
2020-12-11 08:00:00 ....A       149504               VS2013/MS64/dll/libusb-1.0.dll
2020-12-11 08:00:00 ....A       126464               VS2015/MS32/dll/libusb-1.0.dll
2020-12-11 08:00:00 ....A       154624               VS2015/MS64/dll/libusb-1.0.dll
2020-12-11 08:00:00 ....A       132096               VS2017/MS32/dll/libusb-1.0.dll
2020-12-11 08:00:00 ....A       162304               VS2017/MS64/dll/libusb-1.0.dll
2020-12-11 08:00:00 ....A       135680               VS2019/MS32/dll/libusb-1.0.dll
2020-12-11 08:00:00 ....A       166912               VS2019/MS64/dll/libusb-1.0.dll

Having no idea about which one I should pick (does it depend on what VS version python is built against ? is it just for projects using one these compilers to have everything they need for their specific version ? is picking the oldest one a safe choice ?), I am turning to you for advice.

There is good news too: this release is signed, so I now not only sign my own releases, but I also check libusb's signature (in addition to the existing "fail if version changed so I notice" sha check).

whitequark commented 3 years ago

@vpelletier Based on my investigation, you can safely pick any of the VS DLLs; I am not sure why there are multiple in the first place. (You might want to ask on the libusb mailing list, which is the only way the libusb project accepts questions--I personally do not have time to deal with mailing lists these days.)

None of the Windows-specific code has #ifdefs that change behavior based on compiler or SDK versions that I can see, and libusb does not directly link to any libraries that expose different sets of functions depending on OS version (this is done at runtime), so I believe all of the DLLs would work for python-libusb1. Whether you pick the oldest or the newest is up to you; the oldest is a few KB smaller, the newest supports ARM64 (although it does not seem like that is packaged in release assets yet). I would probably pick the newest but I have no strong preference.

And, since you do not exchange CRT objects like file descriptors with libusb, there are no constraints between the Python VS version and the libusb VS version. I'm pretty sure the MinGW DLL would work fine with VS Python here.