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.

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

module: add support for abi stable module API #11975

Closed
wants to merge 22 commits into from
Closed

Conversation

@jasongin
Copy link
Contributor

jasongin commented Mar 21, 2017

Add support for abi stable module API (N-API) as "Experimental feature".
The goal of this API is to provide a stable Node API for native
module developers. N-API aims to provide ABI compatibility guarantees
across different Node versions and also across different
Node VMs - allowing N-API enabled native modules to just work
across different versions and flavors of Node.js without recompilation.

A more detailed introduction is provided in:
https://github.com/nodejs/node-eps/blob/master/005-ABI-Stable-Module-API.md
and https://github.com/nodejs/abi-stable-node/blob/doc/VM%20Summit.pdf.

The feature, during its experimental state, will be guarded by a runtime
flag "--napi-modules". Only when this flag is added to the command line
will N-API modules along with regular non N-API modules be supported.

The API is defined by the methods in src/node_api.h,
src/node_api_types.h and "src/node_api_async.h". This is the best
starting point to review the API surface. More documentation will follow.

In addition to the implementation of the API using V8, which is included
in this PR, the API has also been validated against chakracore and that
port is available in
https://github.com/nodejs/abi-stable-node/tree/api-prototype-chakracore-8.x.

The current plan is to provide N-API support in versions 8.X and 6.X
directly. For older versions, such as 4.X or pre N-API versions of 6.X,
we plan to create an external npm module to provide a migration path
that will allow modules targeting older Node.js versions to use the API,
albeit without getting the advantage of not having to recompile.

In addition, the external npm package will include C++ sugar to
simplify the use of the API. The sugar will be in-line only and will
only use the exported N-API methods but is not part of the N-API
itself. The current version is in:
https://github.com/nodejs/node-api.

This PR is a result of work in the abi-stable-node repo:
https://github.com/nodejs/abi-stable-node/tree/doc,
with this PR being the cumulative work on the api-prototype-8.x
branch with the following contributors in alphabetical order:

Arunesh Chandra (@aruneshchandra)
Gabriel Schulhof (@gabrielschulhof)
Hitesh Kanwathirtha (@digitalinfinity)
Ian Halliday (@ianwjhalliday)
Jason Ginchereau (@jasongin)
Michael Dawson (@mhdawson)
Sampson Gao (@sampsongao)
Taylor Woll (@boingoing)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added - IN PROGRESS
  • commit message follows commit guidelines
Affected core subsystem(s)

module

Add support for abi stable module API (N-API) as "Experimental feature".
The goal of this API is to provide a stable Node API for native
module developers. N-API aims to provide ABI compatibility guarantees
across different Node versions and also across different
Node VMs - allowing N-API enabled native modules to just work
across different versions and flavors of Node.js without recompilation.

A more detailed introduction is provided in:
https://github.com/nodejs/node-eps/blob/master/005-ABI-Stable-Module-API.md
and https://github.com/nodejs/abi-stable-node/blob/doc/VM%20Summit.pdf.

The feature, during its experimental state, will be guarded by a runtime
flag "--napi-modules". Only when this flag is added to the command line
will N-API modules along with regular non N-API modules be supported.

The API is defined by the methods in "src/node_api.h",
"src/node_api_types.h" and "src/node_api_async.h". This is the best
starting point to review the API surface. More documentation will follow.

In addition to the implementation of the API using V8, which is included
in this PR, the API has also been validated against chakracore and that
port is available in
https://github.com/nodejs/abi-stable-node/tree/api-prototype-chakracore-8.x.

The current plan is to provide N-API support in versions 8.X and 6.X
directly. For older versions, such as 4.X or pre N-API versions of 6.X,
we plan to create an external npm module to provide a migration path
that will allow modules targeting older Node.js versions to use the API,
albeit without getting the advantage of not having to recompile.

In addition, we also plan an external npm package with C++ sugar to
simplify the use of the API. The sugar will be in-line only and will
only use the exported N-API methods but is not part of the N-API
itself. The current version is in:
https://github.com/nodejs/node-api.

This PR is a result of work in the abi-stable-node repo:
https://github.com/nodejs/abi-stable-node/tree/doc,
with this PR being the cumulative work on the api-prototype-8.x
branch with the following contributors in alphabetical order:

 Arunesh Chandra (@aruneshchandra)
 Gabriel Schulhof (@gabrielschulhof)
 Hitesh Kanwathirtha (@digitalinfinity)
 Ian Halliday (@ianwjhalliday)
 Jason Ginchereau (@jasongin)
 Michael Dawson (@mhdawson)
 Sampson Gao (@sampsongao)
 Taylor Woll (@boingoing)
src/node.cc Outdated
@@ -3535,6 +3547,7 @@ static void PrintHelp() {
" --trace-deprecation show stack traces on deprecations\n"
" --throw-deprecation throw an exception on deprecations\n"
" --no-warnings silence all process warnings\n"
" --napi-modules[=yes|no] load N-API modules (default no)\n"

This comment has been minimized.

@richardlau

richardlau Mar 21, 2017 Member

As the comment at the beginning of this function says:

   // XXX: If you add an option here, please also add it to doc/node.1 and
   // doc/api/cli.md

This comment has been minimized.

@addaleax

addaleax Mar 22, 2017 Member

I think this would be our only option that has a [=yes|no] format (apart from configure flags, I guess). Is there a reason for that?

This comment has been minimized.

@gabrielschulhof

gabrielschulhof Mar 22, 2017 Contributor

If somebody really wants to avoid loading N-API modules even after we've eventually set the default to "yes", thus rendering the use of the command line option unnecessary, they will be unable to do so unless we give them the option to say "no".

Of course, if the consensus is that we do not need to provide users with a way to explicitly disable the support for N-API modules, then the option can be reducded to --napi-modules.

This comment has been minimized.

@jasongin

jasongin Mar 22, 2017 Author Contributor

If somebody really wants to avoid loading N-API modules even after we've eventually set the default to "yes"

I'm not sure that's something that we'll need to support at that point. If N-API is successful it should become the normal way of writing native modules, so it wouldn't really make sense to disable it.

This comment has been minimized.

@mhdawson

mhdawson Mar 22, 2017 Member

I agree. Once non-experimental we would would most likely just remove the flag. I think at this point we could just likely make it --napi-modules with no options.

This comment has been minimized.

@Fishrock123

Fishrock123 Mar 22, 2017 Member

I personally would prefer --napi-modules and then once it goes live we could add an optional =false or something to force-disable?

This comment has been minimized.

@jasongin

jasongin Mar 22, 2017 Author Contributor

@Fishrock123 can you explain why you (or someone) might want to disable the feature after it is out of experimental status?

This comment has been minimized.

@Fishrock123

Fishrock123 Mar 22, 2017 Member

No, actually. I really do not know, but I would prefer to not have =yes.

This comment has been minimized.

@jasongin

jasongin Mar 23, 2017 Author Contributor

Resolved by 87c42e7

@mscdex mscdex added the addons label Mar 21, 2017
@mhdawson mhdawson self-assigned this Mar 21, 2017
Copy link
Contributor

trevnorris left a comment

Quick review done. Great work on the tests. Will give more thorough review later, but only have a few small comments for now.

NAPI_EXTERN napi_status napi_create_string_utf16(napi_env env,
const char16_t* s,
int length,
napi_value* result);

This comment has been minimized.

@trevnorris

trevnorris Mar 21, 2017 Contributor

why no napi_create_string_latin1? That's a fairly important type for string conversion.

This comment has been minimized.

@jasongin

jasongin Mar 22, 2017 Author Contributor

Good feedback, we should consider adding it. I opened nodejs/abi-stable-node#179 to track this.

Note missing APIs should probably not block this PR unless it is something critical. The goal for this PR is to establish an initial set of APIs that provide enough functionality for native module developers to begin experimenting and give us feedback. We have plans to expand on these APIs in the coming months, before the feature graduates out of experimental status.

This comment has been minimized.

@mhdawson

mhdawson Mar 22, 2017 Member

As @jasongin mentioned, we don't believe missing APIs should be a blocker. There are a few others we have in our plans to add which we will prioritized based on feedback from people using the API.

}

napi_status napi_escape_handle(napi_env e,
napi_escapable_handle_scope scope,

This comment has been minimized.

@trevnorris

trevnorris Mar 21, 2017 Contributor

Why not use v8::Object::GetPrototype() here?

This comment has been minimized.

@jasongin

jasongin Mar 21, 2017 Author Contributor

Did you mean to put this comment somewhere else?

I'm guessing you might be referring to the access of the prototype property in napi_instanceof() here. In that case, it is really a function's prototype property that is needed, that is different from the __proto__ property that is returned by GetPrototype(). The "instanceof" logic had to be implemented in this way rather than using v8::FunctionTemplate::HasInstance() because N-API does not expose the V8-specific concept of function or object templates, nor does it track them internally, so the FunctionTemplate that was used to create the function passed to napi_instanceof() is not available at that point.

(There is also a napi_get_prototype() API that simply calls v8::Object::GetPrototype().)

This comment has been minimized.

@trevnorris

trevnorris Mar 30, 2017 Contributor

ah yup. see what you're saying.

RETURN_STATUS_IF_FALSE(v8impl::TryCatch::lastException().IsEmpty(), \
napi_pending_exception); \
napi_clear_last_error(); \
v8impl::TryCatch tryCatch(v8impl::V8IsolateFromJsEnv((e)))

This comment has been minimized.

@trevnorris

trevnorris Mar 21, 2017 Contributor

Looks like NAP_PREAMBLE is used in many functions that don't execute javascript. what's the rational for using TryCatch when no JS is called and the v8 Maybe APIs are used?

This comment has been minimized.

@boingoing

boingoing Mar 21, 2017 Contributor

We have made an effort to avoid using NAPI_PREAMBLE for functions which don't execute JavaScript (see napi_is_error for example) but we haven't done that exhaustively. I think we took the more conservative approach of including TryCatch when we weren't sure if it was strictly required or not because the guidance seems to be that using TryCatch was cheap enough that this would be fine. If we're sure no JavaScript is getting executed, removing the NAPI_PREAMBLE seems like the right thing to do.

This comment has been minimized.

@jasongin

jasongin Mar 21, 2017 Author Contributor

Also we know we need to remove the NAPI_PREAMBLE from at least a few of the functions, for https://github.com/nodejs/abi-stable-node/issues/122 

This comment has been minimized.

@trevnorris

trevnorris Mar 22, 2017 Contributor

Thanks for the explanation. To clarify, I don't see this as a blocker.

} else if (finalize_cb != nullptr) {
// Create a self-deleting reference just for the finalize callback.
new v8impl::Reference(
isolate, obj, 0, true, finalize_cb, nativeObj, finalize_hint);

This comment has been minimized.

@trevnorris

trevnorris Mar 21, 2017 Contributor

Not sure I can appreciate this auto-magical deletion of the the pointer. One reason is because there's no way to know whether the pointer was made via new or malloc. Even in node_base_object.h there's an explicit call to make the object weak.

This comment has been minimized.

@trevnorris

trevnorris Mar 22, 2017 Contributor

Edit: missed the "self-deleting" part. Nothing to see here.

}

if (deleteSelf) {
delete reference;

This comment has been minimized.

@trevnorris

trevnorris Mar 21, 2017 Contributor

It feels counter intuitive to go to such lengths to make the public API be C, but then use delete on a user-supplied pointer.

This comment has been minimized.

@boingoing

boingoing Mar 21, 2017 Contributor

The delete here is deleting our Reference instance - not the user-supplied pointer. We aren't (hopefully) doing anything which would preclude a module written in C.

There is another delete reference in napi_delete_reference which operates on a user-supplied pointer (the napi_ref argument). We are expecting the user to hand us back a Reference* which we allocated (via new) in napi_create_reference. This Reference class acts as a container for us to do ref-counting.

If the user code calls napi_delete_reference with any random pointer, I guess behavior would be undefined. We would try to use ~Reference which might crash. But this is true of all the API which take an opaque pointer and cast it to an engine-specific type. We have a similar pattern for napi_close_handle_scope and napi_close_escapable_handle_scope.

This comment has been minimized.

@addaleax

addaleax Mar 22, 2017 Member

Yeah, the code looks correct. I’d say for these cases it would be nice to turn the constructor private and re-expose it as a static method (New() or something), so that it’s more obvious that the instance was allocated using new. (Plus, imho it’s good practice to try and keep corresponding new and delete calls reasonably close to each other)

This comment has been minimized.

@gabrielschulhof

gabrielschulhof Mar 22, 2017 Contributor

@trevnorris it sounds like we should have a test where an addon is implemented in C, just to keep us honest.

This comment has been minimized.

@jasongin

jasongin Mar 23, 2017 Author Contributor

fb8ced4 adds New() and Delete() methods on the Reference class for clarity.

This comment has been minimized.

@trevnorris

trevnorris Mar 30, 2017 Contributor

my bad. misunderstood what was going on here.

The snprintf API is not available with older versions of MSVC. Found by a CI run.
@addaleax
Copy link
Member

addaleax commented Mar 22, 2017

Btw, in ba4847e we listed the authors as Author: name <email> as PR metadata, it looks like doing the same thing might make sense here, too?

@addaleax addaleax added the module label Mar 22, 2017
src/node.cc Outdated
snprintf(errmsg,
sizeof(errmsg),
"The module '%s'"
"\nwas compiled against the Node.js API. This feature is "

This comment has been minimized.

@addaleax

addaleax Mar 22, 2017 Member

This error message could be a bit more specific than the Node.js API, I’d say. Also, maybe display the flag name here directly?

This comment has been minimized.

@jasongin

jasongin Mar 23, 2017 Author Contributor

Resolved by b129624

src/node.cc Outdated
@@ -3535,6 +3547,7 @@ static void PrintHelp() {
" --trace-deprecation show stack traces on deprecations\n"
" --throw-deprecation throw an exception on deprecations\n"
" --no-warnings silence all process warnings\n"
" --napi-modules[=yes|no] load N-API modules (default no)\n"

This comment has been minimized.

@addaleax

addaleax Mar 22, 2017 Member

I think this would be our only option that has a [=yes|no] format (apart from configure flags, I guess). Is there a reason for that?

ProcessEmitWarning(&env, "N-API is an experimental feature "
"and could change at any time.");
}

This comment has been minimized.

@addaleax

addaleax Mar 22, 2017 Member

I’m not a fan of printing warnings like these… if the user opted into the feature, then they opted in because they know what they were asking for.

This comment has been minimized.

@jasongin

jasongin Mar 22, 2017 Author Contributor

CTC has specifically requested command-line enablement along with a warning message for features with experimental status. Tracing does the same thing.

This comment has been minimized.

@mikeal

mikeal Mar 23, 2017 Contributor

Is there a link somewhere to that decision?

I don't quite understand the justification for a printed warning for a feature that can't even be used without a flag. If you're using the flag, you wanted the feature and are quite aware that it is experimental. Do any V8 features do this?

This comment has been minimized.

@aruneshchandra

aruneshchandra Mar 23, 2017 Member

VM Summit 3 meeting notes has this
For 8.0, include an opt-in runtime flag, show experimental warning when a NAPI module is loaded.

This comment has been minimized.

@jasongin

jasongin Mar 24, 2017 Author Contributor

If consensus is that documentation of experimental status is sufficient and this warning message is not necessary, we can take it out. I don't recall anyone feeling very strongly about this at the VM summit.

This comment has been minimized.

@addaleax

addaleax Mar 24, 2017 Member

@jasongin I’d say take it out, and we can re-visit that in the unlikely case anybody complains.

This comment has been minimized.

@mhdawson

mhdawson Mar 27, 2017 Member

I think the main motivation was to ensure that anybody turning it on is fully aware of the status and to be consistent with what we have done for other experimental features.

v8::Local<v8::Value> l;
U(v8::Local<v8::Value> _l) : l(_l) {}
} u(local);
assert(sizeof(u.v) == sizeof(u.l));

This comment has been minimized.

@addaleax

addaleax Mar 22, 2017 Member

static_assert for all of these

This comment has been minimized.

@jasongin

jasongin Mar 23, 2017 Author Contributor

Resolved by 116bd19

} u(local);
assert(sizeof(u.v) == sizeof(u.l));
return u.v;
}

This comment has been minimized.

@addaleax

addaleax Mar 22, 2017 Member

Yeah, awkward is a good description, I’m not even sure these would work if we didn’t compile with -fno-strict-aliasing. Is there anything speaking against a simple return reinterpret_cast<napi_value>(*local), at least for the V8-to-napi_value conversion?

This comment has been minimized.

@jasongin

jasongin Mar 22, 2017 Author Contributor

return reinterpret_cast<napi_value>(*local)

Yes, I agree that's correct and more straightforward. I'll update it.

(This is some of the oldest code in the N-API project; I'm guessing whoever wrote it was initially trying to avoid using reinterpret_cast<>, though the union accomplishes the same thing in a roundabout way.)

This comment has been minimized.

@jasongin

jasongin Mar 23, 2017 Author Contributor

Resolved by 116bd19

// Gets the number of CHARACTERS in the string.
napi_status napi_get_value_string_length(napi_env e,
napi_value value,
int* result) {

This comment has been minimized.

@addaleax

addaleax Mar 22, 2017 Member

It seems like int is a V8 implementation detail here too, size_t is probably better for string lengths (also here and below)?

This comment has been minimized.

@jasongin

jasongin Mar 22, 2017 Author Contributor

Yes, good point.

This comment has been minimized.

@jasongin

jasongin Mar 23, 2017 Author Contributor

Resolved by 9e3ab83

}

napi_status napi_escape_handle(napi_env e,
napi_escapable_handle_scope scope,

This comment has been minimized.

@addaleax

addaleax Mar 22, 2017 Member

It seems to me like int is a V8 implementation detail, size_t might better for argument counts (here and elsewhere)?

This comment has been minimized.

@jasongin

jasongin Mar 23, 2017 Author Contributor

Resolved by 9e3ab83

CHECK_ARG(ref);

v8impl::Reference* reference = reinterpret_cast<v8impl::Reference*>(ref);
delete reference;

This comment has been minimized.

@addaleax

addaleax Mar 22, 2017 Member

(re: my comment above … maybe also make the Reference destructor private and add a Delete static? I know it might seem a bit superfluous but I think it would really help with readability)

This comment has been minimized.

@jasongin

jasongin Mar 23, 2017 Author Contributor

Resolved by fb8ced4

}

napi_status napi_escape_handle(napi_env e,
napi_escapable_handle_scope scope,

This comment has been minimized.

@addaleax

addaleax Mar 22, 2017 Member

v8::Handlev8::Local (also something for search & replace… 😄)

This comment has been minimized.

@jasongin

jasongin Mar 23, 2017 Author Contributor

Resolved by 116bd19

}

napi_status napi_escape_handle(napi_env e,
napi_escapable_handle_scope scope,

This comment has been minimized.

@addaleax

addaleax Mar 22, 2017 Member

If we want to support an instanceof implementation that doesn’t conform to ES6 (in that it isn’t based on [Symbol.hasInstance]()), could/should we call it something else? Alternatively, could this wrapper just manually call out to constructor[Symbol.hasInstance](object)?

This comment has been minimized.

@jasongin

jasongin Mar 22, 2017 Author Contributor

This does conform to ES6 instanceof. There are extensive tests at test/addons-napi/test_instanceof that re-run all of V8's instanceof tests using this API.

This comment has been minimized.

@gabrielschulhof

gabrielschulhof Mar 22, 2017 Contributor

@addaleax I implemented napi_instanceof() this way because I didn't have access to the implementation of the operator. However, if v8 provides access to an implementation that does not require retaining/acquiring the FunctionTemplate then I'd be happy to change this API. Additionally, it looks like Symbol.hasInstance() allows users to customize the behaviour of the instanceof operator, so we absolutely have to use Symbol.hasInstance() when available.

This comment has been minimized.

@addaleax

addaleax Mar 22, 2017 Member

@jasongin Yeah, it matches OrdinaryHasInstance(). I’m good with just naming it that.

Otoh, it should really not be a problem for V8 to expose InstanceOf() and OrdinaryHasInstance() directly…

gabrielschulhof added a commit to gabrielschulhof/abi-stable-node that referenced this pull request Mar 22, 2017
@jasnell jasnell added this to the 8.0.0 milestone Mar 22, 2017
gabrielschulhof added a commit to gabrielschulhof/abi-stable-node that referenced this pull request Mar 22, 2017
gabrielschulhof added a commit to gabrielschulhof/abi-stable-node that referenced this pull request Mar 22, 2017
This updates the documentation, the error message upon module load
failure, the command line option parsing of the flag, and the way
the N-API addon tests pass the flag to node.

Re nodejs/node#11975 (comment)
Re nodejs/node#11975 (comment)
Fixes nodejs#184
gabrielschulhof added a commit to gabrielschulhof/abi-stable-node that referenced this pull request Mar 22, 2017
This updates the documentation, the error message upon module load
failure, the command line option parsing of the flag, and the way
the N-API addon tests pass the flag to node.

Re nodejs/node#11975 (comment)
Re nodejs/node#11975 (comment)
Fixes nodejs#184
Closes nodejs#186
gabrielschulhof added a commit to nodejs/abi-stable-node that referenced this pull request Mar 22, 2017
This updates the documentation, the error message upon module load
failure, the command line option parsing of the flag, and the way
the N-API addon tests pass the flag to node.

Re nodejs/node#11975 (comment)
Re nodejs/node#11975 (comment)
Fixes #184
Closes #186
gabrielschulhof added a commit to gabrielschulhof/abi-stable-node that referenced this pull request Mar 23, 2017
This updates the documentation, the error message upon module load
failure, the command line option parsing of the flag, and the way
the N-API addon tests pass the flag to node.

Re #11975 (comment)
Re #11975 (comment)
Fixes nodejs/abi-stable-node#184
Closes nodejs/abi-stable-node#186
@kkoopa
Copy link

kkoopa commented Apr 1, 2017

Kinda wish it was a bit more like the duktape api* which is quite similar in API shape generally, but returns things from functions you;d expect to return things, rather than the status.

From a glance, it appears that duktape is implemented as a stack machine, so it is very different.

@addaleax
Copy link
Member

addaleax commented Apr 3, 2017

Landed in 56e881d 🎉

@nodejs/abi-stable-node I think that also means that from now on you can open PRs against this repository instead of nodejs/abi-stable-node. :)

@addaleax addaleax closed this Apr 3, 2017
addaleax added a commit that referenced this pull request Apr 3, 2017
Add support for abi stable module API (N-API) as "Experimental feature".
The goal of this API is to provide a stable Node API for native
module developers. N-API aims to provide ABI compatibility guarantees
across different Node versions and also across different
Node VMs - allowing N-API enabled native modules to just work
across different versions and flavors of Node.js without recompilation.

A more detailed introduction is provided in:
https://github.com/nodejs/node-eps/blob/master/005-ABI-Stable-Module-API.md
and https://github.com/nodejs/abi-stable-node/blob/doc/VM%20Summit.pdf.

The feature, during its experimental state, will be guarded by a runtime
flag "--napi-modules". Only when this flag is added to the command line
will N-API modules along with regular non N-API modules be supported.

The API is defined by the methods in "src/node_api.h" and
"src/node_api_types.h". This is the best
starting point to review the API surface. More documentation will follow.

In addition to the implementation of the API using V8, which is included
in this PR, the API has also been validated against chakracore and that
port is available in
https://github.com/nodejs/abi-stable-node/tree/api-prototype-chakracore-8.x.

The current plan is to provide N-API support in versions 8.X and 6.X
directly. For older versions, such as 4.X or pre N-API versions of 6.X,
we plan to create an external npm module to provide a migration path
that will allow modules targeting older Node.js versions to use the API,
albeit without getting the advantage of not having to recompile.

In addition, we also plan an external npm package with C++ sugar to
simplify the use of the API. The sugar will be in-line only and will
only use the exported N-API methods but is not part of the N-API
itself. The current version is in:
https://github.com/nodejs/node-api.

This PR is a result of work in the abi-stable-node repo:
https://github.com/nodejs/abi-stable-node/tree/doc,
with this PR being the cumulative work on the api-prototype-8.x
branch with the following contributors in alphabetical order:

Author: Arunesh Chandra <arunesh.chandra@microsoft.com>
Author: Gabriel Schulhof <gabriel.schulhof@intel.com>
Author: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Author: Ian Halliday <ianhall@microsoft.com>
Author: Jason Ginchereau <jasongin@microsoft.com>
Author: Michael Dawson <michael_dawson@ca.ibm.com>
Author: Sampson Gao <sampsong@ca.ibm.com>
Author: Taylor Woll <taylor.woll@microsoft.com>
PR-URL: #11975
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax addaleax removed the ctc-review label Apr 3, 2017
@jasnell jasnell mentioned this pull request Apr 4, 2017
@addaleax addaleax mentioned this pull request Apr 6, 2017
3 of 3 tasks complete
@addaleax addaleax mentioned this pull request Apr 29, 2017
2 of 2 tasks complete
jasnell added a commit to jasnell/node that referenced this pull request May 29, 2017
* **Async Hooks**
  * The `async_hooks` module has landed in core
    [[`4a7233c178`](nodejs@4a7233c)]
    [nodejs#12892](nodejs#12892).

* **Buffer**
  * Using the `--pending-deprecation` flag will cause Node.js to emit a
    deprecation warning when using `new Buffer(num)` or `Buffer(num)`.
    [[`d2d32ea5a2`](nodejs@d2d32ea)]
    [nodejs#11968](nodejs#11968).
  * `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances
    [[`7eb1b4658e`](nodejs@7eb1b46)]
    [nodejs#12141](nodejs#12141).
  * Many `Buffer` methods now accept `Uint8Array` as input
    [[`beca3244e2`](nodejs@beca324)]
    [nodejs#10236](nodejs#10236).

* **Child Process**
  * Argument and kill signal validations have been improved
    [[`97a77288ce`](nodejs@97a7728)]
    [nodejs#12348](nodejs#12348),
    [[`d75fdd96aa`](nodejs@d75fdd9)]
    [nodejs#10423](nodejs#10423).
  * Child Process methods accept `Uint8Array` as input
    [[`627ecee9ed`](nodejs@627ecee)]
    [nodejs#10653](nodejs#10653).

* **Console**
  * Error events emitted when using `console` methods are now supressed.
    [[`f18e08d820`](nodejs@f18e08d)]
    [nodejs#9744](nodejs#9744).

* **Dependencies**
  * The npm client has been updated to 5.0.0
    [[`3c3b36af0f`](nodejs@3c3b36a)]
    [nodejs#12936](nodejs#12936).
  * V8 has been updated to 5.8 with forward ABI stability to 6.0
    [[`60d1aac8d2`](nodejs@60d1aac)]
    [nodejs#12784](nodejs#12784).

* **Domains**
  * Native `Promise` instances are now `Domain` aware
    [[`84dabe8373`](nodejs@84dabe8)]
    [nodejs#12489](nodejs#12489).

* **Errors**
  * We have started assigning static error codes to errors generated by Node.js.
    This has been done through multiple commits and is still a work in
    progress.

* **File System**
  * The utility class `fs.SyncWriteStream` has been deprecated
    [[`7a55e34ef4`](nodejs@7a55e34)]
    [nodejs#10467](nodejs#10467).
  * The deprecated `fs.read()` string interface has been removed
    [[`3c2a9361ff`](nodejs@3c2a936)]
    [nodejs#9683](nodejs#9683).

* **HTTP**
  * Improved support for userland implemented Agents
    [[`90403dd1d0`](nodejs@90403dd)]
    [nodejs#11567](nodejs#11567).
  * Outgoing Cookie headers are concatenated into a single string
    [[`d3480776c7`](nodejs@d348077)]
    [nodejs#11259](nodejs#11259).
  * The `httpResponse.writeHeader()` method has been deprecated
    [[`fb71ba4921`](nodejs@fb71ba4)]
    [nodejs#11355](nodejs#11355).
  * New methods for accessing HTTP headers have been added to `OutgoingMessage`
    [[`3e6f1032a4`](nodejs@3e6f103)]
    [nodejs#10805](nodejs#10805).

* **Lib**
  * All deprecation messages have been assigned static identifiers
    [[`5de3cf099c`](nodejs@5de3cf0)]
    [nodejs#10116](nodejs#10116).
  * The legacy `linkedlist` module has been removed
    [[`84a23391f6`](nodejs@84a2339)]
    [nodejs#12113](nodejs#12113).

* **N-API**
  * Experimental support for the new N-API API has been added
    [[`56e881d0b0`](nodejs@56e881d)]
    [nodejs#11975](nodejs#11975).

* **Process**
  * Process warning output can be redirected to a file using the
    `--redirect-warnings` command-line argument
    [[`03e89b3ff2`](nodejs@03e89b3)]
    [nodejs#10116](nodejs#10116).
  * Process warnings may now include additional detail
    [[`dd20e68b0f`](nodejs@dd20e68)]
    [nodejs#12725](nodejs#12725).

* **REPL**
  * REPL magic mode has been deprecated
    [[`3f27f02da0`](nodejs@3f27f02)]
    [nodejs#11599](nodejs#11599).

* **Src**
  * `NODE_MODULE_VERSION` has been updated to 57
    (nodejs@ec7cbaf)]
    [nodejs#12995](nodejs#12995).
  * Add `--pending-deprecation` command-line argument and
    `NODE_PENDING_DEPRECATION` environment variable
    [[`a16b570f8c`](nodejs@a16b570)]
    [nodejs#11968](nodejs#11968).
  * The `--debug` command-line argument has been deprecated. Note that
    using `--debug` will enable the *new* Inspector-based debug protocol
    as the legacy Debugger protocol previously used by Node.js has been
    removed. [[`010f864426`](nodejs@010f864)]
    [nodejs#12949](nodejs#12949).
  * Throw when the `-c` and `-e` command-line arguments are used at the same
    time [[`a5f91ab230`](nodejs@a5f91ab)]
    [nodejs#11689](nodejs#11689).
  * Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line
    arguments are used at the same time.
    [[`8a7db9d4b5`](nodejs@8a7db9d)]
    [nodejs#12087](nodejs#12087).

* **Stream**
  * `Stream` now supports `destroy()` and `_destroy()` APIs
    [[`b6e1d22fa6`](nodejs@b6e1d22)]
    [nodejs#12925](nodejs#12925).
  * `Stream` now supports the `_final()` API
    [[`07c7f198db`](nodejs@07c7f19)]
    [nodejs#12828](nodejs#12828).

* **TLS**
  * The `rejectUnauthorized` option now defaults to `true`
    [[`348cc80a3c`](nodejs@348cc80)]
    [nodejs#5923](nodejs#5923).
  * The `tls.createSecurePair()` API now emits a runtime deprecation
    [[`a2ae08999b`](nodejs@a2ae089)]
    [nodejs#11349](nodejs#11349).
  * A runtime deprecation will now be emitted when `dhparam` is less than
    2048 bits [[`d523eb9c40`](nodejs@d523eb9)]
    [nodejs#11447](nodejs#11447).

* **URL**
  * The WHATWG URL implementation is now a fully-supported Node.js API
    [[`d080ead0f9`](nodejs@d080ead)]
    [nodejs#12710](nodejs#12710).

* **Util**
  * `Symbol` keys are now displayed by default when using `util.inspect()`
    [[`5bfd13b81e`](nodejs@5bfd13b)]
    [nodejs#9726](nodejs#9726).
  * `toJSON` errors will be thrown when formatting `%j`
    [[`455e6f1dd8`](nodejs@455e6f1)]
    [nodejs#11708](nodejs#11708).
  * Convert `inspect.styles` and `inspect.colors` to prototype-less objects
    [[`aab0d202f8`](nodejs@aab0d20)]
    [nodejs#11624](nodejs#11624).
  * The new `util.promisify()` API has been added
    [[`99da8e8e02`](nodejs@99da8e8)]
    [nodejs#12442](nodejs#12442).

* **Zlib**
  * Support `Uint8Array` in Zlib convenience methods
    [[`91383e47fd`](nodejs@91383e4)]
    [nodejs#12001](nodejs#12001).
  * Zlib errors now use `RangeError` and `TypeError` consistently
    [[`b514bd231e`](nodejs@b514bd2)]
    [nodejs#11391](nodejs#11391).
jasnell added a commit that referenced this pull request May 30, 2017
* **Async Hooks**
  * The `async_hooks` module has landed in core
    [[`4a7233c178`](4a7233c)]
    [#12892](#12892).

* **Buffer**
  * Using the `--pending-deprecation` flag will cause Node.js to emit a
    deprecation warning when using `new Buffer(num)` or `Buffer(num)`.
    [[`d2d32ea5a2`](d2d32ea)]
    [#11968](#11968).
  * `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances
    [[`7eb1b4658e`](7eb1b46)]
    [#12141](#12141).
  * Many `Buffer` methods now accept `Uint8Array` as input
    [[`beca3244e2`](beca324)]
    [#10236](#10236).

* **Child Process**
  * Argument and kill signal validations have been improved
    [[`97a77288ce`](97a7728)]
    [#12348](#12348),
    [[`d75fdd96aa`](d75fdd9)]
    [#10423](#10423).
  * Child Process methods accept `Uint8Array` as input
    [[`627ecee9ed`](627ecee)]
    [#10653](#10653).

* **Console**
  * Error events emitted when using `console` methods are now supressed.
    [[`f18e08d820`](f18e08d)]
    [#9744](#9744).

* **Dependencies**
  * The npm client has been updated to 5.0.0
    [[`3c3b36af0f`](3c3b36a)]
    [#12936](#12936).
  * V8 has been updated to 5.8 with forward ABI stability to 6.0
    [[`60d1aac8d2`](60d1aac)]
    [#12784](#12784).

* **Domains**
  * Native `Promise` instances are now `Domain` aware
    [[`84dabe8373`](84dabe8)]
    [#12489](#12489).

* **Errors**
  * We have started assigning static error codes to errors generated by Node.js.
    This has been done through multiple commits and is still a work in
    progress.

* **File System**
  * The utility class `fs.SyncWriteStream` has been deprecated
    [[`7a55e34ef4`](7a55e34)]
    [#10467](#10467).
  * The deprecated `fs.read()` string interface has been removed
    [[`3c2a9361ff`](3c2a936)]
    [#9683](#9683).

* **HTTP**
  * Improved support for userland implemented Agents
    [[`90403dd1d0`](90403dd)]
    [#11567](#11567).
  * Outgoing Cookie headers are concatenated into a single string
    [[`d3480776c7`](d348077)]
    [#11259](#11259).
  * The `httpResponse.writeHeader()` method has been deprecated
    [[`fb71ba4921`](fb71ba4)]
    [#11355](#11355).
  * New methods for accessing HTTP headers have been added to `OutgoingMessage`
    [[`3e6f1032a4`](3e6f103)]
    [#10805](#10805).

* **Lib**
  * All deprecation messages have been assigned static identifiers
    [[`5de3cf099c`](5de3cf0)]
    [#10116](#10116).
  * The legacy `linkedlist` module has been removed
    [[`84a23391f6`](84a2339)]
    [#12113](#12113).

* **N-API**
  * Experimental support for the new N-API API has been added
    [[`56e881d0b0`](56e881d)]
    [#11975](#11975).

* **Process**
  * Process warning output can be redirected to a file using the
    `--redirect-warnings` command-line argument
    [[`03e89b3ff2`](03e89b3)]
    [#10116](#10116).
  * Process warnings may now include additional detail
    [[`dd20e68b0f`](dd20e68)]
    [#12725](#12725).

* **REPL**
  * REPL magic mode has been deprecated
    [[`3f27f02da0`](3f27f02)]
    [#11599](#11599).

* **Src**
  * `NODE_MODULE_VERSION` has been updated to 57
    (ec7cbaf)]
    [#12995](#12995).
  * Add `--pending-deprecation` command-line argument and
    `NODE_PENDING_DEPRECATION` environment variable
    [[`a16b570f8c`](a16b570)]
    [#11968](#11968).
  * The `--debug` command-line argument has been deprecated. Note that
    using `--debug` will enable the *new* Inspector-based debug protocol
    as the legacy Debugger protocol previously used by Node.js has been
    removed. [[`010f864426`](010f864)]
    [#12949](#12949).
  * Throw when the `-c` and `-e` command-line arguments are used at the same
    time [[`a5f91ab230`](a5f91ab)]
    [#11689](#11689).
  * Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line
    arguments are used at the same time.
    [[`8a7db9d4b5`](8a7db9d)]
    [#12087](#12087).

* **Stream**
  * `Stream` now supports `destroy()` and `_destroy()` APIs
    [[`b6e1d22fa6`](b6e1d22)]
    [#12925](#12925).
  * `Stream` now supports the `_final()` API
    [[`07c7f198db`](07c7f19)]
    [#12828](#12828).

* **TLS**
  * The `rejectUnauthorized` option now defaults to `true`
    [[`348cc80a3c`](348cc80)]
    [#5923](#5923).
  * The `tls.createSecurePair()` API now emits a runtime deprecation
    [[`a2ae08999b`](a2ae089)]
    [#11349](#11349).
  * A runtime deprecation will now be emitted when `dhparam` is less than
    2048 bits [[`d523eb9c40`](d523eb9)]
    [#11447](#11447).

* **URL**
  * The WHATWG URL implementation is now a fully-supported Node.js API
    [[`d080ead0f9`](d080ead)]
    [#12710](#12710).

* **Util**
  * `Symbol` keys are now displayed by default when using `util.inspect()`
    [[`5bfd13b81e`](5bfd13b)]
    [#9726](#9726).
  * `toJSON` errors will be thrown when formatting `%j`
    [[`455e6f1dd8`](455e6f1)]
    [#11708](#11708).
  * Convert `inspect.styles` and `inspect.colors` to prototype-less objects
    [[`aab0d202f8`](aab0d20)]
    [#11624](#11624).
  * The new `util.promisify()` API has been added
    [[`99da8e8e02`](99da8e8)]
    [#12442](#12442).

* **Zlib**
  * Support `Uint8Array` in Zlib convenience methods
    [[`91383e47fd`](91383e4)]
    [#12001](#12001).
  * Zlib errors now use `RangeError` and `TypeError` consistently
    [[`b514bd231e`](b514bd2)]
    [#11391](#11391).
jasnell added a commit that referenced this pull request May 30, 2017
* **Async Hooks**
  * The `async_hooks` module has landed in core
    [[`4a7233c178`](4a7233c)]
    [#12892](#12892).

* **Buffer**
  * Using the `--pending-deprecation` flag will cause Node.js to emit a
    deprecation warning when using `new Buffer(num)` or `Buffer(num)`.
    [[`d2d32ea5a2`](d2d32ea)]
    [#11968](#11968).
  * `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances
    [[`7eb1b4658e`](7eb1b46)]
    [#12141](#12141).
  * Many `Buffer` methods now accept `Uint8Array` as input
    [[`beca3244e2`](beca324)]
    [#10236](#10236).

* **Child Process**
  * Argument and kill signal validations have been improved
    [[`97a77288ce`](97a7728)]
    [#12348](#12348),
    [[`d75fdd96aa`](d75fdd9)]
    [#10423](#10423).
  * Child Process methods accept `Uint8Array` as input
    [[`627ecee9ed`](627ecee)]
    [#10653](#10653).

* **Console**
  * Error events emitted when using `console` methods are now supressed.
    [[`f18e08d820`](f18e08d)]
    [#9744](#9744).

* **Dependencies**
  * The npm client has been updated to 5.0.0
    [[`3c3b36af0f`](3c3b36a)]
    [#12936](#12936).
  * V8 has been updated to 5.8 with forward ABI stability to 6.0
    [[`60d1aac8d2`](60d1aac)]
    [#12784](#12784).

* **Domains**
  * Native `Promise` instances are now `Domain` aware
    [[`84dabe8373`](84dabe8)]
    [#12489](#12489).

* **Errors**
  * We have started assigning static error codes to errors generated by Node.js.
    This has been done through multiple commits and is still a work in
    progress.

* **File System**
  * The utility class `fs.SyncWriteStream` has been deprecated
    [[`7a55e34ef4`](7a55e34)]
    [#10467](#10467).
  * The deprecated `fs.read()` string interface has been removed
    [[`3c2a9361ff`](3c2a936)]
    [#9683](#9683).

* **HTTP**
  * Improved support for userland implemented Agents
    [[`90403dd1d0`](90403dd)]
    [#11567](#11567).
  * Outgoing Cookie headers are concatenated into a single string
    [[`d3480776c7`](d348077)]
    [#11259](#11259).
  * The `httpResponse.writeHeader()` method has been deprecated
    [[`fb71ba4921`](fb71ba4)]
    [#11355](#11355).
  * New methods for accessing HTTP headers have been added to `OutgoingMessage`
    [[`3e6f1032a4`](3e6f103)]
    [#10805](#10805).

* **Lib**
  * All deprecation messages have been assigned static identifiers
    [[`5de3cf099c`](5de3cf0)]
    [#10116](#10116).
  * The legacy `linkedlist` module has been removed
    [[`84a23391f6`](84a2339)]
    [#12113](#12113).

* **N-API**
  * Experimental support for the new N-API API has been added
    [[`56e881d0b0`](56e881d)]
    [#11975](#11975).

* **Process**
  * Process warning output can be redirected to a file using the
    `--redirect-warnings` command-line argument
    [[`03e89b3ff2`](03e89b3)]
    [#10116](#10116).
  * Process warnings may now include additional detail
    [[`dd20e68b0f`](dd20e68)]
    [#12725](#12725).

* **REPL**
  * REPL magic mode has been deprecated
    [[`3f27f02da0`](3f27f02)]
    [#11599](#11599).

* **Src**
  * `NODE_MODULE_VERSION` has been updated to 57
    (ec7cbaf)]
    [#12995](#12995).
  * Add `--pending-deprecation` command-line argument and
    `NODE_PENDING_DEPRECATION` environment variable
    [[`a16b570f8c`](a16b570)]
    [#11968](#11968).
  * The `--debug` command-line argument has been deprecated. Note that
    using `--debug` will enable the *new* Inspector-based debug protocol
    as the legacy Debugger protocol previously used by Node.js has been
    removed. [[`010f864426`](010f864)]
    [#12949](#12949).
  * Throw when the `-c` and `-e` command-line arguments are used at the same
    time [[`a5f91ab230`](a5f91ab)]
    [#11689](#11689).
  * Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line
    arguments are used at the same time.
    [[`8a7db9d4b5`](8a7db9d)]
    [#12087](#12087).

* **Stream**
  * `Stream` now supports `destroy()` and `_destroy()` APIs
    [[`b6e1d22fa6`](b6e1d22)]
    [#12925](#12925).
  * `Stream` now supports the `_final()` API
    [[`07c7f198db`](07c7f19)]
    [#12828](#12828).

* **TLS**
  * The `rejectUnauthorized` option now defaults to `true`
    [[`348cc80a3c`](348cc80)]
    [#5923](#5923).
  * The `tls.createSecurePair()` API now emits a runtime deprecation
    [[`a2ae08999b`](a2ae089)]
    [#11349](#11349).
  * A runtime deprecation will now be emitted when `dhparam` is less than
    2048 bits [[`d523eb9c40`](d523eb9)]
    [#11447](#11447).

* **URL**
  * The WHATWG URL implementation is now a fully-supported Node.js API
    [[`d080ead0f9`](d080ead)]
    [#12710](#12710).

* **Util**
  * `Symbol` keys are now displayed by default when using `util.inspect()`
    [[`5bfd13b81e`](5bfd13b)]
    [#9726](#9726).
  * `toJSON` errors will be thrown when formatting `%j`
    [[`455e6f1dd8`](455e6f1)]
    [#11708](#11708).
  * Convert `inspect.styles` and `inspect.colors` to prototype-less objects
    [[`aab0d202f8`](aab0d20)]
    [#11624](#11624).
  * The new `util.promisify()` API has been added
    [[`99da8e8e02`](99da8e8)]
    [#12442](#12442).

* **Zlib**
  * Support `Uint8Array` in Zlib convenience methods
    [[`91383e47fd`](91383e4)]
    [#12001](#12001).
  * Zlib errors now use `RangeError` and `TypeError` consistently
    [[`b514bd231e`](b514bd2)]
    [#11391](#11391).
gabrielschulhof added a commit to gabrielschulhof/node that referenced this pull request Apr 10, 2018
Add support for abi stable module API (N-API) as "Experimental feature".
The goal of this API is to provide a stable Node API for native
module developers. N-API aims to provide ABI compatibility guarantees
across different Node versions and also across different
Node VMs - allowing N-API enabled native modules to just work
across different versions and flavors of Node.js without recompilation.

A more detailed introduction is provided in:
https://github.com/nodejs/node-eps/blob/master/005-ABI-Stable-Module-API.md
and https://github.com/nodejs/abi-stable-node/blob/doc/VM%20Summit.pdf.

The feature, during its experimental state, will be guarded by a runtime
flag "--napi-modules". Only when this flag is added to the command line
will N-API modules along with regular non N-API modules be supported.

The API is defined by the methods in "src/node_api.h" and
"src/node_api_types.h". This is the best
starting point to review the API surface. More documentation will follow.

In addition to the implementation of the API using V8, which is included
in this PR, the API has also been validated against chakracore and that
port is available in
https://github.com/nodejs/abi-stable-node/tree/api-prototype-chakracore-8.x.

The current plan is to provide N-API support in versions 8.X and 6.X
directly. For older versions, such as 4.X or pre N-API versions of 6.X,
we plan to create an external npm module to provide a migration path
that will allow modules targeting older Node.js versions to use the API,
albeit without getting the advantage of not having to recompile.

In addition, we also plan an external npm package with C++ sugar to
simplify the use of the API. The sugar will be in-line only and will
only use the exported N-API methods but is not part of the N-API
itself. The current version is in:
https://github.com/nodejs/node-api.

This PR is a result of work in the abi-stable-node repo:
https://github.com/nodejs/abi-stable-node/tree/doc,
with this PR being the cumulative work on the api-prototype-8.x
branch with the following contributors in alphabetical order:

Author: Arunesh Chandra <arunesh.chandra@microsoft.com>
Author: Gabriel Schulhof <gabriel.schulhof@intel.com>
Author: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Author: Ian Halliday <ianhall@microsoft.com>
Author: Jason Ginchereau <jasongin@microsoft.com>
Author: Michael Dawson <michael_dawson@ca.ibm.com>
Author: Sampson Gao <sampsong@ca.ibm.com>
Author: Taylor Woll <taylor.woll@microsoft.com>
PR-URL: nodejs#11975
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins added a commit that referenced this pull request Apr 16, 2018
Add support for abi stable module API (N-API) as "Experimental feature".
The goal of this API is to provide a stable Node API for native
module developers. N-API aims to provide ABI compatibility guarantees
across different Node versions and also across different
Node VMs - allowing N-API enabled native modules to just work
across different versions and flavors of Node.js without recompilation.

A more detailed introduction is provided in:
https://github.com/nodejs/node-eps/blob/master/005-ABI-Stable-Module-API.md
and https://github.com/nodejs/abi-stable-node/blob/doc/VM%20Summit.pdf.

The feature, during its experimental state, will be guarded by a runtime
flag "--napi-modules". Only when this flag is added to the command line
will N-API modules along with regular non N-API modules be supported.

The API is defined by the methods in "src/node_api.h" and
"src/node_api_types.h". This is the best
starting point to review the API surface. More documentation will follow.

In addition to the implementation of the API using V8, which is included
in this PR, the API has also been validated against chakracore and that
port is available in
https://github.com/nodejs/abi-stable-node/tree/api-prototype-chakracore-8.x.

The current plan is to provide N-API support in versions 8.X and 6.X
directly. For older versions, such as 4.X or pre N-API versions of 6.X,
we plan to create an external npm module to provide a migration path
that will allow modules targeting older Node.js versions to use the API,
albeit without getting the advantage of not having to recompile.

In addition, we also plan an external npm package with C++ sugar to
simplify the use of the API. The sugar will be in-line only and will
only use the exported N-API methods but is not part of the N-API
itself. The current version is in:
https://github.com/nodejs/node-api.

This PR is a result of work in the abi-stable-node repo:
https://github.com/nodejs/abi-stable-node/tree/doc,
with this PR being the cumulative work on the api-prototype-8.x
branch with the following contributors in alphabetical order:

Author: Arunesh Chandra <arunesh.chandra@microsoft.com>
Author: Gabriel Schulhof <gabriel.schulhof@intel.com>
Author: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Author: Ian Halliday <ianhall@microsoft.com>
Author: Jason Ginchereau <jasongin@microsoft.com>
Author: Michael Dawson <michael_dawson@ca.ibm.com>
Author: Sampson Gao <sampsong@ca.ibm.com>
Author: Taylor Woll <taylor.woll@microsoft.com>
Backport-PR-URL: #19447
PR-URL: #11975
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Apr 16, 2018
@bnoordhuis bnoordhuis mentioned this pull request Apr 18, 2018
3 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.