NODE_MODULE_VERSION for Electron Major Releases

[codebytere]

Related to #621
See Also: electron/node-abi#55

At present Electron has the same ABI number for Electron 3 and Electron 4, when in reality the ABI changed between them and so this is causing issues for some consumers.

Would it be possible for us to get our own ABI versions for each major Electron release?

cc @MarshallOfSound

[mcollina]

Given that Electron is an embedder of Node.js, what is currently blocking you to do this?

Will it have any downsides?

[mhdawson]

I think there has been some earlier discussion along these lines. I don't think Node.js would do any releases etc. instead what would be needed is so that the format of the ABI number be structured so that embedders would add their own versions without overlapping each other of further Node.js versions. I think @ofrobots had suggested possibly assigning some of the higher bits for this purpose.

[mhdawson]

I see the issue above #621 is where @ofrobots made his suggestion :). I also see that @rvagg had an alternate approach where there would be a registry with a way to reserve numbers.

I guess the action being requested is for us (Node.js) to agree/decide on one of these approaches.

[MarshallOfSound]

I guess the action being requested is for us (Node.js) to agree/decide on one of these approaches.

Pretty much 😄

We've been following along with these issues and the PR that @rvagg raised but now this has actually manifested as an issue with Electron 3 vs 4 we need to get a unique ABI number. Either the higher-order-bit, an allocated range, or laying claim to a number in a registry works for us (although the higher-order-bit or an allocated range would be easier for us to manage). Just need Node.JS to say "do this" 😀

[Trott]

Is it too early to put a tsc-agenda label on this just to get a decision made?

[mhdawson]

I'd say add it to the agenda so we can discuss and get agreement (if we can) on which of the two directions is preferred. Bits or registry. I'll add, if people disagree please let me know and we can remove.

[gabrielschulhof]

I'm concerned about #621 (comment) (explosion of binaries with different versions numbers for pre-compiled addon providers to maintain). There are also two issues here, AFAICT:

1. For maintainers to know which Node.js version ships which ABIs, and
2. For the addon to advertise which (combination of) Node.js ABIs it supports.

I believe that, if we are to avoid the combinatorial explosion resulting from assigning version numbers to each ABI Node.js exposes, we need more than just vendor plus Node.js ABI version in the nm_version field. We also need a way for the field to indicate that the module does not care about, say, the openssl ABI version. So, we need to further partition the bits, and have a zero in a given field indicate that the module does not care about that particular ABI. This would be true for all ABI-related fields except the highest order one, which would be the vendor field, where a zero would indicate that the module is designed to run against vanilla Node.js.

The registry approach, although AFAICT more open-ended, I don't think allows modules to indicate which ABIs they don't care about.

Do we have a list of ABIs whose version would influence the final version number advertised by the addon? I'm thinking, like,

ABI
vendor
V8
openssl
libuv
ICU(?)
c-ares(?)

We can also make struct node::node_module a variable-length structure where the fields currently present would be followed by a new field indicating how many ABI versions follow and then a sequence of ABI versions. A value of -2 in nm_version (-1 is taken by N-API) would then indicate to the addon loader that it needs to consult the ABI versions in detail.

[mcollina]

How is this could affect prebuilt libraries? Could an author load a different prebuilt based on this composed ABI?

[gabrielschulhof]

@mcollina the prebuild creation tools like node-pre-gyp and prebuild would have to be updated to deal with all these new versions that Node.js exposes and encode them in the file names that get uploaded, but, if authors do not care about most of the ABIs they would still only need to build a few binaries for the combinations they care about.

[gabrielschulhof]

There's another aspect too. We are trying to move addons towards using a special symbol for startup rather than node::node_addon + a library constructor, because library constructors only get called once per process, so such addons would not support workers and/or multiple contexts.

Currently the special symbol's name includes the module version. It's declared, like,

void node_register_module_v64(v8::Local<v8::Object> exports,
                              v8::Local<v8::Object> module,
                              v8::Local<v8::Context> context);

with the help of src/node.h#L540-L560

So, if we want such an addon to first report the ABI versions it supports, we might want to make this a two-step process where the special symbol is a function that returns the list of ABIs supported and the entry point for the addon.

[MylesBorins]

I'm personally interested in @ofrobots initial suggest of embedders being able to register a most significant bit for their version. This scales to n number of releases and avoids us having to increment the version number every time new platforms need to bump (which I don't see as scalable)

[mhdawson]

@MylesBorins in the meeting today we agreed to split that longer-term discussion out to a new issue. @gabrielschulhof is going to comment in this issue how Electron can get an ABI number to address the specific current issue that is affecting them ( following the existing process) and then we can use the new issue to discuss how improve the process without forcing Electron to wait for that to be agreed.

[ofrobots]

(explosion of binaries with different versions numbers for pre-compiled addon providers to maintain)

This problem already exists. For example, the grpc native module ships around ~130 builds of their native module for different environments. Many of these environments have the same NODE_MODULE_VERSION, so there is no safety net for users at load time. The only way an addon author would know that their precompiled binary didn't work in a specific environment is when a user reports a segfault.

Do we agree that different environments that present different ABIs should use different NODE_MODULE_VERSION? If so, the mechanism through which we express the different version doesn't change the calculus; whether it is a reserved range, a registry or a higher order bit.

The problem with the registry is that each potential vendor/environment has to continue working with us on an ongoing basis to register their ABI for each change (i.e. not scalable).

For maintainers to know which Node.js version ships which ABIs, and

Actually this is not a requirement. They can simply not ship a precompiled binary, or ship precompiled binaries for environments they care about (say official node builds).

For the addon to advertise which (combination of) Node.js ABIs it supports.

Again, not shipping a binary works, which means everything is supported. An addon doesn't need to ship a binary for an environment it doesn't care about. That is not a question about support, but rather about whether the addon cares enough about the UX for specific ABI environments to ship a precompiled version for it.

We also need a way for the field to indicate that the module does not care about, say, the openssl ABI version. So, we need to further partition the bits, and have a zero in a given field indicate that the module does not care about that particular ABI.

This is going to be 1) incredibly hard for addon authors, and 2) makes the version numbers explode even more. Tooling can be written to manage this, but it is not clear to me whether newly created tooling will have broad ecosystem adoption at this point. IMO, this makes the situation more complex, not less.

[ofrobots]

For the shorter term, to unblock electron, I think it is okay to allocate another ABI version number. I'm happy to split the longer term discussion into another issue.

[gabrielschulhof]

(explosion of binaries with different versions numbers for pre-compiled addon providers to maintain)

This problem already exists. For example, the grpc native module ships around ~130 builds of their native module for different environments. Many of these environments have the same NODE_MODULE_VERSION, so there is no safety net for users at load time. The only way an addon author would know that their precompiled binary didn't work in a specific environment is when a user reports a segfault.

From the ~130 builds it sounds like grpc has found a way around the false success of a matching NODE_MODULE_VERSION. Can you link to the code that decides which binary of the 130 to load in any given running Node.js process?

Do we agree that different environments that present different ABIs should use different NODE_MODULE_VERSION? If so, the mechanism through which we express the different version doesn't change the calculus; whether it is a reserved range, a registry or a higher order bit.

Different environments that present different ABIs should definitely be distinguishable, but we need to first of all decide the granularity with which we wish to summarize an ABI signature. So far we have vendor and V8. Does a single value for vendor encapsulate a unique combination of OpenSSL, libuv, libicu, and c-ares? If not, then what are the ABIs that appear alongside one another independently?

The problem with the registry is that each potential vendor/environment has to continue working with us on an ongoing basis to register their ABI for each change (i.e. not scalable).

Agreed, but a global picture guarantees that clashes will be avoided, even with the high-order-bit solution. Granted though, the high-order-bit solution seems to me more scalable.

For maintainers to know which Node.js version ships which ABIs, and

Actually this is not a requirement. They can simply not ship a precompiled binary, or ship precompiled binaries for environments they care about (say official node builds).

For the addon to advertise which (combination of) Node.js ABIs it supports.

Again, not shipping a binary works, which means everything is supported. An addon doesn't need to ship a binary for an environment it doesn't care about. That is not a question about support, but rather about whether the addon cares enough about the UX for specific ABI environments to ship a precompiled version for it.

IINM this discussion is limited to those addon maintainers who wish to provide pre-build binaries – which by no means renders it a niche discussion. Quite the contrary, it is highly desirable to not have to tell your users to install a compiler, make, and python. So it is desirable to provide a list of environments from which maintainers can choose the ones for which they care to pre-build their binaries. The list today is rather well-known even if unwritten: Node.js, Electron, Debian (at least AFAIK :)).

We also need a way for the field to indicate that the module does not care about, say, the openssl ABI version. So, we need to further partition the bits, and have a zero in a given field indicate that the module does not care about that particular ABI.

This is going to be 1) incredibly hard for addon authors, and 2) makes the version numbers explode even more. Tooling can be written to manage this, but it is not clear to me whether newly created tooling will have broad ecosystem adoption at this point. IMO, this makes the situation more complex, not less.

Today we have vendor/V8. If we decide that this level of granularity is sufficient, then the high-order-bit solution will work. It will also mean that any part of the ABI (such as OpenSSL) can only be changed along with a new major version of Node.js. For Debian, where nodejs.deb depends on libssl.deb, this means that libssl.deb cannot be upgraded to a new major version until nodejs.deb is upgraded to a new major version.

OTOH, if we decide that we want more granularity, we will absolutely have to have a way of saying that an addon doesn't care about OpenSSL, or any other ABI we choose to start tracking separately.

[gabrielschulhof]

For addon authors to declare which ABIs their addon uses should not be hard. With nodejs/node#25539 it would be as simple as

NODE_MODULE_DECLARE_ABI(
    NODE_MODULE_ABI_LIBUV_VERSION,
    NODE_MODULE_ABI_OPENSSL_VERSION)

in addition to the current

NODE_MODULE(NODE_GYP_MODULE_NAME, Init)

[ofrobots]

Today we have vendor/V8. If we decide that this level of granularity is sufficient, then the high-order-bit solution will work.

I am arguing that this is sufficient.

It will also mean that any part of the ABI (such as OpenSSL) can only be changed along with a new major version of Node.js.

Right. As intended. We do not ship ABI changes in minor/patch versions.

For Debian, where nodejs.deb depends on libssl.deb, this means that libssl.deb cannot be upgraded to a new major version until nodejs.deb is upgraded to a new major version.

Right. As intended. Debian cannot make an ABI in-compatible change to libssl.deb without breaking the rest of the system.

[gabrielschulhof]

@ofrobots wait a sec ... we've been talking about OpenSSL here. But what about addons that only use V8? Is the V8 portion of the ABI also incompatible between Debian, Electron, and nodejs.org for any given major version? I mean, there are instances where, if the NODE_MODULE_VERSION matches, it really does mean that the addon will work. Now, for those cases and for those addons, we're asking maintainers to add as many builds as there will be high-order-bit values even though they don't need to.

[mhdawson]

@gabrielschulhof I think the next action we wanted was to unblock Electron by getting them a number they can use in the case.

[gabrielschulhof]

@mhdawson @codebytere I think the easiest solution is for @codebytere to submit a PR which simply bumps the NODE_MODULE_VERSION.

[gabrielschulhof]

NM – the ABI break is in V8.

[Trott]

Removing tsc-agenda label per @mhdawson who added it in the first place. If anyone disagrees with removing this item from this week's TSC agenda, they may leave a comment in #659 and the item will be re-added to the agenda.

[mhdawson]

@Trott thanks, missed that part when I removed it from the agenda.

[rvagg]

So we've got quite side-tracked here from the original request, for good reason obviously, but we need to get Electron unblocked according to the current state of play.

We discussed this in our TSC meeting today (private section sorry, there was nothing sensitive about the discussion, I just forgot to raise it while we were recording). And came up with a rough plan for moving forward. First though we need to unblock this issue.

@codebytere asked:

Would it be possible for us to get our own ABI versions for each major Electron release?

The answer is: yes, you can have as many as you need. But for right now, the process requires some coordination so let's reserve the numbers you need.

If Electron would like one, right now 69 is available. I have the latest list updated in nodejs/node#24114 and if you want 69 I can add that in and we'll make sure we don't use it ourselves. Let us know here and we can consider this issue resolved.

For now, the process as documented is still to open an issue here to request a new number and we'll take care of the rest, as in #621. But that will likely change, because it's leading to discussions like this and slow resolution to something that should be simple!

---

So, on to the other matters discussed here.

The idea of handing out higher order bits so you can move independently is on the table, but even if we do that, a central registry is going to be helpful for coordinating amongst prebuild tools that manage binaries so we'll probably want some central reporting mechanism—it just won't need to be used as a mutex and can take reporting after the fact. We're also going to end up with awkward strings which addon authors are going to have to use (i.e. they currently put the modules number in the filename of binaries (e.g)), not a blocker, just likely confusing. 16777285 or 0x1000045 if we gave Electron an incremented highest order bit for 32-bit integers for example (I think). @ofrobots are you seeing a slightly more elegant version than this? We should probably check with prebuild tool authors on this when we have a clearer proposal and perhaps we should come up with a proposal specifically for the Electron case first since many addons are already publishing binaries for it.

There's also @gabrielschulhof's nodejs/node#25539, which is additive rather than a replacement to anything we're talking about. A registry, or at least record of values behind some of the _ABI_ numbers may also be useful rather than just tracking increments in our header files, so I've made a note in my registry PR nodejs/node#24114 that being able to accommodate such data in the future should be considered.

[gabrielschulhof]

The goals and facts below are reflected to some extent in my PR, but I think we should think about them first before we think about any implementation.

Goals:

• An addon loads exactly when its ABI matches and fails with a thrown exception otherwise. Bonus: The exception reveals where in the ABI the mismatch was found.
• Addon maintainers need to build as few binaries as possible.
• Addon maintainers need to run their build as rarely as possible. Ideally they never need to bump their version because a new version of Node.js appears bearing a new version of an ABI that they don't use in their addon.

Facts:

• Vendors are the only source of ABI-incompatibility within a major version.
• The ABI for a given dependency (like, say, libuv) may match across vendors, even though the ABI for another dependency (like, say, OpenSSL) does not (I'm unsure of this last one).

Interesting use case:
An addon uses N-API and libuv. Its maintainer needs to ship binaries for each vendor libuv ABI. Once they've done that, they can go for essentially years without shipping again, because N-API doesn't change at all, and libuv changes very rarely, even across major Node.js versions.

[codebytere]

@rvagg nice, that'd be great! We'll also need one for Electron v5, which is in the beta process right now. Should we open a new issue for that or can we continue that in this thread?

[nornagon]

Ping on ^. We're currently using NODE_MODULE_VERSION=68 (somewhat by accident) in Electron v5. It looks like node will be skipping past 68, so there should be no conflict there.

Before this happens again for Electron v6, we'd like to reserve a NODE_MODULE_VERSION. Should we open a new issue?

[Trott]

If there's not a single agreed-upon clear path forward here, it might be time to add the tsc-agenda label again.

[rvagg]

@nornagon 68 is taken for the V8 7.1 upgrade, see nodejs/node#23423

I've added 69 for Electron 4 and 70 for Electron 5 in nodejs/node#24114

  { "modules": 70, "runtime": "electron", "variant": "electron",             "versions": "5" },
  { "modules": 69, "runtime": "electron", "variant": "electron",             "versions": "4" },
  { "modules": 68, "runtime": "node",     "variant": "v8_7.1",               "versions": "12.0.0-pre" },
  { "modules": 67, "runtime": "node",     "variant": "node",                 "versions": "11" },

The onus is on us to make sure we don't step on those toes. Maybe Node 12 gets 71.

@Trott I don't think there's really disagreement here, there's multiple approaches but they don't necessarily conflict, it's more about ideals and timeframes. I think we can just move forward on making nodejs/node#24114 workable and merged, then we can progress from there with alternative approaches to this whole thing.

[rvagg]

FYI the 7.3 PR nodejs/node#25852 is getting 71. It might not need another bump for Node 12 since the ABI pieces will probably be stable after V8 7.3 is landed.

[targos]

It might need another bump if we can get V8 7.4 in.

[miniak]

@nornagon newer version of Electron should definitely not have a lower NODE_MODULE_VERSION

[nornagon]

In seriousness though, I agree though and I think we should bump the NODE_MODULE_VERSION in Electron 5 beta. Looks like we're free to take 70 so let's do that.

[rvagg]

I think that the ABI version registry @ nodejs/node#24114 is ready to land. I've renamed it to "abi_version_registry.json" and bumped all of the module version numbers into a "NODE_MODULE_VERSION" property of the parent object, so now it's extensible and should be able to adapt to future approaches to dealing with ABI matching (if it requires a registry). For now though, it's a helpful record of who's using what and it'll be a good record for that into the future even if we move to a new mechanism.

Please review and register any objections so we can get this in and on the official record.

[mhdawson]

@codebytere did you get the number you needed and if so can we close this issue?