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

Add maturin workflow to generate wheels #747

Merged
merged 7 commits into from
Oct 25, 2024
Merged

Add maturin workflow to generate wheels #747

merged 7 commits into from
Oct 25, 2024

Conversation

ia0
Copy link
Member

@ia0 ia0 commented Oct 10, 2024

No description provided.

@ia0 ia0 force-pushed the maturin branch 8 times, most recently from f8d2712 to 299f430 Compare October 10, 2024 11:33
@ia0
Copy link
Member Author

ia0 commented Oct 10, 2024

@reyammer So the conclusion is that ort needs GLIBC >= 2.31 for pre-built binaries while the latest manylinux is 2_28. This means that we would need to compile ONNX Runtime from source and link it when building Magika, see pykeio/ort#268 and https://ort.pyke.io/setup/platforms and https://ort.pyke.io/setup/linking. (That's assuming compiling ONNX Runtime from source with GLIBC 2.28 will work.)

@ia0 ia0 closed this Oct 10, 2024
@ia0 ia0 deleted the maturin branch October 10, 2024 11:44
@ia0 ia0 restored the maturin branch October 18, 2024 10:54
@ia0
Copy link
Member Author

ia0 commented Oct 18, 2024

Reopening this to test compiling ONNX Runtime from source.

@ia0 ia0 reopened this Oct 18, 2024
@ia0 ia0 force-pushed the maturin branch 15 times, most recently from ffbaf3c to c6294c4 Compare October 18, 2024 16:12
@ia0 ia0 force-pushed the maturin branch 7 times, most recently from 9c46b72 to a2df5cc Compare October 24, 2024 12:28
@ia0 ia0 changed the title DO NOT MERGE: Testing maturin Add maturin workflow to generate wheels Oct 24, 2024
.github/workflows/maturin.yml Outdated Show resolved Hide resolved
@ia0 ia0 marked this pull request as ready for review October 24, 2024 12:49
@ia0
Copy link
Member Author

ia0 commented Oct 24, 2024

Here are the wheels for examination.

.github/workflows/maturin.yml Show resolved Hide resolved
.github/workflows/maturin.yml Show resolved Hide resolved
.github/workflows/maturin.yml Outdated Show resolved Hide resolved
.github/workflows/maturin.yml Show resolved Hide resolved
@ia0
Copy link
Member Author

ia0 commented Oct 25, 2024

Here are the wheels. I checked the linux one and the versions (in wheel name, Rust CLI, and Python version) are correct. Not sure what else to check. The workflow anyway checks even more.

@reyammer
Copy link
Collaborator

About the wheels: yesterday I checked the linux one (but on a modern box) and the mac ones, and they worked! will add tests for the old ubuntu in a different PR. Other than this, this looks good!

@ia0
Copy link
Member Author

ia0 commented Oct 25, 2024

About the wheels: yesterday I checked the linux one (but on a modern box) and the mac ones, and they worked! will add tests for the old ubuntu in a different PR. Other than this, this looks good!

The only missing test is the wheel test in the container (thus for Linux). We already test everything else (including the CLI in the container). If you believe the wheel and python code could behave differently in the container, then it's worth looking into, otherwise I believe this PR should be sufficient.

@ia0 ia0 merged commit b040aec into google:main Oct 25, 2024
35 checks passed
@ia0 ia0 deleted the maturin branch October 25, 2024 12:18
@ia0
Copy link
Member Author

ia0 commented Oct 26, 2024

I checked the linux one and the versions (in wheel name, Rust CLI, and Python version) are correct.

I should also have checked sdist because it's a different flow. Sending #769 to fix the wheel version.

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