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

ci: enable arm64/aarch64 wheel builds #3430

Closed
wants to merge 6 commits into from
Closed

ci: enable arm64/aarch64 wheel builds #3430

wants to merge 6 commits into from

Conversation

nikaro
Copy link
Contributor

@nikaro nikaro commented Jan 16, 2023

I gave a try at enabling builds for these architectures but did not succeed for linux/aarch64. What seems to happen (i may be wrong) is that the ARM docker images used by multibuild cannot be run on Linux amd64 platforms (the one used by GitHub runners), even with --platform linux/arm64 flag. Cf. https://github.com/nikaro/gensim/actions/runs/3933985826

I cannot allocate more time try to fix this so i will left this PR as draft here, if someone want to continue the work on it or take inspiration from it (or close it) feel free to do it.

May be rewriting the wheels build CI from scratch using cibuildwheel could be easier, i did not test but to me it looks cleaner that the mess of bash scripts that multibuild is.

Refs #2994 #3425 #3341

Move lint steps into its own workflow that can be called from other
workflows (`on.workflow_dispatch`).
@nikaro nikaro changed the title ci: enable arm64/aarch64 wheels build ci: enable arm64/aarch64 wheel builds Jan 16, 2023
@piskvorky piskvorky added this to the Next release milestone Jan 17, 2023
@piskvorky
Copy link
Owner

Thanks @nikaro , build-improving / maintenance tickets are always appreciated.

I'll leave this PR open in case you or someone else want to revisit it in the future.

@nikaro
Copy link
Contributor Author

nikaro commented Jan 17, 2023

@piskvorky i finally gave a try to cibuildwheel and have something that seems to work, while being simpler. Can you take a look?

Edit: ubuntu-20.04 ended up being cancelled because it took way too much time, that's because builds and tests are run in qemu. May be we should run tests for wheels only on Python 3.11 and/or native architecture (not ARM)?

Cf. https://github.com/nikaro/gensim/actions/workflows/build-wheels.yml

@nikaro nikaro marked this pull request as ready for review January 17, 2023 16:28
@nikaro
Copy link
Contributor Author

nikaro commented Jan 23, 2023

Is there anything missing? I would like to be able to use this lib on ARM platforms with Python 3.11.

@piskvorky
Copy link
Owner

piskvorky commented Jan 23, 2023

@niharo thanks! What's needed now is a review by @mpenkov .

In the meantime, you can use a "privately built" wheel, right? Or is the merge / release somehow blocking for you?

@nikaro
Copy link
Contributor Author

nikaro commented Jan 23, 2023

In the meantime, you can use a "privately build" wheel, right? Or is the merge / release somehow blocking for you?

Actually i'm not myself the "end-user" of it. It is used by some devs at my job, so it is in the dependencies of our app and thus shared by everyone. We are in the process of evaluating the upgrade to Python 3.11, and this lib is the last one preventing us make the jump 😅 But there is nothing urgent so i will wait for the merge and next release.

Thank you for your answer.

@piskvorky
Copy link
Owner

No problem. I appreciate the PR and consider chipping in at https://github.com/sponsors/piskvorky if you can (or your company). We're just a handful of volunteers with busy non-open-source jobs ourselves.

@nikaro nikaro closed this by deleting the head repository Feb 26, 2023
@piskvorky
Copy link
Owner

@mpenkov what's the status here? @nikaro why did you close the PR?

@nikaro
Copy link
Contributor Author

nikaro commented Feb 26, 2023

Oh sorry, i think it closed automatically since i removed the fork from my repositories.

@nikaro nikaro reopened this Feb 26, 2023
@mpenkov
Copy link
Collaborator

mpenkov commented Feb 27, 2023

OK. Please give me some time to review this. This pretty much entirely changes the way we build wheels, so we need to make sure it works.

BTW, did you have a chance to test out the wheels built by your proposed changes?

@nikaro
Copy link
Contributor Author

nikaro commented Feb 27, 2023

BTW, did you have a chance to test out the wheels built by your proposed changes?

I've been able to test the linux x86_64 wheel: pip install wheelhouse/gensim-*.whl and import gensim work fine (build with cibuildwheel --only cp310-manylinux_x86_64 command).

@piskvorky
Copy link
Owner

piskvorky commented Mar 2, 2023

@nikaro is there a step to automatically test each wheel after building it? If not, can you add it?

The same way we test them now, in the current process. Thanks!

@nikaro
Copy link
Contributor Author

nikaro commented Mar 2, 2023

@piskvorky this is already done, cf. https://github.com/RaRe-Technologies/gensim/actions/runs/4276229889/jobs/7444479965#step:4:467, excluding the ones listed in CIBW_TEST_SKIP env var.

@piskvorky
Copy link
Owner

@mpenkov over to you :)

@mpenkov
Copy link
Collaborator

mpenkov commented Mar 3, 2023

@nikaro Where is the source code for this PR? Github is telling me that https://github.com/nikaro/gensim/tree/ci-qemu-cibuildwheel is missing.

@nikaro
Copy link
Contributor Author

nikaro commented Mar 3, 2023

@mpenkov i removed my fork, that's why it was missing. I reinstated it you can check it out again now.

@mpenkov
Copy link
Collaborator

mpenkov commented Mar 4, 2023

@nikaro Thanks! Can you please adjust this PR to allow maintainer commits? I need to fix a few things, e.g. the CIBW_TEST_SKIP variable, but I need your permission to do so.

@nikaro
Copy link
Contributor Author

nikaro commented Mar 4, 2023

@mpenkov i will close this PR and recreate a new one. The removal of my fork seems to have messed up things a bit.

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

Successfully merging this pull request may close these issues.

None yet

3 participants