Skip to content

Added fix for building node 10 - #48

Merged
vweevers merged 1 commit into
masterfrom
build_v8_with_gn
Nov 18, 2020
Merged

vweevers merged 1 commit into
masterfrom
build_v8_with_gn

Conversation

@mathiask88

Copy link
Copy Markdown
Member

see prebuild/prebuild#223

would be nice to have this workaround here as well

@mafintosh

Copy link
Copy Markdown
Collaborator

Only relevant for 10 yea? Should prob only enable it there

@mathiask88

mathiask88 commented Nov 18, 2020 •

Copy link
Copy Markdown
Member Author

Yes only relevant for 10 AFAIK, sure I can add a version check for this condition

//Edit: Short background for this: I have a N-API module that needs at least N-API v5, so I build explicitly against node v10.17.0 and ended up with that bug

@vweevers

Copy link
Copy Markdown
Member

It looks like the flag has been removed in node 15: nodejs/node#27576. The default value in node was the same as the value we set here with --build_v8_with_gn=false, so it should not affect node 10-14.

I can add a version check for this condition

👍 Better to be safe than sorry, and it'll remind us to remove it in the future.

@vweevers

Copy link
Copy Markdown
Member

Closes #10

@mathiask88

Copy link
Copy Markdown
Member Author

I added a version check for 10 and 11 only because I had no issues with other node versions.

@vweevers

Copy link
Copy Markdown
Member

Travis is taking too long, I'm canceling.

@vweevers
vweevers merged commit d5a1bcc into master Nov 18, 2020
@vweevers
vweevers deleted the build_v8_with_gn branch November 18, 2020 22:19
@vweevers

Copy link
Copy Markdown
Member

4.1.1

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