WiseLibs / better-sqlite3

The fastest and simplest library for SQLite3 in Node.js.
MIT License
5.37k stars 394 forks source link

Electron 32 support #1225

Closed Prinzhorn closed 3 weeks ago

Prinzhorn commented 1 month ago

It's still in beta but I just happened to try to compile better-sqlite3. We're also not alone, e.g. https://github.com/xmppo/node-expat/issues/230

> electron-rebuild --build-from-source -f -w better-sqlite3

⠧ Building module: better-sqlite3, Completed: 0make: Entering directory '/src/better-sqlite3-test/electron-quick-start/node_modules/better-sqlite3/build'
  TOUCH ba23eeee118cd63e16015df367567cb043fed872.intermediate
  ACTION deps_sqlite3_gyp_locate_sqlite3_target_copy_builtin_sqlite3 ba23eeee118cd63e16015df367567cb043fed872.intermediate
  TOUCH Release/obj.target/deps/locate_sqlite3.stamp
  CC(target) Release/obj.target/sqlite3/gen/sqlite3/sqlite3.o
rm -f Release/obj.target/deps/sqlite3.a Release/obj.target/deps/sqlite3.a.ar-file-list; mkdir -p `dirname Release/obj.target/deps/sqlite3.a`
ar crs Release/obj.target/deps/sqlite3.a @Release/obj.target/deps/sqlite3.a.ar-file-list
  COPY Release/sqlite3.a
  CXX(target) Release/obj.target/better_sqlite3/src/better_sqlite3.o
In file included from /home/user/.electron-gyp/32.0.0-alpha.8/include/node/cppgc/common.h:8,
                 from /home/user/.electron-gyp/32.0.0-alpha.8/include/node/v8.h:23,
                 from /home/user/.electron-gyp/32.0.0-alpha.8/include/node/node.h:79,
                 from ./src/better_sqlite3.lzz:11,
                 from ../src/better_sqlite3.cpp:4:
/home/user/.electron-gyp/32.0.0-alpha.8/include/node/v8config.h:13:2: error: #error "C++20 or later required."
   13 | #error "C++20 or later required."
      |  ^~~~~
./src/util/macros.lzz:146:2: error: ‘v8::AccessorGetterCallback’ has not been declared
./src/util/macros.lzz:155:2: error: ‘v8::AccessorGetterCallback’ has not been declared
./src/util/macros.lzz: In function ‘void SetPrototypeGetter(v8::Isolate*, v8::Local<v8::External>, v8::Local<v8::FunctionTemplate>, const char*, int)’:
./src/util/macros.lzz:169:28: error: ‘class v8::ObjectTemplate’ has no member named ‘SetAccessor’
./src/better_sqlite3.lzz: At global scope:
/home/user/.electron-gyp/32.0.0-alpha.8/include/node/node.h:1257:7: warning: cast between incompatible function types from ‘void (*)(v8::Local<v8::Object>, v8::Local<v8::Value>, v8::Local<v8::Context>)’ to ‘node::addon_context_register_func’ {aka ‘void (*)(v8::Local<v8::Object>, v8::Local<v8::Value>, v8::Local<v8::Context>, void*)’} [-Wcast-function-type]
 1257 |       (node::addon_context_register_func) (regfunc),                  \
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/user/.electron-gyp/32.0.0-alpha.8/include/node/node.h:1275:3: note: in expansion of macro ‘NODE_MODULE_CONTEXT_AWARE_X’
 1275 |   NODE_MODULE_CONTEXT_AWARE_X(modname, regfunc, NULL, 0)
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/user/.electron-gyp/32.0.0-alpha.8/include/node/node.h:1306:3: note: in expansion of macro ‘NODE_MODULE_CONTEXT_AWARE’
 1306 |   NODE_MODULE_CONTEXT_AWARE(NODE_GYP_MODULE_NAME,                     \
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~
./src/better_sqlite3.lzz:67:1: note: in expansion of macro ‘NODE_MODULE_INIT’
./src/objects/database.lzz: In static member function ‘static v8::Local<v8::Function> Database::Init(v8::Isolate*, v8::Local<v8::External>)’:
./src/objects/database.lzz:17:35: error: invalid conversion from ‘void (*)(v8::Local<v8::String>, const v8::PropertyCallbackInfo<v8::Value>&)’ to ‘int’ [-fpermissive]
./src/util/macros.lzz:155:29: note:   initializing argument 5 of ‘void SetPrototypeGetter(v8::Isolate*, v8::Local<v8::External>, v8::Local<v8::FunctionTemplate>, const char*, int)’
./src/objects/database.lzz:18:35: error: invalid conversion from ‘void (*)(v8::Local<v8::String>, const v8::PropertyCallbackInfo<v8::Value>&)’ to ‘int’ [-fpermissive]
./src/util/macros.lzz:155:29: note:   initializing argument 5 of ‘void SetPrototypeGetter(v8::Isolate*, v8::Local<v8::External>, v8::Local<v8::FunctionTemplate>, const char*, int)’
./src/objects/statement.lzz: In static member function ‘static v8::Local<v8::Function> Statement::Init(v8::Isolate*, v8::Local<v8::External>)’:
./src/objects/statement.lzz:16:35: error: invalid conversion from ‘void (*)(v8::Local<v8::String>, const v8::PropertyCallbackInfo<v8::Value>&)’ to ‘int’ [-fpermissive]
./src/util/macros.lzz:155:29: note:   initializing argument 5 of ‘void SetPrototypeGetter(v8::Isolate*, v8::Local<v8::External>, v8::Local<v8::FunctionTemplate>, const char*, int)’
./src/util/custom-table.lzz: At global scope:
./src/util/custom-table.lzz:45:9: warning: missing initializer for member ‘sqlite3_module::xIntegrity’ [-Wmissing-field-initializers]
./src/util/custom-table.lzz:72:9: warning: missing initializer for member ‘sqlite3_module::xIntegrity’ [-Wmissing-field-initializers]
./src/util/data.lzz: In function ‘v8::Local<v8::Value> Data::GetValueJS(v8::Isolate*, sqlite3_stmt*, int, bool)’:
./src/util/data.lzz:73:92: warning: this statement may fall through [-Wimplicit-fallthrough=]
./src/util/data.lzz:73:197: note: here
./src/util/data.lzz: In function ‘v8::Local<v8::Value> Data::GetValueJS(v8::Isolate*, sqlite3_value*, bool)’:
./src/util/data.lzz:77:81: warning: this statement may fall through [-Wimplicit-fallthrough=]
./src/util/data.lzz:77:175: note: here
make: *** [better_sqlite3.target.mk:134: Release/obj.target/better_sqlite3/src/better_sqlite3.o] Error 1
rm ba23eeee118cd63e16015df367567cb043fed872.intermediate
make: Leaving directory '/src/better-sqlite3-test/electron-quick-start/node_modules/better-sqlite3/build'
Error: `make` failed with exit code: 2
    at ChildProcess.onExit (/src/better-sqlite3-test/electron-quick-start/node_modules/node-gyp/lib/build.js:203:23)
    at ChildProcess.emit (node:events:514:28)
    at ChildProcess._handle.onexit (node:internal/child_process:294:12)

✖ Rebuild Failed

An unhandled error occurred inside electron-rebuild
node-gyp failed to rebuild '/src/better-sqlite3-test/electron-quick-start/node_modules/better-sqlite3'

Error: node-gyp failed to rebuild '/src/better-sqlite3-test/electron-quick-start/node_modules/better-sqlite3'
    at ChildProcess.<anonymous> (/src/better-sqlite3-test/electron-quick-start/node_modules/@electron/rebuild/lib/module-type/node-gyp/node-gyp.js:118:24)
    at ChildProcess.emit (node:events:514:28)
    at ChildProcess._handle.onexit (node:internal/child_process:294:12)
neoxpert commented 1 month ago

Looks like AccessorGetterCallback has finally been removed from the api after having been marked as deprecated for some years now.

You can give it a try and change the c++ Standard from c++17 to c++ 20.

Also change the macro NODE_GETTER within macros.lzz from

#define NODE_GETTER(name) static void name(v8::Local<v8::String> _, const v8::PropertyCallbackInfo<v8::Value>& info)

to

#define NODE_GETTER(name) static void name(v8::Local<v8::Name> _, const v8::PropertyCallbackInfo<v8::Value>& info)

Also within macros.lzz replace AccessorGetterCallback with AccessorNameGetterCallback (2 occurrences).

Prinzhorn commented 1 month ago

With all your changes what's left is

/src/util/macros.lzz:169:28: error: ‘class v8::ObjectTemplate’ has no member named ‘SetAccessor’
neoxpert commented 1 month ago

Oh, yes. You might also replace SetAccessor with SetNativeDataProperty within macros.lzz.

Prinzhorn commented 1 month ago

It does indeed compile, thanks! Will you provide a PR in the future?

neoxpert commented 1 month ago

I actually already created a branch in my fork repo. But there are issues with setting c++20 option within the Ubuntu container, as it is not known by the used GCC / G++ version but named c++2a instead. Need to figure out how to adjust the build.yml and / or the bindings.gyp.

Prinzhorn commented 1 month ago

Fun fact I'm on Ubuntu (23.10), so I assume it's because the container is just outdated?

$ gcc --version
gcc (Ubuntu 13.2.0-4ubuntu3) 13.2.0
neoxpert commented 1 month ago

That seems to be the case. The runner-image documentation states that gcc 9.3.0 is preinstalled, which does not yet support the c++20 option. But newer versions already replaced the c++2a version with c++20, so using c++2a would cause issues :-/.

neoxpert commented 1 month ago

Well, the GCC and G++ where available within the Ubuntu 20 repo. I opened up a pull request.

mceachen commented 3 weeks ago

Thanks for the PR, @neoxpert ! #1226 just got merged.