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

build falis with node v12 #18

Closed
nodech opened this issue May 1, 2019 · 4 comments
Closed

build falis with node v12 #18

nodech opened this issue May 1, 2019 · 4 comments

Comments

@nodech
Copy link
Member

nodech commented May 1, 2019

It seems v8 api changed and bcrypto no longer compiles:

../src/aead.cc: In static member function ‘static void BAEAD::Init(v8::Local<v8::Object>&)’:
../src/aead.cc:39:68: error: no matching function for call to ‘v8::FunctionTemplate::GetFunction()’
   target->Set(Nan::New("AEAD").ToLocalChecked(), ctor->GetFunction());
                                                                    ^
In file included from /home/nod/.node-gyp/12.1.0/include/node/node.h:63,
                 from ../src/common.h:3,
                 from ../src/aead.cc:1:
/home/nod/.node-gyp/12.1.0/include/node/v8.h:5947:46: note: candidate: ‘v8::MaybeLocal<v8::Function> v8::FunctionTemplate::GetFunction(v8::Local<v8::Context>)’

Codebase is big to rewrite to NAPI, but probably worth it in the long run.

@chjj
Copy link
Member

chjj commented May 1, 2019

Shame on them for not showing a proper deprecation warning for this one. I thought we had this all ready.

Codebase is big to rewrite to NAPI, but probably worth it in the long run.

Something I've considered for a while along with the other rewrite to nettle. The problem I have with that is that it's impossible to do incrementally. It's all or nothing. So we couldn't even test the code until it's completely rewritten.

@nodech
Copy link
Member Author

nodech commented May 1, 2019

It seems Set/Get is will need to change as well.

../src/aead.cc: In static member function ‘static void BAEAD::Init(v8::Local<v8::Object>&)’:
../src/aead.cc:40:44: warning: ‘bool v8::Object::Set(v8::Local<v8::Value>, v8::Local<v8::Value>)’ is deprecated: Use maybe version [-Wdeprecated-declarations]
     Nan::GetFunction(ctor).ToLocalChecked());
                                            ^
In file included from /home/nd/.node-gyp/12.1.0/include/node/v8-internal.h:14,
                 from /home/nd/.node-gyp/12.1.0/include/node/v8.h:25,
                 from /home/nd/.node-gyp/12.1.0/include/node/node.h:63,
                 from ../src/common.h:3,
                 from ../src/aead.cc:1:
/home/nd/.node-gyp/12.1.0/include/node/v8.h:3359:26: note: declared here
                     bool Set(Local<Value> key, Local<Value> value));
                          ^~~

I think these apply to bcrypto, deprecated:

  • Local<Value> Get(uint32_t index)
  • bool Set(Local<Value> key, Local<Value> value)
  • bool Set(uint32_t index, Local<Value> value)

@chjj
Copy link
Member

chjj commented May 1, 2019

I'm going to build node 12 now and test this. If Set is deprecated on return values, we have a lot of work to do =/

@nodech
Copy link
Member Author

nodech commented May 1, 2019

It should be Nan::Set, I think it will handle the issue.
If there are no problems Nan::Set

@nodech nodech mentioned this issue May 1, 2019
chjj added a commit that referenced this issue May 1, 2019
@nodech nodech closed this as completed May 2, 2019
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