bcoin-org / bcrypto

JS crypto library
Other
99 stars 41 forks source link

build falis with node v12 #18

Closed nodech closed 5 years ago

nodech commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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:

chjj commented 5 years ago

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 commented 5 years ago

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