Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

build,test: various fixes to align with upstream #446

Closed
wants to merge 0 commits into from

Conversation

kfarnung
Copy link
Contributor

While preparing the v8.x branch I noticed a few oddities in the node.gyp file. This change attempts to bring the file more closely into alignment with upstream.

I also removed an orphaned test status entry.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build, test

@kfarnung kfarnung self-assigned this Jan 11, 2018
@kfarnung
Copy link
Contributor Author

Copy link
Contributor

@digitalinfinity digitalinfinity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable

node.gyp Outdated
@@ -870,7 +870,7 @@
'deps/v8/include'
],
'conditions' : [
['node_use_v8_platform=="true" and node_engine=="v8"', {
['node_use_v8_platform=="true"', {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember we had an issue where we were using v8_platform incorrectly -- I think @digitalinfinity looked into it? Wasn't this the gtest issue?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, nice catch! @kfarnung #431 is the relevant PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@digitalinfinity this one is already embedded inside a node_engine=="v8" condition, so this should be unnecessary. I think the bug you were seeing was caused by the unnecessary duplicate entry:

https://github.com/nodejs/node-chakracore/pull/446/files#diff-259e627be3fafed9e9c10a7fcb26879eL998

kfarnung added a commit that referenced this pull request Jan 12, 2018
PR-URL: #446
Reviewed-By: Hitesh Kanwathirtha <[email protected]>
Reviewed-By: Jack Horton <[email protected]>
kfarnung added a commit that referenced this pull request Jan 12, 2018
PR-URL: #446
Reviewed-By: Hitesh Kanwathirtha <[email protected]>
Reviewed-By: Jack Horton <[email protected]>
@kfarnung kfarnung closed this Jan 12, 2018
@kfarnung kfarnung deleted the nodegyp branch January 12, 2018 01:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants