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

Fix Linux Python 3.12 wheel build #102

Merged
merged 2 commits into from
Oct 5, 2023
Merged

Conversation

alugowski
Copy link
Collaborator

The Python 3.12 on Linux wheel build process appears to need an additional libffi install. The cibuildwheel docs just happen to use yum install -y libffi-devel as an example, and that works well to solve the issue.

This makes Python 3.12 wheels work on all three platforms supported by cibuildwheel (macOS and Windows worked already).

See example run: https://github.com/alugowski/python-suitesparse-graphblas/actions/runs/6388570568/job/17338631246
(The PyPy 3.9 fails are fixed by #100)

@eriknw
Copy link
Member

eriknw commented Oct 4, 2023

OMFG, thanks so much @alugowski!!!

I'm sorry I just now saw these PRs. They look good to me... should we go ahead and merge all three?

Also, any thoughts on my recent PR #99? Can we undo this?

I was just taking a stab in the dark due to failures here:
https://github.com/GraphBLAS/python-suitesparse-graphblas/actions/runs/6385412971

@alugowski
Copy link
Collaborator Author

Yes, I think all three are worthwhile.

I think what happened is that cibuildwheel has added some new platforms since we last went through the wheel build process, and they broke the process a little. Hence this PR; just last month they turned on Python 3.12 builds.

#100 should resolve the issue you're seeing in that Actions run. I don't think #99 broke anything because it's the same error messages I saw that prompted me to make that PR. Basically oldest-supported-numpy would fetch a numpy version that does not work with PyPy. I don't know what the best solution is, but that PR is what SciPy does so it can't be too bad. The main downside of that PR is to remember to re-enable PyPy 3.10 wheels when numpy starts supporting it in future versions. I don't know of a clean way to tell cibuildwheel to just match whatever numpy supports.

#101 is because I noticed some false workflow green checkmarks when checking into the PyPy issues, but only because the tests simply weren't run. It seems the GraphBLAS issue is fixed now and the tests pass.

@alugowski
Copy link
Collaborator Author

If you'd like I can make a PR to use the cibuildwheel Action instead of the pip install in the current system. That way the cibuildwheel version can be managed by dependabot. It can get noisy since they release a version about once a month, but the changelog will tell you what versions they add/drop so you won't get surprised by such changes. You could run the wheel workflow for those PRs to ensure that the update doesn't break anything.

@alugowski
Copy link
Collaborator Author

To answer your question, I think merging #100 and #102 will make wheels build. I think it would be best to try reverting #99 (i.e. unpin cffi) before distributing wheels to PyPI.

@eriknw
Copy link
Member

eriknw commented Oct 5, 2023

I'm happy to defer to your judgement here @alugowski, so I'll go ahead and merge all three. I do closely read the changes and your comments, b/c I want to get better at this stuff.

If you'd like I can make a PR to use the cibuildwheel Action instead of the pip install in the current system. That way the cibuildwheel version can be managed by dependabot. It can get noisy since they release a version about once a month, but the changelog will tell you what versions they add/drop so you won't get surprised by such changes.

I think we can live with this noise, and testing regularly is probably a good thing. Your call.

You could run the wheel workflow for those PRs to ensure that the update doesn't break anything.

Good idea, I need to get in the habbit of running the wheel workflows before I tag a release. Actually, can we always run wheel builds for all PRs and commits to main? These should be the fast versions of wheels with no pre-compiled types.

One more thing...

Would you like to add yourself as an author and/or maintainer to this project? I really can't express enough how much I/we appreciate your contributions :)

@eriknw eriknw merged commit 35393b1 into GraphBLAS:main Oct 5, 2023
13 checks passed
@alugowski
Copy link
Collaborator Author

Good idea, I need to get in the habbit of running the wheel workflows before I tag a release. Actually, can we always run wheel builds for all PRs and commits to main? These should be the fast versions of wheels with no pre-compiled types.

That's a better idea. See #105.

One more thing...

Would you like to add yourself as an author and/or maintainer to this project? I really can't express enough how much I/we appreciate your contributions :)

Sure, it would be an honor :) What's the best way to do so?

@eriknw
Copy link
Member

eriknw commented Oct 6, 2023

Would you like to add yourself as an author and/or maintainer to this project? I really can't express enough how much I/we appreciate your contributions :)

Sure, it would be an honor :) What's the best way to do so?

In the [project] section of pyproject.toml, simply add yourself to "authors" and/or "maintainers".

If you want to be a "maintainer" in the metadata, I will add you to PyPI https://pypi.org/project/suitesparse-graphblas/ too. I should ask before adding you to the GraphBLAS github org to give you access to this repo, but I want to give you write access.

@alugowski alugowski deleted the linuxcp12 branch October 7, 2023 01:53
@alugowski
Copy link
Collaborator Author

Would you like to add yourself as an author and/or maintainer to this project? I really can't express enough how much I/we appreciate your contributions :)

Sure, it would be an honor :) What's the best way to do so?

In the [project] section of pyproject.toml, simply add yourself to "authors" and/or "maintainers".

If you want to be a "maintainer" in the metadata, I will add you to PyPI https://pypi.org/project/suitesparse-graphblas/ too. I should ask before adding you to the GraphBLAS github org to give you access to this repo, but I want to give you write access.

Sounds good! PR to add to pyproject.toml: #108
This is my PyPI account: https://pypi.org/user/alugowski/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants