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

doc: add guide about abi stability #23229

Closed
wants to merge 2 commits into from

Conversation

gabrielschulhof
Copy link
Contributor

Re: nodejs/abi-stable-node#332 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Oct 2, 2018
@vsemozhetbyt vsemozhetbyt added the node-api Issues and PRs related to the Node-API. label Oct 2, 2018
# ABI Stability

## Introduction
An Application Binary Interface (ABI) is the compiled version of an Application
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might start with "An Application Binary Interface (ABI) is a way for programs to call other functions and use data structures from other compiled programs (binaries)" or something like that?

agree with those with which the ABI provider was compiled. This is usually
accomplished by compiling against the headers provided by the ABI provider.

Since the ABI provider and the ABI user may be compiled at different times with
Copy link
Member

@benjamingr benjamingr Oct 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ABI provider ('the library being required') and ABI user ('the consuming code') in the document maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't say "the library being required", because in the case of a native addon, it's actually the executable program that is being required.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also wouldn't say "consuming" either, because that's too abstract. I can change it to "provider of the ABI" and "user of the ABI" to make it more down-to-Earth.

the Node.js project has adopted [semantic versioning](https://semver.org/).
This ensures that the APIs provided by the project will result in a stable ABI
for all minor and patch versions of Node.js released within one major version.
In practice, this means that a Node.js native addon compiled against a given
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice, this means that Node.js commits that a native addon compiled against...?


## N-API
Demand has arisen for equipping Node.js with an API that results in an ABI that
remains stable across multiple Node.js versions. The motivation for creating
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multiple major Node.js versions ?


N-API is versioned because new APIs are added from time to time. Unlike
semantic versioning, N-API versioning is cumulative. That is, each version of
N-API conveys the same meaning as a minor version in the semver system.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be painfully obvious: ...and remains backwards compatible.?

existing users of the ABI incompatible with the new version.

## ABI Stability in Node.js
Node.js provides header files maintained by several independent teams. For
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would link to the N-API and native-addon docs here perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean https://nodejs.org/docs/latest/api/addons.html and https://nodejs.org/docs/latest/api/n-api.html? I don't quite understand why I would link to the docs here?

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm very +1 on adding this guide overall and it contains good content - left some nits -I think the start could be more new-user friendly in terms of motivating ABI.

@gabrielschulhof
Copy link
Contributor Author

@benjamingr I addressed your review comments, and I have questions about some of them.

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gabrielschulhof
Copy link
Contributor Author

@gabrielschulhof
Copy link
Contributor Author

@gabrielschulhof
Copy link
Contributor Author

Landed in 65366ad.

gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Oct 6, 2018
Re: nodejs/abi-stable-node#332 (comment)
PR-URL: nodejs#23229
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@gabrielschulhof gabrielschulhof deleted the add-abi-guide branch October 6, 2018 08:56
targos pushed a commit that referenced this pull request Oct 6, 2018
Re: nodejs/abi-stable-node#332 (comment)
PR-URL: #23229
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
targos pushed a commit that referenced this pull request Oct 7, 2018
Re: nodejs/abi-stable-node#332 (comment)
PR-URL: #23229
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants