Skip to content

Conversation

@RaisinTen
Copy link
Member

@RaisinTenRaisinTen commented May 30, 2022

This is a clean cherry-pick of 6ac1ccc and 4d2d44f. With this change, it
is possible to cross-compile from x86_64 to ARM64 on macOS.

Fixes: #40350

cc @targos


Original commit messages:

PR-URL: nodejs#40488 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: James M Snell <[email protected]>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/v8-update

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. v16.x v8 engine Issues and PRs related to the V8 dependency. labels May 30, 2022
@nodejs-github-bot

This comment was marked as outdated.

@gengjiawen
Copy link
Member

Should we add this to github actions ?

@targos
Copy link
Member

Should we add this to github actions ?

@gengjiawen what do you mean?

@gengjiawen
Copy link
Member

Should we add this to github actions ?

@gengjiawen what do you mean?

cross-compile check on macOS in github actions.

@RaisinTen
Copy link
MemberAuthor

I think we could do that in a separate PR.

@RaisinTenRaisinTen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 30, 2022
@targos
Copy link
Member

macOS compilation is very slow in GitHub actions. I'd prefer if we tried to do it on our own machines

`handler-outside-simulator.cc` uses inline assembly, which is not supported by MSVC. PR-URL: nodejs#40488 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: James M Snell <[email protected]>
@RaisinTen
Copy link
MemberAuthor

The win-vs2019-arm64 build was failing with the same error as reported in #40488 (comment):

C:\workspace\node-compile-windows\node\deps\v8\src\trap-handler\handler-outside-simulator.cc(33,56): error C2290: C++ 'asm' syntax ignored. Use __asm. [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\v8_base_without_compiler_host.vcxproj] 

so I also cherry-picked 4d2d44f.

@nodejs-github-bot
Copy link
Collaborator

juanarbol pushed a commit that referenced this pull request May 31, 2022
PR-URL: #40488 Backport-PR-URL: #43247 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: James M Snell <[email protected]>
juanarbol pushed a commit that referenced this pull request May 31, 2022
`handler-outside-simulator.cc` uses inline assembly, which is not supported by MSVC. PR-URL: #40488 Backport-PR-URL: #43247 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: James M Snell <[email protected]>
juanarbol pushed a commit that referenced this pull request Jun 1, 2022
PR-URL: #40488 Backport-PR-URL: #43247 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: James M Snell <[email protected]>
juanarbol pushed a commit that referenced this pull request Jun 1, 2022
`handler-outside-simulator.cc` uses inline assembly, which is not supported by MSVC. PR-URL: #40488 Backport-PR-URL: #43247 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: James M Snell <[email protected]>
@juanarbol
Copy link
Member

juanarbol commented Jun 1, 2022

Thank you!!!

Landed in 447f9a0...feac215

@RaisinTenRaisinTen deleted the backport-cross-compile-fix-to-v16.x branch June 1, 2022 04:26
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.needs-ciPRs that need a full CI run.toolsIssues and PRs related to the tools directory.v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@RaisinTen@nodejs-github-bot@gengjiawen@targos@juanarbol@BethGriggs