GeezerVison is showing github.com — n-api: remove n-api module loading flag by gabrielschulhof · Pull Request #14902 · nodejs/node
n-api: remove n-api module loading flag
#14902
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
Uh oh!
There was an error while loading. Please reload this page.
Show file tree
Hide file tree
Changes from all commits
Commits
File filter
Filter by extension
.c
(3)
.cc
(3)
.gyp
(1)
.h
(2)
.js
(3)
.md
(2)
.py
(1)
All 7 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Hide whitespace
Diff view
Hide whitespace
There are no files selected for viewing
Original file line number
Diff line number
Diff line change
Expand Up
@@ -178,14 +178,6 @@ added: v8.4.0
Enable the experimental `'http2'` module.
### `--napi-modules`
<!-- YAML
added: v8.0.0
-->
Enable loading native modules compiled with the ABI-stable Node.js API (N-API)
(experimental).
### `--abort-on-uncaught-exception`
<!-- YAML
added: v0.10
Expand Down
Expand Up
@@ -453,7 +445,6 @@ Node options that are allowed are:
- `--inspect-brk`
- `--inspect-port`
- `--inspect`
- `--napi-modules`
- `--no-deprecation`
- `--no-warnings`
- `--openssl-config`
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -63,14 +63,6 @@ For example:
#include <node_api.h>
```
As the feature is experimental it must be enabled with the
following command line
[option](https://nodejs.org/dist/latest-v8.x/docs/api/cli.html#cli_napi_modules):
```bash
--napi-modules
```
## Basic N-API Data Types
N-API exposes the following fundamental datatypes as abstractions that are
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -288,6 +288,7 @@ inline Environment::Environment(IsolateData* isolate_data,
printed_error_(false),
trace_sync_io_(false),
abort_on_uncaught_exception_(false),
emit_napi_warning_(true),
makecallback_cntr_(0),
#if HAVE_INSPECTOR
inspector_agent_(this),
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -213,6 +213,12 @@ bool Environment::RemovePromiseHook(promise_hook_func fn, void* arg) {
return true;
}
bool Environment::EmitNapiWarning() {
bool current_value = emit_napi_warning_;
emit_napi_warning_ = false;
return current_value;
}
void Environment::EnvPromiseHook(v8::PromiseHookType type,
v8::Local<v8::Promise> promise,
v8::Local<v8::Value> parent) {
Expand Down
2 changes: 2 additions & 0 deletions
2
src/env.h
Original file line number
Diff line number
Diff line change
Expand Up
@@ -669,6 +669,7 @@ class Environment {
void AddPromiseHook(promise_hook_func fn, void* arg);
bool RemovePromiseHook(promise_hook_func fn, void* arg);
bool EmitNapiWarning();
private:
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
Expand All
@@ -690,6 +691,7 @@ class Environment {
bool printed_error_;
bool trace_sync_io_;
bool abort_on_uncaught_exception_;
bool emit_napi_warning_;
size_t makecallback_cntr_;
std::vector<double> destroy_ids_list_;
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -197,9 +197,6 @@ unsigned int reverted = 0;
std::string icu_data_dir; // NOLINT(runtime/string)
#endif
// N-API is in experimental state, disabled by default.
bool load_napi_modules = false;
// used by C++ modules as well
bool no_deprecation = false;
Expand Down
Expand Up
@@ -2703,27 +2700,22 @@ static void DLOpen(const FunctionCallbackInfo<Value>& args) {
env->ThrowError("Module did not self-register.");
return;
}
if (mp->nm_version != NODE_MODULE_VERSION) {
char errmsg[1024];
if (mp->nm_version == -1) {
snprintf(errmsg,
sizeof(errmsg),
"The module '%s'"
"\nwas compiled against the ABI-stable Node.js API (N-API)."
"\nThis feature is experimental and must be enabled on the "
"\ncommand-line by adding --napi-modules.",
*filename);
} else {
snprintf(errmsg,
sizeof(errmsg),
"The module '%s'"
"\nwas compiled against a different Node.js version using"
"\nNODE_MODULE_VERSION %d. This version of Node.js requires"
"\nNODE_MODULE_VERSION %d. Please try re-compiling or "
"re-installing\nthe module (for instance, using `npm rebuild` "
"or `npm install`).",
*filename, mp->nm_version, NODE_MODULE_VERSION);
if (mp->nm_version == -1) {
if (env->EmitNapiWarning()) {
ProcessEmitWarning(env, "N-API is an experimental feature and could "
"change at any time.");
}
} else if (mp->nm_version != NODE_MODULE_VERSION) {
char errmsg[1024];
snprintf(errmsg,
sizeof(errmsg),
"The module '%s'"
"\nwas compiled against a different Node.js version using"
"\nNODE_MODULE_VERSION %d. This version of Node.js requires"
"\nNODE_MODULE_VERSION %d. Please try re-compiling or "
"re-installing\nthe module (for instance, using `npm rebuild` "
"or `npm install`).",
*filename, mp->nm_version, NODE_MODULE_VERSION);
// NOTE: `mp` is allocated inside of the shared library's memory, calling
// `dlclose` will deallocate it
Expand Down
Expand Up
@@ -3822,7 +3814,8 @@ static void PrintHelp() {
" --throw-deprecation throw an exception on deprecations\n"
" --pending-deprecation emit pending deprecation warnings\n"
" --no-warnings silence all process warnings\n"
" --napi-modules load N-API modules\n"
" --napi-modules load N-API modules (no-op - option\n"
" kept for compatibility)\n"
" --abort-on-uncaught-exception\n"
" aborting instead of exiting causes a\n"
" core file to be generated for analysis\n"
Expand Down
Expand Up
@@ -4080,7 +4073,7 @@ static void ParseArgs(int* argc,
} else if (strcmp(arg, "--no-deprecation") == 0) {
no_deprecation = true;
} else if (strcmp(arg, "--napi-modules") == 0) {
load_napi_modules = true;
// no-op
} else if (strcmp(arg, "--no-warnings") == 0) {
no_process_warnings = true;
} else if (strcmp(arg, "--trace-warnings") == 0) {
Expand Down
Expand Up
@@ -4735,11 +4728,6 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
env.set_trace_sync_io(trace_sync_io);
if (load_napi_modules) {
ProcessEmitWarning(&env, "N-API is an experimental feature "
"and could change at any time.");
}
{
SealHandleScope seal(isolate);
bool more;
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -841,28 +841,10 @@ void napi_module_register_cb(v8::Local<v8::Object> exports,
} // end of anonymous namespace
#ifndef EXTERNAL_NAPI
namespace node {
// Indicates whether NAPI was enabled/disabled via the node command-line.
extern bool load_napi_modules;
}
#endif // EXTERNAL_NAPI
// Registers a NAPI module.
void napi_module_register(napi_module* mod) {
// NAPI modules always work with the current node version.
int module_version = NODE_MODULE_VERSION;
#ifndef EXTERNAL_NAPI
if (!node::load_napi_modules) {
// NAPI is disabled, so set the module version to -1 to cause the module
// to be unloaded.
module_version = -1;
Comment thread
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more .
If you remove this line, where is the value set to -1?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
}
#endif // EXTERNAL_NAPI
node::node_module* nm = new node::node_module {
module_version ,
-1 ,
mod->nm_flags,
nullptr,
mod->nm_filename,
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -15,7 +15,7 @@ if (process.argv[2] === 'child') {
return;
}
const p = child_process.spawnSync(
process.execPath, [ '--napi-modules', __filename, 'child' ]);
process.execPath, [ __filename, 'child' ]);
assert.ifError(p.error);
assert.ok(p.stderr.toString().includes(testException));
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -12,7 +12,7 @@ if (process.argv[2] === 'child') {
}
const p = child_process.spawnSync(
process.execPath, [ '--napi-modules', __filename, 'child' ]);
process.execPath, [ __filename, 'child' ]);
assert.ifError(p.error);
assert.ok(p.stderr.toString().includes(
'FATAL ERROR: test_fatal::Test fatal message'));
Original file line number
Diff line number
Diff line change
Expand Up
@@ -26,7 +26,9 @@ napi_value TestCallFunction(napi_env env, napi_callback_info info) {
return result;
}
void TestFunctionName(napi_env env, napi_callback_info info) {}
napi_value TestFunctionName(napi_env env, napi_callback_info info) {
return NULL;
}
napi_value Init(napi_env env, napi_value exports) {
napi_value fn1;
Expand Down
Original file line number
Diff line number
Diff line change
@@ -0,0 +1,12 @@
{
"targets": [
{
"target_name": "test_warning",
"sources": [ "test_warning.c" ]
},
{
"target_name": "test_warning2",
"sources": [ "test_warning2.c" ]
}
]
}
Original file line number
Diff line number
Diff line change
@@ -0,0 +1,18 @@
'use strict';
if (process.argv[2] === 'child') {
const common = require('../../common');
console.log(require(`./build/${common.buildType}/test_warning`));
console.log(require(`./build/${common.buildType}/test_warning2`));
} else {
const run = require('child_process').spawnSync;
const assert = require('assert');
const warning = 'Warning: N-API is an experimental feature and could ' +
'change at any time.';
const result = run(process.execPath, [__filename, 'child']);
assert.deepStrictEqual(result.stdout.toString().match(/\S+/g), ['42', '1337'],
'Modules loaded correctly');
assert.deepStrictEqual(result.stderr.toString().split(warning).length, 2,
'Warning was displayed only once');
}
Original file line number
Diff line number
Diff line change
@@ -0,0 +1,11 @@
#include <node_api.h>
#include "../common.h"
napi_value Init(napi_env env, napi_value exports) {
napi_value result;
NAPI_CALL(env,
napi_create_uint32(env, 42, &result));
return result;
}
NAPI_MODULE(test_warning, Init)
Original file line number
Diff line number
Diff line change
@@ -0,0 +1,11 @@
#include <node_api.h>
#include "../common.h"
napi_value Init(napi_env env, napi_value exports) {
napi_value result;
NAPI_CALL(env,
napi_create_uint32(env, 1337, &result));
return result;
}
NAPI_MODULE(test_warning2, Init)
Original file line number
Diff line number
Diff line change
Expand Up
@@ -3,4 +3,4 @@
import testpy
def GetConfiguration(context, root):
return testpy.AddonTestConfiguration(context, root, 'addons-napi', ['--napi-modules'] )
return testpy.AddonTestConfiguration(context, root, 'addons-napi')
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.