X Tutup
The Wayback Machine - https://web.archive.org/web/20230216210929/https://github.com/nodejs/node/pull/38263
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

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

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
Contributor

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

@nodejs-github-bot
Copy link
Contributor

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

@nodejs-github-bot
Copy link
Contributor

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

@nodejs-github-bot
Copy link
Contributor

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

targos
targos approved these changes Apr 16, 2021
View changes
@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
Trott
Trott approved these changes Apr 17, 2021
View changes
@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
14 checks passed
@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
@targos targos added the dont-land-on-v14.x PRs that should not land on the v14.x-staging branch and should not be released in v14.x. label May 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell

@Trott Trott

@targos targos

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. dont-land-on-v14.x PRs that should not land on the v14.x-staging branch and should not be released in v14.x. 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.

None yet
5 participants
@richardlau @nodejs-github-bot @jasnell @Trott @targos
X Tutup