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 the brand new (beta) GitHub hosted M1 runner into ci.yml #685

Merged
merged 9 commits into from
Feb 6, 2024

Conversation

erogluorhan
Copy link
Member

@erogluorhan erogluorhan commented Jan 31, 2024

Add "macos-14" to CI matrix for M1 testing

Overview

GitHub just announced their brand new M1 runner, "macos-14", for use. Let us give it a try in our CI actions.

See this PR's own action runs for a quick demo of it.

Add "macos-14" to CI matrix for M1 testing
@erogluorhan erogluorhan marked this pull request as draft January 31, 2024 18:44
@erogluorhan erogluorhan changed the title Add the brand new (beta) GitHUb hosted M1 runner into ci.yml Add the brand new (beta) GitHub hosted M1 runner into ci.yml Jan 31, 2024
@rajeeja
Copy link
Contributor

rajeeja commented Jan 31, 2024

this is nice, I use M1:)
do they charge or count hours towards it?
the last i checked they were free for linux and some hour limit appled to anything windows or mac.

@erogluorhan
Copy link
Member Author

No, they don't charge. It is available to open source, just like the other GH-hosted runners such as "macos-latest". Though, it's in beta. Currently our tests failed with it, I didn't dig into it to see if it was our code or their runner's fault. I use an M2 :)

@kafitzgerald
Copy link
Collaborator

kafitzgerald commented Feb 1, 2024

Exciting to see this!

It looks like this is the problem: https://github.com/UXARRAY/uxarray/actions/runs/7734670366/job/21089077449?pr=685#step:4:12

The default must work fine for the current options, but obviously not w/ arm64

https://github.com/conda-incubator/setup-miniconda?tab=readme-ov-file#example-11-alternative-architectures

@kafitzgerald
Copy link
Collaborator

Well, that was false hope, but it looks like there's a workaround in this thread and work in progress to support this.

@erogluorhan
Copy link
Member Author

Well, that was false hope, but it looks like there's a workaround in this thread and work in progress to support this.

Thanks a lot for pointing this out! It helped us overcome the architecture issue. Now is a shell issue I believe, which leads to python command not being recognized. On it!

@erogluorhan
Copy link
Member Author

erogluorhan commented Feb 1, 2024

Yay, we got them all to pass!

Any ideas on cleaning up a bit the X64 vs. ARM64 conditioning for the "conda setup" step in the file would be appreciated.

@erogluorhan erogluorhan marked this pull request as ready for review February 1, 2024 20:24
@kafitzgerald
Copy link
Collaborator

Yay, we got them all to pass!

Any ideas on cleaning up a bit the X64 vs. ARM64 conditioning for the "conda setup" step in the file would be appreciated.

I think this might be the best we can do right now (open to ideas though). I did something similar over on geocat-viz.

I'm hopeful that once setup-miniconda actually has support for this it'll look a bit cleaner. There's a PR in review now to add it.

@erogluorhan erogluorhan merged commit b6bb18b into main Feb 6, 2024
14 checks passed
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.

None yet

4 participants