Skip to content

first attempts at supporting v0.12 #4

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

davidmarkclements
Copy link

So this doesn't work with Node v0.12 - I've made attempts at fixing but out of my depth in C++ (willing to learn though!).

So it seems the first step would be to upgrade nan to 1.6 - this actually works great (with some deprecation warnings.)

The next problem was atomicops.h - in v8 3.27 (node v0.12 is on v8 3.28)
this was relocated to deps/v8/src/base, and then atomicops.h is expecting deps/v8.

So that's why extra paths are added to bindings.gyp.

Then I hit these errors:

In file included from ../src/v8_flags.cc:111:
../src/v8_flag_definitions.h:4:1: error: C++ requires a type specifier for all declarations
DEFINE_BOOL(use_strict, false, "enforce strict mode")
^~~~~~~~~~~
../src/v8_flag_definitions.h:4:13: error: use of undeclared identifier 'use_strict'
DEFINE_BOOL(use_strict, false, "enforce strict mode")
            ^
../src/v8_flag_definitions.h:4:54: error: expected ';' after top level declarator
DEFINE_BOOL(use_strict, false, "enforce strict mode")

And now I do not know what to do...

I notice scripts/preinstall.js performs some pre-processing on the flags definition file...

Maybe there's been a refactor of v8_flag_definitions.h ...

... help?

@thlorenz
Copy link
Owner

Thanks for taking the time to look into this. I didn't expect this to be so complex to do :(
NaN upgrade and extra path are fine.

WRT errors, I'm not sure if this is due to the fact that node 0.12 uses different C++ flags to support C11 or if something changed about the v8_flag_definitions.h.
When I wrote this originally I made sure that things worked with these.

So maybe we could diff this one against the newer one?
That may also reveal a change that broke the preinstall script since that highly depends on the structure of that file.

LMK if you need any help I'd love to have this work for node 0.12 and io.js.
Thanks.

@davidmarkclements
Copy link
Author

Yah I totally need help, I'm not proficient in C - what if we break this into investigative tasks and split them between us?

@thlorenz
Copy link
Owner

I cannot guarantee you that I'll have time to look into this seriously any time soon.
How would you like to split it?

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

Successfully merging this pull request may close these issues.

2 participants