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.

Sign up for GitHub

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

Jump to bottom

[v16.x backport] tools: update V8 gypfiles for 9.6 #43247

Closed
wants to merge 2 commits into from
Closed

[v16.x backport] tools: update V8 gypfiles for 9.6 #43247

wants to merge 2 commits into from

Conversation

RaisinTen
Copy link
Contributor

@RaisinTen RaisinTen commented May 30, 2022 •
edited
Loading

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:

@targos @RaisinTen
tools: update V8 gypfiles for 9.6
f101e43 
PR-URL: nodejs#40488
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

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

@nodejs-github-bot nodejs-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.

Sign in to view
@RaisinTen RaisinTen mentioned this pull request May 30, 2022
Closed
@gengjiawen
Copy link
Member

Should we add this to github actions ?

@targos
Copy link
Member

targos commented May 30, 2022

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
Contributor Author

I think we could do that in a separate PR.

@RaisinTen RaisinTen 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

targos commented May 30, 2022

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

@targos @RaisinTen
tools: disable trap handler for Windows cross-compiler
2e56e92 
`handler-outside-simulator.cc` uses inline assembly, which is not
supported by MSVC.

PR-URL: nodejs#40488
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RaisinTen
Copy link
Contributor Author

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

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

juanarbol pushed a commit that referenced this pull request May 31, 2022
@targos @juanarbol
tools: update V8 gypfiles for 9.6
776d11a 
PR-URL: #40488
Backport-PR-URL: #43247
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this pull request May 31, 2022
@targos @juanarbol
tools: disable trap handler for Windows cross-compiler
520e5d9 
`handler-outside-simulator.cc` uses inline assembly, which is not
supported by MSVC.

PR-URL: #40488
Backport-PR-URL: #43247
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this pull request Jun 1, 2022
@targos @juanarbol
tools: update V8 gypfiles for 9.6
47cdddf 
PR-URL: #40488
Backport-PR-URL: #43247
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this pull request Jun 1, 2022
@targos @juanarbol
tools: disable trap handler for Windows cross-compiler
feac215 
`handler-outside-simulator.cc` uses inline assembly, which is not
supported by MSVC.

PR-URL: #40488
Backport-PR-URL: #43247
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@juanarbol
Copy link
Member

juanarbol commented Jun 1, 2022 •
edited
Loading

Thank you!!!

Landed in 447f9a0...feac215

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

@BethGriggs BethGriggs BethGriggs approved these changes

@gengjiawen gengjiawen gengjiawen approved these changes

@targos targos targos approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@RaisinTen @nodejs-github-bot @gengjiawen @targos @juanarbol @BethGriggs