Skip to content

Commit

Permalink
build: add V8 embedder version string
Browse files Browse the repository at this point in the history
After this commit, `process.versions.v8` will look like:
"6.0.287.53-node.0".
The goal is that everytime we apply a non-official patch to `deps/v8`,
we increment our own number instead of V8's patch level.
This number must be set back to 0 after major V8 updates.

Fixes: nodejs#15698
PR-URL: nodejs#15785
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
  • Loading branch information
targos committed Oct 18, 2017
1 parent acb9b8f commit 70832bc
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 8 deletions.
4 changes: 4 additions & 0 deletions common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
# Default to -O0 for debug builds.
'v8_optimized_debug%': 0,

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.0',

# Enable disassembler for `--print-code` v8 options
'v8_enable_disassembler': 1,

Expand Down
5 changes: 4 additions & 1 deletion doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -1790,6 +1790,9 @@ changes:
- version: v4.2.0
pr-url: https://github.com/nodejs/node/pull/3102
description: The `icu` property is now supported.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/15785
description: The `v8` property now includes a Node.js specific suffix.
-->

* {Object}
Expand All @@ -1810,7 +1813,7 @@ Will generate an object similar to:
{
http_parser: '2.3.0',
node: '1.1.1',
v8: '4.1.0.14',
v8: '6.1.534.42-node.0',
uv: '1.3.0',
zlib: '1.2.8',
ares: '1.10.0-DEV',
Expand Down
4 changes: 3 additions & 1 deletion doc/guides/maintaining-V8.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ to be cherry-picked in the Node.js repository and V8-CI must test the change.

* For each abandoned V8 branch corresponding to an LTS branch that is affected by the bug:
* Open a cherry-pick PR on nodejs/node targeting the appropriate *vY.x-staging* branch (e.g. *v6.x-staging* to fix an issue in V8-5.1).
* Increase the patch level version in v8-version.h. This will not cause any problems with versioning because V8 will not publish other patches for this branch, so Node.js can effectively bump the patch version.
* On Node.js < 9.0.0: Increase the patch level version in v8-version.h. This will not cause any problems with versioning because V8 will not publish other patches for this branch, so Node.js can effectively bump the patch version.
* On Node.js >= 9.0.0: Increase the `v8_embedder_string` number in `common.gypi`.
* In some cases the patch may require extra effort to merge in case V8 has changed substantially. For important issues we may be able to lean on the V8 team to get help with reimplementing the patch.
* Run the Node.js [V8-CI](https://ci.nodejs.org/job/node-test-commit-v8-linux/) in addition to the [Node.js CI](https://ci.nodejs.org/job/node-test-pull-request/).

Expand Down Expand Up @@ -265,6 +266,7 @@ above. A better strategy is to

1. Audit the current master branch and look at the patches that have been floated since the last major V8 update.
1. Replace the copy of V8 in Node.js with a fresh checkout of the latest stable V8 branch. Special care must be taken to recursively update the DEPS that V8 has a compile time dependency on (at the moment of this writing, these are only trace_event and gtest_prod.h)
1. Reset the `v8_embedder_string` variable to "-node.0" in `common.gypi`.
1. Refloat (cherry-pick) all the patches from list computed in 1) as necessary. Some of the patches may no longer be necessary.

To audit for floating patches:
Expand Down
12 changes: 7 additions & 5 deletions lib/internal/v8_prof_polyfill.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,16 @@ function readline() {
}

function versionCheck() {
// v8-version looks like "v8-version,$major,$minor,$build,$patch,$candidate"
// whereas process.versions.v8 is either "$major.$minor.$build" or
// "$major.$minor.$build.$patch".
// v8-version looks like
// "v8-version,$major,$minor,$build,$patch[,$embedder],$candidate"
// whereas process.versions.v8 is either "$major.$minor.$build-$embedder" or
// "$major.$minor.$build.$patch-$embedder".
var firstLine = readline();
line = firstLine + '\n' + line;
firstLine = firstLine.split(',');
const curVer = process.versions.v8.split('.');
if (firstLine.length !== 6 && firstLine[0] !== 'v8-version') {
const curVer = process.versions.v8.split(/\.-/);
if (firstLine.length !== 6 && firstLine.length !== 7 ||
firstLine[0] !== 'v8-version') {
console.log('Unable to read v8-version from log file.');
return;
}
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-process-versions.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ assert(commonTemplate.test(process.versions.node));
assert(commonTemplate.test(process.versions.uv));
assert(commonTemplate.test(process.versions.zlib));

assert(/^\d+\.\d+\.\d+(?:\.\d+)?(?: \(candidate\))?$/
assert(/^\d+\.\d+\.\d+(?:\.\d+)?-node\.\d+(?: \(candidate\))?$/
.test(process.versions.v8));
assert(/^\d+$/.test(process.versions.modules));

Expand Down

0 comments on commit 70832bc

Please sign in to comment.