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

Added fix for building node 10 #48

Merged
merged 1 commit into from
Nov 18, 2020
Merged

Added fix for building node 10 #48

merged 1 commit into from
Nov 18, 2020

Conversation

mathiask88
Copy link
Member

see prebuild/prebuild#223

would be nice to have this workaround here as well

@mafintosh
Copy link
Collaborator

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

@mathiask88
Copy link
Member Author

mathiask88 commented Nov 18, 2020

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
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
Member

Closes #10

@mathiask88
Copy link
Member Author

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

@vweevers
Copy link
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
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