10000 CI Adds arm wheel build in cirrus CI by thomasjpfan · Pull Request #25362 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

CI Adds arm wheel build in cirrus CI #25362

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 13, 2023
Merged

CI Adds arm wheel build in cirrus CI #25362

merged 3 commits into from
Jan 13, 2023

Conversation

thomasjpfan
Copy link
Member
@thomasjpfan thomasjpfan commented Jan 11, 2023 •
edited
Loading

This PR adds Linux ARM wheel building to Cirrus CI. After this PR, all the ARM wheels can be built with Cirrus CI.

The next steps as follow up PRs:

  1. Add step to open issue if any of the wheels fail.
  2. Upload wheels to nightly or staging.
  3. Update https://github.com/scikit-learn/scikit-learn/blob/main/build_tools/github/check_wheels.py to use the cirrus configuration to verify the expected number of wheels.

@ogrisel
Copy link
Member
ogrisel commented Jan 11, 2023

Thanks for moving this forward!

The next steps are

Just to clarify, do you intend to implement the next steps as part of this PR?

ogrisel
ogrisel approved these changes Jan 11, 2023
View reviewed changes
Copy link
Member
@ogrisel ogrisel left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as it is but we could probably delete the linux ARM build of circle ci as a result, no?

The cibuildwheel output looks good:

https://cirrus-ci.com/task/6644411111571456

build_tools/cirrus/arm_wheel.yml Outdated
SKLEARN_SKIP_NETWORK_TESTS=1
SKLEARN_BUILD_PARALLEL=5
CPU_COUNT=4
CIBW_TEST_COMMAND: bash {project}/build_tools/github/test_wheels.sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should have a build_tools/common folder for scripts that are used by more than one CI.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have the folder named: build_tools/wheels to place all the scripts for wheel building.

@thomasjpfan
Copy link
Member Author

Just to clarify, do you intend to implement the next steps as part of this PR?

I meant for the steps to be follow up PRs.

@thomasjpfan
Copy link
Member Author

LGTM as it is but we could probably delete the linux ARM build of circle ci as a result, no?

This PR is strictly for wheeling, so we can remove Travis CI after we hook up CirrusCI to upload the wheels to the nightly and staging indexes.

Because the Linux ARM build on CircleCI runs on every PR, we will ha 8000 ve to add one more job to CirrusCI that runs on every PR to fully replace it.

@thomasjpfan
Copy link
Member Author

I opened #25366 to migrate the Linux ARM job from CircleCI to CirrusCI.

@ogrisel ogrisel added Quick Review For PRs that are quick to review Waiting for Second Reviewer First reviewer is done, need a second one! labels Jan 12, 2023
jjerphan
jjerphan approved these changes Jan 12, 2023
View reviewed changes
Copy link
Member
@jjerphan jjerphan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you, @thomasjpfan.

@ogrisel ogrisel merged commit 5af8c6b into scikit-learn:main Jan 13, 2023
@ogrisel
Copy link
Member
ogrisel commented Jan 13, 2023

Merged. Thanks @thomasjpfan. We will be able to stop using travis for linux arm wheels once the follow-up steps are complete.

jjerphan pushed a commit to jjerphan/scikit-learn that referenced this pull request Jan 20, 2023
@thomasjpfan @jjerphan
CI Adds arm wheel build in cirrus CI (scikit-learn#25362)
691bab0
jjerphan pushed a commit to jjerphan/scikit-learn that referenced this pull request Jan 20, 2023
@thomasjpfan @jjerphan
CI Adds arm wheel build in cirrus CI (scikit-learn#25362)
046acaa
jjerphan pushed a commit to jjerphan/scikit-learn that referenced this pull request Jan 23, 2023
@thomasjpfan @jjerphan
CI Adds arm wheel build in cirrus CI (scikit-learn#25362)
35a4161
adrinjalali pushed a commit that referenced this pull request Jan 24, 2023
@thomasjpfan @adrinjalali
CI Adds arm wheel build in cirrus CI (#25362)
c6da8a0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogrisel ogrisel ogrisel approved these changes

@jjerphan jjerphan jjerphan approved these changes

Assignees
No one assigned
Labels
Build / CI Quick Review For PRs that are quick to review Waiting for Second Reviewer First reviewer is done, need a second one!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thomasjpfan @ogrisel @jjerphan
0