Skip to content

Comments

Add macos-arm64 arch#41

Merged
benweissmann merged 5 commits intobenweissmann:masterfrom
Nevalicjus:master
Apr 6, 2025
Merged

Add macos-arm64 arch#41
benweissmann merged 5 commits intobenweissmann:masterfrom
Nevalicjus:master

Conversation

@Nevalicjus
Copy link
Contributor

Description

My pr adds support for macos-arm64 release, so micro installed via the script doesn't require rosetta to run.
I've also modified the test workflow to run on macos-13 and macos-14 instead of just macos-12.
I've also added macos-arm64 as a valid release in both the error message and readme (respecting the alphabetical order).

How Has This Been Tested?

I've ran the updated script on an intel mac and on an arm mac, and it worked.
My changes have also passed existing tests.

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • If I added new user-facing functionality, I have made corresponding changes to the README documenting the changes
  • If this is a code change, I have updated the test configuration and/or test scripts to cover the changes I've made.

Copy link
Owner

@benweissmann benweissmann left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution! I have one small request, but other than that this looks great.

index.sh Outdated
"darwin")
case "$machine" in
"arm64") platform='macos-arm64' ;;
"x86_64") platform='osx' ;;
Copy link
Owner

Choose a reason for hiding this comment

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

Since I don't have a pre-M1 machine handy to test this fallback on -- let's have this branch use *) platform='osx' ;; instead of specifically matching x86_64 so we guarantee no change in behavior unless uname -m reports arm64.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was indeed torn between * and x86_64 case for osx, but finally decided on x86_64 to match the other precisely defined cases - but I do think that would be more reasonable

@Nevalicjus Nevalicjus requested a review from benweissmann April 6, 2025 20:27
@benweissmann benweissmann merged commit d1a77a5 into benweissmann:master Apr 6, 2025
47 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.

2 participants