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

deps: fix gypi sysroot settings on V8 #21494

Merged
merged 0 commits into from
Jun 25, 2018

Conversation

mmarchini
Copy link
Contributor

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: #21433

  • 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

@nodejs-github-bot nodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label Jun 24, 2018
@mmarchini
Copy link
Contributor Author

@mmarchini
Copy link
Contributor Author

cc @targos @nodejs/v8 @nodejs/v8-update

@ofrobots ofrobots added the fast-track PRs that do not need to wait for 48 hours to land. label Jun 25, 2018
@ofrobots
Copy link
Contributor

This is currently blocking 8.x backports, so it would be good if it could be landed soon. Adding fast-track label. /cc @nodejs/collaborators

@mmarchini
Copy link
Contributor Author

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

Let's see if we can get a green before landing.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

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

@mmarchini
Copy link
Contributor Author

Landed in fc45b16

@mmarchini mmarchini merged commit fc45b16 into nodejs:master Jun 25, 2018
mmarchini pushed a commit to mmarchini/node that referenced this pull request Jun 25, 2018
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>
@MylesBorins
Copy link
Contributor

marked as don't land for 6.x and 8.x... assuming that is the case based on the commit message. Please update if incorrect @mmarchini

targos pushed a commit that referenced this pull request Jun 26, 2018
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: #21433

PR-URL: #21494
Refs: #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>
@targos targos mentioned this pull request Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fast-track PRs that do not need to wait for 48 hours to land. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants