Skip to content

CI: cirrus for building aarch64#17029

Merged
tylerjereddy merged 10 commits intoscipy:mainfrom
andyfaff:cirrus
Oct 2, 2022
Merged

CI: cirrus for building aarch64#17029
tylerjereddy merged 10 commits intoscipy:mainfrom
andyfaff:cirrus

Conversation

@andyfaff
Copy link
Copy Markdown
Contributor

@andyfaff andyfaff commented Sep 15, 2022

@rgommers, things that need to be done:

  • the cirrus-ci app has to be setup for the scipy organisation. See https://cirrus-ci.org/guide/quick-start/. We have the ability just to install it for the scipy/scipy repo. The job won't run until the app has been installed.
  • AFAICT cirrus-ci has no access to github tokens, so I think we need to create an encrypted variable on the repository settings on cirrus-ci.com.
  • THe cron job for running nightly builds has to be setup from the repository settings. The CI script assumes that the cron job is called nightly.
  • At the moment building 4 wheels + full test suite doesn't get under the 60 min limit for a single task. I'm not sure if it's possible to get around this limit by dispersing tasks (e.g. I don't know if using a matrix of Python versions would get around this), so I'm just running the linalg test suite as before.
  • I removed the linux_aarch64 from the github actions run.

@tupui
Copy link
Copy Markdown
Member

tupui commented Sep 15, 2022

Thanks @andyfaff. Did we have a public conversation/decision about using yet another CI provider? AFAIR you just mentioned in the ML that you did some work around wheels.

@andyfaff
Copy link
Copy Markdown
Contributor Author

There was a brief discussion (if you could call it that) on Slack. There was a suggestion to look into cirrus because it offers native macosx_arm64 and linux_aarch64. On GH Actions it takes ~2 hours to build 4 linux_aarch64 wheels in parallel, so 8 hours of process time. On cirrus-ci it takes ~25 mins to build those wheels in series in a single process, so it uses 1/16 of a CI resource.
This would allow us to do more testing of the wheel.

@andyfaff andyfaff requested a review from rgommers September 16, 2022 01:16
@andyfaff
Copy link
Copy Markdown
Contributor Author

My previous comment was solely directed towards wheel building. It would also be possible to do regular native CI testing on linux_aarch64 and macosx_arm64 (possibly other archs), which we're not currently doing

@rgommers rgommers added the CI Items related to the CI tools such as CircleCI, GitHub Actions or Azure label Sep 16, 2022
@rgommers
Copy link
Copy Markdown
Member

My previous comment was solely directed towards wheel building. It would also be possible to do regular native CI testing on linux_aarch64 and macosx_arm64 (possibly other archs), which we're not currently doing

We probably should do that as well, otherwise we are going to find out at release time that we have issues. Can be done later/separately though.

@rgommers
Copy link
Copy Markdown
Member

the cirrus-ci app has to be setup for the scipy organisation. See https://cirrus-ci.org/guide/quick-start/. We have the ability just to install it for the scipy/scipy repo. The job won't run until the app has been installed.

I have done this.

@andyfaff
Copy link
Copy Markdown
Contributor Author

Thanks @rgommers. I retriggered the wheel build run and all the linux_aarch64 builds worked (including test suite) on cirrus-ci, https://cirrus-ci.com/build/4966426158039040.

The only job remaining for this PR is for you to create staging and nightly build tokens on anaconda, you suggested specific tokens for cirrus-ci jobs. Once you've created those tokens you go to https://cirrus-ci.com/settings/repository/4894635377033216 (this is the settings page for the scipy/scipy repository on cirrus-ci), and go to the encrypted variables entry.
You enter the value in the box and press encrypt. This will make an encrypted string something along the lines of ENCRYPTED[qwerty239abc]. We then place those two encrypted variables into the .cirrus.yml file:

  env:
    SCIPY_STAGING_UPLOAD_TOKEN: ENCRYPTED[qwerty239abc]
    SCIPY_NIGHTLY_UPLOAD_TOKEN: ENCRYPTED[qwerty239abc2]

See https://cirrus-ci.org/guide/writing-tasks/#encrypted-variables

@andyfaff
Copy link
Copy Markdown
Contributor Author

I'll leave macosx_arm64 inclusion into cirrus-ci for another PR.

@rgommers
Copy link
Copy Markdown
Member

@andyfaff here are the tokens:

  • SCIPY_STAGING_UPLOAD_TOKEN_CIRRUS: ENCRYPTED[92da39a48bc14c0d14f05c057b011e321915337a8e0a664da56be892e3d19c5c991d12bc7ef79470693214620d3e3b1a]
  • SCIPY_NIGHTLY_UPLOAD_TOKEN_CIRRUS: ENCRYPTED[3084f2103e33493cfc1992a22ec8e67665f835ab31eec7a84711dc9b2f78ea6d6e189e871214b7e9ba41e240ad4f8af4]

@andyfaff
Copy link
Copy Markdown
Contributor Author

This should be ready to merge now.

andyfaff and others added 8 commits September 24, 2022 13:00
[skip azp][skip gh][cirrus wheel build]
[skip azp][skip gh][cirrus wheel build]
[skip azp][skip gh][cirrus wheel build]
[skip azp][skip gh][cirrus wheel build]
[skip gh][skip azp]
[wheel build] [skip gh] [skip azp]
@andyfaff andyfaff force-pushed the cirrus branch 3 times, most recently from bb68b09 to 4c9e241 Compare September 24, 2022 04:21
@tylerjereddy tylerjereddy added the Official binaries Items related to the official SciPy binaries, including wheels and vendored libs label Sep 25, 2022
Copy link
Copy Markdown
Contributor

@tylerjereddy tylerjereddy left a comment

Choose a reason for hiding this comment

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

I have one concern here -- isn't the cirrus job that checks the commit message failing the CI rather than skipping/cancelling when a commit is such that it shouldn't trigger a wheel build?

image

@andyfaff
Copy link
Copy Markdown
Contributor Author

You're correct in that @tylerjereddy. I was trying to get all the logic in the trigger step into a single only_if, but couldn't figure it out. I don't think we should merge this, or the follow-up until that's sorted. I'm on holiday for a week, so if anyone figures out the logic feelfree to push it to the branch. I've asked cirrus-ci help to give me a hand

@rgommers rgommers added this to the 1.10.0 milestone Sep 25, 2022
Copy link
Copy Markdown
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Looks good to me logic-wise. Once the CI status thing is resolved, let's merged this.

Comment thread .cirrus.yml Outdated
Comment thread .cirrus.yml
@tupui
Copy link
Copy Markdown
Member

tupui commented Sep 30, 2022

@andyfaff just a note about CI skipping comments, for GH it should be [skip actions]. Also you might want to add [skip circle] in the mix here.

And here or in a follow up, it would be good to add a mention of this CI in our doc.

[wheel build] [skip actions] [skip azp] [skip circle]
@andyfaff
Copy link
Copy Markdown
Contributor Author

@tylerjereddy @rgommers, I believe I've fixed the triggering of the cirrus-ci run.

To accomplish this is we now have a starlark script (https://cirrus-ci.org/guide/programming-tasks/ , similar to Python), .cirrus.star, that creates the tasks for the CI to run. The script figures out when the conditions are right to run a wheel build; if they're ok then all it has to do is return the tasks specified in the original .cirrus.yml file (renamed to ci/cirrus_wheels.yml)

@andyfaff
Copy link
Copy Markdown
Contributor Author

@tylerjereddy it might be worth backporting this to 1.9.x after all because it also fixes an issue in the wheels.yml configuration. SCIPY_STAGING_UPLOAD_TOKEN_GHA should actually be called SCIPY_STAGING_UPLOAD_TOKEN. If the fix isn't applied the wheels won't be uploaded to multibuild-wheels-staging because the upload script will try to look at a non-existent environment variable.

@andyfaff andyfaff removed the request for review from larsoner September 30, 2022 12:15
@tylerjereddy tylerjereddy added the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Oct 2, 2022
@tylerjereddy tylerjereddy dismissed their stale review October 2, 2022 18:01

comments addressed

@tylerjereddy tylerjereddy merged commit 268ae4e into scipy:main Oct 2, 2022
@tylerjereddy
Copy link
Copy Markdown
Contributor

Ok, thanks, I squash merged to facilitate backporting.

@tylerjereddy
Copy link
Copy Markdown
Contributor

I'll need to strip the backport label for now--wheels.yml doesn't even exist on the maintenance branch so I'll need clear labels of what to backport first, etc.

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

Labels

CI Items related to the CI tools such as CircleCI, GitHub Actions or Azure Official binaries Items related to the official SciPy binaries, including wheels and vendored libs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants