NODE_MODULE_VERSION debian/sid 8 and 10

[kapouer]

Currently debian/sid ships openssl 1.1.1, and since it favors building nodejs using the shared library,
the NODE_MODULE_VERSION needs to be different for both Node 8 and 10.

Note that this is made possible by work done at nodejs/node#18770.

So i'd like to reserve these for debian:

For Node 8
NODE_MODULE_VERSION 58

For Node 10
NODE_MODULE_VERSION 65

Thanks !

[Trott]

I don't think the TSC is the right repo/group for this request, but I'm not sure what is. The main repo? A release/LTS repo? Something else? @nodejs/tsc

[eli-schwartz]

This is definitely the right repo for this request, via nodejs/node#22237 which added this documentation blurb: https://github.com/nodejs/node/blob/master/BUILDING.md#note-for-downstream-distributors-of-nodejs

[MylesBorins]

/cc @ofrobots who had a good idea for a module numbering scheme

I think the idea was for different organizations to be able to select a most significant bit so we could keep module version number sequential for core

[ofrobots]

Yeah, I was thinking of using the most-significant 8-bits to encode a vendor. Something along the lines of this patch:

diff --git a/src/node_version.h b/src/node_version.h
index d542193332..747cb16bf8 100644
--- a/src/node_version.h
+++ b/src/node_version.h
@@ -114,7 +114,18 @@
  *
  * More information can be found at https://nodejs.org/en/download/releases/
  */
-#define NODE_MODULE_VERSION 67
+#define NODE_MODULE_VERSION_BASE 67
+
+#define NODE_MODULE_VENDOR_OFFICIAL 0
+#define NODE_MODULE_VENDOR_ELECTRON 1
+#define NODE_MODULE_VENDOR_DEBIAN   2
+// More vendors can be added here.
+#define NODE_MODULE_VENDOR_UNKNOWN  255
+
+#define NODE_MODULE_VENDOR NODE_MODULE_VENDOR_UNKNOWN
+
+#define NODE_MODULE_VERSION ((NODE_MODULE_VENDOR << 24) | NODE_MODULE_VERSION_BASE)
+

 // the NAPI_VERSION provided by this version of the runtime
 #define NAPI_VERSION  3

[nicolasnoble]

I'd like to mention that @ofrobots's suggestion means that even if it's adopted now, it can be retroactively and transparently applied to all current versions of the nodejs runtime. As such, Debian's assigned runtime numbers would be 0x02000039 for node 8 (or 33554489 in base 10), and 0x02000040 (or 33554496 in base 10) for node 10.

[kapouer]

Isn't that going to make a somewhat weird shared library name ? libnode33554489.so ?
edit it would be libnode.so.33554489

[Flarna]

Wouldn't this lead to a needless explosion of binaries to build/maintain for native addons shipping pre-compiled binaries?

Most addons use only the v8 APIs therefore difference in OpenSSL,... is no issue for them. But there is no possibility to detect this based on the NODE_MODULE_VERSION.
Native addons are loaded from Javascript where version of all third parties are available. By using this addons which use more then the v8 API can add a dimension to select the right binary.

Similar for addons which are always compiled on NPM install I think it makes more sense to offer specific defines for the third party to select the right API if needed.

For sure it would be nice to allow node to verify compatibility of addons not just based on NODE_MODULE_VERSION, maybe an addon could specify during register the API versions it uses from all third parties?

Edit: I assume it's not planned that debian,... also ships precompiled binaries for NPM modules which do this for official node variant.

[kapouer]

@Flarna you assume right: the "correct" behavior for debian users is to either install a debian package (of a node addon) or build it from source - the only two viable options from a debian user perspective.

(besides, you're very much right about registering/checking API versions a module uses).

[rvagg]

How about we just add a docs/node_module_versions.md doc with a record of what's using what number, then our release docs can be updated to say that you should check that doc to find the "next" number for Node.

This might be helpful to have centrally cause it's getting a bit out of hand and it's useful to know for NAN and probably N-API too. Add-on authors can then cross-reference it for deciding what binaries they want to provide too.

[rvagg]

Trying out the idea of a registry here: nodejs/node#24114, lemme know what you think.

[rvagg]

Hey @kapouer, you'd better double-check whether 1.1.1 against Node 10 is going to be ABI breaking? It's not intended to be on our side and support should be merged into 10 when it's ready in nodejs/node#18770. Of course we need to double check on our end but the OpenSSL team have said it should be ABI and API compatible with 1.1.0 and that's the assumption we're working on too. 1.1.0 support ceases before 10 LTS so this better be the case!
Perhaps you only need to reserve one for 8+1.1.1?

[kapouer]

Yes it is only for nodejs 8+1.1.0. It might not even be needed if i manage to get node 10 in next debian release, so it's more a concern for derivatives right now (ubuntu ?)

[MylesBorins]

What remains for us to do here?

[kapouer]

IMO this has been addressed correctly. Distributors are now aware of the problem thanks to the documentation. I'm inclined to close this.

[MylesBorins]

Closing