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: introduce embedder version number for V8 #9754

Closed
wants to merge 3 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Nov 23, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

V8

Description of change

Refs: #9730

@targos targos added the v8 engine Issues and PRs related to the V8 dependency. label Nov 23, 2016
@targos
Copy link
Member Author

targos commented Nov 23, 2016

/cc @nodejs/v8

CheckVersion(2, 5, 10, 7, false, "2.5.10.7 SIMULATOR", "libv8-2.5.10.7.so");
CheckVersion(2, 5, 10, 7, true,
"2.5.10.7 (candidate) SIMULATOR", "libv8-2.5.10.7-candidate.so");
CheckVersion(1, 0, 0, 1, 0, false, "1.0.0.1.0 SIMULATOR", "libv8-1.0.0.1.0.so");
Copy link
Member Author

Choose a reason for hiding this comment

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

I know there are some long lines. Will fix them if we decide to go further

@bnoordhuis
Copy link
Member

LGTM modulo style issues. Can you add a line or two to the commit log explaining why this change is introduced?

versionCheck() in lib/internal/v8_prof_polyfill.js needs an update after this, you could add that as a separate commit.

It should handle both 6 and 7 tuple records because of ./configure --without-bundled-v8.

@MylesBorins
Copy link
Contributor

is this something we should backport? should we be using it for all patches
moving forward?

On Wed, Nov 23, 2016, 6:15 PM Ben Noordhuis notifications@github.com
wrote:

LGTM modulo style issues. Can you add a line or two to the commit log
explaining why this change is introduced?

versionCheck() in lib/internal/v8_prof_polyfill.js needs an update after
this, you could add that as a separate commit.

It should handle both 6 and 7 tuple records because of ./configure
--without-bundled-v8.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#9754 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAecV8DVuHHqPsufNOWa8Ow8yiZUJ6g0ks5rBBI1gaJpZM4K6Y1m
.

@rvagg
Copy link
Member

rvagg commented Nov 23, 2016

How about we just draw a line here and say from this point onward we're going to be bumping this number whenever we float stuff but leave previous branches as they are and patch how we have been. I can't think of why people would be parsing the full version number of V8 but you could argue it's a breaking change to extend it.

@targos targos added the semver-major PRs that contain breaking changes and should be released in the next major version. label Nov 23, 2016
@targos
Copy link
Member Author

targos commented Nov 23, 2016

Yeah I agree this is a major change

@rvagg
Copy link
Member

rvagg commented Nov 23, 2016

Major change but we'll need it in v7 for #9730 won't we or are we going to get by with a clash with that one?

@Fishrock123
Copy link
Contributor

Could this be upstreamed to V8?

@ofrobots
Copy link
Contributor

Yep, I would like to see this upstreamed too, once we are happy on our side /cc @natorion.

@addaleax
Copy link
Member

In that case, should the suffix maybe be something embedder-specified? Like, x.y.z.w.node1 for our versions?

@targos
Copy link
Member Author

targos commented Nov 27, 2016

Fixed the style issues and added a comment to explain the change.

versionCheck() in lib/internal/v8_prof_polyfill.js needs an update after this, you could add that as a separate commit.

I don't think it needs to be changed. The function only checks the first 3 numbers of the version string.

In that case, should the suffix maybe be something embedder-specified? Like, x.y.z.w.node1 for our versions?

I implemented it as a separate commit (so it's easy to revert). I had to remove embedder_ from base::hash_combine(major_, minor_, build_, patch_)); because there is no hash function for the char* type.
The version number looks like this now:

./node -p 'process.versions.v8'
5.4.500.43.node.0

@bnoordhuis
Copy link
Member

The function only checks the first 3 numbers of the version string.

Yes, but it also checks that line.split(',').length === 6.

@jasnell
Copy link
Member

jasnell commented Dec 5, 2016

Needs to be rebased and updated now!

@targos targos force-pushed the v8-version-embedder branch 2 times, most recently from 02626e3 to 98c6c02 Compare December 6, 2016 08:40
@targos
Copy link
Member Author

targos commented Dec 6, 2016

Updated, but the conflict confirmed something I feared. If we add this line in v8-version.h, we can no longer directly apply a patch from upstream.
So I put it below V8_IS_CANDIDATE_VERSION instead.

@@ -17,4 +17,7 @@
// (Boolean macro values are not supported by all preprocessors.)
#define V8_IS_CANDIDATE_VERSION 0

// Modify this when changes are made by the embedder.
#define V8_EMBEDDER_VERSION "node.0"
Copy link
Member

Choose a reason for hiding this comment

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

I think it's nicer to write this like this:

#ifndef V8_EMBEDDER_STRING
#define V8_EMBEDDER_STRING ""
#endif  // V8_EMBEDDER_STRING

And then override it at build time. That would also make it more likely for this patch to get accepted upstream.

Copy link
Member Author

Choose a reason for hiding this comment

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

override it at build time

You mean in node.gyp ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes.

Copy link
Member Author

Choose a reason for hiding this comment

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

I like the idea but I'm not sure how to update the other files. Should I add conditions everywhere to check if the embedder string is empty ?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not quite sure what you mean. You'd check that GetEmbedder() and V8_EMBEDDER_STRING are equal.

Just make it the responsibility of the embedder to include the dot: ['defines': 'V8_EMBEDDER_STRING=".node.0"'].

// whereas process.versions.v8 is either "$major.$minor.$build" or
// "$major.$minor.$build.$patch".
var firstLine = readline();
line = firstLine + '\n' + line;
firstLine = firstLine.split(',');
const curVer = process.versions.v8.split('.');
if (firstLine.length !== 6 && firstLine[0] !== 'v8-version') {
if (![6, 7].includes(firstLine.length) || firstLine[0] !== 'v8-version') {
Copy link
Member

Choose a reason for hiding this comment

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

This is IMO less legible than (firstLine.length !== 6 && firstLine.length !== 7).

@targos targos force-pushed the v8-version-embedder branch 2 times, most recently from 4192a60 to f21cdd1 Compare December 6, 2016 17:06
@@ -236,6 +236,7 @@
'NODE_WANT_INTERNALS=1',
# Warn when using deprecated V8 APIs.
'V8_DEPRECATION_WARNINGS=1',
'V8_EMBEDDER_STRING=".node.0"'
Copy link
Member Author

Choose a reason for hiding this comment

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

This doesn't seem to work. Should it be somewhere else ?

Copy link
Member

Choose a reason for hiding this comment

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

common.gypi or config.gypi? With common.gypi it's kind of gnarly that it ends up being defined everywhere, including add-ons.

@@ -56,9 +56,9 @@ void Log::Initialize(const char* log_file_name) {

if (output_handle_ != nullptr) {
Log::MessageBuilder msg(this);
msg.Append("v8-version,%d,%d,%d,%d,%d", Version::GetMajor(),
msg.Append("v8-version,%d,%d,%d,%d,%s,%d", Version::GetMajor(),
Copy link
Member Author

Choose a reason for hiding this comment

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

What should I do here ?

Copy link
Member

Choose a reason for hiding this comment

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

You mean print unconditionally or only when it's a non-empty string? The former is easier but really, it's up to you.

Sometimes upstream V8 may not want to merge a fix to their stable
branches, but we might. This adds a new version string that the embedder
can set independently of the official V8 version to avoid running
into conflicts.

Refs: nodejs#9730
Update the processor to accept a V8 with or without an embedder version
string.
@targos
Copy link
Member Author

targos commented Dec 7, 2016

I found a way to define it in common.gypi without polluting everything else.
I know I have to make a similar addition to BUILD.gn or something but I'd like some feedback on the approach, especially from @ofrobots because I'm not convinced it is the best way to go.
In the future, we hope to be able to maintain our copies of V8 in the nodejs/v8 repo. Aren't we going to directly add our version string there? In that case editing v8-version.h seems like the simplest action.

@targos
Copy link
Member Author

targos commented Dec 12, 2016

Ping ^

Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM. Completely untested but I expect the GN equivalent looks like this:

diff --git a/BUILD.gn b/BUILD.gn
index 71ebc5c..013e96f 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -26,6 +26,8 @@ declare_args() {
   # Enable compiler warnings when using V8_DEPRECATED apis.
   v8_deprecation_warnings = false
 
+  v8_embedder_string = ""
+
   # Enable compiler warnings when using V8_DEPRECATE_SOON apis.
   v8_imminent_deprecation_warnings = ""
 
@@ -171,6 +173,9 @@ config("features") {
 
   defines = []
 
+  if (v8_embedder_string != "") {
+    defines += [ "V8_EMBEDDER_STRING=\"$v8_embedder_string\"" ]
+  }
   if (v8_enable_disassembler) {
     defines += [ "ENABLE_DISASSEMBLER" ]
   }

@ofrobots
Copy link
Contributor

@natorion: any thoughts on the above change that adds embedder version to the V8 version string? This will solve one of the problems we have discussed in the past with Node.js needing to float a patch to V8 stable/beta that V8 is unwilling to merge upstream. It would be to good to get this change upstream.

It would be curious to see if the ecosystem is tightly dependent on the V8 version string format. Here's a CITGM run: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/474/

@natorion
Copy link

I will run this idea by our infra folks internally. Out of my head: This will break a few services in V8's infra, which we should prepare before this is upstreamed.

@ofrobots
Copy link
Contributor

@natorion I don't believe the default version format changes upstream. Embedders are able to add an embedder string to the version if they want to.

@natorion
Copy link

I create an upstream tracking bug: https://bugs.chromium.org/p/v8/issues/detail?id=5740.

kisg pushed a commit to paul99/v8mips that referenced this pull request Jan 10, 2017
Sometimes, the embedder might want to merge a fix to an abandoned branch
or to a supported branch but the fix is not relevant to Chromium.
This adds a new version string that the embedder can set on compile time
and that will be appended to the official V8 version.
The separator must be provided in the string. For instance, to have a
full version string like "5.5.372.37.custom.1", the embedder must set
V8_EMBEDDER_STRING to ".custom.1".

Related Node.js issue: nodejs/node#9754

BUG=v8:5740
R=machenbach@chromium.org,hablich@chromium.com,ofrobots@google.com

Review-Url: https://codereview.chromium.org/2619213002
Cr-Commit-Position: refs/heads/master@{#42175}
kisg pushed a commit to paul99/v8mips that referenced this pull request Jan 10, 2017
…id:20001 of https://codereview.chromium.org/2619213002/ )

Reason for revert:
Seems to break the Chromium build: https://codereview.chromium.org/2619193005/

Message:

[1832/9671] CXX obj/v8/v8_base/version.o
FAILED: obj/v8/v8_base/version.o
/b/c/cipd/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/v8/v8_base/version.o.d -DV8_DEPRECATION_WARNINGS -DDCHECK_ALWAYS_ON=1 -DUSE_UDEV -DUI_COMPOSITOR_IMAGE_TRANSPORT -DUSE_AURA=1 -DUSE_PANGO=1 -DUSE_CAIRO=1 -DUSE_GLIB=1 -DUSE_NSS_CERTS=1 -DUSE_X11=1 -DUSE_PROPRIETARY_CODECS -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=289944-2 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DV8_I18N_SUPPORT -DENABLE_HANDLE_ZAPPING -DV8_USE_EXTERNAL_STARTUP_DATA -DV8_TARGET_ARCH_X64 -DDEBUG -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -I../.. -Igen -I../../v8 -I../../v8/include -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -funwind-tables -fPIC -pipe -B../../third_party/binutils/Linux_x64/Release/bin -fcolor-diagnostics -fdebug-prefix-map=/b/c/b/linux/src=. -m64 -march=x86-64 -pthread -g1 --sysroot=../../build/linux/debian_wheezy_amd64-sysroot -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-ipc -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Werror -Wall -Wno-unused-variable -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-deprecated-register -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-shift-negative-value -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wsign-compare -Winconsistent-missing-override -Wshorten-64-to-32 -O3 -fno-ident -fdata-sections -ffunction-sections -fno-threadsafe-statics -fvisibility-inlines-hidden -std=gnu++11 -fno-rtti -fno-exceptions -Wno-deprecated -c ../../v8/src/version.cc -o obj/v8/v8_base/version.o
../../v8/src/version.cc:42:34: error: use of undeclared identifier 'V8_EMBEDDER_STRING'
const char* Version::embedder_ = V8_EMBEDDER_STRING;
                                 ^
1 error generated.

Original issue's description:
> [build] Introduce an embedder version string
>
> Sometimes, the embedder might want to merge a fix to an abandoned branch
> or to a supported branch but the fix is not relevant to Chromium.
> This adds a new version string that the embedder can set on compile time
> and that will be appended to the official V8 version.
> The separator must be provided in the string. For instance, to have a
> full version string like "5.5.372.37.custom.1", the embedder must set
> V8_EMBEDDER_STRING to ".custom.1".
>
> Related Node.js issue: nodejs/node#9754
>
> BUG=v8:5740
> R=machenbach@chromium.org,hablich@chromium.com,ofrobots@google.com
>
> Review-Url: https://codereview.chromium.org/2619213002
> Cr-Commit-Position: refs/heads/master@{#42175}
> Committed: https://chromium.googlesource.com/v8/v8/+/fc86d4329b253bf21c1dd85469f1ef4b6e5ba01a

TBR=hablich@chromium.com,machenbach@chromium.org,ofrobots@google.com,mic.besace@gmail.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5740

Review-Url: https://codereview.chromium.org/2621033002
Cr-Commit-Position: refs/heads/master@{#42182}
@targos
Copy link
Member Author

targos commented Jan 21, 2017

This moved to a V8 CL here: https://codereview.chromium.org/2619213002/
Closing.

@targos targos closed this Jan 21, 2017
@targos targos deleted the v8-version-embedder branch January 21, 2017 17:26
kisg pushed a commit to paul99/v8mips that referenced this pull request Jan 21, 2017
Sometimes, the embedder might want to merge a fix to an abandoned branch
or to a supported branch but the fix is not relevant to Chromium.
This adds a new version string that the embedder can set on compile time
and that will be appended to the official V8 version.
The separator must be provided in the string. For instance, to have a
full version string like "5.5.372.37.custom.1", the embedder must set
V8_EMBEDDER_STRING to ".custom.1".

Related Node.js issue: nodejs/node#9754

BUG=v8:5740
R=machenbach@chromium.org,hablich@chromium.com,ofrobots@google.com

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng

Review-Url: https://codereview.chromium.org/2619213002
Cr-Original-Commit-Position: refs/heads/master@{#42175}
Committed: https://chromium.googlesource.com/v8/v8/+/fc86d4329b253bf21c1dd85469f1ef4b6e5ba01a
Review-Url: https://codereview.chromium.org/2619213002
Cr-Commit-Position: refs/heads/master@{#42582}
kisg pushed a commit to paul99/v8mips that referenced this pull request Jan 21, 2017
…id:40001 of https://codereview.chromium.org/2619213002/ )

Reason for revert:
Blocks roll https://codereview.chromium.org/2647183002/

Original issue's description:
> [build] Introduce an embedder version string
>
> Sometimes, the embedder might want to merge a fix to an abandoned branch
> or to a supported branch but the fix is not relevant to Chromium.
> This adds a new version string that the embedder can set on compile time
> and that will be appended to the official V8 version.
> The separator must be provided in the string. For instance, to have a
> full version string like "5.5.372.37.custom.1", the embedder must set
> V8_EMBEDDER_STRING to ".custom.1".
>
> Related Node.js issue: nodejs/node#9754
>
> BUG=v8:5740
> R=machenbach@chromium.org,hablich@chromium.com,ofrobots@google.com
>
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng
>
> Review-Url: https://codereview.chromium.org/2619213002
> Cr-Original-Commit-Position: refs/heads/master@{#42175}
> Committed: https://chromium.googlesource.com/v8/v8/+/fc86d4329b253bf21c1dd85469f1ef4b6e5ba01a
> Review-Url: https://codereview.chromium.org/2619213002
> Cr-Commit-Position: refs/heads/master@{#42582}
> Committed: https://chromium.googlesource.com/v8/v8/+/2c1d1e60883882011ed50310a9b09e95dc61234a

TBR=hablich@chromium.com,machenbach@chromium.org,ofrobots@google.com,mic.besace@gmail.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5740

Review-Url: https://codereview.chromium.org/2643393004
Cr-Commit-Position: refs/heads/master@{#42583}
kisg pushed a commit to paul99/v8mips that referenced this pull request Oct 5, 2017
Sometimes, the embedder might want to merge a fix to an abandoned branch
or to a supported branch but the fix is not relevant to Chromium.
This adds a new version string that the embedder can set at compile time
and that will be appended to the official V8 version.
The separator must be provided in the string. For instance, to have a
full version string like "6.0.287.53-emb.1", the embedder must set
V8_EMBEDDER_STRING to "-emb.1".

Related Node.js issue: nodejs/node#9754

BUG=v8:5740
R=machenbach@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: Ifa2d9bd213795e6d54886436f8c3787ac6162823
Reviewed-on: https://chromium-review.googlesource.com/690475
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
Cr-Commit-Position: refs/heads/master@{#48301}
targos added a commit to targos/node that referenced this pull request Oct 5, 2017
Original commit message:

    [build] Introduce an embedder version string

    Sometimes, the embedder might want to merge a fix to an abandoned branch
    or to a supported branch but the fix is not relevant to Chromium.
    This adds a new version string that the embedder can set at compile time
    and that will be appended to the official V8 version.
    The separator must be provided in the string. For instance, to have a
    full version string like "6.0.287.53-emb.1", the embedder must set
    V8_EMBEDDER_STRING to "-emb.1".

    Related Node.js issue: nodejs#9754

    BUG=v8:5740
    R=machenbach@chromium.org

    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ifa2d9bd213795e6d54886436f8c3787ac6162823
    Reviewed-on: https://chromium-review.googlesource.com/690475
    Reviewed-by: Michael Achenbach <machenbach@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{nodejs#48301}
targos added a commit that referenced this pull request Oct 18, 2017
Original commit message:

    [build] Introduce an embedder version string

    Sometimes, the embedder might want to merge a fix to an abandoned branch
    or to a supported branch but the fix is not relevant to Chromium.
    This adds a new version string that the embedder can set at compile time
    and that will be appended to the official V8 version.
    The separator must be provided in the string. For instance, to have a
    full version string like "6.0.287.53-emb.1", the embedder must set
    V8_EMBEDDER_STRING to "-emb.1".

    Related Node.js issue: #9754

    BUG=v8:5740
    R=machenbach@chromium.org

    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ifa2d9bd213795e6d54886436f8c3787ac6162823
    Reviewed-on: https://chromium-review.googlesource.com/690475
    Reviewed-by: Michael Achenbach <machenbach@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#48301}

Refs: v8/v8@b096c44
PR-URL: #15785
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
targos added a commit to targos/node that referenced this pull request Oct 18, 2017
Original commit message:

    [build] Introduce an embedder version string

    Sometimes, the embedder might want to merge a fix to an abandoned branch
    or to a supported branch but the fix is not relevant to Chromium.
    This adds a new version string that the embedder can set at compile time
    and that will be appended to the official V8 version.
    The separator must be provided in the string. For instance, to have a
    full version string like "6.0.287.53-emb.1", the embedder must set
    V8_EMBEDDER_STRING to "-emb.1".

    Related Node.js issue: nodejs#9754

    BUG=v8:5740
    R=machenbach@chromium.org

    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ifa2d9bd213795e6d54886436f8c3787ac6162823
    Reviewed-on: https://chromium-review.googlesource.com/690475
    Reviewed-by: Michael Achenbach <machenbach@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{nodejs#48301}

Refs: v8/v8@b096c44
PR-URL: nodejs#15785
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
targos added a commit that referenced this pull request Oct 19, 2017
Original commit message:

    [build] Introduce an embedder version string

    Sometimes, the embedder might want to merge a fix to an abandoned branch
    or to a supported branch but the fix is not relevant to Chromium.
    This adds a new version string that the embedder can set at compile time
    and that will be appended to the official V8 version.
    The separator must be provided in the string. For instance, to have a
    full version string like "6.0.287.53-emb.1", the embedder must set
    V8_EMBEDDER_STRING to "-emb.1".

    Related Node.js issue: #9754

    BUG=v8:5740
    R=machenbach@chromium.org

    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ifa2d9bd213795e6d54886436f8c3787ac6162823
    Reviewed-on: https://chromium-review.googlesource.com/690475
    Reviewed-by: Michael Achenbach <machenbach@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#48301}

Refs: v8/v8@b096c44
PR-URL: #15785
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 26, 2017
Original commit message:

    [build] Introduce an embedder version string

    Sometimes, the embedder might want to merge a fix to an abandoned branch
    or to a supported branch but the fix is not relevant to Chromium.
    This adds a new version string that the embedder can set at compile time
    and that will be appended to the official V8 version.
    The separator must be provided in the string. For instance, to have a
    full version string like "6.0.287.53-emb.1", the embedder must set
    V8_EMBEDDER_STRING to "-emb.1".

    Related Node.js issue: nodejs/node#9754

    BUG=v8:5740
    R=machenbach@chromium.org

    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ifa2d9bd213795e6d54886436f8c3787ac6162823
    Reviewed-on: https://chromium-review.googlesource.com/690475
    Reviewed-by: Michael Achenbach <machenbach@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#48301}

Refs: v8/v8@b096c44
PR-URL: nodejs/node#15785
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 26, 2017
Original commit message:

    [build] Introduce an embedder version string

    Sometimes, the embedder might want to merge a fix to an abandoned branch
    or to a supported branch but the fix is not relevant to Chromium.
    This adds a new version string that the embedder can set at compile time
    and that will be appended to the official V8 version.
    The separator must be provided in the string. For instance, to have a
    full version string like "6.0.287.53-emb.1", the embedder must set
    V8_EMBEDDER_STRING to "-emb.1".

    Related Node.js issue: nodejs/node#9754

    BUG=v8:5740
    R=machenbach@chromium.org

    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ifa2d9bd213795e6d54886436f8c3787ac6162823
    Reviewed-on: https://chromium-review.googlesource.com/690475
    Reviewed-by: Michael Achenbach <machenbach@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#48301}

Refs: v8/v8@b096c44
PR-URL: nodejs/node#15785
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
Original commit message:

    [build] Introduce an embedder version string

    Sometimes, the embedder might want to merge a fix to an abandoned branch
    or to a supported branch but the fix is not relevant to Chromium.
    This adds a new version string that the embedder can set at compile time
    and that will be appended to the official V8 version.
    The separator must be provided in the string. For instance, to have a
    full version string like "6.0.287.53-emb.1", the embedder must set
    V8_EMBEDDER_STRING to "-emb.1".

    Related Node.js issue: nodejs/node#9754

    BUG=v8:5740
    R=machenbach@chromium.org

    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ifa2d9bd213795e6d54886436f8c3787ac6162823
    Reviewed-on: https://chromium-review.googlesource.com/690475
    Reviewed-by: Michael Achenbach <machenbach@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#48301}

Refs: v8/v8@b096c44
PR-URL: nodejs/node#15785
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
Original commit message:

    [build] Introduce an embedder version string

    Sometimes, the embedder might want to merge a fix to an abandoned branch
    or to a supported branch but the fix is not relevant to Chromium.
    This adds a new version string that the embedder can set at compile time
    and that will be appended to the official V8 version.
    The separator must be provided in the string. For instance, to have a
    full version string like "6.0.287.53-emb.1", the embedder must set
    V8_EMBEDDER_STRING to "-emb.1".

    Related Node.js issue: nodejs/node#9754

    BUG=v8:5740
    R=machenbach@chromium.org

    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ifa2d9bd213795e6d54886436f8c3787ac6162823
    Reviewed-on: https://chromium-review.googlesource.com/690475
    Reviewed-by: Michael Achenbach <machenbach@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#48301}

Refs: v8/v8@b096c44
PR-URL: nodejs/node#15785
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-major PRs that contain breaking changes and should be released in the next major version. 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