Skip to content
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

Precompiled wheels for all operating systems #187

Closed
andreped opened this issue Dec 9, 2022 · 7 comments
Closed

Precompiled wheels for all operating systems #187

andreped opened this issue Dec 9, 2022 · 7 comments

Comments

@andreped
Copy link

andreped commented Dec 9, 2022

Context

Issue type (bug report or feature request):
There are various situations where building the package might fail. For instance on Windows, if VS Build Tools are not available. I see that this has been handled by building precompiled wheels for Windows specifically (see here). However, depending on the end-user's settings, it may happen that the user is unable to build themselves, even on Ubuntu, during installation.

I therefore, believe it would be more optimal if we build wheels for all relevant operating systems and python versions, which should remove this issue.

That is quite easy to do as we only need to modify the workflow to do that:
https://github.com/openslide/openslide-python/blob/main/.github/workflows/python.yml

Details

This was recently a problem when trying to install MONAI Label on a client machine which did not have install python dev tools which resulted in python setup.py install failing for openslide-python. This will no longer be a problem if precompiled wheels are provided. I believe the MONAI Label team would be very interested in this (@diazandr3s).

If this is of interest, shall I make a PR to add support for various relevant operating systems and specific python versions?

@bgilbert
Copy link
Member

bgilbert commented Dec 9, 2022

xref #126 and #144. If there's a relatively clean way to do this within GitHub Actions, a PR would be great. Thanks!

@andreped
Copy link
Author

andreped commented Dec 9, 2022

xref #126 and #144. If there's a relatively clean way to do this within GitHub Actions, a PR would be great. Thanks!

@bgilbert I can make an attempt later today and make a PR.

@andreped
Copy link
Author

@bgilbert Made a PR which resolves this issue: #188

@andreped
Copy link
Author

andreped commented Dec 11, 2022

Oh, I just went through the proposed edits you mentioned here, which looks very similar to mine. What exactly were the issues? Seemed to work fine in my case. At least when testing the precompiled wheel on my external macOS. I guess you are talking about backward-compatibility on older linux-versions, or?


EDIT: Should've read the discussion more carefully here. What exactly is the reason why precompiled wheels (with the current solution) might be a bad idea?

@bgilbert
Copy link
Member

Yup, the wheels built by that PR failed to load on older Linux versions, which seemed to defeat the point. As I understand it, we'd need to build in a special container, but that should be doable.

There are two separate questions: whether to build OpenSlide Python wheels for Linux and macOS, and whether to bundle OpenSlide (the actual C library) into those wheels. I'm in favor of the former, but the latter is complex (OpenSlide has a lot of dependencies) and I'd consider it out of scope.

@andreped
Copy link
Author

andreped commented Dec 11, 2022

Yup, the wheels built by that PR failed to load on older Linux versions

What we often do is to build on the oldest relevant Linux version, as python packages tend to be forward compatible on Ubuntu. However, it is true that there is no guarantee. Manylinux solution might be the best fit.

However, I see that there exists an Action that can be used for exactly this purpose:
https://github.com/marketplace/actions/python-wheels-manylinux-build

If I understand correctly, building inside this container, it should work as intended, right? Shall i make an attempt? I can make commits to the same branch as in the PR.

@bgilbert
Copy link
Member

Yes, please do. I'd prefer to use explicit build steps rather than having the Action do it for us, but if that turns out to be too complex, the Action could be a reasonable alternative.

bgilbert added a commit to bgilbert/openslide-python that referenced this issue Nov 1, 2023
The convert module doesn't use any libc APIs, so we can build on any Linux
distro and auditwheel will tag the wheel as the oldest manylinux.

auditwheel requires setuptools on Python 3.12 for distutils.

Fixes: openslide#126
Fixes: openslide#187
Signed-off-by: Benjamin Gilbert <bgilbert@cs.cmu.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants