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 aarch64 target for darwin desktops #78

Closed
wants to merge 3 commits into from

Conversation

skhamis
Copy link
Contributor

@skhamis skhamis commented Dec 15, 2021

fixes #77

Turned out to be a very small patch! Tested with both x86_64 and arm64 JDKs and both seem to compile nicely (the a-s issue mentioned in #77) . The only issue is I don't necessarily know the best way to handle the deprecation of darwin to officially split darwin to use the {-x86_64, -aarch64}. Throwing this up for early visibility....

@skhamis skhamis marked this pull request as ready for review December 15, 2021 23:13
@skhamis
Copy link
Contributor Author

skhamis commented Dec 15, 2021

@ncalexan What do you think of going the darwin + darwin-{x86_64, aarch64} vs just removing the darwin target (breaking change) and only have the darwin with the modifiers? Curious your thoughts!

Copy link
Member

@ncalexan ncalexan left a comment

Choose a reason for hiding this comment

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

Just nits -- everything looks fine. I'll land this squashed if you prefer to not squash it yourself. I would like to see it rebased onto #81 once those tests are green in automation, the new tests updated to include the new Darwin targets and locations, and repushed to be sure we've got this correct. If you have time, please do it; otherwise, I'll try to get to it shortly.

Thanks, @skhamis!

@skhamis
Copy link
Contributor Author

skhamis commented Jan 12, 2022

@ncalexan Thanks for the review! I have applied all requested nits/changes and rebased ontop of #81 for those CI runs. I also squashed and bumped the plugin version -- let me know if I missed anything! Should look good once the CIs pass!

@ncalexan
Copy link
Member

I landed this manually onto master: c4bf584.

Sorry for the unusual landing; I had some CI testing to work through. Thanks for pushing this forward, @skhamis!

@ncalexan ncalexan closed this Jan 15, 2022
@ncalexan
Copy link
Member

Oh -- I cut v0.9.1 with this change as well. Please test and make sure it works for y'all!

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.

Support for Apple M1 aarch64
2 participants