Skip to content

Commit

Permalink
build: place N-API-related changes behind a configure/runtime flag
Browse files Browse the repository at this point in the history 
Currently support for N-API modules is on by default both at compile
time and at runtime.

Fixes #80
Closes #89
  • Loading branch information
Gabriel Schulhof committed Feb 23, 2017
1 parent 30da58a commit 26adcc2
Show file tree
Hide file tree
Showing 14 changed files with 140 additions and 158 deletions.
47 changes: 33 additions & 14 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ FLAKY_TESTS ?= run
TEST_CI_ARGS ?=
STAGINGSERVER ?= node-www
LOGLEVEL ?= silent
NAPI ?= True
OSTYPE := $(shell uname -s | tr '[A-Z]' '[a-z]')

ifdef JOBS
Expand Down Expand Up @@ -120,12 +121,17 @@ v8:
tools/make-v8.sh
$(MAKE) -C deps/v8 $(V8_ARCH).$(BUILDTYPE_LOWER) $(V8_BUILD_OPTIONS)

ifeq ($(NAPI),True)
BUILD_ADDONS = build-addons build-addons-abi
TESTS_TO_RUN = addons addons-abi doctool inspector known_issues message pseudo-tty parallel sequential
else
BUILD_ADDONS = build-addons
TESTS_TO_RUN = addons doctool inspector known_issues message pseudo-tty parallel sequential
endif

test: all
$(MAKE) build-addons
$(MAKE) build-addons-abi
$(MAKE) cctest
$(PYTHON) tools/test.py --mode=release -J \
addons addons-abi doctool inspector known_issues message pseudo-tty parallel sequential
$(MAKE) $(BUILD_ADDONS) cctest
$(PYTHON) tools/test.py --mode=release -J $(TESTS_TO_RUN)
$(MAKE) lint

test-parallel: all
Expand Down Expand Up @@ -189,6 +195,8 @@ test/addons/.buildstamp: config.gypi \
# TODO(bnoordhuis) Force rebuild after gyp update.
build-addons: $(NODE_EXE) test/addons/.buildstamp

ifeq ($(NAPI),True)

ADDONS_ABI_BINDING_GYPS := \
$(filter-out test/addons-abi/??_*/binding.gyp, \
$(wildcard test/addons-abi/*/binding.gyp))
Expand All @@ -215,23 +223,26 @@ test/addons-abi/.buildstamp: $(ADDONS_ABI_BINDING_GYPS) \
# TODO(bnoordhuis) Force rebuild after gyp or node-gyp update.
build-addons-abi: $(NODE_EXE) test/addons-abi/.buildstamp

endif #eq ($(NAPI),True)

test-gc: all test/gc/node_modules/weak/build/Release/weakref.node
$(PYTHON) tools/test.py --mode=release gc

test-build: | all build-addons build-addons-abi
test-build: | all $(BUILD_ADDONS)

test-build-addons: all build-addons

ifeq ($(NAPI),True)
test-build-addons-abi: all build-addons-abi
endif #eq ($(NAPI),True)

test-all: test-build test/gc/node_modules/weak/build/Release/weakref.node
$(PYTHON) tools/test.py --mode=debug,release

test-all-valgrind: test-build
$(PYTHON) tools/test.py --mode=debug,release --valgrind

CI_NATIVE_SUITES := addons addons-abi
CI_NATIVE_SUITES := addons $(if $(filter $(NAPI),True),addons-abi)
CI_JS_SUITES := doctool inspector known_issues message parallel pseudo-tty sequential

# Build and test addons without building anything else
Expand All @@ -248,7 +259,7 @@ test-ci-js:
$(TEST_CI_ARGS) $(CI_JS_SUITES)

test-ci: LOGLEVEL := info
test-ci: | build-addons build-addons-abi
test-ci: | $(BUILD_ADDONS)
$(PYTHON) tools/test.py $(PARALLEL_ARGS) -p tap --logfile test.tap \
--mode=release --flaky-tests=$(FLAKY_TESTS) \
$(TEST_CI_ARGS) $(CI_NATIVE_SUITES) addon-abi $(CI_JS_SUITES)
Expand Down Expand Up @@ -292,8 +303,10 @@ test-npm-publish: $(NODE_EXE)
test-addons: test-build-addons
$(PYTHON) tools/test.py --mode=release addons

ifeq ($(NAPI),True)
test-addons-abi: test-build-addons-abi
$(PYTHON) tools/test.py --mode=release addons-abi
endif #eq ($(NAPI),True)

test-timers:
$(MAKE) --directory=tools faketime
Expand Down Expand Up @@ -753,7 +766,9 @@ CPPLINT_EXCLUDE += src/node_root_certs.h
CPPLINT_EXCLUDE += src/queue.h
CPPLINT_EXCLUDE += src/tree.h
CPPLINT_EXCLUDE += $(wildcard test/addons/??_*/*.cc test/addons/??_*/*.h)
ifeq ($(NAPI),True)
CPPLINT_EXCLUDE += $(wildcard test/addons-abi/??_*/*.cc test/addons-abi/??_*/*.h)
endif #eq ($(NAPI),True)

CPPLINT_FILES = $(filter-out $(CPPLINT_EXCLUDE), $(wildcard \
src/*.c \
Expand All @@ -763,8 +778,8 @@ CPPLINT_FILES = $(filter-out $(CPPLINT_EXCLUDE), $(wildcard \
test/addons/*/*.h \
test/cctest/*.cc \
test/cctest/*.h \
test/addons-abi/*/*.cc \
test/addons-abi/*/*.h \
$(if $(filter $(NAPI),True),test/addons-abi/*/*.cc) \
$(if $(filter $(NAPI),True),test/addons-abi/*/*.h) \
tools/icu/*.cc \
tools/icu/*.h \
))
Expand Down Expand Up @@ -794,13 +809,17 @@ lint:
lint-ci: lint
endif

ifeq ($(NAPI),True)
PHONY_ABI = test-addons-abi build-addons-abi
else
PHONY_ABI =
endif #eq ($(NAPI),True)

.PHONY: lint cpplint jslint bench clean docopen docclean doc dist distclean \
check uninstall install install-includes install-bin all staticlib \
dynamiclib test test-all \
test-addons build-addons test-addons-abi build-addons-abi \
test-addons-abi build-addons-abi website-upload pkg \
dynamiclib test test-all test-addons build-addons website-upload pkg \
blog blogclean tar binary release-only bench-http-simple bench-idle \
bench-all bench bench-misc bench-array bench-buffer bench-net \
bench-http bench-fs bench-tls cctest run-ci test-v8 test-v8-intl \
test-v8-benchmarks test-v8-all v8 lint-ci bench-ci jslint-ci doc-only \
$(TARBALL)-headers test-ci test-ci-native test-ci-js build-ci
$(TARBALL)-headers test-ci test-ci-native test-ci-js build-ci $(PHONY_ABI)
14 changes: 14 additions & 0 deletions configure
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,18 @@ 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('--with-napi',
action='store_true',
dest='enable_napi',
default=True,
help='Build with ABI-stable API for addons. (Default)')

parser.add_option('--without-napi',
action='store_false',
dest='enable_napi',
default=True,
help='Build without ABI-stable API for addons.')

(options, args) = parser.parse_args()

# Expand ~ in the install prefix now, it gets written to multiple files.
Expand Down Expand Up @@ -767,6 +779,7 @@ def configure_mips(o):
def configure_node(o):
if options.dest_os == 'android':
o['variables']['OS'] = 'android'
o['variables']['enable_napi'] = b(options.enable_napi)
o['variables']['node_prefix'] = options.prefix
o['variables']['node_install_npm'] = b(not options.without_npm)
o['default_configuration'] = 'Debug' if options.debug else 'Release'
Expand Down Expand Up @@ -1336,6 +1349,7 @@ config = {
'BUILDTYPE': 'Debug' if options.debug else 'Release',
'USE_XCODE': str(int(options.use_xcode or 0)),
'PYTHON': sys.executable,
'NAPI': str(options.enable_napi)
}

if options.prefix:
Expand Down
25 changes: 16 additions & 9 deletions node.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@
'src/handle_wrap.cc',
'src/js_stream.cc',
'src/node.cc',
'src/node_asyncapi.cc',
'src/node_buffer.cc',
'src/node_config.cc',
'src/node_constants.cc',
Expand All @@ -165,7 +164,6 @@
'src/node_file.cc',
'src/node_http_parser.cc',
'src/node_javascript.cc',
'src/node_jsvmapi.cc',
'src/node_main.cc',
'src/node_os.cc',
'src/node_revert.cc',
Expand Down Expand Up @@ -207,10 +205,6 @@
'src/handle_wrap.h',
'src/js_stream.h',
'src/node.h',
'src/node_api_helpers.h',
'src/node_asyncapi.h',
'src/node_asyncapi_internal.h',
'src/node_asyncapi_types.h',
'src/node_buffer.h',
'src/node_constants.h',
'src/node_debug_options.h',
Expand All @@ -219,9 +213,6 @@
'src/node_internals.h',
'src/node_javascript.h',
'src/node_mutex.h',
'src/node_jsvmapi.h',
'src/node_jsvmapi_internal.h',
'src/node_jsvmapi_types.h',
'src/node_root_certs.h',
'src/node_version.h',
'src/node_watchdog.h',
Expand Down Expand Up @@ -265,6 +256,22 @@


'conditions': [
[ 'enable_napi=="true"', {
'sources': [
'src/node_asyncapi.cc',
'src/node_jsvmapi.cc',
'src/node_api_helpers.h',
'src/node_asyncapi.h',
'src/node_asyncapi_internal.h',
'src/node_asyncapi_types.h',
'src/node_jsvmapi.h',
'src/node_jsvmapi_internal.h',
'src/node_jsvmapi_types.h'
],
'defines': [
'ENABLE_NAPI'
]
}],
[ 'node_shared=="false"', {
'msvs_settings': {
'VCManifestTool': {
Expand Down
97 changes: 27 additions & 70 deletions src/node.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
#include "node_lttng.h"
#endif

#if defined ENABLE_NAPI
#include "node_jsvmapi_internal.h"
#endif

#include "ares.h"
#include "async-wrap.h"
Expand Down Expand Up @@ -98,7 +100,6 @@ typedef int mode_t;
extern char **environ;
#endif


namespace node {

using v8::Array;
Expand Down Expand Up @@ -162,6 +163,9 @@ static const char* trace_enabled_categories = nullptr;
static const char* icu_data_dir = nullptr;
#endif

// By default we accept N-API addons
bool no_napi_modules = false;

// used by C++ modules as well
bool no_deprecation = false;

Expand Down Expand Up @@ -2418,73 +2422,19 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
return;
}

// try loading with node 0.10 manner in case of node 0.10
struct node_module_old* mod;
char symbol[1024], *base, *pos;
int r;
if (mp == NULL) {
node::Utf8Value path(env->isolate(), args[1]);
base = *path;

/* Find the shared library filename within the full path. */
#ifdef __POSIX__
pos = strrchr(base, '/');
if (pos != NULL) {
base = pos + 1;
}
#else // Windows
for (;;) {
pos = strpbrk(base, "\\/:");
if (pos == NULL) {
break;
}
base = pos + 1;
}
#endif

/* Strip the .node extension. */
pos = strrchr(base, '.');
if (pos != NULL) {
*pos = '\0';
}
/* Add the `_module` suffix to the extension name. */
r = snprintf(symbol, sizeof symbol, "%s_module", base);
if (r <= 0 || static_cast<size_t>(r) >= sizeof symbol) {
env->ThrowError("Out of memory.");
}

/* Replace dashes with underscores. When loading foo-bar.node,
* look for foo_bar_module, not foo-bar_module.
*/
for (pos = symbol; *pos != '\0'; ++pos) {
if (*pos == '-') *pos = '_';
}

if (uv_dlsym(&lib, symbol, reinterpret_cast<void**>(&mod))) {
char errmsg[1024];
snprintf(errmsg, sizeof(errmsg), "Symbol %s not found.", symbol);
env->ThrowError(errmsg);
return;
}
mp = new struct node_module;
mp->nm_version = mod->version;
mp->nm_flags = 0;
mp->nm_filename = mod->filename;
mp->nm_register_func =
reinterpret_cast<node::addon_register_func>(mod->register_func);
mp->nm_context_register_func = NULL;
mp->nm_modname = mod->modname;
}

if (mp == nullptr) {
uv_dlclose(&lib);
env->ThrowError("Module did not self-register.");
return;
}

bool isNapiModule = mp->nm_version == -1;
#ifdef ENABLE_NAPI
bool isNapiModule = (!no_napi_modules && mp->nm_version == -1);

if (mp->nm_version != NODE_MODULE_VERSION && !isNapiModule) {
#else /* !defined ENABLE_NAPI */
if (mp->nm_version != NODE_MODULE_VERSION) {
#endif /* def ENABLE_NAPI */
char errmsg[1024];
snprintf(errmsg,
sizeof(errmsg),
Expand Down Expand Up @@ -2515,6 +2465,7 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
Local<String> exports_string = env->exports_string();
Local<Object> exports = module->Get(exports_string)->ToObject(env->isolate());

#ifdef ENABLE_NAPI
if (isNapiModule) {
if (mp->nm_register_func != nullptr) {
reinterpret_cast<node::addon_abi_register_func>(mp->nm_register_func)(
Expand All @@ -2525,18 +2476,18 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
} else {
uv_dlclose(&lib);
env->ThrowError("Module has no declared entry point.");
return;
}
return;
}
#endif /* def ENABLE_NAPI */
if (mp->nm_context_register_func != nullptr) {
mp->nm_context_register_func(exports, module, env->context(), mp->nm_priv);
} else if (mp->nm_register_func != nullptr) {
mp->nm_register_func(exports, module, mp->nm_priv);
} else {
if (mp->nm_context_register_func != nullptr) {
mp->nm_context_register_func(exports, module, env->context(), mp->nm_priv);
} else if (mp->nm_register_func != nullptr) {
mp->nm_register_func(exports, module, mp->nm_priv);
} else {
uv_dlclose(&lib);
env->ThrowError("Module has no declared entry point.");
return;
}
uv_dlclose(&lib);
env->ThrowError("Module has no declared entry point.");
return;
}

// Tell coverity that 'handle' should not be freed when we return.
Expand Down Expand Up @@ -2763,6 +2714,7 @@ static void LinkedBinding(const FunctionCallbackInfo<Value>& args) {
env->context(),
mod->nm_priv);
} else if (mod->nm_register_func != nullptr) {
#ifdef ENABLE_NAPI
if (mod->nm_version != -1) {
mod->nm_register_func(exports, module, mod->nm_priv);
} else {
Expand All @@ -2772,6 +2724,9 @@ static void LinkedBinding(const FunctionCallbackInfo<Value>& args) {
v8impl::JsValueFromV8LocalValue(module),
mod->nm_priv);
}
#else /* !defined ENABLE_NAPI */
mod->nm_register_func(exports, module, mod->nm_priv);
#endif /* def ENABLE_NAPI */
} else {
return env->ThrowError("Linked module has no declared entry point.");
}
Expand Down Expand Up @@ -3755,6 +3710,8 @@ static void ParseArgs(int* argc,
force_repl = true;
} else if (strcmp(arg, "--no-deprecation") == 0) {
no_deprecation = true;
} else if (strcmp(arg, "--no-napi-modules") == 0) {
no_napi_modules = true;
} else if (strcmp(arg, "--no-warnings") == 0) {
no_process_warnings = true;
} else if (strcmp(arg, "--trace-warnings") == 0) {
Expand Down
Loading

0 comments on commit 26adcc2

Please sign in to comment.