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.

Sign up for GitHub

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

Jump to bottom

build: sync generation of v8_build_config.json #38263

Merged
merged 1 commit into from
Apr 18, 2021
Merged

build: sync generation of v8_build_config.json #38263

merged 1 commit into from
Apr 18, 2021

Conversation

richardlau
Copy link
Member

@richardlau richardlau commented Apr 16, 2021 •
edited
Loading

The contents of the v8_build_config.json file generated when
configured with --enable-d8 is missing entries which prevent V8's
test runner from using the built d8 binary. Sync the keys written
into the file with those generated in V8's deps/v8/BUILD.gn file.

Prior to this patch V8's test runner would fail to parse v8_build_config.json
as it was missing v8_control_flow_integrity:

$ python2 deps/v8/tools/run-tests.py --outdir ../../out/Release --progress=dots --timeout=120 mjsunit debugger message preparser
Traceback (most recent call last):
  File "/home/rlau/sandbox/github/node/deps/v8/tools/testrunner/base_runner.py", line 283, in execute
    self._load_build_config(options)
  File "/home/rlau/sandbox/github/node/deps/v8/tools/testrunner/base_runner.py", line 428, in _load_build_config
    self.build_config = _do_load_build_config(outdir, options.verbose)
  File "/home/rlau/sandbox/github/node/deps/v8/tools/testrunner/base_runner.py", line 253, in _do_load_build_config
    return BuildConfig(build_config_json)
  File "/home/rlau/sandbox/github/node/deps/v8/tools/testrunner/base_runner.py", line 173, in __init__
    self.control_flow_integrity = build_config['v8_control_flow_integrity']
KeyError: 'v8_control_flow_integrity'

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Apr 16, 2021
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@richardlau richardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 17, 2021
@richardlau
build: sync generation of v8_build_config.json
049b703 
The contents of the `v8_build_config.json` file generated when
configured with `--enable-d8` is missing entries which prevent V8's
test runner from using the built `d8` binary. Sync the keys written
into the file with those generated in V8's `deps/v8/BUILD.gn` file.

PR-URL: nodejs#38263
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@richardlau richardlau merged commit 049b703 into nodejs:master Apr 18, 2021
@richardlau
Copy link
Member Author

Landed in 049b703.

@richardlau richardlau deleted the d8 branch April 18, 2021 20:35
@BethGriggs BethGriggs mentioned this pull request Apr 20, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@Trott Trott Trott approved these changes

@targos targos targos approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@richardlau @nodejs-github-bot @jasnell @Trott @targos