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

Electron 32 support #1225

Closed
Prinzhorn opened this issue Jul 14, 2024 · 9 comments
Closed

Electron 32 support #1225

Prinzhorn opened this issue Jul 14, 2024 · 9 comments

Comments

@Prinzhorn
Copy link
Contributor

Prinzhorn commented Jul 14, 2024

It's still in beta but I just happened to try to compile better-sqlite3. We're also not alone, e.g. xmppo/node-expat#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
Copy link
Contributor

neoxpert commented Jul 15, 2024

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
Copy link
Contributor Author

With all your changes what's left is

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

@neoxpert
Copy link
Contributor

neoxpert commented Jul 15, 2024

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

@Prinzhorn
Copy link
Contributor Author

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

@neoxpert
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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

@mceachen
Copy link
Member

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

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

No branches or pull requests

3 participants