build: remove cctest extension #16680

yhwang · 2017-11-02T04:56:25Z

cctest appends so.59 when building node shared library in linux.
The appending is defined in node.gypi and the cctest target in node.gyp
includes node.gypi. Moving the appending from node.gypi to node target
in node.gyp fix the issue.

Signed-off-by: Yihong Wang [email protected]

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

Affected core subsystem(s)

gibfahn · 2017-11-02T11:58:19Z

cc/ @nodejs/gyp

bnoordhuis

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

yhwang · 2017-11-02T20:24:27Z

despite of node-test-commit-arm-fanned, seems linux and plinux were failed in this test case: parallel/test-http2-server-rst-stream. Is it a intermittent failure recently?

jasnell · 2017-11-02T20:27:04Z

That was a known flaky test. The fix just landed. It's unrelated to this pr

yhwang · 2017-11-02T20:29:42Z

okay, found it : #16690. And it's landed. Can someone rerun the CI? Thanks.

yhwang · 2017-11-02T20:34:28Z

thanks @jasnell then the failures on the CI above are from here https://ci.nodejs.org/job/node-test-binary-arm/11442/. is this critical and need to figure out? The run_subset 6 passed in all pi

mhdawson

LGTM

mhdawson · 2017-11-02T21:21:44Z

New CI run: https://ci.nodejs.org/job/node-test-pull-request/11158/

yhwang · 2017-11-02T22:22:43Z

still got some failures:

parallel/test-async-wrap-uncaughtexception (should be related to test: fix flaky test-async-wrap-uncaughtexception #16598)
sequential/test-inspector-bindings # TODO : Fix flaky test
Build 4,pi1-raspbian-wheezy
Build 3,pi1-raspbian-wheezy

However, I think these are not related to my change.

gibfahn · 2017-11-02T22:26:19Z

However, I think these are not related to my change.

Yep, you never need to worry about the flaky tests, and the Pi failures are known issues.

yhwang · 2017-11-07T19:30:25Z

Ping, Is there anything that I should do in order to get this to be landed?

cctest has `so.59` extension when building node shared library in linux. The appending is defined in node.gypi and the cctest target in node.gyp includes node.gypi. Moving the appending from node.gypi to node target in node.gyp fixes the issue. Signed-off-by: Yihong Wang <[email protected]>

yhwang · 2017-11-10T20:25:57Z

Resolved the conflict and need a new CI. Thanks!

refack · 2017-11-10T21:14:04Z

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

refack · 2017-11-10T21:48:24Z

Landed in b340c2d

cctest has `so.59` extension when building node shared library in linux. The appending is defined in node.gypi and the cctest target in node.gyp includes node.gypi. Moving the appending from node.gypi to node target in node.gyp fixes the issue. Signed-off-by: Yihong Wang <[email protected]> PR-URL: nodejs#16680 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>

yhwang · 2017-11-10T22:13:44Z

thanks @refack

cctest has `so.59` extension when building node shared library in linux. The appending is defined in node.gypi and the cctest target in node.gyp includes node.gypi. Moving the appending from node.gypi to node target in node.gyp fixes the issue. Signed-off-by: Yihong Wang <[email protected]> PR-URL: #16680 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>

MylesBorins · 2017-11-17T15:41:44Z

Should this be backported to v6.x-staging or v8.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

yhwang · 2017-11-17T22:18:24Z

@MylesBorins yes, it should be backported to v6.x-staging or v8.x-staging. I will follow the guide to do the backport. Thanks for the reminding.

cctest has `so.59` extension when building node shared library in linux. The appending is defined in node.gypi and the cctest target in node.gyp includes node.gypi. Moving the appending from node.gypi to node target in node.gyp fixes the issue. Signed-off-by: Yihong Wang <[email protected]> PR-URL: nodejs#16680 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>

cctest has `so.59` extension when building node shared library in linux. The appending is defined in node.gypi and the cctest target in node.gyp includes node.gypi. Moving the appending from node.gypi to node target in node.gyp fixes the issue. Signed-off-by: Yihong Wang <[email protected]> Backport-PR-URL: #17255 PR-URL: #16680 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>

cctest has `so.59` extension when building node shared library in linux. The appending is defined in node.gypi and the cctest target in node.gyp includes node.gypi. Moving the appending from node.gypi to node target in node.gyp fixes the issue. Signed-off-by: Yihong Wang <[email protected]> PR-URL: nodejs#16680 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>

cctest has `so.59` extension when building node shared library in linux. The appending is defined in node.gypi and the cctest target in node.gyp includes node.gypi. Moving the appending from node.gypi to node target in node.gyp fixes the issue. Signed-off-by: Yihong Wang <[email protected]> PR-URL: #16680 Backport-PR-URL: #17258 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>

nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Nov 2, 2017

jasnell requested review from bnoordhuis and refack November 2, 2017 05:06

bnoordhuis approved these changes Nov 2, 2017

View reviewed changes

jasnell approved these changes Nov 2, 2017

View reviewed changes

refack self-assigned this Nov 2, 2017

yhwang force-pushed the cctest-in-node-shared branch from f881e6a to 15be1b7 Compare November 2, 2017 20:28

mhdawson approved these changes Nov 2, 2017

View reviewed changes

cjihrig approved these changes Nov 2, 2017

View reviewed changes

gibfahn approved these changes Nov 2, 2017

View reviewed changes

gireeshpunathil approved these changes Nov 3, 2017

View reviewed changes

danbev approved these changes Nov 3, 2017

View reviewed changes

yhwang force-pushed the cctest-in-node-shared branch from 15be1b7 to b167b28 Compare November 10, 2017 20:23

refack approved these changes Nov 10, 2017

View reviewed changes

refack closed this Nov 10, 2017

yhwang deleted the cctest-in-node-shared branch November 10, 2017 22:13

evanlucas mentioned this pull request Nov 13, 2017

v9.2.0 proposal #16992

Merged

MylesBorins added backport-requested-v6.x labels Nov 17, 2017

yhwang mentioned this pull request Nov 22, 2017

[v6.x backport] build: remove cctest extension #17255

Closed

2 tasks

yhwang mentioned this pull request Nov 22, 2017

[v8.x backport] build: remove cctest extension #17258

Closed

2 tasks

MylesBorins added backported-to-v6.x and removed backport-requested-v6.x labels Nov 28, 2017

MylesBorins mentioned this pull request Nov 28, 2017

v6.12.1 proposal #17180

Merged

BethGriggs added backported-to-v8.x and removed backport-requested-v8.x labels Jun 20, 2018

refack removed their assignment Oct 12, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: remove cctest extension #16680

build: remove cctest extension #16680

yhwang commented Nov 2, 2017 •

edited

Loading

gibfahn commented Nov 2, 2017

bnoordhuis left a comment

yhwang commented Nov 2, 2017

jasnell commented Nov 2, 2017

yhwang commented Nov 2, 2017

yhwang commented Nov 2, 2017

mhdawson left a comment

mhdawson commented Nov 2, 2017

yhwang commented Nov 2, 2017

gibfahn commented Nov 2, 2017

yhwang commented Nov 7, 2017

yhwang commented Nov 10, 2017

refack commented Nov 10, 2017

refack commented Nov 10, 2017 •

edited

Loading

yhwang commented Nov 10, 2017

MylesBorins commented Nov 17, 2017

yhwang commented Nov 17, 2017

build: remove cctest extension #16680

build: remove cctest extension #16680

Conversation

yhwang commented Nov 2, 2017 • edited Loading

Checklist

Affected core subsystem(s)

gibfahn commented Nov 2, 2017

bnoordhuis left a comment

Choose a reason for hiding this comment

yhwang commented Nov 2, 2017

jasnell commented Nov 2, 2017

yhwang commented Nov 2, 2017

yhwang commented Nov 2, 2017

mhdawson left a comment

Choose a reason for hiding this comment

mhdawson commented Nov 2, 2017

yhwang commented Nov 2, 2017

gibfahn commented Nov 2, 2017

yhwang commented Nov 7, 2017

yhwang commented Nov 10, 2017

refack commented Nov 10, 2017

refack commented Nov 10, 2017 • edited Loading

yhwang commented Nov 10, 2017

MylesBorins commented Nov 17, 2017

yhwang commented Nov 17, 2017

yhwang commented Nov 2, 2017 •

edited

Loading

refack commented Nov 10, 2017 •

edited

Loading