Skip to content

Commit

Permalink
build: add option to build v8 with GN
Browse files Browse the repository at this point in the history
PR-URL: #19201
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
hashseed authored and MylesBorins committed Apr 11, 2018
1 parent dd49677 commit 5c93b3b
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 7 deletions.
15 changes: 12 additions & 3 deletions common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@
'obj_dir': '<(PRODUCT_DIR)/obj',
'v8_base': '<(PRODUCT_DIR)/obj/deps/v8/gypfiles/libv8_base.a',
}, {
'obj_dir%': '<(PRODUCT_DIR)/obj.target',
'v8_base%': '<(PRODUCT_DIR)/obj.target/deps/v8/gypfiles/libv8_base.a',
'obj_dir%': '<(PRODUCT_DIR)/obj.target',
'v8_base': '<(PRODUCT_DIR)/obj.target/deps/v8/gypfiles/libv8_base.a',
}],
['OS == "win"', {
'os_posix': 0,
Expand All @@ -63,10 +63,19 @@
'os_posix': 1,
'v8_postmortem_support%': 'true',
}],
['OS== "mac"', {
['OS == "mac"', {
'obj_dir%': '<(PRODUCT_DIR)/obj.target',
'v8_base': '<(PRODUCT_DIR)/libv8_base.a',
}],
['build_v8_with_gn == "true"', {
'conditions': [
['GENERATOR == "ninja"', {
'v8_base': '<(PRODUCT_DIR)/obj/deps/v8/gypfiles/v8_monolith.gen/gn/obj/libv8_monolith.a',
}, {
'v8_base': '<(PRODUCT_DIR)/obji.target/v8_monolith/geni/gn/obj/libv8_monolith.a',
}],
],
}],
['openssl_fips != ""', {
'openssl_product': '<(STATIC_LIB_PREFIX)crypto<(STATIC_LIB_SUFFIX)',
}, {
Expand Down
18 changes: 18 additions & 0 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ sys.path.insert(0, 'tools')
import getmoduleversion
from gyp_node import run_gyp

# imports in deps/v8/tools/node
sys.path.insert(0, os.path.join('deps', 'v8', 'tools', 'node'))
from fetch_deps import FetchDeps

# parse our options
parser = optparse.OptionParser()

Expand Down Expand Up @@ -548,6 +552,12 @@ parser.add_option('--without-bundled-v8',
help='do not use V8 includes from the bundled deps folder. ' +
'(This mode is not officially supported for regular applications)')

parser.add_option('--build-v8-with-gn',
action='store_true',
dest='build_v8_with_gn',
default=False,
help='build V8 using GN instead of gyp')

# Create compile_commands.json in out/Debug and out/Release.
parser.add_option('-C',
action='store_true',
Expand Down Expand Up @@ -1060,6 +1070,14 @@ def configure_v8(o):
o['variables']['test_isolation_mode'] = 'noop' # Needed by d8.gyp.
if options.without_bundled_v8 and options.enable_d8:
raise Exception('--enable-d8 is incompatible with --without-bundled-v8.')
if options.without_bundled_v8 and options.build_v8_with_gn:
raise Exception(
'--build-v8-with-gn is incompatible with --without-bundled-v8.')
if options.build_v8_with_gn:
v8_path = os.path.join('deps', 'v8')
print('Fetching dependencies to build V8 with GN')
options.build_v8_with_gn = FetchDeps(v8_path)
o['variables']['build_v8_with_gn'] = b(options.build_v8_with_gn)


def configure_openssl(o):
Expand Down
17 changes: 13 additions & 4 deletions node.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,15 @@
'dependencies': [ 'deps/v8/gypfiles/d8.gyp:d8' ],
}],
[ 'node_use_bundled_v8=="true"', {
'dependencies': [
'deps/v8/gypfiles/v8.gyp:v8',
'deps/v8/gypfiles/v8.gyp:v8_libplatform'
'conditions': [
[ 'build_v8_with_gn=="true"', {
'dependencies': ['deps/v8/gypfiles/v8.gyp:v8_monolith'],
}, {
'dependencies': [
'deps/v8/gypfiles/v8.gyp:v8',
'deps/v8/gypfiles/v8.gyp:v8_libplatform',
],
}],
],
}],
[ 'node_use_v8_platform=="true"', {
Expand Down Expand Up @@ -111,7 +117,6 @@
'defines': [ 'NODE_NO_BROWSER_GLOBALS' ],
} ],
[ 'node_use_bundled_v8=="true" and v8_postmortem_support=="true"', {
'dependencies': [ 'deps/v8/gypfiles/v8.gyp:postmortem-metadata' ],
'conditions': [
# -force_load is not applicable for the static library
[ 'force_load=="true"', {
Expand All @@ -121,6 +126,10 @@
],
},
}],
# when building with GN, the v8_monolith target already includes postmortem metadata
[ 'build_v8_with_gn=="false"', {
'dependencies': [ 'deps/v8/gypfiles/v8.gyp:postmortem-metadata' ],
}],
],
}],
[ 'node_shared_zlib=="false"', {
Expand Down

2 comments on commit 5c93b3b

@refack
Copy link
Contributor

@refack refack commented on 5c93b3b Apr 13, 2018

Choose a reason for hiding this comment

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

IMHO this should have been a separate PR and not slip-streamed into a V8 bump.

So in case you missed this, @nodejs/build @nodejs/python

@hashseed, AFAICT for now this is to enable the V8 team to run integration tests on their CI (not necessarily V8 6.6), correct?

@hashseed
Copy link
Member Author

Choose a reason for hiding this comment

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

Right. V8 no longer maintains gyp files, and we need to use GN to test Node.js against the newest V8.

Please sign in to comment.