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

Support macos-aarch64 as a platform keyword #60

Merged
merged 5 commits into from
Sep 27, 2022

Conversation

JohnnyMorganz
Copy link
Contributor

@JohnnyMorganz JohnnyMorganz commented Sep 27, 2022

StyLua produces macOS M1 binaries using the macos-aarch64 name, rather than macos-arm64.

We add in macos-aarch64 as a platform keyword so it doesn't fall back to the Rosetta x86_64 binary

Copy link
Contributor

@oltrep oltrep 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 taking the time to submit a contribution 👍

Comment on lines 11 to 14
"macos-aarch64",
"darwin-aarch64",
"macos-x86_64",
"darwin-x86_64",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep the macos-x86_64 and darwin-x86_64 here? I think we could just remove them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that should be possible actually - since the fallback "macos" will be picked anyways

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fair to assume that if there are any disambiguated binaries, they'll probably all be disambiguated (rather than having macos-arm64 and macos, you'd have macos-arm64 and macos-x86_64 to make sure that both names are clear).

So I'm onboard with that as well; don't need to do it in this PR if you don't want to, though.

Copy link
Contributor

@ZoteTheMighty ZoteTheMighty 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! Thanks for the contribution!

Comment on lines 11 to 14
"macos-aarch64",
"darwin-aarch64",
"macos-x86_64",
"darwin-x86_64",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fair to assume that if there are any disambiguated binaries, they'll probably all be disambiguated (rather than having macos-arm64 and macos, you'd have macos-arm64 and macos-x86_64 to make sure that both names are clear).

So I'm onboard with that as well; don't need to do it in this PR if you don't want to, though.

@oltrep
Copy link
Contributor

oltrep commented Sep 27, 2022

@JohnnyMorganz all that's needed is for the CLA check to pass. There used to be a bot comment that told what the contributor needed to do but it looks like it's not working properly.

I think if you comment "I have read the CLA Document and I hereby sign the CLA" to the PR, then we can re-run the check and that should do it.

@github-actions
Copy link

github-actions bot commented Sep 27, 2022

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@JohnnyMorganz
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@ZoteTheMighty ZoteTheMighty merged commit ae5447a into Roblox:master Sep 27, 2022
@JohnnyMorganz JohnnyMorganz deleted the macos-aarch64 branch September 27, 2022 17:37
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.

3 participants