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

NODE_MODULE_VERSION for io.js #150

Closed
springmeyer opened this issue Mar 6, 2015 · 4 comments
Closed

NODE_MODULE_VERSION for io.js #150

springmeyer opened this issue Mar 6, 2015 · 4 comments

Comments

@springmeyer
Copy link
Contributor

@rvagg - could you help me understand why there is an apparent discrepancy in NODE_MODULE_VERSION between the https://iojs.org/download/release/index.json and the released io.js binaries?

I'm noticing that io.js v1.1.0 and v1.2.0 declared NODE_MODULE_VERSION 43 at time of release according to https://iojs.org/download/release/index.json and this got serialized into node-pre-gyp at 683f7a5#diff-a5c0267c9158157e6cb61d2c7d42591a and b3bf90a#diff-a5c0267c9158157e6cb61d2c7d42591a

But today I notice that https://iojs.org/download/release/index.json is now reporting 42 for these releases while 43 is still used at https://github.com/iojs/io.js/blob/b27931b0fedc5fad638d637525aba84b2754ed5f/src/node_version.h#L48

I see some discussion of reverts at nodejs/node#952 and nodejs/node#1026 but I'm not following what is happening. Can you clarify?

@rvagg
Copy link

rvagg commented Mar 6, 2015

@springmeyer this is purely a problem with the index files, a bug that I thought I squashed earlier on but seems to be persisting.

Check out the code that generates it here: https://github.com/iojs/build/tree/master/dist/dist-indexer

I think it's to do with the caching mechanism for these values. The file is re-generated on each release and it shouldn't change old values but apparently it is!

I'll try and make time to look in to this but you're more than welcome to do so yourself using the program at the above URL.

@rvagg
Copy link

rvagg commented Mar 6, 2015

Oh, and the discussion @ nodejs/node#952 and nodejs/node#1026 is adjacent to this. That's all about an ABI change that was introduced by V8 at 4.1.19 or thereabouts that Ben decided not to land so as to avoid the pain that comes with bumping NODE_MODULE_VERSION. We're now live with 4.1.0.21 patched to remove that ABI change. At some point we'll just have to revert the patch and move on with a bump to NODE_MODULE_VERSION but we don't have to just now. My guess is that it'll stay stable for the next ~6 weeks while we wait for 4.2 to go gold at which point we'll just bite the bullet and let the ABI change.

Also, FYI, the V8 team announced yet another round of major revisions to the API so we're probably going to have to introduce a third layer of complexity to NAN and there will be addon pain at that point. I think that might start hitting us in 4.3.

@springmeyer
Copy link
Contributor Author

Thank you for all the details @rvagg! This is super helpful: if confirms that I don't have a broken node-pre-gyp release out there and I'll avoid updating the abi from 42 -> 43. BTW, the reason I store a file with ABI mapping internally is to support the --target flag for when a dev wants to build binaries for a release that is not the version installed. I could potentially grep into the node-gyp downloaded headers to dynamically get the NODE_MODULE_VERSION but pre-computing this has always seemed smarter (with the downside of needing to keep up!).

@springmeyer
Copy link
Contributor Author

this is working now, thanks to whomever fixed this upstream!

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

No branches or pull requests

2 participants