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 arm64 binaries since Node.js v16 #970

Merged
merged 1 commit into from
May 6, 2021

Conversation

seregamorph
Copy link
Contributor

Summary
Node.js supports native arm64 binary release since v16:
https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V16.md#toolchain-and-compiler-upgrades
For example, you can see darwin-arm64 releases here: https://nodejs.org/download/release/v16.0.0/

Old logic: for mac arm64 always prefer x64 instead.
New logic: for mac arm64 prefer arm64 since node v16, otherwise fallback to old behavior.

@seregamorph
Copy link
Contributor Author

seregamorph commented May 6, 2021

@ajthom90 please check this change also

Proof of new logic. Tested on Apple M1 machine:

For node v16.1.0 - darwin-arm64:

[INFO] --- frontend-maven-plugin:1.11.4-SNAPSHOT:install-node-and-npm (install node and npm) @ webservice ---
[INFO] Installing node version v16.1.0
[INFO] Downloading https://nodejs.org/dist/v16.1.0/node-v16.1.0-darwin-arm64.tar.gz to /Users/morph/.m2/repository/com/github/eirslett/node/16.1.0/node-16.1.0-darwin-arm64.tar.gz
[INFO] No proxies configured
[INFO] No proxy was configured, downloading directly
[INFO] Unpacking /Users/morph/.m2/repository/com/github/eirslett/node/16.1.0/node-16.1.0-darwin-arm64.tar.gz into /Users/morph/Projects/acme/webservice/node/tmp
[INFO] Copying node binary from /Users/morph/Projects/acme/webservice/node/tmp/node-v16.1.0-darwin-arm64/bin/node to /Users/morph/Projects/acme/webservice/node/node

Exactly the same project, but with node version v12.14.0 - darwin-x64 fallback:

[INFO] --- frontend-maven-plugin:1.11.4-SNAPSHOT:install-node-and-npm (install node and npm) @ webservice ---
[INFO] Installing node version v12.14.0
[INFO] Downloading https://nodejs.org/dist/v12.14.0/node-v12.14.0-darwin-x64.tar.gz to /Users/morph/.m2/repository/com/github/eirslett/node/12.14.0/node-12.14.0-darwin-x64.tar.gz
[INFO] No proxies configured
[INFO] No proxy was configured, downloading directly
[INFO] Unpacking /Users/morph/.m2/repository/com/github/eirslett/node/12.14.0/node-12.14.0-darwin-x64.tar.gz into /Users/morph/Projects/acme/webservice/node/tmp
[INFO] Copying node binary from /Users/morph/Projects/acme/webservice/node/tmp/node-v12.14.0-darwin-x64/bin/node to /Users/morph/Projects/acme/webservice/node/node

@ajthom90
Copy link
Contributor

ajthom90 commented May 6, 2021

@seregamorph This looks great to me. I was working on this last week and got a little sidetracked (and I cancelled my MacStadium subscription, so I didn't have an M1 Mac to test it on), but my logic looks almost identical to yours. And your code looks much cleaner than mine. Thanks for taking care of this!

This idea just popped into my head though and maybe necessarily doesn't need to be done in this PR (or ever be done), but would we possibly want a config option for Mac users to say "I always want the x64 binary" even if they are on an M1 Mac? I'd assume that most people would want to run Node natively when they're on an arm64 machine, but maybe just in case people want the option? I'm good either way, like I said, it just popped into my head, so I figured I'd bring it up here.

Thanks again!

@seregamorph
Copy link
Contributor Author

@ajthom90 I had the same idea. IMHO it'll be better to introduce such enhancement in next major release.
Right now user can choose of three possible strategies (old, fixed by you and fixed by me) and it should cover all cases.

@ajthom90
Copy link
Contributor

ajthom90 commented May 6, 2021

@seregamorph I like it. Thanks again!

@seregamorph seregamorph changed the title Support arm64 binaries since Node.js v16.0.0 Support arm64 binaries since Node.js v16 May 6, 2021
@eirslett
Copy link
Owner

eirslett commented May 6, 2021

LGTM, thanks!

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