Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Proposal: increment NODE_MODULE_VERSION on each dev release #5789

Closed
rvagg opened this issue Jul 4, 2013 · 12 comments
Closed

Proposal: increment NODE_MODULE_VERSION on each dev release #5789

rvagg opened this issue Jul 4, 2013 · 12 comments

Comments

@rvagg
Copy link
Member

rvagg commented Jul 4, 2013

Each 0.11 release seems to have some breakage of the C++ API, which is to be expected. But this causes breakage for native addons and even though dev release are caveat emptor we have a simple mechanism in place to help deal with the breakage: NODE_MODULE_VERSION.

I'd like to propose that, regardless of what's going on with the C++ API, that this number be incremented for each 0.11 (dev) release. it can be incremented for stable releases when absolutely necessary but this is rare. The incrementing between dev and stable would need to be synchronized, so if we got to 0.11.10 and were on 15 and a stable 0.10.20 needed to be incremented then it could just be made 16 and 0.11.11 would become 17. There's no great significance to these numbers except that they need to be unique for each version of the API.

Lots of people are jumping on 0.11 because of the --harmony goodness (generators seem to be exciting atm), how about we make it easier for them when they're also doing it with native addons and instead of giving them cryptic errors because of incompatibilities when they hit them, give them a clear message about version incompatibilities and the need to recompile.

@TooTallNate
Copy link

I don't particularly see anything wrong with this idea...

@tjfontaine
Copy link

I think it's sane that when we do introduce an incompatibility we bump it, and certainly things are more fragile during the unstable release, but I'm not sure we want to be in the habit of rev'ing while we're iterating.

However, I would hope that those the most willing to adopt and play with our unstable releases are also the ones most tolerant and understanding of the implicit (non)contract that comes with living on the edge. One would also imagine that they are the ones most likely to file bugs and report issues as they crop up.

In general, I agree we need to be more disciplined and vigilant in this area, as indicated by the fact that https://github.com/joyent/node/wiki/API-changes-between-v0.10-and-v0.12 is woefully empty, and we've had at least one major breaking change in the form of @trevnorris's new buffer implementation. Although I'm relatively certain that we did bump the version when that change landed.

(edited some spelling)

@rvagg
Copy link
Member Author

rvagg commented Jul 4, 2013

Well, there's this: #5703 which is why I'm making a new issue to discuss this. There was the new Buffer work, the v8 upgrade which required the use of the current isolate everywhere to avoid deprecation warnings, the removal of node_isolate from the public API, and I thought there was some other minor change that I can't find the details of right now (an additional argument was added to something, somewhere).
So, instead of having to think too much about it, I'm proposing that incrementing NODE_MODULE_VERSION is just something that gets done for each 0.11 because stuff is likely to have broken underneath and it'd be nice to see a sensible error message rather than something whacky.

@tjfontaine
Copy link

I hope you've updated the wiki with all those things :)

I understand your pain point, I'm just not necessarily a fan of doing it automatically, I'd prefer it be a better part of process in general.

But I'll wait for others to chime in.

@trevnorris
Copy link

Well, technically we shouldn't need to increment that value in a stable release. In fact I'm pretty sure we'd purposefully delay implementation of an api change so it wouldn't happen.

What I'm getting at is if you're on master then you should be on latest release, and honestly I think mentions in the ChangeLog or some such that a change broke a specific subset of API should be enough.

As far as the upgrade doc from v0.10 to v0.12, it's on my to do list, but there are more significant changes coming down the pipe so it doesn't seem useful to do right now. But it seems this should be kept up to date with each release so developers know what's going on.

Honestly though, there are so many API changes coming from v8 I'd be surprised if developers could keep up with master. Ben's working on implementing v8 3.20 right now and that's going to introduce another set of pain. And we still haven't gotten to them deprecating Handle.

@bnoordhuis
Copy link
Member

So, are we going to do anything with this?

My $.02: I'm -0.5 on auto-incrementing NODE_MODULE_VERSION. I don't mind it too much if we start doing that but it seems kind of pointless. Dev releases are by nature unstable - if you (generic you) as a user don't know what that entails or how to deal with the inevitable breakage, you shouldn't be running them.

Incrementing NODE_MODULE_VERSION when we put out -rc1 on the other hand seems like a very good idea. :-)

@springmeyer
Copy link

Landing here because:

  • I maintain the userland tool node-pre-gyp that allows for shipping binaries for node c++ addons and depends on NODE_MODULE_VERSION / process.versions.modules
  • The existence and stability of NODE_MODULE_VERSION across stable/even node.js versions is extremely helpful and it is hard to image node-pre-gyp being viable without it. So thank you for the vision and maintenance of this property.
  • However, I assumed that NODE_MODULE_VERSION was meaningful for both stable and dev series and therefore;
  • Have been shipping users ABI incompatible binaries recently leading to crashes like Segmentation Fault Under Node v0.11.14 TryGhost/node-sqlite3#352 when users upgrading to v0.11.14 still got binaries built against v0.11.13.

So, I'm here to report that:

  • I've found that shipping binaries - even for the unstable series - is very valuable
  • Now that I realize there is no consensus or promise that NODE_MODULE_VERSION is incremented during the unstable series, I will move to make node-pre-gyp version the node unstable series on major.minor.patch instead of NODE_MODULE_VERSION: Version Node unstable/odd series on major.minor.patch mapbox/node-pre-gyp#124
  • However, as you can imagine, I would love to see NODE_MODULE_VERSION be meaningful in the future in a consistent way across all node releases because it would simply node-pre-gyp and it would allow binaries to work in some cases across unstable versions (say when v8 is not upgraded between releases).

@springmeyer
Copy link

Hmm, for the record I am noticing that NODE_MODULE_VERSION has been incremented in the v0.11.x series. Node v0.11.0 started at 12, node v0.11.8 bumped to 13, and v0.11.13 bumped to 14. This is serialized at https://github.com/mapbox/node-pre-gyp/blob/e5a8618f75453a4b7739fc8d80ce8add6c7f6e7f/lib/util/abi_crosswalk.json#L254-L313 (generated via https://github.com/mapbox/node-pre-gyp/blob/master/scripts/abi_crosswalk.js)

@bnoordhuis
Copy link
Member

FWIW, I've changed my mind on this. My original objection was that dev releases are just for testing. But with v0.11's, shall we say, longevity, people are running it in production. We should be nice to them.

@jasnell
Copy link
Member

jasnell commented May 26, 2015

@rvagg ... any reason to keep this open here?

@rvagg
Copy link
Member Author

rvagg commented May 27, 2015

hah, 2 years old .. closing

@rvagg rvagg closed this as completed May 27, 2015
@mikemorris
Copy link

Discovered that the Node.js v0.12.x stable series is upgrading v8 and libuv in patch versions without bumping NODE_MODULE_VERSION. This is breaking important assumptions in node-pre-gyp, and may be causing binaries built on v0.12.0 to be ABI-incompatible when attempting to run on v0.12.4.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants