-
Notifications
You must be signed in to change notification settings - Fork 234
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 mac m1 runner to ci matrix (#451) #487
Conversation
@@ -17,6 +17,7 @@ jobs: | |||
- windows-latest | |||
- ubuntu-latest | |||
- macos-latest | |||
- macos-14 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference between this and the latest version above? Are both needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, according to https://github.blog/changelog/2024-01-30-github-actions-macos-14-sonoma-is-now-available/
macos-14 is running on m1 machines and macos-latest is now targeting macos12 and is running on x64 architecture
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
macos-latest
will become macos-14
at least from my interpretation of the blog post
I believe we should pin to macos-12
to ensure we retain x86_64 builds, when macos-latest
switches
The macOS 12 runner image will remain latest until migration of the latest YAML workflow label to macOS 14 in Q2 FY24 (April – June 2024). While macOS 13 is now generally available under the macos-13 label, this image will not be migrated to latest. Following this announcement, macOS 11 runner image will begin deprecation immediately with retirement expected to complete by June 2024.
The runner images show readme's for ARM/AMD versions of macos-14 but there doesn't seem to be a way to specify an arch (that I've seen), so if intel mac's get phased out on gh actions, we may have to rely on rosetta
We've done that against the other two repos currently using these runners
@@ -36,7 +37,7 @@ jobs: | |||
run: dotnet build --no-restore | |||
|
|||
- name: Test | |||
run: dotnet test --no-build --verbosity normal -- RunConfiguration.TargetPlatform=x64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this removing the Mac x86-64 CI? We still need both
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as this step is executed for all matrix os's, it will get executed for macos-latest in x64 and for macos-14 in arm64
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep thats how I read that, it will execute for both steps and use the systems platform+arch, the latter of which will differ in the case of the two distinct macos images which are arch specific
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd just pin macos-latest
to macos-12
. Thanks for the addition!
I think we need to make it more explicit which job is using which arch. Just from looking at the CI definition it's unclear that 12 just happens to be x86 and 14 is ARM, plus those could change in future. |
Added in |
As github recently introduced support for mac m1/m2 runners (https://github.blog/changelog/2024-01-30-github-actions-introducing-the-new-m1-macos-runner-available-to-open-source/), giving it a try