From 70832bc3538d8b4e1c67ce8c49617c6e06fe3187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Fri, 29 Sep 2017 13:39:26 +0200 Subject: [PATCH] build: add V8 embedder version string 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: https://github.com/nodejs/node/issues/15698 PR-URL: https://github.com/nodejs/node/pull/15785 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Ben Noordhuis Reviewed-By: Gibson Fahnestock Reviewed-By: Ali Ijaz Sheikh Reviewed-By: James M Snell Reviewed-By: Michael Dawson --- common.gypi | 4 ++++ doc/api/process.md | 5 ++++- doc/guides/maintaining-V8.md | 4 +++- lib/internal/v8_prof_polyfill.js | 12 +++++++----- test/parallel/test-process-versions.js | 2 +- 5 files changed, 19 insertions(+), 8 deletions(-) diff --git a/common.gypi b/common.gypi index 98268068f94..598ba76d55e 100644 --- a/common.gypi +++ b/common.gypi @@ -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, diff --git a/doc/api/process.md b/doc/api/process.md index 8b8ce0f43dd..ca5f13ed561 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -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} @@ -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', diff --git a/doc/guides/maintaining-V8.md b/doc/guides/maintaining-V8.md index dd5416677ee..73ba14bf53a 100644 --- a/doc/guides/maintaining-V8.md +++ b/doc/guides/maintaining-V8.md @@ -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/). @@ -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: diff --git a/lib/internal/v8_prof_polyfill.js b/lib/internal/v8_prof_polyfill.js index e77f1f76bcb..1c9ba4ebc54 100644 --- a/lib/internal/v8_prof_polyfill.js +++ b/lib/internal/v8_prof_polyfill.js @@ -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; } diff --git a/test/parallel/test-process-versions.js b/test/parallel/test-process-versions.js index 65634ece6ce..5644ac4658b 100644 --- a/test/parallel/test-process-versions.js +++ b/test/parallel/test-process-versions.js @@ -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));