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.

Sign up for GitHub

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

build: introduce configure --shared #6994

Closed
wants to merge 9 commits into from
Closed

build: introduce configure --shared #6994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3f68d0e
build: configure --shared/--no-v8-platform
indutny Mar 27, 2016
eeeeeaf
build: additions/fixes for shared library support
stefanmb May 10, 2016
6d027f6
WIP: Add soname & fix make install
stefanmb May 31, 2016
66fd9d4
WIP: Fixes from review
stefanmb Jun 2, 2016
f9d4754
WIP: Remove trailing comma
stefanmb Jun 14, 2016
8f418f5
WIP: More review fixes
stefanmb Jun 24, 2016
38acb90
WIP: Review Fixes
stefanmb Jun 27, 2016
eeb315b
WIP: Added test & inverted logic
stefanmb Jun 28, 2016
53c7adc
WIP: Update error string
stefanmb Jun 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions common.gypi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
'msvs_multi_core_compile': '0', # we do enable multicore compiles, but not using the V8 way
'python%': 'python',

'node_shared%': 'false',
'node_use_v8_platform%': 'true',
'node_use_bundled_v8%': 'true',
'node_module_version%': '',

'node_tag%': '',
'uv_library%': 'static_library',

Expand Down Expand Up @@ -290,6 +295,9 @@
],
'ldflags!': [ '-rdynamic' ],
}],
[ 'node_shared=="true"', {
'cflags': [ '-fPIC' ],
}],
],
}],
[ 'OS=="android"', {
Expand Down
28 changes: 28 additions & 0 deletions configure
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ from gyp.common import GetFlavor
sys.path.insert(0, os.path.join(root_dir, 'tools', 'configure.d'))
import nodedownload

# imports in tools/
sys.path.insert(0, os.path.join(root_dir, 'tools'))
import getmoduleversion

# parse our options
parser = optparse.OptionParser()

Expand Down Expand Up @@ -420,6 +424,26 @@ parser.add_option('--without-inspector',
dest='without_inspector',
help='disable experimental V8 inspector support')

parser.add_option('--shared',
action='store_true',
dest='shared',
help='compile shared library for embedding node in another project. ' +
'(This mode is not officially supported for regular applications)')

parser.add_option('--without-v8-platform',
action='store_true',
dest='without_v8_platform',
default=False,
help='do not initialize v8 platform during node.js startup. ' +
'(This mode is not officially supported for regular applications)')

parser.add_option('--without-bundled-v8',
action='store_true',
dest='without_bundled_v8',
default=False,
help='do not use V8 includes from the bundled deps folder. ' +
'(This mode is not officially supported for regular applications)')

(options, args) = parser.parse_args()

# Expand ~ in the install prefix now, it gets written to multiple files.
Expand Down Expand Up @@ -810,6 +834,10 @@ def configure_node(o):
o['variables']['node_target_type'] = 'static_library'

o['variables']['node_no_browser_globals'] = b(options.no_browser_globals)
o['variables']['node_shared'] = b(options.shared)
o['variables']['node_use_v8_platform'] = b(not options.without_v8_platform)
o['variables']['node_use_bundled_v8'] = b(not options.without_bundled_v8)
o['variables']['node_module_version'] = int(getmoduleversion.get_version())

if options.linked_module:
o['variables']['library_files'] = options.linked_module
Expand Down
81 changes: 64 additions & 17 deletions node.gyp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
'node_use_etw%': 'false',
'node_use_perfctr%': 'false',
'node_no_browser_globals%': 'false',
'node_use_v8_platform%': 'true',
'node_use_bundled_v8%': 'true',
'node_shared%': 'false',
'node_module_version%': '',
'node_shared_zlib%': 'false',
'node_shared_http_parser%': 'false',
'node_shared_cares%': 'false',
Expand All @@ -14,7 +18,6 @@
'node_shared_openssl%': 'false',
'node_v8_options%': '',
'node_enable_v8_vtunejit%': 'false',
'node_target_type%': 'executable',
'node_core_target_name%': 'node',
'library_files': [
'lib/internal/bootstrap_node.js',
Expand Down Expand Up @@ -100,6 +103,13 @@
'deps/v8/tools/SourceMap.js',
'deps/v8/tools/tickprocessor-driver.js',
],
'conditions': [
[ 'node_shared=="true"', {
'node_target_type%': 'shared_library',
}, {
'node_target_type%': 'executable',
}],
Copy link
Member

@bnoordhuis bnoordhuis Jun 25, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

This breaks ./configure --static.

EDIT: Doesn't it?

],
},

'targets': [
Expand All @@ -109,16 +119,13 @@

'dependencies': [
'node_js2c#host',
'deps/v8/tools/gyp/v8.gyp:v8',
'deps/v8/tools/gyp/v8.gyp:v8_libplatform'
],

'include_dirs': [
'src',
'tools/msvs/genfiles',
'deps/uv/src/ares',
'<(SHARED_INTERMEDIATE_DIR)', # for node_natives.h
'deps/v8' # include/v8_platform.h
],

'sources': [
Expand Down Expand Up @@ -217,6 +224,42 @@


'conditions': [
[ 'node_shared=="false"', {
'msvs_settings': {
'VCManifestTool': {
'EmbedManifest': 'true',
'AdditionalManifestFiles': 'src/res/node.exe.extra.manifest'
}
},
}, {
'defines': [
'NODE_SHARED_MODE',
],
'conditions': [
[ 'node_module_version!=""', {
'product_extension': 'so.<(node_module_version)',
}]
],
}],
[ 'node_use_bundled_v8=="true"', {
'include_dirs': [
'deps/v8', # include/v8_platform.h
],

'dependencies': [
'deps/v8/tools/gyp/v8.gyp:v8',
'deps/v8/tools/gyp/v8.gyp:v8_libplatform'
],
}],
[ 'node_use_v8_platform=="true"', {
'defines': [
'NODE_USE_V8_PLATFORM=1',
],
}, {
'defines': [
'NODE_USE_V8_PLATFORM=0',
],
}],
[ 'node_tag!=""', {
'defines': [ 'NODE_TAG="<(node_tag)"' ],
}],
Expand Down Expand Up @@ -245,7 +288,8 @@
'defines': [ 'NODE_HAVE_SMALL_ICU=1' ],
}]],
}],
[ 'node_enable_v8_vtunejit=="true" and (target_arch=="x64" or \
[ 'node_use_bundled_v8=="true" and \
node_enable_v8_vtunejit=="true" and (target_arch=="x64" or \
target_arch=="ia32" or target_arch=="x32")', {
'defines': [ 'NODE_ENABLE_VTUNE_PROFILING' ],
'dependencies': [
Expand Down Expand Up @@ -308,7 +352,7 @@
],
},
'conditions': [
['OS in "linux freebsd"', {
['OS in "linux freebsd" and node_shared=="false"', {
'ldflags': [
'-Wl,--whole-archive <(PRODUCT_DIR)/<(OPENSSL_PRODUCT)',
'-Wl,--no-whole-archive',
Expand Down Expand Up @@ -395,7 +439,7 @@
[ 'node_no_browser_globals=="true"', {
'defines': [ 'NODE_NO_BROWSER_GLOBALS' ],
} ],
[ 'v8_postmortem_support=="true"', {
[ 'node_use_bundled_v8=="true" and v8_postmortem_support=="true"', {
'dependencies': [ 'deps/v8/tools/gyp/v8.gyp:postmortem-metadata' ],
'conditions': [
# -force_load is not applicable for the static library
Expand Down Expand Up @@ -478,7 +522,7 @@
'NODE_PLATFORM="sunos"',
],
}],
[ 'OS=="freebsd" or OS=="linux"', {
[ '(OS=="freebsd" or OS=="linux") and node_shared=="false"', {
'ldflags': [ '-Wl,-z,noexecstack',
'-Wl,--whole-archive <(V8_BASE)',
'-Wl,--no-whole-archive' ]
Expand All @@ -487,12 +531,6 @@
'ldflags': [ '-Wl,-M,/usr/lib/ld/map.noexstk' ],
}],
],
'msvs_settings': {
'VCManifestTool': {
'EmbedManifest': 'true',
'AdditionalManifestFiles': 'src/res/node.exe.extra.manifest'
}
},
},
# generate ETW header and resource files
{
Expand Down Expand Up @@ -718,8 +756,6 @@
'deps/http_parser/http_parser.gyp:http_parser',
'deps/gtest/gtest.gyp:gtest',
'deps/uv/uv.gyp:libuv',
'deps/v8/tools/gyp/v8.gyp:v8',
'deps/v8/tools/gyp/v8.gyp:v8_libplatform'
],
'include_dirs': [
'src',
Expand Down Expand Up @@ -750,7 +786,18 @@
'src/inspector_socket.cc',
'test/cctest/test_inspector_socket.cc'
]
}]
}],
[ 'node_use_v8_platform=="true"', {
'dependencies': [
'deps/v8/tools/gyp/v8.gyp:v8_libplatform',
],
}],
[ 'node_use_bundled_v8=="true"', {
'dependencies': [
'deps/v8/tools/gyp/v8.gyp:v8',
'deps/v8/tools/gyp/v8.gyp:v8_libplatform'
],
}],
]
}
], # end targets
Expand Down
51 changes: 43 additions & 8 deletions src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@
#include "string_bytes.h"
#include "util.h"
#include "uv.h"
#if NODE_USE_V8_PLATFORM
#include "libplatform/libplatform.h"
#endif // NODE_USE_V8_PLATFORM
#include "v8-debug.h"
#include "v8-profiler.h"
#include "zlib.h"
Expand Down Expand Up @@ -183,7 +185,42 @@ static uv_async_t dispatch_debug_messages_async;

static Mutex node_isolate_mutex;
static v8::Isolate* node_isolate;
static v8::Platform* default_platform;

static struct {
#if NODE_USE_V8_PLATFORM
void Initialize(int thread_pool_size) {
platform_ = v8::platform::CreateDefaultPlatform(thread_pool_size);
V8::InitializePlatform(platform_);
}

void PumpMessageLoop(Isolate* isolate) {
v8::platform::PumpMessageLoop(platform_, isolate);
}

void Dispose() {
delete platform_;
platform_ = nullptr;
}

#if HAVE_INSPECTOR
void StartInspector(Environment *env, int port, bool wait) {
env->inspector_agent()->Start(platform_, port, wait);
}
#endif // HAVE_INSPECTOR

v8::Platform* platform_;
#else // !NODE_USE_V8_PLATFORM
void Initialize(int thread_pool_size) {}
void PumpMessageLoop(Isolate* isolate) {}
void Dispose() {}
#if HAVE_INSPECTOR
void StartInspector(Environment *env, int port, bool wait) {
env->ThrowError("Node compiled with NODE_USE_V8_PLATFORM=0");
}
#endif // HAVE_INSPECTOR

#endif // !NODE_USE_V8_PLATFORM
} v8_platform;

#ifdef __POSIX__
static uv_sem_t debug_semaphore;
Expand Down Expand Up @@ -3652,7 +3689,7 @@ static void StartDebug(Environment* env, bool wait) {
CHECK(!debugger_running);
#if HAVE_INSPECTOR
if (use_inspector) {
env->inspector_agent()->Start(default_platform, inspector_port, wait);
v8_platform.StartInspector(env, inspector_port, wait);
debugger_running = true;
} else {
#endif
Expand Down Expand Up @@ -4299,11 +4336,11 @@ static void StartNodeInstance(void* arg) {
SealHandleScope seal(isolate);
bool more;
do {
v8::platform::PumpMessageLoop(default_platform, isolate);
v8_platform.PumpMessageLoop(isolate);
more = uv_run(env.event_loop(), UV_RUN_ONCE);

if (more == false) {
v8::platform::PumpMessageLoop(default_platform, isolate);
v8_platform.PumpMessageLoop(isolate);
EmitBeforeExit(&env);

// Emit `beforeExit` if the loop became alive either after emitting
Expand Down Expand Up @@ -4364,8 +4401,7 @@ int Start(int argc, char** argv) {
V8::SetEntropySource(crypto::EntropySource);
#endif

default_platform = v8::platform::CreateDefaultPlatform(v8_thread_pool_size);
V8::InitializePlatform(default_platform);
v8_platform.Initialize(v8_thread_pool_size);
V8::Initialize();

int exit_code = 1;
Expand All @@ -4382,8 +4418,7 @@ int Start(int argc, char** argv) {
}
V8::Dispose();

delete default_platform;
default_platform = nullptr;
v8_platform.Dispose();

delete[] exec_argv;
exec_argv = nullptr;
Expand Down
14 changes: 10 additions & 4 deletions src/node.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,17 +411,23 @@ extern "C" NODE_EXTERN void node_module_register(void* mod);
# define NODE_MODULE_EXPORT __attribute__((visibility("default")))
#endif

#ifdef NODE_SHARED_MODE
# define NODE_CTOR_PREFIX
#else
# define NODE_CTOR_PREFIX static
#endif
Copy link
Member

@bnoordhuis bnoordhuis Jun 15, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

One result of removing static (if I read the changes right), is that internal module constructors leak into the list of exported symbols. Or is that mitigated by -fvisibility=hidden?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bnoordhuis Correct, here is an example (notice uppercase T for export):

nm -a out/Release/lib.target/libnode.so.48 | grep _register_
...
0000000000622d90 T _register_async_wrap
0000000000622e10 T _register_buffer
0000000000622dd0 T _register_cares_wrap
0000000000622e30 T _register_config
0000000000622e50 T _register_contextify
0000000000623070 T _register_crypto
0000000000622e70 T _register_fs
0000000000622db0 T _register_fs_event_wrap
0000000000622e90 T _register_http_parser
0000000000622df0 T _register_js_stream
0000000000622eb0 T _register_os
0000000000622f30 T _register_pipe_wrap
0000000000623010 T _register_process_wrap
0000000000622f50 T _register_signal_wrap
0000000000622f70 T _register_spawn_sync
0000000000622f90 T _register_stream_wrap
0000000000622fb0 T _register_tcp_wrap
0000000000622fd0 T _register_timer_wrap
00000000006230d0 T _register_tls_wrap
0000000000622ff0 T _register_tty_wrap
0000000000623030 T _register_udp_wrap
0000000000622ed0 T _register_util
0000000000623050 T _register_uv
0000000000622ef0 T _register_v8
0000000000622f10 T _register_zlib

This change was in the original PR by Fedor. I believe he included it due to Electron, see here electron/node@0828dfa and the uses here electron/electron@69adff1


#if defined(_MSC_VER)
#pragma section(".CRT$XCU", read)
#define NODE_C_CTOR(fn) \
static void __cdecl fn(void); \
NODE_CTOR_PREFIX void __cdecl fn(void); \
__declspec(dllexport, allocate(".CRT$XCU")) \
void (__cdecl*fn ## _)(void) = fn; \
static void __cdecl fn(void)
NODE_CTOR_PREFIX void __cdecl fn(void)
#else
#define NODE_C_CTOR(fn) \
static void fn(void) __attribute__((constructor)); \
static void fn(void)
NODE_CTOR_PREFIX void fn(void) __attribute__((constructor)); \
NODE_CTOR_PREFIX void fn(void)
#endif

#define NODE_MODULE_X(modname, regfunc, priv, flags) \
Expand Down
10 changes: 10 additions & 0 deletions test/parallel/test-module-version.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
'use strict';
require('../common');
var assert = require('assert');

// check for existence
assert(process.config.variables.hasOwnProperty('node_module_version'));

// ensure that `node_module_version` is an Integer > 0
assert(Number.isInteger(process.config.variables.node_module_version));
assert(process.config.variables.node_module_version > 0);
Loading