Support for Electron?

[bengotow]

Hey! Was just checking out better-sqlite3. We currently use node-sqlite3 in https://github.com/nylas/N1 and are having a really obscure bug that we think is related to it's async implementation. Wanted to try switching to better-sqlite3, but I'm running into this issue building it against Electron 1.4.1. Any ideas?

export npm_config_target=1.4.1
# The architecture of Electron, can be ia32 or x64.
export npm_config_arch=x64
export npm_config_target_arch=x64
# Download headers for Electron.
export npm_config_disturl=https://atom.io/download/atom-shell
# Tell node-pre-gyp that we are building for Electron.
export npm_config_runtime=electron
# Tell node-pre-gyp to build module from source code.
export npm_config_build_from_source=true
# Install all dependencies, and store cache to ~/.electron-gyp.
HOME=~/.electron-gyp npm install

npm install better-sqlite3

npm WARN prefer global node-gyp@3.4.0 should be installed with -g

> sqlite3@3.1.4 install /Users/bengotow/Nylas/N1/node_modules/better-sqlite3/node_modules/sqlite3
> node-pre-gyp install --fallback-to-build

  ACTION deps_sqlite3_gyp_action_before_build_target_unpack_sqlite_dep Release/obj/gen/sqlite-autoconf-3090100/sqlite3.c
  TOUCH Release/obj.target/deps/action_before_build.stamp
  CC(target) Release/obj.target/sqlite3/gen/sqlite-autoconf-3090100/sqlite3.o
Release/obj/gen/sqlite-autoconf-3090100/sqlite3.c:9704:26: warning: unused variable 'sqlite3one' [-Wunused-const-variable]
SQLITE_PRIVATE const int sqlite3one = 1;
                         ^
1 warning generated.
  LIBTOOL-STATIC Release/sqlite3.a
  CXX(target) Release/obj.target/node_sqlite3/src/database.o
../src/database.cc:132:18: warning: 'ForceSet' is deprecated [-Wdeprecated-declarations]
    info.This()->ForceSet(Nan::New("filename").ToLocalChecked(), info[0].As<String>(), ReadOnly);
                 ^
/Users/bengotow/.node-gyp/iojs-1.4.1/deps/v8/include/v8.h:2702:22: note: 'ForceSet' has been explicitly marked deprecated here
                bool ForceSet(Local<Value> key, Local<Value> value,
                     ^
../src/database.cc:133:18: warning: 'ForceSet' is deprecated [-Wdeprecated-declarations]
    info.This()->ForceSet(Nan::New("mode").ToLocalChecked(), Nan::New(mode), ReadOnly);
                 ^
/Users/bengotow/.node-gyp/iojs-1.4.1/deps/v8/include/v8.h:2702:22: note: 'ForceSet' has been explicitly marked deprecated here
                bool ForceSet(Local<Value> key, Local<Value> value,
                     ^
../src/database.cc:143:9: warning: unused variable 'status' [-Wunused-variable]
    int status = uv_queue_work(uv_default_loop(),
        ^
../src/database.cc:227:9: warning: unused variable 'status' [-Wunused-variable]
    int status = uv_queue_work(uv_default_loop(),
        ^
../src/database.cc:505:9: warning: unused variable 'status' [-Wunused-variable]
    int status = uv_queue_work(uv_default_loop(),
        ^
../src/database.cc:605:9: warning: unused variable 'status' [-Wunused-variable]
    int status = uv_queue_work(uv_default_loop(),
        ^
6 warnings generated.
  CXX(target) Release/obj.target/node_sqlite3/src/node_sqlite3.o
  CXX(target) Release/obj.target/node_sqlite3/src/statement.o
../src/statement.cc:103:18: warning: 'ForceSet' is deprecated [-Wdeprecated-declarations]
    info.This()->ForceSet(Nan::New("sql").ToLocalChecked(), sql, ReadOnly);
                 ^
/Users/bengotow/.node-gyp/iojs-1.4.1/deps/v8/include/v8.h:2702:22: note: 'ForceSet' has been explicitly marked deprecated here
                bool ForceSet(Local<Value> key, Local<Value> value,
                     ^
../src/statement.cc:118:9: warning: unused variable 'status' [-Wunused-variable]
    int status = uv_queue_work(uv_default_loop(),
        ^
../src/statement.cc:322:5: warning: unused variable 'status' [-Wunused-variable]
    STATEMENT_BEGIN(Bind);
    ^
../src/macros.h:125:9: note: expanded from macro 'STATEMENT_BEGIN'
    int status = uv_queue_work(uv_default_loop(),                              \
        ^
../src/statement.cc:370:5: warning: unused variable 'status' [-Wunused-variable]
    STATEMENT_BEGIN(Get);
    ^
../src/macros.h:125:9: note: expanded from macro 'STATEMENT_BEGIN'
    int status = uv_queue_work(uv_default_loop(),                              \
        ^
../src/statement.cc:438:5: warning: unused variable 'status' [-Wunused-variable]
    STATEMENT_BEGIN(Run);
    ^
../src/macros.h:125:9: note: expanded from macro 'STATEMENT_BEGIN'
    int status = uv_queue_work(uv_default_loop(),                              \
        ^
../src/statement.cc:504:5: warning: unused variable 'status' [-Wunused-variable]
    STATEMENT_BEGIN(All);
    ^
../src/macros.h:125:9: note: expanded from macro 'STATEMENT_BEGIN'
    int status = uv_queue_work(uv_default_loop(),                              \
        ^
../src/statement.cc:601:5: warning: unused variable 'status' [-Wunused-variable]
    STATEMENT_BEGIN(Each);
    ^
../src/macros.h:125:9: note: expanded from macro 'STATEMENT_BEGIN'
    int status = uv_queue_work(uv_default_loop(),                              \
        ^
../src/statement.cc:724:5: warning: unused variable 'status' [-Wunused-variable]
    STATEMENT_BEGIN(Reset);
    ^
../src/macros.h:125:9: note: expanded from macro 'STATEMENT_BEGIN'
    int status = uv_queue_work(uv_default_loop(),                              \
        ^
8 warnings generated.
  SOLINK_MODULE(target) Release/node_sqlite3.node
  COPY /Users/bengotow/Nylas/N1/node_modules/better-sqlite3/node_modules/sqlite3/lib/binding/electron-v1.4-darwin-x64/node_sqlite3.node
  TOUCH Release/obj.target/action_after_build.stamp

> better-sqlite3@1.2.1 install /Users/bengotow/Nylas/N1/node_modules/better-sqlite3
> if [ "$CI" = "true" ]; then node-gyp rebuild --debug; else node-gyp rebuild; fi;

  ACTION deps_sqlite3_gyp_action_before_build_target_unpack_sqlite_dep Release/obj/gen/sqlite-autoconf-3140100/sqlite3.c
  TOUCH Release/obj.target/deps/action_before_build.stamp
  CC(target) Release/obj.target/sqlite3/gen/sqlite-autoconf-3140100/sqlite3.o
  LIBTOOL-STATIC Release/sqlite3.a
  CXX(target) Release/obj.target/better_sqlite3/src/objects/int64/int64.o
  CXX(target) Release/obj.target/better_sqlite3/src/util/data.o
  CXX(target) Release/obj.target/better_sqlite3/src/objects/database/database.o
  CXX(target) Release/obj.target/better_sqlite3/src/objects/statement/statement.o
In file included from ../src/objects/statement/statement.cc:20:
../src/objects/statement/util.cc:11:48: error: no member named 'GetHiddenValue' in 'v8::Object'
                return v8::Local<v8::Object>::Cast(handle()->GetHiddenValue(Nan::EmptyString()));
                                                   ~~~~~~~~  ^
../src/objects/statement/util.cc:23:13: error: no member named 'SetHiddenValue' in 'v8::Object'
                handle()->SetHiddenValue(Nan::EmptyString(), namedParams);
                ~~~~~~~~  ^
2 errors generated.
make: *** [Release/obj.target/better_sqlite3/src/objects/statement/statement.o] Error 1
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/Users/bengotow/.nvm/versions/node/v6.2.2/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:276:23)
gyp ERR! stack     at emitTwo (events.js:106:13)
gyp ERR! stack     at ChildProcess.emit (events.js:191:7)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:204:12)
gyp ERR! System Darwin 15.4.0
gyp ERR! command "/Users/bengotow/.nvm/versions/node/v6.2.2/bin/node" "/Users/bengotow/.nvm/versions/node/v6.2.2/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /Users/bengotow/Nylas/N1/node_modules/better-sqlite3
gyp ERR! node -v v6.2.2
gyp ERR! node-gyp -v v3.3.1
gyp ERR! not ok 

[JoshuaWise]

It looks like you're using an incompatible version of V8. Do you know which version of V8 is being used in Electron 1.4.1?

[bengotow]

Looks like Electron is using V8 5.3.332.45. Here's the full print out I get from process.versions in our app, in case any of these are also relevant:

chrome:"53.0.2785.113"
electron:"1.4.0"
modules:"50"
node:"6.5.0"
uv:"1.9.1"
v8: "5.3.332.45"

Do you know how hard it'd be to update those couple references? I haven't migrated between V8 versions before, but it looks like the node-sqlite folks did recently so the changes may be the same.

[JoshuaWise]

Well, I could conditionally use V8's new Private interface instead of the depreciated HiddenValue interface, but the documentation for V8 5.3.332.45 states that the Private interface is experimental. I find that strange because, if they removed HiddenValue in that version, it can't be that experimental (as there are no other alternatives).

That said, I'm happy to go ahead and try it out. Give me a day to try it and test it. If it goes well, I'll publish a new patch and close this issue.

[bengotow]

Ahh cool—it looks like they removed HiddenValue (https://codereview.chromium.org/1942233002) in V8 5.2. I wonder when Private was added? Anyway—thanks for taking a look. If you get it going on a branch or something, I'm happy to plug it into N1. It's probably a pretty good stress test :-)

[bengotow]

(Just a heads up that I threw together a patch that resolves this on https://github.com/bengotow/better-sqlite3 — the issue we're having with node-sqlite is a pretty big one, so we're using better-sqlite3 to see if it's due to a Node 6 + node-sqlite issue. Will submit it as a PR once we've done some more testing.)

[JoshuaWise]

Great! I apologize for not getting to it first. I didn't have time before leaving for a vacation, but normally I try to be very responsive about issues like this. I'm glad you were able to resolve it. I look forward to hear the results of your testing

[Steviey]

Running into the same issue here. Any fix available?
I can't install any sqlite3 package at all, since every package I try stops with the gyp-horror.

[alexweber]

@Steviey there's a number of articles out there explaining how to install the sqlite3 + electron, you basically need to run electron-rebuild to recompile it against the electron headers and it should work. (confirmed for me with Electron 1.4.1 + SQlite3 on MacOS)

The issue with this package is a different one entirely, related to chromium V8 versions...

[JoshuaWise]

The fix is currently a WIP via #3.

[Steviey]

@alexweber unconfirmed on Win7

[bengotow]

Hey! If the patch above didn't fix it for you, it's most likely an issue with your build environment - the discussions in the node-sqlite3 issue tracker should still be helpful if you're using better-sqlite3. (On windows, I think you might need visual studio and some other things, but I'm not sure.)

[alexweber]

@Steviey oops, sorry didn't catch that.

[stemcc]

Edit: For the time being, I'm using @bengotow's patch on a fork of the latest better-sqlite3 master and everything seems to be working fine.

I'm also working with Electron and trying to get this plugin working. @bengotow, after @JoshuaWise cleaned up some sections of code on October 12 per this comment, is your patch for Electron still working for you?

It looks like you are still using your forked better-sqlite3 repo on Nylas/N1 -- is your plan to stay forked or to try merging these again?

Or does it make sense to push the changes needed for Electron into a permanent fork like better-sqlite3-electron, that can stay updated with @JoshuaWise's repo while the Electron community can tweak the code as needed?

[manikantag]

#2 (comment) fix by @bengotow working for me.

I m using "better-sqlite3": "bengotow/better-sqlite3#a888061ad334c76d2db4c06554c90785cc6e7cce" in package.json.

Taken from nylas package.json.

[manikantag]

Hi @bengotow, I heard that sqlite is working quite well in N1 email client, in a video. And I see that you are using better-sqlite3 instead of sqlite3 package. I m facing some itermittent issue with sqlite3 (issue: TryGhost/node-sqlite3#742).

How is your experience with better-sqlite3 regarding stability & performance of better-sqlite3? I m interested because better-sqlite3 has sync api and claims it has much more performance than sqlite3.

[JoshuaWise]

Electron is now supported by better-sqlite3 v1.4.0 👍