-
Notifications
You must be signed in to change notification settings - Fork 263
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
Improve N-API support #402
Comments
I think this would be a good addition. |
@rolftimmermans I am close to submitting a PR. Will you please refresh my memory concerning the circumstances in which a |
@jschlight I believe in all cases? Without a configure step I receive the following error message after a build fails:
The directory Any compiler error (by making a syntax error for example) is enough to trigger it. |
Thank you @rolftimmermans. Got it! It should be fixed in this PR. |
#405 is merged. @jschlight can this now be closed. #405 will go out in the v0.11.0 release. |
Thank you @springmeyer. I'm available to address any issues that may arise from the PR. |
Thanks @jschlight - will /cc if I see anything comeup, as will @mapsam who is also looking after this repo. |
I'd like to create a PR to improve support for N-API builds. I'm posting my proposed changes here for review and comment by the community.
Support NAN and N-API builds from the same project
For some projects, it is desirable to build both legacy NAN modules and N-API modules from the same package. In the current node-pre-gyp implementation, if the project specifies one or more N-API builds by including the
binary.napi_versions
property in thepackage.json
file, and the Node version being built against does not support N-API, node-pre-gyp issues the following error message:The
package.json
substitution string{node_abi_napi}
is intended facilitate NAN and N-API builds from the same project. It can be used to distinguish projects that need this capability.I propose to modify the current build behavior to attempt a NAN build when all of the following conditions are true:
binary.napi_versions
property in thepackage.json
file.binary.package_name
property in thepackage.json
file contains the text{node_abi_napi}{napi_build_version}
.The code that attempts downloads would follow the same rules.
For new modules that are designed for N-API only, there is no need to include the
{node_abi_napi}{napi_build_version}
text. They would not trigger this special rule.The documentation would also be updated to describe this pattern and its usage.
The C/C++ code would need to determine whether the build being requested is for NAN or N-API. The
NAPI_VERSION
symbol communicated to the compiler by node-pre-gyp can be used for this purpose.Verify each napi_build_version against what’s supported by Node
When building for publishing, node-pre-gyp will verify that the build for each N-API version specified in the package.json file is supported by the version of Node being used for the build. If the version is not supported, node-pre-gyp will issue a warning. If no version can be built, node-pre-gyp will issue an error. This will permit add-ons to be tested against older versions of Node that do not support newer N-API versions.
Implement a
--build-latest-napi-version-only
argumentImplement a new build argument,
--build-latest-napi-version-only
, that will build only the highest supported N-API version. This will be useful for test builds where only the most current N-API build is of interest by suppressing builds that are eventually ignored.Improve recovery from build errors
Improve recovery from build errors so that a configure is not required after a build failure.
Documentation update
The current N-API documentation suggests defining the symbol
NAPI_BUILD_VERSION
for the C/C++ code. The recommended value isNAPI_VERSION
and the documentation should be updated to reflect this.The text was updated successfully, but these errors were encountered: