atom-shell causing problems #226

kkoopa · 2015-01-14T19:48:57Z

I hope this is just some temporary fork and that they'll move to io.js instead of running yet another fork. It will be impossible to support three things each claiming to be Node without any reliable way of telling them apart. Why can't v8 announce which version it is?

bnoordhuis · 2015-01-14T19:53:49Z

Why can't v8 announce which version it is?

I can try sending a patch. I remember looking into it one time and not quite being able to figure out how their homegrown release script updated version numbers.

kkoopa · 2015-01-14T19:57:03Z

That would be good. However, Node forkers need to backport that patch too, because having it only in the very latest version of v8 won't do much good in the short time.

bnoordhuis · 2015-01-14T19:58:18Z

You can detect that the version macro is not there, that at least tells you something. But yes, it won't do a great deal of good in the short term.

kkoopa · 2015-01-14T20:03:14Z

What if every fork could add a define to node-version.h

#define MY_UNIQUE_DEFINITION 1

or use a standard name, but unique value,

#define NODE_FORK_IDENTIFIER 23

bnoordhuis · 2015-01-14T20:07:38Z

It could work but it's not ideal because we're really only interested in the V8 version. Also, it may be an unreliable indicator when linked against a shared libv8.

waTeim · 2015-01-14T20:08:32Z

Here's some workaround possibilities; tl;dr PREPARSER_H in in 3.25 and 3.26 but not in 3.29, lots of changes in v8 3.14 to 3.25 (V8_ARRAY_BUFFER_VIEW_INTERNAL_FIELD_COUNT for instance) I have not downloaded iojs yet.

node 11.14 and 11.13 (v8 3.26.33, 3.25.30)

bizarro% grep \\#define v8*
v8-debug.h:#define V8_V8_DEBUG_H_
v8-platform.h:#define V8_V8_PLATFORM_H_
v8-preparser.h:#define PREPARSER_H
v8-preparser.h:#define V8EXPORT __declspec(dllexport)
v8-preparser.h:#define V8EXPORT __declspec(dllimport)
v8-preparser.h:#define V8EXPORT
v8-preparser.h:#define V8EXPORT __attribute__ ((visibility("default")))
v8-preparser.h:#define V8EXPORT
v8-profiler.h:#define V8_V8_PROFILER_H_
v8-testing.h:#define V8_V8_TEST_H_
v8-util.h:#define V8_UTIL_H_
v8.h:#define V8_H_
v8.h:#define TYPE_CHECK(T, S)                                       \
v8.h:#define V8_ARRAY_BUFFER_INTERNAL_FIELD_COUNT 2
v8.h:#define V8_ARRAY_BUFFER_VIEW_INTERNAL_FIELD_COUNT 2
v8config.h:#define V8CONFIG_H_
v8stdint.h:#define V8STDINT_H_

atom-shell (v8 3.29.88.17)

bizarro% grep \\#define v8*
v8-debug.h:#define V8_V8_DEBUG_H_
v8-platform.h:#define V8_V8_PLATFORM_H_
v8-profiler.h:#define V8_V8_PROFILER_H_
v8-testing.h:#define V8_V8_TEST_H_
v8-util.h:#define V8_UTIL_H_
v8.h:#define V8_H_
v8.h:#define TYPE_CHECK(T, S)                                       \
v8.h:#define V8_ARRAY_BUFFER_INTERNAL_FIELD_COUNT 2
v8.h:#define V8_ARRAY_BUFFER_VIEW_INTERNAL_FIELD_COUNT 2
v8config.h:#define V8CONFIG_H_
v8stdint.h:#define V8STDINT_H_

node 10.35 (v8 3.14.5.9)

v8-debug.h:#define V8_V8_DEBUG_H_
v8-debug.h:#define EXPORT __declspec(dllexport)
v8-debug.h:#define EXPORT __declspec(dllimport)
v8-debug.h:#define EXPORT
v8-debug.h:#define EXPORT __attribute__ ((visibility("default")))
v8-debug.h:#define EXPORT
v8-platform.h:#define V8_V8_PLATFORM_H_
v8-preparser.h:#define PREPARSER_H
v8-preparser.h:#define V8EXPORT __declspec(dllexport)
v8-preparser.h:#define V8EXPORT __declspec(dllimport)
v8-preparser.h:#define V8EXPORT
v8-preparser.h:#define V8EXPORT __attribute__ ((visibility("default")))
v8-preparser.h:#define V8EXPORT
v8-profiler.h:#define V8_V8_PROFILER_H_
v8-profiler.h:#define V8EXPORT __declspec(dllexport)
v8-profiler.h:#define V8EXPORT __declspec(dllimport)
v8-profiler.h:#define V8EXPORT
v8-profiler.h:#define V8EXPORT __attribute__ ((visibility("default")))
v8-profiler.h:#define V8EXPORT
v8-testing.h:#define V8_V8_TEST_H_
v8-testing.h:#define V8EXPORT __declspec(dllexport)
v8-testing.h:#define V8EXPORT __declspec(dllimport)
v8-testing.h:#define V8EXPORT
v8-testing.h:#define V8EXPORT __attribute__ ((visibility("default")))
v8-testing.h:#define V8EXPORT
v8-util.h:#define V8_UTIL_H_
v8.h:#define V8_H_
v8.h:#define V8EXPORT __declspec(dllexport)
v8.h:#define V8EXPORT __declspec(dllimport)
v8.h:#define V8EXPORT
v8.h:#define V8EXPORT __attribute__ ((visibility("default")))
v8.h:#define V8EXPORT
v8.h:#define V8EXPORT
v8.h:#define TYPE_CHECK(T, S)                                       \
v8config.h:#define V8CONFIG_H_
v8stdint.h:#define V8STDINT_H_

kkoopa · 2015-01-16T14:25:55Z

Seems atom-shell is about to upgrade to Chromium 40 and will raise NODE_MODULE_VERSION to 100 atom/node@83f1b41 and move to io.js. Hopefully the original problem will resolve itself then.

However, being able to actually condition on the v8 version is still highly desirable.

waTeim · 2015-01-16T22:37:13Z

100? lol. But there are still incompatible v8 that contextify (among others) have to worry about, though now it's 3 instead of 4 (although I don't know what will happen with node-webkit) if I remember correctly, the list node minor = 10,11,13,17,100 (but 100 == 17), or are you saying that io.js will also change to 100, but no atom might want their freedom and fork io.js too.

I think I have a solution in mind though, but it will take some cooperation. Does anyone not use node-gyp these days?

kkoopa · 2015-01-21T14:56:16Z

io.js will stay in range 42 -- 100, atom will be 100+, nw.js will bundle io.js
I imagine atom needs forking mostly to match v8 versions with chrome, which no other project cares about.

This might all still be manageable, but the underlying problem of not being able to reliably identify the version of v8 remains.

mathiask88 · 2015-01-21T15:03:12Z

Why do node.js and io.js not offer a V8 version macro of the embedded version? I think the V8 team won't add a macro because there were some tries before and they were rejected. But still a backporting problem :/

kkoopa · 2015-01-21T15:13:27Z

The problem is shared dependencies. Lesser packagers insist on removing bundled dependencies with no regard to patches and modifications.

waTeim · 2015-01-21T16:51:47Z

This can be solved, I feel, so long as the maintainers of node-gyp are agreeable.

zcbenz · 2015-01-23T02:53:43Z

Hi, I'm upgrading atom-shell to Chrome 40, and I have noticed the problem with compatibility with nan.

In v1.6, nan assumes NODE_MODULE_VERSION < 42 is Node, otherwise it would be io.js v1.0.0. However atom-shell uses neither, we are currently using atom/node@0d02515 which is io.js < v.1.0.0, because atom-shell uses the stable release of Chrome and from atom/node@7f9a6c6 io.js started to use ES6 octal literals that is not available in Chrome 40.

Once after atom-shell starts to use Chrome 41, I can safely set NODE_MODULE_VERSION to 42 to make atom-shell compatible with nan, however currently changing NODE_MODULE_VERSION to neither 42 or some value smaller won't work.

How about making nan compatible with V8 3.30.33.15 (used by Chrome 40) when NODE_MODULE_VERSION is set to 41? So nan still works with Node and io.js v1.0.0, and I can set NODE_MODULE_VERSION to 41 in atom-shell to make it compatible with nan.

And please make sure NODE_MODULE_VERSION in io.js bumps when V8 version changes, otherwise we will still have this headache in future.

kkoopa · 2015-01-23T10:20:32Z

That might work, but the version matrix is already rather complicated. Adding another exception is not the preferred way, but remains a last resort. A related problem is testing, does atom run on travis? How to set it up? Can't know if something breaks if we can't test it.

io.js would not be interested in following chrome versions, will this problem not show up again in the future?

bnoordhuis · 2015-01-23T11:48:45Z

There is still some ongoing discussion on how to set up the release train (nodejs/node#544) but I believe everyone agrees that io.js will stick to stable V8 releases in the future, meaning it's going to ship the V8 from the latest Chrome release. We're currently at 4.1 and that won't change until 4.2 goes stable.

zcbenz · 2015-01-23T18:16:14Z

That might work, but the version matrix is already rather complicated. Adding another exception is not the preferred way, but remains a last resort.

The essence of this is to support a pre-release of io.js, so I don't think it is really an exception.

Another way is to use the NODE_VERSION_AT_LEAST and NODE_VERSION_IS_RELEASE to determine the version, the io.js version compatible with Chrome 40 is 1.0.0-pre, but I don't know if it is a better way.

I also tried to tweak the NODE_MODULE_VERSION value, but resulted in vain.

A related problem is testing, does atom run on travis? How to set it up? Can't know if something breaks if we can't test it.

The simplest way to build a-native-module with a specific version of atom-shell is to use apm, following commands can build a-native-module with atom-shell v0.21.0's headers, which uses io.js v1.0.0-pre and set NODE_MODULE_VERSION to 41:

$ npm install -g atom-package-manager
$ cd /path/to/a-native-module
$ env ATOM_NODE_VERSION=0.21.0 apm install .

You can also use node-gyp or npm to test, the trick is to replace the dist-url to atom-shell's headers.

io.js would not be interested in following chrome versions, will this problem not show up again in the future?

As long as io.js bumps the NODE_MODULE_VERSION when it updates the V8 version, it won't be a problem. We have this problem because io.js didn't do that, but it makes sense since it was before v1.0.0.

but I believe everyone agrees that io.js will stick to stable V8 releases in the future, meaning it's going to ship the V8 from the latest Chrome release

I'm glad to see this, from the perspective of embedders, this is really good news.

kkoopa mentioned this issue Jan 14, 2015

Support Atom shell's version of node and v8 brianmcd/contextify#145

Closed

kkoopa mentioned this issue Jan 18, 2015

NAN 1.6 #238

Merged

zcbenz mentioned this issue Jan 23, 2015

Upgrade to Chrome 40 electron/electron#1002

Merged

maxkorp mentioned this issue Feb 3, 2015

Building for atom-shell v0.21.0 fails nodegit/nodegit#376

Closed

kkoopa mentioned this issue Feb 7, 2015

Support atom-shell 0.21 #276

Merged

aackerman mentioned this issue Feb 19, 2015

Looking for help to understand issues installing native modules electron/electron#1152

Closed

kkoopa closed this as completed Feb 24, 2015

kkoopa mentioned this issue Feb 24, 2015

Allow the use of Isolated V8 methods for node-0.12 #285

Closed

kkoopa mentioned this issue Apr 7, 2015

Add a flag for IOJS to distinguish versioning from NodeJS nodejs/node#1364

Closed

saper mentioned this issue Apr 29, 2015

libsass binding not found sass/node-sass#906

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

atom-shell causing problems #226

atom-shell causing problems #226

kkoopa commented Jan 14, 2015

bnoordhuis commented Jan 14, 2015

kkoopa commented Jan 14, 2015

bnoordhuis commented Jan 14, 2015

kkoopa commented Jan 14, 2015

bnoordhuis commented Jan 14, 2015

waTeim commented Jan 14, 2015

kkoopa commented Jan 16, 2015

waTeim commented Jan 16, 2015

kkoopa commented Jan 21, 2015

mathiask88 commented Jan 21, 2015

kkoopa commented Jan 21, 2015

waTeim commented Jan 21, 2015

zcbenz commented Jan 23, 2015

kkoopa commented Jan 23, 2015

bnoordhuis commented Jan 23, 2015

zcbenz commented Jan 23, 2015

atom-shell causing problems #226

atom-shell causing problems #226

Comments

kkoopa commented Jan 14, 2015

bnoordhuis commented Jan 14, 2015

kkoopa commented Jan 14, 2015

bnoordhuis commented Jan 14, 2015

kkoopa commented Jan 14, 2015

bnoordhuis commented Jan 14, 2015

waTeim commented Jan 14, 2015

kkoopa commented Jan 16, 2015

waTeim commented Jan 16, 2015

kkoopa commented Jan 21, 2015

mathiask88 commented Jan 21, 2015

kkoopa commented Jan 21, 2015

waTeim commented Jan 21, 2015

zcbenz commented Jan 23, 2015

kkoopa commented Jan 23, 2015

bnoordhuis commented Jan 23, 2015

zcbenz commented Jan 23, 2015