WiseLibs / better-sqlite3

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

v11 build error on Electron 31 #1199

Closed sethyuan closed 3 months ago

sethyuan commented 3 months ago

BTW, I'm on macOS 14.5, and this is the error I see when I tried to run electron-rebuild:

Building module: better-sqlite3, Completed: 0  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
  LIBTOOL-STATIC Release/sqlite3.a
  CXX(target) Release/obj.target/better_sqlite3/src/better_sqlite3.o
In file included from ../src/better_sqlite3.cpp:4:
./src/util/macros.lzz:31:69: error: no template named 'CopyablePersistentTraits' in namespace 'v8'; did you mean 'NonCopyablePersistentTraits'?
template <class T> using CopyablePersistent = v8::Persistent<T, v8::CopyablePersistentTraits<T>>;
                                                                ~~~~^~~~~~~~~~~~~~~~~~~~~~~~
                                                                    NonCopyablePersistentTraits
/Users/sethyuan/.electron-gyp/31.0.0/include/node/v8-persistent-handle.h:227:7: note: 'NonCopyablePersistentTraits' declared here
class NonCopyablePersistentTraits {
      ^
./src/util/macros.lzz:172:28: warning: 'SetAccessor' is deprecated: Use SetAccessor with Local<Name> instead [-Wdeprecated-declarations]
        recv->InstanceTemplate()->SetAccessor(
                                  ^
/Users/sethyuan/.electron-gyp/31.0.0/include/node/v8-template.h:1066:3: note: 'SetAccessor' has been explicitly marked deprecated here
  V8_DEPRECATED("Use SetAccessor with Local<Name> instead")
  ^
/Users/sethyuan/.electron-gyp/31.0.0/include/node/v8config.h:570:35: note: expanded from macro 'V8_DEPRECATED'
# define V8_DEPRECATED(message) [[deprecated(message)]]
                                  ^
./src/objects/database.lzz:180:21: warning: variable 'status' set but not used [-Wunused-but-set-variable]
                int status = sqlite3_db_config(db_handle, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 1, NULL);
                    ^
./src/util/custom-table.lzz:45:9: warning: missing field 'xIntegrity' initializer [-Wmissing-field-initializers]
        };
        ^
./src/util/custom-table.lzz:72:9: warning: missing field 'xIntegrity' initializer [-Wmissing-field-initializers]
        };
        ^
4 warnings and 1 error generated.
make: *** [Release/obj.target/better_sqlite3/src/better_sqlite3.o] Error 1
rm ba23eeee118cd63e16015df367567cb043fed872.intermediate
m4heshd commented 3 months ago

@JoshuaWise, @mceachen FYI https://github.com/m4heshd/better-sqlite3-multiple-ciphers/actions/runs/9458508204

neoxpert commented 3 months ago

v8::CopyablePersistentTraits has been removed at the end of March (https://github.com/v8/v8/commit/4683daaf774982b1aa0e46ae38f2bdf9c995c90d#) as it has been deprecated for about 2 years now.

To get it compiling again, changing

https://github.com/WiseLibs/better-sqlite3/blob/6acc3fcebe469969aa29319714b187a53ada0934/src/util/macros.lzz#L31

to

template <class T> using CopyablePersistent = v8::Global<T>;

should be enough. But I'm not soure about side effects of this change. At least the tests where run without issues on Node 18 and 20.

mceachen commented 3 months ago

See #1200

mceachen commented 3 months ago

Fixed by #1200 (just merged, will be released soon)

sethyuan commented 3 months ago

Fixed by #1200 (just merged, will be released soon)

@mceachen Hi, I'd like to know when will 11.1.0 be released to npm, I'm waiting for this build to upgrade my electron.