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

[v8.x] Cherry-pick fix for make test-v8 from master #21534

Closed
wants to merge 1 commit into from

Conversation

mmarchini
Copy link
Contributor

Cherry-picking fc45b16 from master to unbreak V8 CI on v8.x-staging.

Closes: #21433

cc @nodejs/release @nodejs/v8-update @targos @ofrobots @hashseed

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

On Node.js v8.x, gn will pass a sysroot parameter to clang to use a
downloaded sysroot files while running `make test-v8`. Recently,
chromium build tools switched to use Debian sid sysroot files instead of
Debian jessie. This patch updates our V8 GYP files to conform with those
changes.

Ref: nodejs#21433

PR-URL: nodejs#21494
Refs: nodejs#21433
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@nodejs-github-bot nodejs-github-bot added v8 engine Issues and PRs related to the V8 dependency. v8.x labels Jun 25, 2018
@mmarchini
Copy link
Contributor Author

@mmarchini
Copy link
Contributor Author

Leave a 👍 in this comment if you agree this should be fast-tracked

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@mhdawson
Copy link
Member

mhdawson commented Jun 25, 2018

I see failures on PPC and s390 related to

18:33:26 make[2]: *** No rule to make target `/home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/deps/v8/out/ppc64.release/obj.target/gmock/testing/gmock/src/gmock-cardinalities.o', needed by `/home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/deps/v8/out/ppc64.release/obj.target/testing/libgmock.a'.  Stop.

I don't think those should block this PR as the failure is unrelated to both the original problem or the change. @mmallick-ca could you give us an update on where our investigation is into the gmock issue for PPC and s390 ?

@mmarchini
Copy link
Contributor Author

Feel free to land once this has enough approvals. I'll be traveling and won't be able to do so before Wednesday.

@mhdawson
Copy link
Member

@mmarchini For some reason it also failed on the x86 machines which is of concern. Not sure why it did not fail that way in earlier tests?

@mmarchini
Copy link
Contributor Author

Definitely looks related 😕

@mmarchini
Copy link
Contributor Author

Running again after cleaning up the workspace on ubuntu1604-intel-64: https://ci.nodejs.org/job/node-test-commit-v8-linux/1487/

Hope this fixes the issue.

@mmallick-ca
Copy link

@mhdawson we think its failing because git is not able to complete the checkout . This is why the gmock files are not present. Currently testing a potential fix.

@ofrobots
Copy link
Contributor

@mmallick-ca any updates?

@mmallick-ca
Copy link

@ofrobots The gitattributes file associated with the v8 master branch has been updated to treat files with *.png as binary. We have verified locally that after this change that build completes successfully. The error was occurring due to git treating certain *.png files in the master branch as text files during "fetch v8". This results in some modifications in the png files such that the subsequent checkout of the appropriate branch fails.

@ofrobots
Copy link
Contributor

@ofrobots
Copy link
Contributor

Failed on x86 with

error: /home/iojs/build/workspace/node-test-commit-v8-linux/nodes/benchmark/v8test/v8test/deps/v8/out/x64.release/mksnapshot: symbol _ZTVNSt7__cxx1115basic_stringbufIcSt11char_traitsIcESaIcEEE, version GLIBCXX_3.4.21 not defined in file libstdc++.so.6 with link time reference

@mmarchini
Copy link
Contributor Author

@ofrobots I'm still trying to figure out why it's failing on x86.

@mmarchini
Copy link
Contributor Author

mmarchini commented Jun 28, 2018

Looks like something changed upstream again, which fixed our V8 CI, and now this PR is breaking it ¯_(ツ)_/¯

V8 CI passing on v8.x-staging: https://ci.nodejs.org/job/node-test-commit-v8-linux/1493/

@ofrobots
Copy link
Contributor

/cc @hashseed, fyi upstream changes that have been causing churn here.

@mmarchini mmarchini added the blocked PRs that are blocked by other issues or PRs. label Jun 30, 2018
@mmarchini
Copy link
Contributor Author

I'll close this and revert #21494. I couldn't find what changed upstream, but the nodes=benchmark test is not passing and apparently node-test-commit-v8-linux is working again.

@mmarchini mmarchini closed this Jun 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked PRs that are blocked by other issues or PRs. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants