Skip to content

Commit 7c049ad

Browse files
Gabriel Schulhofjasongin
authored andcommitted
build: place N-API-related changes behind a configure/runtime flag
Currently support for N-API modules is on by default both at compile time and at runtime. Fixes #80 Closes #89
1 parent 76b5021 commit 7c049ad

14 files changed

Lines changed: 146 additions & 155 deletions

File tree

Makefile

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ PREFIX ?= /usr/local
88
FLAKY_TESTS ?= run
99
TEST_CI_ARGS ?=
1010
STAGINGSERVER ?= node-www
11+
NAPI ?= True
1112
OSTYPE := $(shell uname -s | tr '[A-Z]' '[a-z]')
1213

1314
ifdef JOBS
@@ -114,12 +115,17 @@ v8:
114115
tools/make-v8.sh v8
115116
$(MAKE) -C deps/v8 $(V8_ARCH) $(V8_BUILD_OPTIONS)
116117

118+
ifeq ($(NAPI),True)
119+
BUILD_ADDONS = build-addons build-addons-abi
120+
TESTS_TO_RUN = addons addons-abi doctool known_issues message parallel sequential
121+
else
122+
BUILD_ADDONS = build-addons
123+
TESTS_TO_RUN = addons doctool known_issues message parallel sequential
124+
endif
125+
117126
test: all
118-
$(MAKE) build-addons
119-
$(MAKE) build-addons-abi
120-
$(MAKE) cctest
121-
$(PYTHON) tools/test.py --mode=release -J \
122-
addons addons-abi doctool known_issues message parallel sequential
127+
$(MAKE) $(BUILD_ADDONS) cctest
128+
$(PYTHON) tools/test.py --mode=release -J $(TESTS_TO_RUN)
123129
$(MAKE) lint
124130

125131
test-parallel: all
@@ -167,6 +173,8 @@ test/addons/.buildstamp: $(ADDONS_BINDING_GYPS) \
167173
# TODO(bnoordhuis) Force rebuild after gyp or node-gyp update.
168174
build-addons: $(NODE_EXE) test/addons/.buildstamp
169175

176+
ifeq ($(NAPI),True)
177+
170178
ADDONS_ABI_BINDING_GYPS := \
171179
$(filter-out test/addons-abi/??_*/binding.gyp, \
172180
$(wildcard test/addons-abi/*/binding.gyp))
@@ -193,26 +201,32 @@ test/addons-abi/.buildstamp: $(ADDONS_ABI_BINDING_GYPS) \
193201
# TODO(bnoordhuis) Force rebuild after gyp or node-gyp update.
194202
build-addons-abi: $(NODE_EXE) test/addons-abi/.buildstamp
195203

204+
endif #eq ($(NAPI),True)
196205

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

200-
test-build: | all build-addons build-addons-abi
209+
test-build: | all $(BUILD_ADDONS)
201210

202211
test-build-addons: all build-addons
203212

213+
ifeq ($(NAPI),True)
204214
test-build-addons-abi: all build-addons-abi
215+
endif #eq ($(NAPI),True)
205216

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

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

212-
test-ci: | build-addons build-addons-abi
223+
CI_SUITES := addons $(if $(filter $(NAPI),True),addons-abi) \
224+
doctool known_issues message parallel sequential
225+
226+
test-ci: | $(BUILD_ADDONS)
213227
$(PYTHON) tools/test.py $(PARALLEL_ARGS) -p tap --logfile test.tap \
214228
--mode=release --flaky-tests=$(FLAKY_TESTS) \
215-
$(TEST_CI_ARGS) addons addons-abi doctool known_issues message parallel sequential
229+
$(TEST_CI_ARGS) $(CI_SUITES)
216230

217231
test-release: test-build
218232
$(PYTHON) tools/test.py --mode=release
@@ -247,8 +261,10 @@ test-npm-publish: $(NODE_EXE)
247261
test-addons: test-build-addons
248262
$(PYTHON) tools/test.py --mode=release addons
249263

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

253269
test-timers:
254270
$(MAKE) --directory=tools faketime
@@ -698,7 +714,9 @@ CPPLINT_EXCLUDE += src/queue.h
698714
CPPLINT_EXCLUDE += src/tree.h
699715
CPPLINT_EXCLUDE += src/v8abbr.h
700716
CPPLINT_EXCLUDE += $(wildcard test/addons/??_*/*.cc test/addons/??_*/*.h)
717+
ifeq ($(NAPI),True)
701718
CPPLINT_EXCLUDE += $(wildcard test/addons-abi/??_*/*.cc test/addons-abi/??_*/*.h)
719+
endif #eq ($(NAPI),True)
702720

703721
CPPLINT_FILES = $(filter-out $(CPPLINT_EXCLUDE), $(wildcard \
704722
deps/debugger-agent/include/* \
@@ -708,8 +726,8 @@ CPPLINT_FILES = $(filter-out $(CPPLINT_EXCLUDE), $(wildcard \
708726
src/*.h \
709727
test/addons/*/*.cc \
710728
test/addons/*/*.h \
711-
test/addons-abi/*/*.cc \
712-
test/addons-abi/*/*.h \
729+
$(if $(filter $(NAPI),True),test/addons-abi/*/*.cc) \
730+
$(if $(filter $(NAPI),True),test/addons-abi/*/*.h) \
713731
tools/icu/*.cc \
714732
tools/icu/*.h \
715733
))
@@ -730,13 +748,17 @@ lint:
730748
lint-ci: lint
731749
endif
732750

751+
ifeq ($(NAPI),True)
752+
PHONY_ABI = test-addons-abi build-addons-abi
753+
else
754+
PHONY_ABI =
755+
endif #eq ($(NAPI),True)
756+
733757
.PHONY: lint cpplint jslint bench clean docopen docclean doc dist distclean \
734758
check uninstall install install-includes install-bin all staticlib \
735-
dynamiclib test test-all \
736-
test-addons build-addons test-addons-abi build-addons-abi \
737-
test-addons-abi build-addons-abi website-upload pkg \
759+
dynamiclib test test-all test-addons build-addons website-upload pkg \
738760
blog blogclean tar binary release-only bench-http-simple bench-idle \
739761
bench-all bench bench-misc bench-array bench-buffer bench-net \
740762
bench-http bench-fs bench-tls cctest run-ci test-v8 test-v8-intl \
741763
test-v8-benchmarks test-v8-all v8 lint-ci bench-ci jslint-ci doc-only \
742-
$(TARBALL)-headers
764+
$(TARBALL)-headers $(PHONY_ABI)

configure

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,18 @@ parser.add_option('--no-browser-globals',
408408
help='do not export browser globals like setTimeout, console, etc. ' +
409409
'(This mode is not officially supported for regular applications)')
410410

411+
parser.add_option('--with-napi',
412+
action='store_true',
413+
dest='enable_napi',
414+
default=True,
415+
help='Build with ABI-stable API for addons. (Default)')
416+
417+
parser.add_option('--without-napi',
418+
action='store_false',
419+
dest='enable_napi',
420+
default=True,
421+
help='Build without ABI-stable API for addons.')
422+
411423
(options, args) = parser.parse_args()
412424

413425
# Expand ~ in the install prefix now, it gets written to multiple files.
@@ -704,6 +716,7 @@ def configure_mips(o):
704716
def configure_node(o):
705717
if options.dest_os == 'android':
706718
o['variables']['OS'] = 'android'
719+
o['variables']['enable_napi'] = b(options.enable_napi)
707720
o['variables']['node_prefix'] = options.prefix
708721
o['variables']['node_install_npm'] = b(not options.without_npm)
709722
o['default_configuration'] = 'Debug' if options.debug else 'Release'
@@ -1228,6 +1241,7 @@ config = {
12281241
'BUILDTYPE': 'Debug' if options.debug else 'Release',
12291242
'USE_XCODE': str(int(options.use_xcode or 0)),
12301243
'PYTHON': sys.executable,
1244+
'NAPI': str(options.enable_napi)
12311245
}
12321246

12331247
if options.prefix:

node.gyp

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -129,15 +129,13 @@
129129
'src/handle_wrap.cc',
130130
'src/js_stream.cc',
131131
'src/node.cc',
132-
'src/node_asyncapi.cc',
133132
'src/node_buffer.cc',
134133
'src/node_config.cc',
135134
'src/node_constants.cc',
136135
'src/node_contextify.cc',
137136
'src/node_file.cc',
138137
'src/node_http_parser.cc',
139138
'src/node_javascript.cc',
140-
'src/node_jsvmapi.cc',
141139
'src/node_main.cc',
142140
'src/node_os.cc',
143141
'src/node_revert.cc',
@@ -170,19 +168,12 @@
170168
'src/handle_wrap.h',
171169
'src/js_stream.h',
172170
'src/node.h',
173-
'src/node_api_helpers.h',
174-
'src/node_asyncapi.h',
175-
'src/node_asyncapi_internal.h',
176-
'src/node_asyncapi_types.h',
177171
'src/node_buffer.h',
178172
'src/node_constants.h',
179173
'src/node_file.h',
180174
'src/node_http_parser.h',
181175
'src/node_internals.h',
182176
'src/node_javascript.h',
183-
'src/node_jsvmapi.h',
184-
'src/node_jsvmapi_internal.h',
185-
'src/node_jsvmapi_types.h',
186177
'src/node_root_certs.h',
187178
'src/node_version.h',
188179
'src/node_watchdog.h',
@@ -224,6 +215,22 @@
224215

225216

226217
'conditions': [
218+
[ 'enable_napi=="true"', {
219+
'sources': [
220+
'src/node_asyncapi.cc',
221+
'src/node_jsvmapi.cc',
222+
'src/node_api_helpers.h',
223+
'src/node_asyncapi.h',
224+
'src/node_asyncapi_internal.h',
225+
'src/node_asyncapi_types.h',
226+
'src/node_jsvmapi.h',
227+
'src/node_jsvmapi_internal.h',
228+
'src/node_jsvmapi_types.h'
229+
],
230+
'defines': [
231+
'ENABLE_NAPI'
232+
]
233+
}],
227234
[ 'node_tag!=""', {
228235
'defines': [ 'NODE_TAG="<(node_tag)"' ],
229236
}],

src/node.cc

Lines changed: 27 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@
2828
#include "node_lttng.h"
2929
#endif
3030

31+
#if defined ENABLE_NAPI
3132
#include "node_jsvmapi_internal.h"
33+
#endif
3234

3335
#include "ares.h"
3436
#include "async-wrap.h"
@@ -92,7 +94,6 @@ typedef int mode_t;
9294
extern char **environ;
9395
#endif
9496

95-
9697
namespace node {
9798

9899
using v8::Array;
@@ -158,6 +159,9 @@ static node_module* modlist_addon;
158159
static const char* icu_data_dir = nullptr;
159160
#endif
160161

162+
// By default we accept N-API addons
163+
bool no_napi_modules = false;
164+
161165
// used by C++ modules as well
162166
bool no_deprecation = false;
163167

@@ -2339,73 +2343,19 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
23392343
return;
23402344
}
23412345

2342-
// try loading with node 0.10 manner in case of node 0.10
2343-
struct node_module_old* mod;
2344-
char symbol[1024], *base, *pos;
2345-
int r;
2346-
if (mp == NULL) {
2347-
node::Utf8Value path(env->isolate(), args[1]);
2348-
base = *path;
2349-
2350-
/* Find the shared library filename within the full path. */
2351-
#ifdef __POSIX__
2352-
pos = strrchr(base, '/');
2353-
if (pos != NULL) {
2354-
base = pos + 1;
2355-
}
2356-
#else // Windows
2357-
for (;;) {
2358-
pos = strpbrk(base, "\\/:");
2359-
if (pos == NULL) {
2360-
break;
2361-
}
2362-
base = pos + 1;
2363-
}
2364-
#endif
2365-
2366-
/* Strip the .node extension. */
2367-
pos = strrchr(base, '.');
2368-
if (pos != NULL) {
2369-
*pos = '\0';
2370-
}
2371-
/* Add the `_module` suffix to the extension name. */
2372-
r = snprintf(symbol, sizeof symbol, "%s_module", base);
2373-
if (r <= 0 || static_cast<size_t>(r) >= sizeof symbol) {
2374-
env->ThrowError("Out of memory.");
2375-
}
2376-
2377-
/* Replace dashes with underscores. When loading foo-bar.node,
2378-
* look for foo_bar_module, not foo-bar_module.
2379-
*/
2380-
for (pos = symbol; *pos != '\0'; ++pos) {
2381-
if (*pos == '-') *pos = '_';
2382-
}
2383-
2384-
if (uv_dlsym(&lib, symbol, reinterpret_cast<void**>(&mod))) {
2385-
char errmsg[1024];
2386-
snprintf(errmsg, sizeof(errmsg), "Symbol %s not found.", symbol);
2387-
env->ThrowError(errmsg);
2388-
return;
2389-
}
2390-
mp = new struct node_module;
2391-
mp->nm_version = mod->version;
2392-
mp->nm_flags = 0;
2393-
mp->nm_filename = mod->filename;
2394-
mp->nm_register_func =
2395-
reinterpret_cast<node::addon_register_func>(mod->register_func);
2396-
mp->nm_context_register_func = NULL;
2397-
mp->nm_modname = mod->modname;
2398-
}
2399-
24002346
if (mp == nullptr) {
24012347
uv_dlclose(&lib);
24022348
env->ThrowError("Module did not self-register.");
24032349
return;
24042350
}
24052351

2406-
bool isNapiModule = mp->nm_version == -1;
2352+
#ifdef ENABLE_NAPI
2353+
bool isNapiModule = (!no_napi_modules && mp->nm_version == -1);
24072354

24082355
if (mp->nm_version != NODE_MODULE_VERSION && !isNapiModule) {
2356+
#else /* !defined ENABLE_NAPI */
2357+
if (mp->nm_version != NODE_MODULE_VERSION) {
2358+
#endif /* def ENABLE_NAPI */
24092359
char errmsg[1024];
24102360
snprintf(errmsg,
24112361
sizeof(errmsg),
@@ -2431,6 +2381,7 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
24312381
Local<String> exports_string = env->exports_string();
24322382
Local<Object> exports = module->Get(exports_string)->ToObject(env->isolate());
24332383

2384+
#ifdef ENABLE_NAPI
24342385
if (isNapiModule) {
24352386
if (mp->nm_register_func != nullptr) {
24362387
reinterpret_cast<node::addon_abi_register_func>(mp->nm_register_func)(
@@ -2441,18 +2392,18 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
24412392
} else {
24422393
uv_dlclose(&lib);
24432394
env->ThrowError("Module has no declared entry point.");
2444-
return;
24452395
}
2396+
return;
2397+
}
2398+
#endif /* def ENABLE_NAPI */
2399+
if (mp->nm_context_register_func != nullptr) {
2400+
mp->nm_context_register_func(exports, module, env->context(), mp->nm_priv);
2401+
} else if (mp->nm_register_func != nullptr) {
2402+
mp->nm_register_func(exports, module, mp->nm_priv);
24462403
} else {
2447-
if (mp->nm_context_register_func != nullptr) {
2448-
mp->nm_context_register_func(exports, module, env->context(), mp->nm_priv);
2449-
} else if (mp->nm_register_func != nullptr) {
2450-
mp->nm_register_func(exports, module, mp->nm_priv);
2451-
} else {
2452-
uv_dlclose(&lib);
2453-
env->ThrowError("Module has no declared entry point.");
2454-
return;
2455-
}
2404+
uv_dlclose(&lib);
2405+
env->ThrowError("Module has no declared entry point.");
2406+
return;
24562407
}
24572408

24582409
// Tell coverity that 'handle' should not be freed when we return.
@@ -2640,6 +2591,7 @@ static void LinkedBinding(const FunctionCallbackInfo<Value>& args) {
26402591
env->context(),
26412592
mod->nm_priv);
26422593
} else if (mod->nm_register_func != nullptr) {
2594+
#ifdef ENABLE_NAPI
26432595
if (mod->nm_version != -1) {
26442596
mod->nm_register_func(exports, module, mod->nm_priv);
26452597
} else {
@@ -2649,6 +2601,9 @@ static void LinkedBinding(const FunctionCallbackInfo<Value>& args) {
26492601
v8impl::JsValueFromV8LocalValue(module),
26502602
mod->nm_priv);
26512603
}
2604+
#else /* !defined ENABLE_NAPI */
2605+
mod->nm_register_func(exports, module, mod->nm_priv);
2606+
#endif /* def ENABLE_NAPI */
26522607
} else {
26532608
return env->ThrowError("Linked module has no declared entry point.");
26542609
}
@@ -3668,6 +3623,8 @@ static void ParseArgs(int* argc,
36683623
force_repl = true;
36693624
} else if (strcmp(arg, "--no-deprecation") == 0) {
36703625
no_deprecation = true;
3626+
} else if (strcmp(arg, "--no-napi-modules") == 0) {
3627+
no_napi_modules = true;
36713628
} else if (strcmp(arg, "--no-warnings") == 0) {
36723629
no_process_warnings = true;
36733630
} else if (strcmp(arg, "--trace-warnings") == 0) {

0 commit comments

Comments
 (0)