Add cross-compilation support for Windows on Arm #32867

richard-townsend-arm · 2020-04-15T17:42:17Z

Fixes #32582. CC @richardlau @joaocgreis @jkunkee

This PR fixes the Windows on Arm build of Node.js by applying all the necessary command line flags and workarounds, then adds cross-compilation support for the MSVC backend of node-gyp alongside supporting changes. The way this is implemented is that host GYP targets now end with _host.exe (e.g. mksnapshot_host.exe) and actions are re-written accordingly. In addition to the standard node.sln, a special node_host.sln is also generated with the host-only targets (this because MSBuild doesn't like building solutions with projects targeted at a mix of different architectures).

Current issues

Declaring support for cross-compilation means that some host-only projects are generated for x64/x64 too. The current solution is to compile host tools for all architectures, which increases build time and isn't strictly necessary (although a benefit is that somebody will definitely notice if they break cross-compilation in this setup). Possible changes may be needed to remove _host.exe tools before distribution.
node-gyp changes will need to be contributed upstream (?) and sent back down here.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

nodejs-github-bot · 2020-04-15T17:49:21Z

CI: https://ci.nodejs.org/job/node-test-pull-request/30766/

joaocgreis

Thanks for moving this forward @richard-townsend-arm! I see several minor issues that should be easy to fix, but this fails to build in my x64 machine. WhenI run vcbuild arm64 I get this error building the host part:

msbuild node_host.sln /m:2 /p:Configuration=Release /p:Platform=x64 /clp:NoItemAndPropertyList;Verbosity=minimal /nologo
C:\Users\Administrator\Desktop\Node.js\node\node_host.sln : Solution file error MSB4051: Project {3DA0E800-074E-9F25-3378-2102A04DED74} is referencing a project with GUID {F4CAEC9A-07E1-7F49-5630-116B9D21C367}, but a project with this GUID was not found in the .SLN file.

That seems to be the icudata project.

The commit messages should follow https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md#commit-message-guidelines. The V8 backport should follow https://github.com/nodejs/node/blob/master/doc/guides/maintaining-V8.md#backporting-to-abandoned-branches. I would squash the two gyp commits and all the commits that are not deps together. I see you put some effort into the commit descriptions (thanks!), would be good to keep them. So, what I suggest is:

- 5f8ef62671
  deps: cherry-pick 2cdbb99 from V8 upstream
- b8e7a6c1a2+0ebdb4443e
  tools,gyp: add support for MSVC cross-compilation
- e71a369bc5+67a696acfa+58426be36d+426244cd3a+9627415441
  build,win: support cross-compiling for ARM64

I believe gyp is, or will be, maintained at https://github.com/nodejs/gyp-next. I'm not sure if the gyp here is in sync with that version, but in any case would be good to PR these changes there. I think this can still go forward independently as a floating patch to unblock the process. Any thoughts @nodejs/gyp @targos @cclauss?

vcbuild.bat

nodejs-github-bot · 2020-04-22T04:36:15Z

CI: https://ci.nodejs.org/job/node-test-pull-request/30909/

richard-townsend-arm · 2020-05-04T10:36:43Z

OK, looks like the MacOS CI failure is a flake, ready for review.

joaocgreis · 2020-05-06T04:14:37Z

@richard-townsend-arm I pushed to your branch to make things easier, but I can change it back to 9effddd if you prefer. I changed the first line of the commit messages to follow the Node.js rules, and moved the vcbuild and configure changes to the last commit, so the middle commit is only gyp changes. We usually do this with commits that change dependencies, so it can be easily applied elsewhere or re-floated here if needed.

This looks good and works well for me. I can build x86, x64 and arm64 in my x64 machine. However, the node_host.sln file does not have any projects, it is only the minimal solution boilerplate. All the projects are built as part of the main solution, without errors. Do we really need a separate host sln file?

tools/gyp/pylib/gyp/MSVSNew.py

configure.py

richard-townsend-arm · 2020-05-06T22:02:21Z

@richard-townsend-arm I pushed to your branch to make things easier, but I can change it back to 9effddd if you prefer. I changed the first line of the commit messages to follow the Node.js rules, and moved the vcbuild and configure changes to the last commit, so the middle commit is only gyp changes. We usually do this with commits that change dependencies, so it can be easily applied elsewhere or re-floated here if needed.

Cool, that's great, thank you! (I guess I must have just mixed one of the commits up whilst rebasing).

This looks good and works well for me. I can build x86, x64 and arm64 in my x64 machine. However, the node_host.sln file does not have any projects, it is only the minimal solution boilerplate. All the projects are built as part of the main solution, without errors. Do we really need a separate host sln file?

So I've just double-checked this on my machine and... yeah, it seems to be working fine, without node_host.sln. I think that fixing the x86 build must have also fixed something about how the solution's set up, because I now see that it contains the expected mix of x64 and arm64 projects:

richard-townsend-arm · 2020-05-06T22:05:40Z

Cool, so the next push will remove the node_host.sln file, and implement Christian's recommendations for improving node-gyp. Hopefully will get to it tomorrow.

cclauss

RSLGTM

richard-townsend-arm · 2020-05-12T16:22:52Z

Cool, thanks @cclauss. I'm not authorized to merge. @joaocgreis, any further thoughts?

nodejs-github-bot · 2020-05-14T10:23:25Z

CI: https://ci.nodejs.org/job/node-test-pull-request/31321/

joaocgreis · 2020-05-14T14:17:21Z

I've force pushed from c7e48545d62 to e71e4d365ba.

@richard-townsend-arm I didn't understand the change in configure, if I read it well if the host is x86 or x64 it never defaults to cross-compiling. I think this would be an issue for cross-compiling for the Raspberry Pi Linux, but apparently that job is not running at the moment in CI. I'd much rather leave the logic there as currently is and leave the line in vcbuild that it replaces.

I've rebased and added a fixup commit with my suggestion. The v8 commit is no longer necessary. Does this LGTY?

nodejs-github-bot · 2020-05-14T14:18:38Z

CI: https://ci.nodejs.org/job/node-test-pull-request/31325/

richard-townsend-arm · 2020-05-15T16:48:26Z

Does look good to me, the commit you've pushed though is not building for me due to an unrelated error:

C:\Users\rictow01\node\deps\v8\src\base\platform\platform-win32.cc(1407,5): error C2039: 'GetCurrentThreadStackLimits': is not a member of '`global namespace'' [C:\Us
ers\rictow01\node\tools\v8_gypfiles\v8_libbase.vcxproj]
C:\Users\rictow01\node\deps\v8\src\base\platform\platform-win32.cc(1407,32): error C3861: 'GetCurrentThreadStackLimits': identifier not found [C:\Users\rictow01\node\
tools\v8_gypfiles\v8_libbase.vcxproj]

... but that's separately fixable.

nodejs-github-bot · 2020-05-15T19:06:05Z

CI: https://ci.nodejs.org/job/node-test-pull-request/31352/

This change means that GYP can now generate two sets of projects: one exclusively for a host x64 machine and one containing a mix of x64 and Arm targets. The names of host targets are fixed up to end with _host.exe, and any actions involving them are fixed up. This allows compilation of Node on an x64 server for a Windows on Arm target. Change-Id: I5d9ded8a43b7ec5d5c7f501134080ac94dceacf5

* Fixes cases in icutools where commands were issued without .exe * Changes to build scripts * Add /fp:strict flag so that MSVC's floating point behaves correctly * Enables marmasm Change-Id: Ife1fd90de271edd9dcefdef5fe439d7a9a05e8dc

joaocgreis · 2020-05-18T17:16:21Z

Rebased to get a fresh CI run (e71e4d365ba to 87f3389).

@richard-townsend-arm if this passes CI, I'll land this PR, as the new failure happens only on ARM64.

nodejs-github-bot · 2020-05-18T17:17:05Z

CI: https://ci.nodejs.org/job/node-test-pull-request/31387/

This change means that GYP can now generate two sets of projects: one exclusively for a host x64 machine and one containing a mix of x64 and Arm targets. The names of host targets are fixed up to end with _host.exe, and any actions involving them are fixed up. This allows compilation of Node on an x64 server for a Windows on Arm target. PR-URL: #32867 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: João Reis <reis@janeasystems.com>

* Fixes cases in icutools where commands were issued without .exe * Changes to build scripts * Add /fp:strict flag so that MSVC's floating point behaves correctly * Enables marmasm PR-URL: #32867 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: João Reis <reis@janeasystems.com>

joaocgreis · 2020-05-19T18:38:39Z

Landed in d093e78...8833551

@richard-townsend-arm thanks for pushing this forward! The current failure building for ARM64 was caused by 1d6adf7#diff-4fa95c2da0a6e4a70f97811e38ef0ba8R1407 , the V8 update that landed while this PR was open. Do you know why that doesn't fail in V8 upstream (I believe they have ARM64 CI) and how to fix that here?

richard-townsend-arm · 2020-05-19T19:29:40Z

I do super-vaguely remember fixing something like this on upstream V8, I’ll double-check and back port if necessary.

richard-townsend-arm · 2020-05-20T17:36:41Z

Found it: https://chromium-review.googlesource.com/c/v8/v8/+/2124321

richard-townsend-arm · 2020-05-20T17:40:40Z

Wait, sorry, that may not be quite the right fixup. I'll take a closer look in the next day or so.

richard-townsend-arm · 2020-05-20T18:07:44Z

Actually, turned out to be something quite simple: this function is relatively new, so we need to define _WIN32_WINNT=0x0602 when compiling code involving GetCurrentThreadStackLimits. Will fix up and post a PR.

This change means that GYP can now generate two sets of projects: one exclusively for a host x64 machine and one containing a mix of x64 and Arm targets. The names of host targets are fixed up to end with _host.exe, and any actions involving them are fixed up. This allows compilation of Node on an x64 server for a Windows on Arm target. PR-URL: #32867 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: João Reis <reis@janeasystems.com>

* Fixes cases in icutools where commands were issued without .exe * Changes to build scripts * Add /fp:strict flag so that MSVC's floating point behaves correctly * Enables marmasm PR-URL: #32867 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: João Reis <reis@janeasystems.com>

Original commit message: tools,gyp: add support for MSVC cross-compilation This change means that GYP can now generate two sets of projects: one exclusively for a host x64 machine and one containing a mix of x64 and Arm targets. The names of host targets are fixed up to end with _host.exe, and any actions involving them are fixed up. This allows compilation of Node on an x64 server for a Windows on Arm target. Refs: nodejs/node#32867 Closes: nodejs#40

Original commit message: tools,gyp: add support for MSVC cross-compilation This change means that GYP can now generate two sets of projects: one exclusively for a host x64 machine and one containing a mix of x64 and Arm targets. The names of host targets are fixed up to end with _host.exe, and any actions involving them are fixed up. This allows compilation of Node on an x64 server for a Windows on Arm target. Refs: nodejs/node#32867 Closes: #40

This change means that GYP can now generate two sets of projects: one exclusively for a host x64 machine and one containing a mix of x64 and Arm targets. The names of host targets are fixed up to end with _host.exe, and any actions involving them are fixed up. This allows compilation of Node on an x64 server for a Windows on Arm target. PR-URL: nodejs#32867 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: João Reis <reis@janeasystems.com>

nodejs-github-bot added build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency. windows Issues and PRs related to the Windows platform. labels Apr 15, 2020

richard-townsend-arm changed the title ~~Add x64 cross-compilation for Windows on Arm~~ Add cross-compilation support for Windows on Arm Apr 15, 2020

joaocgreis reviewed Apr 22, 2020

View reviewed changes

vcbuild.bat Outdated Show resolved Hide resolved

vcbuild.bat Outdated Show resolved Hide resolved

vcbuild.bat Outdated Show resolved Hide resolved

richard-townsend-arm force-pushed the woa-crosscompile branch from 0ebdb44 to 9effddd Compare May 1, 2020 11:58

joaocgreis force-pushed the woa-crosscompile branch from 9effddd to ef512ad Compare May 6, 2020 04:05

cclauss requested changes May 6, 2020

View reviewed changes

tools/gyp/pylib/gyp/MSVSNew.py Outdated Show resolved Hide resolved

tools/gyp/pylib/gyp/MSVSNew.py Outdated Show resolved Hide resolved

configure.py Outdated Show resolved Hide resolved

dennisameling mentioned this pull request May 6, 2020

Add Windows ARM64 + Apple Silicon support desktop/desktop#9691

Merged

4 tasks

richard-townsend-arm force-pushed the woa-crosscompile branch from ef512ad to c7e4854 Compare May 7, 2020 15:48

richard-townsend-arm requested a review from cclauss May 11, 2020 16:26

cclauss approved these changes May 11, 2020

View reviewed changes

targos mentioned this pull request May 12, 2020

upstream cross-compilation support for Windows on Arm nodejs/gyp-next#40

Closed

joaocgreis force-pushed the woa-crosscompile branch from c7e4854 to e71e4d3 Compare May 14, 2020 14:11

richard-townsend-arm and others added 3 commits May 18, 2020 18:11

build,win: add support for MSVC cross-compilation

3f58256

* Fixes cases in icutools where commands were issued without .exe * Changes to build scripts * Add /fp:strict flag so that MSVC's floating point behaves correctly * Enables marmasm Change-Id: Ife1fd90de271edd9dcefdef5fe439d7a9a05e8dc

fixup! build,win: add support for MSVC cross-compilation

87f3389

joaocgreis force-pushed the woa-crosscompile branch from e71e4d3 to 87f3389 Compare May 18, 2020 17:13

joaocgreis approved these changes May 19, 2020

View reviewed changes

joaocgreis closed this May 19, 2020

joaocgreis added dont-land-on-v10.x labels May 19, 2020

richard-townsend-arm mentioned this pull request May 22, 2020

tools: fix master branch build for Windows on Arm #33511

Closed

1 task

dennisameling mentioned this pull request Jun 1, 2020

Support ARM64 Windows desktop #25998

Closed

codebytere mentioned this pull request Jun 28, 2020

v14.5.0 proposal #34093

Merged

gengjiawen mentioned this pull request Jul 15, 2020

tools: update gyp to 0.5.0 #32698

Closed

4 tasks

dennisameling mentioned this pull request Oct 1, 2020

Re-add Windows arm64 (un)official builds nodejs/build#2450

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cross-compilation support for Windows on Arm #32867

Add cross-compilation support for Windows on Arm #32867

richard-townsend-arm commented Apr 15, 2020 •

edited by sam-github

nodejs-github-bot commented Apr 15, 2020

joaocgreis left a comment

nodejs-github-bot commented Apr 22, 2020

richard-townsend-arm commented May 4, 2020

joaocgreis commented May 6, 2020

richard-townsend-arm commented May 6, 2020

richard-townsend-arm commented May 6, 2020

cclauss left a comment

richard-townsend-arm commented May 12, 2020

nodejs-github-bot commented May 14, 2020

joaocgreis commented May 14, 2020

nodejs-github-bot commented May 14, 2020

richard-townsend-arm commented May 15, 2020

nodejs-github-bot commented May 15, 2020

joaocgreis commented May 18, 2020

nodejs-github-bot commented May 18, 2020

joaocgreis commented May 19, 2020

richard-townsend-arm commented May 19, 2020

richard-townsend-arm commented May 20, 2020

richard-townsend-arm commented May 20, 2020

richard-townsend-arm commented May 20, 2020

Add cross-compilation support for Windows on Arm #32867

Add cross-compilation support for Windows on Arm #32867

Conversation

richard-townsend-arm commented Apr 15, 2020 • edited by sam-github

Checklist

nodejs-github-bot commented Apr 15, 2020

joaocgreis left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Apr 22, 2020

richard-townsend-arm commented May 4, 2020

joaocgreis commented May 6, 2020

richard-townsend-arm commented May 6, 2020

richard-townsend-arm commented May 6, 2020

cclauss left a comment

Choose a reason for hiding this comment

richard-townsend-arm commented May 12, 2020

nodejs-github-bot commented May 14, 2020

joaocgreis commented May 14, 2020

nodejs-github-bot commented May 14, 2020

richard-townsend-arm commented May 15, 2020

nodejs-github-bot commented May 15, 2020

joaocgreis commented May 18, 2020

nodejs-github-bot commented May 18, 2020

joaocgreis commented May 19, 2020

richard-townsend-arm commented May 19, 2020

richard-townsend-arm commented May 20, 2020

richard-townsend-arm commented May 20, 2020

richard-townsend-arm commented May 20, 2020

richard-townsend-arm commented Apr 15, 2020 •

edited by sam-github